From 9937e94fcd34be74fe09bb549968def5cb0fda95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 20 Mar 2020 15:47:02 +0100 Subject: [PATCH 01/10] Fix flagging debates and comments with AJAX We weren't using `foundation()` in these cases, so after flagging a debate or a comment, we had to reload the page before we could unflag it. We're also adding a test for the fix in commit ea85059d. This test shows it's necessary to filter the elements with JavaSctipt using `first()` if we want the same code to work with comments. Co-Authored-By: taitus --- app/assets/javascripts/flaggable.js | 2 +- app/views/comments/_actions.html.erb | 2 +- .../comments/_refresh_flag_actions.js.erb | 2 +- .../debates/_refresh_flag_actions.js.erb | 2 +- spec/system/comments/debates_spec.rb | 29 ++++++++++++++++++- spec/system/debates_spec.rb | 26 +++++++++++++++++ 6 files changed, 58 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/flaggable.js b/app/assets/javascripts/flaggable.js index 0b2661e05..1bc5c3ebb 100644 --- a/app/assets/javascripts/flaggable.js +++ b/app/assets/javascripts/flaggable.js @@ -2,7 +2,7 @@ "use strict"; App.Flaggable = { update: function(resource_id, button) { - $("#" + resource_id + " .js-flag-actions").html(button).foundation(); + $("#" + resource_id + " .js-flag-actions").first().html(button).foundation(); } }; }).call(this); diff --git a/app/views/comments/_actions.html.erb b/app/views/comments/_actions.html.erb index 235344c75..76aed5b5a 100644 --- a/app/views/comments/_actions.html.erb +++ b/app/views/comments/_actions.html.erb @@ -1,4 +1,4 @@ - + <%= render "comments/flag_actions", comment: comment %> diff --git a/app/views/comments/_refresh_flag_actions.js.erb b/app/views/comments/_refresh_flag_actions.js.erb index 8234e696e..f6ce0bc91 100644 --- a/app/views/comments/_refresh_flag_actions.js.erb +++ b/app/views/comments/_refresh_flag_actions.js.erb @@ -1 +1 @@ -$("#flag-actions-<%= dom_id(@comment) %>").html("<%= j render("comments/flag_actions", comment: @comment) %>"); +App.Flaggable.update("<%= dom_id(@comment) %>", "<%= j render("comments/flag_actions", comment: @comment) %>"); diff --git a/app/views/debates/_refresh_flag_actions.js.erb b/app/views/debates/_refresh_flag_actions.js.erb index f1ba6bc9a..2c1e1701a 100644 --- a/app/views/debates/_refresh_flag_actions.js.erb +++ b/app/views/debates/_refresh_flag_actions.js.erb @@ -1 +1 @@ -$("#<%= dom_id(@debate) %> .js-flag-actions").html("<%= j render("debates/flag_actions", debate: @debate) %>"); +App.Flaggable.update("<%= dom_id(@debate) %>", "<%= j render("debates/flag_actions", debate: @debate) %>"); diff --git a/spec/system/comments/debates_spec.rb b/spec/system/comments/debates_spec.rb index ceaeede4f..d486c9284 100644 --- a/spec/system/comments/debates_spec.rb +++ b/spec/system/comments/debates_spec.rb @@ -381,7 +381,34 @@ describe "Commenting debates" do within "#comment_#{comment.id}" do page.find("#flag-expand-comment-#{comment.id}").click - expect(page).to have_selector("#flag-comment-#{comment.id}") + page.find("#flag-comment-#{comment.id}").click + + expect(page).to have_css("#unflag-expand-comment-#{comment.id}") + expect(Flag.flagged?(user, comment)).to be + + page.find("#unflag-expand-comment-#{comment.id}").click + page.find("#unflag-comment-#{comment.id}").click + + expect(page).to have_css("#flag-expand-comment-#{comment.id}") + end + + expect(Flag.flagged?(user, comment)).not_to be + end + + scenario "Flagging a comment with a child does not update its children", :js do + debate = create(:debate, title: "Should we change the world?") + parent_comment = create(:comment, commentable: debate, body: "Main comment") + child_comment = create(:comment, body: "First subcomment", commentable: debate, parent: parent_comment) + + login_as(user) + visit debate_path(debate) + + within "#comment_#{parent_comment.id}" do + page.find("#flag-expand-comment-#{parent_comment.id}").click + page.find("#flag-comment-#{parent_comment.id}").click + + expect(page).to have_css("#unflag-expand-comment-#{parent_comment.id}") + expect(page).to have_css("#flag-expand-comment-#{child_comment.id}") end end diff --git a/spec/system/debates_spec.rb b/spec/system/debates_spec.rb index cf03f9ccf..3f860b086 100644 --- a/spec/system/debates_spec.rb +++ b/spec/system/debates_spec.rb @@ -380,6 +380,32 @@ describe "Debates" do expect(Flag.flagged?(user, debate)).not_to be end + scenario "Flagging/Unflagging AJAX", :js do + user = create(:user) + debate = create(:debate) + + login_as(user) + visit debate_path(debate) + + within "#debate_#{debate.id}" do + page.find("#flag-expand-debate-#{debate.id}").click + page.find("#flag-debate-#{debate.id}").click + + expect(page).to have_css("#unflag-expand-debate-#{debate.id}") + end + + expect(Flag.flagged?(user, debate)).to be + + within "#debate_#{debate.id}" do + page.find("#unflag-expand-debate-#{debate.id}").click + page.find("#unflag-debate-#{debate.id}").click + + expect(page).to have_css("#flag-expand-debate-#{debate.id}") + end + + expect(Flag.flagged?(user, debate)).not_to be + end + describe "Debate index order filters" do scenario "Default order is hot_score", :js do best_debate = create(:debate, title: "Best") From 91da038b2737afe78dc48f3264046e58d127b53d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 20 Mar 2020 15:49:39 +0100 Subject: [PATCH 02/10] Extract shared tests to flag/unflag a record --- spec/shared/system/flaggable.rb | 59 ++++++++++++++++++++++ spec/system/budgets/investments_spec.rb | 65 +------------------------ spec/system/debates_spec.rb | 62 +---------------------- spec/system/proposals_spec.rb | 64 +----------------------- 4 files changed, 62 insertions(+), 188 deletions(-) create mode 100644 spec/shared/system/flaggable.rb diff --git a/spec/shared/system/flaggable.rb b/spec/shared/system/flaggable.rb new file mode 100644 index 000000000..da7c97417 --- /dev/null +++ b/spec/shared/system/flaggable.rb @@ -0,0 +1,59 @@ +shared_examples "flaggable" do |factory_name| + let(:user) { create(:user) } + let(:flaggable) { create(factory_name) } + let(:path) { polymorphic_path(flaggable) } + + scenario "Flagging as inappropriate", :js do + login_as(user) + visit path + + within ".flag-content" do + find(".icon-flag").click + click_link "Flag as inappropriate" + + expect(page).to have_css ".flag-active" + expect(page).to have_link "Unflag", visible: false + end + + expect(Flag.flagged?(user, flaggable)).to be + end + + scenario "Unflagging", :js do + Flag.flag(user, flaggable) + + login_as(user) + visit path + + within ".flag-content" do + expect(page).to have_css ".flag-active" + + find(".icon-flag").click + click_link "Unflag" + + expect(page).not_to have_css ".flag-active" + expect(page).to have_link "Flag as inappropriate", visible: false + end + + expect(Flag.flagged?(user, flaggable)).not_to be + end + + scenario "Flagging and unflagging", :js do + login_as(user) + visit path + + within ".flag-content" do + find(".icon-flag").click + click_link "Flag as inappropriate" + + expect(page).to have_css ".flag-active" + expect(Flag.flagged?(user, flaggable)).to be + + find(".icon-flag").click + click_link "Unflag" + + expect(page).not_to have_css ".flag-active" + end + + expect(Flag.flagged?(user, flaggable)).not_to be + end +end diff --git a/spec/system/budgets/investments_spec.rb b/spec/system/budgets/investments_spec.rb index 75ad9955e..0066dbf76 100644 --- a/spec/system/budgets/investments_spec.rb +++ b/spec/system/budgets/investments_spec.rb @@ -22,6 +22,7 @@ describe "Budget Investments" do :budget_investment, "budget_investment_path", { "budget_id": "budget_id", "id": "id" } + it_behaves_like "flaggable", :budget_investment end context "Load" do @@ -1795,70 +1796,6 @@ describe "Budget Investments" do end end - scenario "Flagging an investment as innapropriate", :js do - user = create(:user) - investment = create(:budget_investment, heading: heading) - - login_as(user) - - visit budget_investment_path(budget, investment) - - within "#budget_investment_#{investment.id}" do - find("#flag-expand-investment-#{investment.id}").click - find("#flag-investment-#{investment.id}").click - - expect(page).to have_css("#unflag-expand-investment-#{investment.id}") - end - - expect(Flag.flagged?(user, investment)).to be - end - - scenario "Unflagging an investment", :js do - user = create(:user) - investment = create(:budget_investment, heading: heading) - Flag.flag(user, investment) - - login_as(user) - - visit budget_investment_path(budget, investment) - - within "#budget_investment_#{investment.id}" do - find("#unflag-expand-investment-#{investment.id}").click - find("#unflag-investment-#{investment.id}").click - - expect(page).to have_css("#flag-expand-investment-#{investment.id}") - end - - expect(Flag.flagged?(user, investment)).not_to be - end - - scenario "Flagging an investment updates the DOM properly", :js do - user = create(:user) - investment = create(:budget_investment, heading: heading) - - login_as(user) - - visit budget_investment_path(budget, investment) - - within "#budget_investment_#{investment.id}" do - find("#flag-expand-investment-#{investment.id}").click - find("#flag-investment-#{investment.id}").click - - expect(page).to have_css("#unflag-expand-investment-#{investment.id}") - end - - expect(Flag.flagged?(user, investment)).to be - - within "#budget_investment_#{investment.id}" do - find("#unflag-expand-investment-#{investment.id}").click - find("#unflag-investment-#{investment.id}").click - - expect(page).to have_css("#flag-expand-investment-#{investment.id}") - end - - expect(Flag.flagged?(user, investment)).not_to be - end - context "sidebar map" do scenario "Display 6 investment's markers on sidebar map", :js do investment1 = create(:budget_investment, heading: heading) diff --git a/spec/system/debates_spec.rb b/spec/system/debates_spec.rb index 3f860b086..0c14302ae 100644 --- a/spec/system/debates_spec.rb +++ b/spec/system/debates_spec.rb @@ -17,6 +17,7 @@ describe "Debates" do :debate, "debate_path", { "id": "id" } + it_behaves_like "flaggable", :debate end scenario "Index" do @@ -345,67 +346,6 @@ describe "Debates" do expect(page).to have_content error_message end - scenario "Flagging", :js do - user = create(:user) - debate = create(:debate) - - login_as(user) - visit debate_path(debate) - - within "#debate_#{debate.id}" do - page.find("#flag-expand-debate-#{debate.id}").click - page.find("#flag-debate-#{debate.id}").click - - expect(page).to have_css("#unflag-expand-debate-#{debate.id}") - end - - expect(Flag.flagged?(user, debate)).to be - end - - scenario "Unflagging", :js do - user = create(:user) - debate = create(:debate) - Flag.flag(user, debate) - - login_as(user) - visit debate_path(debate) - - within "#debate_#{debate.id}" do - page.find("#unflag-expand-debate-#{debate.id}").click - page.find("#unflag-debate-#{debate.id}").click - - expect(page).to have_css("#flag-expand-debate-#{debate.id}") - end - - expect(Flag.flagged?(user, debate)).not_to be - end - - scenario "Flagging/Unflagging AJAX", :js do - user = create(:user) - debate = create(:debate) - - login_as(user) - visit debate_path(debate) - - within "#debate_#{debate.id}" do - page.find("#flag-expand-debate-#{debate.id}").click - page.find("#flag-debate-#{debate.id}").click - - expect(page).to have_css("#unflag-expand-debate-#{debate.id}") - end - - expect(Flag.flagged?(user, debate)).to be - - within "#debate_#{debate.id}" do - page.find("#unflag-expand-debate-#{debate.id}").click - page.find("#unflag-debate-#{debate.id}").click - - expect(page).to have_css("#flag-expand-debate-#{debate.id}") - end - - expect(Flag.flagged?(user, debate)).not_to be - end - describe "Debate index order filters" do scenario "Default order is hot_score", :js do best_debate = create(:debate, title: "Best") diff --git a/spec/system/proposals_spec.rb b/spec/system/proposals_spec.rb index ca5f5fe91..81df85961 100644 --- a/spec/system/proposals_spec.rb +++ b/spec/system/proposals_spec.rb @@ -19,6 +19,7 @@ describe "Proposals" do :proposal, "proposal_path", { "id": "id" } + it_behaves_like "flaggable", :proposal end context "Index" do @@ -1501,69 +1502,6 @@ describe "Proposals" do expect(page).not_to have_content "This proposal has been flagged as inappropriate by several users." end - scenario "Flagging", :js do - user = create(:user) - proposal = create(:proposal) - - login_as(user) - visit proposal_path(proposal) - - within "#proposal_#{proposal.id}" do - page.find("#flag-expand-proposal-#{proposal.id}").click - page.find("#flag-proposal-#{proposal.id}").click - - expect(page).to have_css("#unflag-expand-proposal-#{proposal.id}") - end - - expect(Flag.flagged?(user, proposal)).to be - end - - scenario "Unflagging", :js do - user = create(:user) - proposal = create(:proposal) - Flag.flag(user, proposal) - - login_as(user) - visit proposal_path(proposal) - - within "#proposal_#{proposal.id}" do - page.find("#unflag-expand-proposal-#{proposal.id}").click - page.find("#unflag-proposal-#{proposal.id}").click - - expect(page).to have_css("#flag-expand-proposal-#{proposal.id}") - end - - expect(Flag.flagged?(user, proposal)).not_to be - end - - scenario "Flagging/Unflagging AJAX", :js do - user = create(:user) - proposal = create(:proposal) - - login_as(user) - visit proposal_path(proposal) - - # Flagging - within "#proposal_#{proposal.id}" do - page.find("#flag-expand-proposal-#{proposal.id}").click - page.find("#flag-proposal-#{proposal.id}").click - - expect(page).to have_css("#unflag-expand-proposal-#{proposal.id}") - end - - expect(Flag.flagged?(user, proposal)).to be - - # Unflagging - within "#proposal_#{proposal.id}" do - page.find("#unflag-expand-proposal-#{proposal.id}").click - page.find("#unflag-proposal-#{proposal.id}").click - - expect(page).to have_css("#flag-expand-proposal-#{proposal.id}") - end - - expect(Flag.flagged?(user, proposal)).not_to be - end - it_behaves_like "followable", "proposal", "proposal_path", { "id": "id" } it_behaves_like "imageable", "proposal", "proposal_path", { "id": "id" } From 0d7c2c7a7ceebaaa2248924af323297cf6ed676f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 20 Mar 2020 16:11:29 +0100 Subject: [PATCH 03/10] Simplify rendering flag actions The `respond_with` method is no longer part of Rails (it's now part of the responders gem) and we barely use it. Using a template forced us to use different criteria for different controllers. This change will also make it easier to fix the flag/unflag actions for legislation proposals. With the old code, we would have to add another condition for the legislation/proposals controller. --- app/controllers/comments_controller.rb | 6 ++++-- app/controllers/concerns/flag_actions.rb | 12 ++---------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 0cb2a5b44..49dee85e9 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -35,13 +35,15 @@ class CommentsController < ApplicationController def flag Flag.flag(current_user, @comment) set_comment_flags(@comment) - respond_with @comment, template: "comments/_refresh_flag_actions" + + render "comments/_refresh_flag_actions" end def unflag Flag.unflag(current_user, @comment) set_comment_flags(@comment) - respond_with @comment, template: "comments/_refresh_flag_actions" + + render "comments/_refresh_flag_actions" end private diff --git a/app/controllers/concerns/flag_actions.rb b/app/controllers/concerns/flag_actions.rb index 5984c24e0..cc305dca1 100644 --- a/app/controllers/concerns/flag_actions.rb +++ b/app/controllers/concerns/flag_actions.rb @@ -4,21 +4,13 @@ module FlagActions def flag Flag.flag(current_user, flaggable) - if controller_name == "investments" - respond_with flaggable, template: "budgets/#{controller_name}/_refresh_flag_actions" - else - respond_with flaggable, template: "#{controller_name}/_refresh_flag_actions" - end + render "_refresh_flag_actions" end def unflag Flag.unflag(current_user, flaggable) - if controller_name == "investments" - respond_with flaggable, template: "budgets/#{controller_name}/_refresh_flag_actions" - else - respond_with flaggable, template: "#{controller_name}/_refresh_flag_actions" - end + render "_refresh_flag_actions" end private From 09fd3ab44a0358fc1b2debf1357d30ad8c4790c3 Mon Sep 17 00:00:00 2001 From: volcov Date: Fri, 11 Oct 2019 01:23:27 -0300 Subject: [PATCH 04/10] Fix legislation proposals flag actions We were treating legislation proposals as if they were proposals, omitting the "legislation" namespace, and so we were flagging/unflagging proposals when we wanted to flag/unflag a legislation proposal. --- app/controllers/concerns/flag_actions.rb | 2 ++ .../proposals/_flag_actions.html.erb | 36 +++++++++---------- .../proposals/_refresh_flag_actions.js.erb | 2 +- app/views/legislation/proposals/show.html.erb | 4 ++- spec/system/legislation/proposals_spec.rb | 1 + 5 files changed, 24 insertions(+), 21 deletions(-) diff --git a/app/controllers/concerns/flag_actions.rb b/app/controllers/concerns/flag_actions.rb index cc305dca1..797b94778 100644 --- a/app/controllers/concerns/flag_actions.rb +++ b/app/controllers/concerns/flag_actions.rb @@ -18,6 +18,8 @@ module FlagActions def flaggable if resource_model.to_s == "Budget::Investment" instance_variable_get("@investment") + elsif resource_model.to_s == "Legislation::Proposal" + instance_variable_get("@proposal") else instance_variable_get("@#{resource_model.to_s.downcase}") end diff --git a/app/views/legislation/proposals/_flag_actions.html.erb b/app/views/legislation/proposals/_flag_actions.html.erb index 7bcb03205..6d86b63a2 100644 --- a/app/views/legislation/proposals/_flag_actions.html.erb +++ b/app/views/legislation/proposals/_flag_actions.html.erb @@ -1,21 +1,19 @@ - - - <% if show_flag_action? proposal %> - "> - - - - <%= link_to t("shared.flag"), flag_proposal_path(proposal), method: :put, remote: true, id: "flag-proposal-#{proposal.id}" %> - - <% end %> + + <% if show_flag_action? proposal %> + "> + + + + <%= link_to t("shared.flag"), flag_legislation_process_proposal_path(proposal.process, proposal), method: :put, remote: true, id: "flag-proposal-#{proposal.id}" %> + + <% end %> - <% if show_unflag_action? proposal %> - "> - - - - <%= link_to t("shared.unflag"), unflag_proposal_path(proposal), method: :put, remote: true, id: "unflag-proposal-#{proposal.id}" %> - - <% end %> - + <% if show_unflag_action? proposal %> + "> + + + + <%= link_to t("shared.unflag"), unflag_legislation_process_proposal_path(proposal.process, proposal), method: :put, remote: true, id: "unflag-proposal-#{proposal.id}" %> + + <% end %> diff --git a/app/views/legislation/proposals/_refresh_flag_actions.js.erb b/app/views/legislation/proposals/_refresh_flag_actions.js.erb index a8e4e0b7b..a9bbab7e3 100644 --- a/app/views/legislation/proposals/_refresh_flag_actions.js.erb +++ b/app/views/legislation/proposals/_refresh_flag_actions.js.erb @@ -1 +1 @@ -$("#<%= dom_id(@proposal) %> .js-flag-actions").html("<%= j render("proposals/flag_actions", proposal: @proposal) %>"); +App.Flaggable.update("<%= dom_id(@proposal) %>", "<%= j render("legislation/proposals/flag_actions", proposal: @proposal) %>"); diff --git a/app/views/legislation/proposals/show.html.erb b/app/views/legislation/proposals/show.html.erb index c581567c5..fd6c2280c 100644 --- a/app/views/legislation/proposals/show.html.erb +++ b/app/views/legislation/proposals/show.html.erb @@ -45,7 +45,9 @@ <% if current_user %>  •  - <%= render "proposals/flag_actions", proposal: @proposal %> + + <%= render "legislation/proposals/flag_actions", proposal: @proposal %> + <% end %> diff --git a/spec/system/legislation/proposals_spec.rb b/spec/system/legislation/proposals_spec.rb index 130d41d91..db773cf69 100644 --- a/spec/system/legislation/proposals_spec.rb +++ b/spec/system/legislation/proposals_spec.rb @@ -8,6 +8,7 @@ describe "Legislation Proposals" do context "Concerns" do it_behaves_like "notifiable in-app", :legislation_proposal + it_behaves_like "flaggable", :legislation_proposal end scenario "Only one menu element has 'active' CSS selector" do From 3c27df592e40ca6755160ce8bd595b292178380d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 20 Mar 2020 18:45:10 +0100 Subject: [PATCH 05/10] Remove test for flagging poll comments This feature hasn't been implemented and there are no plans to implement it in the near future. --- spec/system/comments/polls_spec.rb | 53 ------------------------------ 1 file changed, 53 deletions(-) diff --git a/spec/system/comments/polls_spec.rb b/spec/system/comments/polls_spec.rb index ded564fac..1df15f3e2 100644 --- a/spec/system/comments/polls_spec.rb +++ b/spec/system/comments/polls_spec.rb @@ -298,59 +298,6 @@ describe "Commenting polls" do expect(page).to have_css(".comment.comment.comment.comment.comment.comment.comment.comment") end - scenario "Flagging as inappropriate", :js do - skip "Feature not implemented yet, review soon" - - comment = create(:comment, commentable: poll) - - login_as(user) - visit poll_path(poll) - - within "#comment_#{comment.id}" do - page.find("#flag-expand-comment-#{comment.id}").click - page.find("#flag-comment-#{comment.id}").click - - expect(page).to have_css("#unflag-expand-comment-#{comment.id}") - end - - expect(Flag.flagged?(user, comment)).to be - end - - scenario "Undoing flagging as inappropriate", :js do - skip "Feature not implemented yet, review soon" - - comment = create(:comment, commentable: poll) - Flag.flag(user, comment) - - login_as(user) - visit poll_path(poll) - - within "#comment_#{comment.id}" do - page.find("#unflag-expand-comment-#{comment.id}").click - page.find("#unflag-comment-#{comment.id}").click - - expect(page).to have_css("#flag-expand-comment-#{comment.id}") - end - - expect(Flag.flagged?(user, comment)).not_to be - end - - scenario "Flagging turbolinks sanity check", :js do - skip "Feature not implemented yet, review soon" - - poll = create(:poll, title: "Should we change the world?") - comment = create(:comment, commentable: poll) - - login_as(user) - visit polls_path - click_link "Should we change the world?" - - within "#comment_#{comment.id}" do - page.find("#flag-expand-comment-#{comment.id}").click - expect(page).to have_selector("#flag-comment-#{comment.id}") - end - end - scenario "Erasing a comment's author" do poll = create(:poll) comment = create(:comment, commentable: poll, body: "this should be visible") From 014ccd8374e39c30cc8fd3b9c243b221b1c40cb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 20 Mar 2020 19:03:57 +0100 Subject: [PATCH 06/10] Use shared specs to flag comments --- spec/factories/comments.rb | 6 + spec/shared/system/flaggable.rb | 39 ++++++- .../comments/budget_investments_spec.rb | 49 +------- spec/system/comments/debates_spec.rb | 76 +------------ .../comments/legislation_annotations_spec.rb | 54 +-------- .../comments/legislation_questions_spec.rb | 48 +------- spec/system/comments/proposals_spec.rb | 49 +------- spec/system/comments/topics_spec.rb | 106 +----------------- 8 files changed, 51 insertions(+), 376 deletions(-) diff --git a/spec/factories/comments.rb b/spec/factories/comments.rb index 3e35e0e2d..79402c448 100644 --- a/spec/factories/comments.rb +++ b/spec/factories/comments.rb @@ -4,6 +4,12 @@ FactoryBot.define do user sequence(:body) { |n| "Comment body #{n}" } + %i[budget_investment debate legislation_annotation legislation_question proposal topic_with_community].each do |model| + factory :"#{model}_comment" do + association :commentable, factory: model + end + end + trait :hidden do hidden_at { Time.current } end diff --git a/spec/shared/system/flaggable.rb b/spec/shared/system/flaggable.rb index da7c97417..b43527b2a 100644 --- a/spec/shared/system/flaggable.rb +++ b/spec/shared/system/flaggable.rb @@ -1,13 +1,22 @@ shared_examples "flaggable" do |factory_name| - let(:user) { create(:user) } + include ActionView::RecordIdentifier + + let(:user) { create(:user, :level_two) } let(:flaggable) { create(factory_name) } - let(:path) { polymorphic_path(flaggable) } + + let(:path) do + if flaggable.is_a?(Comment) + polymorphic_path(flaggable.commentable) + else + polymorphic_path(flaggable) + end + end scenario "Flagging as inappropriate", :js do login_as(user) visit path - within ".flag-content" do + within "##{dom_id(flaggable)} .flag-content" do find(".icon-flag").click click_link "Flag as inappropriate" @@ -24,7 +33,7 @@ shared_examples "flaggable" do |factory_name| login_as(user) visit path - within ".flag-content" do + within "##{dom_id(flaggable)} .flag-content" do expect(page).to have_css ".flag-active" find(".icon-flag").click @@ -41,7 +50,7 @@ shared_examples "flaggable" do |factory_name| login_as(user) visit path - within ".flag-content" do + within "##{dom_id(flaggable)} .flag-content" do find(".icon-flag").click click_link "Flag as inappropriate" @@ -56,4 +65,24 @@ shared_examples "flaggable" do |factory_name| expect(Flag.flagged?(user, flaggable)).not_to be end + + scenario "Flagging a comment with a child does not update its children", :js do + skip "Only for comments" unless flaggable.is_a?(Comment) + + child_comment = create(:comment, commentable: flaggable.commentable, parent: flaggable) + + login_as(user) + visit path + + within "##{dom_id(flaggable)} > .comment-body .flag-content" do + find(".icon-flag").click + click_link "Flag as inappropriate" + + expect(page).to have_css ".flag-active" + end + + within "##{dom_id(child_comment)} .flag-content" do + expect(page).not_to have_css ".flag-active" + end + end end diff --git a/spec/system/comments/budget_investments_spec.rb b/spec/system/comments/budget_investments_spec.rb index c2adbbbb4..667d4a970 100644 --- a/spec/system/comments/budget_investments_spec.rb +++ b/spec/system/comments/budget_investments_spec.rb @@ -4,6 +4,8 @@ describe "Commenting Budget::Investments" do let(:user) { create :user } let(:investment) { create :budget_investment } + it_behaves_like "flaggable", :budget_investment_comment + scenario "Index" do 3.times { create(:comment, commentable: investment) } create(:comment, :valuation, commentable: investment, subject: "Not viable") @@ -300,53 +302,6 @@ describe "Commenting Budget::Investments" do expect(page).to have_css(".comment.comment.comment.comment.comment.comment.comment.comment") end - scenario "Flagging as inappropriate", :js do - comment = create(:comment, commentable: investment) - - login_as(user) - visit budget_investment_path(investment.budget, investment) - - within "#comment_#{comment.id}" do - page.find("#flag-expand-comment-#{comment.id}").click - page.find("#flag-comment-#{comment.id}").click - - expect(page).to have_css("#unflag-expand-comment-#{comment.id}") - end - - expect(Flag.flagged?(user, comment)).to be - end - - scenario "Undoing flagging as inappropriate", :js do - comment = create(:comment, commentable: investment) - Flag.flag(user, comment) - - login_as(user) - visit budget_investment_path(investment.budget, investment) - - within "#comment_#{comment.id}" do - page.find("#unflag-expand-comment-#{comment.id}").click - page.find("#unflag-comment-#{comment.id}").click - - expect(page).to have_css("#flag-expand-comment-#{comment.id}") - end - - expect(Flag.flagged?(user, comment)).not_to be - end - - scenario "Flagging turbolinks sanity check", :js do - investment = create(:budget_investment, title: "Should we change the world?") - comment = create(:comment, commentable: investment) - - login_as(user) - visit budget_investments_path(investment.budget) - click_link "Should we change the world?" - - within "#comment_#{comment.id}" do - page.find("#flag-expand-comment-#{comment.id}").click - expect(page).to have_selector("#flag-comment-#{comment.id}") - end - end - scenario "Erasing a comment's author" do investment = create(:budget_investment) comment = create(:comment, commentable: investment, body: "this should be visible") diff --git a/spec/system/comments/debates_spec.rb b/spec/system/comments/debates_spec.rb index d486c9284..f3e38cfb4 100644 --- a/spec/system/comments/debates_spec.rb +++ b/spec/system/comments/debates_spec.rb @@ -4,6 +4,8 @@ describe "Commenting debates" do let(:user) { create :user } let(:debate) { create :debate } + it_behaves_like "flaggable", :debate_comment + scenario "Index" do 3.times { create(:comment, commentable: debate) } @@ -338,80 +340,6 @@ describe "Commenting debates" do expect(page).to have_css(".comment.comment.comment.comment.comment.comment.comment.comment") end - scenario "Flagging as inappropriate", :js do - comment = create(:comment, commentable: debate) - - login_as(user) - visit debate_path(debate) - - within "#comment_#{comment.id}" do - page.find("#flag-expand-comment-#{comment.id}").click - page.find("#flag-comment-#{comment.id}").click - - expect(page).to have_css("#unflag-expand-comment-#{comment.id}") - end - - expect(Flag.flagged?(user, comment)).to be - end - - scenario "Undoing flagging as inappropriate", :js do - comment = create(:comment, commentable: debate) - Flag.flag(user, comment) - - login_as(user) - visit debate_path(debate) - - within "#comment_#{comment.id}" do - page.find("#unflag-expand-comment-#{comment.id}").click - page.find("#unflag-comment-#{comment.id}").click - - expect(page).to have_css("#flag-expand-comment-#{comment.id}") - end - - expect(Flag.flagged?(user, comment)).not_to be - end - - scenario "Flagging turbolinks sanity check", :js do - debate = create(:debate, title: "Should we change the world?") - comment = create(:comment, commentable: debate) - - login_as(user) - visit debates_path - click_link "Should we change the world?" - - within "#comment_#{comment.id}" do - page.find("#flag-expand-comment-#{comment.id}").click - page.find("#flag-comment-#{comment.id}").click - - expect(page).to have_css("#unflag-expand-comment-#{comment.id}") - expect(Flag.flagged?(user, comment)).to be - - page.find("#unflag-expand-comment-#{comment.id}").click - page.find("#unflag-comment-#{comment.id}").click - - expect(page).to have_css("#flag-expand-comment-#{comment.id}") - end - - expect(Flag.flagged?(user, comment)).not_to be - end - - scenario "Flagging a comment with a child does not update its children", :js do - debate = create(:debate, title: "Should we change the world?") - parent_comment = create(:comment, commentable: debate, body: "Main comment") - child_comment = create(:comment, body: "First subcomment", commentable: debate, parent: parent_comment) - - login_as(user) - visit debate_path(debate) - - within "#comment_#{parent_comment.id}" do - page.find("#flag-expand-comment-#{parent_comment.id}").click - page.find("#flag-comment-#{parent_comment.id}").click - - expect(page).to have_css("#unflag-expand-comment-#{parent_comment.id}") - expect(page).to have_css("#flag-expand-comment-#{child_comment.id}") - end - end - scenario "Erasing a comment's author" do debate = create(:debate) comment = create(:comment, commentable: debate, body: "this should be visible") diff --git a/spec/system/comments/legislation_annotations_spec.rb b/spec/system/comments/legislation_annotations_spec.rb index 4e0663ca6..46f9d5baa 100644 --- a/spec/system/comments/legislation_annotations_spec.rb +++ b/spec/system/comments/legislation_annotations_spec.rb @@ -4,6 +4,8 @@ describe "Commenting legislation questions" do let(:user) { create :user } let(:legislation_annotation) { create :legislation_annotation, author: user } + it_behaves_like "flaggable", :legislation_annotation_comment + scenario "Index" do 3.times { create(:comment, commentable: legislation_annotation) } @@ -349,58 +351,6 @@ describe "Commenting legislation questions" do expect(page).to have_css(".comment.comment.comment.comment.comment.comment.comment.comment") end - scenario "Flagging as inappropriate", :js do - comment = create(:comment, commentable: legislation_annotation) - - login_as(user) - visit legislation_process_draft_version_annotation_path(legislation_annotation.draft_version.process, - legislation_annotation.draft_version, - legislation_annotation) - - within "#comment_#{comment.id}" do - page.find("#flag-expand-comment-#{comment.id}").click - page.find("#flag-comment-#{comment.id}").click - - expect(page).to have_css("#unflag-expand-comment-#{comment.id}") - end - - expect(Flag.flagged?(user, comment)).to be - end - - scenario "Undoing flagging as inappropriate", :js do - comment = create(:comment, commentable: legislation_annotation) - Flag.flag(user, comment) - - login_as(user) - visit legislation_process_draft_version_annotation_path(legislation_annotation.draft_version.process, - legislation_annotation.draft_version, - legislation_annotation) - - within "#comment_#{comment.id}" do - page.find("#unflag-expand-comment-#{comment.id}").click - page.find("#unflag-comment-#{comment.id}").click - - expect(page).to have_css("#flag-expand-comment-#{comment.id}") - end - - expect(Flag.flagged?(user, comment)).not_to be - end - - scenario "Flagging turbolinks sanity check", :js do - legislation_annotation = create(:legislation_annotation, text: "Should we change the world?") - comment = create(:comment, commentable: legislation_annotation) - - login_as(user) - visit legislation_process_draft_version_annotation_path(legislation_annotation.draft_version.process, - legislation_annotation.draft_version, - legislation_annotation) - - within "#comment_#{comment.id}" do - page.find("#flag-expand-comment-#{comment.id}").click - expect(page).to have_selector("#flag-comment-#{comment.id}") - end - end - scenario "Erasing a comment's author" do legislation_annotation = create(:legislation_annotation) comment = create(:comment, commentable: legislation_annotation, body: "this should be visible") diff --git a/spec/system/comments/legislation_questions_spec.rb b/spec/system/comments/legislation_questions_spec.rb index 6f55c7d49..1750e9422 100644 --- a/spec/system/comments/legislation_questions_spec.rb +++ b/spec/system/comments/legislation_questions_spec.rb @@ -7,6 +7,7 @@ describe "Commenting legislation questions" do context "Concerns" do it_behaves_like "notifiable in-app", :legislation_question + it_behaves_like "flaggable", :legislation_question_comment end scenario "Index" do @@ -320,53 +321,6 @@ describe "Commenting legislation questions" do expect(page).to have_css(".comment.comment.comment.comment.comment.comment.comment.comment") end - scenario "Flagging as inappropriate", :js do - comment = create(:comment, commentable: legislation_question) - - login_as(user) - visit legislation_process_question_path(legislation_question.process, legislation_question) - - within "#comment_#{comment.id}" do - page.find("#flag-expand-comment-#{comment.id}").click - page.find("#flag-comment-#{comment.id}").click - - expect(page).to have_css("#unflag-expand-comment-#{comment.id}") - end - - expect(Flag.flagged?(user, comment)).to be - end - - scenario "Undoing flagging as inappropriate", :js do - comment = create(:comment, commentable: legislation_question) - Flag.flag(user, comment) - - login_as(user) - visit legislation_process_question_path(legislation_question.process, legislation_question) - - within "#comment_#{comment.id}" do - page.find("#unflag-expand-comment-#{comment.id}").click - page.find("#unflag-comment-#{comment.id}").click - - expect(page).to have_css("#flag-expand-comment-#{comment.id}") - end - - expect(Flag.flagged?(user, comment)).not_to be - end - - scenario "Flagging turbolinks sanity check", :js do - legislation_question = create(:legislation_question, process: process, title: "Should we change the world?") - comment = create(:comment, commentable: legislation_question) - - login_as(user) - visit legislation_process_path(legislation_question.process) - click_link "Should we change the world?" - - within "#comment_#{comment.id}" do - page.find("#flag-expand-comment-#{comment.id}").click - expect(page).to have_selector("#flag-comment-#{comment.id}") - end - end - scenario "Erasing a comment's author" do comment = create(:comment, commentable: legislation_question, body: "this should be visible") comment.user.erase diff --git a/spec/system/comments/proposals_spec.rb b/spec/system/comments/proposals_spec.rb index 02b08440d..89fa11b08 100644 --- a/spec/system/comments/proposals_spec.rb +++ b/spec/system/comments/proposals_spec.rb @@ -4,6 +4,8 @@ describe "Commenting proposals" do let(:user) { create :user } let(:proposal) { create :proposal } + it_behaves_like "flaggable", :proposal_comment + scenario "Index" do 3.times { create(:comment, commentable: proposal) } @@ -296,53 +298,6 @@ describe "Commenting proposals" do expect(page).to have_css(".comment.comment.comment.comment.comment.comment.comment.comment") end - scenario "Flagging as inappropriate", :js do - comment = create(:comment, commentable: proposal) - - login_as(user) - visit proposal_path(proposal) - - within "#comment_#{comment.id}" do - page.find("#flag-expand-comment-#{comment.id}").click - page.find("#flag-comment-#{comment.id}").click - - expect(page).to have_css("#unflag-expand-comment-#{comment.id}") - end - - expect(Flag.flagged?(user, comment)).to be - end - - scenario "Undoing flagging as inappropriate", :js do - comment = create(:comment, commentable: proposal) - Flag.flag(user, comment) - - login_as(user) - visit proposal_path(proposal) - - within "#comment_#{comment.id}" do - page.find("#unflag-expand-comment-#{comment.id}").click - page.find("#unflag-comment-#{comment.id}").click - - expect(page).to have_css("#flag-expand-comment-#{comment.id}") - end - - expect(Flag.flagged?(user, comment)).not_to be - end - - scenario "Flagging turbolinks sanity check", :js do - proposal = create(:proposal, title: "Should we change the world?") - comment = create(:comment, commentable: proposal) - - login_as(user) - visit proposals_path - click_link "Should we change the world?" - - within "#comment_#{comment.id}" do - page.find("#flag-expand-comment-#{comment.id}").click - expect(page).to have_selector("#flag-comment-#{comment.id}") - end - end - scenario "Erasing a comment's author" do proposal = create(:proposal) comment = create(:comment, commentable: proposal, body: "this should be visible") diff --git a/spec/system/comments/topics_spec.rb b/spec/system/comments/topics_spec.rb index 377fc7236..c7537814c 100644 --- a/spec/system/comments/topics_spec.rb +++ b/spec/system/comments/topics_spec.rb @@ -4,6 +4,8 @@ describe "Commenting topics from proposals" do let(:user) { create :user } let(:proposal) { create :proposal } + it_behaves_like "flaggable", :topic_with_community_comment + scenario "Index" do community = proposal.community topic = create(:topic, community: community) @@ -330,58 +332,6 @@ describe "Commenting topics from proposals" do expect(page).to have_css(".comment.comment.comment.comment.comment.comment.comment.comment") end - scenario "Flagging as inappropriate", :js do - community = proposal.community - topic = create(:topic, community: community) - comment = create(:comment, commentable: topic) - - login_as(user) - visit community_topic_path(community, topic) - - within "#comment_#{comment.id}" do - page.find("#flag-expand-comment-#{comment.id}").click - page.find("#flag-comment-#{comment.id}").click - - expect(page).to have_css("#unflag-expand-comment-#{comment.id}") - end - - expect(Flag.flagged?(user, comment)).to be - end - - scenario "Undoing flagging as inappropriate", :js do - community = proposal.community - topic = create(:topic, community: community) - comment = create(:comment, commentable: topic) - Flag.flag(user, comment) - - login_as(user) - visit community_topic_path(community, topic) - - within "#comment_#{comment.id}" do - page.find("#unflag-expand-comment-#{comment.id}").click - page.find("#unflag-comment-#{comment.id}").click - - expect(page).to have_css("#flag-expand-comment-#{comment.id}") - end - - expect(Flag.flagged?(user, comment)).not_to be - end - - scenario "Flagging turbolinks sanity check", :js do - community = proposal.community - topic = create(:topic, community: community, title: "Should we change the world?") - comment = create(:comment, commentable: topic) - - login_as(user) - visit community_path(community) - click_link "Should we change the world?" - - within "#comment_#{comment.id}" do - page.find("#flag-expand-comment-#{comment.id}").click - expect(page).to have_selector("#flag-comment-#{comment.id}") - end - end - scenario "Erasing a comment's author" do community = proposal.community topic = create(:topic, community: community) @@ -890,58 +840,6 @@ describe "Commenting topics from budget investments" do expect(page).to have_css(".comment.comment.comment.comment.comment.comment.comment.comment") end - scenario "Flagging as inappropriate", :js do - community = investment.community - topic = create(:topic, community: community) - comment = create(:comment, commentable: topic) - - login_as(user) - visit community_topic_path(community, topic) - - within "#comment_#{comment.id}" do - page.find("#flag-expand-comment-#{comment.id}").click - page.find("#flag-comment-#{comment.id}").click - - expect(page).to have_css("#unflag-expand-comment-#{comment.id}") - end - - expect(Flag.flagged?(user, comment)).to be - end - - scenario "Undoing flagging as inappropriate", :js do - community = investment.community - topic = create(:topic, community: community) - comment = create(:comment, commentable: topic) - Flag.flag(user, comment) - - login_as(user) - visit community_topic_path(community, topic) - - within "#comment_#{comment.id}" do - page.find("#unflag-expand-comment-#{comment.id}").click - page.find("#unflag-comment-#{comment.id}").click - - expect(page).to have_css("#flag-expand-comment-#{comment.id}") - end - - expect(Flag.flagged?(user, comment)).not_to be - end - - scenario "Flagging turbolinks sanity check", :js do - community = investment.community - topic = create(:topic, community: community, title: "Should we change the world?") - comment = create(:comment, commentable: topic) - - login_as(user) - visit community_path(community) - click_link "Should we change the world?" - - within "#comment_#{comment.id}" do - page.find("#flag-expand-comment-#{comment.id}").click - expect(page).to have_selector("#flag-comment-#{comment.id}") - end - end - scenario "Erasing a comment's author" do community = investment.community topic = create(:topic, community: community) From bd7beed8a18eb0aa079cf15c3960b5cdf36acac3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 20 Mar 2020 19:27:11 +0100 Subject: [PATCH 07/10] Remove no longer necessary flag/unflag HTML IDs They were added in commit 015fe704 because we used them in the specs, but we don't use them anymore and they make the code hard to read. --- app/views/budgets/investments/_flag_actions.html.erb | 10 ++++------ app/views/comments/_flag_actions.html.erb | 12 ++++-------- app/views/debates/_flag_actions.html.erb | 8 ++++---- .../legislation/proposals/_flag_actions.html.erb | 8 ++++---- app/views/proposals/_flag_actions.html.erb | 8 ++++---- 5 files changed, 20 insertions(+), 26 deletions(-) diff --git a/app/views/budgets/investments/_flag_actions.html.erb b/app/views/budgets/investments/_flag_actions.html.erb index d00bbe6b9..44f384c3b 100644 --- a/app/views/budgets/investments/_flag_actions.html.erb +++ b/app/views/budgets/investments/_flag_actions.html.erb @@ -1,25 +1,23 @@ <% if show_flag_action? investment %> - "> + "> <%= link_to t("shared.flag"), flag_budget_investment_path(investment.budget, investment.id), method: :put, - remote: true, - id: "flag-investment-#{investment.id}" %> + remote: true %> <% end %> <% if show_unflag_action? investment %> - "> + "> <%= link_to t("shared.unflag"), unflag_budget_investment_path(investment.budget, investment.id), method: :put, - remote: true, - id: "unflag-investment-#{investment.id}" %> + remote: true %> <% end %> diff --git a/app/views/comments/_flag_actions.html.erb b/app/views/comments/_flag_actions.html.erb index cac924994..16e50f93b 100644 --- a/app/views/comments/_flag_actions.html.erb +++ b/app/views/comments/_flag_actions.html.erb @@ -1,9 +1,7 @@ <% if show_flag_action? comment %>  |  - "> + "> <%= link_to t("shared.flag"), flag_comment_path(comment), method: :put, - remote: true, id: "flag-comment-#{comment.id}" %> + remote: true %> <% end %> <% if show_unflag_action? comment %>  |  - "> + "> <%= link_to t("shared.unflag"), unflag_comment_path(comment), method: :put, - remote: true, id: "unflag-comment-#{comment.id}" %> + remote: true %> <% end %> diff --git a/app/views/debates/_flag_actions.html.erb b/app/views/debates/_flag_actions.html.erb index 66d4e9179..e6b3308df 100644 --- a/app/views/debates/_flag_actions.html.erb +++ b/app/views/debates/_flag_actions.html.erb @@ -1,19 +1,19 @@ <% if show_flag_action? debate %> - "> + "> - <%= link_to t("shared.flag"), flag_debate_path(debate), method: :put, remote: true, id: "flag-debate-#{debate.id}" %> + <%= link_to t("shared.flag"), flag_debate_path(debate), method: :put, remote: true %> <% end %> <% if show_unflag_action? debate %> - "> + "> - <%= link_to t("shared.unflag"), unflag_debate_path(debate), method: :put, remote: true, id: "unflag-debate-#{debate.id}" %> + <%= link_to t("shared.unflag"), unflag_debate_path(debate), method: :put, remote: true %> <% end %> diff --git a/app/views/legislation/proposals/_flag_actions.html.erb b/app/views/legislation/proposals/_flag_actions.html.erb index 6d86b63a2..10a7892cc 100644 --- a/app/views/legislation/proposals/_flag_actions.html.erb +++ b/app/views/legislation/proposals/_flag_actions.html.erb @@ -1,19 +1,19 @@ <% if show_flag_action? proposal %> - "> + "> - <%= link_to t("shared.flag"), flag_legislation_process_proposal_path(proposal.process, proposal), method: :put, remote: true, id: "flag-proposal-#{proposal.id}" %> + <%= link_to t("shared.flag"), flag_legislation_process_proposal_path(proposal.process, proposal), method: :put, remote: true %> <% end %> <% if show_unflag_action? proposal %> - "> + "> - <%= link_to t("shared.unflag"), unflag_legislation_process_proposal_path(proposal.process, proposal), method: :put, remote: true, id: "unflag-proposal-#{proposal.id}" %> + <%= link_to t("shared.unflag"), unflag_legislation_process_proposal_path(proposal.process, proposal), method: :put, remote: true %> <% end %> diff --git a/app/views/proposals/_flag_actions.html.erb b/app/views/proposals/_flag_actions.html.erb index ec012e3df..ed8e7aab0 100644 --- a/app/views/proposals/_flag_actions.html.erb +++ b/app/views/proposals/_flag_actions.html.erb @@ -1,19 +1,19 @@ <% if show_flag_action? proposal %> - "> + "> - <%= link_to t("shared.flag"), flag_proposal_path(proposal), method: :put, remote: true, id: "flag-proposal-#{proposal.id}" %> + <%= link_to t("shared.flag"), flag_proposal_path(proposal), method: :put, remote: true %> <% end %> <% if show_unflag_action? proposal %> - "> + "> - <%= link_to t("shared.unflag"), unflag_proposal_path(proposal), method: :put, remote: true, id: "unflag-proposal-#{proposal.id}" %> + <%= link_to t("shared.unflag"), unflag_proposal_path(proposal), method: :put, remote: true %> <% end %> From 31b65679c37d3b8d0fc35ef8a40a099a955da1c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 20 Mar 2020 19:58:17 +0100 Subject: [PATCH 08/10] Extract partial to render flag actions The main obstacle to extract this partial was probably the paths for the flag and unflag actions. Now that we use Rails 5.1 `resolve` method to handle nested resources, we can use `polymorphic_path`. Also note the code is a bit ugly because comments render a divider. We should probably use a CSS border instead. Co-Authored-By: taitus --- app/views/admin/debates/show.html.erb | 4 +-- .../investments/_flag_actions.html.erb | 23 ---------------- .../investments/_investment_detail.erb | 2 +- .../investments/_refresh_flag_actions.js.erb | 2 +- app/views/comments/_actions.html.erb | 2 +- app/views/comments/_flag_actions.html.erb | 26 ------------------ .../comments/_refresh_flag_actions.js.erb | 2 +- app/views/debates/_flag_actions.html.erb | 19 ------------- .../debates/_refresh_flag_actions.js.erb | 2 +- app/views/debates/show.html.erb | 2 +- .../proposals/_flag_actions.html.erb | 19 ------------- .../proposals/_refresh_flag_actions.js.erb | 2 +- app/views/legislation/proposals/show.html.erb | 2 +- app/views/proposals/_flag_actions.html.erb | 19 ------------- app/views/proposals/_info.html.erb | 2 +- .../proposals/_refresh_flag_actions.js.erb | 3 +-- app/views/shared/_flag_actions.html.erb | 27 +++++++++++++++++++ 17 files changed, 39 insertions(+), 119 deletions(-) delete mode 100644 app/views/budgets/investments/_flag_actions.html.erb delete mode 100644 app/views/comments/_flag_actions.html.erb delete mode 100644 app/views/debates/_flag_actions.html.erb delete mode 100644 app/views/legislation/proposals/_flag_actions.html.erb delete mode 100644 app/views/proposals/_flag_actions.html.erb create mode 100644 app/views/shared/_flag_actions.html.erb diff --git a/app/views/admin/debates/show.html.erb b/app/views/admin/debates/show.html.erb index c0f70cf73..b92110e6b 100644 --- a/app/views/admin/debates/show.html.erb +++ b/app/views/admin/debates/show.html.erb @@ -21,8 +21,8 @@ <%= link_to t("debates.show.comments", count: @debate.comments_count), "#comments" %>  •  - <%= render "debates/flag_actions", debate: @debate %> - + <%= render "shared/flag_actions", flaggable: @debate %> + <%= auto_link_already_sanitized_html wysiwyg(@debate.description) %> diff --git a/app/views/budgets/investments/_flag_actions.html.erb b/app/views/budgets/investments/_flag_actions.html.erb deleted file mode 100644 index 44f384c3b..000000000 --- a/app/views/budgets/investments/_flag_actions.html.erb +++ /dev/null @@ -1,23 +0,0 @@ - - <% if show_flag_action? investment %> - "> - - - - <%= link_to t("shared.flag"), flag_budget_investment_path(investment.budget, investment.id), - method: :put, - remote: true %> - - <% end %> - - <% if show_unflag_action? investment %> - "> - - - - <%= link_to t("shared.unflag"), unflag_budget_investment_path(investment.budget, investment.id), - method: :put, - remote: true %> - - <% end %> - diff --git a/app/views/budgets/investments/_investment_detail.erb b/app/views/budgets/investments/_investment_detail.erb index 80e5dd5e5..28b8e8996 100644 --- a/app/views/budgets/investments/_investment_detail.erb +++ b/app/views/budgets/investments/_investment_detail.erb @@ -9,7 +9,7 @@  •  <% if local_assigns[:preview].nil? %> - <%= render "budgets/investments/flag_actions", investment: investment %> + <%= render "shared/flag_actions", flaggable: investment %> <% end %> diff --git a/app/views/budgets/investments/_refresh_flag_actions.js.erb b/app/views/budgets/investments/_refresh_flag_actions.js.erb index 07cf8d30a..96062f79c 100644 --- a/app/views/budgets/investments/_refresh_flag_actions.js.erb +++ b/app/views/budgets/investments/_refresh_flag_actions.js.erb @@ -1 +1 @@ -App.Flaggable.update("<%= dom_id(@investment) %>", "<%= j render("budgets/investments/flag_actions", investment: @investment) %>") +App.Flaggable.update("<%= dom_id(@investment) %>", "<%= j render("shared/flag_actions", flaggable: @investment) %>") diff --git a/app/views/comments/_actions.html.erb b/app/views/comments/_actions.html.erb index 76aed5b5a..166d05c12 100644 --- a/app/views/comments/_actions.html.erb +++ b/app/views/comments/_actions.html.erb @@ -1,5 +1,5 @@ - <%= render "comments/flag_actions", comment: comment %> + <%= render "shared/flag_actions", flaggable: comment, divider: true %> diff --git a/app/views/comments/_flag_actions.html.erb b/app/views/comments/_flag_actions.html.erb deleted file mode 100644 index 16e50f93b..000000000 --- a/app/views/comments/_flag_actions.html.erb +++ /dev/null @@ -1,26 +0,0 @@ - - <% if show_flag_action? comment %> -  |  - "> - - - - <%= link_to t("shared.flag"), flag_comment_path(comment), method: :put, - remote: true %> - - <% end %> - - <% if show_unflag_action? comment %> -  |  - "> - - - - <%= link_to t("shared.unflag"), unflag_comment_path(comment), method: :put, - remote: true %> - - <% end %> - diff --git a/app/views/comments/_refresh_flag_actions.js.erb b/app/views/comments/_refresh_flag_actions.js.erb index f6ce0bc91..991bb221e 100644 --- a/app/views/comments/_refresh_flag_actions.js.erb +++ b/app/views/comments/_refresh_flag_actions.js.erb @@ -1 +1 @@ -App.Flaggable.update("<%= dom_id(@comment) %>", "<%= j render("comments/flag_actions", comment: @comment) %>"); +App.Flaggable.update("<%= dom_id(@comment) %>", "<%= j render("shared/flag_actions", flaggable: @comment, divider: true) %>"); diff --git a/app/views/debates/_flag_actions.html.erb b/app/views/debates/_flag_actions.html.erb deleted file mode 100644 index e6b3308df..000000000 --- a/app/views/debates/_flag_actions.html.erb +++ /dev/null @@ -1,19 +0,0 @@ - - <% if show_flag_action? debate %> - "> - - - - <%= link_to t("shared.flag"), flag_debate_path(debate), method: :put, remote: true %> - - <% end %> - - <% if show_unflag_action? debate %> - "> - - - - <%= link_to t("shared.unflag"), unflag_debate_path(debate), method: :put, remote: true %> - - <% end %> - diff --git a/app/views/debates/_refresh_flag_actions.js.erb b/app/views/debates/_refresh_flag_actions.js.erb index 2c1e1701a..297a9059b 100644 --- a/app/views/debates/_refresh_flag_actions.js.erb +++ b/app/views/debates/_refresh_flag_actions.js.erb @@ -1 +1 @@ -App.Flaggable.update("<%= dom_id(@debate) %>", "<%= j render("debates/flag_actions", debate: @debate) %>"); +App.Flaggable.update("<%= dom_id(@debate) %>", "<%= j render("shared/flag_actions", flaggable: @debate) %>"); diff --git a/app/views/debates/show.html.erb b/app/views/debates/show.html.erb index 4b7211fdb..6889122f8 100644 --- a/app/views/debates/show.html.erb +++ b/app/views/debates/show.html.erb @@ -26,7 +26,7 @@ <%= link_to t("debates.show.comments", count: @debate.comments_count), "#comments" %>  •  - <%= render "debates/flag_actions", debate: @debate %> + <%= render "shared/flag_actions", flaggable: @debate %> diff --git a/app/views/legislation/proposals/_flag_actions.html.erb b/app/views/legislation/proposals/_flag_actions.html.erb deleted file mode 100644 index 10a7892cc..000000000 --- a/app/views/legislation/proposals/_flag_actions.html.erb +++ /dev/null @@ -1,19 +0,0 @@ - - <% if show_flag_action? proposal %> - "> - - - - <%= link_to t("shared.flag"), flag_legislation_process_proposal_path(proposal.process, proposal), method: :put, remote: true %> - - <% end %> - - <% if show_unflag_action? proposal %> - "> - - - - <%= link_to t("shared.unflag"), unflag_legislation_process_proposal_path(proposal.process, proposal), method: :put, remote: true %> - - <% end %> - diff --git a/app/views/legislation/proposals/_refresh_flag_actions.js.erb b/app/views/legislation/proposals/_refresh_flag_actions.js.erb index a9bbab7e3..38cb6a3c4 100644 --- a/app/views/legislation/proposals/_refresh_flag_actions.js.erb +++ b/app/views/legislation/proposals/_refresh_flag_actions.js.erb @@ -1 +1 @@ -App.Flaggable.update("<%= dom_id(@proposal) %>", "<%= j render("legislation/proposals/flag_actions", proposal: @proposal) %>"); +App.Flaggable.update("<%= dom_id(@proposal) %>", "<%= j render("shared/flag_actions", flaggable: @proposal) %>"); diff --git a/app/views/legislation/proposals/show.html.erb b/app/views/legislation/proposals/show.html.erb index fd6c2280c..fdf7347a7 100644 --- a/app/views/legislation/proposals/show.html.erb +++ b/app/views/legislation/proposals/show.html.erb @@ -46,7 +46,7 @@ <% if current_user %>  •  - <%= render "legislation/proposals/flag_actions", proposal: @proposal %> + <%= render "shared/flag_actions", flaggable: @proposal %> <% end %> diff --git a/app/views/proposals/_flag_actions.html.erb b/app/views/proposals/_flag_actions.html.erb deleted file mode 100644 index ed8e7aab0..000000000 --- a/app/views/proposals/_flag_actions.html.erb +++ /dev/null @@ -1,19 +0,0 @@ - - <% if show_flag_action? proposal %> - "> - - - - <%= link_to t("shared.flag"), flag_proposal_path(proposal), method: :put, remote: true %> - - <% end %> - - <% if show_unflag_action? proposal %> - "> - - - - <%= link_to t("shared.unflag"), unflag_proposal_path(proposal), method: :put, remote: true %> - - <% end %> - diff --git a/app/views/proposals/_info.html.erb b/app/views/proposals/_info.html.erb index f6c5e07b3..15467348b 100644 --- a/app/views/proposals/_info.html.erb +++ b/app/views/proposals/_info.html.erb @@ -13,7 +13,7 @@ <% if current_user %>  •  - <%= render "proposals/flag_actions", proposal: @proposal %> + <%= render "shared/flag_actions", flaggable: @proposal %> <% end %> diff --git a/app/views/proposals/_refresh_flag_actions.js.erb b/app/views/proposals/_refresh_flag_actions.js.erb index e89e316c2..e60ee63b7 100644 --- a/app/views/proposals/_refresh_flag_actions.js.erb +++ b/app/views/proposals/_refresh_flag_actions.js.erb @@ -1,2 +1 @@ -App.Flaggable.update("<%= dom_id(@proposal) %>", - "<%= j render("proposals/flag_actions", proposal: @proposal) %>") +App.Flaggable.update("<%= dom_id(@proposal) %>", "<%= j render("shared/flag_actions", flaggable: @proposal) %>") diff --git a/app/views/shared/_flag_actions.html.erb b/app/views/shared/_flag_actions.html.erb new file mode 100644 index 000000000..fbd2c4ab2 --- /dev/null +++ b/app/views/shared/_flag_actions.html.erb @@ -0,0 +1,27 @@ + + <% if show_flag_action?(flaggable) %> + <% if local_assigns[:divider] %> +  |  + <% end %> + + "> + + + + <%= link_to t("shared.flag"), polymorphic_path(flaggable, action: :flag), method: :put, remote: true %> + + <% end %> + + <% if show_unflag_action?(flaggable) %> + <% if local_assigns[:divider] %> +  |  + <% end %> + + "> + + + + <%= link_to t("shared.unflag"), polymorphic_path(flaggable, action: :unflag), method: :put, remote: true %> + + <% end %> + From 4f30720593dd9237ad7d89ac6b47a626ba12cfdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 7 Jul 2020 21:45:40 +0200 Subject: [PATCH 09/10] Fix flagging/unflagging in the admin section We weren't adding the HTML id our JavaScript expects, and so the page didn't update the flag element. --- app/views/admin/debates/show.html.erb | 2 +- spec/shared/system/flaggable.rb | 13 +++++++++++-- spec/system/admin/debates_spec.rb | 2 ++ 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/app/views/admin/debates/show.html.erb b/app/views/admin/debates/show.html.erb index b92110e6b..7a60a117b 100644 --- a/app/views/admin/debates/show.html.erb +++ b/app/views/admin/debates/show.html.erb @@ -2,7 +2,7 @@ <%= t("admin.header.title") %> - <%= t("admin.menu.debates") %> - <%= @debate.title %> <% end %> -
+

