From c6190d0199dc2a3535d5764fe533f84e4c7e77c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sen=C3=A9n=20Rodero=20Rodr=C3=ADguez?= Date: Wed, 4 May 2022 12:53:30 +0200 Subject: [PATCH] Remove roles when block or delete users After a user assigned as a budget admin deletes their account or gets blocked by a moderator, the application throws an exception while loading the admin investment index page. As an erased user is not really deleted and neither its associated roles, the application was failing when trying to sort and administration without a username. In this case, the application was throwing an `ArgumentError: comparison of NilClass with String failed` exception. As a blocked user is not deleted or its roles, the application failed when trying to access the user name through the delegation in the Administrator. In this case, the application was throwing a `NoMethodError: undefined method `name' for nil:NilClass` exception. --- app/models/user.rb | 10 ++++++++++ spec/models/user_spec.rb | 26 ++++++++++++++++++++++++++ spec/system/account_spec.rb | 22 ++++++++++++++++++++++ spec/system/moderation/users_spec.rb | 22 ++++++++++++++++++++++ 4 files changed, 80 insertions(+) diff --git a/app/models/user.rb b/app/models/user.rb index a54f3874a..6933706e4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -230,6 +230,7 @@ class User < ApplicationRecord Proposal.hide_all proposal_ids Budget::Investment.hide_all budget_investment_ids ProposalNotification.hide_all ProposalNotification.where(author_id: id).ids + remove_roles end def full_restore @@ -263,12 +264,21 @@ class User < ApplicationRecord unconfirmed_phone: nil ) identities.destroy_all + remove_roles end def erased? erased_at.present? end + def remove_roles + administrator&.destroy! + valuator&.destroy! + moderator&.destroy! + manager&.destroy! + sdg_manager&.destroy! + end + def take_votes_if_erased_document(document_number, document_type) erased_user = User.erased.find_by(document_number: document_number, document_type: document_type) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index e8532c1f8..947df5e94 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -538,6 +538,19 @@ describe User do expect(Identity.exists?(identity.id)).not_to be end + + it "removes all user roles" do + user = create(:user) + [:administrator, :moderator, :manager, :sdg_manager, :valuator].each do |role| + create(role, user: user) + end + + expect { user.erase }.to change { Administrator.count }.by(-1) + .and change { Moderator.count }.by(-1) + .and change { Manager.count }.by(-1) + .and change { SDG::Manager.count }.by(-1) + .and change { Valuator.count }.by(-1) + end end describe "#take_votes_from" do @@ -749,6 +762,19 @@ describe User do expect(Legislation::Proposal.all).to eq [other_proposal] expect(Legislation::Proposal.with_hidden).to match_array [proposal, other_proposal] end + + it "removes all user roles" do + user = create(:user) + [:administrator, :moderator, :manager, :sdg_manager, :valuator].each do |role| + create(role, user: user) + end + + expect { user.block }.to change { Administrator.count }.by(-1) + .and change { Moderator.count }.by(-1) + .and change { Manager.count }.by(-1) + .and change { SDG::Manager.count }.by(-1) + .and change { Valuator.count }.by(-1) + end end describe "#full_restore" do diff --git a/spec/system/account_spec.rb b/spec/system/account_spec.rb index 31f3a75a7..63334d348 100644 --- a/spec/system/account_spec.rb +++ b/spec/system/account_spec.rb @@ -173,6 +173,28 @@ describe "Account" do expect(page).to have_content "Invalid Email or username or password" end + scenario "Erasing an account removes all related roles" do + user.update!(username: "Admin") + administrators = [create(:administrator, user: user), + create(:administrator, user: create(:user, username: "Other admin"))] + budget = create(:budget, administrators: administrators) + visit admin_budget_budget_investments_path(budget) + + expect(page).to have_select options: ["All administrators", "Admin", "Other admin"] + + visit account_path + click_link "Erase my account" + fill_in "user_erase_reason", with: "I don't want my roles anymore!" + click_button "Erase my account" + + expect(page).to have_content "Goodbye! Your account has been cancelled. We hope to see you again soon." + + login_as(administrators.last.user) + visit admin_budget_budget_investments_path(budget) + + expect(page).to have_select options: ["All administrators", "Other admin"] + end + context "Recommendations" do scenario "are enabled by default" do visit account_path diff --git a/spec/system/moderation/users_spec.rb b/spec/system/moderation/users_spec.rb index 42247e333..cf712de18 100644 --- a/spec/system/moderation/users_spec.rb +++ b/spec/system/moderation/users_spec.rb @@ -94,4 +94,26 @@ describe "Moderate users" do expect(page).to have_content "Hidden" end end + + scenario "Block a user removes all their roles" do + admin = create(:administrator).user + user = create(:user, username: "Budget administrator") + budget = create(:budget, administrators: [create(:administrator, user: user)]) + debate = create(:debate, author: user) + login_as(admin) + visit admin_budget_budget_investments_path(budget) + + expect(page).to have_select options: ["All administrators", "Budget administrator"] + + visit debate_path(debate) + within("#debate_#{debate.id}") do + accept_confirm { click_button "Block author" } + end + + expect(page).to have_current_path(debates_path) + + visit admin_budget_budget_investments_path(budget) + + expect(page).to have_select options: ["All administrators"] + end end