From f87a332c3eb3e07abd916cac5bdf14e0e8e24dd2 Mon Sep 17 00:00:00 2001 From: taitus Date: Wed, 20 Sep 2023 16:08:44 +0200 Subject: [PATCH] Refactoring: Move 'vote' action to Comments::VotesControllers 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 comment by the current user: ```authorize! :create, Vote.new(voter: current_user, votable: @comment)``` 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: :comment, through_association: :votes_for``` This line tries to load the resource @comment 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 @comment.vote. --- .../comments/votes_component.html.erb | 8 +++--- app/components/comments/votes_component.rb | 4 +++ app/controllers/comments/votes_controller.rb | 26 +++++++++++++++++++ app/controllers/comments_controller.rb | 9 ++----- app/models/abilities/common.rb | 2 +- .../{vote.js.erb => votes/show.js.erb} | 0 config/routes/comment.rb | 3 ++- .../comments/votes_controller_spec.rb | 15 +++++++++++ spec/models/abilities/common_spec.rb | 3 ++- spec/models/abilities/organization_spec.rb | 3 ++- 10 files changed, 58 insertions(+), 15 deletions(-) create mode 100644 app/controllers/comments/votes_controller.rb rename app/views/comments/{vote.js.erb => votes/show.js.erb} (100%) create mode 100644 spec/controllers/comments/votes_controller_spec.rb diff --git a/app/components/comments/votes_component.html.erb b/app/components/comments/votes_component.html.erb index 155ba9ae4..47d23c276 100644 --- a/app/components/comments/votes_component.html.erb +++ b/app/components/comments/votes_component.html.erb @@ -3,9 +3,9 @@  |  - <%= button_to vote_comment_path(comment, value: "yes"), + <%= button_to vote_in_favor_against_path("yes"), method: "post", - remote: can?(:vote, comment), + remote: can?(:create, comment.votes_for.new(voter: current_user)), "aria-pressed": pressed?("yes"), title: t("votes.agree") do %> <%= t("votes.agree") %> @@ -14,9 +14,9 @@ - <%= button_to vote_comment_path(comment, value: "no"), + <%= button_to vote_in_favor_against_path("no"), method: "post", - remote: can?(:vote, comment), + remote: can?(:create, comment.votes_for.new(voter: current_user)), "aria-pressed": pressed?("no"), title: t("votes.disagree") do %> <%= t("votes.disagree") %> diff --git a/app/components/comments/votes_component.rb b/app/components/comments/votes_component.rb index 701b402f9..9b0dc94dc 100644 --- a/app/components/comments/votes_component.rb +++ b/app/components/comments/votes_component.rb @@ -16,4 +16,8 @@ class Comments::VotesComponent < ApplicationComponent false end end + + def vote_in_favor_against_path(value) + comment_votes_path(comment, value: value) + end end diff --git a/app/controllers/comments/votes_controller.rb b/app/controllers/comments/votes_controller.rb new file mode 100644 index 000000000..a1f4b4c4a --- /dev/null +++ b/app/controllers/comments/votes_controller.rb @@ -0,0 +1,26 @@ +module Comments + class VotesController < ApplicationController + load_and_authorize_resource :comment + before_action :authenticate_user! + before_action :verify_comments_open! + + def create + authorize! :create, Vote.new(voter: current_user, votable: @comment) + @comment.vote_by(voter: current_user, vote: params[:value]) + + respond_to do |format| + format.js { render :show } + end + end + + private + + def verify_comments_open! + return if current_user.administrator? || current_user.moderator? + + if @comment.commentable.respond_to?(:comments_closed?) && @comment.commentable.comments_closed? + redirect_to polymorphic_path(@comment.commentable), alert: t("comments.comments_closed") + end + end + end +end diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 8f54125ae..cc60c3fc2 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -1,8 +1,8 @@ class CommentsController < ApplicationController - before_action :authenticate_user!, only: [:create, :hide, :vote] + before_action :authenticate_user!, only: [:create, :hide] before_action :load_commentable, only: :create before_action :verify_resident_for_commentable!, only: :create - before_action :verify_comments_open!, only: [:create, :vote] + before_action :verify_comments_open!, only: [:create] before_action :build_comment, only: :create load_and_authorize_resource @@ -27,11 +27,6 @@ class CommentsController < ApplicationController end end - def vote - @comment.vote_by(voter: current_user, vote: params[:value]) - respond_with @comment - end - def flag Flag.flag(current_user, @comment) set_comment_flags(@comment) diff --git a/app/models/abilities/common.rb b/app/models/abilities/common.rb index f4493840b..921fa0a16 100644 --- a/app/models/abilities/common.rb +++ b/app/models/abilities/common.rb @@ -82,7 +82,7 @@ module Abilities unless user.organization? can [:create, :destroy], ActsAsVotable::Vote, voter_id: user.id, votable_type: "Debate" - can :vote, Comment + can :create, ActsAsVotable::Vote, voter_id: user.id, votable_type: "Comment" end if user.level_two_or_three_verified? diff --git a/app/views/comments/vote.js.erb b/app/views/comments/votes/show.js.erb similarity index 100% rename from app/views/comments/vote.js.erb rename to app/views/comments/votes/show.js.erb diff --git a/config/routes/comment.rb b/config/routes/comment.rb index 68327028c..39f387c4b 100644 --- a/config/routes/comment.rb +++ b/config/routes/comment.rb @@ -1,8 +1,9 @@ resources :comments, only: [:create, :show] do member do - post :vote put :flag put :unflag put :hide end + + resources :votes, controller: "comments/votes", only: :create end diff --git a/spec/controllers/comments/votes_controller_spec.rb b/spec/controllers/comments/votes_controller_spec.rb new file mode 100644 index 000000000..edfbf6361 --- /dev/null +++ b/spec/controllers/comments/votes_controller_spec.rb @@ -0,0 +1,15 @@ +require "rails_helper" + +describe Comments::VotesController do + let(:comment) { create(:comment) } + + describe "POST create" do + it "allows voting" do + sign_in create(:user) + + expect do + post :create, xhr: true, params: { comment_id: comment.id, value: "yes" } + end.to change { comment.reload.votes_for.size }.by(1) + end + end +end diff --git a/spec/models/abilities/common_spec.rb b/spec/models/abilities/common_spec.rb index 5d13ab111..84f2ec341 100644 --- a/spec/models/abilities/common_spec.rb +++ b/spec/models/abilities/common_spec.rb @@ -113,7 +113,8 @@ describe Abilities::Common do describe "Comment" do it { should be_able_to(:create, Comment) } - it { should be_able_to(:vote, Comment) } + it { should be_able_to(:create, user.votes.build(votable: comment)) } + it { should_not be_able_to(:create, another_user.votes.build(votable: comment)) } it { should be_able_to(:hide, own_comment) } it { should_not be_able_to(:hide, comment) } diff --git a/spec/models/abilities/organization_spec.rb b/spec/models/abilities/organization_spec.rb index 50d52dda1..bffea47b7 100644 --- a/spec/models/abilities/organization_spec.rb +++ b/spec/models/abilities/organization_spec.rb @@ -8,6 +8,7 @@ describe "Abilities::Organization" do let(:organization) { create(:organization) } let(:debate) { create(:debate) } let(:proposal) { create(:proposal) } + let(:comment) { create(:comment) } it { should be_able_to(:show, user) } it { should be_able_to(:edit, user) } @@ -22,7 +23,7 @@ describe "Abilities::Organization" do it { should_not be_able_to(:vote, Proposal) } it { should be_able_to(:create, Comment) } - it { should_not be_able_to(:vote, Comment) } + it { should_not be_able_to(:create, user.votes.build(votable: comment)) } it { should_not be_able_to(:read, SDG::Target) }