From cf3286a93a1d94d1e992bc1614a6c298766f3275 Mon Sep 17 00:00:00 2001 From: Marco Zille Date: Thu, 5 Jan 2023 15:41:25 +0100 Subject: [PATCH 1/6] Added service and mutation to set user namespace commit email Changelog: added --- .../users/set_namespace_commit_email.rb | 68 +++++++++++++++ app/graphql/types/mutation_type.rb | 1 + app/models/user.rb | 6 ++ app/policies/user_policy.rb | 1 + .../set_namespace_commit_email_service.rb | 87 +++++++++++++++++++ 5 files changed, 163 insertions(+) create mode 100644 app/graphql/mutations/users/set_namespace_commit_email.rb create mode 100644 app/services/users/set_namespace_commit_email_service.rb diff --git a/app/graphql/mutations/users/set_namespace_commit_email.rb b/app/graphql/mutations/users/set_namespace_commit_email.rb new file mode 100644 index 00000000000000..ddcfad8186bf42 --- /dev/null +++ b/app/graphql/mutations/users/set_namespace_commit_email.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +module Mutations + module Users + class SetNamespaceCommitEmail < BaseMutation + graphql_name 'UserSetNamespaceCommitEmail' + + argument :user_id, + ::Types::GlobalIDType[::User], + required: false, + description: 'Id of the user to set the namespace commit email for (defaults to current user).' + + argument :namespace_id, + ::Types::GlobalIDType[::Namespace], + required: true, + description: 'Id of the namespace to set the namespace commit email for.' + + argument :email_id, + ::Types::GlobalIDType[::Email], + required: false, + description: 'Id of the email to set.' + + # field :namespace_commit_email, + # Types::Users::NamespaceCommitEmailType, + # null: true, + # description: 'User namespace commit email after mutation.' + + def resolve(user_id:, namespace_id:, **args) + user = find_user(user_id) + + # if !user.nil? && !current_user.can?(:read_user_email_address, user) + # raise_resource_not_available_error! + # end + + namespace = find_namespace(namespace_id) + + if namespace.nil? + raise_resource_not_available_error! + end + + email = find_email(args[:email_id]) if args[:email_id].present? + + result = ::Users::SetNamespaceCommitEmailService.new( + current_user, namespace, email, { user: user } + ).execute + + { + # namespace_commit_email: result.payload[:namespace_commit_email], + errors: result.errors + } + end + + private + + def find_namespace(id) + GitlabSchema.object_from_id(id, expected_type: [::Namespace]).sync + end + + def find_email(id) + GitlabSchema.object_from_id(id, expected_type: [::Email]).sync + end + + def find_user(id) + GitlabSchema.object_from_id(id, expected_type: [::User]).sync + end + end + end +end diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index efc7bf89693e2e..c3e44eb65e7fa8 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -181,6 +181,7 @@ class MutationType < BaseObject mount_mutation Mutations::Pages::MarkOnboardingComplete mount_mutation Mutations::SavedReplies::Destroy mount_mutation Mutations::Uploads::Delete + mount_mutation Mutations::Users::SetNamespaceCommitEmail end end diff --git a/app/models/user.rb b/app/models/user.rb index c5c0353e6f438f..76b2bde0f22cc7 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -2298,6 +2298,12 @@ def abuse_metadata } end + def namespace_commit_email_for_namespace(namespace) + return if namespace.nil? + + namespace_commit_emails.find_by(namespace: namespace) + end + protected # override, from Devise::Validatable diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index 1078eda38e7e11..2fd198b8cf4593 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -31,6 +31,7 @@ class UserPolicy < BasePolicy enable :read_user_groups enable :read_saved_replies enable :read_user_email_address + enable :admin_user_email_address end rule { default }.enable :read_user_profile diff --git a/app/services/users/set_namespace_commit_email_service.rb b/app/services/users/set_namespace_commit_email_service.rb new file mode 100644 index 00000000000000..c980e41344b5a1 --- /dev/null +++ b/app/services/users/set_namespace_commit_email_service.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +module Users + class SetNamespaceCommitEmailService + include Gitlab::Allowable + + attr_reader :current_user, :target_user, :namespace, :email + + def initialize(current_user, namespace, email, params) + @current_user = current_user + @target_user = params.delete(:user) || current_user + @namespace = namespace + @email = email + end + + def execute + unless can?(current_user, :admin_user_email_address, target_user) + return error(_("User doesn't exist or you don't have permission to change namespace commit emails.")) + end + + if namespace.nil? + return error(_("Namespace must be provided.")) + end + + existing_namespace_commit_email = target_user.namespace_commit_email_for_namespace(namespace) + + if existing_namespace_commit_email.nil? + create_namespace_commit_email + elsif email.nil? + remove_namespace_commit_email(existing_namespace_commit_email) + else + update_namespace_comit_email(existing_namespace_commit_email) + end + end + + private + + def remove_namespace_commit_email(namespace_commit_email) + namespace_commit_email.destroy + success(namespace_commit_email) + end + + def create_namespace_commit_email + if email.nil? + return error(_("Email must be provided.")) + end + + namespace_commit_email = ::Users::NamespaceCommitEmail.new( + user_id: target_user.id, + namespace_id: namespace.id, + email_id: email.id + ) + + if !namespace_commit_email.save + error_in_save(namespace_commit_email) + else + success(namespace_commit_email) + end + end + + def update_namespace_comit_email(namespace_commit_email) + namespace_commit_email.email_id = email.id + + if !namespace_commit_email.save + error_in_save(namespace_commit_email) + else + success(namespace_commit_email) + end + end + + def success(namespace_commit_email) + ServiceResponse.success(payload: { + namespace_commit_email: namespace_commit_email + }) + end + + def error(message) + ServiceResponse.error(message: message) + end + + def error_in_save(namespace_commit_email) + return error(_("Failed to save namespace commit email")) if namespace_commit_email.errors.empty? + + error(namespace_commit_email.errors.full_messages.to_sentence) + end + end +end -- GitLab From 8ba7060a76350aa256f7a9bb58ffda58f128ac4d Mon Sep 17 00:00:00 2001 From: Lee Tickett Date: Tue, 24 Jan 2023 15:47:29 +0000 Subject: [PATCH 2/6] Writing ruby specs --- .../users/set_namespace_commit_email.rb | 47 ++---- .../set_namespace_commit_email_service.rb | 38 ++--- doc/api/graphql/reference/index.md | 26 ++++ .../users/set_namespace_commit_email_spec.rb | 51 +++++++ spec/policies/user_policy_spec.rb | 4 + .../users/set_namespace_commit_email_spec.rb | 75 ++++++++++ ...set_namespace_commit_email_service_spec.rb | 139 ++++++++++++++++++ 7 files changed, 326 insertions(+), 54 deletions(-) create mode 100644 spec/graphql/mutations/users/set_namespace_commit_email_spec.rb create mode 100644 spec/requests/api/graphql/users/set_namespace_commit_email_spec.rb create mode 100644 spec/services/users/set_namespace_commit_email_service_spec.rb diff --git a/app/graphql/mutations/users/set_namespace_commit_email.rb b/app/graphql/mutations/users/set_namespace_commit_email.rb index ddcfad8186bf42..bf5d17fa99bb68 100644 --- a/app/graphql/mutations/users/set_namespace_commit_email.rb +++ b/app/graphql/mutations/users/set_namespace_commit_email.rb @@ -5,11 +5,6 @@ module Users class SetNamespaceCommitEmail < BaseMutation graphql_name 'UserSetNamespaceCommitEmail' - argument :user_id, - ::Types::GlobalIDType[::User], - required: false, - description: 'Id of the user to set the namespace commit email for (defaults to current user).' - argument :namespace_id, ::Types::GlobalIDType[::Namespace], required: true, @@ -20,49 +15,31 @@ class SetNamespaceCommitEmail < BaseMutation required: false, description: 'Id of the email to set.' - # field :namespace_commit_email, - # Types::Users::NamespaceCommitEmailType, - # null: true, - # description: 'User namespace commit email after mutation.' - - def resolve(user_id:, namespace_id:, **args) - user = find_user(user_id) - - # if !user.nil? && !current_user.can?(:read_user_email_address, user) - # raise_resource_not_available_error! - # end - - namespace = find_namespace(namespace_id) + field :namespace_commit_email, + Types::Users::NamespaceCommitEmailType, + null: true, + description: 'User namespace commit email after mutation.' - if namespace.nil? - raise_resource_not_available_error! - end + authorize :read_namespace - email = find_email(args[:email_id]) if args[:email_id].present? + def resolve(args) + raise RecordNotFoundException unless current_user - result = ::Users::SetNamespaceCommitEmailService.new( - current_user, namespace, email, { user: user } - ).execute + namespace = authorized_find!(args[:namespace_id]) + args[:email_id] = args[:email_id].model_id + result = ::Users::SetNamespaceCommitEmailService.new(current_user, namespace, args[:email_id], {}).execute { - # namespace_commit_email: result.payload[:namespace_commit_email], + namespace_commit_email: result.payload[:namespace_commit_email], errors: result.errors } end private - def find_namespace(id) + def find_object(id) GitlabSchema.object_from_id(id, expected_type: [::Namespace]).sync end - - def find_email(id) - GitlabSchema.object_from_id(id, expected_type: [::Email]).sync - end - - def find_user(id) - GitlabSchema.object_from_id(id, expected_type: [::User]).sync - end end end end diff --git a/app/services/users/set_namespace_commit_email_service.rb b/app/services/users/set_namespace_commit_email_service.rb index c980e41344b5a1..b6d7d430688186 100644 --- a/app/services/users/set_namespace_commit_email_service.rb +++ b/app/services/users/set_namespace_commit_email_service.rb @@ -4,13 +4,13 @@ module Users class SetNamespaceCommitEmailService include Gitlab::Allowable - attr_reader :current_user, :target_user, :namespace, :email + attr_reader :current_user, :target_user, :namespace, :email_id - def initialize(current_user, namespace, email, params) + def initialize(current_user, namespace, email_id, params) @current_user = current_user @target_user = params.delete(:user) || current_user @namespace = namespace - @email = email + @email_id = email_id end def execute @@ -18,18 +18,20 @@ def execute return error(_("User doesn't exist or you don't have permission to change namespace commit emails.")) end - if namespace.nil? - return error(_("Namespace must be provided.")) + return error(_("Namespace must be provided.")) if namespace.nil? + + unless can?(target_user, :read_namespace, namespace) + return error(_("Namespace doesn't exist or you don't have permission.")) end + email = target_user.emails.find_by(id: email_id) unless email_id.nil? # rubocop: disable CodeReuse/ActiveRecord existing_namespace_commit_email = target_user.namespace_commit_email_for_namespace(namespace) - if existing_namespace_commit_email.nil? - create_namespace_commit_email - elsif email.nil? + create_namespace_commit_email(email) + elsif email_id.nil? remove_namespace_commit_email(existing_namespace_commit_email) else - update_namespace_comit_email(existing_namespace_commit_email) + update_namespace_comit_email(existing_namespace_commit_email, email) end end @@ -37,18 +39,16 @@ def execute def remove_namespace_commit_email(namespace_commit_email) namespace_commit_email.destroy - success(namespace_commit_email) + success(nil) end - def create_namespace_commit_email - if email.nil? - return error(_("Email must be provided.")) - end + def create_namespace_commit_email(email) + return error(_("Email must be provided.")) if email.nil? namespace_commit_email = ::Users::NamespaceCommitEmail.new( - user_id: target_user.id, - namespace_id: namespace.id, - email_id: email.id + user: target_user, + namespace: namespace, + email: email ) if !namespace_commit_email.save @@ -58,8 +58,8 @@ def create_namespace_commit_email end end - def update_namespace_comit_email(namespace_commit_email) - namespace_commit_email.email_id = email.id + def update_namespace_comit_email(namespace_commit_email, email) + namespace_commit_email.email = email if !namespace_commit_email.save error_in_save(namespace_commit_email) diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 7661f2ec9a625a..b9cf4e9fc79942 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -6736,6 +6736,26 @@ Input type: `UserPreferencesUpdateInput` | `errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | | `userPreferences` | [`UserPreferences`](#userpreferences) | User preferences after mutation. | +### `Mutation.userSetNamespaceCommitEmail` + +Input type: `UserSetNamespaceCommitEmailInput` + +#### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `emailId` | [`EmailID`](#emailid) | Id of the email to set. | +| `namespaceId` | [`NamespaceID!`](#namespaceid) | Id of the namespace to set the namespace commit email for. | + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | +| `namespaceCommitEmail` | [`NamespaceCommitEmail`](#namespacecommitemail) | User namespace commit email after mutation. | + ### `Mutation.vulnerabilityConfirm` Input type: `VulnerabilityConfirmInput` @@ -26890,6 +26910,12 @@ Duration between two instants, represented as a fractional number of seconds. For example: 12.3334. +### `EmailID` + +A `EmailID` is a global ID. It is encoded as a string. + +An example `EmailID` is: `"gid://gitlab/Email/1"`. + ### `EnvironmentID` A `EnvironmentID` is a global ID. It is encoded as a string. diff --git a/spec/graphql/mutations/users/set_namespace_commit_email_spec.rb b/spec/graphql/mutations/users/set_namespace_commit_email_spec.rb new file mode 100644 index 00000000000000..1b24ee3a6160ca --- /dev/null +++ b/spec/graphql/mutations/users/set_namespace_commit_email_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Mutations::Users::SetNamespaceCommitEmail, feature_category: :user_profile do + include GraphqlHelpers + + let(:current_user) { create(:user) } + let(:group) { create(:group) } + let(:email) { create(:email, user: current_user) } + let(:input) { {} } + let(:namespace_id) { group.to_global_id } + let(:email_id) { email.to_global_id } + + describe '#resolve' do + subject(:resolve_mutation) do + described_class.new(object: nil, context: { current_user: current_user }, field: nil).resolve( + namespace_id: namespace_id, + email_id: email_id + ) + end + + context 'when current_user does not have permission' do + it 'raises an error' do + expect { resolve_mutation }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + .with_message(Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR) + end + end + + context 'when the user has permission' do + before do + group.add_reporter(current_user) + end + + context 'when the params are invalid' do + let(:email_id) { create(:email).to_global_id } + + it 'returns the validation error' do + expect(resolve_mutation[:errors]).to contain_exactly("Email must be provided.") + end + end + + it 'creates namespace commit email with correct values' do + expect(resolve_mutation[:namespace_commit_email]) + .to have_attributes({ namespace_id: namespace_id.model_id.to_i, email_id: email_id.model_id.to_i }) + end + end + end + + specify { expect(described_class).to require_graphql_authorizations(:read_namespace) } +end diff --git a/spec/policies/user_policy_spec.rb b/spec/policies/user_policy_spec.rb index 94b7e2951671f6..9a2caeb74356f7 100644 --- a/spec/policies/user_policy_spec.rb +++ b/spec/policies/user_policy_spec.rb @@ -253,10 +253,12 @@ context 'when admin mode is enabled', :enable_admin_mode do it { is_expected.to be_allowed(:read_user_email_address) } + it { is_expected.to be_allowed(:admin_user_email_address) } end context 'when admin mode is disabled' do it { is_expected.not_to be_allowed(:read_user_email_address) } + it { is_expected.not_to be_allowed(:admin_user_email_address) } end end @@ -265,10 +267,12 @@ subject { described_class.new(current_user, current_user) } it { is_expected.to be_allowed(:read_user_email_address) } + it { is_expected.to be_allowed(:admin_user_email_address) } end context "requesting a different user's" do it { is_expected.not_to be_allowed(:read_user_email_address) } + it { is_expected.not_to be_allowed(:admin_user_email_address) } end end end diff --git a/spec/requests/api/graphql/users/set_namespace_commit_email_spec.rb b/spec/requests/api/graphql/users/set_namespace_commit_email_spec.rb new file mode 100644 index 00000000000000..a32624dfaab5ad --- /dev/null +++ b/spec/requests/api/graphql/users/set_namespace_commit_email_spec.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Setting namespace commit email', feature_category: :user_profile do + include GraphqlHelpers + + let(:current_user) { create(:user) } + let(:group) { create(:group) } + let(:email) { create(:email, user: current_user) } + let(:input) { {} } + let(:namespace_id) { group.to_global_id } + let(:email_id) { email.to_global_id } + + let(:mutation) do + variables = { + namespace_id: namespace_id, + email_id: email_id + } + graphql_mutation(:user_set_namespace_commit_email, variables.merge(input), + <<-QL.strip_heredoc + namespaceCommitEmail { + email { + id + } + } + errors + QL + ) + end + + def mutation_response + graphql_mutation_response(:user_set_namespace_commit_email) + end + + before do + group.add_reporter(current_user) + end + + context 'when current_user is nil' do + it 'returns a top level error' do + post_graphql_mutation(mutation, current_user: nil) + + expect(graphql_errors).not_to be_empty + end + end + + context 'when the user cannot access the namespace' do + let(:namespace_id) { create(:group).to_global_id } + + it 'returns a top level error' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(graphql_errors).not_to be_empty + end + end + + context 'when the service returns an error' do + let(:email_id) { create(:email).to_global_id } + + it 'returns an error' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['errors']).to contain_exactly("Email must be provided.") + expect(mutation_response['namespaceCommitEmail']).to be_nil + end + end + + it 'creates a namespace commit email' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response.dig('namespaceCommitEmail', 'email', 'id')).to eq(email.to_global_id.to_s) + end +end diff --git a/spec/services/users/set_namespace_commit_email_service_spec.rb b/spec/services/users/set_namespace_commit_email_service_spec.rb new file mode 100644 index 00000000000000..8ac3635a922038 --- /dev/null +++ b/spec/services/users/set_namespace_commit_email_service_spec.rb @@ -0,0 +1,139 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Users::SetNamespaceCommitEmailService, feature_category: :user_profile do + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:email) { create(:email, user: user) } + + let(:namespace) { group } + let(:current_user) { user } + let(:target_user) { user } + let(:email_id) { email.id } + let(:params) { { user: target_user } } + let(:service) { described_class.new(current_user, namespace, email_id, params) } + + before_all do + group.add_reporter(user) + end + + describe "#execute" do + context "when current_user is not provided" do + let(:current_user) {} + + it "returns error message" do + expect(service.execute.message) + .to eq("User doesn't exist or you don't have permission to change namespace commit emails.") + end + end + + context "when current_user does not have permission to change namespace commit emails" do + let(:target_user) { create(:user) } + + it "returns error message" do + expect(service.execute.message) + .to eq("User doesn't exist or you don't have permission to change namespace commit emails.") + end + end + + context "when target_user does not have permission to access the namespace" do + let(:namespace) { create(:group) } + + it "returns error message" do + expect(service.execute.message).to eq("Namespace doesn't exist or you don't have permission.") + end + end + + context "when namespace is not provided" do + let(:namespace) { nil } + + it "returns error message" do + expect(service.execute.message).to eq("Namespace must be provided.") + end + end + + context "when email_id is not provided" do + let(:email_id) { nil } + + it "returns error message" do + expect(service.execute.message).to eq("Email must be provided.") + end + end + + context "when namespace commit email does not exist" do + it "creates namespace commit email" do + result = service.execute + + expect(result.payload[:namespace_commit_email]).to be_a(Users::NamespaceCommitEmail) + expect(result.payload[:namespace_commit_email]).to be_persisted + end + + context "when target user is not current user" do + context "when current user is an admin" do + let(:current_user) { create(:user, :admin) } + + context "when admin mode is enabled", :enable_admin_mode do + it "creates namespace commit email" do + result = service.execute + + expect(result.payload[:namespace_commit_email]).to be_a(Users::NamespaceCommitEmail) + expect(result.payload[:namespace_commit_email]).to be_persisted + end + end + + context "when admin mode is not enabled" do + let(:current_user) { create(:user) } + + it "returns error message" do + expect(service.execute.message) + .to eq("User doesn't exist or you don't have permission to change namespace commit emails.") + end + end + end + + context "when current user is not an admin" do + let(:current_user) { create(:user) } + + it "returns error message" do + expect(service.execute.message) + .to eq("User doesn't exist or you don't have permission to change namespace commit emails.") + end + end + end + end + + context "when namespace commit email already exists" do + let!(:existing_namespace_commit_email) do + create(:namespace_commit_email, + user: target_user, + namespace: namespace, + mail: create(:email, user: target_user)) + end + + context "when email_id is not provided" do + let(:email_id) { nil } + + it "destroys the namespace commit email" do + result = service.execute + + expect(result.message).to be_nil + expect(result.payload[:namespace_commit_email]).to be_nil + end + end + + context "and email_id is provided" do + let(:email_id) { create(:email, user: current_user).id } + + it "updates namespace commit email" do + result = service.execute + + existing_namespace_commit_email.reload + + expect(result.payload[:namespace_commit_email]).to eq(existing_namespace_commit_email) + expect(existing_namespace_commit_email.email_id).to eq(email_id) + end + end + end + end +end -- GitLab From a0c1b5feb3f1bf908fbfb3d93ca9611059be265c Mon Sep 17 00:00:00 2001 From: Lee Tickett Date: Thu, 8 Jun 2023 15:33:24 +0100 Subject: [PATCH 3/6] Apply reviewer suggestions --- .../users/set_namespace_commit_email.rb | 23 +++-- doc/api/graphql/reference/index.md | 1 - locale/gitlab.pot | 15 +++ .../users/set_namespace_commit_email_spec.rb | 30 +++++- .../users/set_namespace_commit_email_spec.rb | 67 +++++++++---- ...set_namespace_commit_email_service_spec.rb | 98 ++++++++++++------- 6 files changed, 163 insertions(+), 71 deletions(-) diff --git a/app/graphql/mutations/users/set_namespace_commit_email.rb b/app/graphql/mutations/users/set_namespace_commit_email.rb index bf5d17fa99bb68..cddfd3ccc41797 100644 --- a/app/graphql/mutations/users/set_namespace_commit_email.rb +++ b/app/graphql/mutations/users/set_namespace_commit_email.rb @@ -6,25 +6,23 @@ class SetNamespaceCommitEmail < BaseMutation graphql_name 'UserSetNamespaceCommitEmail' argument :namespace_id, - ::Types::GlobalIDType[::Namespace], - required: true, - description: 'Id of the namespace to set the namespace commit email for.' + ::Types::GlobalIDType[::Namespace], + required: true, + description: 'Id of the namespace to set the namespace commit email for.' argument :email_id, - ::Types::GlobalIDType[::Email], - required: false, - description: 'Id of the email to set.' + ::Types::GlobalIDType[::Email], + required: false, + description: 'Id of the email to set.' field :namespace_commit_email, - Types::Users::NamespaceCommitEmailType, - null: true, - description: 'User namespace commit email after mutation.' + Types::Users::NamespaceCommitEmailType, + null: true, + description: 'User namespace commit email after mutation.' authorize :read_namespace def resolve(args) - raise RecordNotFoundException unless current_user - namespace = authorized_find!(args[:namespace_id]) args[:email_id] = args[:email_id].model_id @@ -38,7 +36,8 @@ def resolve(args) private def find_object(id) - GitlabSchema.object_from_id(id, expected_type: [::Namespace]).sync + GitlabSchema.object_from_id( + id, expected_type: [::Namespace, ::Namespaces::UserNamespace, ::Namespaces::ProjectNamespace]).sync end end end diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index b9cf4e9fc79942..7ae47532488864 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -26096,7 +26096,6 @@ State of a Sentry error. | `EWM_SERVICE` | EwmService type. | | `EXTERNAL_WIKI_SERVICE` | ExternalWikiService type. | | `GITHUB_SERVICE` | GithubService type. | -| `GITLAB_SLACK_APPLICATION_SERVICE` | GitlabSlackApplicationService type. | | `GOOGLE_PLAY_SERVICE` | GooglePlayService type. | | `HANGOUTS_CHAT_SERVICE` | HangoutsChatService type. | | `HARBOR_SERVICE` | HarborService type. | diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 313e37cb81a91b..86b83b45255750 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -16769,6 +16769,9 @@ msgstr "" msgid "Email display name" msgstr "" +msgid "Email must be provided." +msgstr "" + msgid "Email not verified. Please verify your email in Salesforce." msgstr "" @@ -18773,6 +18776,9 @@ msgstr "" msgid "Failed to save merge conflicts resolutions. Please try again!" msgstr "" +msgid "Failed to save namespace commit email" +msgstr "" + msgid "Failed to save new settings" msgstr "" @@ -29669,6 +29675,12 @@ msgstr "" msgid "Namespace Limits" msgstr "" +msgid "Namespace doesn't exist or you don't have permission." +msgstr "" + +msgid "Namespace must be provided." +msgstr "" + msgid "Namespace or group to import repository into does not exist." msgstr "" @@ -49310,6 +49322,9 @@ msgstr "" msgid "User does not have permission to create a Security Policy project." msgstr "" +msgid "User doesn't exist or you don't have permission to change namespace commit emails." +msgstr "" + msgid "User has already been deactivated" msgstr "" diff --git a/spec/graphql/mutations/users/set_namespace_commit_email_spec.rb b/spec/graphql/mutations/users/set_namespace_commit_email_spec.rb index 1b24ee3a6160ca..cef8702a8e0274 100644 --- a/spec/graphql/mutations/users/set_namespace_commit_email_spec.rb +++ b/spec/graphql/mutations/users/set_namespace_commit_email_spec.rb @@ -12,6 +12,13 @@ let(:namespace_id) { group.to_global_id } let(:email_id) { email.to_global_id } + shared_examples 'success' do + it 'creates namespace commit email with correct values' do + expect(resolve_mutation[:namespace_commit_email]) + .to have_attributes({ namespace_id: namespace_id.model_id.to_i, email_id: email_id.model_id.to_i }) + end + end + describe '#resolve' do subject(:resolve_mutation) do described_class.new(object: nil, context: { current_user: current_user }, field: nil).resolve( @@ -40,9 +47,26 @@ end end - it 'creates namespace commit email with correct values' do - expect(resolve_mutation[:namespace_commit_email]) - .to have_attributes({ namespace_id: namespace_id.model_id.to_i, email_id: email_id.model_id.to_i }) + context 'when namespace is a group' do + it_behaves_like 'success' + end + + context 'when namespace is a user' do + let(:namespace_id) { current_user.namespace.to_global_id } + + it_behaves_like 'success' + end + + context 'when namespace is a project' do + let_it_be(:project) { create(:project) } + + let(:namespace_id) { project.project_namespace.to_global_id } + + before do + project.add_reporter(current_user) + end + + it_behaves_like 'success' end end end diff --git a/spec/requests/api/graphql/users/set_namespace_commit_email_spec.rb b/spec/requests/api/graphql/users/set_namespace_commit_email_spec.rb index a32624dfaab5ad..1db6f83ce4fe55 100644 --- a/spec/requests/api/graphql/users/set_namespace_commit_email_spec.rb +++ b/spec/requests/api/graphql/users/set_namespace_commit_email_spec.rb @@ -6,26 +6,30 @@ include GraphqlHelpers let(:current_user) { create(:user) } - let(:group) { create(:group) } - let(:email) { create(:email, user: current_user) } + let(:group) { create(:group, :public) } + let(:email) { create(:email, :confirmed, user: current_user) } let(:input) { {} } let(:namespace_id) { group.to_global_id } let(:email_id) { email.to_global_id } + let(:resource_or_permission_error) do + "The resource that you are attempting to access does not exist or you don't have permission to perform this action" + end + let(:mutation) do variables = { namespace_id: namespace_id, email_id: email_id } graphql_mutation(:user_set_namespace_commit_email, variables.merge(input), - <<-QL.strip_heredoc - namespaceCommitEmail { - email { - id - } - } - errors - QL + <<-QL.strip_heredoc + namespaceCommitEmail { + email { + id + } + } + errors + QL ) end @@ -33,32 +37,44 @@ def mutation_response graphql_mutation_response(:user_set_namespace_commit_email) end + shared_examples 'success' do + it 'creates a namespace commit email' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response.dig('namespaceCommitEmail', 'email', 'id')).to eq(email.to_global_id.to_s) + expect(graphql_errors).to be_nil + end + end + before do group.add_reporter(current_user) end context 'when current_user is nil' do - it 'returns a top level error' do + it 'returns the top level error' do post_graphql_mutation(mutation, current_user: nil) - expect(graphql_errors).not_to be_empty + expect(graphql_errors.first).to match a_hash_including( + 'message' => resource_or_permission_error) end end context 'when the user cannot access the namespace' do let(:namespace_id) { create(:group).to_global_id } - it 'returns a top level error' do + it 'returns the top level error' do post_graphql_mutation(mutation, current_user: current_user) expect(graphql_errors).not_to be_empty + expect(graphql_errors.first).to match a_hash_including( + 'message' => resource_or_permission_error) end end context 'when the service returns an error' do let(:email_id) { create(:email).to_global_id } - it 'returns an error' do + it 'returns the error' do post_graphql_mutation(mutation, current_user: current_user) expect(mutation_response['errors']).to contain_exactly("Email must be provided.") @@ -66,10 +82,25 @@ def mutation_response end end - it 'creates a namespace commit email' do - post_graphql_mutation(mutation, current_user: current_user) + context 'when namespace is a group' do + it_behaves_like 'success' + end + + context 'when namespace is a user' do + let(:namespace_id) { current_user.namespace.to_global_id } + + it_behaves_like 'success' + end + + context 'when namespace is a project' do + let_it_be(:project) { create(:project) } + + let(:namespace_id) { project.project_namespace.to_global_id } + + before do + project.add_reporter(current_user) + end - expect(response).to have_gitlab_http_status(:success) - expect(mutation_response.dig('namespaceCommitEmail', 'email', 'id')).to eq(email.to_global_id.to_s) + it_behaves_like 'success' end end diff --git a/spec/services/users/set_namespace_commit_email_service_spec.rb b/spec/services/users/set_namespace_commit_email_service_spec.rb index 8ac3635a922038..d103e2e3b7597b 100644 --- a/spec/services/users/set_namespace_commit_email_service_spec.rb +++ b/spec/services/users/set_namespace_commit_email_service_spec.rb @@ -18,9 +18,18 @@ group.add_reporter(user) end + shared_examples 'success' do + it "creates namespace commit email" do + result = service.execute + + expect(result.payload[:namespace_commit_email]).to be_a(Users::NamespaceCommitEmail) + expect(result.payload[:namespace_commit_email]).to be_persisted + end + end + describe "#execute" do context "when current_user is not provided" do - let(:current_user) {} + let(:current_user) { nil } it "returns error message" do expect(service.execute.message) @@ -53,62 +62,77 @@ end end - context "when email_id is not provided" do - let(:email_id) { nil } + context "when target user is not current user" do + context "when current user is an admin" do + let(:current_user) { create(:user, :admin) } - it "returns error message" do - expect(service.execute.message).to eq("Email must be provided.") + context "when admin mode is enabled", :enable_admin_mode do + it "creates namespace commit email" do + result = service.execute + + expect(result.payload[:namespace_commit_email]).to be_a(Users::NamespaceCommitEmail) + expect(result.payload[:namespace_commit_email]).to be_persisted + end + end + + context "when admin mode is not enabled" do + let(:current_user) { create(:user) } + + it "returns error message" do + expect(service.execute.message) + .to eq("User doesn't exist or you don't have permission to change namespace commit emails.") + end + end + end + + context "when current user is not an admin" do + let(:current_user) { create(:user) } + + it "returns error message" do + expect(service.execute.message) + .to eq("User doesn't exist or you don't have permission to change namespace commit emails.") + end end end context "when namespace commit email does not exist" do - it "creates namespace commit email" do - result = service.execute + context "when email_id is not provided" do + let(:email_id) { nil } - expect(result.payload[:namespace_commit_email]).to be_a(Users::NamespaceCommitEmail) - expect(result.payload[:namespace_commit_email]).to be_persisted + it "returns error message" do + expect(service.execute.message).to eq("Email must be provided.") + end end - context "when target user is not current user" do - context "when current user is an admin" do - let(:current_user) { create(:user, :admin) } + context 'when namepsace is a group' do + it_behaves_like 'success' + end - context "when admin mode is enabled", :enable_admin_mode do - it "creates namespace commit email" do - result = service.execute + context 'when namespace is a user' do + let(:namespace) { current_user.namespace } - expect(result.payload[:namespace_commit_email]).to be_a(Users::NamespaceCommitEmail) - expect(result.payload[:namespace_commit_email]).to be_persisted - end - end + it_behaves_like 'success' + end - context "when admin mode is not enabled" do - let(:current_user) { create(:user) } + context 'when namespace is a project' do + let_it_be(:project) { create(:project) } - it "returns error message" do - expect(service.execute.message) - .to eq("User doesn't exist or you don't have permission to change namespace commit emails.") - end - end - end - - context "when current user is not an admin" do - let(:current_user) { create(:user) } + let(:namespace) { project.project_namespace } - it "returns error message" do - expect(service.execute.message) - .to eq("User doesn't exist or you don't have permission to change namespace commit emails.") - end + before do + project.add_reporter(current_user) end + + it_behaves_like 'success' end end context "when namespace commit email already exists" do let!(:existing_namespace_commit_email) do create(:namespace_commit_email, - user: target_user, - namespace: namespace, - mail: create(:email, user: target_user)) + user: target_user, + namespace: namespace, + email: create(:email, user: target_user)) end context "when email_id is not provided" do -- GitLab From a7109bc2e29a3a2a8100028a95c89348d7f887cb Mon Sep 17 00:00:00 2001 From: Niklas Date: Thu, 8 Jun 2023 17:52:09 +0000 Subject: [PATCH 4/6] Re-add wrongly removed line from GraphQL docs --- doc/api/graphql/reference/index.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 7ae47532488864..b9cf4e9fc79942 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -26096,6 +26096,7 @@ State of a Sentry error. | `EWM_SERVICE` | EwmService type. | | `EXTERNAL_WIKI_SERVICE` | ExternalWikiService type. | | `GITHUB_SERVICE` | GithubService type. | +| `GITLAB_SLACK_APPLICATION_SERVICE` | GitlabSlackApplicationService type. | | `GOOGLE_PLAY_SERVICE` | GooglePlayService type. | | `HANGOUTS_CHAT_SERVICE` | HangoutsChatService type. | | `HARBOR_SERVICE` | HarborService type. | -- GitLab From ef52812a3b58a84e406295d62f67304592877885 Mon Sep 17 00:00:00 2001 From: Lee Tickett Date: Mon, 12 Jun 2023 13:30:36 +0100 Subject: [PATCH 5/6] Apply reviewer suggestions --- .../users/set_namespace_commit_email.rb | 4 ++-- .../set_namespace_commit_email_service.rb | 16 ++++++------- doc/api/graphql/reference/index.md | 4 ++-- locale/gitlab.pot | 2 +- ...set_namespace_commit_email_service_spec.rb | 23 +++++++++++++++++++ 5 files changed, 36 insertions(+), 13 deletions(-) diff --git a/app/graphql/mutations/users/set_namespace_commit_email.rb b/app/graphql/mutations/users/set_namespace_commit_email.rb index cddfd3ccc41797..72ef0635bb30c1 100644 --- a/app/graphql/mutations/users/set_namespace_commit_email.rb +++ b/app/graphql/mutations/users/set_namespace_commit_email.rb @@ -8,12 +8,12 @@ class SetNamespaceCommitEmail < BaseMutation argument :namespace_id, ::Types::GlobalIDType[::Namespace], required: true, - description: 'Id of the namespace to set the namespace commit email for.' + description: 'ID of the namespace to set the namespace commit email for.' argument :email_id, ::Types::GlobalIDType[::Email], required: false, - description: 'Id of the email to set.' + description: 'ID of the email to set.' field :namespace_commit_email, Types::Users::NamespaceCommitEmailType, diff --git a/app/services/users/set_namespace_commit_email_service.rb b/app/services/users/set_namespace_commit_email_service.rb index b6d7d430688186..9b1db151cc3e70 100644 --- a/app/services/users/set_namespace_commit_email_service.rb +++ b/app/services/users/set_namespace_commit_email_service.rb @@ -31,7 +31,7 @@ def execute elsif email_id.nil? remove_namespace_commit_email(existing_namespace_commit_email) else - update_namespace_comit_email(existing_namespace_commit_email, email) + update_namespace_commit_email(existing_namespace_commit_email, email) end end @@ -51,16 +51,16 @@ def create_namespace_commit_email(email) email: email ) - if !namespace_commit_email.save - error_in_save(namespace_commit_email) - else - success(namespace_commit_email) - end + save_namespace_commit_email(namespace_commit_email) end - def update_namespace_comit_email(namespace_commit_email, email) + def update_namespace_commit_email(namespace_commit_email, email) namespace_commit_email.email = email + save_namespace_commit_email(namespace_commit_email) + end + + def save_namespace_commit_email(namespace_commit_email) if !namespace_commit_email.save error_in_save(namespace_commit_email) else @@ -79,7 +79,7 @@ def error(message) end def error_in_save(namespace_commit_email) - return error(_("Failed to save namespace commit email")) if namespace_commit_email.errors.empty? + return error(_("Failed to save namespace commit email.")) if namespace_commit_email.errors.empty? error(namespace_commit_email.errors.full_messages.to_sentence) end diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index b9cf4e9fc79942..25fa02ed74eba2 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -6745,8 +6745,8 @@ Input type: `UserSetNamespaceCommitEmailInput` | Name | Type | Description | | ---- | ---- | ----------- | | `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | -| `emailId` | [`EmailID`](#emailid) | Id of the email to set. | -| `namespaceId` | [`NamespaceID!`](#namespaceid) | Id of the namespace to set the namespace commit email for. | +| `emailId` | [`EmailID`](#emailid) | ID of the email to set. | +| `namespaceId` | [`NamespaceID!`](#namespaceid) | ID of the namespace to set the namespace commit email for. | #### Fields diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 86b83b45255750..cf20c75e331731 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -18776,7 +18776,7 @@ msgstr "" msgid "Failed to save merge conflicts resolutions. Please try again!" msgstr "" -msgid "Failed to save namespace commit email" +msgid "Failed to save namespace commit email." msgstr "" msgid "Failed to save new settings" diff --git a/spec/services/users/set_namespace_commit_email_service_spec.rb b/spec/services/users/set_namespace_commit_email_service_spec.rb index d103e2e3b7597b..7d1093659f6f4e 100644 --- a/spec/services/users/set_namespace_commit_email_service_spec.rb +++ b/spec/services/users/set_namespace_commit_email_service_spec.rb @@ -3,9 +3,12 @@ require 'spec_helper' RSpec.describe Users::SetNamespaceCommitEmailService, feature_category: :user_profile do + include AfterNextHelpers + let_it_be(:user) { create(:user) } let_it_be(:group) { create(:group) } let_it_be(:email) { create(:email, user: user) } + let_it_be(:existing_achievement) { create(:achievement, namespace: group) } let(:namespace) { group } let(:current_user) { user } @@ -104,6 +107,16 @@ end end + context 'when model save fails' do + before do + allow_next(::Users::NamespaceCommitEmail).to receive(:save).and_return(false) + end + + it "returns error message" do + expect(service.execute.message).to eq("Failed to save namespace commit email.") + end + end + context 'when namepsace is a group' do it_behaves_like 'success' end @@ -158,6 +171,16 @@ expect(existing_namespace_commit_email.email_id).to eq(email_id) end end + + context 'when model save fails' do + before do + allow_any_instance_of(::Users::NamespaceCommitEmail).to receive(:save).and_return(false) # rubocop:disable RSpec/AnyInstanceOf + end + + it "returns error message" do + expect(service.execute.message).to eq("Failed to save namespace commit email.") + end + end end end end -- GitLab From 3adbbccac308855257c4a131da27e5659be2d7ad Mon Sep 17 00:00:00 2001 From: Lee Tickett Date: Tue, 13 Jun 2023 07:53:43 +0100 Subject: [PATCH 6/6] Apply reviewer suggestions --- .../set_namespace_commit_email_service.rb | 10 +-- .../users/set_namespace_commit_email_spec.rb | 2 +- ...set_namespace_commit_email_service_spec.rb | 77 +++++++++++-------- 3 files changed, 49 insertions(+), 40 deletions(-) diff --git a/app/services/users/set_namespace_commit_email_service.rb b/app/services/users/set_namespace_commit_email_service.rb index 9b1db151cc3e70..30ee597120deed 100644 --- a/app/services/users/set_namespace_commit_email_service.rb +++ b/app/services/users/set_namespace_commit_email_service.rb @@ -14,12 +14,12 @@ def initialize(current_user, namespace, email_id, params) end def execute + return error(_('Namespace must be provided.')) if namespace.nil? + unless can?(current_user, :admin_user_email_address, target_user) return error(_("User doesn't exist or you don't have permission to change namespace commit emails.")) end - return error(_("Namespace must be provided.")) if namespace.nil? - unless can?(target_user, :read_namespace, namespace) return error(_("Namespace doesn't exist or you don't have permission.")) end @@ -27,6 +27,8 @@ def execute email = target_user.emails.find_by(id: email_id) unless email_id.nil? # rubocop: disable CodeReuse/ActiveRecord existing_namespace_commit_email = target_user.namespace_commit_email_for_namespace(namespace) if existing_namespace_commit_email.nil? + return error(_('Email must be provided.')) if email.nil? + create_namespace_commit_email(email) elsif email_id.nil? remove_namespace_commit_email(existing_namespace_commit_email) @@ -43,8 +45,6 @@ def remove_namespace_commit_email(namespace_commit_email) end def create_namespace_commit_email(email) - return error(_("Email must be provided.")) if email.nil? - namespace_commit_email = ::Users::NamespaceCommitEmail.new( user: target_user, namespace: namespace, @@ -79,7 +79,7 @@ def error(message) end def error_in_save(namespace_commit_email) - return error(_("Failed to save namespace commit email.")) if namespace_commit_email.errors.empty? + return error(_('Failed to save namespace commit email.')) if namespace_commit_email.errors.empty? error(namespace_commit_email.errors.full_messages.to_sentence) end diff --git a/spec/graphql/mutations/users/set_namespace_commit_email_spec.rb b/spec/graphql/mutations/users/set_namespace_commit_email_spec.rb index cef8702a8e0274..6d8e15ac791e6f 100644 --- a/spec/graphql/mutations/users/set_namespace_commit_email_spec.rb +++ b/spec/graphql/mutations/users/set_namespace_commit_email_spec.rb @@ -39,7 +39,7 @@ group.add_reporter(current_user) end - context 'when the params are invalid' do + context 'when the email does not belong to the target user' do let(:email_id) { create(:email).to_global_id } it 'returns the validation error' do diff --git a/spec/services/users/set_namespace_commit_email_service_spec.rb b/spec/services/users/set_namespace_commit_email_service_spec.rb index 7d1093659f6f4e..4f64d454ecb507 100644 --- a/spec/services/users/set_namespace_commit_email_service_spec.rb +++ b/spec/services/users/set_namespace_commit_email_service_spec.rb @@ -22,7 +22,7 @@ end shared_examples 'success' do - it "creates namespace commit email" do + it 'creates namespace commit email' do result = service.execute expect(result.payload[:namespace_commit_email]).to be_a(Users::NamespaceCommitEmail) @@ -30,47 +30,47 @@ end end - describe "#execute" do - context "when current_user is not provided" do + describe '#execute' do + context 'when current_user is not provided' do let(:current_user) { nil } - it "returns error message" do + it 'returns error message' do expect(service.execute.message) .to eq("User doesn't exist or you don't have permission to change namespace commit emails.") end end - context "when current_user does not have permission to change namespace commit emails" do + context 'when current_user does not have permission to change namespace commit emails' do let(:target_user) { create(:user) } - it "returns error message" do + it 'returns error message' do expect(service.execute.message) .to eq("User doesn't exist or you don't have permission to change namespace commit emails.") end end - context "when target_user does not have permission to access the namespace" do + context 'when target_user does not have permission to access the namespace' do let(:namespace) { create(:group) } - it "returns error message" do + it 'returns error message' do expect(service.execute.message).to eq("Namespace doesn't exist or you don't have permission.") end end - context "when namespace is not provided" do + context 'when namespace is not provided' do let(:namespace) { nil } - it "returns error message" do - expect(service.execute.message).to eq("Namespace must be provided.") + it 'returns error message' do + expect(service.execute.message).to eq('Namespace must be provided.') end end - context "when target user is not current user" do - context "when current user is an admin" do + context 'when target user is not current user' do + context 'when current user is an admin' do let(:current_user) { create(:user, :admin) } - context "when admin mode is enabled", :enable_admin_mode do - it "creates namespace commit email" do + context 'when admin mode is enabled', :enable_admin_mode do + it 'creates namespace commit email' do result = service.execute expect(result.payload[:namespace_commit_email]).to be_a(Users::NamespaceCommitEmail) @@ -78,32 +78,30 @@ end end - context "when admin mode is not enabled" do - let(:current_user) { create(:user) } - - it "returns error message" do + context 'when admin mode is not enabled' do + it 'returns error message' do expect(service.execute.message) .to eq("User doesn't exist or you don't have permission to change namespace commit emails.") end end end - context "when current user is not an admin" do + context 'when current user is not an admin' do let(:current_user) { create(:user) } - it "returns error message" do + it 'returns error message' do expect(service.execute.message) .to eq("User doesn't exist or you don't have permission to change namespace commit emails.") end end end - context "when namespace commit email does not exist" do - context "when email_id is not provided" do + context 'when namespace commit email does not exist' do + context 'when email_id is not provided' do let(:email_id) { nil } - it "returns error message" do - expect(service.execute.message).to eq("Email must be provided.") + it 'returns error message' do + expect(service.execute.message).to eq('Email must be provided.') end end @@ -112,8 +110,8 @@ allow_next(::Users::NamespaceCommitEmail).to receive(:save).and_return(false) end - it "returns error message" do - expect(service.execute.message).to eq("Failed to save namespace commit email.") + it 'returns error message' do + expect(service.execute.message).to eq('Failed to save namespace commit email.') end end @@ -140,7 +138,7 @@ end end - context "when namespace commit email already exists" do + context 'when namespace commit email already exists' do let!(:existing_namespace_commit_email) do create(:namespace_commit_email, user: target_user, @@ -148,10 +146,10 @@ email: create(:email, user: target_user)) end - context "when email_id is not provided" do + context 'when email_id is not provided' do let(:email_id) { nil } - it "destroys the namespace commit email" do + it 'destroys the namespace commit email' do result = service.execute expect(result.message).to be_nil @@ -159,10 +157,10 @@ end end - context "and email_id is provided" do + context 'and email_id is provided' do let(:email_id) { create(:email, user: current_user).id } - it "updates namespace commit email" do + it 'updates namespace commit email' do result = service.execute existing_namespace_commit_email.reload @@ -177,8 +175,19 @@ allow_any_instance_of(::Users::NamespaceCommitEmail).to receive(:save).and_return(false) # rubocop:disable RSpec/AnyInstanceOf end - it "returns error message" do - expect(service.execute.message).to eq("Failed to save namespace commit email.") + it 'returns generic error message' do + expect(service.execute.message).to eq('Failed to save namespace commit email.') + end + + context 'with model errors' do + before do + allow_any_instance_of(::Users::NamespaceCommitEmail).to receive_message_chain(:errors, :empty?).and_return(false) # rubocop:disable RSpec/AnyInstanceOf + allow_any_instance_of(::Users::NamespaceCommitEmail).to receive_message_chain(:errors, :full_messages, :to_sentence).and_return('Model error') # rubocop:disable RSpec/AnyInstanceOf + end + + it 'returns the model error message' do + expect(service.execute.message).to eq('Model error') + end end end end -- GitLab