From 9a8a8ce5ce7e8519653868bc52f4eaaaf9a65a90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 1 Dec 2021 19:29:28 +0100 Subject: [PATCH 01/16] Fix missing closing tag for blocked users It was working because browsers automatically assume one element ends when finding a tag without a . --- app/views/moderation/users/index.html.erb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/moderation/users/index.html.erb b/app/views/moderation/users/index.html.erb index 9008f8fec..c8b13b6e4 100644 --- a/app/views/moderation/users/index.html.erb +++ b/app/views/moderation/users/index.html.erb @@ -17,10 +17,10 @@ <% if user.hidden? %> <%= t("moderation.users.index.hidden") %> <% else %> - <%= link_to t("moderation.users.index.hide"), hide_in_moderation_screen_moderation_user_path(user, request.query_parameters), - method: :put, class: "button hollow alert" %> + <%= link_to t("moderation.users.index.hide"), hide_in_moderation_screen_moderation_user_path(user, request.query_parameters), + method: :put, class: "button hollow alert" %> + <% end %> - <% end %> <% end %> From 600a2bd4c2c87d211050336b16ab7ad8e5d705f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 1 Dec 2021 19:35:16 +0100 Subject: [PATCH 02/16] Use a button instead of a link to block users We're continuing to replace links with buttons, for the reasons explained in commit 5311daadf. We're also adding an ARIA label since on the same page there might be several links to block different users. --- app/views/moderation/users/index.html.erb | 10 ++++++++-- spec/system/admin/activity_spec.rb | 2 +- spec/system/moderation/budget_investments_spec.rb | 2 +- spec/system/moderation/comments_spec.rb | 2 +- spec/system/moderation/debates_spec.rb | 2 +- spec/system/moderation/proposal_notifications_spec.rb | 2 +- spec/system/moderation/proposals_spec.rb | 2 +- spec/system/moderation/users_spec.rb | 2 +- 8 files changed, 15 insertions(+), 9 deletions(-) diff --git a/app/views/moderation/users/index.html.erb b/app/views/moderation/users/index.html.erb index c8b13b6e4..87b5b2dd5 100644 --- a/app/views/moderation/users/index.html.erb +++ b/app/views/moderation/users/index.html.erb @@ -17,8 +17,14 @@ <% if user.hidden? %> <%= t("moderation.users.index.hidden") %> <% else %> - <%= link_to t("moderation.users.index.hide"), hide_in_moderation_screen_moderation_user_path(user, request.query_parameters), - method: :put, class: "button hollow alert" %> + <%= render Admin::ActionComponent.new( + :hide_in_moderation_screen, + user, + text: t("moderation.users.index.hide"), + method: :put, + "aria-label": true, + class: "button hollow alert" + ) %> <% end %> diff --git a/spec/system/admin/activity_spec.rb b/spec/system/admin/activity_spec.rb index f973b041b..a8e1bca8f 100644 --- a/spec/system/admin/activity_spec.rb +++ b/spec/system/admin/activity_spec.rb @@ -239,7 +239,7 @@ describe "Admin activity" do visit moderation_users_path(search: user.username) within("#moderation_users") do - click_link "Block" + click_button "Block" end visit admin_activity_path diff --git a/spec/system/moderation/budget_investments_spec.rb b/spec/system/moderation/budget_investments_spec.rb index aa0014182..6960fc1fc 100644 --- a/spec/system/moderation/budget_investments_spec.rb +++ b/spec/system/moderation/budget_investments_spec.rb @@ -75,7 +75,7 @@ describe "Moderate budget investments" do click_button "Search" within "tr", text: investment.author.name do - expect(page).to have_link "Block" + expect(page).to have_button "Block" end end diff --git a/spec/system/moderation/comments_spec.rb b/spec/system/moderation/comments_spec.rb index 27ca96c10..8321f2c2f 100644 --- a/spec/system/moderation/comments_spec.rb +++ b/spec/system/moderation/comments_spec.rb @@ -95,7 +95,7 @@ describe "Moderate comments" do click_button "Search" within "tr", text: comment.user.name do - expect(page).to have_link "Block" + expect(page).to have_button "Block" end end diff --git a/spec/system/moderation/debates_spec.rb b/spec/system/moderation/debates_spec.rb index 9a743807a..085682d53 100644 --- a/spec/system/moderation/debates_spec.rb +++ b/spec/system/moderation/debates_spec.rb @@ -66,7 +66,7 @@ describe "Moderate debates" do click_button "Search" within "tr", text: debate.author.name do - expect(page).to have_link "Block" + expect(page).to have_button "Block" end end diff --git a/spec/system/moderation/proposal_notifications_spec.rb b/spec/system/moderation/proposal_notifications_spec.rb index b81dba51c..6515c6566 100644 --- a/spec/system/moderation/proposal_notifications_spec.rb +++ b/spec/system/moderation/proposal_notifications_spec.rb @@ -70,7 +70,7 @@ describe "Moderate proposal notifications" do click_button "Search" within "tr", text: proposal_notification.author.name do - expect(page).to have_link "Block" + expect(page).to have_button "Block" end end diff --git a/spec/system/moderation/proposals_spec.rb b/spec/system/moderation/proposals_spec.rb index d2043d923..9ab06e314 100644 --- a/spec/system/moderation/proposals_spec.rb +++ b/spec/system/moderation/proposals_spec.rb @@ -65,7 +65,7 @@ describe "Moderate proposals" do click_button "Search" within "tr", text: proposal.author.name do - expect(page).to have_link "Block" + expect(page).to have_button "Block" end end diff --git a/spec/system/moderation/users_spec.rb b/spec/system/moderation/users_spec.rb index 9bf390005..3e54731bf 100644 --- a/spec/system/moderation/users_spec.rb +++ b/spec/system/moderation/users_spec.rb @@ -64,7 +64,7 @@ describe "Moderate users" do within("#moderation_users") do expect(page).to have_content citizen.name expect(page).not_to have_content "Blocked" - click_link "Block" + click_button "Block" end within("#moderation_users") do From 84c6eeae9c693e8a95e36d78f695a07f007cf98f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 1 Dec 2021 20:21:56 +0100 Subject: [PATCH 03/16] Use headers and actions in users moderation table Having proper headers makes it more accessible. We're also using the table actions component because we're going to add another action. Since table actions use a flex layout, we have to tweak the styles a little bit. For that, I'm adding a
element which will make it possible to style just this table while also providing an extra shortcut for people using screen readers. --- app/assets/stylesheets/application.scss | 1 + .../stylesheets/moderation/users/index.scss | 10 +++ app/views/moderation/users/index.html.erb | 68 ++++++++++--------- 3 files changed, 48 insertions(+), 31 deletions(-) create mode 100644 app/assets/stylesheets/moderation/users/index.scss diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index 91837158a..74675f1c3 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -40,6 +40,7 @@ @import "debates/**/*"; @import "layout/**/*"; @import "machine_learning/**/*"; +@import "moderation/**/*"; @import "proposals/**/*"; @import "relationable/**/*"; @import "sdg/**/*"; diff --git a/app/assets/stylesheets/moderation/users/index.scss b/app/assets/stylesheets/moderation/users/index.scss new file mode 100644 index 000000000..aab07125c --- /dev/null +++ b/app/assets/stylesheets/moderation/users/index.scss @@ -0,0 +1,10 @@ +.moderation-users-index { + th:last-child, + td:last-child { + text-align: $global-right; + } + + .table-actions { + justify-content: flex-end; + } +} diff --git a/app/views/moderation/users/index.html.erb b/app/views/moderation/users/index.html.erb index 87b5b2dd5..e1492c7bb 100644 --- a/app/views/moderation/users/index.html.erb +++ b/app/views/moderation/users/index.html.erb @@ -1,35 +1,41 @@ -

<%= t("moderation.users.index.title") %>

+
+

<%= t("moderation.users.index.title") %>

-<%= render Admin::SearchComponent.new(label: t("moderation.users.index.search_placeholder")) %> + <%= render Admin::SearchComponent.new(label: t("moderation.users.index.search_placeholder")) %> -<% if @users.present? %> -

<%= page_entries_info @users %>

-<% end %> + <% if @users.present? %> +

<%= page_entries_info @users %>

