yochiyochirb/kajaeru

投票すると別の人が投票したことになってしまう

Closed this issue · 1 comments

Vote モデルは voter_idcandidate_id というフィールドを持ち、その関連を見ると、そのフィールド名から想像できるものとは違い、VoterCandidate つまり Role ではなく、Member モデルに対して belongs_to の関連を持っている。

class Vote < ActiveRecord::Base

...

  belongs_to :candidate, class_name: 'Member'
  belongs_to :voter, class_name: 'Member'
end

一方、この Vote がどこで作られるかというと、VotesControllercreate アクションとなっている。

  def create
    @vote = Vote.create!(vote_params.merge(voter_id: current_user.id))
    redirect_to @vote
  end

上記の通り、そのとき、voter_idcurrent_user.id を入れているが、current_user は以下のように Voter のインスタンスである。

  def current_user
...
    case controller_name
    when 'votes', 'members'
      Voter.find_by(member_id: session[:user_id])
...
    end
  end

したがって、Vote.create!voter_id に指定されるのは Voterid となり、これは Member の関連を持つ仕様と合致していない。

その結果、別の人が投票したことになってしまう。例えば、以下の VoteMemberid が 1 の alice が投票したものだが、alice の Voterid である 2 が入ってしまっているので、bob が投票したことになっている。

[3] pry(main)> vote.voter
=> #<Member:0x007fa822b7c790
 id: 2,
 nickname: "bob",
 provider: "github",
 uid: "22222",
 image: "http://pic.prepics-cdn.com/9ece34e247cc4/54052407.jpeg",
 created_at: Tue, 08 Mar 2016 07:07:31 UTC +00:00,
 updated_at: Tue, 08 Mar 2016 07:07:31 UTC +00:00>

仕様から見直した方がいい気もするけど、一旦修正するには、Vote.create! のところで Memberidvoter_id に指定する様にすればいいと思う。

ありがとうございます。これちょうど今朝、 #128 のブランチを手元でいじってたときに「あれ???」って思ってました。。
自分でまるごとリファクタしておきながら言うのも無責任な気がしますが、今の Kajaeru はモデルの構造がわかりづらすぎてすごく扱いにくいなと思っているので、わたしも本当なら根本から見直したいなと思います。