diff --git a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue index ff9781385cdc0362c6bb633963ab15330492e847..b6ebcabd33c590a92f89b76e0e709f005b6f1abf 100644 --- a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue +++ b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue @@ -277,7 +277,7 @@ export default { requestAccessEnabled: true, enforceAuthChecksOnUploads: true, highlightChangesClass: false, - emailsDisabled: false, + emailsEnabled: true, cveIdRequestEnabled: true, featureAccessLevelEveryone, featureAccessLevelMembers, @@ -1001,14 +1001,19 @@ export default { :full-path="confirmationPhrase" /> - { emails_disabled_changed? } after_create -> { create_or_load_association(:project_feature) } after_create -> { create_or_load_association(:ci_cd_settings) } @@ -520,6 +522,7 @@ def self.integration_association_name(name) delegate :has_shimo? delegate :show_diff_preview_in_email, :show_diff_preview_in_email=, :show_diff_preview_in_email? delegate :runner_registration_enabled, :runner_registration_enabled=, :runner_registration_enabled? + delegate :emails_enabled, :emails_enabled=, :emails_enabled? delegate :squash_always?, :squash_never?, :squash_enabled_by_default?, :squash_readonly? delegate :mr_default_target_self, :mr_default_target_self= delegate :previous_default_branch, :previous_default_branch= @@ -1209,14 +1212,8 @@ def ancestors_upto_ids(...) end def emails_disabled? - strong_memoize(:emails_disabled) do - # disabling in the namespace overrides the project setting - super || namespace.emails_disabled? - end - end - - def emails_enabled? - !emails_disabled? + # disabling in the namespace overrides the project setting + !emails_enabled? end override :lfs_enabled? @@ -3502,17 +3499,6 @@ def enabled_package_registry_access_level_by_project_visibility end 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) - elsif project_setting - project_setting.emails_enabled = !emails_disabled - end - end - def runners_token_prefix RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX end diff --git a/app/models/project_setting.rb b/app/models/project_setting.rb index 7ca74d4e970802357687e9a184735231e2a3e4f2..e8573d5c87483fc47fcbacbd42924e609524969a 100644 --- a/app/models/project_setting.rb +++ b/app/models/project_setting.rb @@ -97,6 +97,11 @@ def runner_registration_enabled Gitlab::CurrentSettings.valid_runner_registrars.include?('project') && read_attribute(:runner_registration_enabled) end + def emails_enabled? + super && project.namespace.emails_enabled? + end + strong_memoize_attr :emails_enabled? + private def validates_mr_default_target_self diff --git a/app/presenters/issue_presenter.rb b/app/presenters/issue_presenter.rb index 69d775d8125ef2c8fddf386249ec001e172bc528..42ecbc9988e58db7549f9329b6af7a72d488f9d7 100644 --- a/app/presenters/issue_presenter.rb +++ b/app/presenters/issue_presenter.rb @@ -16,6 +16,10 @@ def project_emails_disabled? issue.project.emails_disabled? end + def project_emails_enabled? + issue.project.emails_enabled? + end + delegator_override :service_desk_reply_to def service_desk_reply_to return unless super.present? diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index 7f25ab5883f857fe2fdbd3ae8fba8752be60142b..c470cedf0b820a4f3eea2e030bc88d749dfe3429 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -119,7 +119,7 @@ def after_default_branch_change(previous_default_branch) end def remove_unallowed_params - params.delete(:emails_disabled) unless can?(current_user, :set_emails_disabled, project) + params.delete(:emails_enabled) unless can?(current_user, :set_emails_disabled, project) params.delete(:runner_registration_enabled) if Gitlab::CurrentSettings.valid_runner_registrars.exclude?('project') end diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 7e0a50a01ae8d858da8036c755178c3d54940efd..c691fd09ce70626ee8ca14550a1a0727e9779483 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -15533,7 +15533,8 @@ Relationship between an epic and an issue. | `discussions` | [`DiscussionConnection!`](#discussionconnection) | All discussions on this noteable. (see [Connections](#connections)) | | `downvotes` | [`Int!`](#int) | Number of downvotes the issue has received. | | `dueDate` | [`Time`](#time) | Due date of the issue. | -| `emailsDisabled` | [`Boolean!`](#boolean) | Indicates if a project has email notifications disabled: `true` if email notifications are disabled. | +| `emailsDisabled` **{warning-solid}** | [`Boolean!`](#boolean) | **Deprecated** in 16.3. Use `emails_enabled`. | +| `emailsEnabled` | [`Boolean!`](#boolean) | Indicates if a project has email notifications disabled: `false` if email notifications are disabled. | | `epic` | [`Epic`](#epic) | Epic to which this issue belongs. | | `epicIssueId` | [`ID!`](#id) | ID of the epic-issue relation. | | `escalationPolicy` | [`EscalationPolicyType`](#escalationpolicytype) | Escalation policy associated with the issue. Available for issues which support escalation. | @@ -17632,7 +17633,8 @@ Describes an issuable resource link for incident issues. | `discussions` | [`DiscussionConnection!`](#discussionconnection) | All discussions on this noteable. (see [Connections](#connections)) | | `downvotes` | [`Int!`](#int) | Number of downvotes the issue has received. | | `dueDate` | [`Time`](#time) | Due date of the issue. | -| `emailsDisabled` | [`Boolean!`](#boolean) | Indicates if a project has email notifications disabled: `true` if email notifications are disabled. | +| `emailsDisabled` **{warning-solid}** | [`Boolean!`](#boolean) | **Deprecated** in 16.3. Use `emails_enabled`. | +| `emailsEnabled` | [`Boolean!`](#boolean) | Indicates if a project has email notifications disabled: `false` if email notifications are disabled. | | `epic` | [`Epic`](#epic) | Epic to which this issue belongs. | | `escalationPolicy` | [`EscalationPolicyType`](#escalationpolicytype) | Escalation policy associated with the issue. Available for issues which support escalation. | | `escalationStatus` | [`IssueEscalationStatus`](#issueescalationstatus) | Escalation status of the issue. | diff --git a/ee/spec/controllers/projects/settings/repository_controller_spec.rb b/ee/spec/controllers/projects/settings/repository_controller_spec.rb index 824ae52d0c432d69bd4a4e8c687c7e3e52d65706..db108422bdeab51a59a9fac2e2600efe0fe54576 100644 --- a/ee/spec/controllers/projects/settings/repository_controller_spec.rb +++ b/ee/spec/controllers/projects/settings/repository_controller_spec.rb @@ -24,8 +24,7 @@ it 'is connected to project_settings' do get :show, params: { namespace_id: project.namespace, project_id: project } - - expect(project.project_setting.push_rule).to eq(subject) + expect(project.reload.project_setting.push_rule).to eq(subject) end context 'unlicensed' do diff --git a/lib/api/entities/project.rb b/lib/api/entities/project.rb index 61feacd6586d57bcff9019988565041f04751553..d2cffa36ba0060432fa2248e4570176f778e12be 100644 --- a/lib/api/entities/project.rb +++ b/lib/api/entities/project.rb @@ -85,7 +85,9 @@ class Project < BasicProjectDetails expose(:infrastructure_access_level, documentation: { type: 'string', example: 'enabled' }) { |project, options| project_feature_string_access_level(project, :infrastructure) } expose(:monitor_access_level, documentation: { type: 'string', example: 'enabled' }) { |project, options| project_feature_string_access_level(project, :monitor) } - expose :emails_disabled, documentation: { type: 'boolean' } + expose(:emails_disabled, documentation: { type: 'boolean' }) { |project, options| project.emails_disabled? } + expose :emails_enabled, documentation: { type: 'boolean' } + expose :shared_runners_enabled, documentation: { type: 'boolean' } expose :lfs_enabled?, as: :lfs_enabled, documentation: { type: 'boolean' } expose :creator_id, documentation: { type: 'integer', example: 1 } diff --git a/lib/api/helpers/projects_helpers.rb b/lib/api/helpers/projects_helpers.rb index 642963768f8ddd819c6f5709fd5d1c49dc6ffe0b..07e5889a9f7e57ddc76bc9207433d1d6c0f9d16a 100644 --- a/lib/api/helpers/projects_helpers.rb +++ b/lib/api/helpers/projects_helpers.rb @@ -40,7 +40,8 @@ module ProjectsHelpers optional :infrastructure_access_level, type: String, values: %w(disabled private enabled), desc: 'Infrastructure access level. One of `disabled`, `private` or `enabled`' optional :monitor_access_level, type: String, values: %w(disabled private enabled), desc: 'Monitor access level. One of `disabled`, `private` or `enabled`' - optional :emails_disabled, type: Boolean, desc: 'Disable email notifications' + optional :emails_disabled, type: Boolean, desc: 'Deprecated: Use emails_enabled instead.' + optional :emails_enabled, type: Boolean, desc: 'Enable email notifications' optional :show_default_award_emojis, type: Boolean, desc: 'Show default award emojis' optional :show_diff_preview_in_email, type: Boolean, desc: 'Include the code diff preview in merge request notification emails' optional :warn_about_potentially_unwanted_characters, type: Boolean, desc: 'Warn about Potentially Unwanted Characters' @@ -144,7 +145,7 @@ def self.update_params_at_least_one_of :container_expiration_policy_attributes, :default_branch, :description, - :emails_disabled, + :emails_enabled, :forking_access_level, :issues_access_level, :lfs_enabled, diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 3bfce5e0273be5d17da7af3c0e13d05796e8214a..91763b9b108b525417caf2d1c9aed20d55995f5d 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -36303,21 +36303,24 @@ msgstr "" msgid "ProjectSettings|Determine what happens to the commit history when you merge a merge request." msgstr "" -msgid "ProjectSettings|Disable email notifications" -msgstr "" - msgid "ProjectSettings|Do not allow" msgstr "" msgid "ProjectSettings|Enable \"Delete source branch\" option by default" msgstr "" +msgid "ProjectSettings|Enable email notifications" +msgstr "" + msgid "ProjectSettings|Enable merge trains" msgstr "" msgid "ProjectSettings|Enable merged results pipelines" msgstr "" +msgid "ProjectSettings|Enable sending email notifications for this project" +msgstr "" + msgid "ProjectSettings|Enable suggested reviewers" msgstr "" @@ -36471,9 +36474,6 @@ msgstr "" msgid "ProjectSettings|Override instance analytics configuration for this project" msgstr "" -msgid "ProjectSettings|Override user notification preferences for all project members." -msgstr "" - msgid "ProjectSettings|Package registry" msgstr "" diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 46913cfa6496ca132e0692f1fccf091a19ba55f6..7d7bebb7106e234ff994c226fea4e4016a514610 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -974,7 +974,8 @@ def update_project(**parameters) project: { project_setting_attributes: { show_default_award_emojis: boolean_value, - enforce_auth_checks_on_uploads: boolean_value + enforce_auth_checks_on_uploads: boolean_value, + emails_enabled: boolean_value } } } @@ -983,6 +984,8 @@ def update_project(**parameters) expect(project.show_default_award_emojis?).to eq(result) expect(project.enforce_auth_checks_on_uploads?).to eq(result) + expect(project.emails_enabled?).to eq(result) + expect(project.emails_disabled?).to eq(!result) end end end diff --git a/spec/features/issues/user_toggles_subscription_spec.rb b/spec/features/issues/user_toggles_subscription_spec.rb index 00b04c10d3344671daec14fd745a1391d1be1c6a..0ddf49578a088c1359c7e41c2355d4e455db2c66 100644 --- a/spec/features/issues/user_toggles_subscription_spec.rb +++ b/spec/features/issues/user_toggles_subscription_spec.rb @@ -42,7 +42,7 @@ end context 'when project emails are disabled' do - let(:project) { create(:project_empty_repo, :public, emails_disabled: true) } + let_it_be(:project) { create(:project_empty_repo, :public, emails_enabled: false) } it 'is disabled' do expect(page).to have_content('Disabled by project owner') diff --git a/spec/features/profiles/user_visits_notifications_tab_spec.rb b/spec/features/profiles/user_visits_notifications_tab_spec.rb index 1295a0b6150aed98e85f484898220091acb4ae08..7d858e3c92c919c589b09c86aa9219e7569da5c9 100644 --- a/spec/features/profiles/user_visits_notifications_tab_spec.rb +++ b/spec/features/profiles/user_visits_notifications_tab_spec.rb @@ -30,7 +30,7 @@ end context 'when project emails are disabled' do - let(:project) { create(:project, emails_disabled: true) } + let_it_be(:project) { create(:project, emails_enabled: false) } it 'notification button is disabled' do expect(page).to have_selector('[data-testid="notification-dropdown"] .disabled') diff --git a/spec/features/projects/pipelines/pipelines_spec.rb b/spec/features/projects/pipelines/pipelines_spec.rb index 25eddf64f996035ca916463d598758339e101dab..f742ec3dfa6cbe82c40569fc82a46a623f68554a 100644 --- a/spec/features/projects/pipelines/pipelines_spec.rb +++ b/spec/features/projects/pipelines/pipelines_spec.rb @@ -818,7 +818,7 @@ def create_build(stage, stage_idx, name, status) describe 'when the `ios_specific_templates` experiment is enabled and the "Set up a runner" button is clicked' do before do stub_experiments(ios_specific_templates: :candidate) - create(:project_setting, project: project, target_platforms: %w(ios)) + project.project_setting.update!(target_platforms: %w(ios)) visit project_pipelines_path(project) click_button 'Set up a runner' end diff --git a/spec/features/projects/settings/visibility_settings_spec.rb b/spec/features/projects/settings/visibility_settings_spec.rb index 7d41b60199c7e9ba5038638d2be3ea5ee3ba6674..890f514d3dac95364afd2220ad70adbb3b93573a 100644 --- a/spec/features/projects/settings/visibility_settings_spec.rb +++ b/spec/features/projects/settings/visibility_settings_spec.rb @@ -30,11 +30,11 @@ context 'disable email notifications' do it 'is visible' do - expect(page).to have_selector('.js-emails-disabled', visible: true) + expect(page).to have_selector('.js-emails-enabled', visible: true) end it 'accepts the changed state' do - find('.js-emails-disabled input[type="checkbox"]').click + find('.js-emails-enabled input[type="checkbox"]').click expect { save_permissions_group }.to change { updated_emails_disabled? }.to(true) end @@ -59,7 +59,7 @@ context 'disable email notifications' do it 'is not available' do - expect(page).not_to have_selector('.js-emails-disabled', visible: true) + expect(page).not_to have_selector('.js-emails-enabled', visible: true) end end end diff --git a/spec/features/projects/show/user_manages_notifications_spec.rb b/spec/features/projects/show/user_manages_notifications_spec.rb index 455b931e7f3cd25228cd40dab6538e18b789813b..bbf31c1e1e157bae429d7d3af88c73a04f93fbf4 100644 --- a/spec/features/projects/show/user_manages_notifications_spec.rb +++ b/spec/features/projects/show/user_manages_notifications_spec.rb @@ -77,7 +77,7 @@ def click_notifications_button end context 'when project emails are disabled' do - let(:project) { create(:project, :public, :repository, emails_disabled: true) } + let_it_be(:project) { create(:project, :public, :repository, emails_enabled: false) } it 'is disabled' do visit project_path(project) diff --git a/spec/features/projects/user_changes_project_visibility_spec.rb b/spec/features/projects/user_changes_project_visibility_spec.rb index f27a659f65fc8cbc8955e133d9e34cb10bba88b5..24f24229f9c478875cabfc0107051c3682fb1d44 100644 --- a/spec/features/projects/user_changes_project_visibility_spec.rb +++ b/spec/features/projects/user_changes_project_visibility_spec.rb @@ -66,8 +66,8 @@ let(:project) { create(:project, :empty_repo, :public) } it 'saves without confirmation' do - expect(page).to have_selector('.js-emails-disabled', visible: true) - find('.js-emails-disabled input[type="checkbox"]').click + expect(page).to have_selector('.js-emails-enabled', visible: true) + find('.js-emails-enabled input[type="checkbox"]').click page.within('#js-shared-permissions') do click_button 'Save changes' diff --git a/spec/graphql/types/issue_type_spec.rb b/spec/graphql/types/issue_type_spec.rb index 7c4f2a061472b557c03a7dc5ed7462c45473e8e5..6c4e68fba6b655486721466368c6672f0c2fd7c4 100644 --- a/spec/graphql/types/issue_type_spec.rb +++ b/spec/graphql/types/issue_type_spec.rb @@ -19,7 +19,7 @@ it 'has specific fields' do fields = %i[id iid title description state reference author assignees updated_by participants labels milestone due_date confidential hidden discussion_locked upvotes downvotes merge_requests_count user_notes_count user_discussions_count web_path web_url relative_position - emails_disabled subscribed time_estimate total_time_spent human_time_estimate human_total_time_spent closed_at created_at updated_at task_completion_status + emails_disabled emails_enabled subscribed time_estimate total_time_spent human_time_estimate human_total_time_spent closed_at created_at updated_at task_completion_status design_collection alert_management_alert alert_management_alerts severity current_user_todos moved moved_to closed_as_duplicate_of create_note_email timelogs project_id customer_relations_contacts escalation_status] diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index 768038d8736e8ed15946c64ec6f5451d7e86de59..f450f46fefca85dfe865767a70c030d9498b4eb3 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -1006,7 +1006,7 @@ def license_name analyticsAccessLevel: project.project_feature.analytics_access_level, containerRegistryEnabled: !!project.container_registry_enabled, lfsEnabled: !!project.lfs_enabled, - emailsDisabled: project.emails_disabled?, + emailsEnabled: project.emails_enabled?, showDefaultAwardEmojis: project.show_default_award_emojis?, securityAndComplianceAccessLevel: project.security_and_compliance_access_level, containerRegistryAccessLevel: project.project_feature.container_registry_access_level, diff --git a/spec/lib/gitlab/pagination/keyset/in_operator_optimization/strategies/record_loader_strategy_spec.rb b/spec/lib/gitlab/pagination/keyset/in_operator_optimization/strategies/record_loader_strategy_spec.rb index 3fe858f33da396c811c0e9c906227990ef5963fb..ddaf555dae6d3376429a72c25f148be347eeaf3f 100644 --- a/spec/lib/gitlab/pagination/keyset/in_operator_optimization/strategies/record_loader_strategy_spec.rb +++ b/spec/lib/gitlab/pagination/keyset/in_operator_optimization/strategies/record_loader_strategy_spec.rb @@ -32,6 +32,12 @@ end end + let_it_be(:model_without_ignored_columns) do + Class.new(ApplicationRecord) do + self.table_name = 'projects' + end + end + subject(:strategy) { described_class.new(finder_query, model, order_by_columns) } describe '#initializer_columns' do @@ -70,6 +76,8 @@ describe '#final_projections' do context 'when model does not have ignored columns' do + let(:model) { model_without_ignored_columns } + it 'does not specify the selected column names' do expect(strategy.final_projections).to contain_exactly("(#{described_class::RECORDS_COLUMN}).*") end diff --git a/spec/models/project_setting_spec.rb b/spec/models/project_setting_spec.rb index 6928cc8ba0832127e94085201a81b2f171171fd4..ba7378138593cf37d825441ebdc64b0967be21c0 100644 --- a/spec/models/project_setting_spec.rb +++ b/spec/models/project_setting_spec.rb @@ -111,7 +111,7 @@ def valid_target_platform_combinations end describe '#show_diff_preview_in_email?' do - context 'when a project is a top-level namespace' do + context 'when a project has no parent group' do let(:project_settings) { create(:project_setting, show_diff_preview_in_email: false) } let(:project) { create(:project, project_setting: project_settings) } @@ -133,75 +133,75 @@ 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 + context 'when a parent group overrides project settings' 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) } + let(:group) { create(:group, namespace_settings: namespace_settings) } + let(:project) { create(:project, namespace_id: group.id, project_setting: project_settings) } - it "returns false when updating project settings" do - project.update_attribute(:emails_disabled, false) - expect(project.emails_enabled?).to be_truthy + context 'when show_diff_preview_in_email is disabled for the parent group' do + it 'returns false' do + expect(project).not_to be_show_diff_preview_in_email 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) + context 'when all ancestors have enabled diff previews' do + let(:namespace_settings) { create(:namespace_settings, show_diff_preview_in_email: true) } - expect(project.emails_enabled?).to be_falsey - end + it 'returns true' do + expect(project).to be_show_diff_preview_in_email end + end + end + end - context 'when emails are enabled in parent group' do - before do - allow(project.namespace).to receive(:emails_enabled?).and_return(true) - end + describe '#emails_enabled?' do + context "when a project does not have a parent group" do + let_it_be(:project_settings) { create(:project_setting, emails_enabled: true) } + let_it_be(:project) { create(:project, project_setting: project_settings) } - it 'returns true' do - expect(project.emails_enabled?).to be_truthy - 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) + it "returns false when project_settings are set to false" do + project.project_setting.clear_memoization(:emails_enabled?) + project.update_attribute(:emails_enabled, false) - expect(project.emails_enabled?).to be_falsey - end - end + expect(project.emails_enabled?).to be_falsey 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) } + 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) { create(:project, namespace_id: group.id, project_setting: project_settings) } + let(:project) do + create(:project, namespace_id: group.id, + project_setting: project_settings) + end - context 'when show_diff_preview_in_email is disabled for the parent group' do + context 'when emails have been disabled in parent group' do it 'returns false' do - expect(project).not_to be_show_diff_preview_in_email + group.update_attribute(:emails_disabled, true) + + expect(project.emails_enabled?).to be_falsey end end - context 'when all ancestors have enabled diff previews' do - let(:namespace_settings) { create(:namespace_settings, show_diff_preview_in_email: true) } + context 'when emails are enabled in parent group' do + before do + allow(project.namespace).to receive(:emails_disabled?).and_return(false) + end it 'returns true' do - group.update_attribute(:show_diff_preview_in_email, true) + expect(project.emails_enabled?).to be_truthy + end - expect(project).to be_show_diff_preview_in_email + it 'returns false when disabled at the project' do + project.update_attribute(:emails_enabled, false) + + expect(project.emails_enabled?).to be_falsey end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 20b78eec17abf5dbbd73bf04820503fe45652b77..ad71b38aa1c6647fe79a35d5d5596ba72531f297 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -3952,51 +3952,10 @@ def has_external_wiki end describe '#emails_disabled?' do - let_it_be(:namespace) { create(:namespace) } - - let(:project) { build(:project, namespace: namespace, emails_disabled: false) } - - context 'emails disabled in group' do - it 'returns true' do - allow(project.namespace).to receive(:emails_disabled?) { true } - - expect(project.emails_disabled?).to be_truthy - end - end - - context 'emails enabled in group' do - before do - allow(project.namespace).to receive(:emails_disabled?) { false } - end - - it 'returns false' do - expect(project.emails_disabled?).to be_falsey - end + let(:project) { build(:project, emails_enabled: true) } - it 'returns true' do - project.update_attribute(:emails_disabled, true) - - expect(project.emails_disabled?).to be_truthy - end - end - end - - describe '#emails_enabled?' do - 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 - 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 + it "is the opposite of emails_disabled" do + expect(project.emails_disabled?).to be_falsey end end diff --git a/spec/requests/api/project_attributes.yml b/spec/requests/api/project_attributes.yml index 86ff739da7e11d4f6710e56a06b93edf49eec62e..d2480c57e11a4c190f3f7f5ed3706a104fbeab04 100644 --- a/spec/requests/api/project_attributes.yml +++ b/spec/requests/api/project_attributes.yml @@ -60,6 +60,7 @@ itself: # project - container_registry_image_prefix - default_branch - empty_repo + - emails_disabled - forks_count - http_url_to_repo - import_status @@ -163,7 +164,6 @@ project_setting: - jitsu_key - mirror_branch_regex - allow_pipeline_trigger_approve_deployment - - emails_enabled - pages_unique_domain_enabled - pages_unique_domain - runner_registration_enabled diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 1ff2adfcbcfa1cc9c509a3f49e4ab6696b2b10fb..3f114e1fbab85c22734544f4e703f6c0dce6ef7f 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -2583,7 +2583,6 @@ def request link = create(:project_group_link, project: project, group: group) get api(path, admin, admin_mode: true) - expect(response).to have_gitlab_http_status(:ok) expect(json_response['id']).to eq(project.id) expect(json_response['description']).to eq(project.description) @@ -2634,6 +2633,8 @@ def request expect(json_response['feature_flags_access_level']).to be_present expect(json_response['infrastructure_access_level']).to be_present expect(json_response['monitor_access_level']).to be_present + expect(json_response).to have_key('emails_disabled') + expect(json_response).to have_key('emails_enabled') end it 'exposes all necessary attributes' do @@ -2707,7 +2708,6 @@ def failure_message(diff) expect(json_response['feature_flags_access_level']).to be_present expect(json_response['infrastructure_access_level']).to be_present expect(json_response['monitor_access_level']).to be_present - expect(json_response).to have_key('emails_disabled') expect(json_response['resolve_outdated_diff_discussions']).to eq(project.resolve_outdated_diff_discussions) expect(json_response['remove_source_branch_after_merge']).to be_truthy expect(json_response['container_registry_enabled']).to be_present @@ -3938,7 +3938,7 @@ def failure_message(diff) end it 'updates emails_disabled' do - project_param = { emails_disabled: true } + project_param = { emails_enabled: false } put api("/projects/#{project3.id}", user), params: project_param @@ -3947,6 +3947,16 @@ def failure_message(diff) expect(json_response['emails_disabled']).to eq(true) end + it 'updates emails_enabled?' do + project_param = { emails_enabled: false } + + put api("/projects/#{project3.id}", user), params: project_param + + expect(response).to have_gitlab_http_status(:ok) + + expect(json_response['emails_enabled']).to eq(false) + end + it 'updates build_git_strategy' do project_param = { build_git_strategy: 'clone' } diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 1f6408b28ed5512af82081b20881c22c73232828..2186e7aa06cf22f2be63c46401643a2c57b164e8 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -504,19 +504,19 @@ end end - context 'when updating #emails_disabled' do + context 'when updating #emails_enabled' do it 'updates the attribute for the project owner' do - expect { update_project(project, user, emails_disabled: true) } - .to change { project.emails_disabled } - .to(true) + expect { update_project(project, user, emails_enabled: false) } + .to change { project.emails_enabled } + .to(false) end it 'does not update when not project owner' do maintainer = create(:user) project.add_member(maintainer, :maintainer) - expect { update_project(project, maintainer, emails_disabled: true) } - .not_to change { project.emails_disabled } + expect { update_project(project, maintainer, emails_enabled: false) } + .not_to change { project.emails_enabled } end end diff --git a/spec/support/shared_examples/features/sidebar_shared_examples.rb b/spec/support/shared_examples/features/sidebar_shared_examples.rb index c2c50e8762fff7aa3f8b19a9dd27f633570fd04f..f402a1bc91a46c92216a807eb53912121691eebb 100644 --- a/spec/support/shared_examples/features/sidebar_shared_examples.rb +++ b/spec/support/shared_examples/features/sidebar_shared_examples.rb @@ -100,7 +100,7 @@ context 'when notifications have been disabled' do before do - project.update_attribute(:emails_disabled, true) + project.update_attribute(:emails_enabled, false) refresh_and_click_first_card end diff --git a/spec/support/shared_examples/services/notification_service_shared_examples.rb b/spec/support/shared_examples/services/notification_service_shared_examples.rb index cfd674e3c43e1ba55a1563f18affec22f11afdfb..df1ae67a590da5161f011f1e068bcf779fcc7af1 100644 --- a/spec/support/shared_examples/services/notification_service_shared_examples.rb +++ b/spec/support/shared_examples/services/notification_service_shared_examples.rb @@ -8,16 +8,16 @@ before do reset_delivered_emails! - target_project.clear_memoization(:emails_disabled) + target_project.project_setting.clear_memoization(:emails_enabled?) end it 'sends no emails with project emails disabled' do - target_project.update_attribute(:emails_disabled, true) + target_project.project_setting.update_attribute(:emails_enabled, false) notification_trigger if check_delivery_jobs_queue - # Only check enqueud jobs, not delivered emails + # Only check enqueued jobs, not delivered emails expect_no_delivery_jobs else # Deprecated: Check actual delivered emails @@ -26,12 +26,12 @@ end it 'sends emails to someone' do - target_project.update_attribute(:emails_disabled, false) + target_project.project_setting.update_attribute(:emails_enabled, true) notification_trigger if check_delivery_jobs_queue - # Only check enqueud jobs, not delivered emails + # Only check enqueued jobs, not delivered emails expect_any_delivery_jobs else # Deprecated: Check actual delivered emails