- - - <% @users.each do |user| %> - - - - - <% end %> - -
- <%= user.name %> - - <% if user.hidden? %> - <%= t("moderation.users.index.hidden") %> - <% else %> - <%= render Admin::ActionComponent.new( - :hide_in_moderation_screen, - user, - text: t("moderation.users.index.hide"), - method: :put, - "aria-label": true, - class: "button hollow alert" - ) %> - <% end %> -
+ + + + + + + <% @users.each do |user| %> + + + + + <% end %> + +
<%= t("admin.hidden_users.index.user") %><%= t("admin.actions.actions") %>
+ <%= user.name %> + + <% if user.hidden? %> + <%= t("moderation.users.index.hidden") %> + <% else %> + <%= render Admin::TableActionsComponent.new(user, actions: []) do |actions| %> + <%= actions.action( + :hide_in_moderation_screen, + text: t("moderation.users.index.hide"), + method: :put, + class: "button hollow alert" + ) %> + <% end %> + <% end %> +
-<%= paginate @users %> + <%= paginate @users %> + <% end %> +
From 4c8dfb6695c10c7523ac35064eb819b2baa1b796 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 1 Dec 2021 21:01:58 +0100 Subject: [PATCH 04/16] Use just one action to hide users Other than removing a redundant action, we're fixing two bugs when blocking an author using the links in the public views: * We were always redirecting to the debates index, even if we blocked the author of a proposal or an investment * We weren't showing any kind of success message --- .../moderation/users_controller.rb | 21 ++++++++++----- app/models/ability.rb | 4 --- app/views/moderation/users/index.html.erb | 2 +- config/routes/moderation.rb | 1 - .../moderation/users_controller_spec.rb | 26 ++++++++++++++++--- spec/models/abilities/moderator_spec.rb | 3 --- spec/system/admin/activity_spec.rb | 2 +- spec/system/admin/hidden_comments_spec.rb | 2 +- .../moderation/budget_investments_spec.rb | 5 +--- 9 files changed, 40 insertions(+), 26 deletions(-) diff --git a/app/controllers/moderation/users_controller.rb b/app/controllers/moderation/users_controller.rb index 2c958eda8..b53dbd722 100644 --- a/app/controllers/moderation/users_controller.rb +++ b/app/controllers/moderation/users_controller.rb @@ -6,16 +6,10 @@ class Moderation::UsersController < Moderation::BaseController def index end - def hide_in_moderation_screen - block_user - - redirect_with_query_params_to({ action: :index }, { notice: I18n.t("moderation.users.notice_hide") }) - end - def hide block_user - redirect_to debates_path + redirect_with_query_params_to index_path_options, { notice: I18n.t("moderation.users.notice_hide") } end private @@ -28,4 +22,17 @@ class Moderation::UsersController < Moderation::BaseController @user.block Activity.log(current_user, :block, @user) end + + def index_path_options + if request.referer + referer_params = Rails.application.routes.recognize_path(request.referer) + + referer_params.except(:id).merge({ + controller: "/#{referer_params[:controller]}", + action: :index + }) + else + { action: :index } + end + end end diff --git a/app/models/ability.rb b/app/models/ability.rb index afe6f416e..45207eaae 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -2,10 +2,6 @@ class Ability include CanCan::Ability def initialize(user) - # If someone can hide something, he can also hide it - # from the moderation screen - alias_action :hide_in_moderation_screen, to: :hide - if user # logged-in users merge Abilities::Valuator.new(user) if user.valuator? diff --git a/app/views/moderation/users/index.html.erb b/app/views/moderation/users/index.html.erb index e1492c7bb..9068d0505 100644 --- a/app/views/moderation/users/index.html.erb +++ b/app/views/moderation/users/index.html.erb @@ -23,7 +23,7 @@ <% else %> <%= render Admin::TableActionsComponent.new(user, actions: []) do |actions| %> <%= actions.action( - :hide_in_moderation_screen, + :hide, text: t("moderation.users.index.hide"), method: :put, class: "button hollow alert" diff --git a/config/routes/moderation.rb b/config/routes/moderation.rb index 83984badd..e28f28336 100644 --- a/config/routes/moderation.rb +++ b/config/routes/moderation.rb @@ -4,7 +4,6 @@ namespace :moderation do resources :users, only: :index do member do put :hide - put :hide_in_moderation_screen end end diff --git a/spec/controllers/moderation/users_controller_spec.rb b/spec/controllers/moderation/users_controller_spec.rb index 1699c9c4e..f4818f0eb 100644 --- a/spec/controllers/moderation/users_controller_spec.rb +++ b/spec/controllers/moderation/users_controller_spec.rb @@ -2,14 +2,32 @@ require "rails_helper" describe Moderation::UsersController do before { sign_in create(:moderator).user } + let(:user) { create(:user, email: "user@consul.dev") } - describe "PUT hide_in_moderation_screen" do + describe "PUT hide" do it "keeps query parameters while using protected redirects" do - user = create(:user, email: "user@consul.dev") - - get :hide_in_moderation_screen, params: { id: user, search: "user@consul.dev", host: "evil.dev" } + get :hide, params: { id: user, search: "user@consul.dev", host: "evil.dev" } expect(response).to redirect_to "/moderation/users?search=user%40consul.dev" end + + it "redirects to the index of the section where it was called with a notice" do + proposal = create(:proposal, author: user) + request.env["HTTP_REFERER"] = proposal_path(proposal) + + put :hide, params: { id: user } + + expect(response).to redirect_to proposals_path + expect(flash[:notice]).to eq "User blocked. All of this user's debates and comments have been hidden." + end + + it "redirects to the index with a nested resource" do + investment = create(:budget_investment, author: user) + request.env["HTTP_REFERER"] = budget_investment_path(investment.budget, investment) + + put :hide, params: { id: user } + + expect(response).to redirect_to budget_investments_path(investment.budget) + end end end diff --git a/spec/models/abilities/moderator_spec.rb b/spec/models/abilities/moderator_spec.rb index 77bf1ae36..9f3cb14dc 100644 --- a/spec/models/abilities/moderator_spec.rb +++ b/spec/models/abilities/moderator_spec.rb @@ -52,7 +52,6 @@ describe Abilities::Moderator do let(:ignored_proposal) { create(:proposal, :with_ignored_flag) } it { should be_able_to(:hide, comment) } - it { should be_able_to(:hide_in_moderation_screen, comment) } it { should_not be_able_to(:hide, hidden_comment) } it { should be_able_to(:hide, own_comment) } @@ -60,12 +59,10 @@ describe Abilities::Moderator do it { should_not be_able_to(:moderate, own_comment) } it { should be_able_to(:hide, debate) } - it { should be_able_to(:hide_in_moderation_screen, debate) } it { should_not be_able_to(:hide, hidden_debate) } it { should_not be_able_to(:hide, own_debate) } it { should be_able_to(:hide, proposal) } - it { should be_able_to(:hide_in_moderation_screen, proposal) } it { should be_able_to(:hide, own_proposal) } it { should_not be_able_to(:hide, hidden_proposal) } diff --git a/spec/system/admin/activity_spec.rb b/spec/system/admin/activity_spec.rb index a8e1bca8f..09236ac67 100644 --- a/spec/system/admin/activity_spec.rb +++ b/spec/system/admin/activity_spec.rb @@ -219,7 +219,7 @@ describe "Admin activity" do within("#proposal_#{proposal.id}") do accept_confirm("Are you sure? Hide author \"#{proposal.author.name}\"") { click_link "Hide author" } - expect(page).to have_current_path(debates_path) + expect(page).to have_current_path(proposals_path) end visit admin_activity_path diff --git a/spec/system/admin/hidden_comments_spec.rb b/spec/system/admin/hidden_comments_spec.rb index 2ac07cfa9..407207fa9 100644 --- a/spec/system/admin/hidden_comments_spec.rb +++ b/spec/system/admin/hidden_comments_spec.rb @@ -16,7 +16,7 @@ describe "Admin hidden comments", :admin do accept_confirm("Are you sure? Hide author \"#{proposal.author.name}\"") { click_link "Hide author" } end - expect(page).to have_current_path debates_path + expect(page).to have_current_path proposals_path visit admin_hidden_comments_path diff --git a/spec/system/moderation/budget_investments_spec.rb b/spec/system/moderation/budget_investments_spec.rb index 6960fc1fc..9cfaf7849 100644 --- a/spec/system/moderation/budget_investments_spec.rb +++ b/spec/system/moderation/budget_investments_spec.rb @@ -25,10 +25,7 @@ describe "Moderate budget investments" do accept_confirm("Are you sure? Hide author \"#{investment.author.name}\"") { click_link "Hide author" } - expect(page).to have_current_path(debates_path) - - visit budget_investments_path(budget.id, heading_id: heading.id) - + expect(page).to have_current_path(budget_investments_path(budget)) expect(page).not_to have_content(investment.title) end From 49edd6a9b1100c0045c2312f13b28411d4c07f4c Mon Sep 17 00:00:00 2001 From: Carlos Iniesta Date: Sun, 25 Oct 2020 22:26:29 +0100 Subject: [PATCH 05/16] Add soft block button in moderation user view --- app/assets/stylesheets/moderation/users/index.scss | 8 ++++++++ app/controllers/moderation/users_controller.rb | 11 +++++++++++ app/models/abilities/moderation.rb | 3 +++ app/models/activity.rb | 2 +- app/views/moderation/users/index.html.erb | 8 ++------ config/locales/en/moderation.yml | 2 ++ config/locales/es/moderation.yml | 2 ++ config/routes/moderation.rb | 1 + spec/controllers/moderation/users_controller_spec.rb | 10 ++++++++++ spec/models/abilities/moderator_spec.rb | 3 +++ 10 files changed, 43 insertions(+), 7 deletions(-) diff --git a/app/assets/stylesheets/moderation/users/index.scss b/app/assets/stylesheets/moderation/users/index.scss index aab07125c..47471d2d5 100644 --- a/app/assets/stylesheets/moderation/users/index.scss +++ b/app/assets/stylesheets/moderation/users/index.scss @@ -7,4 +7,12 @@ .table-actions { justify-content: flex-end; } + + .soft-block-link { + @include hollow-button($color-warning); + } + + .hide-link { + @include hollow-button($alert-color); + } } diff --git a/app/controllers/moderation/users_controller.rb b/app/controllers/moderation/users_controller.rb index b53dbd722..3466e695c 100644 --- a/app/controllers/moderation/users_controller.rb +++ b/app/controllers/moderation/users_controller.rb @@ -6,6 +6,12 @@ class Moderation::UsersController < Moderation::BaseController def index end + def soft_block + soft_block_user + + redirect_with_query_params_to({ action: :index }, { notice: I18n.t("moderation.users.notice_soft_hide") }) + end + def hide block_user @@ -18,6 +24,11 @@ class Moderation::UsersController < Moderation::BaseController @users = User.with_hidden.search(params[:search]).page(params[:page]).for_render end + def soft_block_user + @user.hide + Activity.log(current_user, :soft_block, @user) + end + def block_user @user.block Activity.log(current_user, :block, @user) diff --git a/app/models/abilities/moderation.rb b/app/models/abilities/moderation.rb index bda41b203..9c6cd3857 100644 --- a/app/models/abilities/moderation.rb +++ b/app/models/abilities/moderation.rb @@ -47,6 +47,9 @@ module Abilities can :hide, User cannot :hide, User, id: user.id + can :soft_block, User + cannot :soft_block, User, id: user.id + can :block, User cannot :block, User, id: user.id diff --git a/app/models/activity.rb b/app/models/activity.rb index 3accc495b..864ef0e5b 100644 --- a/app/models/activity.rb +++ b/app/models/activity.rb @@ -2,7 +2,7 @@ class Activity < ApplicationRecord belongs_to :actionable, -> { with_hidden }, polymorphic: true belongs_to :user, -> { with_hidden }, inverse_of: :activities - VALID_ACTIONS = %w[hide block restore valuate email].freeze + VALID_ACTIONS = %w[hide soft_block block restore valuate email].freeze validates :action, inclusion: { in: VALID_ACTIONS } diff --git a/app/views/moderation/users/index.html.erb b/app/views/moderation/users/index.html.erb index 9068d0505..56d15acd0 100644 --- a/app/views/moderation/users/index.html.erb +++ b/app/views/moderation/users/index.html.erb @@ -22,12 +22,8 @@ <%= t("moderation.users.index.hidden") %> <% else %> <%= render Admin::TableActionsComponent.new(user, actions: []) do |actions| %> - <%= actions.action( - :hide, - text: t("moderation.users.index.hide"), - method: :put, - class: "button hollow alert" - ) %> + <%= actions.action(:soft_block, text: t("moderation.users.index.soft_hide"), method: :put) %> + <%= actions.action(:hide, text: t("moderation.users.index.hide"), method: :put) %> <% end %> <% end %> diff --git a/config/locales/en/moderation.yml b/config/locales/en/moderation.yml index 7f3b8d83c..63c834fcd 100644 --- a/config/locales/en/moderation.yml +++ b/config/locales/en/moderation.yml @@ -104,5 +104,7 @@ en: hidden: Blocked hide: Block search_placeholder: email or name of user + soft_hide: Soft Block title: Block users notice_hide: User blocked. All of this user's debates and comments have been hidden. + notice_soft_hide: User blocked. All ot this user's debates and comments are still available. diff --git a/config/locales/es/moderation.yml b/config/locales/es/moderation.yml index 1cc2f47d0..2fff7b996 100644 --- a/config/locales/es/moderation.yml +++ b/config/locales/es/moderation.yml @@ -104,5 +104,7 @@ es: hidden: Bloqueado hide: Bloquear search_placeholder: email o nombre de usuario + soft_hide: Ocultar title: Bloquear usuarios notice_hide: Usuario bloqueado. Se han ocultado todos sus debates y comentarios. + notice_soft_hide: Usuario bloqueado. Todos sus debates y comentarios siguen disponibles. diff --git a/config/routes/moderation.rb b/config/routes/moderation.rb index e28f28336..009d9484a 100644 --- a/config/routes/moderation.rb +++ b/config/routes/moderation.rb @@ -4,6 +4,7 @@ namespace :moderation do resources :users, only: :index do member do put :hide + put :soft_block end end diff --git a/spec/controllers/moderation/users_controller_spec.rb b/spec/controllers/moderation/users_controller_spec.rb index f4818f0eb..59d2e7f41 100644 --- a/spec/controllers/moderation/users_controller_spec.rb +++ b/spec/controllers/moderation/users_controller_spec.rb @@ -30,4 +30,14 @@ describe Moderation::UsersController do expect(response).to redirect_to budget_investments_path(investment.budget) end end + + describe "PUT soft_block" do + it "keeps query parameters while using protected redirects" do + user = create(:user, email: "user@consul.dev") + + get :soft_block, params: { id: user, name_or_email: "user@consul.dev", host: "evil.dev" } + + expect(response).to redirect_to "/moderation/users?name_or_email=user%40consul.dev" + end + end end diff --git a/spec/models/abilities/moderator_spec.rb b/spec/models/abilities/moderator_spec.rb index 9f3cb14dc..508d268f0 100644 --- a/spec/models/abilities/moderator_spec.rb +++ b/spec/models/abilities/moderator_spec.rb @@ -90,6 +90,9 @@ describe Abilities::Moderator do it { should_not be_able_to(:hide, user) } it { should be_able_to(:hide, other_user) } + it { should_not be_able_to(:soft_block, user) } + it { should be_able_to(:soft_block, other_user) } + it { should_not be_able_to(:block, user) } it { should be_able_to(:block, other_user) } 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 06/16] 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 From 021fef07b643a2a2a87d31ab1214b7215ecbdda2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 2 Dec 2021 03:02:37 +0100 Subject: [PATCH 07/16] Make action names to block and hide more clear The `hide` action was calling the `block` method while the `soft_block` action was calling the `hide` method. Combined with the fact that we also have a `block` permission which is used in `ModerateActions` the logic was hard to follow. --- .../stylesheets/moderation/users/index.scss | 4 +-- .../moderation_actions_component.html.erb | 4 +-- .../shared/moderation_actions_component.rb | 4 +-- .../moderation/users_controller.rb | 14 +++++----- app/models/abilities/moderation.rb | 3 -- app/models/activity.rb | 2 +- app/views/moderation/users/index.html.erb | 2 +- config/locales/en/admin.yml | 2 +- config/locales/en/moderation.yml | 8 +++--- config/locales/es/admin.yml | 2 +- config/locales/es/moderation.yml | 8 +++--- config/routes/moderation.rb | 2 +- .../moderation/users_controller_spec.rb | 28 ++++++++++--------- spec/models/abilities/moderator_spec.rb | 3 -- spec/system/admin/activity_spec.rb | 2 +- spec/system/admin/hidden_comments_spec.rb | 2 +- .../moderation/budget_investments_spec.rb | 4 +-- spec/system/moderation/users_spec.rb | 2 +- 18 files changed, 46 insertions(+), 50 deletions(-) diff --git a/app/assets/stylesheets/moderation/users/index.scss b/app/assets/stylesheets/moderation/users/index.scss index 47471d2d5..849153b24 100644 --- a/app/assets/stylesheets/moderation/users/index.scss +++ b/app/assets/stylesheets/moderation/users/index.scss @@ -8,11 +8,11 @@ justify-content: flex-end; } - .soft-block-link { + .hide-link { @include hollow-button($color-warning); } - .hide-link { + .block-link { @include hollow-button($alert-color); } } diff --git a/app/components/shared/moderation_actions_component.html.erb b/app/components/shared/moderation_actions_component.html.erb index c0d05d560..5d37fed88 100644 --- a/app/components/shared/moderation_actions_component.html.erb +++ b/app/components/shared/moderation_actions_component.html.erb @@ -6,7 +6,7 @@ <% 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 } %> + <%= link_to t("admin.actions.block_author").capitalize, block_moderation_user_path(record.author_id), + method: :put, data: { confirm: confirm_block_author_text } %> <% end %> diff --git a/app/components/shared/moderation_actions_component.rb b/app/components/shared/moderation_actions_component.rb index 43e4fd217..94f3212b6 100644 --- a/app/components/shared/moderation_actions_component.rb +++ b/app/components/shared/moderation_actions_component.rb @@ -20,8 +20,8 @@ class Shared::ModerationActionsComponent < ApplicationComponent 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) + def confirm_block_author_text + t("admin.actions.confirm_action", action: t("admin.actions.block_author"), name: record.author.name) end def separator diff --git a/app/controllers/moderation/users_controller.rb b/app/controllers/moderation/users_controller.rb index 3466e695c..b3a043d96 100644 --- a/app/controllers/moderation/users_controller.rb +++ b/app/controllers/moderation/users_controller.rb @@ -6,16 +6,16 @@ class Moderation::UsersController < Moderation::BaseController def index end - def soft_block - soft_block_user + def hide + hide_user - redirect_with_query_params_to({ action: :index }, { notice: I18n.t("moderation.users.notice_soft_hide") }) + redirect_with_query_params_to({ action: :index }, { notice: I18n.t("moderation.users.notice_hide") }) end - def hide + def block block_user - redirect_with_query_params_to index_path_options, { notice: I18n.t("moderation.users.notice_hide") } + redirect_with_query_params_to index_path_options, { notice: I18n.t("moderation.users.notice_block") } end private @@ -24,9 +24,9 @@ class Moderation::UsersController < Moderation::BaseController @users = User.with_hidden.search(params[:search]).page(params[:page]).for_render end - def soft_block_user + def hide_user @user.hide - Activity.log(current_user, :soft_block, @user) + Activity.log(current_user, :hide, @user) end def block_user diff --git a/app/models/abilities/moderation.rb b/app/models/abilities/moderation.rb index 9c6cd3857..bda41b203 100644 --- a/app/models/abilities/moderation.rb +++ b/app/models/abilities/moderation.rb @@ -47,9 +47,6 @@ module Abilities can :hide, User cannot :hide, User, id: user.id - can :soft_block, User - cannot :soft_block, User, id: user.id - can :block, User cannot :block, User, id: user.id diff --git a/app/models/activity.rb b/app/models/activity.rb index 864ef0e5b..3accc495b 100644 --- a/app/models/activity.rb +++ b/app/models/activity.rb @@ -2,7 +2,7 @@ class Activity < ApplicationRecord belongs_to :actionable, -> { with_hidden }, polymorphic: true belongs_to :user, -> { with_hidden }, inverse_of: :activities - VALID_ACTIONS = %w[hide soft_block block restore valuate email].freeze + VALID_ACTIONS = %w[hide block restore valuate email].freeze validates :action, inclusion: { in: VALID_ACTIONS } diff --git a/app/views/moderation/users/index.html.erb b/app/views/moderation/users/index.html.erb index 56d15acd0..c2617282a 100644 --- a/app/views/moderation/users/index.html.erb +++ b/app/views/moderation/users/index.html.erb @@ -22,8 +22,8 @@ <%= t("moderation.users.index.hidden") %> <% else %> <%= render Admin::TableActionsComponent.new(user, actions: []) do |actions| %> - <%= actions.action(:soft_block, text: t("moderation.users.index.soft_hide"), method: :put) %> <%= actions.action(:hide, text: t("moderation.users.index.hide"), method: :put) %> + <%= actions.action(:block, text: t("moderation.users.index.block"), method: :put) %> <% end %> <% end %> diff --git a/config/locales/en/admin.yml b/config/locales/en/admin.yml index ea4897c6a..a740a897a 100644 --- a/config/locales/en/admin.yml +++ b/config/locales/en/admin.yml @@ -4,12 +4,12 @@ en: title: Administration actions: actions: Actions + block_author: "Block author" confirm_action: "Are you sure? %{action} \"%{name}\"" confirm_delete: "Are you sure? This action will delete \"%{name}\" and can't be undone." confirm_hide: Confirm moderation delete: "Delete" hide: Hide - hide_author: Hide author label: "%{action} %{name}" restore: Restore mark_featured: Featured diff --git a/config/locales/en/moderation.yml b/config/locales/en/moderation.yml index 63c834fcd..6fc1e98c4 100644 --- a/config/locales/en/moderation.yml +++ b/config/locales/en/moderation.yml @@ -101,10 +101,10 @@ en: title: Proposal notifications users: index: + block: Block hidden: Blocked - hide: Block + hide: Hide search_placeholder: email or name of user - soft_hide: Soft Block title: Block users - notice_hide: User blocked. All of this user's debates and comments have been hidden. - notice_soft_hide: User blocked. All ot this user's debates and comments are still available. + notice_block: User blocked. All of this user's debates and comments have been hidden. + notice_hide: User blocked. All ot this user's debates and comments are still available. diff --git a/config/locales/es/admin.yml b/config/locales/es/admin.yml index 82a55c334..9ff38ec50 100644 --- a/config/locales/es/admin.yml +++ b/config/locales/es/admin.yml @@ -4,12 +4,12 @@ es: title: Administración actions: actions: Acciones + block_author: "Bloquear al autor" confirm_action: "¿Estás seguro? %{action} \"%{name}\"" confirm_delete: "¿Estás seguro? Esta acción borrará \"%{name}\" y no se puede deshacer." confirm_hide: Confirmar moderación delete: Borrar hide: Ocultar - hide_author: Bloquear al autor label: "%{action} %{name}" restore: Volver a mostrar mark_featured: Destacar diff --git a/config/locales/es/moderation.yml b/config/locales/es/moderation.yml index 2fff7b996..445dca9f9 100644 --- a/config/locales/es/moderation.yml +++ b/config/locales/es/moderation.yml @@ -101,10 +101,10 @@ es: title: Notificaciones de propuestas users: index: + block: Bloquear hidden: Bloqueado - hide: Bloquear + hide: Ocultar search_placeholder: email o nombre de usuario - soft_hide: Ocultar title: Bloquear usuarios - notice_hide: Usuario bloqueado. Se han ocultado todos sus debates y comentarios. - notice_soft_hide: Usuario bloqueado. Todos sus debates y comentarios siguen disponibles. + notice_block: Usuario bloqueado. Se han ocultado todos sus debates y comentarios. + notice_hide: Usuario bloqueado. Todos sus debates y comentarios siguen disponibles. diff --git a/config/routes/moderation.rb b/config/routes/moderation.rb index 009d9484a..2bf1f41e7 100644 --- a/config/routes/moderation.rb +++ b/config/routes/moderation.rb @@ -4,7 +4,7 @@ namespace :moderation do resources :users, only: :index do member do put :hide - put :soft_block + put :block end end diff --git a/spec/controllers/moderation/users_controller_spec.rb b/spec/controllers/moderation/users_controller_spec.rb index 59d2e7f41..4f9fdea6d 100644 --- a/spec/controllers/moderation/users_controller_spec.rb +++ b/spec/controllers/moderation/users_controller_spec.rb @@ -6,7 +6,19 @@ describe Moderation::UsersController do describe "PUT hide" do it "keeps query parameters while using protected redirects" do - get :hide, params: { id: user, search: "user@consul.dev", host: "evil.dev" } + user = create(:user, email: "user@consul.dev") + + get :hide, params: { id: user, name_or_email: "user@consul.dev", host: "evil.dev" } + + expect(response).to redirect_to "/moderation/users?name_or_email=user%40consul.dev" + end + end + + describe "PUT block" do + it "keeps query parameters while using protected redirects" do + user = create(:user, email: "user@consul.dev") + + get :block, params: { id: user, search: "user@consul.dev", host: "evil.dev" } expect(response).to redirect_to "/moderation/users?search=user%40consul.dev" end @@ -15,7 +27,7 @@ describe Moderation::UsersController do proposal = create(:proposal, author: user) request.env["HTTP_REFERER"] = proposal_path(proposal) - put :hide, params: { id: user } + put :block, params: { id: user } expect(response).to redirect_to proposals_path expect(flash[:notice]).to eq "User blocked. All of this user's debates and comments have been hidden." @@ -25,19 +37,9 @@ describe Moderation::UsersController do investment = create(:budget_investment, author: user) request.env["HTTP_REFERER"] = budget_investment_path(investment.budget, investment) - put :hide, params: { id: user } + put :block, params: { id: user } expect(response).to redirect_to budget_investments_path(investment.budget) end end - - describe "PUT soft_block" do - it "keeps query parameters while using protected redirects" do - user = create(:user, email: "user@consul.dev") - - get :soft_block, params: { id: user, name_or_email: "user@consul.dev", host: "evil.dev" } - - expect(response).to redirect_to "/moderation/users?name_or_email=user%40consul.dev" - end - end end diff --git a/spec/models/abilities/moderator_spec.rb b/spec/models/abilities/moderator_spec.rb index 508d268f0..9f3cb14dc 100644 --- a/spec/models/abilities/moderator_spec.rb +++ b/spec/models/abilities/moderator_spec.rb @@ -90,9 +90,6 @@ describe Abilities::Moderator do it { should_not be_able_to(:hide, user) } it { should be_able_to(:hide, other_user) } - it { should_not be_able_to(:soft_block, user) } - it { should be_able_to(:soft_block, other_user) } - it { should_not be_able_to(:block, user) } it { should be_able_to(:block, other_user) } diff --git a/spec/system/admin/activity_spec.rb b/spec/system/admin/activity_spec.rb index 09236ac67..e501de72f 100644 --- a/spec/system/admin/activity_spec.rb +++ b/spec/system/admin/activity_spec.rb @@ -217,7 +217,7 @@ describe "Admin activity" do visit proposal_path(proposal) within("#proposal_#{proposal.id}") do - accept_confirm("Are you sure? Hide author \"#{proposal.author.name}\"") { click_link "Hide author" } + accept_confirm("Are you sure? Block author \"#{proposal.author.name}\"") { click_link "Block author" } expect(page).to have_current_path(proposals_path) end diff --git a/spec/system/admin/hidden_comments_spec.rb b/spec/system/admin/hidden_comments_spec.rb index 407207fa9..b73651257 100644 --- a/spec/system/admin/hidden_comments_spec.rb +++ b/spec/system/admin/hidden_comments_spec.rb @@ -13,7 +13,7 @@ describe "Admin hidden comments", :admin do visit proposal_path(proposal) within("#proposal_#{proposal.id}") do - accept_confirm("Are you sure? Hide author \"#{proposal.author.name}\"") { click_link "Hide author" } + accept_confirm("Are you sure? Block author \"#{proposal.author.name}\"") { click_link "Block author" } end expect(page).to have_current_path proposals_path diff --git a/spec/system/moderation/budget_investments_spec.rb b/spec/system/moderation/budget_investments_spec.rb index 9cfaf7849..aafd765f9 100644 --- a/spec/system/moderation/budget_investments_spec.rb +++ b/spec/system/moderation/budget_investments_spec.rb @@ -23,7 +23,7 @@ describe "Moderate budget investments" do login_as(mod.user) visit budget_investment_path(budget, investment) - accept_confirm("Are you sure? Hide author \"#{investment.author.name}\"") { click_link "Hide author" } + accept_confirm("Are you sure? Block author \"#{investment.author.name}\"") { click_link "Block author" } expect(page).to have_current_path(budget_investments_path(budget)) expect(page).not_to have_content(investment.title) @@ -37,7 +37,7 @@ describe "Moderate budget investments" do within "#budget_investment_#{investment.id}" do expect(page).not_to have_link("Hide") - expect(page).not_to have_link("Hide author") + expect(page).not_to have_link("Block author") end end diff --git a/spec/system/moderation/users_spec.rb b/spec/system/moderation/users_spec.rb index 3e54731bf..cd4c83ef2 100644 --- a/spec/system/moderation/users_spec.rb +++ b/spec/system/moderation/users_spec.rb @@ -24,7 +24,7 @@ describe "Moderate users" do visit debate_path(debate1) within("#debate_#{debate1.id}") do - accept_confirm("Are you sure? Hide author \"#{debate1.author.name}\"") { click_link "Hide author" } + accept_confirm("Are you sure? Block author \"#{debate1.author.name}\"") { click_link "Block author" } end expect(page).to have_current_path(debates_path) From 76555495f62b5dcae247f7106192d9bae88ae579 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 2 Dec 2021 03:38:33 +0100 Subject: [PATCH 08/16] Hide legislation proposals when blocking a user We're also updating the notice messages to specify all contents have been hidden (not just debates). --- app/models/user.rb | 2 ++ config/locales/en/moderation.yml | 4 ++-- config/locales/es/moderation.yml | 4 ++-- .../moderation/users_controller_spec.rb | 2 +- spec/factories/legislations.rb | 4 ++++ spec/models/user_spec.rb | 21 +++++++++++++++++++ 6 files changed, 32 insertions(+), 5 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 0e4be645d..0c6e03bfe 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -241,6 +241,7 @@ class User < ApplicationRecord Debate.hide_all debate_ids Comment.hide_all comment_ids + Legislation::Proposal.hide_all legislation_proposal_ids Proposal.hide_all proposal_ids Budget::Investment.hide_all budget_investment_ids ProposalNotification.hide_all ProposalNotification.where(author_id: id).ids @@ -250,6 +251,7 @@ class User < ApplicationRecord ActiveRecord::Base.transaction do Debate.restore_all debates.where("hidden_at >= ?", hidden_at) Comment.restore_all comments.where("hidden_at >= ?", hidden_at) + Legislation::Proposal.restore_all legislation_proposals.only_hidden.where("hidden_at >= ?", hidden_at) Proposal.restore_all proposals.where("hidden_at >= ?", hidden_at) Budget::Investment.restore_all budget_investments.where("hidden_at >= ?", hidden_at) ProposalNotification.restore_all( diff --git a/config/locales/en/moderation.yml b/config/locales/en/moderation.yml index 6fc1e98c4..473e7238b 100644 --- a/config/locales/en/moderation.yml +++ b/config/locales/en/moderation.yml @@ -106,5 +106,5 @@ en: hide: Hide search_placeholder: email or name of user title: Block users - notice_block: User blocked. All of this user's debates and comments have been hidden. - notice_hide: User blocked. All ot this user's debates and comments are still available. + notice_block: The user has been blocked. All contents authored by this user have been hidden. + notice_hide: The user has been hidden. Contents authored by this user are still available. diff --git a/config/locales/es/moderation.yml b/config/locales/es/moderation.yml index 445dca9f9..14d2c4d47 100644 --- a/config/locales/es/moderation.yml +++ b/config/locales/es/moderation.yml @@ -106,5 +106,5 @@ es: hide: Ocultar search_placeholder: email o nombre de usuario title: Bloquear usuarios - notice_block: Usuario bloqueado. Se han ocultado todos sus debates y comentarios. - notice_hide: Usuario bloqueado. Todos sus debates y comentarios siguen disponibles. + notice_block: Usuario bloqueado. Se han ocultado todos sus contenidos y comentarios. + notice_hide: Usuario ocultado. Todos sus contenidos y comentarios siguen disponibles. diff --git a/spec/controllers/moderation/users_controller_spec.rb b/spec/controllers/moderation/users_controller_spec.rb index 4f9fdea6d..a1e9fcea2 100644 --- a/spec/controllers/moderation/users_controller_spec.rb +++ b/spec/controllers/moderation/users_controller_spec.rb @@ -30,7 +30,7 @@ describe Moderation::UsersController do put :block, params: { id: user } expect(response).to redirect_to proposals_path - expect(flash[:notice]).to eq "User blocked. All of this user's debates and comments have been hidden." + expect(flash[:notice]).to eq "The user has been blocked. All contents authored by this user have been hidden." end it "redirects to the index with a nested resource" do diff --git a/spec/factories/legislations.rb b/spec/factories/legislations.rb index 94693417b..069899884 100644 --- a/spec/factories/legislations.rb +++ b/spec/factories/legislations.rb @@ -164,5 +164,9 @@ FactoryBot.define do terms_of_service { "1" } process factory: :legislation_process author factory: :user + + trait :hidden do + hidden_at { Time.current } + end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index d24392549..9c326f846 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -758,6 +758,21 @@ describe User do end end + describe "#block" do + it "hides legislation proposals created by the user" do + user = create(:user) + other_user = create(:user) + + proposal = create(:legislation_proposal, author: user) + other_proposal = create(:legislation_proposal, author: other_user) + + user.block + + expect(Legislation::Proposal.all).to eq [other_proposal] + expect(Legislation::Proposal.with_hidden).to match_array [proposal, other_proposal] + end + end + describe "#full_restore" do it "restore all previous hidden user content" do user = create(:user, :hidden) @@ -768,18 +783,21 @@ describe User do investment = create(:budget_investment, :hidden, author: user) proposal = create(:proposal, :hidden, author: user) proposal_notification = create(:proposal_notification, :hidden, proposal: proposal) + legislation_proposal = create(:legislation_proposal, :hidden, author: user) old_hidden_comment = create(:comment, hidden_at: 3.days.ago, author: user) old_hidden_debate = create(:debate, hidden_at: 3.days.ago, author: user) old_hidden_investment = create(:budget_investment, hidden_at: 3.days.ago, author: user) old_hidden_proposal = create(:proposal, hidden_at: 3.days.ago, author: user) old_hidden_proposal_notification = create(:proposal_notification, hidden_at: 3.days.ago, proposal: proposal) + old_hidden_legislation_proposal = create(:legislation_proposal, hidden_at: 3.days.ago, author: user) other_user_comment = create(:comment, :hidden, author: other_user) other_user_debate = create(:debate, :hidden, author: other_user) other_user_proposal = create(:proposal, :hidden, author: other_user) other_user_investment = create(:budget_investment, :hidden, author: other_user) other_user_proposal_notification = create(:proposal_notification, :hidden, proposal: other_user_proposal) + other_user_legislation_proposal = create(:legislation_proposal, :hidden, author: other_user) user.full_restore @@ -788,18 +806,21 @@ describe User do expect(investment.reload).not_to be_hidden expect(proposal.reload).not_to be_hidden expect(proposal_notification.reload).not_to be_hidden + expect(legislation_proposal.reload).not_to be_hidden expect(old_hidden_comment.reload).to be_hidden expect(old_hidden_debate.reload).to be_hidden expect(old_hidden_investment.reload).to be_hidden expect(old_hidden_proposal.reload).to be_hidden expect(old_hidden_proposal_notification.reload).to be_hidden + expect(old_hidden_legislation_proposal.reload).to be_hidden expect(other_user_comment.reload).to be_hidden expect(other_user_debate.reload).to be_hidden expect(other_user_investment.reload).to be_hidden expect(other_user_proposal.reload).to be_hidden expect(other_user_proposal_notification.reload).to be_hidden + expect(other_user_legislation_proposal.reload).to be_hidden end end end From f1389b2409e783de8a2e288a07837a2ff4661091 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 2 Dec 2021 16:13:10 +0100 Subject: [PATCH 09/16] Remove invalid HTML in proposal notifications This HTML wasn't valid because it was a containing the
element and it wasn't needed because there aren't any flag actions in proposal notifications. --- app/views/proposals/_notifications.html.erb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/views/proposals/_notifications.html.erb b/app/views/proposals/_notifications.html.erb index c91058d27..6452bdc66 100644 --- a/app/views/proposals/_notifications.html.erb +++ b/app/views/proposals/_notifications.html.erb @@ -13,9 +13,7 @@

