From 7fd773773adfbd59c15ed53b3c459f0329eae34b Mon Sep 17 00:00:00 2001 From: taitus Date: Thu, 25 May 2023 11:10:24 +0200 Subject: [PATCH 01/12] Unify votes specs --- spec/system/votes_spec.rb | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/spec/system/votes_spec.rb b/spec/system/votes_spec.rb index 294d91bc7..83ca2f305 100644 --- a/spec/system/votes_spec.rb +++ b/spec/system/votes_spec.rb @@ -74,9 +74,11 @@ describe "Votes" do end end - scenario "Update" do + scenario "Create and update from debate show" do visit debate_path(create(:debate)) + expect(page).to have_content "No votes" + click_button "I agree" within(".in-favor") do @@ -84,6 +86,13 @@ describe "Votes" do expect(page).to have_button class: "voted" end + within(".against") do + expect(page).to have_content "0%" + expect(page).to have_css("button.no-voted") + end + + expect(page).to have_content "1 vote" + click_button "I disagree" within(".in-favor") do @@ -136,24 +145,6 @@ describe "Votes" do end end - scenario "Create from debate show" do - visit debate_path(create(:debate)) - - click_button "I agree" - - within(".in-favor") do - expect(page).to have_content "100%" - expect(page).to have_button class: "voted" - end - - within(".against") do - expect(page).to have_content "0%" - expect(page).to have_button class: "no-voted" - end - - expect(page).to have_content "1 vote" - end - scenario "Create in index" do create(:debate) visit debates_path From daf96927538ab2246d54ea08edb46ad9956c4819 Mon Sep 17 00:00:00 2001 From: taitus Date: Mon, 29 May 2023 10:49:16 +0200 Subject: [PATCH 02/12] Add aria-pressed to in favor against component In order to the users using screen readers know whether the button is pressed or not. --- .../in_favor_against_component.html.erb | 2 + .../shared/in_favor_against_component.rb | 11 ++++ .../debates/votes_component_spec.rb | 14 ----- .../proposals/votes_component_spec.rb | 14 ----- .../shared/in_favor_against_component_spec.rb | 54 +++++++++++++++++++ 5 files changed, 67 insertions(+), 28 deletions(-) create mode 100644 spec/components/shared/in_favor_against_component_spec.rb diff --git a/app/components/shared/in_favor_against_component.html.erb b/app/components/shared/in_favor_against_component.html.erb index b78e245f2..671215bd4 100644 --- a/app/components/shared/in_favor_against_component.html.erb +++ b/app/components/shared/in_favor_against_component.html.erb @@ -4,6 +4,7 @@ class: "like #{voted_classes[:in_favor]}", title: t("votes.agree"), "aria-label": agree_aria_label, + "aria-pressed": pressed?("yes"), method: "post", remote: true do %> <%= t("votes.agree") %> @@ -16,6 +17,7 @@ class: "unlike #{voted_classes[:against]}", title: t("votes.disagree"), "aria-label": disagree_aria_label, + "aria-pressed": pressed?("no"), method: "post", remote: true do %> <%= t("votes.disagree") %> diff --git a/app/components/shared/in_favor_against_component.rb b/app/components/shared/in_favor_against_component.rb index 8806057c6..419a51995 100644 --- a/app/components/shared/in_favor_against_component.rb +++ b/app/components/shared/in_favor_against_component.rb @@ -30,4 +30,15 @@ class Shared::InFavorAgainstComponent < ApplicationComponent def disagree_aria_label t("votes.disagree_label", title: votable.title) end + + def pressed?(value) + case current_user&.voted_as_when_voted_for(votable) + when true + value == "yes" + when false + value == "no" + else + false + end + end end diff --git a/spec/components/debates/votes_component_spec.rb b/spec/components/debates/votes_component_spec.rb index 6d1c973aa..b859f58ed 100644 --- a/spec/components/debates/votes_component_spec.rb +++ b/spec/components/debates/votes_component_spec.rb @@ -25,19 +25,5 @@ describe Debates::VotesComponent do expect(page).to have_button "I don't agree with What about the 2030 agenda?" expect(page).not_to have_content "You must sign in or sign up to continue." end - - it "does not include result percentages" do - create(:vote, votable: debate) - sign_in(create(:user)) - - render_inline component - - expect(page).to have_button count: 2 - expect(page).to have_button "I agree" - expect(page).to have_button "I disagree" - expect(page).not_to have_button text: "%" - expect(page).not_to have_button text: "100" - expect(page).not_to have_button text: "0" - end end end diff --git a/spec/components/legislation/proposals/votes_component_spec.rb b/spec/components/legislation/proposals/votes_component_spec.rb index 81649bf9f..65418b001 100644 --- a/spec/components/legislation/proposals/votes_component_spec.rb +++ b/spec/components/legislation/proposals/votes_component_spec.rb @@ -38,19 +38,5 @@ describe Legislation::Proposals::VotesComponent do expect(page).to have_button "I don't agree with Require wearing masks at home" expect(page).not_to have_content "You must sign in or sign up to continue." end - - it "does not include result percentages" do - create(:vote, votable: proposal) - sign_in(create(:user)) - - render_inline component - - expect(page).to have_button count: 2 - expect(page).to have_button "I agree" - expect(page).to have_button "I disagree" - expect(page).not_to have_button text: "%" - expect(page).not_to have_button text: "100" - expect(page).not_to have_button text: "0" - end end end diff --git a/spec/components/shared/in_favor_against_component_spec.rb b/spec/components/shared/in_favor_against_component_spec.rb new file mode 100644 index 000000000..32d88f574 --- /dev/null +++ b/spec/components/shared/in_favor_against_component_spec.rb @@ -0,0 +1,54 @@ +require "rails_helper" + +describe Shared::InFavorAgainstComponent do + let(:debate) { create(:debate) } + let(:component) { Shared::InFavorAgainstComponent.new(debate) } + let(:user) { create(:user) } + + describe "Agree and disagree buttons" do + it "does not include result percentages" do + create(:vote, votable: debate) + sign_in(user) + + render_inline component + + expect(page).to have_button count: 2 + expect(page).to have_button "I agree" + expect(page).to have_button "I disagree" + expect(page).not_to have_button text: "%" + expect(page).not_to have_button text: "100" + expect(page).not_to have_button text: "0" + end + + describe "aria-pressed attribute" do + it "is true when the in-favor button is pressed" do + debate.register_vote(user, "yes") + sign_in(user) + + render_inline component + + expect(page.find(".in-favor")).to have_css "button[aria-pressed='true']" + expect(page.find(".against")).to have_css "button[aria-pressed='false']" + end + + it "is true when the against button is pressed" do + debate.register_vote(user, "no") + sign_in(user) + + render_inline component + + expect(page.find(".in-favor")).to have_css "button[aria-pressed='false']" + expect(page.find(".against")).to have_css "button[aria-pressed='true']" + end + + it "is false when neither the 'in-favor' button nor the 'against' button are pressed" do + sign_in(user) + + render_inline component + + expect(page.find(".in-favor")).to have_css "button[aria-pressed='false']" + expect(page.find(".against")).to have_css "button[aria-pressed='false']" + end + end + end +end From 10cfa0e59f0e603172ce58d8d5bcc60327b8747f Mon Sep 17 00:00:00 2001 From: taitus Date: Mon, 29 May 2023 12:10:21 +0200 Subject: [PATCH 03/12] Refactor scss for in favor against component In order to reduce the code used to add styles to the buttons, we removed the classes that had been added and handled it with the new aria-pressed attribute --- app/assets/stylesheets/in_favor_against.scss | 34 +++------------- .../in_favor_against_component.html.erb | 2 - .../shared/in_favor_against_component.rb | 15 ------- spec/system/votes_spec.rb | 40 ++++++++----------- 4 files changed, 22 insertions(+), 69 deletions(-) diff --git a/app/assets/stylesheets/in_favor_against.scss b/app/assets/stylesheets/in_favor_against.scss index f3c6988e7..6e8d48481 100644 --- a/app/assets/stylesheets/in_favor_against.scss +++ b/app/assets/stylesheets/in_favor_against.scss @@ -25,10 +25,10 @@ &:not([disabled]) { &:hover, - &:active { + &:active, + &[aria-pressed=true] { color: #fff; cursor: pointer; - opacity: 1 !important; } } } @@ -38,7 +38,8 @@ &:not([disabled]) { &:hover, - &:active { + &:active, + &[aria-pressed=true] { background: $like; border: 2px solid $like; } @@ -50,19 +51,14 @@ &:not([disabled]) { &:hover, - &:active { + &:active, + &[aria-pressed=true] { background: $unlike; border: 2px solid $unlike; } } } - .like, - .unlike { - vertical-align: super; - text-decoration: none; - } - .percentage { display: inline-block; font-size: $small-font-size; @@ -76,22 +72,4 @@ padding-right: 0; } } - - .voted { - color: #fff; - } - - .in-favor .voted { - background: $like; - border: 2px solid $like; - } - - .against .voted { - background: $unlike; - border: 2px solid $unlike; - } - - .no-voted { - opacity: 0.3; - } } diff --git a/app/components/shared/in_favor_against_component.html.erb b/app/components/shared/in_favor_against_component.html.erb index 671215bd4..be4040983 100644 --- a/app/components/shared/in_favor_against_component.html.erb +++ b/app/components/shared/in_favor_against_component.html.erb @@ -1,7 +1,6 @@
<%= button_to polymorphic_path(votable, action: :vote, value: "yes"), - class: "like #{voted_classes[:in_favor]}", title: t("votes.agree"), "aria-label": agree_aria_label, "aria-pressed": pressed?("yes"), @@ -14,7 +13,6 @@
<%= button_to polymorphic_path(votable, action: :vote, value: "no"), - class: "unlike #{voted_classes[:against]}", title: t("votes.disagree"), "aria-label": disagree_aria_label, "aria-pressed": pressed?("no"), diff --git a/app/components/shared/in_favor_against_component.rb b/app/components/shared/in_favor_against_component.rb index 419a51995..a8561e458 100644 --- a/app/components/shared/in_favor_against_component.rb +++ b/app/components/shared/in_favor_against_component.rb @@ -8,21 +8,6 @@ class Shared::InFavorAgainstComponent < ApplicationComponent private - def voted_classes - @voted_classes ||= css_classes_for_vote - end - - def css_classes_for_vote - case current_user&.voted_as_when_voted_for(votable) - when true - { in_favor: "voted", against: "no-voted" } - when false - { in_favor: "no-voted", against: "voted" } - else - { in_favor: "", against: "" } - end - end - def agree_aria_label t("votes.agree_label", title: votable.title) end diff --git a/spec/system/votes_spec.rb b/spec/system/votes_spec.rb index 83ca2f305..abed25032 100644 --- a/spec/system/votes_spec.rb +++ b/spec/system/votes_spec.rb @@ -19,37 +19,31 @@ describe "Votes" do within("#debates") do within("#debate_#{debate1.id}_votes") do within(".in-favor") do - expect(page).to have_button class: "voted" - expect(page).not_to have_button class: "no-voted" + expect(page).to have_css "button[aria-pressed='true']" end within(".against") do - expect(page).to have_button class: "no-voted" - expect(page).not_to have_button class: "voted" + expect(page).to have_css "button[aria-pressed='false']" end end within("#debate_#{debate2.id}_votes") do within(".in-favor") do - expect(page).not_to have_button class: "voted" - expect(page).not_to have_button class: "no-voted" + expect(page).to have_css "button[aria-pressed='false']" end within(".against") do - expect(page).not_to have_button class: "no-voted" - expect(page).not_to have_button class: "voted" + expect(page).to have_css "button[aria-pressed='false']" end end within("#debate_#{debate3.id}_votes") do within(".in-favor") do - expect(page).to have_button class: "no-voted" - expect(page).not_to have_button class: "voted" + expect(page).to have_css "button[aria-pressed='false']" end within(".against") do - expect(page).to have_button class: "voted" - expect(page).not_to have_button class: "no-voted" + expect(page).to have_css "button[aria-pressed='true']" end end end @@ -63,14 +57,12 @@ describe "Votes" do within(".in-favor") do expect(page).to have_content "0%" - expect(page).not_to have_button class: "voted" - expect(page).not_to have_button class: "no-voted" + expect(page).to have_css "button[aria-pressed='false']" end within(".against") do expect(page).to have_content "0%" - expect(page).not_to have_button class: "voted" - expect(page).not_to have_button class: "no-voted" + expect(page).to have_css "button[aria-pressed='false']" end end @@ -83,12 +75,12 @@ describe "Votes" do within(".in-favor") do expect(page).to have_content "100%" - expect(page).to have_button class: "voted" + expect(page).to have_css "button[aria-pressed='true']" end within(".against") do expect(page).to have_content "0%" - expect(page).to have_css("button.no-voted") + expect(page).to have_css "button[aria-pressed='false']" end expect(page).to have_content "1 vote" @@ -97,12 +89,12 @@ describe "Votes" do within(".in-favor") do expect(page).to have_content "0%" - expect(page).to have_button class: "no-voted" + expect(page).to have_css "button[aria-pressed='false']" end within(".against") do expect(page).to have_content "100%" - expect(page).to have_button class: "voted" + expect(page).to have_css "button[aria-pressed='true']" end expect(page).to have_content "1 vote" @@ -136,12 +128,12 @@ describe "Votes" do within(".in-favor") do expect(page).to have_content "50%" - expect(page).to have_button class: "voted" + expect(page).to have_css "button[aria-pressed='true']" end within(".against") do expect(page).to have_content "50%" - expect(page).to have_button class: "no-voted" + expect(page).to have_css "button[aria-pressed='false']" end end @@ -154,12 +146,12 @@ describe "Votes" do within(".in-favor") do expect(page).to have_content "100%" - expect(page).to have_button class: "voted" + expect(page).to have_css "button[aria-pressed='true']" end within(".against") do expect(page).to have_content "0%" - expect(page).to have_button class: "no-voted" + expect(page).to have_css "button[aria-pressed='false']" end expect(page).to have_content "1 vote" From 5009bf6c378fdaa398907c1d1d2ade79dfba5ef3 Mon Sep 17 00:00:00 2001 From: taitus Date: Mon, 29 May 2023 11:21:03 +0200 Subject: [PATCH 04/12] Add aria-pressed to in comments votes component In order to the users using screen readers know whether the button is pressed or not. --- app/assets/stylesheets/layout.scss | 6 ++- .../comments/votes_component.html.erb | 2 + app/components/comments/votes_component.rb | 13 ++++- .../comments/votes_component_spec.rb | 53 +++++++++++++++++++ 4 files changed, 71 insertions(+), 3 deletions(-) create mode 100644 spec/components/comments/votes_component_spec.rb diff --git a/app/assets/stylesheets/layout.scss b/app/assets/stylesheets/layout.scss index 9c3780f56..d16c4f9be 100644 --- a/app/assets/stylesheets/layout.scss +++ b/app/assets/stylesheets/layout.scss @@ -1683,7 +1683,8 @@ table { .in-favor button { @include has-fa-icon(thumbs-up, solid); - &:hover { + &:hover, + &[aria-pressed=true] { color: $like; } } @@ -1691,7 +1692,8 @@ table { .against button { @include has-fa-icon(thumbs-down, solid); - &:hover { + &:hover, + &[aria-pressed=true] { color: $unlike; } } diff --git a/app/components/comments/votes_component.html.erb b/app/components/comments/votes_component.html.erb index 9e3deae86..155ba9ae4 100644 --- a/app/components/comments/votes_component.html.erb +++ b/app/components/comments/votes_component.html.erb @@ -6,6 +6,7 @@ <%= button_to vote_comment_path(comment, value: "yes"), method: "post", remote: can?(:vote, comment), + "aria-pressed": pressed?("yes"), title: t("votes.agree") do %> <%= t("votes.agree") %> <% end %> @@ -16,6 +17,7 @@ <%= button_to vote_comment_path(comment, value: "no"), method: "post", remote: can?(:vote, comment), + "aria-pressed": pressed?("no"), title: t("votes.disagree") do %> <%= t("votes.disagree") %> <% end %> diff --git a/app/components/comments/votes_component.rb b/app/components/comments/votes_component.rb index d93a24b0f..701b402f9 100644 --- a/app/components/comments/votes_component.rb +++ b/app/components/comments/votes_component.rb @@ -1,8 +1,19 @@ class Comments::VotesComponent < ApplicationComponent attr_reader :comment - delegate :can?, to: :helpers + delegate :can?, :current_user, to: :helpers def initialize(comment) @comment = comment end + + def pressed?(value) + case current_user&.voted_as_when_voted_for(comment) + when true + value == "yes" + when false + value == "no" + else + false + end + end end diff --git a/spec/components/comments/votes_component_spec.rb b/spec/components/comments/votes_component_spec.rb new file mode 100644 index 000000000..884a6765c --- /dev/null +++ b/spec/components/comments/votes_component_spec.rb @@ -0,0 +1,53 @@ +require "rails_helper" + +describe Comments::VotesComponent do + let(:user) { create(:user) } + let(:comment) { create(:comment, user: user) } + let(:component) { Comments::VotesComponent.new(comment) } + + describe "aria-pressed attribute" do + it "is true when the in-favor button is pressed" do + comment.vote_by(voter: user, vote: "yes") + sign_in(user) + + render_inline component + + page.find(".in-favor") do |in_favor_block| + expect(in_favor_block).to have_css "button[aria-pressed='true']" + end + + page.find(".against") do |against_block| + expect(against_block).to have_css "button[aria-pressed='false']" + end + end + + it "is true when the against button is pressed" do + comment.vote_by(voter: user, vote: "no") + sign_in(user) + + render_inline component + + page.find(".in-favor") do |in_favor_block| + expect(in_favor_block).to have_css "button[aria-pressed='false']" + end + + page.find(".against") do |against_block| + expect(against_block).to have_css "button[aria-pressed='true']" + end + end + + it "is false when neither the 'in-favor' button nor the 'against' button are pressed" do + sign_in(user) + + render_inline component + + page.find(".in-favor") do |in_favor_block| + expect(in_favor_block).to have_css "button[aria-pressed='false']" + end + + page.find(".against") do |against_block| + expect(against_block).to have_css "button[aria-pressed='false']" + end + end + end +end From c96e3b027f95a9b870f2f437169f57d4023cc521 Mon Sep 17 00:00:00 2001 From: taitus Date: Wed, 20 Sep 2023 11:52:52 +0200 Subject: [PATCH 05/12] Replace Partials with Direct Component Rendering In this commit, we have performed a refactoring to enhance code organization. Several partials that were solely responsible for rendering components have been removed. Instead, we are now directly rendering the components within the views where these partials were previously used. --- app/views/comments/_comment.html.erb | 2 +- app/views/comments/_votes.html.erb | 1 - app/views/comments/vote.js.erb | 2 +- app/views/debates/_debate.html.erb | 2 +- app/views/debates/_votes.html.erb | 1 - app/views/debates/show.html.erb | 2 +- app/views/debates/vote.js.erb | 2 +- app/views/legislation/annotations/_comments.html.erb | 2 +- 8 files changed, 6 insertions(+), 8 deletions(-) delete mode 100644 app/views/comments/_votes.html.erb delete mode 100644 app/views/debates/_votes.html.erb diff --git a/app/views/comments/_comment.html.erb b/app/views/comments/_comment.html.erb index 75d3887ca..549bab2d1 100644 --- a/app/views/comments/_comment.html.erb +++ b/app/views/comments/_comment.html.erb @@ -81,7 +81,7 @@
<% unless valuation %>
- <%= render "comments/votes", comment: comment %> + <%= render Comments::VotesComponent.new(comment) %>
<% end %> diff --git a/app/views/comments/_votes.html.erb b/app/views/comments/_votes.html.erb deleted file mode 100644 index e45be3c2c..000000000 --- a/app/views/comments/_votes.html.erb +++ /dev/null @@ -1 +0,0 @@ -<%= render Comments::VotesComponent.new(comment) %> diff --git a/app/views/comments/vote.js.erb b/app/views/comments/vote.js.erb index 2fe859cbd..f26291e73 100644 --- a/app/views/comments/vote.js.erb +++ b/app/views/comments/vote.js.erb @@ -1 +1 @@ -$("#<%= dom_id(@comment) %>_votes").html("<%= j render("comments/votes", comment: @comment) %>"); +$("#<%= dom_id(@comment) %>_votes").html("<%= j render Comments::VotesComponent.new(@comment) %>"); diff --git a/app/views/debates/_debate.html.erb b/app/views/debates/_debate.html.erb index 905c3ec79..68de5a60d 100644 --- a/app/views/debates/_debate.html.erb +++ b/app/views/debates/_debate.html.erb @@ -49,7 +49,7 @@
- <%= render "debates/votes", debate: debate %> + <%= render Debates::VotesComponent.new(debate) %>
diff --git a/app/views/debates/_votes.html.erb b/app/views/debates/_votes.html.erb deleted file mode 100644 index 01a1f2b3f..000000000 --- a/app/views/debates/_votes.html.erb +++ /dev/null @@ -1 +0,0 @@ -<%= render Debates::VotesComponent.new(debate) %> diff --git a/app/views/debates/show.html.erb b/app/views/debates/show.html.erb index 52146ee68..38fc11825 100644 --- a/app/views/debates/show.html.erb +++ b/app/views/debates/show.html.erb @@ -57,7 +57,7 @@

<%= t("votes.supports") %>

- <%= render "debates/votes", debate: @debate %> + <%= render Debates::VotesComponent.new(@debate) %>
<%= render "shared/social_share", share_title: t("debates.show.share"), diff --git a/app/views/debates/vote.js.erb b/app/views/debates/vote.js.erb index aae060dbd..bee28fcc7 100644 --- a/app/views/debates/vote.js.erb +++ b/app/views/debates/vote.js.erb @@ -1 +1 @@ -$("#<%= dom_id(@debate) %>_votes").html("<%= j render("debates/votes", debate: @debate) %>"); +$("#<%= dom_id(@debate) %>_votes").html("<%= j render Debates::VotesComponent.new(@debate) %>"); diff --git a/app/views/legislation/annotations/_comments.html.erb b/app/views/legislation/annotations/_comments.html.erb index aad5a4248..c8c1bd29d 100644 --- a/app/views/legislation/annotations/_comments.html.erb +++ b/app/views/legislation/annotations/_comments.html.erb @@ -24,7 +24,7 @@
- <%= render "comments/votes", comment: comment %> + <%= render Comments::VotesComponent.new(comment) %>
From fd5fa2da79d67757a2ee514f0005883857d30953 Mon Sep 17 00:00:00 2001 From: taitus Date: Wed, 20 Sep 2023 15:06:45 +0200 Subject: [PATCH 06/12] 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). --- .../legislation/proposals/votes_component.rb | 4 +- .../in_favor_against_component.html.erb | 4 +- .../shared/in_favor_against_component.rb | 8 ++++ app/controllers/debates/votes_controller.rb | 15 +++++++ app/controllers/debates_controller.rb | 4 -- .../legislation/proposals/votes_controller.rb | 18 ++++++++ .../legislation/proposals_controller.rb | 4 -- app/models/abilities/common.rb | 5 ++- app/models/budget/investment.rb | 2 +- app/models/legislation/proposal.rb | 8 ---- .../{vote.js.erb => votes/show.js.erb} | 0 .../{vote.js.erb => votes/show.js.erb} | 0 config/routes/debate.rb | 3 +- config/routes/legislation.rb | 3 +- .../shared/in_favor_against_component_spec.rb | 16 +++++++ .../debates/votes_controller_spec.rb | 43 +++++++++++++++++++ spec/controllers/debates_controller_spec.rb | 22 ---------- .../proposals/votes_controller_spec.rb | 40 +++++++++++++++++ spec/models/abilities/administrator_spec.rb | 1 - spec/models/abilities/common_spec.rb | 29 +++++++++---- spec/models/abilities/moderator_spec.rb | 1 - spec/models/abilities/organization_spec.rb | 2 +- 22 files changed, 173 insertions(+), 59 deletions(-) create mode 100644 app/controllers/debates/votes_controller.rb create mode 100644 app/controllers/legislation/proposals/votes_controller.rb rename app/views/debates/{vote.js.erb => votes/show.js.erb} (100%) rename app/views/legislation/proposals/{vote.js.erb => votes/show.js.erb} (100%) create mode 100644 spec/controllers/debates/votes_controller_spec.rb create mode 100644 spec/controllers/legislation/proposals/votes_controller_spec.rb diff --git a/app/components/legislation/proposals/votes_component.rb b/app/components/legislation/proposals/votes_component.rb index 96495800f..10d16e74f 100644 --- a/app/components/legislation/proposals/votes_component.rb +++ b/app/components/legislation/proposals/votes_component.rb @@ -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 diff --git a/app/components/shared/in_favor_against_component.html.erb b/app/components/shared/in_favor_against_component.html.erb index be4040983..c4c403649 100644 --- a/app/components/shared/in_favor_against_component.html.erb +++ b/app/components/shared/in_favor_against_component.html.erb @@ -1,6 +1,6 @@
- <%= 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 @@
- <%= 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"), diff --git a/app/components/shared/in_favor_against_component.rb b/app/components/shared/in_favor_against_component.rb index a8561e458..0c0736407 100644 --- a/app/components/shared/in_favor_against_component.rb +++ b/app/components/shared/in_favor_against_component.rb @@ -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 diff --git a/app/controllers/debates/votes_controller.rb b/app/controllers/debates/votes_controller.rb new file mode 100644 index 000000000..e0b92c472 --- /dev/null +++ b/app/controllers/debates/votes_controller.rb @@ -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 diff --git a/app/controllers/debates_controller.rb b/app/controllers/debates_controller.rb index c08963a6c..33c7fd899 100644 --- a/app/controllers/debates_controller.rb +++ b/app/controllers/debates_controller.rb @@ -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 diff --git a/app/controllers/legislation/proposals/votes_controller.rb b/app/controllers/legislation/proposals/votes_controller.rb new file mode 100644 index 000000000..03886c41c --- /dev/null +++ b/app/controllers/legislation/proposals/votes_controller.rb @@ -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 diff --git a/app/controllers/legislation/proposals_controller.rb b/app/controllers/legislation/proposals_controller.rb index 86b34bce0..d9adb14e3 100644 --- a/app/controllers/legislation/proposals_controller.rb +++ b/app/controllers/legislation/proposals_controller.rb @@ -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 diff --git a/app/models/abilities/common.rb b/app/models/abilities/common.rb index fe96be60b..53378b4be 100644 --- a/app/models/abilities/common.rb +++ b/app/models/abilities/common.rb @@ -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" } diff --git a/app/models/budget/investment.rb b/app/models/budget/investment.rb index 086a4265b..1808d8033 100644 --- a/app/models/budget/investment.rb +++ b/app/models/budget/investment.rb @@ -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 diff --git a/app/models/legislation/proposal.rb b/app/models/legislation/proposal.rb index ec653d4c3..497cea5db 100644 --- a/app/models/legislation/proposal.rb +++ b/app/models/legislation/proposal.rb @@ -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 diff --git a/app/views/debates/vote.js.erb b/app/views/debates/votes/show.js.erb similarity index 100% rename from app/views/debates/vote.js.erb rename to app/views/debates/votes/show.js.erb diff --git a/app/views/legislation/proposals/vote.js.erb b/app/views/legislation/proposals/votes/show.js.erb similarity index 100% rename from app/views/legislation/proposals/vote.js.erb rename to app/views/legislation/proposals/votes/show.js.erb diff --git a/config/routes/debate.rb b/config/routes/debate.rb index 58f1f9215..45d6f06f6 100644 --- a/config/routes/debate.rb +++ b/config/routes/debate.rb @@ -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 diff --git a/config/routes/legislation.rb b/config/routes/legislation.rb index 4a18ef1cd..7e3890c42 100644 --- a/config/routes/legislation.rb +++ b/config/routes/legislation.rb @@ -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 diff --git a/spec/components/shared/in_favor_against_component_spec.rb b/spec/components/shared/in_favor_against_component_spec.rb index 32d88f574..ece805749 100644 --- a/spec/components/shared/in_favor_against_component_spec.rb +++ b/spec/components/shared/in_favor_against_component_spec.rb @@ -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) diff --git a/spec/controllers/debates/votes_controller_spec.rb b/spec/controllers/debates/votes_controller_spec.rb new file mode 100644 index 000000000..9e6090ee6 --- /dev/null +++ b/spec/controllers/debates/votes_controller_spec.rb @@ -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 diff --git a/spec/controllers/debates_controller_spec.rb b/spec/controllers/debates_controller_spec.rb index 01af1db82..c3d96dcea 100644 --- a/spec/controllers/debates_controller_spec.rb +++ b/spec/controllers/debates_controller_spec.rb @@ -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) diff --git a/spec/controllers/legislation/proposals/votes_controller_spec.rb b/spec/controllers/legislation/proposals/votes_controller_spec.rb new file mode 100644 index 000000000..fd8c780a6 --- /dev/null +++ b/spec/controllers/legislation/proposals/votes_controller_spec.rb @@ -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 diff --git a/spec/models/abilities/administrator_spec.rb b/spec/models/abilities/administrator_spec.rb index 39a42dbd6..7fb48b225 100644 --- a/spec/models/abilities/administrator_spec.rb +++ b/spec/models/abilities/administrator_spec.rb @@ -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) } diff --git a/spec/models/abilities/common_spec.rb b/spec/models/abilities/common_spec.rb index 6c3918ec8..9ba947776 100644 --- a/spec/models/abilities/common_spec.rb +++ b/spec/models/abilities/common_spec.rb @@ -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) } diff --git a/spec/models/abilities/moderator_spec.rb b/spec/models/abilities/moderator_spec.rb index b9df69067..9dd4f4c0c 100644 --- a/spec/models/abilities/moderator_spec.rb +++ b/spec/models/abilities/moderator_spec.rb @@ -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) } diff --git a/spec/models/abilities/organization_spec.rb b/spec/models/abilities/organization_spec.rb index 874a5ce40..9bfd9b69f 100644 --- a/spec/models/abilities/organization_spec.rb +++ b/spec/models/abilities/organization_spec.rb @@ -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) } From 108a05a66df8f5e7b6b46ca42acf2f3542bc6bb3 Mon Sep 17 00:00:00 2001 From: taitus Date: Wed, 20 Sep 2023 15:23:23 +0200 Subject: [PATCH 07/12] Allow undo votes in favor against component --- .../in_favor_against_component.html.erb | 4 +- .../shared/in_favor_against_component.rb | 27 +++++++++++-- app/controllers/debates/votes_controller.rb | 9 +++++ .../legislation/proposals/votes_controller.rb | 9 +++++ app/models/abilities/common.rb | 4 +- config/routes/debate.rb | 2 +- config/routes/legislation.rb | 2 +- .../shared/in_favor_against_component_spec.rb | 18 +++++++++ .../debates/votes_controller_spec.rb | 13 +++++++ .../proposals/votes_controller_spec.rb | 16 ++++++++ spec/models/abilities/common_spec.rb | 5 +++ spec/models/abilities/organization_spec.rb | 1 + spec/system/votes_spec.rb | 38 ++++++++++++++++++- 13 files changed, 137 insertions(+), 11 deletions(-) diff --git a/app/components/shared/in_favor_against_component.html.erb b/app/components/shared/in_favor_against_component.html.erb index c4c403649..954c10ddb 100644 --- a/app/components/shared/in_favor_against_component.html.erb +++ b/app/components/shared/in_favor_against_component.html.erb @@ -4,7 +4,7 @@ title: t("votes.agree"), "aria-label": agree_aria_label, "aria-pressed": pressed?("yes"), - method: "post", + method: user_already_voted_with("yes") ? "delete" : "post", remote: true do %> <%= t("votes.agree") %> <% end %> @@ -16,7 +16,7 @@ title: t("votes.disagree"), "aria-label": disagree_aria_label, "aria-pressed": pressed?("no"), - method: "post", + method: user_already_voted_with("no") ? "delete" : "post", remote: true do %> <%= t("votes.disagree") %> <% end %> diff --git a/app/components/shared/in_favor_against_component.rb b/app/components/shared/in_favor_against_component.rb index 0c0736407..68e96e99b 100644 --- a/app/components/shared/in_favor_against_component.rb +++ b/app/components/shared/in_favor_against_component.rb @@ -28,10 +28,31 @@ class Shared::InFavorAgainstComponent < ApplicationComponent end def vote_in_favor_against_path(value) - if votable.class.name == "Debate" - debate_votes_path(votable, value: value) + if user_already_voted_with(value) + remove_vote_path(value) else - legislation_process_proposal_votes_path(votable.process, votable, value: 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 + + def user_already_voted_with(value) + current_user&.voted_as_when_voted_for(votable) == parse_vote(value) + end + + def remove_vote_path(value) + vote = votable.votes_for.find_by!(voter: current_user) + if votable.class.name == "Debate" + debate_vote_path(votable, vote, value: value) + else + legislation_process_proposal_vote_path(votable.process, votable, vote, value: value) + end + end + + def parse_vote(value) + value == "yes" ? true : false + end end diff --git a/app/controllers/debates/votes_controller.rb b/app/controllers/debates/votes_controller.rb index e0b92c472..ae0cf33d5 100644 --- a/app/controllers/debates/votes_controller.rb +++ b/app/controllers/debates/votes_controller.rb @@ -2,6 +2,7 @@ module Debates class VotesController < ApplicationController before_action :authenticate_user! load_and_authorize_resource :debate + load_and_authorize_resource through: :debate, through_association: :votes_for, only: :destroy def create authorize! :create, Vote.new(voter: current_user, votable: @debate) @@ -11,5 +12,13 @@ module Debates format.js { render :show } end end + + def destroy + @debate.unvote_by(current_user) + + respond_to do |format| + format.js { render :show } + end + end end end diff --git a/app/controllers/legislation/proposals/votes_controller.rb b/app/controllers/legislation/proposals/votes_controller.rb index 03886c41c..e51d97db5 100644 --- a/app/controllers/legislation/proposals/votes_controller.rb +++ b/app/controllers/legislation/proposals/votes_controller.rb @@ -4,6 +4,7 @@ module Legislation before_action :authenticate_user! load_and_authorize_resource :process, class: "Legislation::Process" load_and_authorize_resource :proposal, class: "Legislation::Proposal", through: :process + load_and_authorize_resource through: :proposal, through_association: :votes_for, only: :destroy def create authorize! :create, Vote.new(voter: current_user, votable: @proposal) @@ -13,6 +14,14 @@ module Legislation format.js { render :show } end end + + def destroy + @proposal.unvote_by(current_user) + + respond_to do |format| + format.js { render :show } + end + end end end end diff --git a/app/models/abilities/common.rb b/app/models/abilities/common.rb index 53378b4be..f4493840b 100644 --- a/app/models/abilities/common.rb +++ b/app/models/abilities/common.rb @@ -81,14 +81,14 @@ module Abilities can [:create, :destroy], DirectUpload unless user.organization? - can :create, ActsAsVotable::Vote, voter_id: user.id, votable_type: "Debate" + can [:create, :destroy], 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 :create, ActsAsVotable::Vote, voter_id: user.id, votable_type: "Legislation::Proposal" + can [:create, :destroy], ActsAsVotable::Vote, voter_id: user.id, votable_type: "Legislation::Proposal" can :create, Legislation::Answer diff --git a/config/routes/debate.rb b/config/routes/debate.rb index 45d6f06f6..03735cc15 100644 --- a/config/routes/debate.rb +++ b/config/routes/debate.rb @@ -11,5 +11,5 @@ resources :debates do put "recommendations/disable", only: :index, controller: "debates", action: :disable_recommendations end - resources :votes, controller: "debates/votes", only: :create + resources :votes, controller: "debates/votes", only: [:create, :destroy] end diff --git a/config/routes/legislation.rb b/config/routes/legislation.rb index 7e3890c42..9b3f1a0f9 100644 --- a/config/routes/legislation.rb +++ b/config/routes/legislation.rb @@ -23,7 +23,7 @@ namespace :legislation do get :suggest end - resources :votes, controller: "proposals/votes", only: :create + resources :votes, controller: "proposals/votes", only: [:create, :destroy] end resources :draft_versions, only: [:show] do diff --git a/spec/components/shared/in_favor_against_component_spec.rb b/spec/components/shared/in_favor_against_component_spec.rb index ece805749..f3ade0ec9 100644 --- a/spec/components/shared/in_favor_against_component_spec.rb +++ b/spec/components/shared/in_favor_against_component_spec.rb @@ -22,6 +22,24 @@ describe Shared::InFavorAgainstComponent do end end + it "can undo vote when the user has already voted" do + create(:vote, votable: debate, voter: user) + + 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).to have_css "input[name='_method'][value='delete']", visible: :hidden + 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) diff --git a/spec/controllers/debates/votes_controller_spec.rb b/spec/controllers/debates/votes_controller_spec.rb index 9e6090ee6..74bd009d2 100644 --- a/spec/controllers/debates/votes_controller_spec.rb +++ b/spec/controllers/debates/votes_controller_spec.rb @@ -40,4 +40,17 @@ describe Debates::VotesController do end end end + + describe "DELETE destroy" do + let(:user) { create(:user) } + let(:debate) { create(:debate) } + let!(:vote) { create(:vote, votable: debate, voter: user) } + before { sign_in user } + + it "allows undoing a vote if user is allowed" do + expect do + delete :destroy, xhr: true, params: { debate_id: debate.id, id: vote } + end.to change { debate.reload.votes_for.size }.by(-1) + end + end end diff --git a/spec/controllers/legislation/proposals/votes_controller_spec.rb b/spec/controllers/legislation/proposals/votes_controller_spec.rb index fd8c780a6..f618277b1 100644 --- a/spec/controllers/legislation/proposals/votes_controller_spec.rb +++ b/spec/controllers/legislation/proposals/votes_controller_spec.rb @@ -37,4 +37,20 @@ describe Legislation::Proposals::VotesController do end.not_to change { proposal.reload.votes_for.size } end end + + describe "DELETE destroy" do + let(:user) { create(:user, :level_two) } + let!(:vote) { create(:vote, votable: proposal, voter: user) } + let(:vote_params) do + { process_id: legislation_process.id, legislation_proposal_id: proposal.id, id: vote } + end + + it "allows undoing a vote" do + sign_in user + + expect do + delete :destroy, xhr: true, params: vote_params + end.to change { proposal.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 9ba947776..5d13ab111 100644 --- a/spec/models/abilities/common_spec.rb +++ b/spec/models/abilities/common_spec.rb @@ -77,6 +77,8 @@ describe Abilities::Common do it { should be_able_to(:show, 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(:destroy, user.votes.build(votable: debate)) } + it { should_not be_able_to(:destroy, another_user.votes.build(votable: debate)) } it { should be_able_to(:show, user) } it { should be_able_to(:edit, user) } @@ -184,12 +186,15 @@ describe Abilities::Common do 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)) } + it { should_not be_able_to(:destroy, 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)) } + it { should be_able_to(:destroy, user.votes.build(votable: legislation_proposal)) } + it { should_not be_able_to(:destroy, another_user.votes.build(votable: legislation_proposal)) } end end diff --git a/spec/models/abilities/organization_spec.rb b/spec/models/abilities/organization_spec.rb index 9bfd9b69f..50d52dda1 100644 --- a/spec/models/abilities/organization_spec.rb +++ b/spec/models/abilities/organization_spec.rb @@ -15,6 +15,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(:create, user.votes.build(votable: debate)) } + it { should_not be_able_to(:destroy, user.votes.build(votable: debate)) } it { should be_able_to(:index, Proposal) } it { should be_able_to(:show, proposal) } diff --git a/spec/system/votes_spec.rb b/spec/system/votes_spec.rb index abed25032..96eaf11e5 100644 --- a/spec/system/votes_spec.rb +++ b/spec/system/votes_spec.rb @@ -100,20 +100,27 @@ describe "Votes" do expect(page).to have_content "1 vote" end - scenario "Trying to vote multiple times" do + scenario "Allow undoing votes" do visit debate_path(create(:debate)) click_button "I agree" + expect(page).to have_content "1 vote" + expect(page).not_to have_content "No votes" + click_button "I agree" + + expect(page).to have_content "No votes" expect(page).not_to have_content "2 votes" within(".in-favor") do - expect(page).to have_content "100%" + expect(page).to have_content "0%" + expect(page).to have_css "button[aria-pressed='false']" end within(".against") do expect(page).to have_content "0%" + expect(page).to have_css "button[aria-pressed='false']" end end @@ -337,4 +344,31 @@ describe "Votes" do expect(page).not_to have_button "Support", disabled: :all end end + + describe "Legislation Proposals" do + let(:proposal) { create(:legislation_proposal) } + + scenario "Allow undoing votes" do + login_as verified + visit legislation_process_proposal_path(proposal.process, proposal) + + click_button "I agree" + + expect(page).to have_content "1 vote" + expect(page).not_to have_content "No votes" + + click_button "I agree" + + expect(page).to have_content "No votes" + expect(page).not_to have_content "2 votes" + + within(".in-favor") do + expect(page).to have_content "0%" + end + + within(".against") do + expect(page).to have_content "0%" + end + end + end end From e0dc7c96c31bdbe8ad08e71159335283b491c843 Mon Sep 17 00:00:00 2001 From: taitus Date: Wed, 28 Jun 2023 15:44:08 +0200 Subject: [PATCH 08/12] Use polimorphic path in favor against component --- .../shared/in_favor_against_component.rb | 18 +++-------------- .../legislation/proposals/votes_controller.rb | 4 +++- config/routes/legislation.rb | 6 ++++++ spec/routing/polymorphic_routes_spec.rb | 20 +++++++++++++++++++ 4 files changed, 32 insertions(+), 16 deletions(-) diff --git a/app/components/shared/in_favor_against_component.rb b/app/components/shared/in_favor_against_component.rb index 68e96e99b..c14f7008a 100644 --- a/app/components/shared/in_favor_against_component.rb +++ b/app/components/shared/in_favor_against_component.rb @@ -29,13 +29,10 @@ class Shared::InFavorAgainstComponent < ApplicationComponent def vote_in_favor_against_path(value) if user_already_voted_with(value) - remove_vote_path(value) + vote = Vote.find_by!(votable: votable, voter: current_user) + polymorphic_path(vote) else - if votable.class.name == "Debate" - debate_votes_path(votable, value: value) - else - legislation_process_proposal_votes_path(votable.process, votable, value: value) - end + polymorphic_path(Vote.new(votable: votable), value: value) end end @@ -43,15 +40,6 @@ class Shared::InFavorAgainstComponent < ApplicationComponent current_user&.voted_as_when_voted_for(votable) == parse_vote(value) end - def remove_vote_path(value) - vote = votable.votes_for.find_by!(voter: current_user) - if votable.class.name == "Debate" - debate_vote_path(votable, vote, value: value) - else - legislation_process_proposal_vote_path(votable.process, votable, vote, value: value) - end - end - def parse_vote(value) value == "yes" ? true : false end diff --git a/app/controllers/legislation/proposals/votes_controller.rb b/app/controllers/legislation/proposals/votes_controller.rb index e51d97db5..5f70a5b95 100644 --- a/app/controllers/legislation/proposals/votes_controller.rb +++ b/app/controllers/legislation/proposals/votes_controller.rb @@ -3,7 +3,9 @@ module Legislation 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 + load_and_authorize_resource :proposal, class: "Legislation::Proposal", + through: :process, + id_param: "legislation_proposal_id" load_and_authorize_resource through: :proposal, through_association: :votes_for, only: :destroy def create diff --git a/config/routes/legislation.rb b/config/routes/legislation.rb index 9b3f1a0f9..ae37f818b 100644 --- a/config/routes/legislation.rb +++ b/config/routes/legislation.rb @@ -22,7 +22,9 @@ namespace :legislation do collection do get :suggest end + end + resources :legislation_proposals, path: "proposals", only: [] do resources :votes, controller: "proposals/votes", only: [:create, :destroy] end @@ -42,6 +44,10 @@ resolve "Legislation::Proposal" do |proposal, options| [proposal.process, :proposal, options.merge(id: proposal)] end +resolve "Vote" do |vote, options| + [*resource_hierarchy_for(vote.votable), vote, options] +end + resolve "Legislation::Question" do |question, options| [question.process, :question, options.merge(id: question)] end diff --git a/spec/routing/polymorphic_routes_spec.rb b/spec/routing/polymorphic_routes_spec.rb index 5ff9bef09..9810cfcda 100644 --- a/spec/routing/polymorphic_routes_spec.rb +++ b/spec/routing/polymorphic_routes_spec.rb @@ -16,6 +16,26 @@ describe "Polymorphic routes" do expect(polymorphic_path(proposal)).to eq legislation_process_proposal_path(process, proposal) end + it "routes legislation proposals vote" do + process = create(:legislation_process) + proposal = create(:legislation_proposal, process: process) + path = polymorphic_path(Vote.new(votable: proposal), value: true) + + expect(path).to eq legislation_process_legislation_proposal_votes_path(process, proposal, value: true) + end + + it "routes legislation proposals remove vote" do + process = create(:legislation_process) + proposal = create(:legislation_proposal, process: process) + vote = create(:vote, votable: proposal) + path = polymorphic_path(vote, value: true) + + expect(path).to eq legislation_process_legislation_proposal_vote_path(process, + proposal, + vote, + value: true) + end + it "routes legislation questions" do process = create(:legislation_process) question = create(:legislation_question, process: process) From b39b64a674a8c2a067c6f99438dcec26e3c6a891 Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 6 Oct 2023 09:24:55 +0200 Subject: [PATCH 09/12] Remove unnecessary 'shallow' in comments routes Since this commit 6e27917d6e1 it seems that it is no longer necessary to continue using shallow. --- config/routes/comment.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/routes/comment.rb b/config/routes/comment.rb index c973446e2..68327028c 100644 --- a/config/routes/comment.rb +++ b/config/routes/comment.rb @@ -1,4 +1,4 @@ -resources :comments, only: [:create, :show], shallow: true do +resources :comments, only: [:create, :show] do member do post :vote put :flag From f87a332c3eb3e07abd916cac5bdf14e0e8e24dd2 Mon Sep 17 00:00:00 2001 From: taitus Date: Wed, 20 Sep 2023 16:08:44 +0200 Subject: [PATCH 10/12] 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) } From 718fcba6d870140987259d75d25b8792be343778 Mon Sep 17 00:00:00 2001 From: taitus Date: Wed, 20 Sep 2023 16:10:36 +0200 Subject: [PATCH 11/12] Allow undo votes in comments votes component --- .../comments/votes_component.html.erb | 8 +++---- app/components/comments/votes_component.rb | 24 ++++++++++++++++++- app/controllers/comments/votes_controller.rb | 11 ++++++++- app/models/abilities/common.rb | 2 +- config/routes/comment.rb | 2 +- .../comments/votes_component_spec.rb | 20 ++++++++++++---- .../comments/votes_controller_spec.rb | 19 +++++++++++++++ spec/models/abilities/common_spec.rb | 2 ++ spec/models/abilities/organization_spec.rb | 1 + .../comments/budget_investments_spec.rb | 6 ++--- spec/system/comments/debates_spec.rb | 6 ++--- .../comments/legislation_annotations_spec.rb | 6 ++--- .../comments/legislation_questions_spec.rb | 6 ++--- spec/system/comments/polls_spec.rb | 6 ++--- spec/system/comments/proposals_spec.rb | 6 ++--- spec/system/comments/topics_spec.rb | 12 +++++----- 16 files changed, 101 insertions(+), 36 deletions(-) diff --git a/app/components/comments/votes_component.html.erb b/app/components/comments/votes_component.html.erb index 47d23c276..41df0614e 100644 --- a/app/components/comments/votes_component.html.erb +++ b/app/components/comments/votes_component.html.erb @@ -4,8 +4,8 @@ <%= button_to vote_in_favor_against_path("yes"), - method: "post", - remote: can?(:create, comment.votes_for.new(voter: current_user)), + method: user_already_voted_with("yes") ? "delete" : "post", + remote: remote_submit("yes"), "aria-pressed": pressed?("yes"), title: t("votes.agree") do %> <%= t("votes.agree") %> @@ -15,8 +15,8 @@ <%= button_to vote_in_favor_against_path("no"), - method: "post", - remote: can?(:create, comment.votes_for.new(voter: current_user)), + method: user_already_voted_with("no") ? "delete" : "post", + remote: remote_submit("no"), "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 9b0dc94dc..143327f24 100644 --- a/app/components/comments/votes_component.rb +++ b/app/components/comments/votes_component.rb @@ -18,6 +18,28 @@ class Comments::VotesComponent < ApplicationComponent end def vote_in_favor_against_path(value) - comment_votes_path(comment, value: value) + if user_already_voted_with(value) + vote = comment.votes_for.find_by!(voter: current_user) + + comment_vote_path(comment, vote, value: value) + else + comment_votes_path(comment, value: value) + end + end + + def user_already_voted_with(value) + current_user&.voted_as_when_voted_for(comment) == parse_vote(value) + end + + def parse_vote(value) + value == "yes" ? true : false + end + + def remote_submit(value) + if user_already_voted_with(value) + can?(:destroy, comment.votes_for.new(voter: current_user)) + else + can?(:create, comment.votes_for.new(voter: current_user)) + end end end diff --git a/app/controllers/comments/votes_controller.rb b/app/controllers/comments/votes_controller.rb index a1f4b4c4a..d8289f9cf 100644 --- a/app/controllers/comments/votes_controller.rb +++ b/app/controllers/comments/votes_controller.rb @@ -1,7 +1,8 @@ module Comments class VotesController < ApplicationController - load_and_authorize_resource :comment before_action :authenticate_user! + load_and_authorize_resource :comment + load_and_authorize_resource through: :comment, through_association: :votes_for, only: :destroy before_action :verify_comments_open! def create @@ -13,6 +14,14 @@ module Comments end end + def destroy + @comment.unvote_by(current_user) + + respond_to do |format| + format.js { render :show } + end + end + private def verify_comments_open! diff --git a/app/models/abilities/common.rb b/app/models/abilities/common.rb index 921fa0a16..e14469539 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 :create, ActsAsVotable::Vote, voter_id: user.id, votable_type: "Comment" + can [:create, :destroy], ActsAsVotable::Vote, voter_id: user.id, votable_type: "Comment" end if user.level_two_or_three_verified? diff --git a/config/routes/comment.rb b/config/routes/comment.rb index 39f387c4b..d461be5fb 100644 --- a/config/routes/comment.rb +++ b/config/routes/comment.rb @@ -5,5 +5,5 @@ resources :comments, only: [:create, :show] do put :hide end - resources :votes, controller: "comments/votes", only: :create + resources :votes, controller: "comments/votes", only: [:create, :destroy] end diff --git a/spec/components/comments/votes_component_spec.rb b/spec/components/comments/votes_component_spec.rb index 884a6765c..59708d71a 100644 --- a/spec/components/comments/votes_component_spec.rb +++ b/spec/components/comments/votes_component_spec.rb @@ -5,8 +5,8 @@ describe Comments::VotesComponent do let(:comment) { create(:comment, user: user) } let(:component) { Comments::VotesComponent.new(comment) } - describe "aria-pressed attribute" do - it "is true when the in-favor button is pressed" do + describe "aria-pressed and method attributes" do + it "have expected values when the in-favor button is pressed" do comment.vote_by(voter: user, vote: "yes") sign_in(user) @@ -14,14 +14,18 @@ describe Comments::VotesComponent do page.find(".in-favor") do |in_favor_block| expect(in_favor_block).to have_css "button[aria-pressed='true']" + expect(in_favor_block).to have_css "form[action*='votes'][method='post']" + expect(in_favor_block).to have_css "input[name='_method'][value='delete']", visible: :hidden end page.find(".against") do |against_block| expect(against_block).to have_css "button[aria-pressed='false']" + 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 "is true when the against button is pressed" do + it "have expected values when the against button is pressed" do comment.vote_by(voter: user, vote: "no") sign_in(user) @@ -29,24 +33,32 @@ describe Comments::VotesComponent do page.find(".in-favor") do |in_favor_block| expect(in_favor_block).to have_css "button[aria-pressed='false']" + 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 "button[aria-pressed='true']" + expect(against_block).to have_css "form[action*='votes'][method='post']" + expect(against_block).to have_css "input[name='_method'][value='delete']", visible: :hidden end end - it "is false when neither the 'in-favor' button nor the 'against' button are pressed" do + it "have expected values when neither the 'in-favor' button nor the 'against' button are pressed" do sign_in(user) render_inline component page.find(".in-favor") do |in_favor_block| expect(in_favor_block).to have_css "button[aria-pressed='false']" + 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 "button[aria-pressed='false']" + 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 end diff --git a/spec/controllers/comments/votes_controller_spec.rb b/spec/controllers/comments/votes_controller_spec.rb index edfbf6361..cc164423f 100644 --- a/spec/controllers/comments/votes_controller_spec.rb +++ b/spec/controllers/comments/votes_controller_spec.rb @@ -12,4 +12,23 @@ describe Comments::VotesController do end.to change { comment.reload.votes_for.size }.by(1) end end + + describe "DELETE destroy" do + let(:user) { create(:user) } + let!(:vote) { create(:vote, votable: comment, voter: user) } + + it "redirects unidentified users to the sign in page" do + delete :destroy, params: { comment_id: comment.id, id: vote } + + expect(response).to redirect_to new_user_session_path + end + + it "allows undoing a vote" do + sign_in user + + expect do + delete :destroy, xhr: true, params: { comment_id: comment.id, id: vote } + 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 84f2ec341..392d4367b 100644 --- a/spec/models/abilities/common_spec.rb +++ b/spec/models/abilities/common_spec.rb @@ -115,6 +115,8 @@ describe Abilities::Common do it { should be_able_to(:create, 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(:destroy, user.votes.build(votable: comment)) } + it { should_not be_able_to(:destroy, 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 bffea47b7..f38c60b14 100644 --- a/spec/models/abilities/organization_spec.rb +++ b/spec/models/abilities/organization_spec.rb @@ -24,6 +24,7 @@ describe "Abilities::Organization" do it { should be_able_to(:create, Comment) } it { should_not be_able_to(:create, user.votes.build(votable: comment)) } + it { should_not be_able_to(:destroy, user.votes.build(votable: comment)) } it { should_not be_able_to(:read, SDG::Target) } diff --git a/spec/system/comments/budget_investments_spec.rb b/spec/system/comments/budget_investments_spec.rb index f0c39c266..f7f643a6c 100644 --- a/spec/system/comments/budget_investments_spec.rb +++ b/spec/system/comments/budget_investments_spec.rb @@ -578,7 +578,7 @@ describe "Commenting Budget::Investments" do end end - scenario "Trying to vote multiple times" do + scenario "Allow undoing votes" do visit budget_investment_path(budget, investment) within("#comment_#{comment.id}_votes") do @@ -591,14 +591,14 @@ describe "Commenting Budget::Investments" do click_button "I agree" within(".in-favor") do - expect(page).to have_content "1" + expect(page).to have_content "0" end within(".against") do expect(page).to have_content "0" end - expect(page).to have_content "1 vote" + expect(page).to have_content "No votes" end end end diff --git a/spec/system/comments/debates_spec.rb b/spec/system/comments/debates_spec.rb index b6550b3d9..3242b8943 100644 --- a/spec/system/comments/debates_spec.rb +++ b/spec/system/comments/debates_spec.rb @@ -614,7 +614,7 @@ describe "Commenting debates" do end end - scenario "Trying to vote multiple times" do + scenario "Allow undoing votes" do visit debate_path(debate) within("#comment_#{comment.id}_votes") do @@ -626,14 +626,14 @@ describe "Commenting debates" do click_button "I agree" within(".in-favor") do expect(page).not_to have_content "2" - expect(page).to have_content "1" + expect(page).to have_content "0" end within(".against") do expect(page).to have_content "0" end - expect(page).to have_content "1 vote" + expect(page).to have_content "No votes" end end end diff --git a/spec/system/comments/legislation_annotations_spec.rb b/spec/system/comments/legislation_annotations_spec.rb index aae580f14..63b89850b 100644 --- a/spec/system/comments/legislation_annotations_spec.rb +++ b/spec/system/comments/legislation_annotations_spec.rb @@ -550,7 +550,7 @@ describe "Commenting legislation questions" do end end - scenario "Trying to vote multiple times" do + scenario "Allow undoing votes" do visit polymorphic_path(annotation) within("#comment_#{comment.id}_votes") do @@ -562,14 +562,14 @@ describe "Commenting legislation questions" do click_button "I agree" within(".in-favor") do expect(page).not_to have_content "2" - expect(page).to have_content "1" + expect(page).to have_content "0" end within(".against") do expect(page).to have_content "0" end - expect(page).to have_content "1 vote" + expect(page).to have_content "No votes" end end end diff --git a/spec/system/comments/legislation_questions_spec.rb b/spec/system/comments/legislation_questions_spec.rb index 6cc9c3685..bc50f7610 100644 --- a/spec/system/comments/legislation_questions_spec.rb +++ b/spec/system/comments/legislation_questions_spec.rb @@ -534,7 +534,7 @@ describe "Commenting legislation questions" do end end - scenario "Trying to vote multiple times" do + scenario "Allow undoing votes" do visit legislation_process_question_path(question.process, question) within("#comment_#{comment.id}_votes") do @@ -546,14 +546,14 @@ describe "Commenting legislation questions" do click_button "I agree" within(".in-favor") do expect(page).not_to have_content "2" - expect(page).to have_content "1" + expect(page).to have_content "0" end within(".against") do expect(page).to have_content "0" end - expect(page).to have_content "1 vote" + expect(page).to have_content "No votes" end end end diff --git a/spec/system/comments/polls_spec.rb b/spec/system/comments/polls_spec.rb index ca0708398..8f3d00759 100644 --- a/spec/system/comments/polls_spec.rb +++ b/spec/system/comments/polls_spec.rb @@ -495,7 +495,7 @@ describe "Commenting polls" do end end - scenario "Trying to vote multiple times" do + scenario "Allow undoing votes" do visit poll_path(poll) within("#comment_#{comment.id}_votes") do @@ -508,14 +508,14 @@ describe "Commenting polls" do click_button "I agree" within(".in-favor") do - expect(page).to have_content "1" + expect(page).to have_content "0" end within(".against") do expect(page).to have_content "0" end - expect(page).to have_content "1 vote" + expect(page).to have_content "No votes" end end end diff --git a/spec/system/comments/proposals_spec.rb b/spec/system/comments/proposals_spec.rb index 5d2f42329..a2da45137 100644 --- a/spec/system/comments/proposals_spec.rb +++ b/spec/system/comments/proposals_spec.rb @@ -500,7 +500,7 @@ describe "Commenting proposals" do end end - scenario "Trying to vote multiple times" do + scenario "Allow undoing votes" do visit proposal_path(proposal) within("#comment_#{comment.id}_votes") do @@ -513,14 +513,14 @@ describe "Commenting proposals" do click_button "I agree" within(".in-favor") do - expect(page).to have_content "1" + expect(page).to have_content "0" end within(".against") do expect(page).to have_content "0" end - expect(page).to have_content "1 vote" + expect(page).to have_content "No votes" end end end diff --git a/spec/system/comments/topics_spec.rb b/spec/system/comments/topics_spec.rb index e4a603bc7..c41ef234a 100644 --- a/spec/system/comments/topics_spec.rb +++ b/spec/system/comments/topics_spec.rb @@ -547,7 +547,7 @@ describe "Commenting topics from proposals" do end end - scenario "Trying to vote multiple times" do + scenario "Allow undoing votes" do visit community_topic_path(proposal.community, topic) within("#comment_#{comment.id}_votes") do @@ -560,14 +560,14 @@ describe "Commenting topics from proposals" do click_button "I agree" within(".in-favor") do - expect(page).to have_content "1" + expect(page).to have_content "0" end within(".against") do expect(page).to have_content "0" end - expect(page).to have_content "1 vote" + expect(page).to have_content "No votes" end end end @@ -1060,7 +1060,7 @@ describe "Commenting topics from budget investments" do end end - scenario "Trying to vote multiple times" do + scenario "Allow undoing votes" do visit community_topic_path(investment.community, topic) within("#comment_#{comment.id}_votes") do @@ -1073,14 +1073,14 @@ describe "Commenting topics from budget investments" do click_button "I agree" within(".in-favor") do - expect(page).to have_content "1" + expect(page).to have_content "0" end within(".against") do expect(page).to have_content "0" end - expect(page).to have_content "1 vote" + expect(page).to have_content "No votes" end end end From 00ff47e7e6048e53663ad2a4b6655ad0b1e5dc9f Mon Sep 17 00:00:00 2001 From: taitus Date: Wed, 28 Jun 2023 10:26:44 +0200 Subject: [PATCH 12/12] Add component in order to reduce duplicated code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Javi Martín --- .../comments/votes_component.html.erb | 20 +++----- app/components/comments/votes_component.rb | 38 --------------- .../in_favor_against_component.html.erb | 24 ++++------ .../shared/in_favor_against_component.rb | 30 +----------- .../shared/vote_button_component.html.erb | 3 ++ .../shared/vote_button_component.rb | 48 +++++++++++++++++++ 6 files changed, 66 insertions(+), 97 deletions(-) create mode 100644 app/components/shared/vote_button_component.html.erb create mode 100644 app/components/shared/vote_button_component.rb diff --git a/app/components/comments/votes_component.html.erb b/app/components/comments/votes_component.html.erb index 41df0614e..8ef3a8293 100644 --- a/app/components/comments/votes_component.html.erb +++ b/app/components/comments/votes_component.html.erb @@ -3,24 +3,16 @@  |  - <%= button_to vote_in_favor_against_path("yes"), - method: user_already_voted_with("yes") ? "delete" : "post", - remote: remote_submit("yes"), - "aria-pressed": pressed?("yes"), - title: t("votes.agree") do %> - <%= t("votes.agree") %> - <% end %> + <%= render Shared::VoteButtonComponent.new(comment, + value: "yes", + title: t("votes.agree")) %> <%= comment.total_likes %> - <%= button_to vote_in_favor_against_path("no"), - method: user_already_voted_with("no") ? "delete" : "post", - remote: remote_submit("no"), - "aria-pressed": pressed?("no"), - title: t("votes.disagree") do %> - <%= t("votes.disagree") %> - <% end %> + <%= render Shared::VoteButtonComponent.new(comment, + value: "no", + title: t("votes.disagree")) %> <%= comment.total_dislikes %>
diff --git a/app/components/comments/votes_component.rb b/app/components/comments/votes_component.rb index 143327f24..50cf56a86 100644 --- a/app/components/comments/votes_component.rb +++ b/app/components/comments/votes_component.rb @@ -1,45 +1,7 @@ class Comments::VotesComponent < ApplicationComponent attr_reader :comment - delegate :can?, :current_user, to: :helpers def initialize(comment) @comment = comment end - - def pressed?(value) - case current_user&.voted_as_when_voted_for(comment) - when true - value == "yes" - when false - value == "no" - else - false - end - end - - def vote_in_favor_against_path(value) - if user_already_voted_with(value) - vote = comment.votes_for.find_by!(voter: current_user) - - comment_vote_path(comment, vote, value: value) - else - comment_votes_path(comment, value: value) - end - end - - def user_already_voted_with(value) - current_user&.voted_as_when_voted_for(comment) == parse_vote(value) - end - - def parse_vote(value) - value == "yes" ? true : false - end - - def remote_submit(value) - if user_already_voted_with(value) - can?(:destroy, comment.votes_for.new(voter: current_user)) - else - can?(:create, comment.votes_for.new(voter: current_user)) - end - end end diff --git a/app/components/shared/in_favor_against_component.html.erb b/app/components/shared/in_favor_against_component.html.erb index 954c10ddb..fbe2fb611 100644 --- a/app/components/shared/in_favor_against_component.html.erb +++ b/app/components/shared/in_favor_against_component.html.erb @@ -1,25 +1,17 @@
- <%= button_to vote_in_favor_against_path("yes"), - title: t("votes.agree"), - "aria-label": agree_aria_label, - "aria-pressed": pressed?("yes"), - method: user_already_voted_with("yes") ? "delete" : "post", - remote: true do %> - <%= t("votes.agree") %> - <% end %> + <%= render Shared::VoteButtonComponent.new(votable, + value: "yes", + "aria-label": agree_aria_label, + title: t("votes.agree")) %> <%= votes_percentage("likes", votable) %>
- <%= button_to vote_in_favor_against_path("no"), - title: t("votes.disagree"), - "aria-label": disagree_aria_label, - "aria-pressed": pressed?("no"), - method: user_already_voted_with("no") ? "delete" : "post", - remote: true do %> - <%= t("votes.disagree") %> - <% end %> + <%= render Shared::VoteButtonComponent.new(votable, + value: "no", + "aria-label": disagree_aria_label, + title: t("votes.disagree")) %> <%= votes_percentage("dislikes", votable) %>
diff --git a/app/components/shared/in_favor_against_component.rb b/app/components/shared/in_favor_against_component.rb index c14f7008a..ee490ecd6 100644 --- a/app/components/shared/in_favor_against_component.rb +++ b/app/components/shared/in_favor_against_component.rb @@ -1,6 +1,6 @@ class Shared::InFavorAgainstComponent < ApplicationComponent attr_reader :votable - delegate :current_user, :votes_percentage, to: :helpers + delegate :votes_percentage, to: :helpers def initialize(votable) @votable = votable @@ -15,32 +15,4 @@ class Shared::InFavorAgainstComponent < ApplicationComponent def disagree_aria_label t("votes.disagree_label", title: votable.title) end - - def pressed?(value) - case current_user&.voted_as_when_voted_for(votable) - when true - value == "yes" - when false - value == "no" - else - false - end - end - - def vote_in_favor_against_path(value) - if user_already_voted_with(value) - vote = Vote.find_by!(votable: votable, voter: current_user) - polymorphic_path(vote) - else - polymorphic_path(Vote.new(votable: votable), value: value) - end - end - - def user_already_voted_with(value) - current_user&.voted_as_when_voted_for(votable) == parse_vote(value) - end - - def parse_vote(value) - value == "yes" ? true : false - end end diff --git a/app/components/shared/vote_button_component.html.erb b/app/components/shared/vote_button_component.html.erb new file mode 100644 index 000000000..19b35fc06 --- /dev/null +++ b/app/components/shared/vote_button_component.html.erb @@ -0,0 +1,3 @@ +<%= button_to path, default_options.merge(options) do %> + <%= options[:title] %> +<% end %> diff --git a/app/components/shared/vote_button_component.rb b/app/components/shared/vote_button_component.rb new file mode 100644 index 000000000..f71e3e93c --- /dev/null +++ b/app/components/shared/vote_button_component.rb @@ -0,0 +1,48 @@ +class Shared::VoteButtonComponent < ApplicationComponent + attr_reader :votable, :value, :options + delegate :current_user, :can?, to: :helpers + + def initialize(votable, value:, **options) + @votable = votable + @value = value + @options = options + end + + private + + def path + if already_voted? + polymorphic_path(vote) + else + polymorphic_path(vote, value: value) + end + end + + def default_options + if already_voted? + { + "aria-pressed": true, + method: :delete, + remote: can?(:destroy, vote) + } + else + { + "aria-pressed": false, + method: :post, + remote: can?(:create, vote) + } + end + end + + def vote + @vote ||= Vote.find_or_initialize_by(votable: votable, voter: current_user, vote_flag: parsed_value) + end + + def already_voted? + vote.persisted? + end + + def parsed_value + value == "yes" + end +end