diff --git a/app/views/groups/settings/_permissions.html.haml b/app/views/groups/settings/_permissions.html.haml index 8d459b7f3d3e3344656d581966b81991dbdf48bd..25b0b34b32ef6804dda9b2ec08a9d7cd9cc5b139 100644 --- a/app/views/groups/settings/_permissions.html.haml +++ b/app/views/groups/settings/_permissions.html.haml @@ -32,7 +32,6 @@ = render_if_exists 'groups/settings/wiki', f: f, group: @group = render 'groups/settings/lfs', f: f = render_if_exists 'groups/settings/auto_assign_duo_pro', f: f, group: @group - = render_if_exists 'groups/settings/duo_features_enabled', f: f, group: @group = render_if_exists 'groups/settings/product_analytics_settings', f: f, group: @group = render 'groups/settings/git_access_protocols', f: f, group: @group = render 'groups/settings/project_creation_level', f: f, group: @group diff --git a/db/post_migrate/20240926061034_create_temporary_index_for_duo_features.rb b/db/post_migrate/20240926061034_create_temporary_index_for_duo_features.rb new file mode 100644 index 0000000000000000000000000000000000000000..75c17479e9f5d2161ade1a52edde2f7640e1280b --- /dev/null +++ b/db/post_migrate/20240926061034_create_temporary_index_for_duo_features.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class CreateTemporaryIndexForDuoFeatures < Gitlab::Database::Migration[2.2] + disable_ddl_transaction! + + milestone '17.5' + + TABLE_NAME = :namespace_settings + INDEX_COLUMNS = %i[duo_features_enabled lock_duo_features_enabled] + INDEX_NAME = :tmp_duo_features_enabled_index + + def up + return if index_exists?(TABLE_NAME, INDEX_COLUMNS, name: INDEX_NAME) + + add_concurrent_index(TABLE_NAME, INDEX_COLUMNS, name: INDEX_NAME) + end + + def down + return unless index_exists?(TABLE_NAME, INDEX_COLUMNS, name: INDEX_NAME) + + remove_concurrent_index_by_name(TABLE_NAME, INDEX_NAME) + end +end diff --git a/db/post_migrate/20240926061349_lock_duo_features_enabled_update.rb b/db/post_migrate/20240926061349_lock_duo_features_enabled_update.rb new file mode 100644 index 0000000000000000000000000000000000000000..b12a3a3a20a2c5df6b80450917d0a21c97b8929c --- /dev/null +++ b/db/post_migrate/20240926061349_lock_duo_features_enabled_update.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class LockDuoFeaturesEnabledUpdate < Gitlab::Database::Migration[2.2] + milestone '17.5' + + restrict_gitlab_migration gitlab_schema: :gitlab_main + + TABLE_NAME = :namespace_settings + + def up + execute <<-SQL.squish + UPDATE #{TABLE_NAME} + SET lock_duo_features_enabled = false + WHERE duo_features_enabled = true AND lock_duo_features_enabled = true + SQL + end + + def down + # no-op. We can't update all records `where(duo_features_enabled: true, lock_duo_features_enabled: false)` + # because some of them may have been pre-existing. + end +end diff --git a/db/post_migrate/20240926061454_remove_temporary_index_for_duo_features.rb b/db/post_migrate/20240926061454_remove_temporary_index_for_duo_features.rb new file mode 100644 index 0000000000000000000000000000000000000000..f36ad230cd13c99690f22497df332bfbb08b9f28 --- /dev/null +++ b/db/post_migrate/20240926061454_remove_temporary_index_for_duo_features.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class RemoveTemporaryIndexForDuoFeatures < Gitlab::Database::Migration[2.2] + disable_ddl_transaction! + + milestone '17.5' + + TABLE_NAME = :namespace_settings + INDEX_COLUMNS = %i[duo_features_enabled lock_duo_features_enabled] + INDEX_NAME = :tmp_duo_features_enabled_index + + def up + return unless index_exists?(TABLE_NAME, INDEX_COLUMNS, name: INDEX_NAME) + + remove_concurrent_index_by_name(TABLE_NAME, INDEX_NAME) + end + + def down + return if index_exists?(TABLE_NAME, INDEX_COLUMNS, name: INDEX_NAME) + + add_concurrent_index(TABLE_NAME, INDEX_COLUMNS, name: INDEX_NAME) + end +end diff --git a/db/schema_migrations/20240926061034 b/db/schema_migrations/20240926061034 new file mode 100644 index 0000000000000000000000000000000000000000..94c5fd14695a2d8612d0e7ace4dc4d89069f3e44 --- /dev/null +++ b/db/schema_migrations/20240926061034 @@ -0,0 +1 @@ +e629def9d54ca6bd2fd8e21cef8371c62fe1e716a49b8010b766e5e347f5ffac \ No newline at end of file diff --git a/db/schema_migrations/20240926061349 b/db/schema_migrations/20240926061349 new file mode 100644 index 0000000000000000000000000000000000000000..ce95f28c3c6a91558d2a8f278c69c756e0087ff6 --- /dev/null +++ b/db/schema_migrations/20240926061349 @@ -0,0 +1 @@ +18b807fd2b52b21795fb86beef1969078614f0394e8cbf060f62c24591e43e96 \ No newline at end of file diff --git a/db/schema_migrations/20240926061454 b/db/schema_migrations/20240926061454 new file mode 100644 index 0000000000000000000000000000000000000000..4511fc3eae6de22c0c081bebd78a50b68b42a9c2 --- /dev/null +++ b/db/schema_migrations/20240926061454 @@ -0,0 +1 @@ +e0ac7dea16f5d54621d36ba2c6ff7e668b5825fa944845338a76c19f913dcbc2 \ No newline at end of file diff --git a/ee/app/views/groups/settings/_duo_features_enabled.html.haml b/ee/app/views/groups/settings/_duo_features_enabled.html.haml deleted file mode 100644 index e59c1f985e9448dfbe8614f0fd24e3b790f84b1e..0000000000000000000000000000000000000000 --- a/ee/app/views/groups/settings/_duo_features_enabled.html.haml +++ /dev/null @@ -1,24 +0,0 @@ -- return unless @group.licensed_ai_features_available? - -- link = link_to('', help_page_path('user/ai_features.md'), target: '_blank', rel: 'noopener noreferrer') -- duo_features_enabled_locked = cascading_namespace_setting_locked?(:duo_features_enabled, @group) - -%h5 - = _('GitLab Duo features') - -.form-group.gl-mb-3 - = render 'shared/namespaces/cascading_settings/setting_checkbox', attribute: :duo_features_enabled, - checked: @group.namespace_settings.duo_features_enabled, - group: @group, - form: f, - setting_locked: duo_features_enabled_locked, - settings_path_helper: ->(locked_ancestor) { edit_group_path(locked_ancestor, anchor: 'js-permissions-settings') }, - help_text: safe_format(s_('GroupSettings|Enable GitLab Duo features for this group. %{link_start}Learn more%{link_end}.'), tag_pair(link, :link_start, :link_end)) do - %div - = s_('GroupSettings|Use GitLab Duo features') - .gl-ml-6.gl-mt-2 - = render 'shared/namespaces/cascading_settings/enforcement_checkbox', - attribute: :duo_features_enabled, - group: @group, - form: f, - setting_locked: duo_features_enabled_locked diff --git a/ee/spec/views/groups/settings/_permissions.html.haml_spec.rb b/ee/spec/views/groups/settings/_permissions.html.haml_spec.rb index 929187551e3f00499db429fb7ad7e7b09f01aa3f..bb444dc6b91e6fa418b209282d761ba31e9788c3 100644 --- a/ee/spec/views/groups/settings/_permissions.html.haml_spec.rb +++ b/ee/spec/views/groups/settings/_permissions.html.haml_spec.rb @@ -11,34 +11,6 @@ allow(view).to receive(:current_user).and_return(build(:user)) end - context 'for duo features enabled' do - before do - allow(group).to receive(:licensed_ai_features_available?).and_call_original - end - - context 'when licensed ai features is not available' do - it 'renders nothing' do - allow(group).to receive(:licensed_ai_features_available?).and_return(false) - - render - - expect(rendered).to render_template('groups/settings/_duo_features_enabled') - expect(rendered).not_to have_content('Use GitLab Duo features') - end - end - - context 'when licensed ai features are available and feature flag is available' do - it 'renders the experiment settings' do - allow(group).to receive(:licensed_ai_features_available?).and_return(true) - - render - - expect(rendered).to render_template('groups/settings/_duo_features_enabled') - expect(rendered).to have_content('Use GitLab Duo features') - end - end - end - context 'for auto assign duo pro seats' do context 'when on SM' do before do diff --git a/locale/gitlab.pot b/locale/gitlab.pot index b403460b6a770452ce23b8e90bd4a1eb29b125f8..c2f7112db0479f9bdb5cb056f3f718e8a50c17e7 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -26572,9 +26572,6 @@ msgstr "" msgid "GroupSettings|Emails are not encrypted. Concerned administrators may want to disable diff previews." msgstr "" -msgid "GroupSettings|Enable GitLab Duo features for this group. %{link_start}Learn more%{link_end}." -msgstr "" - msgid "GroupSettings|Enable caching of hierarchical objects (subgroups and projects) to improve the performance of group-level features within a large group." msgstr "" @@ -26710,9 +26707,6 @@ msgstr "" msgid "GroupSettings|Transfer group" msgstr "" -msgid "GroupSettings|Use GitLab Duo features" -msgstr "" - msgid "GroupSettings|Users can create %{link_start_project}project access tokens%{link_end} and %{link_start_group}group access tokens%{link_end} in this group" msgstr "" diff --git a/spec/migrations/lock_duo_features_enabled_update_spec.rb b/spec/migrations/lock_duo_features_enabled_update_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..b2aca1902f92beb7919fc0526b1380480e9f1a45 --- /dev/null +++ b/spec/migrations/lock_duo_features_enabled_update_spec.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe LockDuoFeaturesEnabledUpdate, feature_category: :ai_abstraction_layer do + let(:namespaces) { table(:namespaces) } + let(:namespace_settings) { table(:namespace_settings) } + + before do + namespaces.create!(name: 'test', path: 'test') + + allow_next_instance_of(described_class) do |instance| + allow(instance).to receive(:add_concurrent_index).and_return(true) + end + allow_next_instance_of(described_class) do |instance| + allow(instance).to receive(:remove_concurrent_index).and_return(true) + end + allow_next_instance_of(described_class) do |instance| + allow(instance).to receive(:update_column_in_batches).and_yield(namespace_settings) + end + end + + it 'correctly updates lock_duo_features_enabled' do + namespace = namespaces.first + namespace_settings.create!( + namespace_id: namespace.id, + duo_features_enabled: true, + lock_duo_features_enabled: true + ) + + expect(namespace_settings.where(duo_features_enabled: true, lock_duo_features_enabled: true).count).to eq(1) + + migrate! + + expect(namespace_settings.where(duo_features_enabled: true, lock_duo_features_enabled: false).count).to eq(1) + expect(namespace_settings.where(duo_features_enabled: true, lock_duo_features_enabled: true).count).to eq(0) + end + + it 'does not update records where duo_features_enabled is false' do + namespace = namespaces.first + namespace_settings.create!( + namespace_id: namespace.id, + duo_features_enabled: false, + lock_duo_features_enabled: true + ) + + expect(namespace_settings.where(duo_features_enabled: false, lock_duo_features_enabled: true).count).to eq(1) + + migrate! + + expect(namespace_settings.where(duo_features_enabled: false, lock_duo_features_enabled: true).count).to eq(1) + end + + it 'does not update records where lock_duo_features_enabled is already false' do + namespace = namespaces.first + namespace_settings.create!( + namespace_id: namespace.id, + duo_features_enabled: true, + lock_duo_features_enabled: false + ) + + expect(namespace_settings.where(duo_features_enabled: true, lock_duo_features_enabled: false).count).to eq(1) + + migrate! + + expect(namespace_settings.where(duo_features_enabled: true, lock_duo_features_enabled: false).count).to eq(1) + end + + it 'handles multiple records correctly' do + namespace1 = namespaces.create!(name: 'test1', path: 'test1') + namespace2 = namespaces.create!(name: 'test2', path: 'test2') + + namespace_settings.create!( + namespace_id: namespace1.id, + duo_features_enabled: true, + lock_duo_features_enabled: true + ) + namespace_settings.create!( + namespace_id: namespace2.id, + duo_features_enabled: true, + lock_duo_features_enabled: true + ) + + expect(namespace_settings.where(duo_features_enabled: true, lock_duo_features_enabled: true).count).to eq(2) + + migrate! + + expect(namespace_settings.where(duo_features_enabled: true, lock_duo_features_enabled: false).count).to eq(2) + expect(namespace_settings.where(duo_features_enabled: true, lock_duo_features_enabled: true).count).to eq(0) + end + + describe '#down' do + it 'does not modify any records' do + namespace = namespaces.first + namespace_settings.create!( + namespace_id: namespace.id, + duo_features_enabled: true, + lock_duo_features_enabled: false + ) + + expect do + described_class.new.down + end.not_to change { namespace_settings.first } + end + end +end