From ede1275172bfc96f0faa74976168bf6ddcd08386 Mon Sep 17 00:00:00 2001 From: stephanie rousset Date: Mon, 20 May 2024 14:46:57 +0200 Subject: [PATCH 1/8] fix: deletes the follows of destroyed private user for assembly that is not transparent and its components --- ...estroy_participatory_space_private_user.rb | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/decidim-admin/app/commands/decidim/admin/destroy_participatory_space_private_user.rb b/decidim-admin/app/commands/decidim/admin/destroy_participatory_space_private_user.rb index df0fcaf2a4f26..b4160c977b360 100644 --- a/decidim-admin/app/commands/decidim/admin/destroy_participatory_space_private_user.rb +++ b/decidim-admin/app/commands/decidim/admin/destroy_participatory_space_private_user.rb @@ -13,6 +13,32 @@ def extra_params } } end + + def run_after_hooks + # A hook to destroy the follows of user on private non transparent assembly and its children + # when private user is destroyed + return unless resource.privatable_to_type == "Decidim::Assembly" + + assembly = Decidim::Assembly.find(resource.privatable_to_id) + return unless assembly.private_space == true && assembly.is_transparent == false + + user = Decidim::User.find(resource.decidim_user_id) + ids = [] + ids << Decidim::Follow.where(user:) + .where(decidim_followable_type: "Decidim::Assembly") + .where(decidim_followable_id: assembly.id) + .first.id + children_ids = Decidim::Follow.where(user:) + .select { |follow| find_object(follow).respond_to?("decidim_component_id") } + .select { |follow| assembly.components.ids.include?(find_object(follow).decidim_component_id) } + .map(&:id) + ids << children_ids + Decidim::Follow.where(user:).where(id: ids.flatten).destroy_all if ids.present? + end + + def find_object(follow) + follow.decidim_followable_type.constantize.find(follow.decidim_followable_id) + end end end end From d0cd2120167969bedc00e09ed6d966de474c2707 Mon Sep 17 00:00:00 2001 From: stephanie rousset Date: Mon, 20 May 2024 17:27:57 +0200 Subject: [PATCH 2/8] test: add tests for destroy private user and its follows --- ...y_participatory_space_private_user_spec.rb | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/decidim-admin/spec/commands/decidim/admin/destroy_participatory_space_private_user_spec.rb b/decidim-admin/spec/commands/decidim/admin/destroy_participatory_space_private_user_spec.rb index c91eb5e6c65fd..6eb2ce841dc0b 100644 --- a/decidim-admin/spec/commands/decidim/admin/destroy_participatory_space_private_user_spec.rb +++ b/decidim-admin/spec/commands/decidim/admin/destroy_participatory_space_private_user_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "spec_helper" +require "decidim/meetings/test/factories" module Decidim::Admin describe DestroyParticipatorySpacePrivateUser do @@ -37,5 +38,33 @@ module Decidim::Admin action_log = Decidim::ActionLog.last expect(action_log.version).to be_nil end + + context "when assembly is not transparent" do + let(:normal_user) { create(:user, organization:) } + let(:assembly) { create(:assembly, :private, :opaque, :published, organization: user.organization) } + let!(:participatory_space_private_user) { create(:participatory_space_private_user, user: normal_user, privatable_to: assembly) } + + context "and user follows assembly" do + let!(:follow) { create(:follow, followable: assembly, user: normal_user) } + + it "destroys the follow" do + expect(Decidim::Follow.where(user: normal_user).count).to eq(1) + subject.call + expect(Decidim::Follow.where(user: normal_user).count).to eq(0) + end + + context "and user follows meeting belonging to assembly" do + let(:meetings_component) { create(:component, manifest_name: "meetings", participatory_space: assembly) } + let(:meeting) { create(:meeting, component: meetings_component) } + let!(:second_follow) { create(:follow, followable: meeting, user: normal_user) } + + it "destroys all follows" do + expect(Decidim::Follow.where(user: normal_user).count).to eq(2) + subject.call + expect(Decidim::Follow.where(user: normal_user).count).to eq(0) + end + end + end + end end end From 5ac5c40a93b9677ce4b4dbcfc0a4e411afbd50fc Mon Sep 17 00:00:00 2001 From: stephanie rousset Date: Tue, 21 May 2024 15:51:53 +0200 Subject: [PATCH 3/8] refactor: update run_after_hooks method to handle also participatory_process and update tests --- ...estroy_participatory_space_private_user.rb | 30 +++++++++---------- ...y_participatory_space_private_user_spec.rb | 28 +++++++++++++++++ 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/decidim-admin/app/commands/decidim/admin/destroy_participatory_space_private_user.rb b/decidim-admin/app/commands/decidim/admin/destroy_participatory_space_private_user.rb index b4160c977b360..9b6e72de1199f 100644 --- a/decidim-admin/app/commands/decidim/admin/destroy_participatory_space_private_user.rb +++ b/decidim-admin/app/commands/decidim/admin/destroy_participatory_space_private_user.rb @@ -15,28 +15,28 @@ def extra_params end def run_after_hooks - # A hook to destroy the follows of user on private non transparent assembly and its children - # when private user is destroyed - return unless resource.privatable_to_type == "Decidim::Assembly" + # When private user is destroyed, a hook to destroy the follows of user on private non transparent assembly + # or private participatory process and the follows of their children + space = resource.privatable_to_type.constantize.find(resource.privatable_to_id) - assembly = Decidim::Assembly.find(resource.privatable_to_id) - return unless assembly.private_space == true && assembly.is_transparent == false + return unless space.private_space? + + return if space.respond_to?("is_transparent") && space.is_transparent? user = Decidim::User.find(resource.decidim_user_id) ids = [] - ids << Decidim::Follow.where(user:) - .where(decidim_followable_type: "Decidim::Assembly") - .where(decidim_followable_id: assembly.id) - .first.id - children_ids = Decidim::Follow.where(user:) - .select { |follow| find_object(follow).respond_to?("decidim_component_id") } - .select { |follow| assembly.components.ids.include?(find_object(follow).decidim_component_id) } - .map(&:id) + follows = Decidim::Follow.where(user:) + ids << follows.where(decidim_followable_type: resource.privatable_to_type) + .where(decidim_followable_id: space.id) + .first.id + children_ids = follows.select { |follow| find_object_followed(follow).respond_to?("decidim_component_id") } + .select { |follow| space.components.ids.include?(find_object_followed(follow).decidim_component_id) } + .map(&:id) ids << children_ids - Decidim::Follow.where(user:).where(id: ids.flatten).destroy_all if ids.present? + follows.where(id: ids.flatten).destroy_all if ids.present? end - def find_object(follow) + def find_object_followed(follow) follow.decidim_followable_type.constantize.find(follow.decidim_followable_id) end end diff --git a/decidim-admin/spec/commands/decidim/admin/destroy_participatory_space_private_user_spec.rb b/decidim-admin/spec/commands/decidim/admin/destroy_participatory_space_private_user_spec.rb index 6eb2ce841dc0b..35c0bcd3bcdd2 100644 --- a/decidim-admin/spec/commands/decidim/admin/destroy_participatory_space_private_user_spec.rb +++ b/decidim-admin/spec/commands/decidim/admin/destroy_participatory_space_private_user_spec.rb @@ -66,5 +66,33 @@ module Decidim::Admin end end end + + context "when participatory process is private" do + let(:normal_user) { create(:user, organization:) } + let(:participatory_process) { create(:participatory_process, :private, :published, organization: user.organization) } + let!(:participatory_space_private_user) { create(:participatory_space_private_user, user: normal_user, privatable_to: participatory_process) } + + context "and user follows process" do + let!(:follow) { create(:follow, followable: participatory_process, user: normal_user) } + + it "destroys the follow" do + expect(Decidim::Follow.where(user: normal_user).count).to eq(1) + subject.call + expect(Decidim::Follow.where(user: normal_user).count).to eq(0) + end + + context "and user follows meeting belonging to process" do + let(:meetings_component) { create(:component, manifest_name: "meetings", participatory_space: participatory_process) } + let(:meeting) { create(:meeting, component: meetings_component) } + let!(:second_follow) { create(:follow, followable: meeting, user: normal_user) } + + it "destroys all follows" do + expect(Decidim::Follow.where(user: normal_user).count).to eq(2) + subject.call + expect(Decidim::Follow.where(user: normal_user).count).to eq(0) + end + end + end + end end end From 148a90645eea04bc13237320b1bcd78917e1b669 Mon Sep 17 00:00:00 2001 From: stephanie rousset Date: Wed, 22 May 2024 10:04:54 +0200 Subject: [PATCH 4/8] fix: update code to handle case where there are no follows --- .../admin/destroy_participatory_space_private_user.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/decidim-admin/app/commands/decidim/admin/destroy_participatory_space_private_user.rb b/decidim-admin/app/commands/decidim/admin/destroy_participatory_space_private_user.rb index 9b6e72de1199f..7304270c42b0e 100644 --- a/decidim-admin/app/commands/decidim/admin/destroy_participatory_space_private_user.rb +++ b/decidim-admin/app/commands/decidim/admin/destroy_participatory_space_private_user.rb @@ -28,11 +28,12 @@ def run_after_hooks follows = Decidim::Follow.where(user:) ids << follows.where(decidim_followable_type: resource.privatable_to_type) .where(decidim_followable_id: space.id) - .first.id + &.first&.id children_ids = follows.select { |follow| find_object_followed(follow).respond_to?("decidim_component_id") } .select { |follow| space.components.ids.include?(find_object_followed(follow).decidim_component_id) } - .map(&:id) + &.map(&:id) ids << children_ids + follows.where(id: ids.flatten).destroy_all if ids.present? end From 938615d89bea3de394c33e971ef06d8d19972521 Mon Sep 17 00:00:00 2001 From: stephanie rousset Date: Wed, 22 May 2024 11:37:37 +0200 Subject: [PATCH 5/8] refactor: update run_after_hooks method --- ...estroy_participatory_space_private_user.rb | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/decidim-admin/app/commands/decidim/admin/destroy_participatory_space_private_user.rb b/decidim-admin/app/commands/decidim/admin/destroy_participatory_space_private_user.rb index 7304270c42b0e..617d2a6e2f14d 100644 --- a/decidim-admin/app/commands/decidim/admin/destroy_participatory_space_private_user.rb +++ b/decidim-admin/app/commands/decidim/admin/destroy_participatory_space_private_user.rb @@ -26,17 +26,23 @@ def run_after_hooks user = Decidim::User.find(resource.decidim_user_id) ids = [] follows = Decidim::Follow.where(user:) - ids << follows.where(decidim_followable_type: resource.privatable_to_type) - .where(decidim_followable_id: space.id) - &.first&.id - children_ids = follows.select { |follow| find_object_followed(follow).respond_to?("decidim_component_id") } - .select { |follow| space.components.ids.include?(find_object_followed(follow).decidim_component_id) } - &.map(&:id) - ids << children_ids - + ids << find_space_follow_id(follows, resource, space) + ids << find_children_follows_ids(follows, space) follows.where(id: ids.flatten).destroy_all if ids.present? end + def find_space_follow_id(follows, resource, space) + follows.where(decidim_followable_type: resource.privatable_to_type) + .where(decidim_followable_id: space.id) + &.first&.id + end + + def find_children_follows_ids(follows, space) + follows.select { |follow| find_object_followed(follow).respond_to?("decidim_component_id") } + .select { |follow| space.components.ids.include?(find_object_followed(follow).decidim_component_id) } + &.map(&:id) + end + def find_object_followed(follow) follow.decidim_followable_type.constantize.find(follow.decidim_followable_id) end From 09373ac30795639c4563da7f3248677749b8766a Mon Sep 17 00:00:00 2001 From: stephanie rousset Date: Wed, 22 May 2024 14:56:45 +0200 Subject: [PATCH 6/8] refactor: update test --- ...y_participatory_space_private_user_spec.rb | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/decidim-admin/spec/commands/decidim/admin/destroy_participatory_space_private_user_spec.rb b/decidim-admin/spec/commands/decidim/admin/destroy_participatory_space_private_user_spec.rb index 35c0bcd3bcdd2..4d8924899a8d0 100644 --- a/decidim-admin/spec/commands/decidim/admin/destroy_participatory_space_private_user_spec.rb +++ b/decidim-admin/spec/commands/decidim/admin/destroy_participatory_space_private_user_spec.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "spec_helper" -require "decidim/meetings/test/factories" module Decidim::Admin describe DestroyParticipatorySpacePrivateUser do @@ -55,10 +54,12 @@ module Decidim::Admin context "and user follows meeting belonging to assembly" do let(:meetings_component) { create(:component, manifest_name: "meetings", participatory_space: assembly) } - let(:meeting) { create(:meeting, component: meetings_component) } - let!(:second_follow) { create(:follow, followable: meeting, user: normal_user) } it "destroys all follows" do + meeting = Decidim::Meetings::Meeting.create!(title: generate_localized_title(:meeting_title, skip_injection: false), + description: generate_localized_description(:meeting_description, skip_injection: false), + component: meetings_component, author: user) + create(:follow, followable: meeting, user: normal_user) expect(Decidim::Follow.where(user: normal_user).count).to eq(2) subject.call expect(Decidim::Follow.where(user: normal_user).count).to eq(0) @@ -77,19 +78,23 @@ module Decidim::Admin it "destroys the follow" do expect(Decidim::Follow.where(user: normal_user).count).to eq(1) - subject.call - expect(Decidim::Follow.where(user: normal_user).count).to eq(0) + expect do + subject.call + end.to change(Decidim::Follow, :count).by(-1) end context "and user follows meeting belonging to process" do let(:meetings_component) { create(:component, manifest_name: "meetings", participatory_space: participatory_process) } - let(:meeting) { create(:meeting, component: meetings_component) } - let!(:second_follow) { create(:follow, followable: meeting, user: normal_user) } it "destroys all follows" do + meeting = Decidim::Meetings::Meeting.create!(title: generate_localized_title(:meeting_title, skip_injection: false), + description: generate_localized_description(:meeting_description, skip_injection: false), + component: meetings_component, author: user) + create(:follow, followable: meeting, user: normal_user) expect(Decidim::Follow.where(user: normal_user).count).to eq(2) - subject.call - expect(Decidim::Follow.where(user: normal_user).count).to eq(0) + expect do + subject.call + end.to change(Decidim::Follow, :count).by(-2) end end end From 78b4edc7901791378ff8b5e7a08558801d7ba1f2 Mon Sep 17 00:00:00 2001 From: stephanie rousset Date: Thu, 23 May 2024 11:32:30 +0200 Subject: [PATCH 7/8] refactor: update after second review --- .../admin/destroy_participatory_space_private_user.rb | 11 ++++++----- .../destroy_participatory_space_private_user_spec.rb | 10 ++++++---- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/decidim-admin/app/commands/decidim/admin/destroy_participatory_space_private_user.rb b/decidim-admin/app/commands/decidim/admin/destroy_participatory_space_private_user.rb index 617d2a6e2f14d..b09d5dd524a83 100644 --- a/decidim-admin/app/commands/decidim/admin/destroy_participatory_space_private_user.rb +++ b/decidim-admin/app/commands/decidim/admin/destroy_participatory_space_private_user.rb @@ -23,9 +23,8 @@ def run_after_hooks return if space.respond_to?("is_transparent") && space.is_transparent? - user = Decidim::User.find(resource.decidim_user_id) ids = [] - follows = Decidim::Follow.where(user:) + follows = Decidim::Follow.where(user: resource.decidim_user_id) ids << find_space_follow_id(follows, resource, space) ids << find_children_follows_ids(follows, space) follows.where(id: ids.flatten).destroy_all if ids.present? @@ -38,9 +37,11 @@ def find_space_follow_id(follows, resource, space) end def find_children_follows_ids(follows, space) - follows.select { |follow| find_object_followed(follow).respond_to?("decidim_component_id") } - .select { |follow| space.components.ids.include?(find_object_followed(follow).decidim_component_id) } - &.map(&:id) + follows.map do |follow| + object = find_object_followed(follow) || nil + next unless object.respond_to?("decidim_component_id") + return follow.id if space.components.ids.include?(object.decidim_component_id) + end end def find_object_followed(follow) diff --git a/decidim-admin/spec/commands/decidim/admin/destroy_participatory_space_private_user_spec.rb b/decidim-admin/spec/commands/decidim/admin/destroy_participatory_space_private_user_spec.rb index 4d8924899a8d0..c1d254caa58a7 100644 --- a/decidim-admin/spec/commands/decidim/admin/destroy_participatory_space_private_user_spec.rb +++ b/decidim-admin/spec/commands/decidim/admin/destroy_participatory_space_private_user_spec.rb @@ -48,8 +48,9 @@ module Decidim::Admin it "destroys the follow" do expect(Decidim::Follow.where(user: normal_user).count).to eq(1) - subject.call - expect(Decidim::Follow.where(user: normal_user).count).to eq(0) + expect do + subject.call + end.to change(Decidim::Follow, :count).by(-1) end context "and user follows meeting belonging to assembly" do @@ -61,8 +62,9 @@ module Decidim::Admin component: meetings_component, author: user) create(:follow, followable: meeting, user: normal_user) expect(Decidim::Follow.where(user: normal_user).count).to eq(2) - subject.call - expect(Decidim::Follow.where(user: normal_user).count).to eq(0) + expect do + subject.call + end.to change(Decidim::Follow, :count).by(-2) end end end From 48618cd5cbe81077013c44167917c4703cad1013 Mon Sep 17 00:00:00 2001 From: stephanie rousset Date: Thu, 23 May 2024 16:31:23 +0200 Subject: [PATCH 8/8] refactor: last update --- .../decidim/admin/destroy_participatory_space_private_user.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/decidim-admin/app/commands/decidim/admin/destroy_participatory_space_private_user.rb b/decidim-admin/app/commands/decidim/admin/destroy_participatory_space_private_user.rb index b09d5dd524a83..e246f45149131 100644 --- a/decidim-admin/app/commands/decidim/admin/destroy_participatory_space_private_user.rb +++ b/decidim-admin/app/commands/decidim/admin/destroy_participatory_space_private_user.rb @@ -38,10 +38,10 @@ def find_space_follow_id(follows, resource, space) def find_children_follows_ids(follows, space) follows.map do |follow| - object = find_object_followed(follow) || nil + object = find_object_followed(follow).presence next unless object.respond_to?("decidim_component_id") return follow.id if space.components.ids.include?(object.decidim_component_id) - end + end.compact end def find_object_followed(follow)