<%= notification.created_at.to_date %>

<%= simple_format sanitize_and_auto_link(notification.body), {}, sanitize: false %> - - <%= render "proposal_notifications/actions", notification: notification %> - + <%= render "proposal_notifications/actions", notification: notification %>
<% end %>
From a5c66c728162958746de5408d7b69a4add54a0d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 2 Dec 2021 20:48:43 +0100 Subject: [PATCH 10/16] Use buttons instead of links to hide content We're continuing to replace links with buttons, for the reasons explained in commit 5311daadf. Since we're using the admin action component, we can also simplify the logic handling the confirmation message. In order to avoid duplicate IDs when generating buttons to block the same author more than once in a page, we're including the record dom_id in the ID of the button to block an author. --- app/assets/stylesheets/application.scss | 1 + .../stylesheets/moderation_actions.scss | 10 +++++++ .../moderation_actions_component.html.erb | 27 ++++++++++++++----- .../shared/moderation_actions_component.rb | 14 ++++------ .../moderation_actions_component_spec.rb | 26 +++++++++++++----- spec/system/admin/activity_spec.rb | 8 +++--- spec/system/admin/hidden_comments_spec.rb | 2 +- .../moderation/budget_investments_spec.rb | 8 +++--- spec/system/moderation/comments_spec.rb | 6 ++--- spec/system/moderation/debates_spec.rb | 6 ++--- .../moderation/legislation_proposals_spec.rb | 4 +-- .../moderation/proposal_notifications_spec.rb | 6 ++--- spec/system/moderation/proposals_spec.rb | 6 ++--- spec/system/moderation/users_spec.rb | 2 +- 14 files changed, 79 insertions(+), 47 deletions(-) create mode 100644 app/assets/stylesheets/moderation_actions.scss diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index 74675f1c3..08bb60faa 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -30,6 +30,7 @@ @import "legislation"; @import "legislation_process"; @import "legislation_process_form"; +@import "moderation_actions"; @import "notification_item"; @import "community"; @import "stats"; diff --git a/app/assets/stylesheets/moderation_actions.scss b/app/assets/stylesheets/moderation_actions.scss new file mode 100644 index 000000000..c73206718 --- /dev/null +++ b/app/assets/stylesheets/moderation_actions.scss @@ -0,0 +1,10 @@ +.moderation-actions { + &, + form { + display: inline; + } + + button { + @include link; + } +} diff --git a/app/components/shared/moderation_actions_component.html.erb b/app/components/shared/moderation_actions_component.html.erb index 5d37fed88..c56a95e74 100644 --- a/app/components/shared/moderation_actions_component.html.erb +++ b/app/components/shared/moderation_actions_component.html.erb @@ -1,12 +1,25 @@ - +
<% if can? :hide, record %> - <%= link_to t("admin.actions.hide").capitalize, hide_path, - method: :put, remote: true, data: { confirm: confirm_hide_text } %> + <%= render Admin::ActionComponent.new( + :hide, + record, + path: hide_path, + method: :put, + remote: true, + confirm: true + ) %> <% end %> - <% if can? :hide, record.author %> + <% if can? :hide, author %> <%= raw separator %> - <%= link_to t("admin.actions.block_author").capitalize, block_moderation_user_path(record.author_id), - method: :put, data: { confirm: confirm_block_author_text } %> + + <%= render Admin::ActionComponent.new( + :block_author, + author, + path: block_moderation_user_path(author), + id: dom_id(author, "#{dom_id(record)}_block_author"), + method: :put, + confirm: true + ) %> <% end %> - +
diff --git a/app/components/shared/moderation_actions_component.rb b/app/components/shared/moderation_actions_component.rb index 94f3212b6..2207e340d 100644 --- a/app/components/shared/moderation_actions_component.rb +++ b/app/components/shared/moderation_actions_component.rb @@ -7,23 +7,19 @@ class Shared::ModerationActionsComponent < ApplicationComponent end def render? - can?(:hide, record) || can?(:hide, record.author) + can?(:hide, record) || can?(:hide, author) end private + def author + record.author + end + 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_block_author_text - t("admin.actions.confirm_action", action: t("admin.actions.block_author"), name: record.author.name) - end - def separator if record.is_a?(Comment) " • " diff --git a/spec/components/shared/moderation_actions_component_spec.rb b/spec/components/shared/moderation_actions_component_spec.rb index be6237c7d..f305a14d6 100644 --- a/spec/components/shared/moderation_actions_component_spec.rb +++ b/spec/components/shared/moderation_actions_component_spec.rb @@ -4,13 +4,15 @@ describe Shared::ModerationActionsComponent do include Rails.application.routes.url_helpers before { sign_in(create(:administrator).user) } - describe "Hide link" do + describe "Hide button" 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) + page.find("form[action='#{hide_moderation_debate_path(debate)}']") do + expect(page).to have_button "Hide" + end end it "is shown for proposals" do @@ -18,7 +20,9 @@ describe Shared::ModerationActionsComponent do render_inline Shared::ModerationActionsComponent.new(proposal) - expect(page).to have_link "Hide", href: hide_moderation_proposal_path(proposal) + page.find("form[action='#{hide_moderation_proposal_path(proposal)}']") do + expect(page).to have_button "Hide" + end end it "is shown for proposal notifications" do @@ -26,7 +30,9 @@ describe Shared::ModerationActionsComponent do render_inline Shared::ModerationActionsComponent.new(notification) - expect(page).to have_link "Hide", href: hide_moderation_proposal_notification_path(notification) + page.find("form[action='#{hide_moderation_proposal_notification_path(notification)}']") do + expect(page).to have_button "Hide" + end end it "is shown for comments" do @@ -34,7 +40,9 @@ describe Shared::ModerationActionsComponent do render_inline Shared::ModerationActionsComponent.new(comment) - expect(page).to have_link "Hide", href: hide_moderation_comment_path(comment) + page.find("form[action='#{hide_moderation_comment_path(comment)}']") do + expect(page).to have_button "Hide" + end end it "is shown for budget investments" do @@ -42,7 +50,9 @@ describe Shared::ModerationActionsComponent do render_inline Shared::ModerationActionsComponent.new(investment) - expect(page).to have_link "Hide", href: hide_moderation_budget_investment_path(investment) + page.find("form[action='#{hide_moderation_budget_investment_path(investment)}']") do + expect(page).to have_button "Hide" + end end it "is shown for legislation proposals" do @@ -50,7 +60,9 @@ describe Shared::ModerationActionsComponent do render_inline Shared::ModerationActionsComponent.new(proposal) - expect(page).to have_link "Hide", href: hide_moderation_legislation_proposal_path(proposal) + page.find("form[action='#{hide_moderation_legislation_proposal_path(proposal)}']") do + expect(page).to have_button "Hide" + end end end end diff --git a/spec/system/admin/activity_spec.rb b/spec/system/admin/activity_spec.rb index e501de72f..537fe6c34 100644 --- a/spec/system/admin/activity_spec.rb +++ b/spec/system/admin/activity_spec.rb @@ -14,7 +14,7 @@ describe "Admin activity" do visit proposal_path(proposal) within("#proposal_#{proposal.id}") do - accept_confirm("Are you sure? Hide \"#{proposal.title}\"") { click_link "Hide" } + accept_confirm("Are you sure? Hide \"#{proposal.title}\"") { click_button "Hide" } end expect(page).to have_css("#proposal_#{proposal.id}.faded") @@ -82,7 +82,7 @@ describe "Admin activity" do visit debate_path(debate) within("#debate_#{debate.id}") do - accept_confirm("Are you sure? Hide \"#{debate.title}\"") { click_link "Hide" } + accept_confirm("Are you sure? Hide \"#{debate.title}\"") { click_button "Hide" } end expect(page).to have_css("#debate_#{debate.id}.faded") @@ -150,7 +150,7 @@ describe "Admin activity" do visit debate_path(debate) within("#comment_#{comment.id}") do - accept_confirm("Are you sure? Hide \"#{comment.body}\"") { click_link "Hide" } + accept_confirm("Are you sure? Hide \"#{comment.body}\"") { click_button "Hide" } expect(page).to have_css(".faded") end @@ -217,7 +217,7 @@ describe "Admin activity" do visit proposal_path(proposal) within("#proposal_#{proposal.id}") do - accept_confirm("Are you sure? Block author \"#{proposal.author.name}\"") { click_link "Block author" } + accept_confirm("Are you sure? Block author \"#{proposal.author.name}\"") { click_button "Block author" } expect(page).to have_current_path(proposals_path) end diff --git a/spec/system/admin/hidden_comments_spec.rb b/spec/system/admin/hidden_comments_spec.rb index b73651257..d92b811cd 100644 --- a/spec/system/admin/hidden_comments_spec.rb +++ b/spec/system/admin/hidden_comments_spec.rb @@ -13,7 +13,7 @@ describe "Admin hidden comments", :admin do visit proposal_path(proposal) within("#proposal_#{proposal.id}") do - accept_confirm("Are you sure? Block author \"#{proposal.author.name}\"") { click_link "Block author" } + accept_confirm("Are you sure? Block author \"#{proposal.author.name}\"") { click_button "Block author" } end expect(page).to have_current_path proposals_path diff --git a/spec/system/moderation/budget_investments_spec.rb b/spec/system/moderation/budget_investments_spec.rb index aafd765f9..f24291c73 100644 --- a/spec/system/moderation/budget_investments_spec.rb +++ b/spec/system/moderation/budget_investments_spec.rb @@ -10,7 +10,7 @@ describe "Moderate budget investments" do login_as(mod.user) visit budget_investment_path(budget, investment) - accept_confirm("Are you sure? Hide \"#{investment.title}\"") { click_link "Hide" } + accept_confirm("Are you sure? Hide \"#{investment.title}\"") { click_button "Hide" } expect(page).to have_css(".faded", count: 2) @@ -23,7 +23,7 @@ describe "Moderate budget investments" do login_as(mod.user) visit budget_investment_path(budget, investment) - accept_confirm("Are you sure? Block author \"#{investment.author.name}\"") { click_link "Block author" } + accept_confirm("Are you sure? Block author \"#{investment.author.name}\"") { click_button "Block author" } expect(page).to have_current_path(budget_investments_path(budget)) expect(page).not_to have_content(investment.title) @@ -36,8 +36,8 @@ describe "Moderate budget investments" do visit budget_investment_path(budget, investment) within "#budget_investment_#{investment.id}" do - expect(page).not_to have_link("Hide") - expect(page).not_to have_link("Block author") + expect(page).not_to have_button "Hide" + expect(page).not_to have_button "Block author" end end diff --git a/spec/system/moderation/comments_spec.rb b/spec/system/moderation/comments_spec.rb index 8321f2c2f..aeaf5c280 100644 --- a/spec/system/moderation/comments_spec.rb +++ b/spec/system/moderation/comments_spec.rb @@ -11,7 +11,7 @@ describe "Moderate comments" do visit debate_path(comment.commentable) within("#comment_#{comment.id}") do - accept_confirm("Are you sure? Hide") { click_link "Hide" } + accept_confirm("Are you sure? Hide") { click_button "Hide" } expect(page).to have_css(".comment .faded") end @@ -31,8 +31,8 @@ describe "Moderate comments" do visit debate_path(comment.commentable) within("#comment_#{comment.id}") do - expect(page).not_to have_link("Hide") - expect(page).not_to have_link("Block author") + expect(page).not_to have_button "Hide" + expect(page).not_to have_button "Block author" end end diff --git a/spec/system/moderation/debates_spec.rb b/spec/system/moderation/debates_spec.rb index 085682d53..8cd734858 100644 --- a/spec/system/moderation/debates_spec.rb +++ b/spec/system/moderation/debates_spec.rb @@ -11,7 +11,7 @@ describe "Moderate debates" do visit debate_path(debate) within("#debate_#{debate.id}") do - accept_confirm("Are you sure? Hide") { click_link "Hide" } + accept_confirm("Are you sure? Hide") { click_button "Hide" } end expect(find("div#debate_#{debate.id}.faded")).to have_text debate.title @@ -30,8 +30,8 @@ describe "Moderate debates" do visit debate_path(debate) within("#debate_#{debate.id}") do - expect(page).not_to have_link("Hide") - expect(page).not_to have_link("Block author") + expect(page).not_to have_button "Hide" + expect(page).not_to have_button "Block author" end end diff --git a/spec/system/moderation/legislation_proposals_spec.rb b/spec/system/moderation/legislation_proposals_spec.rb index 98f7ea48a..3da95c244 100644 --- a/spec/system/moderation/legislation_proposals_spec.rb +++ b/spec/system/moderation/legislation_proposals_spec.rb @@ -11,7 +11,7 @@ describe "Moderate legislation proposals" do visit legislation_process_proposal_path(legislation_process, legislation_proposal) within("#legislation_proposal_#{legislation_proposal.id}") do - accept_confirm("Are you sure? Hide \"#{legislation_proposal.title}\"") { click_link "Hide" } + accept_confirm("Are you sure? Hide \"#{legislation_proposal.title}\"") { click_button "Hide" } end expect(page).to have_css("#legislation_proposal_#{legislation_proposal.id}.faded") @@ -21,6 +21,6 @@ describe "Moderate legislation proposals" do visit legislation_process_proposals_path(legislation_process) expect(page).to have_css(".proposal-content", count: 0) - expect(page).not_to have_link("Hide") + expect(page).not_to have_button "Hide" end end diff --git a/spec/system/moderation/proposal_notifications_spec.rb b/spec/system/moderation/proposal_notifications_spec.rb index 6515c6566..d2ad9b7d2 100644 --- a/spec/system/moderation/proposal_notifications_spec.rb +++ b/spec/system/moderation/proposal_notifications_spec.rb @@ -12,7 +12,7 @@ describe "Moderate proposal notifications" do click_link "Notifications (1)" within("#proposal_notification_#{proposal_notification.id}") do - accept_confirm("Are you sure? Hide") { click_link "Hide" } + accept_confirm("Are you sure? Hide") { click_button "Hide" } end expect(page).to have_css("#proposal_notification_#{proposal_notification.id}.faded") @@ -34,8 +34,8 @@ describe "Moderate proposal notifications" do click_link "Notifications (1)" within("#proposal_notification_#{proposal_notification.id}") do - expect(page).not_to have_link("Hide") - expect(page).not_to have_link("Block author") + expect(page).not_to have_button "Hide" + expect(page).not_to have_button "Block author" end end diff --git a/spec/system/moderation/proposals_spec.rb b/spec/system/moderation/proposals_spec.rb index 9ab06e314..ceef0bf86 100644 --- a/spec/system/moderation/proposals_spec.rb +++ b/spec/system/moderation/proposals_spec.rb @@ -10,7 +10,7 @@ describe "Moderate proposals" do visit proposal_path(proposal) within("#proposal_#{proposal.id}") do - accept_confirm("Are you sure? Hide") { click_link "Hide" } + accept_confirm("Are you sure? Hide") { click_button "Hide" } end expect(page).to have_css("#proposal_#{proposal.id}.faded") @@ -29,8 +29,8 @@ describe "Moderate proposals" do visit proposal_path(proposal) within("#proposal_#{proposal.id}") do - expect(page).to have_link("Hide") - expect(page).not_to have_link("Block author") + expect(page).to have_button "Hide" + expect(page).not_to have_button "Block author" end end diff --git a/spec/system/moderation/users_spec.rb b/spec/system/moderation/users_spec.rb index cd4c83ef2..fa2ace29f 100644 --- a/spec/system/moderation/users_spec.rb +++ b/spec/system/moderation/users_spec.rb @@ -24,7 +24,7 @@ describe "Moderate users" do visit debate_path(debate1) within("#debate_#{debate1.id}") do - accept_confirm("Are you sure? Block author \"#{debate1.author.name}\"") { click_link "Block author" } + accept_confirm("Are you sure? Block author \"#{debate1.author.name}\"") { click_button "Block author" } end expect(page).to have_current_path(debates_path) From 3b2b09be2bee7e563a5465aebf81064918341ba3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 2 Dec 2021 21:02:01 +0100 Subject: [PATCH 11/16] Use CSS to show separators in moderation actions This is useful for people using screen readers, since the character used as a separator won't be read aloud. Since many screen readers also read content generated via CSS pseudoelements, we aren't using `content: "|";` or similar but using elements with a very small width instead. --- .../stylesheets/moderation_actions.scss | 26 +++++++++++++++++++ .../moderation_actions_component.html.erb | 2 -- .../shared/moderation_actions_component.rb | 8 ------ app/views/comments/_actions.html.erb | 2 +- 4 files changed, 27 insertions(+), 11 deletions(-) diff --git a/app/assets/stylesheets/moderation_actions.scss b/app/assets/stylesheets/moderation_actions.scss index c73206718..f3a00d662 100644 --- a/app/assets/stylesheets/moderation_actions.scss +++ b/app/assets/stylesheets/moderation_actions.scss @@ -7,4 +7,30 @@ button { @include link; } + + @mixin separator { + content: ""; + display: inline-block; + margin: 0 0.3em; + vertical-align: middle; + } + + > * + *::before { + @include separator; + background: currentcolor; + height: 1em; + opacity: 0.5; + width: 1px; + } + + .comment & { + > *::before { + @include separator; + background: $text-light; + border-radius: 100%; + opacity: 1; + height: 0.25em; + width: 0.25em; + } + } } diff --git a/app/components/shared/moderation_actions_component.html.erb b/app/components/shared/moderation_actions_component.html.erb index c56a95e74..9f6fc1346 100644 --- a/app/components/shared/moderation_actions_component.html.erb +++ b/app/components/shared/moderation_actions_component.html.erb @@ -11,8 +11,6 @@ <% end %> <% if can? :hide, author %> - <%= raw separator %> - <%= render Admin::ActionComponent.new( :block_author, author, diff --git a/app/components/shared/moderation_actions_component.rb b/app/components/shared/moderation_actions_component.rb index 2207e340d..f9f287e09 100644 --- a/app/components/shared/moderation_actions_component.rb +++ b/app/components/shared/moderation_actions_component.rb @@ -19,12 +19,4 @@ class Shared::ModerationActionsComponent < ApplicationComponent def hide_path polymorphic_path([:moderation, record], action: :hide) end - - def separator - if record.is_a?(Comment) - " • " - else - " | " - end - end end diff --git a/app/views/comments/_actions.html.erb b/app/views/comments/_actions.html.erb index 4710f2931..7780d9167 100644 --- a/app/views/comments/_actions.html.erb +++ b/app/views/comments/_actions.html.erb @@ -3,8 +3,8 @@ <% 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", From 7caee9a93c0b07b92b971b50fa57b05a8cab66ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 22 Dec 2021 15:50:32 +0100 Subject: [PATCH 12/16] Show comments with hidden authors In the past, whenever we hid users, we also hid their comments. However, we've now implemented an action to hide users without hiding their comments. In this case, we still want to show the comment, but we weren't doing so. --- app/views/comments/_comment.html.erb | 2 +- spec/system/comments/debates_spec.rb | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/app/views/comments/_comment.html.erb b/app/views/comments/_comment.html.erb index ae0cb1276..0ea042746 100644 --- a/app/views/comments/_comment.html.erb +++ b/app/views/comments/_comment.html.erb @@ -2,7 +2,7 @@ <% cache [locale_and_user_status(comment), comment, commentable_cache_key(comment.commentable), comment.author] do %>
- <% if comment.hidden? || comment.user.hidden? %> + <% if comment.hidden? %> <% if comment.children.size > 0 %>

<%= t("comments.comment.deleted") %>

diff --git a/spec/system/comments/debates_spec.rb b/spec/system/comments/debates_spec.rb index 706cd8142..57cca11d7 100644 --- a/spec/system/comments/debates_spec.rb +++ b/spec/system/comments/debates_spec.rb @@ -371,6 +371,16 @@ describe "Commenting debates" do end end + scenario "Show comment when the author is hidden" do + create(:comment, body: "This is pointless", commentable: debate, author: create(:user, :hidden)) + + visit debate_path(debate) + + within ".comment", text: "This is pointless" do + expect(page).to have_content "User deleted" + end + end + scenario "Errors on reply" do comment = create(:comment, commentable: debate, user: user) From 992da1fef3e0fd1bad0038c01d658c9dc114d2b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 22 Dec 2021 16:02:49 +0100 Subject: [PATCH 13/16] Make sure hidden users are shown in order The test "Action links remember the pagination setting and the filter" was failing sometimes because it assumed the third user created was going to appear in the third place, but that wasn't always the case. So we're using the same order we use in the rest of the sections dealing with hidden content. --- app/controllers/admin/hidden_users_controller.rb | 2 +- spec/system/admin/hidden_users_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/admin/hidden_users_controller.rb b/app/controllers/admin/hidden_users_controller.rb index 1b5635270..86801fcda 100644 --- a/app/controllers/admin/hidden_users_controller.rb +++ b/app/controllers/admin/hidden_users_controller.rb @@ -4,7 +4,7 @@ class Admin::HiddenUsersController < Admin::BaseController before_action :load_user, only: [:confirm_hide, :restore] def index - @users = User.only_hidden.send(@current_filter).page(params[:page]) + @users = User.only_hidden.send(@current_filter).order(hidden_at: :desc).page(params[:page]) end def show diff --git a/spec/system/admin/hidden_users_spec.rb b/spec/system/admin/hidden_users_spec.rb index 59b19602e..0cf4744d4 100644 --- a/spec/system/admin/hidden_users_spec.rb +++ b/spec/system/admin/hidden_users_spec.rb @@ -83,7 +83,7 @@ describe "Admin hidden users", :admin do visit admin_hidden_users_path(filter: "with_confirmed_hide", page: 2) - accept_confirm("Are you sure? Restore \"#{users[2].name}\"") do + accept_confirm("Are you sure? Restore \"#{users[-3].name}\"") do click_button "Restore", match: :first, exact: true end From a31e73bf23bbfd82482d228644c8b75e7eb6f876 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 29 Dec 2021 20:33:15 +0100 Subject: [PATCH 14/16] Ask for confirmation when hiding/blocking users In the moderation section there's no clear indicator as to what the "Hide" and "Block" buttons do and the difference between them. Since we're using confirmation dialogs in all moderation actions except these ones, we're adding them here as well, so the difference will appear in the dialog. This isn't a very good solution, though, since the confirmation dialog comes after clicking the button and users have already been wondering whether clicking that button will be the right choice. A better solution would be making the purpose clear before the button is clicked, although that's something we don't do anywhere in the admin/moderation sections. --- app/components/admin/action_component.rb | 2 ++ .../shared/moderation_actions_component.html.erb | 2 +- app/views/moderation/users/index.html.erb | 14 ++++++++++++-- config/locales/en/moderation.yml | 2 ++ config/locales/es/moderation.yml | 2 ++ spec/system/admin/activity_spec.rb | 8 ++++++-- spec/system/admin/hidden_comments_spec.rb | 4 +++- spec/system/moderation/budget_investments_spec.rb | 4 +++- spec/system/moderation/users_spec.rb | 7 +++++-- 9 files changed, 36 insertions(+), 9 deletions(-) diff --git a/app/components/admin/action_component.rb b/app/components/admin/action_component.rb index 2235bb682..33e00e01d 100644 --- a/app/components/admin/action_component.rb +++ b/app/components/admin/action_component.rb @@ -68,6 +68,8 @@ class Admin::ActionComponent < ApplicationComponent else t("admin.actions.confirm_action", action: text, name: record_name) end + elsif options[:confirm].respond_to?(:call) + options[:confirm].call(record_name) else options[:confirm] end diff --git a/app/components/shared/moderation_actions_component.html.erb b/app/components/shared/moderation_actions_component.html.erb index 9f6fc1346..a3aed9369 100644 --- a/app/components/shared/moderation_actions_component.html.erb +++ b/app/components/shared/moderation_actions_component.html.erb @@ -17,7 +17,7 @@ path: block_moderation_user_path(author), id: dom_id(author, "#{dom_id(record)}_block_author"), method: :put, - confirm: true + confirm: ->(name) { t("moderation.users.index.confirm_block", name: name) } ) %> <% end %>
diff --git a/app/views/moderation/users/index.html.erb b/app/views/moderation/users/index.html.erb index c2617282a..2e85185e3 100644 --- a/app/views/moderation/users/index.html.erb +++ b/app/views/moderation/users/index.html.erb @@ -22,8 +22,18 @@ <%= t("moderation.users.index.hidden") %> <% else %> <%= render Admin::TableActionsComponent.new(user, actions: []) do |actions| %> - <%= actions.action(:hide, text: t("moderation.users.index.hide"), method: :put) %> - <%= actions.action(:block, text: t("moderation.users.index.block"), method: :put) %> + <%= actions.action( + :hide, + text: t("moderation.users.index.hide"), + confirm: ->(name) { t("moderation.users.index.confirm_hide", name: name) }, + method: :put + ) %> + <%= actions.action( + :block, + text: t("moderation.users.index.block"), + confirm: ->(name) { t("moderation.users.index.confirm_block", name: name) }, + method: :put + ) %> <% end %> <% end %> diff --git a/config/locales/en/moderation.yml b/config/locales/en/moderation.yml index 473e7238b..ff0fb98c6 100644 --- a/config/locales/en/moderation.yml +++ b/config/locales/en/moderation.yml @@ -102,6 +102,8 @@ en: users: index: block: Block + confirm_block: "Are you sure? This will hide the user \"%{name}\" and all their contents." + confirm_hide: "Are you sure? This will hide the user \"%{name}\" without hiding their contents." hidden: Blocked hide: Hide search_placeholder: email or name of user diff --git a/config/locales/es/moderation.yml b/config/locales/es/moderation.yml index 14d2c4d47..b5580317a 100644 --- a/config/locales/es/moderation.yml +++ b/config/locales/es/moderation.yml @@ -102,6 +102,8 @@ es: users: index: block: Bloquear + confirm_block: "¿Seguro? Esto ocultará a \"%{name}\" así como todos sus contenidos." + confirm_hide: "¿Seguro? Esto ocultará a \"%{name}\" sin ocultar sus contenidos." hidden: Bloqueado hide: Ocultar search_placeholder: email o nombre de usuario diff --git a/spec/system/admin/activity_spec.rb b/spec/system/admin/activity_spec.rb index 537fe6c34..ee6023993 100644 --- a/spec/system/admin/activity_spec.rb +++ b/spec/system/admin/activity_spec.rb @@ -217,7 +217,9 @@ describe "Admin activity" do visit proposal_path(proposal) within("#proposal_#{proposal.id}") do - accept_confirm("Are you sure? Block author \"#{proposal.author.name}\"") { click_button "Block author" } + accept_confirm("Are you sure? This will hide the user \"#{proposal.author.name}\" and all their contents.") do + click_button "Block author" + end expect(page).to have_current_path(proposals_path) end @@ -239,9 +241,11 @@ describe "Admin activity" do visit moderation_users_path(search: user.username) within("#moderation_users") do - click_button "Block" + accept_confirm { click_button "Block" } end + expect(page).to have_content "The user has been blocked" + visit admin_activity_path within first("tbody tr") do diff --git a/spec/system/admin/hidden_comments_spec.rb b/spec/system/admin/hidden_comments_spec.rb index d92b811cd..36902e4d8 100644 --- a/spec/system/admin/hidden_comments_spec.rb +++ b/spec/system/admin/hidden_comments_spec.rb @@ -13,7 +13,9 @@ describe "Admin hidden comments", :admin do visit proposal_path(proposal) within("#proposal_#{proposal.id}") do - accept_confirm("Are you sure? Block author \"#{proposal.author.name}\"") { click_button "Block author" } + accept_confirm("Are you sure? This will hide the user \"#{proposal.author.name}\" and all their contents.") do + click_button "Block author" + end end expect(page).to have_current_path proposals_path diff --git a/spec/system/moderation/budget_investments_spec.rb b/spec/system/moderation/budget_investments_spec.rb index f24291c73..76d3f7c2f 100644 --- a/spec/system/moderation/budget_investments_spec.rb +++ b/spec/system/moderation/budget_investments_spec.rb @@ -23,7 +23,9 @@ describe "Moderate budget investments" do login_as(mod.user) visit budget_investment_path(budget, investment) - accept_confirm("Are you sure? Block author \"#{investment.author.name}\"") { click_button "Block author" } + accept_confirm("Are you sure? This will hide the user \"#{investment.author.name}\" and all their contents.") do + click_button "Block author" + end expect(page).to have_current_path(budget_investments_path(budget)) expect(page).not_to have_content(investment.title) diff --git a/spec/system/moderation/users_spec.rb b/spec/system/moderation/users_spec.rb index fa2ace29f..d9e92e5b4 100644 --- a/spec/system/moderation/users_spec.rb +++ b/spec/system/moderation/users_spec.rb @@ -24,7 +24,9 @@ describe "Moderate users" do visit debate_path(debate1) within("#debate_#{debate1.id}") do - accept_confirm("Are you sure? Block author \"#{debate1.author.name}\"") { click_button "Block author" } + accept_confirm("Are you sure? This will hide the user \"#{debate1.author.name}\" and all their contents.") do + click_button "Block author" + end end expect(page).to have_current_path(debates_path) @@ -64,7 +66,8 @@ describe "Moderate users" do within("#moderation_users") do expect(page).to have_content citizen.name expect(page).not_to have_content "Blocked" - click_button "Block" + + accept_confirm { click_button "Block" } end within("#moderation_users") do From e9d9789c25a27957fc3f419710c4f07a279d82e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 30 Dec 2021 13:21:02 +0100 Subject: [PATCH 15/16] Move users moderation index view to a component This way it's easier to change the logic and write tests for it. --- .../moderation/users/index_component.html.erb | 47 ++++++++++++++++++ .../moderation/users/index_component.rb | 7 +++ app/views/moderation/users/index.html.erb | 48 +------------------ .../moderation/users/index_component_spec.rb | 30 ++++++++++++ 4 files changed, 85 insertions(+), 47 deletions(-) create mode 100644 app/components/moderation/users/index_component.html.erb create mode 100644 app/components/moderation/users/index_component.rb create mode 100644 spec/components/moderation/users/index_component_spec.rb diff --git a/app/components/moderation/users/index_component.html.erb b/app/components/moderation/users/index_component.html.erb new file mode 100644 index 000000000..d5b995825 --- /dev/null +++ b/app/components/moderation/users/index_component.html.erb @@ -0,0 +1,47 @@ +
+

