From 5b8625a1609c755307374189d7c20e955f424252 Mon Sep 17 00:00:00 2001
From: Lindsey Shelton <lshelton@gitlab.com>
Date: Mon, 7 Oct 2024 16:33:06 +0000
Subject: [PATCH] Fix for Personal Projects Settings bug
Update namespaces_helper to use project.parent
Update associated specs
Changelog: fixed
---
app/helpers/namespaces_helper.rb | 26 +++---
app/helpers/projects_helper.rb | 2 +-
doc/development/cascading_settings.md | 2 +-
spec/helpers/namespaces_helper_spec.rb | 124 ++++++++++++++++---------
4 files changed, 93 insertions(+), 61 deletions(-)
diff --git a/app/helpers/namespaces_helper.rb b/app/helpers/namespaces_helper.rb
index 90f5a4ede7c8a..44db2fc336937 100644
--- a/app/helpers/namespaces_helper.rb
+++ b/app/helpers/namespaces_helper.rb
@@ -56,26 +56,22 @@ def cascading_namespace_settings_tooltip_raw_data(attribute, group, settings_pat
def project_cascading_namespace_settings_tooltip_data(attribute, project, settings_path_helper)
return unless attribute && project && settings_path_helper
- data = cascading_namespace_settings_tooltip_raw_data(attribute, project.group, settings_path_helper)
- return {} if data.nil?
+ data = cascading_namespace_settings_tooltip_raw_data(attribute, project.parent, settings_path_helper)
+ return data if data[:locked_by_ancestor]
- update_project_data_with_lock_info(data, "#{attribute}_locked?", project)
+ data[:locked_by_application_setting] = check_project_lock(project, "#{attribute}_locked_by_application_setting?")
+ locked_by_ancestor = check_project_lock(project, "#{attribute}_locked_by_ancestor?")
+ locked_by_project = check_project_lock(project, "#{attribute}_locked?")
- Gitlab::Json.dump(data)
- data.to_json
- end
-
- def update_project_data_with_lock_info(data, attribute, project)
- return if data["locked_by_ancestor"]
+ return data unless locked_by_ancestor || locked_by_project
- locked_by_group = check_project_lock(project, attribute)
- return unless locked_by_group
-
- data[:locked_by_ancestor] = locked_by_group
+ data[:locked_by_ancestor] = true
data[:ancestor_namespace] = {
- full_name: project.group.name,
- path: edit_group_path(project.group)
+ full_name: project.parent.name,
+ path: Gitlab::UrlBuilder.build(project.parent, only_path: true)
}
+
+ data
end
def cascading_namespace_setting_locked?(attribute, group, **args)
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index 2b33bf5e9f2c4..03c468a45e1dc 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -922,7 +922,7 @@ def confirm_reduce_visibility_message(project)
def project_permissions_data(project, target_form_id = nil)
data = visibility_confirm_modal_data(project, target_form_id)
- cascading_settings_data = project_cascading_namespace_settings_tooltip_data(:duo_features_enabled, project, method(:edit_group_path))
+ cascading_settings_data = project_cascading_namespace_settings_tooltip_data(:duo_features_enabled, project, method(:edit_group_path)).to_json
data.merge!(
{
cascading_settings_data: cascading_settings_data
diff --git a/doc/development/cascading_settings.md b/doc/development/cascading_settings.md
index fa5c7311596c7..4009785459097 100644
--- a/doc/development/cascading_settings.md
+++ b/doc/development/cascading_settings.md
@@ -248,7 +248,7 @@ initCascadingSettingsLockTooltips();
```ruby
# Example call from your Ruby helper method for projects
-cascading_settings_data = project_cascading_namespace_settings_tooltip_data(:duo_features_enabled, project, method(:edit_group_path))
+cascading_settings_data = project_cascading_namespace_settings_tooltip_data(:duo_features_enabled, project, method(:edit_group_path)).to_json
```
1. From your Vue's `index.js` file, be sure to convert the data into JSON and camel case format. This will make it easier to use in Vue.
diff --git a/spec/helpers/namespaces_helper_spec.rb b/spec/helpers/namespaces_helper_spec.rb
index 96fff67bcd312..04868693587e5 100644
--- a/spec/helpers/namespaces_helper_spec.rb
+++ b/spec/helpers/namespaces_helper_spec.rb
@@ -37,14 +37,20 @@
)
end
+ let!(:project1) { build(:project, namespace: subgroup1) }
+ let!(:project2) do
+ user.create_namespace!(path: user.username, name: user.name) unless user.namespace
+ build(:project, namespace: user.namespace)
+ end
+
+ let_it_be(:attribute) { :math_rendering_limits_enabled }
+
before do
admin_group.add_owner(admin)
user_group.add_owner(user)
end
describe '#check_group_lock' do
- attribute = :math_rendering_limits_enabled
-
context 'when the method exists on namespace_settings' do
it 'calls the method on namespace_settings' do
expect(subgroup1.namespace_settings).to receive(attribute).and_return(true)
@@ -62,8 +68,6 @@
describe '#check_project_lock' do
let(:project) { build(:project, group: subgroup1) }
- attribute = :math_rendering_limits_enabled
-
it 'returns true when the method exists and returns true' do
allow(project.project_setting).to receive(attribute).and_return(true)
expect(helper.check_project_lock(project, attribute)).to be true
@@ -75,8 +79,6 @@
end
describe '#cascading_namespace_settings_tooltip_data' do
- attribute = :math_rendering_limits_enabled
-
it 'returns tooltip data with testid' do
allow(helper).to receive(:cascading_namespace_settings_tooltip_raw_data).and_return({ key: 'value' })
result = helper.cascading_namespace_settings_tooltip_data(attribute, subgroup1, -> {})
@@ -86,8 +88,6 @@
end
describe '#cascading_namespace_settings_tooltip_raw_data' do
- attribute = :math_rendering_limits_enabled
-
subject do
helper.cascading_namespace_settings_tooltip_data(
attribute,
@@ -137,61 +137,97 @@
end
describe '#project_cascading_namespace_settings_tooltip_data' do
- let(:project) { build(:project, group: subgroup1) }
+ using RSpec::Parameterized::TableSyntax
- attribute = :math_rendering_limits_enabled
- settings_path_helper = ->(locked_ancestor) { edit_group_path(locked_ancestor) }
- subject { helper.project_cascading_namespace_settings_tooltip_data(attribute, project, settings_path_helper) }
+ let(:settings_path_helper) { ->(locked_ancestor) { edit_group_path(locked_ancestor) } }
- context 'when not locked by ancestor' do
- before do
- allow(helper).to receive(:cascading_namespace_settings_tooltip_data)
- .and_return(tooltip_data: { locked_by_ancestor: false }.to_json)
+ subject do
+ helper.project_cascading_namespace_settings_tooltip_data(attribute, project, settings_path_helper)
+ end
+
+ shared_examples 'returns correct data' do
+ context 'when attribute is nil' do
+ let(:attribute) { nil }
+
+ it { is_expected.to be nil }
end
- context 'when locked by project group' do
- before do
- allow(project.project_setting).to receive("#{attribute}_locked?").and_return(true)
- end
+ context 'when project is nil' do
+ let(:project) { nil }
- it 'returns JSON with locked_by_ancestor true and ancestor_namespace object' do
- result = Gitlab::Json.parse(subject)
- expect(result['locked_by_ancestor']).to be true
- expect(result['ancestor_namespace']).to eq({
- 'full_name' => project.group.name,
- 'path' => edit_group_path(project.group)
- })
- end
+ it { is_expected.to be nil }
end
- context 'when not locked by project group' do
+ context 'when settings_path_helper is nil' do
+ let(:settings_path_helper) { nil }
+
+ it { is_expected.to be nil }
+ end
+
+ where(:locked_by_ancestor, :locked_by_application_setting, :locked_by_project, :expected_result) do
+ false | false | false | { locked_by_application_setting: false, locked_by_ancestor: false }
+ false | true | false | { locked_by_application_setting: true, locked_by_ancestor: false }
+ true | false | false | { locked_by_application_setting: false, locked_by_ancestor: true, ancestor_namespace: { full_name: String, path: String } }
+ false | false | true | { locked_by_application_setting: false, locked_by_ancestor: true, ancestor_namespace: { full_name: String, path: String } }
+ end
+
+ with_them do
before do
- allow(project.project_setting).to receive("#{attribute}_locked").and_return(false)
+ allow(helper).to receive(:check_project_lock)
+ .with(project, "#{attribute}_locked_by_application_setting?")
+ .and_return(locked_by_application_setting)
+
+ allow(helper).to receive(:check_project_lock)
+ .with(project, "#{attribute}_locked_by_ancestor?")
+ .and_return(locked_by_ancestor)
+
+ allow(helper).to receive(:check_project_lock)
+ .with(project, "#{attribute}_locked?")
+ .and_return(locked_by_project)
+ end
+
+ it 'returns the expected result' do
+ is_expected.to include(expected_result)
+
+ if expected_result[:ancestor_namespace]
+ expect(subject[:ancestor_namespace][:full_name]).to eq(project.parent.name)
+ expect(subject[:ancestor_namespace][:path]).to eq(Gitlab::UrlBuilder.build(project.parent, only_path: true))
+ end
end
- it 'returns JSON without changing locked_by_ancestor' do
- expect(Gitlab::Json.parse(subject)['locked_by_ancestor']).to be false
- expect(Gitlab::Json.parse(subject)).not_to have_key('ancestor_namespace')
+ context 'when data is already locked by ancestor' do
+ before do
+ allow(helper).to receive(:check_group_lock)
+ .with(project.namespace, "#{attribute}_locked_by_ancestor?")
+ .and_return(true)
+ allow(helper).to receive(:check_group_lock)
+ .with(project.namespace, "#{attribute}_locked_by_application_setting?")
+ .and_return(false)
+ end
+
+ it 'returns early without modifying data' do
+ expect(helper).not_to receive(:check_project_lock)
+
+ is_expected.to eq({ locked_by_ancestor: true, locked_by_application_setting: false })
+ end
end
end
end
- context 'when locked by ancestor' do
- before do
- allow(helper).to receive(:cascading_namespace_settings_tooltip_raw_data)
- .and_return({ locked_by_ancestor: true })
- end
+ context 'when project parent is a group' do
+ let(:project) { project1 }
- it 'returns JSON without changing locked_by_ancestor' do
- expect(Gitlab::Json.parse(subject)['locked_by_ancestor']).to be true
- expect(Gitlab::Json.parse(subject)).not_to have_key('ancestor_namespace')
- end
+ it_behaves_like 'returns correct data'
+ end
+
+ context 'when project parent is a user namespace' do
+ let(:project) { project2 }
+
+ it_behaves_like 'returns correct data'
end
end
describe '#cascading_namespace_setting_locked?' do
- let(:attribute) { :math_rendering_limits_enabled }
-
context 'when `group` argument is `nil`' do
it 'returns `false`' do
expect(helper.cascading_namespace_setting_locked?(attribute, nil)).to eq(false)
--
GitLab