From d777670a8f51eb587a6a3c6dc339ef530fd5c677 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 6 Sep 2024 15:03:51 +0300 Subject: [PATCH] fix[Op#57579]: stick to `select_custom_fields` perm check Removes the admi only guard as it's possible project admins (who are'nt admins) can modify custom field activations at project level. Hence restricting to just admins does not make sense --- .../custom_field_projects/base_contract.rb | 4 +--- config/locales/en.yml | 1 + .../base_contract_spec.rb | 18 +++++++++++++++--- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/app/contracts/custom_fields/custom_field_projects/base_contract.rb b/app/contracts/custom_fields/custom_field_projects/base_contract.rb index e2994ab423c0..d2d260d11119 100644 --- a/app/contracts/custom_fields/custom_field_projects/base_contract.rb +++ b/app/contracts/custom_fields/custom_field_projects/base_contract.rb @@ -29,8 +29,6 @@ module CustomFields module CustomFieldProjects class BaseContract < ::ModelContract - include RequiresAdminGuard - attribute :project_id attribute :custom_field_id @@ -47,7 +45,7 @@ def not_for_all # Only mappings of custom fields which are not enabled for all projects can be manipulated by the user return if model.custom_field.nil? || !model.custom_field.is_for_all? - errors.add :custom_field_id, :cannot_delete_mapping + errors.add :custom_field_id, :is_for_all_cannot_modify end end end diff --git a/config/locales/en.yml b/config/locales/en.yml index 558a0f7da028..97c02d8aeb17 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -884,6 +884,7 @@ en: blank: "can't be blank." blank_nested: "needs to have the property '%{property}' set." cannot_delete_mapping: "is required. Cannot be deleted." + is_for_all_cannot_modify: "is for all. Cannot be modified." cant_link_a_work_package_with_a_descendant: "A work package cannot be linked to one of its subtasks." circular_dependency: "This relation would create a circular dependency." confirmation: "doesn't match %{attribute}." diff --git a/spec/contracts/custom_fields/custom_field_projects/base_contract_spec.rb b/spec/contracts/custom_fields/custom_field_projects/base_contract_spec.rb index a63f7428635d..56d187781635 100644 --- a/spec/contracts/custom_fields/custom_field_projects/base_contract_spec.rb +++ b/spec/contracts/custom_fields/custom_field_projects/base_contract_spec.rb @@ -42,13 +42,25 @@ let(:custom_field) { build_stubbed(:custom_field, is_for_all: true) } let(:custom_field_project) { build_stubbed(:custom_fields_project, custom_field:) } - it_behaves_like "contract is invalid" + it_behaves_like "contract is invalid", custom_field_id: :is_for_all_cannot_modify end - context "with non-admin user" do + context "with authorised user" do let(:user) { build_stubbed(:user) } - it_behaves_like "contract is invalid", base: %i[error_unauthorized error_unauthorized] + before do + mock_permissions_for(user) do |mock| + mock.allow_in_project(:select_custom_fields, project: custom_field_project.project) + end + end + + it_behaves_like "contract is valid" + end + + context "with unauthorised user" do + let(:user) { build_stubbed(:user) } + + it_behaves_like "contract is invalid", base: :error_unauthorized end include_examples "contract reuses the model errors"