<%= t("moderation.users.index.title") %>

+ + <%= render Admin::SearchComponent.new(label: t("moderation.users.index.search_placeholder")) %> + + <% if users.present? %> +

<%= page_entries_info users %>

+ + + + + + + + <% users.each do |user| %> + + + + + <% end %> + +
<%= t("admin.hidden_users.index.user") %><%= t("admin.actions.actions") %>
+ <%= user.name %> + + <% if user.hidden? %> + <%= t("moderation.users.index.hidden") %> + <% else %> + <%= render Admin::TableActionsComponent.new(user, actions: []) do |actions| %> + <%= actions.action( + :hide, + text: t("moderation.users.index.hide"), + confirm: ->(name) { t("moderation.users.index.confirm_hide", name: name) }, + method: :put + ) %> + <%= actions.action( + :block, + text: t("moderation.users.index.block"), + confirm: ->(name) { t("moderation.users.index.confirm_block", name: name) }, + method: :put + ) %> + <% end %> + <% end %> +
+ + <%= paginate users %> + <% end %> +
diff --git a/app/components/moderation/users/index_component.rb b/app/components/moderation/users/index_component.rb new file mode 100644 index 000000000..8e738032c --- /dev/null +++ b/app/components/moderation/users/index_component.rb @@ -0,0 +1,7 @@ +class Moderation::Users::IndexComponent < ApplicationComponent + attr_reader :users + + def initialize(users) + @users = users + end +end diff --git a/app/views/moderation/users/index.html.erb b/app/views/moderation/users/index.html.erb index 2e85185e3..39b526378 100644 --- a/app/views/moderation/users/index.html.erb +++ b/app/views/moderation/users/index.html.erb @@ -1,47 +1 @@ -
-

