新しく画像を投稿すると全ての他の投稿にも新しい投稿画像が反映されてしまう

Question

railsでツイッター系のサイトの開発をしているのですが、画像を投稿すると他の全ての投稿にも新しい画像が反映されてしまいます。
どこかで間違えがあると思いますので教えていただければ幸いです。

class UsersController < ApplicationController
  def index
    @users = User.all
  end
  
  def show
    @user = User.find_by(id: params[:id])
  end
  
  def new
    @user = User.new
  end
  
  def create
    @user = User.new(
      name: params[:name],
      email: params[:email],
      content: params[:content],
      image_name: "noimage.jpg"
      )
      
      if params[:image]
      @user.image_name = "#{@user.id}.jpg"
      image = params[:image]
      File.binwrite("public/user_images/#{@user.image_name}", image.read)
      end
      
      if @user.save
        flash[:notice] = "ユーザー登録が完了しました"
        redirect_to("/users/#{@user.id}")
      else
        render("users/new")
      end
  end
  
  def edit
    @user = User.find_by(id: params[:id])
  end
  
  def update
    @user = User.find_by(id: params[:id])
    @user.name = params[:name]
    @user.email = params[:email]
    @user.content = params[:content]
    
    if params[:image]
      @user.image_name = "#{@user.id}.jpg"
      image = params[:image]
      File.binwrite("public/user_images/#{@user.image_name}", image.read)
    end
    
    if @user.save
      flash[:notice] = "ユーザー情報を編集しました"
      redirect_to("/users/#{@user.id}")
    else
      render("users/edit")
    end
  end
end

index.html.erb↓

<div class="row">
        <% @users.each do |user| %>
        <div class="col-md-6">
          <div class="panel panel-default">
            <div class="panel-heading">
              <h3 class="panel-title text-center"><%= link_to(user.name, "/users/#{user.id}") %></h3>
            </div>
            <%# ここはeach文の@postsがあるので@post.image_nameではなくpost.image_nameでいい%>
            <img src="<%= "/user_images/#{user.image_name}" %>" alt="" width="265" height="318" class="img-thumbnail img-post">
           
          
            <button type="button" class="btn btn-warning"><%= link_to("詳細", "/users/#{user.id}") %></button>
          </div>
        </div>
        <% end %>
      </div>

new.html.erb↓

<div class="row">
    <div class="col-sm-8 col-sm-push-4">
      <h2 class="text-center"><strong><em> 研究者として登録</em></strong></h2>
        <%= form_tag("/users/create", {multipart: true}) do %>
        	<div class="form-group">
        		<label for="Inputimage">写真</label>
             <input type="file" class="form-control" id="Inputimag" name="image">
         </div>
        	<button  class="btn btn-warning" type="submit" value="投稿">登録</button>
    <% end %>
    </div>

Answer 1

    @user = User.new(
      name: params[:name],
      email: params[:email],
      content: params[:content],
      image_name: "noimage.jpg"
      )

      if params[:image]
      @user.image_name = "#{@user.id}.jpg"
      image = params[:image]
      File.binwrite("public/user_images/#{@user.image_name}", image.read)
      end

newの段階ではidはnullなので、image_nameは全て同じ値になりそうです。
今の作りを変えないなら、ifの前に@user.saveをするかuniqueな値を発行する処理(generate_image_name)を入れると良いかと思います。

class User < ApplicationRecord
  def generate_image_name
    self.image_name = loop do
      random_token = SecureRandom.urlsafe_base64(nil, false) + ".jpg"
      break random_token unless self.class.exists?(image_name: random_token)
    end
  end
end

Comments

[renren643]

回答ありがとうございます。
「ifの前に@user.saveをする」とは if params[:image]の上に@user.saveを記述するだけですか？

[akichim21]

unless @user.save
render("users/new")
end

ifの前に上記を追加するか

if params[:image]
@user.generate_image_name
image = params[:image]
File.binwrite("public/user_images/#{@user.image_name}", image.read)
end
ifの中を上記のようにして、Userクラスにgenerate_image_nameを加えれば良いかと思います。

[renren643]

createアクション内で以下のようにすると解決しました。ありがとうございました。
unless @user.save
render("users/new")
end

if params[:image]
@user.image_name = "#{@user.id}.jpg"
image = params[:image]
File.binwrite("public/user_images/#{@user.image_name}", image.read)
end

しかし、何故上記のようにするとうまくいくのかわかりません。差し支えなければ教えてください

[akichim21]

User.newの段階ではidはnullの状態で、image_nameが全て同じ値になるからです。
@user.saveするとDBに保存され、id = 1,2,3とユニークな値になるため解決します。

[renren643]

@user.saveが成功すると、（それに伴い固有のidが付与されるため、それに基づく画像が）DBに保存され、失敗するとusers/newにレンダーされます、ということでよろしいでしょうか？

[akichim21]

そうです！

[renren643]

ありがとうございます！

Answer 2

そもそも、UserモデルとImageモデルを分けるべきなのではないですか？
こちらのコードを読む限り、Userにつき一つの画像しか保持できないよう作りになっているように見えます。

UserとUserが投稿した画像が1対多の関係になるリレーションを構築すべきでしょう。

大前提として、画像を投稿する度にpublic/user_images/配下に画像ファイルが増えていくはずですが、増えていくのは確認できていますでしょうか？

Comments

[renren643]

回答ありがとうございます。
はい、１対多とはなっていません。
progateというサイトを見ながら、シナリオ通りそのまま作ったのでそういうことまで気が配れませんでした。
１対多にするにはUserモデルとImageモデルを分け、後から、その両者を紐付けるというやり方でしょうか？

>画像を投稿する度にpublic/user_images/配下に画像ファイルが増えていくはずですが、増えていくのは確認できていますでしょうか？

これが増えていないんです。その理由がわかりません。

[mtdsnsk]

はい、同じ名前で画像が作成されているため、画像は上書き保存されます。そのため、増えないのが正常の動作となります。

まずは、画像を追加してDBに貯めていくというシンプルな機能を作成し、そのあと、どうやったらUserと画像を紐づけられるかを考えた方が近道だと思います。

最終的には画像パスとuserのidが入った画像テーブルができるはずです。