From 20031cb447365256206dae5ff395a0986d6536d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20Verg=C3=A9s?= Date: Sat, 27 Jul 2024 16:24:23 +0200 Subject: [PATCH] fix valuators from other spaces being able to comment on closed comentables --- .../cells/decidim/comments/comment_cell.rb | 6 ++- .../cells/decidim/comments/comments_cell.rb | 6 ++- .../forms/decidim/comments/comment_form.rb | 3 +- .../comments/commentable_with_component.rb | 2 +- .../decidim/comments/comment_cell_spec.rb | 42 ++++++++++++++++ .../decidim/comments/comments_cell_spec.rb | 33 ++++++++++++ .../spec/forms/comment_form_spec.rb | 50 +++++++++++++------ .../app/cells/decidim/comments_button_cell.rb | 2 +- .../concerns/decidim/user_role_checker.rb | 32 ++++++++---- .../test/shared_examples/comments_examples.rb | 15 ++++-- 10 files changed, 157 insertions(+), 34 deletions(-) diff --git a/decidim-comments/app/cells/decidim/comments/comment_cell.rb b/decidim-comments/app/cells/decidim/comments/comment_cell.rb index a514ff885822d..4577097807bc0 100644 --- a/decidim-comments/app/cells/decidim/comments/comment_cell.rb +++ b/decidim-comments/app/cells/decidim/comments/comment_cell.rb @@ -96,7 +96,7 @@ def context_menu_id end def can_reply? - return true if user_has_any_role?(current_user) + return true if current_participatory_space && user_has_any_role?(current_user, current_participatory_space) user_signed_in? && accepts_new_comments? && root_commentable.user_allowed_to_comment?(current_user) @@ -210,6 +210,10 @@ def current_component root_commentable.try(:component) end + def current_participatory_space + current_component&.participatory_space + end + def vote_button_to(path, params, &) # actions are linked to objects belonging to a component # To apply :comment permission, the modal authorizer should be refactored to allow participatory spaces-level comments diff --git a/decidim-comments/app/cells/decidim/comments/comments_cell.rb b/decidim-comments/app/cells/decidim/comments/comments_cell.rb index bb4da2d64fad2..8f37e42948897 100644 --- a/decidim-comments/app/cells/decidim/comments/comments_cell.rb +++ b/decidim-comments/app/cells/decidim/comments/comments_cell.rb @@ -40,7 +40,7 @@ def user_comments_blocked_warning private def can_add_comments? - return true if user_has_any_role?(current_user) + return true if current_participatory_space && user_has_any_role?(current_user, current_participatory_space) return if single_comment? return if comments_blocked? return if user_comments_blocked? @@ -146,6 +146,10 @@ def current_component model.try(:component) end + def current_participatory_space + model.try(:participatory_space) + end + def blocked_comments_for_unauthorized_user_warning_link options = if current_component.present? { resource: model } diff --git a/decidim-comments/app/forms/decidim/comments/comment_form.rb b/decidim-comments/app/forms/decidim/comments/comment_form.rb index 642803e91d50f..7b80e7acd3963 100644 --- a/decidim-comments/app/forms/decidim/comments/comment_form.rb +++ b/decidim-comments/app/forms/decidim/comments/comment_form.rb @@ -42,7 +42,8 @@ def max_depth # Private: Check if commentable can have comments and if not adds # a validation error to the model def commentable_can_have_comments - return if user_has_any_role?(current_user) + return unless current_component && current_component.participatory_space + return if user_has_any_role?(current_user, current_component.participatory_space) errors.add(:commentable, :cannot_have_comments) unless commentable.accepts_new_comments? end diff --git a/decidim-comments/lib/decidim/comments/commentable_with_component.rb b/decidim-comments/lib/decidim/comments/commentable_with_component.rb index 90c690ab91756..d3c55e78249e4 100644 --- a/decidim-comments/lib/decidim/comments/commentable_with_component.rb +++ b/decidim-comments/lib/decidim/comments/commentable_with_component.rb @@ -24,7 +24,7 @@ def accepts_new_comments? # Public: Whether the object can have new comments or not. def user_allowed_to_comment?(user) - return true if user_has_any_role?(user) + return true if user_has_any_role?(user, component.participatory_space) return unless can_participate?(user) ActionAuthorizer.new(user, "comment", component, self).authorize.ok? diff --git a/decidim-comments/spec/cells/decidim/comments/comment_cell_spec.rb b/decidim-comments/spec/cells/decidim/comments/comment_cell_spec.rb index 980e979ff690c..ddf97ad3412de 100644 --- a/decidim-comments/spec/cells/decidim/comments/comment_cell_spec.rb +++ b/decidim-comments/spec/cells/decidim/comments/comment_cell_spec.rb @@ -191,6 +191,48 @@ module Decidim::Comments end end end + + context "when coments are blocked" do + before do + allow(commentable).to receive(:user_allowed_to_comment?).and_return(false) + end + + it "does not render the reply form" do + expect(subject).to have_no_css(".add-comment") + end + + context "and the user is an admin" do + let(:current_user) { create(:user, :admin, :confirmed, organization: component.organization) } + + it "renders the reply form" do + expect(subject).to have_css(".add-comment") + end + end + + context "and the user is a user manager" do + let(:current_user) { create(:user, :user_manager, :confirmed, organization: component.organization) } + + it "renders the reply form" do + expect(subject).to have_css(".add-comment") + end + end + + context "and the user is a valuator in the same participatory space" do + let!(:valuator_role) { create(:participatory_process_user_role, user: current_user, participatory_process: component.participatory_space, role: :valuator) } + + it "renders the reply form" do + expect(subject).to have_css(".add-comment") + end + end + + context "and the user is a valuator in another participatory space" do + let!(:valuator_role) { create(:participatory_process_user_role, user: current_user, participatory_process: create(:participatory_process, organization: component.organization), role: :valuator) } + + it "does not render the reply form" do + expect(subject).to have_no_css(".add-comment") + end + end + end end end diff --git a/decidim-comments/spec/cells/decidim/comments/comments_cell_spec.rb b/decidim-comments/spec/cells/decidim/comments/comments_cell_spec.rb index 0aff29bd225c5..bf31d333972f2 100644 --- a/decidim-comments/spec/cells/decidim/comments/comments_cell_spec.rb +++ b/decidim-comments/spec/cells/decidim/comments/comments_cell_spec.rb @@ -107,6 +107,39 @@ module Decidim::Comments it "renders the comments blocked warning" do expect(subject).to have_css(".flash.warning", text: I18n.t("decidim.components.comments.blocked_comments_warning")) expect(subject).to have_no_css(".flash.warning", text: I18n.t("decidim.components.comments.blocked_comments_for_user_warning")) + expect(subject).to have_no_css(".add-comment #new_comment_for_DummyResource_#{commentable.id}") + end + + context "and the user is an admin" do + let(:current_user) { create(:user, :admin, :confirmed, organization: component.organization) } + + it "renders add comment" do + expect(subject).to have_css(".add-comment #new_comment_for_DummyResource_#{commentable.id}") + end + end + + context "and the user is a user manager" do + let(:current_user) { create(:user, :user_manager, :confirmed, organization: component.organization) } + + it "renders add comment" do + expect(subject).to have_css(".add-comment #new_comment_for_DummyResource_#{commentable.id}") + end + end + + context "and the user is a valuator in the same participatory space" do + let!(:valuator_role) { create(:participatory_process_user_role, user: current_user, participatory_process: component.participatory_space, role: :valuator) } + + it "renders add comment" do + expect(subject).to have_css(".add-comment #new_comment_for_DummyResource_#{commentable.id}") + end + end + + context "and the user is a valuator in a different participatory space" do + let!(:valuator_role) { create(:participatory_process_user_role, user: current_user, role: :valuator) } + + it "does not render add comment" do + expect(subject).to have_no_css(".add-comment #new_comment_for_DummyResource_#{commentable.id}") + end end end diff --git a/decidim-comments/spec/forms/comment_form_spec.rb b/decidim-comments/spec/forms/comment_form_spec.rb index acdda212561fb..0f991fe87225b 100644 --- a/decidim-comments/spec/forms/comment_form_spec.rb +++ b/decidim-comments/spec/forms/comment_form_spec.rb @@ -17,9 +17,9 @@ module Comments let(:organization) { create(:organization) } let(:user) { create(:user, :confirmed, organization:) } - let(:admin_user) { create(:user, :admin, :confirmed, organization:) } - let(:user_manager) { create(:user, :user_manager, :confirmed, organization:) } - let!(:component) { create(:component, organization:) } + let!(:component) { create(:component, participatory_space: assembly) } + let(:assembly) { create(:assembly, organization:) } + let(:another_assembly) { create(:assembly, organization:) } let(:body) { "This is a new comment" } let(:alignment) { 1 } let(:user_group) { create(:user_group, :verified) } @@ -115,6 +115,13 @@ module Comments end end + shared_examples "does not allow commenting" do + it "does not allow comments" do + expect(subject.send(:commentable_can_have_comments)).not_to be_nil + expect(subject).not_to be_valid + end + end + describe "#commentable_can_have_comments" do let(:accepts_new_comments) { true } @@ -122,24 +129,35 @@ module Comments allow(commentable).to receive(:accepts_new_comments?).and_return(accepts_new_comments) end - context "when user is admin" do - let(:current_user) { admin_user } + it_behaves_like "allows commenting" - it_behaves_like "allows commenting" - end + context "when no comments are accepted" do + let!(:accepts_new_comments) { false } - context "when user is user manager" do - let(:current_user) { user_manager } + it_behaves_like "does not allow commenting" - it_behaves_like "allows commenting" - end + context "when user is admin" do + let(:user) { create(:user, :admin, :confirmed, organization:) } - context "when user is a normal user" do - let!(:accepts_new_comments) { false } + it_behaves_like "allows commenting" + end + + context "when user is user manager" do + let(:user) { create(:user, :user_manager, :confirmed, organization:) } + + it_behaves_like "allows commenting" + end + + context "when user is moderator in the same participatory space" do + let!(:moderator_role) { create(:assembly_user_role, user:, assembly:, role: :moderator) } + + it_behaves_like "allows commenting" + end + + context "when user is moderator in another participatory space" do + let!(:moderator_role) { create(:assembly_user_role, user:, assembly: another_assembly, role: :moderator) } - it "does not allow comments" do - expect(subject.send(:commentable_can_have_comments)).not_to be_nil - expect(subject).not_to be_valid + it_behaves_like "does not allow commenting" end end end diff --git a/decidim-core/app/cells/decidim/comments_button_cell.rb b/decidim-core/app/cells/decidim/comments_button_cell.rb index fe44f69f0bd68..57e5b5f7a30d3 100644 --- a/decidim-core/app/cells/decidim/comments_button_cell.rb +++ b/decidim-core/app/cells/decidim/comments_button_cell.rb @@ -17,7 +17,7 @@ def show private def comments_enabled? - return true if user_has_any_role?(current_user) + return true if user_has_any_role?(current_user, current_participatory_space) component_settings.comments_enabled? && !current_settings.try(:comments_blocked?) end diff --git a/decidim-core/app/helpers/concerns/decidim/user_role_checker.rb b/decidim-core/app/helpers/concerns/decidim/user_role_checker.rb index f77b1f832ebb3..9eb836bb9d2ce 100644 --- a/decidim-core/app/helpers/concerns/decidim/user_role_checker.rb +++ b/decidim-core/app/helpers/concerns/decidim/user_role_checker.rb @@ -7,33 +7,45 @@ module UserRoleChecker private - def user_has_any_role?(user) + def user_has_any_role?(user, participatory_space = nil) return false unless user return true if user.admin return true if user.roles.any? - return true if participatory_process_user_role?(user) - return true if assembly_user_role?(user) - return true if conference_user_role?(user) + return true if participatory_process_user_role?(user, participatory_space) + return true if assembly_user_role?(user, participatory_space) + return true if conference_user_role?(user, participatory_space) false end - def participatory_process_user_role?(user) + def participatory_process_user_role?(user, participatory_process = nil) return false unless Decidim.module_installed?(:participatory_processes) - true if Decidim::ParticipatoryProcessUserRole.exists?(user:) + if participatory_process.is_a?(Decidim::ParticipatoryProcess) + Decidim::ParticipatoryProcessUserRole.exists?(user:, participatory_process:) + else + Decidim::ParticipatoryProcessUserRole.exists?(user:) + end end - def assembly_user_role?(user) + def assembly_user_role?(user, assembly = nil) return false unless Decidim.module_installed?(:assemblies) - true if Decidim::AssemblyUserRole.exists?(user:) + if assembly.is_a?(Decidim::Assembly) + Decidim::AssemblyUserRole.exists?(user:, assembly:) + else + Decidim::AssemblyUserRole.exists?(user:) + end end - def conference_user_role?(user) + def conference_user_role?(user, conference = nil) return false unless Decidim.module_installed?(:conferences) - true if Decidim::ConferenceUserRole.exists?(user:) + if conference.is_a?(Decidim::Conference) + Decidim::ConferenceUserRole.exists?(user:, conference:) + else + Decidim::ConferenceUserRole.exists?(user:) + end end end end diff --git a/decidim-core/lib/decidim/core/test/shared_examples/comments_examples.rb b/decidim-core/lib/decidim/core/test/shared_examples/comments_examples.rb index 2ee5ae4f35089..b3ce8bb86e134 100644 --- a/decidim-core/lib/decidim/core/test/shared_examples/comments_examples.rb +++ b/decidim-core/lib/decidim/core/test/shared_examples/comments_examples.rb @@ -1075,12 +1075,21 @@ it_behaves_like "can answer comments" end - context "when the user has an evaluator role" do - let!(:participatory_process) { create(:participatory_process, :with_steps, organization:) } - let!(:valuator_role) { create(:participatory_process_user_role, role: :valuator, user:, participatory_process:) } + context "when the user has an evaluator role in the same participatory space" do + let!(:valuator_role) { create(:participatory_process_user_role, role: :valuator, user:, participatory_process: participatory_space) } it_behaves_like "can answer comments" end + + context "when the user has an evaluator role in a different participatory space" do + let!(:valuator_role) { create(:participatory_process_user_role, role: :valuator, user:, participatory_process: create(:participatory_process, organization:)) } + + it "cannot answer" do + visit resource_path + expect(page).to have_content("Comments are currently disabled, only administrators can reply or post new ones.") + expect(page).to have_no_content("You need to be verified to comment at this moment") + end + end end end end