<%= t("moderation.users.index.title") %>

- - <%= render Admin::SearchComponent.new(label: t("moderation.users.index.search_placeholder")) %> - - <% if @users.present? %> -

<%= page_entries_info @users %>

- - - - - - - - <% @users.each do |user| %> - - - - - <% end %> - -
<%= t("admin.hidden_users.index.user") %><%= t("admin.actions.actions") %>
- <%= user.name %> - - <% if user.hidden? %> - <%= t("moderation.users.index.hidden") %> - <% else %> - <%= render Admin::TableActionsComponent.new(user, actions: []) do |actions| %> - <%= actions.action( - :hide, - text: t("moderation.users.index.hide"), - confirm: ->(name) { t("moderation.users.index.confirm_hide", name: name) }, - method: :put - ) %> - <%= actions.action( - :block, - text: t("moderation.users.index.block"), - confirm: ->(name) { t("moderation.users.index.confirm_block", name: name) }, - method: :put - ) %> - <% end %> - <% end %> -
- - <%= paginate @users %> - <% end %> -
+<%= render Moderation::Users::IndexComponent.new(@users) %> diff --git a/spec/components/moderation/users/index_component_spec.rb b/spec/components/moderation/users/index_component_spec.rb new file mode 100644 index 000000000..49648ff7d --- /dev/null +++ b/spec/components/moderation/users/index_component_spec.rb @@ -0,0 +1,30 @@ +require "rails_helper" + +describe Moderation::Users::IndexComponent, controller: Moderation::UsersController do + describe "actions or status" do + let(:component) { Moderation::Users::IndexComponent.new(User.with_hidden.page(1)) } + + it "shows actions to block or hide users" do + create(:user) + + render_inline component + + page.find("table") do |table| + expect(table).to have_button "Hide" + expect(table).to have_button "Block" + expect(table).not_to have_content "Blocked" + end + end + + it "shows 'blocked' for hidden users" do + create(:user, :hidden) + + render_inline component + + page.find("table") do |table| + expect(table).to have_content "Blocked" + expect(table).not_to have_button + end + end + end +end From efb7ad9e420dd82632dff6cef8a0b7143a939e96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 30 Dec 2021 14:29:03 +0100 Subject: [PATCH 16/16] Differentiate between blocked and hidden users It was a bit confusing to press the "hide" button and then see the user listed as "blocked". Some moderators might think they accidentally pressed the wrong button. --- .../moderation/users/index_component.html.erb | 2 +- .../moderation/users/index_component.rb | 10 +++ config/locales/en/moderation.yml | 1 - config/locales/es/moderation.yml | 1 - .../moderation/users/index_component_spec.rb | 85 +++++++++++++++++-- spec/system/moderation/users_spec.rb | 19 +++++ 6 files changed, 109 insertions(+), 9 deletions(-) diff --git a/app/components/moderation/users/index_component.html.erb b/app/components/moderation/users/index_component.html.erb index d5b995825..ef5cb7db2 100644 --- a/app/components/moderation/users/index_component.html.erb +++ b/app/components/moderation/users/index_component.html.erb @@ -19,7 +19,7 @@ <% if user.hidden? %> - <%= t("moderation.users.index.hidden") %> + <%= status(user) %> <% else %> <%= render Admin::TableActionsComponent.new(user, actions: []) do |actions| %> <%= actions.action( diff --git a/app/components/moderation/users/index_component.rb b/app/components/moderation/users/index_component.rb index 8e738032c..ddfa20e85 100644 --- a/app/components/moderation/users/index_component.rb +++ b/app/components/moderation/users/index_component.rb @@ -4,4 +4,14 @@ class Moderation::Users::IndexComponent < ApplicationComponent def initialize(users) @users = users end + + private + + def status(user) + t("admin.activity.show.actions.#{activity_action(user)}") + end + + def activity_action(user) + Activity.where(actionable: user, action: [:hide, :block]).last&.action || "block" + end end diff --git a/config/locales/en/moderation.yml b/config/locales/en/moderation.yml index ff0fb98c6..cdf884c4c 100644 --- a/config/locales/en/moderation.yml +++ b/config/locales/en/moderation.yml @@ -104,7 +104,6 @@ en: block: Block confirm_block: "Are you sure? This will hide the user \"%{name}\" and all their contents." confirm_hide: "Are you sure? This will hide the user \"%{name}\" without hiding their contents." - hidden: Blocked hide: Hide search_placeholder: email or name of user title: Block users diff --git a/config/locales/es/moderation.yml b/config/locales/es/moderation.yml index b5580317a..706b61735 100644 --- a/config/locales/es/moderation.yml +++ b/config/locales/es/moderation.yml @@ -104,7 +104,6 @@ es: block: Bloquear confirm_block: "¿Seguro? Esto ocultará a \"%{name}\" así como todos sus contenidos." confirm_hide: "¿Seguro? Esto ocultará a \"%{name}\" sin ocultar sus contenidos." - hidden: Bloqueado hide: Ocultar search_placeholder: email o nombre de usuario title: Bloquear usuarios diff --git a/spec/components/moderation/users/index_component_spec.rb b/spec/components/moderation/users/index_component_spec.rb index 49648ff7d..da1f8505f 100644 --- a/spec/components/moderation/users/index_component_spec.rb +++ b/spec/components/moderation/users/index_component_spec.rb @@ -13,17 +13,90 @@ describe Moderation::Users::IndexComponent, controller: Moderation::UsersControl expect(table).to have_button "Hide" expect(table).to have_button "Block" expect(table).not_to have_content "Blocked" + expect(table).not_to have_content "Hidden" end end - it "shows 'blocked' for hidden users" do - create(:user, :hidden) + context "for hidden users" do + it "shows 'blocked' when the user has been blocked" do + user = create(:user, :hidden) + Activity.log(nil, :block, user) - render_inline component + render_inline component - page.find("table") do |table| - expect(table).to have_content "Blocked" - expect(table).not_to have_button + page.find("table") do |table| + expect(table).to have_content "Blocked" + expect(table).not_to have_content "Hidden" + expect(table).not_to have_button + end + end + + it "shows 'hidden' when the user has been hidden" do + user = create(:user, :hidden) + Activity.log(nil, :hide, user) + + render_inline component + + page.find("table") do |table| + expect(table).to have_content "Hidden" + expect(table).not_to have_content "Blocked" + expect(table).not_to have_button + end + end + + it "shows 'blocked' when there are no activities to hide the user" do + create(:user, :hidden) + + render_inline component + + page.find("table") do |table| + expect(table).to have_content "Blocked" + end + end + + it "is not affected by activities for other users" do + blocked = create(:user, :hidden, username: "Very bad user") + hidden = create(:user, :hidden, username: "Slightly bad user") + + Activity.log(nil, :block, blocked) + Activity.log(nil, :hide, hidden) + + render_inline component + + page.find("tr", text: "Very bad user") do |row| + expect(row).to have_content "Blocked" + end + + page.find("tr", text: "Slightly bad user") do |row| + expect(row).to have_content "Hidden" + end + end + + it "doesn't consider activities other than block or hide" do + user = create(:user, :hidden) + Activity.log(nil, :block, user) + Activity.log(nil, :restore, user) + + render_inline component + + page.find("table") do |table| + expect(table).to have_content "Blocked" + end + end + + it "shows actions after the user has been restored" do + user = create(:user, :hidden) + Activity.log(nil, :block, user) + user.restore + + render_inline component + + page.find("table") do |table| + expect(table).to have_button "Hide" + expect(table).to have_button "Block" + expect(table).not_to have_content "Blocked" + expect(table).not_to have_content "Hidden" + end end end end diff --git a/spec/system/moderation/users_spec.rb b/spec/system/moderation/users_spec.rb index d9e92e5b4..42247e333 100644 --- a/spec/system/moderation/users_spec.rb +++ b/spec/system/moderation/users_spec.rb @@ -75,4 +75,23 @@ describe "Moderate users" do expect(page).to have_content "Blocked" end end + + scenario "Hide users in the moderation section" do + create(:user, username: "Rick") + + login_as(create(:moderator).user) + visit moderation_users_path(search: "Rick") + + within("#moderation_users") do + accept_confirm('This will hide the user "Rick" without hiding their contents') do + click_button "Hide" + end + end + + expect(page).to have_content "The user has been hidden" + + within("#moderation_users") do + expect(page).to have_content "Hidden" + end + end end