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] 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