Refactoring: Move 'vote' action to Votes Controllers
As far as possible I think the code is clearer if we use CRUD actions rather than custom actions. This will make it easier to add the action to remove votes in the next commit. Note that we are adding this line as we need to validate it that a vote can be created on a debate by the current user: ```authorize! :create, Vote.new(voter: current_user, votable: @debate)``` We have done it this way and not with the following code as you might expect, as this way two votes are created instead of one. ```load_and_authorize_resource through: :debate, through_association: :votes_for``` This line tries to load the resource @debate and through the association "votes_for" it tries to create a new vote associated to that debate. Therefore a vote is created when trying to authorise the resource and then another one in the create action, when calling @debate.vote_by (which is called by @debate.register_vote).
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
class Legislation::Proposals::VotesComponent < ApplicationComponent
|
||||
attr_reader :proposal
|
||||
delegate :current_user, :link_to_verify_account, to: :helpers
|
||||
delegate :current_user, :link_to_verify_account, :can?, to: :helpers
|
||||
|
||||
def initialize(proposal)
|
||||
@proposal = proposal
|
||||
@@ -9,7 +9,7 @@ class Legislation::Proposals::VotesComponent < ApplicationComponent
|
||||
private
|
||||
|
||||
def can_vote?
|
||||
proposal.votable_by?(current_user)
|
||||
can?(:create, proposal.votes_for.new(voter: current_user))
|
||||
end
|
||||
|
||||
def cannot_vote_text
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
<div class="in-favor-against">
|
||||
<div class="in-favor">
|
||||
<%= button_to polymorphic_path(votable, action: :vote, value: "yes"),
|
||||
<%= button_to vote_in_favor_against_path("yes"),
|
||||
title: t("votes.agree"),
|
||||
"aria-label": agree_aria_label,
|
||||
"aria-pressed": pressed?("yes"),
|
||||
@@ -12,7 +12,7 @@
|
||||
</div>
|
||||
|
||||
<div class="against">
|
||||
<%= button_to polymorphic_path(votable, action: :vote, value: "no"),
|
||||
<%= button_to vote_in_favor_against_path("no"),
|
||||
title: t("votes.disagree"),
|
||||
"aria-label": disagree_aria_label,
|
||||
"aria-pressed": pressed?("no"),
|
||||
|
||||
@@ -26,4 +26,12 @@ class Shared::InFavorAgainstComponent < ApplicationComponent
|
||||
false
|
||||
end
|
||||
end
|
||||
|
||||
def vote_in_favor_against_path(value)
|
||||
if votable.class.name == "Debate"
|
||||
debate_votes_path(votable, value: value)
|
||||
else
|
||||
legislation_process_proposal_votes_path(votable.process, votable, value: value)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
15
app/controllers/debates/votes_controller.rb
Normal file
15
app/controllers/debates/votes_controller.rb
Normal file
@@ -0,0 +1,15 @@
|
||||
module Debates
|
||||
class VotesController < ApplicationController
|
||||
before_action :authenticate_user!
|
||||
load_and_authorize_resource :debate
|
||||
|
||||
def create
|
||||
authorize! :create, Vote.new(voter: current_user, votable: @debate)
|
||||
@debate.register_vote(current_user, params[:value])
|
||||
|
||||
respond_to do |format|
|
||||
format.js { render :show }
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -28,10 +28,6 @@ class DebatesController < ApplicationController
|
||||
redirect_to debate_path(@debate), status: :moved_permanently if request.path != debate_path(@debate)
|
||||
end
|
||||
|
||||
def vote
|
||||
@debate.register_vote(current_user, params[:value])
|
||||
end
|
||||
|
||||
def unmark_featured
|
||||
@debate.update!(featured_at: nil)
|
||||
redirect_to debates_path
|
||||
|
||||
18
app/controllers/legislation/proposals/votes_controller.rb
Normal file
18
app/controllers/legislation/proposals/votes_controller.rb
Normal file
@@ -0,0 +1,18 @@
|
||||
module Legislation
|
||||
module Proposals
|
||||
class VotesController < ApplicationController
|
||||
before_action :authenticate_user!
|
||||
load_and_authorize_resource :process, class: "Legislation::Process"
|
||||
load_and_authorize_resource :proposal, class: "Legislation::Proposal", through: :process
|
||||
|
||||
def create
|
||||
authorize! :create, Vote.new(voter: current_user, votable: @proposal)
|
||||
@proposal.vote_by(voter: current_user, vote: params[:value])
|
||||
|
||||
respond_to do |format|
|
||||
format.js { render :show }
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -37,10 +37,6 @@ class Legislation::ProposalsController < Legislation::BaseController
|
||||
end
|
||||
end
|
||||
|
||||
def vote
|
||||
@proposal.register_vote(current_user, params[:value])
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def proposal_params
|
||||
|
||||
@@ -81,14 +81,15 @@ module Abilities
|
||||
can [:create, :destroy], DirectUpload
|
||||
|
||||
unless user.organization?
|
||||
can :vote, Debate
|
||||
can :create, ActsAsVotable::Vote, voter_id: user.id, votable_type: "Debate"
|
||||
can :vote, Comment
|
||||
end
|
||||
|
||||
if user.level_two_or_three_verified?
|
||||
can :vote, Proposal, &:published?
|
||||
|
||||
can :vote, Legislation::Proposal
|
||||
can :create, ActsAsVotable::Vote, voter_id: user.id, votable_type: "Legislation::Proposal"
|
||||
|
||||
can :create, Legislation::Answer
|
||||
|
||||
can :create, Budget::Investment, budget: { phase: "accepting" }
|
||||
|
||||
@@ -286,7 +286,7 @@ class Budget
|
||||
def permission_problem(user)
|
||||
return :not_logged_in unless user
|
||||
return :organization if user.organization?
|
||||
return :not_verified unless user.can?(:create, ActsAsVotable::Vote)
|
||||
return :not_verified unless user.level_two_or_three_verified?
|
||||
|
||||
nil
|
||||
end
|
||||
|
||||
@@ -108,14 +108,6 @@ class Legislation::Proposal < ApplicationRecord
|
||||
author_id == user.id && editable?
|
||||
end
|
||||
|
||||
def votable_by?(user)
|
||||
user&.level_two_or_three_verified?
|
||||
end
|
||||
|
||||
def register_vote(user, vote_value)
|
||||
vote_by(voter: user, vote: vote_value) if votable_by?(user)
|
||||
end
|
||||
|
||||
def code
|
||||
"#{Setting["proposal_code_prefix"]}-#{created_at.strftime("%Y-%m")}-#{id}"
|
||||
end
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
resources :debates do
|
||||
member do
|
||||
post :vote
|
||||
put :flag
|
||||
put :unflag
|
||||
put :mark_featured
|
||||
@@ -11,4 +10,6 @@ resources :debates do
|
||||
get :suggest
|
||||
put "recommendations/disable", only: :index, controller: "debates", action: :disable_recommendations
|
||||
end
|
||||
|
||||
resources :votes, controller: "debates/votes", only: :create
|
||||
end
|
||||
|
||||
@@ -16,13 +16,14 @@ namespace :legislation do
|
||||
|
||||
resources :proposals, except: [:index] do
|
||||
member do
|
||||
post :vote
|
||||
put :flag
|
||||
put :unflag
|
||||
end
|
||||
collection do
|
||||
get :suggest
|
||||
end
|
||||
|
||||
resources :votes, controller: "proposals/votes", only: :create
|
||||
end
|
||||
|
||||
resources :draft_versions, only: [:show] do
|
||||
|
||||
@@ -6,6 +6,22 @@ describe Shared::InFavorAgainstComponent do
|
||||
let(:user) { create(:user) }
|
||||
|
||||
describe "Agree and disagree buttons" do
|
||||
it "can create a vote when the user has not yet voted" do
|
||||
sign_in user
|
||||
|
||||
render_inline component
|
||||
|
||||
page.find(".in-favor") do |in_favor_block|
|
||||
expect(in_favor_block).to have_css "form[action*='votes'][method='post']"
|
||||
expect(in_favor_block).not_to have_css "input[name='_method']", visible: :all
|
||||
end
|
||||
|
||||
page.find(".against") do |against_block|
|
||||
expect(against_block).to have_css "form[action*='votes'][method='post']"
|
||||
expect(against_block).not_to have_css "input[name='_method']", visible: :all
|
||||
end
|
||||
end
|
||||
|
||||
it "does not include result percentages" do
|
||||
create(:vote, votable: debate)
|
||||
sign_in(user)
|
||||
|
||||
43
spec/controllers/debates/votes_controller_spec.rb
Normal file
43
spec/controllers/debates/votes_controller_spec.rb
Normal file
@@ -0,0 +1,43 @@
|
||||
require "rails_helper"
|
||||
|
||||
describe Debates::VotesController do
|
||||
describe "POST create" do
|
||||
it "does not authorize unauthenticated users" do
|
||||
debate = create(:debate)
|
||||
|
||||
post :create, xhr: true, params: { debate_id: debate.id, value: "yes" }
|
||||
|
||||
expect(response).to be_unauthorized
|
||||
end
|
||||
|
||||
it "redirects unauthenticated users without JavaScript to the sign in page" do
|
||||
debate = create(:debate)
|
||||
|
||||
post :create, params: { debate_id: debate.id, value: "yes" }
|
||||
|
||||
expect(response).to redirect_to new_user_session_path
|
||||
end
|
||||
|
||||
describe "Vote with too many anonymous votes" do
|
||||
it "allows vote if user is allowed" do
|
||||
Setting["max_ratio_anon_votes_on_debates"] = 100
|
||||
debate = create(:debate)
|
||||
sign_in create(:user)
|
||||
|
||||
expect do
|
||||
post :create, xhr: true, params: { debate_id: debate.id, value: "yes" }
|
||||
end.to change { debate.reload.votes_for.size }.by(1)
|
||||
end
|
||||
|
||||
it "does not allow voting if user is not allowed" do
|
||||
Setting["max_ratio_anon_votes_on_debates"] = 0
|
||||
debate = create(:debate, cached_votes_total: 1000)
|
||||
sign_in create(:user)
|
||||
|
||||
expect do
|
||||
post :create, xhr: true, params: { debate_id: debate.id, value: "yes" }
|
||||
end.not_to change { debate.reload.votes_for.size }
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -37,28 +37,6 @@ describe DebatesController do
|
||||
end
|
||||
end
|
||||
|
||||
describe "Vote with too many anonymous votes" do
|
||||
it "allows vote if user is allowed" do
|
||||
Setting["max_ratio_anon_votes_on_debates"] = 100
|
||||
debate = create(:debate)
|
||||
sign_in create(:user)
|
||||
|
||||
expect do
|
||||
post :vote, xhr: true, params: { id: debate.id, value: "yes" }
|
||||
end.to change { debate.reload.votes_for.size }.by(1)
|
||||
end
|
||||
|
||||
it "does not allow vote if user is not allowed" do
|
||||
Setting["max_ratio_anon_votes_on_debates"] = 0
|
||||
debate = create(:debate, cached_votes_total: 1000)
|
||||
sign_in create(:user)
|
||||
|
||||
expect do
|
||||
post :vote, xhr: true, params: { id: debate.id, value: "yes" }
|
||||
end.not_to change { debate.reload.votes_for.size }
|
||||
end
|
||||
end
|
||||
|
||||
describe "PUT mark_featured" do
|
||||
it "ignores query parameters" do
|
||||
debate = create(:debate)
|
||||
|
||||
@@ -0,0 +1,40 @@
|
||||
require "rails_helper"
|
||||
|
||||
describe Legislation::Proposals::VotesController do
|
||||
let(:legislation_process) { create(:legislation_process) }
|
||||
let(:proposal) { create(:legislation_proposal, process: legislation_process) }
|
||||
|
||||
describe "POST create" do
|
||||
let(:vote_params) do
|
||||
{ process_id: legislation_process.id, legislation_proposal_id: proposal.id, value: "yes" }
|
||||
end
|
||||
|
||||
it "does not authorize unauthenticated users" do
|
||||
post :create, xhr: true, params: vote_params
|
||||
|
||||
expect(response).to be_unauthorized
|
||||
end
|
||||
|
||||
it "redirects unauthenticated users without JavaScript to the sign in page" do
|
||||
post :create, params: vote_params
|
||||
|
||||
expect(response).to redirect_to new_user_session_path
|
||||
end
|
||||
|
||||
it "allows vote if user is level_two_or_three_verified" do
|
||||
sign_in create(:user, :level_two)
|
||||
|
||||
expect do
|
||||
post :create, xhr: true, params: vote_params
|
||||
end.to change { proposal.reload.votes_for.size }.by(1)
|
||||
end
|
||||
|
||||
it "does not allow voting if user is not level_two_or_three_verified" do
|
||||
sign_in create(:user)
|
||||
|
||||
expect do
|
||||
post :create, xhr: true, params: vote_params
|
||||
end.not_to change { proposal.reload.votes_for.size }
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -48,7 +48,6 @@ describe Abilities::Administrator do
|
||||
|
||||
it { should be_able_to(:index, Debate) }
|
||||
it { should be_able_to(:show, debate) }
|
||||
it { should be_able_to(:vote, debate) }
|
||||
|
||||
it { should be_able_to(:index, Proposal) }
|
||||
it { should be_able_to(:show, proposal) }
|
||||
|
||||
@@ -7,6 +7,7 @@ describe Abilities::Common do
|
||||
let(:geozone) { create(:geozone) }
|
||||
|
||||
let(:user) { create(:user, geozone: geozone) }
|
||||
let(:another_user) { create(:user) }
|
||||
|
||||
let(:debate) { create(:debate) }
|
||||
let(:comment) { create(:comment) }
|
||||
@@ -15,6 +16,7 @@ describe Abilities::Common do
|
||||
let(:own_comment) { create(:comment, author: user) }
|
||||
let(:own_proposal) { create(:proposal, author: user) }
|
||||
let(:own_legislation_proposal) { create(:legislation_proposal, author: user) }
|
||||
let(:legislation_proposal) { create(:legislation_proposal) }
|
||||
|
||||
let(:accepting_budget) { create(:budget, :accepting) }
|
||||
let(:reviewing_budget) { create(:budget, :reviewing) }
|
||||
@@ -73,7 +75,8 @@ describe Abilities::Common do
|
||||
|
||||
it { should be_able_to(:index, Debate) }
|
||||
it { should be_able_to(:show, debate) }
|
||||
it { should be_able_to(:vote, debate) }
|
||||
it { should be_able_to(:create, user.votes.build(votable: debate)) }
|
||||
it { should_not be_able_to(:create, another_user.votes.build(votable: debate)) }
|
||||
|
||||
it { should be_able_to(:show, user) }
|
||||
it { should be_able_to(:edit, user) }
|
||||
@@ -137,20 +140,16 @@ describe Abilities::Common do
|
||||
end
|
||||
|
||||
describe "follows" do
|
||||
let(:other_user) { create(:user) }
|
||||
|
||||
it { should be_able_to(:create, build(:follow, :followed_proposal, user: user)) }
|
||||
it { should_not be_able_to(:create, build(:follow, :followed_proposal, user: other_user)) }
|
||||
it { should_not be_able_to(:create, build(:follow, :followed_proposal, user: another_user)) }
|
||||
|
||||
it { should be_able_to(:destroy, create(:follow, :followed_proposal, user: user)) }
|
||||
it { should_not be_able_to(:destroy, create(:follow, :followed_proposal, user: other_user)) }
|
||||
it { should_not be_able_to(:destroy, create(:follow, :followed_proposal, user: another_user)) }
|
||||
end
|
||||
|
||||
describe "other users" do
|
||||
let(:other_user) { create(:user) }
|
||||
|
||||
it { should be_able_to(:show, other_user) }
|
||||
it { should_not be_able_to(:edit, other_user) }
|
||||
it { should be_able_to(:show, another_user) }
|
||||
it { should_not be_able_to(:edit, another_user) }
|
||||
end
|
||||
|
||||
describe "editing debates" do
|
||||
@@ -182,6 +181,18 @@ describe Abilities::Common do
|
||||
it { should_not be_able_to(:edit, own_legislation_proposal) }
|
||||
it { should_not be_able_to(:update, own_legislation_proposal) }
|
||||
|
||||
describe "vote legislation proposal" do
|
||||
context "when user is not level_two_or_three_verified" do
|
||||
it { should_not be_able_to(:create, user.votes.build(votable: legislation_proposal)) }
|
||||
end
|
||||
|
||||
context "when user is level_two_or_three_verified" do
|
||||
before { user.update(level_two_verified_at: Date.current) }
|
||||
it { should be_able_to(:create, user.votes.build(votable: legislation_proposal)) }
|
||||
it { should_not be_able_to(:create, another_user.votes.build(votable: legislation_proposal)) }
|
||||
end
|
||||
end
|
||||
|
||||
describe "proposals dashboard" do
|
||||
it { should be_able_to(:dashboard, own_proposal) }
|
||||
it { should_not be_able_to(:dashboard, proposal) }
|
||||
|
||||
@@ -25,7 +25,6 @@ describe Abilities::Moderator do
|
||||
|
||||
it { should be_able_to(:index, Debate) }
|
||||
it { should be_able_to(:show, debate) }
|
||||
it { should be_able_to(:vote, debate) }
|
||||
|
||||
it { should be_able_to(:index, Proposal) }
|
||||
it { should be_able_to(:show, proposal) }
|
||||
|
||||
@@ -14,7 +14,7 @@ describe "Abilities::Organization" do
|
||||
|
||||
it { should be_able_to(:index, Debate) }
|
||||
it { should be_able_to(:show, debate) }
|
||||
it { should_not be_able_to(:vote, debate) }
|
||||
it { should_not be_able_to(:create, user.votes.build(votable: debate)) }
|
||||
|
||||
it { should be_able_to(:index, Proposal) }
|
||||
it { should be_able_to(:show, proposal) }
|
||||
|
||||
Reference in New Issue
Block a user