From 8fd033defe630f05ecd8d8850428e77206095608 Mon Sep 17 00:00:00 2001 From: Joseph Snyder Date: Mon, 12 Dec 2022 15:06:33 -0500 Subject: [PATCH 1/4] Introduce migration for emails_enabled column Add the migrations to create and populate the *****_settings column for emails enabled for both projects and namespaces. Changelog: added --- app/models/namespace.rb | 10 +++ app/models/project.rb | 9 +++ ...4507_add_projects_emails_enabled_column.rb | 7 ++ ...39_add_namespaces_emails_enabled_column.rb | 8 ++ ...d_namespaces_emails_enabled_column_data.rb | 28 +++++++ ...add_projects_emails_enabled_column_data.rb | 28 +++++++ db/schema_migrations/20221116134507 | 1 + db/schema_migrations/20221116134539 | 1 + db/schema_migrations/20221116134611 | 1 + db/schema_migrations/20221116134633 | 1 + db/structure.sql | 2 + ...d_namespaces_emails_enabled_column_data.rb | 32 ++++++++ ...add_projects_emails_enabled_column_data.rb | 32 ++++++++ ...espaces_emails_enabled_column_data_spec.rb | 69 +++++++++++++++++ ...rojects_emails_enabled_column_data_spec.rb | 75 +++++++++++++++++++ spec/models/namespace_setting_spec.rb | 30 ++++++++ spec/models/namespace_spec.rb | 17 ++++- spec/models/project_setting_spec.rb | 50 +++++++++++++ spec/models/project_spec.rb | 17 ++++- spec/requests/api/project_attributes.yml | 1 + 20 files changed, 413 insertions(+), 6 deletions(-) create mode 100644 db/migrate/20221116134507_add_projects_emails_enabled_column.rb create mode 100644 db/migrate/20221116134539_add_namespaces_emails_enabled_column.rb create mode 100644 db/post_migrate/20221116134611_add_namespaces_emails_enabled_column_data.rb create mode 100644 db/post_migrate/20221116134633_add_projects_emails_enabled_column_data.rb create mode 100644 db/schema_migrations/20221116134507 create mode 100644 db/schema_migrations/20221116134539 create mode 100644 db/schema_migrations/20221116134611 create mode 100644 db/schema_migrations/20221116134633 create mode 100644 lib/gitlab/background_migration/add_namespaces_emails_enabled_column_data.rb create mode 100644 lib/gitlab/background_migration/add_projects_emails_enabled_column_data.rb create mode 100644 spec/migrations/add_namespaces_emails_enabled_column_data_spec.rb create mode 100644 spec/migrations/add_projects_emails_enabled_column_data_spec.rb diff --git a/app/models/namespace.rb b/app/models/namespace.rb index a0d44bcc26eba1..b83a293dd818e2 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -141,12 +141,14 @@ class Namespace < ApplicationRecord :npm_package_requests_forwarding, to: :package_settings + before_save :update_new_emails_created_column, if: -> { emails_disabled_changed? } before_create :sync_share_with_group_lock_with_parent before_update :sync_share_with_group_lock_with_parent, if: :parent_changed? after_update :force_share_with_group_lock_on_descendants, if: -> { saved_change_to_share_with_group_lock? && share_with_group_lock? } after_update :expire_first_auto_devops_config_cache, if: -> { saved_change_to_auto_devops_enabled? } after_update :move_dir, if: :saved_change_to_path_or_parent?, unless: -> { is_a?(Namespaces::ProjectNamespace) } after_destroy :rm_dir + after_save :reload_namespace_details after_commit :refresh_access_of_projects_invited_groups, on: :update, if: -> { previous_changes.key?('share_with_group_lock') } @@ -599,6 +601,14 @@ def all_ancestors_have_runner_registration_enabled? private + def update_new_emails_created_column + if namespace_settings.persisted? + namespace_settings&.update!(emails_enabled: !emails_disabled) + else + namespace_settings.emails_enabled = !emails_disabled + end + end + def cluster_enabled_granted? (Gitlab.com? || Gitlab.dev_or_test_env?) && root_ancestor.cluster_enabled_grant.present? end diff --git a/app/models/project.rb b/app/models/project.rb index 0012603466e7a1..87be073357cab3 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -119,6 +119,7 @@ class Project < ApplicationRecord before_validation :remove_leading_spaces_on_name after_validation :check_pending_delete before_save :ensure_runners_token + before_save :update_new_emails_created_column, if: -> { emails_disabled_changed? } after_create -> { create_or_load_association(:project_feature) } after_create -> { create_or_load_association(:ci_cd_settings) } @@ -3389,6 +3390,14 @@ def enabled_package_registry_access_level_by_project_visibility ProjectFeature::PRIVATE end end + + def update_new_emails_created_column + if project_setting.persisted? + project_setting.update!(emails_enabled: !emails_disabled) + else + project_setting.emails_enabled = !emails_disabled + end + end end Project.prepend_mod_with('Project') diff --git a/db/migrate/20221116134507_add_projects_emails_enabled_column.rb b/db/migrate/20221116134507_add_projects_emails_enabled_column.rb new file mode 100644 index 00000000000000..1499ea2752bd3f --- /dev/null +++ b/db/migrate/20221116134507_add_projects_emails_enabled_column.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true +class AddProjectsEmailsEnabledColumn < Gitlab::Database::Migration[2.0] + enable_lock_retries! + def change + add_column :project_settings, :emails_enabled, :boolean, default: true, null: false + end +end diff --git a/db/migrate/20221116134539_add_namespaces_emails_enabled_column.rb b/db/migrate/20221116134539_add_namespaces_emails_enabled_column.rb new file mode 100644 index 00000000000000..e979cbb8aa5a58 --- /dev/null +++ b/db/migrate/20221116134539_add_namespaces_emails_enabled_column.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true +class AddNamespacesEmailsEnabledColumn < Gitlab::Database::Migration[2.0] + enable_lock_retries! + + def change + add_column :namespace_settings, :emails_enabled, :boolean, default: true, null: false + end +end diff --git a/db/post_migrate/20221116134611_add_namespaces_emails_enabled_column_data.rb b/db/post_migrate/20221116134611_add_namespaces_emails_enabled_column_data.rb new file mode 100644 index 00000000000000..e3efc18f3fde29 --- /dev/null +++ b/db/post_migrate/20221116134611_add_namespaces_emails_enabled_column_data.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true +class AddNamespacesEmailsEnabledColumnData < Gitlab::Database::Migration[2.0] + disable_ddl_transaction! + restrict_gitlab_migration gitlab_schema: :gitlab_main + + MIGRATION = 'AddNamespacesEmailsEnabledColumnData' + DELAY_INTERVAL = 2.minutes.to_i + BATCH_SIZE = 200 + MAX_BATCH_SIZE = 1000 + SUB_BATCH_SIZE = 20 + + def up + queue_batched_background_migration( + MIGRATION, + :namespaces, + :id, + job_interval: DELAY_INTERVAL, + batch_size: BATCH_SIZE, + max_batch_size: MAX_BATCH_SIZE, + sub_batch_size: SUB_BATCH_SIZE, + gitlab_schema: :gitlab_main + ) + end + + def down + delete_batched_background_migration(MIGRATION, :namespaces, :id, []) + end +end diff --git a/db/post_migrate/20221116134633_add_projects_emails_enabled_column_data.rb b/db/post_migrate/20221116134633_add_projects_emails_enabled_column_data.rb new file mode 100644 index 00000000000000..9f0edbd0707e26 --- /dev/null +++ b/db/post_migrate/20221116134633_add_projects_emails_enabled_column_data.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true +class AddProjectsEmailsEnabledColumnData < Gitlab::Database::Migration[2.0] + disable_ddl_transaction! + restrict_gitlab_migration gitlab_schema: :gitlab_main + + MIGRATION = 'AddProjectsEmailsEnabledColumnData' + DELAY_INTERVAL = 2.minutes.to_i + BATCH_SIZE = 200 + MAX_BATCH_SIZE = 1000 + SUB_BATCH_SIZE = 20 + + def up + queue_batched_background_migration( + MIGRATION, + :projects, + :id, + job_interval: DELAY_INTERVAL, + batch_size: BATCH_SIZE, + max_batch_size: MAX_BATCH_SIZE, + sub_batch_size: SUB_BATCH_SIZE, + gitlab_schema: :gitlab_main + ) + end + + def down + delete_batched_background_migration(MIGRATION, :projects, :id, []) + end +end diff --git a/db/schema_migrations/20221116134507 b/db/schema_migrations/20221116134507 new file mode 100644 index 00000000000000..cb761de5adb968 --- /dev/null +++ b/db/schema_migrations/20221116134507 @@ -0,0 +1 @@ +80504a4700681db9e46d729f4175dc077fae7e1b0235c9178558293b83f7a006 \ No newline at end of file diff --git a/db/schema_migrations/20221116134539 b/db/schema_migrations/20221116134539 new file mode 100644 index 00000000000000..4f25094c3a83e3 --- /dev/null +++ b/db/schema_migrations/20221116134539 @@ -0,0 +1 @@ +dd36d2586454c8799effa598c0a058a6adf332622877eae16dd95d468f9b3958 \ No newline at end of file diff --git a/db/schema_migrations/20221116134611 b/db/schema_migrations/20221116134611 new file mode 100644 index 00000000000000..9e47c46b7e3f4b --- /dev/null +++ b/db/schema_migrations/20221116134611 @@ -0,0 +1 @@ +0668760d6df566ac3081bd9fa2a053497da7a7af652225e91831110435166dcb \ No newline at end of file diff --git a/db/schema_migrations/20221116134633 b/db/schema_migrations/20221116134633 new file mode 100644 index 00000000000000..3f89ce1ff9d63c --- /dev/null +++ b/db/schema_migrations/20221116134633 @@ -0,0 +1 @@ +ceaf6a2b15da0dde23ba37f1166aa5135a9dce1abbe9fca81a12a41cc0319fd9 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 4d5e58cd0727e9..bcc7245aab7c93 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -18356,6 +18356,7 @@ CREATE TABLE namespace_settings ( runner_registration_enabled boolean DEFAULT true, allow_runner_registration_token boolean DEFAULT true NOT NULL, unique_project_download_limit_alertlist integer[] DEFAULT '{}'::integer[] NOT NULL, + emails_enabled boolean DEFAULT true NOT NULL, CONSTRAINT check_0ba93c78c7 CHECK ((char_length(default_branch_name) <= 255)), CONSTRAINT namespace_settings_unique_project_download_limit_alertlist_size CHECK ((cardinality(unique_project_download_limit_alertlist) <= 100)), CONSTRAINT namespace_settings_unique_project_download_limit_allowlist_size CHECK ((cardinality(unique_project_download_limit_allowlist) <= 100)) @@ -20674,6 +20675,7 @@ CREATE TABLE project_settings ( only_allow_merge_if_all_status_checks_passed boolean DEFAULT false NOT NULL, mirror_branch_regex text, allow_pipeline_trigger_approve_deployment boolean DEFAULT false NOT NULL, + emails_enabled boolean DEFAULT true NOT NULL, CONSTRAINT check_2981f15877 CHECK ((char_length(jitsu_key) <= 100)), CONSTRAINT check_3a03e7557a CHECK ((char_length(previous_default_branch) <= 4096)), CONSTRAINT check_3ca5cbffe6 CHECK ((char_length(issue_branch_template) <= 255)), diff --git a/lib/gitlab/background_migration/add_namespaces_emails_enabled_column_data.rb b/lib/gitlab/background_migration/add_namespaces_emails_enabled_column_data.rb new file mode 100644 index 00000000000000..46e2d5cb93018a --- /dev/null +++ b/lib/gitlab/background_migration/add_namespaces_emails_enabled_column_data.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # Iterates through the namespaces table and attempts to set the + # opposite of the value of the column "emails_disabled" to a new + # column in namespace_settings called emails_enabled + class AddNamespacesEmailsEnabledColumnData < BatchedMigrationJob + feature_category :database + operation_name :add_namespaces_emails_enabled_column_data + + # Targeted table + class NamespaceSetting < ApplicationRecord + self.table_name = 'namespace_settings' + end + + def perform + each_sub_batch do |sub_batch| + plucked_list = sub_batch.where('NOT emails_disabled IS NULL').pluck(:id, :emails_disabled) + next if plucked_list.empty? + + ApplicationRecord.connection.execute <<~SQL + UPDATE namespace_settings + SET emails_enabled= NOT subquery.emails_enabled + FROM (SELECT * FROM (#{Arel::Nodes::ValuesList.new(plucked_list).to_sql}) AS updates(namespace_id, emails_enabled)) AS subquery + WHERE namespace_settings.namespace_id=subquery.namespace_id + SQL + end + end + end + end +end diff --git a/lib/gitlab/background_migration/add_projects_emails_enabled_column_data.rb b/lib/gitlab/background_migration/add_projects_emails_enabled_column_data.rb new file mode 100644 index 00000000000000..a0ce5d22597fbc --- /dev/null +++ b/lib/gitlab/background_migration/add_projects_emails_enabled_column_data.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # Iterates through the Projects table and attempts to set the + # opposite of the value of the column "emails_disabled" to a new + # column in project_settings called emails_enabled + class AddProjectsEmailsEnabledColumnData < BatchedMigrationJob + feature_category :database + operation_name :add_projects_emails_enabled_column_data + + # Targeted table + class ProjectSetting < ApplicationRecord + self.table_name = 'project_settings' + end + + def perform + each_sub_batch do |sub_batch| + plucked_list = sub_batch.where('NOT emails_disabled IS NULL').pluck(:id, :emails_disabled) + next if plucked_list.empty? + + ApplicationRecord.connection.execute <<~SQL + UPDATE project_settings + SET emails_enabled=NOT subquery.emails_enabled + FROM (SELECT * FROM (#{Arel::Nodes::ValuesList.new(plucked_list).to_sql}) AS updates(project_id, emails_enabled)) AS subquery + WHERE project_settings.project_id=subquery.project_id + SQL + end + end + end + end +end diff --git a/spec/migrations/add_namespaces_emails_enabled_column_data_spec.rb b/spec/migrations/add_namespaces_emails_enabled_column_data_spec.rb new file mode 100644 index 00000000000000..96afcb23f3531a --- /dev/null +++ b/spec/migrations/add_namespaces_emails_enabled_column_data_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'rake_helper' +require_migration! + +RSpec.describe AddNamespacesEmailsEnabledColumnData, :migration, feature_category: :database do + before :all do + Rake.application.rake_require 'active_record/railties/databases' + Rake.application.rake_require 'tasks/gitlab/db' + + # empty task as env is already loaded + Rake::Task.define_task :environment + end + + let(:migration) { described_class::MIGRATION } + let(:projects) { table(:projects) } + let(:namespace_settings_table) { table(:namespace_settings) } + let(:namespaces) { table(:namespaces) } + + before do + stub_const("#{described_class.name}::SUB_BATCH_SIZE", 2) + end + + it 'schedules background migrations', :aggregate_failures do + migrate! + + expect(migration).to have_scheduled_batched_migration( + table_name: :namespaces, + column_name: :id, + interval: described_class::DELAY_INTERVAL + ) + end + + describe '#down' do + it 'deletes all batched migration records' do + migrate! + schema_migrate_down! + + expect(migration).not_to have_scheduled_batched_migration + end + end + + it 'sets emails_enabled to be the opposite of emails_disabled' do + disabled_records_to_migrate = 6 + enabled_records_to_migrate = 4 + + disabled_records_to_migrate.times do |i| + namespace = namespaces.create!(name: 'namespace', + path: "namespace#{i}", + emails_disabled: true) + namespace_settings_table.create!(namespace_id: namespace.id) + end + + enabled_records_to_migrate.times do |i| + namespace = namespaces.create!(name: 'namespace', + path: "namespace#{i}", + emails_disabled: false) + namespace_settings_table.create!(namespace_id: namespace.id) + end + + migrate! + run_rake_task('gitlab:db:execute_batched_migrations') + # rubocop: disable CodeReuse/ActiveRecord + expect(namespace_settings_table.where(emails_enabled: true).count).to eq(enabled_records_to_migrate) + expect(namespace_settings_table.where(emails_enabled: false).count).to eq(disabled_records_to_migrate) + # rubocop: enable CodeReuse/ActiveRecord + end +end diff --git a/spec/migrations/add_projects_emails_enabled_column_data_spec.rb b/spec/migrations/add_projects_emails_enabled_column_data_spec.rb new file mode 100644 index 00000000000000..c167b13d83aa6d --- /dev/null +++ b/spec/migrations/add_projects_emails_enabled_column_data_spec.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'rake_helper' +require_migration! + +RSpec.describe AddProjectsEmailsEnabledColumnData, :migration, feature_category: :database do + before :all do + Rake.application.rake_require 'active_record/railties/databases' + Rake.application.rake_require 'tasks/gitlab/db' + + # empty task as env is already loaded + Rake::Task.define_task :environment + end + + let(:migration) { described_class::MIGRATION } + let(:project_settings) { table(:project_settings) } + let(:projects) { table(:projects) } + let(:namespaces) { table(:namespaces) } + + before do + stub_const("#{described_class.name}::SUB_BATCH_SIZE", 2) + end + + it 'schedules background migrations', :aggregate_failures do + migrate! + + expect(migration).to have_scheduled_batched_migration( + table_name: :projects, + column_name: :id, + interval: described_class::DELAY_INTERVAL + ) + end + + describe '#down' do + it 'deletes all batched migration records' do + migrate! + schema_migrate_down! + + expect(migration).not_to have_scheduled_batched_migration + end + end + + it 'sets emails_enabled to be the opposite of emails_disabled' do + disabled_records_to_migrate = 4 + enabled_records_to_migrate = 2 + + disabled_records_to_migrate.times do |i| + namespace = namespaces.create!(name: 'namespace', path: "namespace#{i}") + project = projects.create!(name: "Project Disabled #{i}", + path: "projectDisabled#{i}", + namespace_id: namespace.id, + project_namespace_id: namespace.id, + emails_disabled: true) + project_settings.create!(project_id: project.id) + end + + enabled_records_to_migrate.times do |i| + namespace = namespaces.create!(name: 'namespace', path: "namespace#{i}") + project = projects.create!(name: "Project Enabled #{i}", + path: "projectEnabled#{i}", + namespace_id: namespace.id, + project_namespace_id: namespace.id, + emails_disabled: false) + project_settings.create!(project_id: project.id) + end + + migrate! + run_rake_task('gitlab:db:execute_batched_migrations') + # rubocop: disable CodeReuse/ActiveRecord + expect(project_settings.where(emails_enabled: true).count).to eq(enabled_records_to_migrate) + expect(project_settings.where(emails_enabled: false).count).to eq(disabled_records_to_migrate) + # rubocop: enable CodeReuse/ActiveRecord + end +end diff --git a/spec/models/namespace_setting_spec.rb b/spec/models/namespace_setting_spec.rb index 15b80749aa2e9c..b7cc59b5af3074 100644 --- a/spec/models/namespace_setting_spec.rb +++ b/spec/models/namespace_setting_spec.rb @@ -159,6 +159,36 @@ end end + describe '#emails_enabled?' do + context 'when a group has no parent' + let(:settings) { create(:namespace_settings, emails_enabled: true) } + let(:grandparent) { create(:group) } + let(:parent) { create(:group, parent: grandparent) } + let(:group) { create(:group, parent: parent, namespace_settings: settings) } + + context 'when the groups setting is changed' do + it 'returns false when the attribute is false' do + group.update_attribute(:emails_disabled, true) + + expect(group.emails_enabled?).to be_falsey + end + end + + context 'when a group has a parent' do + it 'returns true when no parent has disabled emails' do + expect(group.emails_enabled?).to be_truthy + end + + context 'when ancestor emails are disabled' do + it 'returns false' do + grandparent.update_attribute(:emails_disabled, true) + + expect(group.emails_enabled?).to be_falsey + end + end + end + end + context 'when a group has parent groups' do let(:grandparent) { create(:group, namespace_settings: settings) } let(:parent) { create(:group, parent: grandparent) } diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index b8cb90032657b5..a0698ac30f5a45 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -2087,10 +2087,21 @@ def expect_project_directories_at(namespace_path, with_pages: true) end describe '#emails_enabled?' do - it "is the opposite of emails_disabled" do - group = create(:group, emails_disabled: false) + context 'without a persisted namespace_setting object' do + let(:group) { build(:group, emails_disabled: false) } - expect(group.emails_enabled?).to be_truthy + it "is the opposite of emails_disabled" do + expect(group.emails_enabled?).to be_truthy + end + end + + context 'with a persisted namespace_setting object' do + let(:namespace_settings) { create(:namespace_settings, emails_enabled: true) } + let(:group) { build(:group, emails_disabled: false, namespace_settings: namespace_settings) } + + it "is the opposite of emails_disabled" do + expect(group.emails_enabled?).to be_truthy + end end end diff --git a/spec/models/project_setting_spec.rb b/spec/models/project_setting_spec.rb index 94a2e2fe3f98ce..328a98b4ffbe97 100644 --- a/spec/models/project_setting_spec.rb +++ b/spec/models/project_setting_spec.rb @@ -96,6 +96,56 @@ def valid_target_platform_combinations end end + describe '#emails_enabled?' do + context "when a project does not have a parent group" do + let(:project_settings) { create(:project_setting, emails_enabled: true) } + let(:project) { create(:project, project_setting: project_settings) } + + it "returns true" do + expect(project.emails_enabled?).to be_truthy + end + + it "returns false when updating project settings" do + project.update_attribute(:emails_disabled, false) + expect(project.emails_enabled?).to be_truthy + end + end + + context "when a project has a parent group" do + let(:namespace_settings) { create(:namespace_settings, emails_enabled: true) } + let(:project_settings) { create(:project_setting, emails_enabled: true) } + let(:group) { create(:group, namespace_settings: namespace_settings) } + let(:project) do + create(:project, namespace_id: group.id, + project_setting: project_settings) + end + + context 'when emails have been disabled in parent group' do + it 'returns false' do + group.update_attribute(:emails_disabled, true) + + expect(project.emails_enabled?).to be_falsey + end + end + + context 'when emails are enabled in parent group' do + before do + allow(project.namespace).to receive(:emails_enabled?).and_return(true) + end + + it 'returns true' do + expect(project.emails_enabled?).to be_truthy + end + + it 'returns false when disabled at the project' do + project.update_attribute(:emails_disabled, true) + + expect(project.emails_enabled?).to be_falsey + end + end + end + end + context 'when a parent group has a parent group' do let(:namespace_settings) { create(:namespace_settings, show_diff_preview_in_email: false) } let(:project_settings) { create(:project_setting, show_diff_preview_in_email: true) } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index b438984322d472..7da1915bc4557f 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -3946,10 +3946,21 @@ def has_external_wiki end describe '#emails_enabled?' do - let(:project) { build(:project, emails_disabled: false) } + context 'without a persisted project_setting object' do + let(:project) { build(:project, emails_disabled: false) } - it "is the opposite of emails_disabled" do - expect(project.emails_enabled?).to be_truthy + it "is the opposite of emails_disabled" do + expect(project.emails_enabled?).to be_truthy + end + end + + context 'with a persisted project_setting object' do + let(:project_settings) { create(:project_setting, emails_enabled: true) } + let(:project) { build(:project, emails_disabled: false, project_setting: project_settings) } + + it "is the opposite of emails_disabled" do + expect(project.emails_enabled?).to be_truthy + end end end diff --git a/spec/requests/api/project_attributes.yml b/spec/requests/api/project_attributes.yml index 66cca8fbe562e7..7f38b4eec83ff9 100644 --- a/spec/requests/api/project_attributes.yml +++ b/spec/requests/api/project_attributes.yml @@ -161,6 +161,7 @@ project_setting: - jitsu_key - mirror_branch_regex - allow_pipeline_trigger_approve_deployment + - emails_enabled build_service_desk_setting: # service_desk_setting unexposed_attributes: -- GitLab From e16e8f9794b684d57d35b994a006a24116b3333e Mon Sep 17 00:00:00 2001 From: Joseph Snyder Date: Thu, 2 Feb 2023 14:26:32 -0500 Subject: [PATCH 2/4] Update function for saving email settings Based on the suggestion of A. Hegyi in a WIP branch: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/110877/diffs?commit_id=3f3fa5e7eec17f97a9b7b19c705655f8ae5e37eb --- app/models/namespace.rb | 7 +++++-- app/models/project.rb | 5 ++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/app/models/namespace.rb b/app/models/namespace.rb index b83a293dd818e2..9d9b09e35622f2 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -602,9 +602,12 @@ def all_ancestors_have_runner_registration_enabled? private def update_new_emails_created_column + return if namespace_settings.nil? + return if namespace_settings.emails_enabled == !emails_disabled + if namespace_settings.persisted? - namespace_settings&.update!(emails_enabled: !emails_disabled) - else + namespace_settings.update!(emails_enabled: !emails_disabled) + elsif namespace_settings namespace_settings.emails_enabled = !emails_disabled end end diff --git a/app/models/project.rb b/app/models/project.rb index 87be073357cab3..d464a8f7d2ad97 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -3392,9 +3392,12 @@ def enabled_package_registry_access_level_by_project_visibility end def update_new_emails_created_column + return if project_setting.nil? + return if project_setting.emails_enabled == !emails_disabled + if project_setting.persisted? project_setting.update!(emails_enabled: !emails_disabled) - else + elsif project_setting project_setting.emails_enabled = !emails_disabled end end -- GitLab From 00d16bac8692f2ddd00e343682c62f8b7e611b55 Mon Sep 17 00:00:00 2001 From: Joseph Snyder Date: Wed, 8 Feb 2023 15:54:45 -0500 Subject: [PATCH 3/4] Fix indentation issues Accept the autocorrections of rubocop for line indentations. --- ...namespaces_emails_enabled_column_data_spec.rb | 8 ++++---- ...d_projects_emails_enabled_column_data_spec.rb | 16 ++++++++-------- spec/models/project_setting_spec.rb | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/spec/migrations/add_namespaces_emails_enabled_column_data_spec.rb b/spec/migrations/add_namespaces_emails_enabled_column_data_spec.rb index 96afcb23f3531a..6cab3ca3d8fac4 100644 --- a/spec/migrations/add_namespaces_emails_enabled_column_data_spec.rb +++ b/spec/migrations/add_namespaces_emails_enabled_column_data_spec.rb @@ -47,15 +47,15 @@ disabled_records_to_migrate.times do |i| namespace = namespaces.create!(name: 'namespace', - path: "namespace#{i}", - emails_disabled: true) + path: "namespace#{i}", + emails_disabled: true) namespace_settings_table.create!(namespace_id: namespace.id) end enabled_records_to_migrate.times do |i| namespace = namespaces.create!(name: 'namespace', - path: "namespace#{i}", - emails_disabled: false) + path: "namespace#{i}", + emails_disabled: false) namespace_settings_table.create!(namespace_id: namespace.id) end diff --git a/spec/migrations/add_projects_emails_enabled_column_data_spec.rb b/spec/migrations/add_projects_emails_enabled_column_data_spec.rb index c167b13d83aa6d..1d021ecd439a63 100644 --- a/spec/migrations/add_projects_emails_enabled_column_data_spec.rb +++ b/spec/migrations/add_projects_emails_enabled_column_data_spec.rb @@ -48,20 +48,20 @@ disabled_records_to_migrate.times do |i| namespace = namespaces.create!(name: 'namespace', path: "namespace#{i}") project = projects.create!(name: "Project Disabled #{i}", - path: "projectDisabled#{i}", - namespace_id: namespace.id, - project_namespace_id: namespace.id, - emails_disabled: true) + path: "projectDisabled#{i}", + namespace_id: namespace.id, + project_namespace_id: namespace.id, + emails_disabled: true) project_settings.create!(project_id: project.id) end enabled_records_to_migrate.times do |i| namespace = namespaces.create!(name: 'namespace', path: "namespace#{i}") project = projects.create!(name: "Project Enabled #{i}", - path: "projectEnabled#{i}", - namespace_id: namespace.id, - project_namespace_id: namespace.id, - emails_disabled: false) + path: "projectEnabled#{i}", + namespace_id: namespace.id, + project_namespace_id: namespace.id, + emails_disabled: false) project_settings.create!(project_id: project.id) end diff --git a/spec/models/project_setting_spec.rb b/spec/models/project_setting_spec.rb index 328a98b4ffbe97..feb5985818ba2e 100644 --- a/spec/models/project_setting_spec.rb +++ b/spec/models/project_setting_spec.rb @@ -117,7 +117,7 @@ def valid_target_platform_combinations let(:group) { create(:group, namespace_settings: namespace_settings) } let(:project) do create(:project, namespace_id: group.id, - project_setting: project_settings) + project_setting: project_settings) end context 'when emails have been disabled in parent group' do -- GitLab From 9ba670d3b2fc2445916aacebb105de2c8f85476c Mon Sep 17 00:00:00 2001 From: Danger bot Date: Mon, 13 Feb 2023 18:18:15 +0000 Subject: [PATCH 4/4] Apply Dangerbot suggestion for test project creation. --- spec/models/project_setting_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/project_setting_spec.rb b/spec/models/project_setting_spec.rb index feb5985818ba2e..01e6f901c44e06 100644 --- a/spec/models/project_setting_spec.rb +++ b/spec/models/project_setting_spec.rb @@ -99,7 +99,7 @@ def valid_target_platform_combinations describe '#emails_enabled?' do context "when a project does not have a parent group" do let(:project_settings) { create(:project_setting, emails_enabled: true) } - let(:project) { create(:project, project_setting: project_settings) } + let_it_be(:project) { create(:project, project_setting: project_settings) } it "returns true" do expect(project.emails_enabled?).to be_truthy -- GitLab