From a3dcdc44ac04f0e31439a7d059192a3dd3c38aa4 Mon Sep 17 00:00:00 2001 From: smriti Date: Wed, 30 Apr 2025 23:44:56 +0530 Subject: [PATCH 1/5] DB and controller changes for disable_invite_users Changed default value to false --- ...112641_add_disable_invite_users_setting.rb | 9 ++ db/schema_migrations/20250430112641 | 1 + db/structure.sql | 1 + .../controllers/concerns/ee/groups/params.rb | 5 ++ ee/app/models/ee/group.rb | 3 + .../assign_attributes_service.rb | 5 +- .../namespace_setting_changes_auditor_spec.rb | 2 +- ee/spec/models/ee/group_spec.rb | 5 +- ee/spec/models/namespace_setting_spec.rb | 22 +++++ ee/spec/requests/groups_controller_spec.rb | 84 +++++++++++++++++++ .../assign_attributes_service_spec.rb | 46 ++++++++++ 11 files changed, 180 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20250430112641_add_disable_invite_users_setting.rb create mode 100644 db/schema_migrations/20250430112641 diff --git a/db/migrate/20250430112641_add_disable_invite_users_setting.rb b/db/migrate/20250430112641_add_disable_invite_users_setting.rb new file mode 100644 index 00000000000000..d1fd123ea2e911 --- /dev/null +++ b/db/migrate/20250430112641_add_disable_invite_users_setting.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddDisableInviteUsersSetting < Gitlab::Database::Migration[2.3] + milestone '18.0' + + def change + add_column :namespace_settings, :disable_invite_users, :boolean, null: false, default: false + end +end diff --git a/db/schema_migrations/20250430112641 b/db/schema_migrations/20250430112641 new file mode 100644 index 00000000000000..adae7c850ae510 --- /dev/null +++ b/db/schema_migrations/20250430112641 @@ -0,0 +1 @@ +ed77819b8477efd235ab2c1eb8868922fee96fce0ab91aee94eae9fffb69af9f \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 3a7881ae6d8390..70d2f989c8b9fc 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -17917,6 +17917,7 @@ CREATE TABLE namespace_settings ( job_token_policies_enabled boolean DEFAULT false NOT NULL, security_policies jsonb DEFAULT '{}'::jsonb NOT NULL, duo_nano_features_enabled boolean, + disable_invite_users boolean DEFAULT false NOT NULL, CONSTRAINT check_0ba93c78c7 CHECK ((char_length(default_branch_name) <= 255)), CONSTRAINT check_namespace_settings_security_policies_is_hash CHECK ((jsonb_typeof(security_policies) = 'object'::text)), CONSTRAINT namespace_settings_unique_project_download_limit_alertlist_size CHECK ((cardinality(unique_project_download_limit_alertlist) <= 100)), diff --git a/ee/app/controllers/concerns/ee/groups/params.rb b/ee/app/controllers/concerns/ee/groups/params.rb index 7cfbb98b02ec86..04196212487767 100644 --- a/ee/app/controllers/concerns/ee/groups/params.rb +++ b/ee/app/controllers/concerns/ee/groups/params.rb @@ -89,6 +89,11 @@ def group_params_ee can?(current_user, :admin_group, current_group) params_ee << :require_dpop_for_manage_api_endpoints end + + if !current_group.has_parent? && + can?(current_user, :owner_access, current_group) + params_ee << :disable_invite_users + end end end # rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity, Metrics/AbcSize diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb index c975d82fec231a..7ead2bb126fafb 100644 --- a/ee/app/models/ee/group.rb +++ b/ee/app/models/ee/group.rb @@ -119,6 +119,9 @@ module Group delegate :extended_grat_expiry_webhooks_execute, :extended_grat_expiry_webhooks_execute=, to: :namespace_settings + delegate :disable_invite_users, :disable_invite_users=, to: :namespace_settings + delegate :disable_invite_users?, to: :namespace_settings + # Use +checked_file_template_project+ instead, which implements important # visibility checks private :file_template_project diff --git a/ee/app/services/ee/namespace_settings/assign_attributes_service.rb b/ee/app/services/ee/namespace_settings/assign_attributes_service.rb index fe9c8bf8552d67..b362956fb73539 100644 --- a/ee/app/services/ee/namespace_settings/assign_attributes_service.rb +++ b/ee/app/services/ee/namespace_settings/assign_attributes_service.rb @@ -31,7 +31,10 @@ def execute param_key: :lock_duo_features_enabled, user_policy: :admin_group ) - + validate_settings_param_for_root_group( + param_key: :disable_invite_users, + user_policy: :owner_access + ) super end diff --git a/ee/spec/lib/namespaces/namespace_setting_changes_auditor_spec.rb b/ee/spec/lib/namespaces/namespace_setting_changes_auditor_spec.rb index 73d6fb655a8ecf..58af552894be92 100644 --- a/ee/spec/lib/namespaces/namespace_setting_changes_auditor_spec.rb +++ b/ee/spec/lib/namespaces/namespace_setting_changes_auditor_spec.rb @@ -125,7 +125,7 @@ resource_access_token_notify_inherited lock_resource_access_token_notify_inherited pipeline_variables_default_role extended_grat_expiry_webhooks_execute force_pages_access_control jwt_ci_cd_job_token_enabled jwt_ci_cd_job_token_opted_out require_dpop_for_manage_api_endpoints - job_token_policies_enabled security_policies duo_nano_features_enabled] + disable_invite_users job_token_policies_enabled security_policies duo_nano_features_enabled] columns_to_audit = Namespaces::NamespaceSettingChangesAuditor::EVENT_NAME_PER_COLUMN.keys.map(&:to_s) diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb index 203280cb9a38aa..bb1ef3bb29cc90 100644 --- a/ee/spec/models/ee/group_spec.rb +++ b/ee/spec/models/ee/group_spec.rb @@ -310,9 +310,12 @@ it { is_expected.to delegate_method(:duo_availability=).to(:namespace_settings).with_arguments(:args) } it { is_expected.to delegate_method(:experiment_settings_allowed?).to(:namespace_settings) } it { is_expected.to delegate_method(:user_cap_enabled?).to(:namespace_settings) } - it { is_expected.to delegate_method(:require_dpop_for_manage_api_endpoints?).to(:namespace_settings) } + it { is_expected.to delegate_method(:require_dpop_for_manage_api_endpoints?).to(:namespace_settings).with_arguments(:args) } + it { is_expected.to delegate_method(:disable_invite_users_signup=).to(:namespace_settings) } it { is_expected.to delegate_method(:require_dpop_for_manage_api_endpoints).to(:namespace_settings) } it { is_expected.to delegate_method(:require_dpop_for_manage_api_endpoints=).to(:namespace_settings).with_arguments(:args) } + it { is_expected.to delegate_method(:disable_invite_users=).to(:namespace_settings).with_arguments(:args) } + it { is_expected.to delegate_method(:disable_invite_users?).to(:namespace_settings) } it { is_expected.to delegate_method(:enterprise_users_extensions_marketplace_enabled=).to(:namespace_settings).with_arguments(:args) } end diff --git a/ee/spec/models/namespace_setting_spec.rb b/ee/spec/models/namespace_setting_spec.rb index fffe8c63cb46d4..bc5c21416750e6 100644 --- a/ee/spec/models/namespace_setting_spec.rb +++ b/ee/spec/models/namespace_setting_spec.rb @@ -486,6 +486,28 @@ end end + describe "#disable_invite_users" do + context 'when disable_invite_users = true' do + before do + setting.disable_invite_users = true + end + + it 'returns true' do + expect(setting.disable_invite_users?).to eq(true) + end + end + + context 'when disable_invite_users = false' do + before do + setting.disable_invite_users = false + end + + it 'returns false' do + expect(setting.disable_invite_users?).to eq(false) + end + end + end + describe '#user_cap_enabled?', feature_category: :consumables_cost_management do where(:seat_control, :new_user_signups_cap, :root_namespace, :expectation) do :off | nil | false | false diff --git a/ee/spec/requests/groups_controller_spec.rb b/ee/spec/requests/groups_controller_spec.rb index 6dd53bc867e637..a1bad6c6bf546e 100644 --- a/ee/spec/requests/groups_controller_spec.rb +++ b/ee/spec/requests/groups_controller_spec.rb @@ -363,6 +363,90 @@ end end + context 'when setting disable_invite_users' do + let(:params) { { group: { disable_invite_users: true } } } + + context 'when group is a top level group' do + context 'when user is a group owner' do + before do + group.add_owner(user) + end + + it 'successfully changes the column' do + expect(response).to have_gitlab_http_status(:found) + expect { request }.to change { group.reload.disable_invite_users? } + end + end + + context 'when user is not group owner' do + before do + group.owners.delete(user) + group.add_maintainer(user) + end + + it 'does not change the column and returns not found' do + expect { request }.not_to change { group.reload.disable_invite_users? } + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'when group is not a top level group' do + let(:group) { create(:group, :nested) } + + it 'does not change the column and returns not found' do + expect(group.disable_invite_users?).to be(false) + + request + + expect(response).to have_gitlab_http_status(:found) + expect(group.reload.disable_invite_users?).to be(false) + end + end + end + + context 'when setting disable_invite_users' do + let(:params) { { group: { disable_invite_users: true } } } + + context 'when group is a top level group' do + context 'when user is a group owner' do + before do + group.add_owner(user) + end + + it 'successfully changes the column' do + expect(response).to have_gitlab_http_status(:found) + expect { request }.to change { group.reload.disable_invite_users? } + end + end + + context 'when user is not group owner' do + before do + group.owners.delete(user) + group.add_maintainer(user) + end + + it 'does not change the column and returns not found' do + expect { request }.not_to change { group.reload.disable_invite_users? } + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'when group is not a top level group' do + let(:group) { create(:group, :nested) } + + it 'does not change the column and returns not found' do + expect(group.disable_invite_users?).to be(false) + + request + + expect(response).to have_gitlab_http_status(:found) + expect(group.reload.disable_invite_users?).to be(false) + end + end + end + context 'when setting the require_dpop_for_manage_api_endpoints param (default disabled)' do let(:params) { { group: { require_dpop_for_manage_api_endpoints: true } } } diff --git a/ee/spec/services/ee/namespace_settings/assign_attributes_service_spec.rb b/ee/spec/services/ee/namespace_settings/assign_attributes_service_spec.rb index dde2bacdd9fe50..b57b85eddccad7 100644 --- a/ee/spec/services/ee/namespace_settings/assign_attributes_service_spec.rb +++ b/ee/spec/services/ee/namespace_settings/assign_attributes_service_spec.rb @@ -9,6 +9,52 @@ subject(:update_settings) { NamespaceSettings::AssignAttributesService.new(user, group, params).execute } describe '#execute' do + context 'when disable_invite_users param present' do + let(:params) { { disable_invite_users: true } } + + context 'as a non-owner' do + it 'does not change settings' do + expect { update_settings } + .not_to(change { group.reload.disable_invite_users? }) + end + end + + context 'as a group owner' do + before_all do + group.add_owner(user) + end + + it 'changes settings' do + update_settings + + expect(group.disable_invite_users?).to eq(true) + end + end + + context 'as a non-group owner' do + before_all do + group.add_maintainer(user) + end + + it "does not change settings" do + expect { update_settings } + .not_to(change { group.namespace_settings.reload.disable_invite_users? }) + end + end + + context 'when not top-level group' do + before do + group.parent = create(:group) + group.save! + end + + it 'does not change settings' do + expect { update_settings } + .not_to(change { group.namespace_settings.reload.disable_invite_users? }) + end + end + end + context 'when prevent_forking_outside_group param present' do let(:params) { { prevent_forking_outside_group: true } } -- GitLab From b4574f110eeacd03d2bffbacb4c3456734bf38b1 Mon Sep 17 00:00:00 2001 From: smriti Date: Thu, 1 May 2025 11:32:30 +0530 Subject: [PATCH 2/5] Resolved conflict Resolved conflict --- .../controllers/concerns/ee/groups/params.rb | 2 +- ee/spec/requests/groups_controller_spec.rb | 42 ------------------- 2 files changed, 1 insertion(+), 43 deletions(-) diff --git a/ee/app/controllers/concerns/ee/groups/params.rb b/ee/app/controllers/concerns/ee/groups/params.rb index 04196212487767..a88c4e6cf50492 100644 --- a/ee/app/controllers/concerns/ee/groups/params.rb +++ b/ee/app/controllers/concerns/ee/groups/params.rb @@ -90,7 +90,7 @@ def group_params_ee params_ee << :require_dpop_for_manage_api_endpoints end - if !current_group.has_parent? && + if !current_group&.has_parent? && can?(current_user, :owner_access, current_group) params_ee << :disable_invite_users end diff --git a/ee/spec/requests/groups_controller_spec.rb b/ee/spec/requests/groups_controller_spec.rb index a1bad6c6bf546e..c99ea619caba0d 100644 --- a/ee/spec/requests/groups_controller_spec.rb +++ b/ee/spec/requests/groups_controller_spec.rb @@ -373,50 +373,8 @@ end it 'successfully changes the column' do - expect(response).to have_gitlab_http_status(:found) expect { request }.to change { group.reload.disable_invite_users? } - end - end - - context 'when user is not group owner' do - before do - group.owners.delete(user) - group.add_maintainer(user) - end - - it 'does not change the column and returns not found' do - expect { request }.not_to change { group.reload.disable_invite_users? } - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - - context 'when group is not a top level group' do - let(:group) { create(:group, :nested) } - - it 'does not change the column and returns not found' do - expect(group.disable_invite_users?).to be(false) - - request - - expect(response).to have_gitlab_http_status(:found) - expect(group.reload.disable_invite_users?).to be(false) - end - end - end - - context 'when setting disable_invite_users' do - let(:params) { { group: { disable_invite_users: true } } } - - context 'when group is a top level group' do - context 'when user is a group owner' do - before do - group.add_owner(user) - end - - it 'successfully changes the column' do expect(response).to have_gitlab_http_status(:found) - expect { request }.to change { group.reload.disable_invite_users? } end end -- GitLab From a51ef686113547bd94ac0518e527bb2da64bb5ff Mon Sep 17 00:00:00 2001 From: smriti Date: Thu, 1 May 2025 15:41:50 +0530 Subject: [PATCH 3/5] Added new DB column to disable inviting users to group This is a new setting added to namespace table to disable user invite Changelog: added --- ee/spec/models/ee/group_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb index bb1ef3bb29cc90..ce2c8087acf32a 100644 --- a/ee/spec/models/ee/group_spec.rb +++ b/ee/spec/models/ee/group_spec.rb @@ -310,8 +310,7 @@ it { is_expected.to delegate_method(:duo_availability=).to(:namespace_settings).with_arguments(:args) } it { is_expected.to delegate_method(:experiment_settings_allowed?).to(:namespace_settings) } it { is_expected.to delegate_method(:user_cap_enabled?).to(:namespace_settings) } - it { is_expected.to delegate_method(:require_dpop_for_manage_api_endpoints?).to(:namespace_settings).with_arguments(:args) } - it { is_expected.to delegate_method(:disable_invite_users_signup=).to(:namespace_settings) } + it { is_expected.to delegate_method(:require_dpop_for_manage_api_endpoints?).to(:namespace_settings) } it { is_expected.to delegate_method(:require_dpop_for_manage_api_endpoints).to(:namespace_settings) } it { is_expected.to delegate_method(:require_dpop_for_manage_api_endpoints=).to(:namespace_settings).with_arguments(:args) } it { is_expected.to delegate_method(:disable_invite_users=).to(:namespace_settings).with_arguments(:args) } -- GitLab From 32c4ffab752b904ca7ecd4738ec05c6a707e5265 Mon Sep 17 00:00:00 2001 From: smriti Date: Fri, 2 May 2025 13:49:58 +0530 Subject: [PATCH 4/5] Renamed the attribute to disable_invite_members --- ...12641_add_disable_invite_members_setting.rb | 9 +++++++++ ...0112641_add_disable_invite_users_setting.rb | 9 --------- db/structure.sql | 2 +- .../controllers/concerns/ee/groups/params.rb | 2 +- ee/app/models/ee/group.rb | 4 ++-- .../assign_attributes_service.rb | 2 +- .../namespace_setting_changes_auditor_spec.rb | 2 +- ee/spec/models/ee/group_spec.rb | 4 ++-- ee/spec/models/namespace_setting_spec.rb | 14 +++++++------- ee/spec/requests/groups_controller_spec.rb | 12 ++++++------ .../assign_attributes_service_spec.rb | 18 +++++++++++------- 11 files changed, 41 insertions(+), 37 deletions(-) create mode 100644 db/migrate/20250430112641_add_disable_invite_members_setting.rb delete mode 100644 db/migrate/20250430112641_add_disable_invite_users_setting.rb diff --git a/db/migrate/20250430112641_add_disable_invite_members_setting.rb b/db/migrate/20250430112641_add_disable_invite_members_setting.rb new file mode 100644 index 00000000000000..c9815e3824a811 --- /dev/null +++ b/db/migrate/20250430112641_add_disable_invite_members_setting.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddDisableInviteMembersSetting < Gitlab::Database::Migration[2.3] + milestone '18.0' + + def change + add_column :namespace_settings, :disable_invite_members, :boolean, null: false, default: false + end +end diff --git a/db/migrate/20250430112641_add_disable_invite_users_setting.rb b/db/migrate/20250430112641_add_disable_invite_users_setting.rb deleted file mode 100644 index d1fd123ea2e911..00000000000000 --- a/db/migrate/20250430112641_add_disable_invite_users_setting.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -class AddDisableInviteUsersSetting < Gitlab::Database::Migration[2.3] - milestone '18.0' - - def change - add_column :namespace_settings, :disable_invite_users, :boolean, null: false, default: false - end -end diff --git a/db/structure.sql b/db/structure.sql index 70d2f989c8b9fc..8acb0c9d1985e6 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -17917,7 +17917,7 @@ CREATE TABLE namespace_settings ( job_token_policies_enabled boolean DEFAULT false NOT NULL, security_policies jsonb DEFAULT '{}'::jsonb NOT NULL, duo_nano_features_enabled boolean, - disable_invite_users boolean DEFAULT false NOT NULL, + disable_invite_members boolean DEFAULT false NOT NULL, CONSTRAINT check_0ba93c78c7 CHECK ((char_length(default_branch_name) <= 255)), CONSTRAINT check_namespace_settings_security_policies_is_hash CHECK ((jsonb_typeof(security_policies) = 'object'::text)), CONSTRAINT namespace_settings_unique_project_download_limit_alertlist_size CHECK ((cardinality(unique_project_download_limit_alertlist) <= 100)), diff --git a/ee/app/controllers/concerns/ee/groups/params.rb b/ee/app/controllers/concerns/ee/groups/params.rb index a88c4e6cf50492..b64fd221d62999 100644 --- a/ee/app/controllers/concerns/ee/groups/params.rb +++ b/ee/app/controllers/concerns/ee/groups/params.rb @@ -92,7 +92,7 @@ def group_params_ee if !current_group&.has_parent? && can?(current_user, :owner_access, current_group) - params_ee << :disable_invite_users + params_ee << :disable_invite_members end end end diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb index 7ead2bb126fafb..0d56630537e6f4 100644 --- a/ee/app/models/ee/group.rb +++ b/ee/app/models/ee/group.rb @@ -119,8 +119,8 @@ module Group delegate :extended_grat_expiry_webhooks_execute, :extended_grat_expiry_webhooks_execute=, to: :namespace_settings - delegate :disable_invite_users, :disable_invite_users=, to: :namespace_settings - delegate :disable_invite_users?, to: :namespace_settings + delegate :disable_invite_members, :disable_invite_members=, to: :namespace_settings + delegate :disable_invite_members?, to: :namespace_settings # Use +checked_file_template_project+ instead, which implements important # visibility checks diff --git a/ee/app/services/ee/namespace_settings/assign_attributes_service.rb b/ee/app/services/ee/namespace_settings/assign_attributes_service.rb index b362956fb73539..5c4420753c0d0b 100644 --- a/ee/app/services/ee/namespace_settings/assign_attributes_service.rb +++ b/ee/app/services/ee/namespace_settings/assign_attributes_service.rb @@ -32,7 +32,7 @@ def execute user_policy: :admin_group ) validate_settings_param_for_root_group( - param_key: :disable_invite_users, + param_key: :disable_invite_members, user_policy: :owner_access ) super diff --git a/ee/spec/lib/namespaces/namespace_setting_changes_auditor_spec.rb b/ee/spec/lib/namespaces/namespace_setting_changes_auditor_spec.rb index 58af552894be92..118526a41c3eb3 100644 --- a/ee/spec/lib/namespaces/namespace_setting_changes_auditor_spec.rb +++ b/ee/spec/lib/namespaces/namespace_setting_changes_auditor_spec.rb @@ -125,7 +125,7 @@ resource_access_token_notify_inherited lock_resource_access_token_notify_inherited pipeline_variables_default_role extended_grat_expiry_webhooks_execute force_pages_access_control jwt_ci_cd_job_token_enabled jwt_ci_cd_job_token_opted_out require_dpop_for_manage_api_endpoints - disable_invite_users job_token_policies_enabled security_policies duo_nano_features_enabled] + disable_invite_members job_token_policies_enabled security_policies duo_nano_features_enabled] columns_to_audit = Namespaces::NamespaceSettingChangesAuditor::EVENT_NAME_PER_COLUMN.keys.map(&:to_s) diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb index ce2c8087acf32a..5fb1994ced378f 100644 --- a/ee/spec/models/ee/group_spec.rb +++ b/ee/spec/models/ee/group_spec.rb @@ -313,8 +313,8 @@ it { is_expected.to delegate_method(:require_dpop_for_manage_api_endpoints?).to(:namespace_settings) } it { is_expected.to delegate_method(:require_dpop_for_manage_api_endpoints).to(:namespace_settings) } it { is_expected.to delegate_method(:require_dpop_for_manage_api_endpoints=).to(:namespace_settings).with_arguments(:args) } - it { is_expected.to delegate_method(:disable_invite_users=).to(:namespace_settings).with_arguments(:args) } - it { is_expected.to delegate_method(:disable_invite_users?).to(:namespace_settings) } + it { is_expected.to delegate_method(:disable_invite_members=).to(:namespace_settings).with_arguments(:args) } + it { is_expected.to delegate_method(:disable_invite_members?).to(:namespace_settings) } it { is_expected.to delegate_method(:enterprise_users_extensions_marketplace_enabled=).to(:namespace_settings).with_arguments(:args) } end diff --git a/ee/spec/models/namespace_setting_spec.rb b/ee/spec/models/namespace_setting_spec.rb index bc5c21416750e6..4d67ba1cf97f96 100644 --- a/ee/spec/models/namespace_setting_spec.rb +++ b/ee/spec/models/namespace_setting_spec.rb @@ -486,24 +486,24 @@ end end - describe "#disable_invite_users" do - context 'when disable_invite_users = true' do + describe "#disable_invite_members" do + context 'when disable_invite_members = true' do before do - setting.disable_invite_users = true + setting.disable_invite_members = true end it 'returns true' do - expect(setting.disable_invite_users?).to eq(true) + expect(setting.disable_invite_members?).to eq(true) end end - context 'when disable_invite_users = false' do + context 'when disable_invite_members = false' do before do - setting.disable_invite_users = false + setting.disable_invite_members = false end it 'returns false' do - expect(setting.disable_invite_users?).to eq(false) + expect(setting.disable_invite_members?).to eq(false) end end end diff --git a/ee/spec/requests/groups_controller_spec.rb b/ee/spec/requests/groups_controller_spec.rb index c99ea619caba0d..0c3406cc6f3d1d 100644 --- a/ee/spec/requests/groups_controller_spec.rb +++ b/ee/spec/requests/groups_controller_spec.rb @@ -363,8 +363,8 @@ end end - context 'when setting disable_invite_users' do - let(:params) { { group: { disable_invite_users: true } } } + context 'when setting disable_invite_members' do + let(:params) { { group: { disable_invite_members: true } } } context 'when group is a top level group' do context 'when user is a group owner' do @@ -373,7 +373,7 @@ end it 'successfully changes the column' do - expect { request }.to change { group.reload.disable_invite_users? } + expect { request }.to change { group.reload.disable_invite_members? } expect(response).to have_gitlab_http_status(:found) end end @@ -385,7 +385,7 @@ end it 'does not change the column and returns not found' do - expect { request }.not_to change { group.reload.disable_invite_users? } + expect { request }.not_to change { group.reload.disable_invite_members? } expect(response).to have_gitlab_http_status(:not_found) end end @@ -395,12 +395,12 @@ let(:group) { create(:group, :nested) } it 'does not change the column and returns not found' do - expect(group.disable_invite_users?).to be(false) + expect(group.disable_invite_members?).to be(false) request expect(response).to have_gitlab_http_status(:found) - expect(group.reload.disable_invite_users?).to be(false) + expect(group.reload.disable_invite_members?).to be(false) end end end diff --git a/ee/spec/services/ee/namespace_settings/assign_attributes_service_spec.rb b/ee/spec/services/ee/namespace_settings/assign_attributes_service_spec.rb index b57b85eddccad7..ce4c420f6f2abf 100644 --- a/ee/spec/services/ee/namespace_settings/assign_attributes_service_spec.rb +++ b/ee/spec/services/ee/namespace_settings/assign_attributes_service_spec.rb @@ -9,13 +9,17 @@ subject(:update_settings) { NamespaceSettings::AssignAttributesService.new(user, group, params).execute } describe '#execute' do - context 'when disable_invite_users param present' do - let(:params) { { disable_invite_users: true } } + context 'when disable_invite_members param present' do + let(:params) { { disable_invite_members: true } } context 'as a non-owner' do it 'does not change settings' do - expect { update_settings } - .not_to(change { group.reload.disable_invite_users? }) + group.update!(disable_invite_members: true) + group.save! + + update_settings + + expect(group.disable_invite_members?).to eq(true) end end @@ -27,7 +31,7 @@ it 'changes settings' do update_settings - expect(group.disable_invite_users?).to eq(true) + expect(group.disable_invite_members?).to eq(true) end end @@ -38,7 +42,7 @@ it "does not change settings" do expect { update_settings } - .not_to(change { group.namespace_settings.reload.disable_invite_users? }) + .not_to(change { group.disable_invite_members? }) end end @@ -50,7 +54,7 @@ it 'does not change settings' do expect { update_settings } - .not_to(change { group.namespace_settings.reload.disable_invite_users? }) + .not_to(change { group.disable_invite_members? }) end end end -- GitLab From f8d15c929bab70f6157beeaa0299a10aa88e3052 Mon Sep 17 00:00:00 2001 From: Smriti Garg Date: Fri, 2 May 2025 16:21:21 +0000 Subject: [PATCH 5/5] Readability suggestion --- ee/app/controllers/concerns/ee/groups/params.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/app/controllers/concerns/ee/groups/params.rb b/ee/app/controllers/concerns/ee/groups/params.rb index b64fd221d62999..e7a6be9a7d405c 100644 --- a/ee/app/controllers/concerns/ee/groups/params.rb +++ b/ee/app/controllers/concerns/ee/groups/params.rb @@ -90,7 +90,7 @@ def group_params_ee params_ee << :require_dpop_for_manage_api_endpoints end - if !current_group&.has_parent? && + if current_group&.root? && can?(current_user, :owner_access, current_group) params_ee << :disable_invite_members end -- GitLab