<%= @debate.title %>

<% if @debate.conflictive? %> diff --git a/spec/shared/system/flaggable.rb b/spec/shared/system/flaggable.rb index b43527b2a..68e9301e8 100644 --- a/spec/shared/system/flaggable.rb +++ b/spec/shared/system/flaggable.rb @@ -1,12 +1,21 @@ -shared_examples "flaggable" do |factory_name| +shared_examples "flaggable" do |factory_name, admin: false| include ActionView::RecordIdentifier - let(:user) { create(:user, :level_two) } let(:flaggable) { create(factory_name) } + let(:user) do + if admin + create(:administrator).user + else + create(:user, :level_two) + end + end + let(:path) do if flaggable.is_a?(Comment) polymorphic_path(flaggable.commentable) + elsif admin + admin_polymorphic_path(flaggable) else polymorphic_path(flaggable) end diff --git a/spec/system/admin/debates_spec.rb b/spec/system/admin/debates_spec.rb index 38795295d..c1df1a281 100644 --- a/spec/system/admin/debates_spec.rb +++ b/spec/system/admin/debates_spec.rb @@ -6,6 +6,8 @@ describe "Admin debates" do login_as(admin.user) end + it_behaves_like "flaggable", :debate, admin: true + scenario "Index" do create(:debate, title: "Best beaches") From a5f1245b7e91590a5ebfeed2336cac70814b4ad2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 7 Jul 2020 22:22:22 +0200 Subject: [PATCH 10/10] Extract partial to refresh flag actions Now that we're rendering `shared/flag_actions` everywhere, we can use the same code in all cases. --- app/controllers/comments_controller.rb | 4 ++-- app/controllers/concerns/flag_actions.rb | 4 ++-- app/views/budgets/investments/_refresh_flag_actions.js.erb | 1 - app/views/comments/_refresh_flag_actions.js.erb | 1 - app/views/debates/_refresh_flag_actions.js.erb | 1 - app/views/legislation/proposals/_refresh_flag_actions.js.erb | 1 - app/views/proposals/_refresh_flag_actions.js.erb | 1 - app/views/shared/_refresh_flag_actions.js.erb | 3 +++ 8 files changed, 7 insertions(+), 9 deletions(-) delete mode 100644 app/views/budgets/investments/_refresh_flag_actions.js.erb delete mode 100644 app/views/comments/_refresh_flag_actions.js.erb delete mode 100644 app/views/debates/_refresh_flag_actions.js.erb delete mode 100644 app/views/legislation/proposals/_refresh_flag_actions.js.erb delete mode 100644 app/views/proposals/_refresh_flag_actions.js.erb create mode 100644 app/views/shared/_refresh_flag_actions.js.erb diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 49dee85e9..3839475d1 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -36,14 +36,14 @@ class CommentsController < ApplicationController Flag.flag(current_user, @comment) set_comment_flags(@comment) - render "comments/_refresh_flag_actions" + render "shared/_refresh_flag_actions", locals: { flaggable: @comment, divider: true } end def unflag Flag.unflag(current_user, @comment) set_comment_flags(@comment) - render "comments/_refresh_flag_actions" + render "shared/_refresh_flag_actions", locals: { flaggable: @comment, divider: true } end private diff --git a/app/controllers/concerns/flag_actions.rb b/app/controllers/concerns/flag_actions.rb index 797b94778..acbc96882 100644 --- a/app/controllers/concerns/flag_actions.rb +++ b/app/controllers/concerns/flag_actions.rb @@ -4,13 +4,13 @@ module FlagActions def flag Flag.flag(current_user, flaggable) - render "_refresh_flag_actions" + render "shared/_refresh_flag_actions", locals: { flaggable: flaggable } end def unflag Flag.unflag(current_user, flaggable) - render "_refresh_flag_actions" + render "shared/_refresh_flag_actions", locals: { flaggable: flaggable } end private diff --git a/app/views/budgets/investments/_refresh_flag_actions.js.erb b/app/views/budgets/investments/_refresh_flag_actions.js.erb deleted file mode 100644 index 96062f79c..000000000 --- a/app/views/budgets/investments/_refresh_flag_actions.js.erb +++ /dev/null @@ -1 +0,0 @@ -App.Flaggable.update("<%= dom_id(@investment) %>", "<%= j render("shared/flag_actions", flaggable: @investment) %>") diff --git a/app/views/comments/_refresh_flag_actions.js.erb b/app/views/comments/_refresh_flag_actions.js.erb deleted file mode 100644 index 991bb221e..000000000 --- a/app/views/comments/_refresh_flag_actions.js.erb +++ /dev/null @@ -1 +0,0 @@ -App.Flaggable.update("<%= dom_id(@comment) %>", "<%= j render("shared/flag_actions", flaggable: @comment, divider: true) %>"); diff --git a/app/views/debates/_refresh_flag_actions.js.erb b/app/views/debates/_refresh_flag_actions.js.erb deleted file mode 100644 index 297a9059b..000000000 --- a/app/views/debates/_refresh_flag_actions.js.erb +++ /dev/null @@ -1 +0,0 @@ -App.Flaggable.update("<%= dom_id(@debate) %>", "<%= j render("shared/flag_actions", flaggable: @debate) %>"); diff --git a/app/views/legislation/proposals/_refresh_flag_actions.js.erb b/app/views/legislation/proposals/_refresh_flag_actions.js.erb deleted file mode 100644 index 38cb6a3c4..000000000 --- a/app/views/legislation/proposals/_refresh_flag_actions.js.erb +++ /dev/null @@ -1 +0,0 @@ -App.Flaggable.update("<%= dom_id(@proposal) %>", "<%= j render("shared/flag_actions", flaggable: @proposal) %>"); diff --git a/app/views/proposals/_refresh_flag_actions.js.erb b/app/views/proposals/_refresh_flag_actions.js.erb deleted file mode 100644 index e60ee63b7..000000000 --- a/app/views/proposals/_refresh_flag_actions.js.erb +++ /dev/null @@ -1 +0,0 @@ -App.Flaggable.update("<%= dom_id(@proposal) %>", "<%= j render("shared/flag_actions", flaggable: @proposal) %>") diff --git a/app/views/shared/_refresh_flag_actions.js.erb b/app/views/shared/_refresh_flag_actions.js.erb new file mode 100644 index 000000000..94aa42086 --- /dev/null +++ b/app/views/shared/_refresh_flag_actions.js.erb @@ -0,0 +1,3 @@ +App.Flaggable.update("<%= dom_id(flaggable) %>", + "<%= j render("shared/flag_actions", flaggable: flaggable, divider: local_assigns[:divider]) %>" +);