From cac24b0159b7c6eca0474045ab3454d2bfb5c29c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 2 Dec 2021 02:18:46 +0100 Subject: [PATCH] Extract component to show moderation actions Note that in proposal notifications we're writing the call to render the component in the same line as the
definition in order to be able to use the `:empty` selector when the component renders nothing. No browser matches whitespace with the `:empty` selector, so we can't add newline characters inside the tag. A more elegant solution would be extracting the proposal notification actions to a component and only rendering it if the moderation actions component is rendered. --- app/assets/stylesheets/layout.scss | 4 ++ .../moderation_actions_component.html.erb | 12 ++++ .../shared/moderation_actions_component.rb | 34 +++++++++++ app/models/comment.rb | 4 ++ .../budgets/investments/_actions.html.erb | 11 +--- app/views/comments/_actions.html.erb | 29 ++++------ app/views/debates/_actions.html.erb | 11 +--- .../legislation/proposals/_actions.html.erb | 11 +--- .../proposal_notifications/_actions.html.erb | 17 +----- app/views/proposals/_actions.html.erb | 11 +--- .../moderation_actions_component_spec.rb | 56 +++++++++++++++++++ .../budget_investments_valuation_spec.rb | 2 +- 12 files changed, 126 insertions(+), 76 deletions(-) create mode 100644 app/components/shared/moderation_actions_component.html.erb create mode 100644 app/components/shared/moderation_actions_component.rb create mode 100644 spec/components/shared/moderation_actions_component_spec.rb diff --git a/app/assets/stylesheets/layout.scss b/app/assets/stylesheets/layout.scss index 45a7c45df..4c38b27d7 100644 --- a/app/assets/stylesheets/layout.scss +++ b/app/assets/stylesheets/layout.scss @@ -1877,6 +1877,10 @@ table { padding: $line-height / 4; position: relative; + &:empty { + display: none; + } + .divider { color: $text-light; display: inline-block; diff --git a/app/components/shared/moderation_actions_component.html.erb b/app/components/shared/moderation_actions_component.html.erb new file mode 100644 index 000000000..c0d05d560 --- /dev/null +++ b/app/components/shared/moderation_actions_component.html.erb @@ -0,0 +1,12 @@ + + <% if can? :hide, record %> + <%= link_to t("admin.actions.hide").capitalize, hide_path, + method: :put, remote: true, data: { confirm: confirm_hide_text } %> + <% end %> + + <% if can? :hide, record.author %> + <%= raw separator %> + <%= link_to t("admin.actions.hide_author").capitalize, hide_moderation_user_path(record.author_id), + method: :put, data: { confirm: confirm_hide_author_text } %> + <% end %> + diff --git a/app/components/shared/moderation_actions_component.rb b/app/components/shared/moderation_actions_component.rb new file mode 100644 index 000000000..43e4fd217 --- /dev/null +++ b/app/components/shared/moderation_actions_component.rb @@ -0,0 +1,34 @@ +class Shared::ModerationActionsComponent < ApplicationComponent + attr_reader :record + delegate :can?, to: :helpers + + def initialize(record) + @record = record + end + + def render? + can?(:hide, record) || can?(:hide, record.author) + end + + private + + def hide_path + polymorphic_path([:moderation, record], action: :hide) + end + + def confirm_hide_text + t("admin.actions.confirm_action", action: t("admin.actions.hide"), name: record.human_name) + end + + def confirm_hide_author_text + t("admin.actions.confirm_action", action: t("admin.actions.hide_author"), name: record.author.name) + end + + def separator + if record.is_a?(Comment) + " • " + else + " | " + end + end +end diff --git a/app/models/comment.rb b/app/models/comment.rb index 89e6c88f1..443d2322d 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -83,6 +83,10 @@ class Comment < ApplicationRecord self.user = author end + def human_name + body.truncate(32) + end + def total_votes cached_votes_total end diff --git a/app/views/budgets/investments/_actions.html.erb b/app/views/budgets/investments/_actions.html.erb index 9711a8268..8fd2110fa 100644 --- a/app/views/budgets/investments/_actions.html.erb +++ b/app/views/budgets/investments/_actions.html.erb @@ -1,10 +1 @@ -<% if can? :hide, investment %> - <%= link_to t("admin.actions.hide").capitalize, hide_moderation_budget_investment_path(investment), - method: :put, remote: true, data: { confirm: t("admin.actions.confirm_action", action: t("admin.actions.hide"), name: investment.title) } %> -<% end %> - -<% if can? :hide, investment.author %> -  |  - <%= link_to t("admin.actions.hide_author").capitalize, hide_moderation_user_path(investment.author_id), - method: :put, data: { confirm: t("admin.actions.confirm_action", action: t("admin.actions.hide_author"), name: investment.author.name) } %> -<% end %> +<%= render Shared::ModerationActionsComponent.new(investment) %> diff --git a/app/views/comments/_actions.html.erb b/app/views/comments/_actions.html.erb index 13a97c96a..4710f2931 100644 --- a/app/views/comments/_actions.html.erb +++ b/app/views/comments/_actions.html.erb @@ -2,23 +2,14 @@ <%= render "shared/flag_actions", flaggable: comment, divider: true %> - - <% if can? :hide, comment %> -  •  - <% if comment.author == current_user %> - <%= link_to t("comments.actions.delete"), - hide_comment_path(comment), - method: :put, remote: true, class: "delete-comment", - data: { confirm: t("comments.actions.confirm_delete") } %> - <% else %> - <%= link_to t("admin.actions.hide").capitalize, hide_moderation_comment_path(comment), - method: :put, remote: true, data: { confirm: t("admin.actions.confirm_action", action: t("admin.actions.hide"), name: comment.body.truncate(32)) } %> - <% end %> +<% if can?(:hide, comment) || can?(:hide, comment.user) %> +  •  + <% if comment.author == current_user %> + <%= link_to t("comments.actions.delete"), + hide_comment_path(comment), + method: :put, remote: true, class: "delete-comment", + data: { confirm: t("comments.actions.confirm_delete") } %> + <% else %> + <%= render Shared::ModerationActionsComponent.new(comment) %> <% end %> - - <% if can? :hide, comment.user %> -  •  - <%= link_to t("admin.actions.hide_author").capitalize, hide_moderation_user_path(comment.user_id), - method: :put, data: { confirm: t("admin.actions.confirm_action", action: t("admin.actions.hide_author"), name: comment.author.name) } %> - <% end %> - +<% end %> diff --git a/app/views/debates/_actions.html.erb b/app/views/debates/_actions.html.erb index 1959ef29a..3605d3fcf 100644 --- a/app/views/debates/_actions.html.erb +++ b/app/views/debates/_actions.html.erb @@ -1,13 +1,4 @@ -<% if can? :hide, debate %> - <%= link_to t("admin.actions.hide").capitalize, hide_moderation_debate_path(debate), - method: :put, remote: true, data: { confirm: t("admin.actions.confirm_action", action: t("admin.actions.hide"), name: debate.title) } %> -<% end %> - -<% if can? :hide, debate.author %> -  |  - <%= link_to t("admin.actions.hide_author").capitalize, hide_moderation_user_path(debate.author_id), - method: :put, data: { confirm: t("admin.actions.confirm_action", action: t("admin.actions.hide_author"), name: debate.author.name) } %> -<% end %> +<%= render Shared::ModerationActionsComponent.new(debate) %> <% if can? :mark_featured, debate %>  |  diff --git a/app/views/legislation/proposals/_actions.html.erb b/app/views/legislation/proposals/_actions.html.erb index a887f422c..a0f59692d 100644 --- a/app/views/legislation/proposals/_actions.html.erb +++ b/app/views/legislation/proposals/_actions.html.erb @@ -1,10 +1 @@ -<% if can? :hide, proposal %> - <%= link_to t("admin.actions.hide").capitalize, hide_moderation_legislation_proposal_path(proposal), - method: :put, remote: true, data: { confirm: t("admin.actions.confirm_action", action: t("admin.actions.hide"), name: proposal.title) } %> -<% end %> - -<% if can? :hide, proposal.author %> -  |  - <%= link_to t("admin.actions.hide_author").capitalize, hide_moderation_user_path(proposal.author_id), - method: :put, data: { confirm: t("admin.actions.confirm_action", action: t("admin.actions.hide_author"), name: proposal.author.name) } %> -<% end %> +<%= render Shared::ModerationActionsComponent.new(proposal) %> diff --git a/app/views/proposal_notifications/_actions.html.erb b/app/views/proposal_notifications/_actions.html.erb index 5c3f96ff1..e3c4b356c 100644 --- a/app/views/proposal_notifications/_actions.html.erb +++ b/app/views/proposal_notifications/_actions.html.erb @@ -1,16 +1 @@ -<% if can? :hide, (notification || notification.author) %> -
- - <% if can? :hide, notification %> - <%= link_to t("admin.actions.hide").capitalize, hide_moderation_proposal_notification_path(notification), - method: :put, remote: true, data: { confirm: t("admin.actions.confirm_action", action: t("admin.actions.hide"), name: notification.title) } %> - <% end %> - - <% if can? :hide, notification.author %> -  •  - <%= link_to t("admin.actions.hide_author").capitalize, hide_moderation_user_path(notification.author_id), - method: :put, data: { confirm: t("admin.actions.confirm_action", action: t("admin.actions.hide_author"), name: notification.author.name) } %> - <% end %> - -
-<% end %> +
<%= render Shared::ModerationActionsComponent.new(notification) %>
diff --git a/app/views/proposals/_actions.html.erb b/app/views/proposals/_actions.html.erb index a568c04df..a0f59692d 100644 --- a/app/views/proposals/_actions.html.erb +++ b/app/views/proposals/_actions.html.erb @@ -1,10 +1 @@ -<% if can? :hide, proposal %> - <%= link_to t("admin.actions.hide").capitalize, hide_moderation_proposal_path(proposal), - method: :put, remote: true, data: { confirm: t("admin.actions.confirm_action", action: t("admin.actions.hide"), name: proposal.title) } %> -<% end %> - -<% if can? :hide, proposal.author %> -  |  - <%= link_to t("admin.actions.hide_author").capitalize, hide_moderation_user_path(proposal.author_id), - method: :put, data: { confirm: t("admin.actions.confirm_action", action: t("admin.actions.hide_author"), name: proposal.author.name) } %> -<% end %> +<%= render Shared::ModerationActionsComponent.new(proposal) %> diff --git a/spec/components/shared/moderation_actions_component_spec.rb b/spec/components/shared/moderation_actions_component_spec.rb new file mode 100644 index 000000000..be6237c7d --- /dev/null +++ b/spec/components/shared/moderation_actions_component_spec.rb @@ -0,0 +1,56 @@ +require "rails_helper" + +describe Shared::ModerationActionsComponent do + include Rails.application.routes.url_helpers + before { sign_in(create(:administrator).user) } + + describe "Hide link" do + it "is shown for debates" do + debate = create(:debate) + + render_inline Shared::ModerationActionsComponent.new(debate) + + expect(page).to have_link "Hide", href: hide_moderation_debate_path(debate) + end + + it "is shown for proposals" do + proposal = create(:proposal) + + render_inline Shared::ModerationActionsComponent.new(proposal) + + expect(page).to have_link "Hide", href: hide_moderation_proposal_path(proposal) + end + + it "is shown for proposal notifications" do + notification = create(:proposal_notification) + + render_inline Shared::ModerationActionsComponent.new(notification) + + expect(page).to have_link "Hide", href: hide_moderation_proposal_notification_path(notification) + end + + it "is shown for comments" do + comment = create(:comment) + + render_inline Shared::ModerationActionsComponent.new(comment) + + expect(page).to have_link "Hide", href: hide_moderation_comment_path(comment) + end + + it "is shown for budget investments" do + investment = create(:budget_investment) + + render_inline Shared::ModerationActionsComponent.new(investment) + + expect(page).to have_link "Hide", href: hide_moderation_budget_investment_path(investment) + end + + it "is shown for legislation proposals" do + proposal = create(:legislation_proposal) + + render_inline Shared::ModerationActionsComponent.new(proposal) + + expect(page).to have_link "Hide", href: hide_moderation_legislation_proposal_path(proposal) + end + end +end diff --git a/spec/system/comments/budget_investments_valuation_spec.rb b/spec/system/comments/budget_investments_valuation_spec.rb index eccb8ce26..47d9823cc 100644 --- a/spec/system/comments/budget_investments_valuation_spec.rb +++ b/spec/system/comments/budget_investments_valuation_spec.rb @@ -275,7 +275,7 @@ describe "Internal valuation comments on Budget::Investments" do expect(page).to have_no_css(".comment-votes") expect(page).to have_no_css(".js-flag-actions") - expect(page).to have_no_css(".js-moderation-actions") + expect(page).to have_no_css(".moderation-actions") end end