Skip to content

Commit

Permalink
fix[Op#57579]: stick to select_custom_fields perm check
Browse files Browse the repository at this point in the history
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
  • Loading branch information
akabiru committed Sep 6, 2024
1 parent b2c6e4e commit 19fdbe3
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@
module CustomFields
module CustomFieldProjects
class BaseContract < ::ModelContract
include RequiresAdminGuard

attribute :project_id
attribute :custom_field_id

Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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}."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit 19fdbe3

Please sign in to comment.