Commit 443695e9 authored by Michael Kozono's avatar Michael Kozono

Merge branch 'dblessing_fix_cascading_locked_method' into 'master'

Add ability to include self in cascading setting lock check

See merge request gitlab-org/gitlab!60031
parents adac6df7 10298a1d
...@@ -85,7 +85,7 @@ module CascadingNamespaceSettingAttribute ...@@ -85,7 +85,7 @@ module CascadingNamespaceSettingAttribute
next self[attribute] unless self.class.cascading_settings_feature_enabled? next self[attribute] unless self.class.cascading_settings_feature_enabled?
next self[attribute] if will_save_change_to_attribute?(attribute) next self[attribute] if will_save_change_to_attribute?(attribute)
next locked_value(attribute) if cascading_attribute_locked?(attribute) next locked_value(attribute) if cascading_attribute_locked?(attribute, include_self: false)
next self[attribute] unless self[attribute].nil? next self[attribute] unless self[attribute].nil?
cascaded_value = cascaded_ancestor_value(attribute) cascaded_value = cascaded_ancestor_value(attribute)
...@@ -115,8 +115,8 @@ module CascadingNamespaceSettingAttribute ...@@ -115,8 +115,8 @@ module CascadingNamespaceSettingAttribute
end end
def define_lock_methods(attribute) def define_lock_methods(attribute)
define_method("#{attribute}_locked?") do define_method("#{attribute}_locked?") do |include_self: false|
cascading_attribute_locked?(attribute) cascading_attribute_locked?(attribute, include_self: include_self)
end end
define_method("#{attribute}_locked_by_ancestor?") do define_method("#{attribute}_locked_by_ancestor?") do
...@@ -144,7 +144,7 @@ module CascadingNamespaceSettingAttribute ...@@ -144,7 +144,7 @@ module CascadingNamespaceSettingAttribute
def define_validator_methods(attribute) def define_validator_methods(attribute)
define_method("#{attribute}_changeable?") do define_method("#{attribute}_changeable?") do
return unless cascading_attribute_changed?(attribute) return unless cascading_attribute_changed?(attribute)
return unless cascading_attribute_locked?(attribute) return unless cascading_attribute_locked?(attribute, include_self: false)
errors.add(attribute, s_('CascadingSettings|cannot be changed because it is locked by an ancestor')) errors.add(attribute, s_('CascadingSettings|cannot be changed because it is locked by an ancestor'))
end end
...@@ -152,7 +152,7 @@ module CascadingNamespaceSettingAttribute ...@@ -152,7 +152,7 @@ module CascadingNamespaceSettingAttribute
define_method("lock_#{attribute}_changeable?") do define_method("lock_#{attribute}_changeable?") do
return unless cascading_attribute_changed?("lock_#{attribute}") return unless cascading_attribute_changed?("lock_#{attribute}")
if cascading_attribute_locked?(attribute) if cascading_attribute_locked?(attribute, include_self: false)
return errors.add(:"lock_#{attribute}", s_('CascadingSettings|cannot be changed because it is locked by an ancestor')) return errors.add(:"lock_#{attribute}", s_('CascadingSettings|cannot be changed because it is locked by an ancestor'))
end end
...@@ -213,8 +213,9 @@ module CascadingNamespaceSettingAttribute ...@@ -213,8 +213,9 @@ module CascadingNamespaceSettingAttribute
Gitlab::CurrentSettings.public_send("lock_#{attribute}") # rubocop:disable GitlabSecurity/PublicSend Gitlab::CurrentSettings.public_send("lock_#{attribute}") # rubocop:disable GitlabSecurity/PublicSend
end end
def cascading_attribute_locked?(attribute) def cascading_attribute_locked?(attribute, include_self:)
locked_by_ancestor?(attribute) || locked_by_application_setting?(attribute) locked_by_self = include_self ? public_send("lock_#{attribute}?") : false # rubocop:disable GitlabSecurity/PublicSend
locked_by_self || locked_by_ancestor?(attribute) || locked_by_application_setting?(attribute)
end end
def cascading_attribute_changed?(attribute) def cascading_attribute_changed?(attribute)
......
---
title: Add ability to include self in cascading setting lock check
merge_request: 60031
author:
type: changed
...@@ -82,7 +82,9 @@ cascading_attr :delayed_project_removal ...@@ -82,7 +82,9 @@ cascading_attr :delayed_project_removal
- `delayed_project_removal_locked_by_ancestor?` - `delayed_project_removal_locked_by_ancestor?`
- `delayed_project_removal_locked_by_application_setting?` - `delayed_project_removal_locked_by_application_setting?`
- `delayed_project_removal?` (Boolean attributes only) - `delayed_project_removal?` (Boolean attributes only)
- `delayed_project_removal_locked_ancestor` - (Returns locked namespace settings object [namespace_id]) - `delayed_project_removal_locked_ancestor` (Returns locked namespace settings object [namespace_id])
### Attribute reader method (`delayed_project_removal`)
The attribute reader method (`delayed_project_removal`) returns the correct The attribute reader method (`delayed_project_removal`) returns the correct
cascaded value using the following criteria: cascaded value using the following criteria:
...@@ -94,3 +96,12 @@ cascaded value using the following criteria: ...@@ -94,3 +96,12 @@ cascaded value using the following criteria:
1. Return this namespace's attribute, if not nil. 1. Return this namespace's attribute, if not nil.
1. Return value from nearest ancestor where value is not nil. 1. Return value from nearest ancestor where value is not nil.
1. Return instance-level application setting. 1. Return instance-level application setting.
### `_locked?` method
By default, the `_locked?` method (`delayed_project_removal_locked?`) returns
`true` if an ancestor of the group or application setting locks the attribute.
It returns `false` when called from the group that locked the attribute.
When `include_self: true` is specified, it returns `true` when called from the group that locked the attribute.
This would be relevant, for example, when checking if an attribute is locked from a project.
...@@ -202,6 +202,20 @@ RSpec.describe NamespaceSetting, 'CascadingNamespaceSettingAttribute' do ...@@ -202,6 +202,20 @@ RSpec.describe NamespaceSetting, 'CascadingNamespaceSettingAttribute' do
it_behaves_like 'not locked' it_behaves_like 'not locked'
end end
context 'when attribute is locked by self' do
before do
subgroup_settings.update!(lock_delayed_project_removal: true)
end
it 'is not locked by default' do
expect(subgroup_settings.delayed_project_removal_locked?).to eq(false)
end
it 'is locked when including self' do
expect(subgroup_settings.delayed_project_removal_locked?(include_self: true)).to eq(true)
end
end
context 'when parent does not lock the attribute' do context 'when parent does not lock the attribute' do
it_behaves_like 'not locked' it_behaves_like 'not locked'
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment