Automatic merge of gitlab-org/gitlab master

app/controllers/admin/groups_controller.rb

@@ -62,7 +62,7 @@ class Admin::GroupsController < Admin::ApplicationController
 
   def members_update
     member_params = params.permit(:user_ids, :access_level, :expires_at)
-    result = Members::CreateService.new(current_user, member_params.merge(limit: -1)).execute(@group)
+    result = Members::CreateService.new(current_user, member_params.merge(limit: -1, source: @group)).execute
 
     if result[:status] == :success
       redirect_to [:admin, @group], notice: _('Users were successfully added.')

app/controllers/concerns/membership_actions.rb

@@ -6,7 +6,7 @@ module MembershipActions
 
   def create
     create_params = params.permit(:user_ids, :access_level, :expires_at)
-    result = Members::CreateService.new(current_user, create_params).execute(membershipable)
+    result = Members::CreateService.new(current_user, create_params.merge({ source: membershipable })).execute
 
     if result[:status] == :success
       redirect_to members_page_url, notice: _('Users were successfully added.')

app/models/member.rb

@@ -284,6 +284,10 @@ class Member < ApplicationRecord
       Gitlab::Access.sym_options
     end
 
+    def valid_email?(email)
+      Devise.email_regexp.match?(email)
+    end
+
     private
 
     def parse_users_list(source, list)
@@ -305,6 +309,7 @@ class Member < ApplicationRecord
 
       if user_ids.present?
         users.concat(User.where(id: user_ids))
+        # the below will automatically discard invalid user_ids
         existing_members = source.members_and_requesters.where(user_id: user_ids).index_by(&:user_id)
       end
 

app/services/members/create_service.rb

@@ -2,67 +2,98 @@
 
 module Members
   class CreateService < Members::BaseService
-    include Gitlab::Utils::StrongMemoize
+    BlankInvitesError = Class.new(StandardError)
+    TooManyInvitesError = Class.new(StandardError)
 
-    DEFAULT_LIMIT = 100
+    DEFAULT_INVITE_LIMIT = 100
 
-    def execute(source)
-      return error(s_('AddMember|No users specified.')) if user_ids.blank?
+    def initialize(*args)
+      super
 
-      return error(s_("AddMember|Too many users specified (limit is %{user_limit})") % { user_limit: user_limit }) if
-        user_limit && user_ids.size > user_limit
+      @errors = []
+      @invites = invites_from_params&.split(',')&.uniq&.flatten
+      @source = params[:source]
+    end
+
+    def execute
+      validate_invites!
+
+      add_members
+      enqueue_onboarding_progress_action
+      result
+    rescue BlankInvitesError, TooManyInvitesError => e
+      error(e.message)
+    end
+
+    private
+
+    attr_reader :source, :errors, :invites, :member_created_namespace_id
+
+    def invites_from_params
+      params[:user_ids]
+    end
+
+    def validate_invites!
+      raise BlankInvitesError, blank_invites_message if invites.blank?
+
+      return unless user_limit && invites.size > user_limit
+
+      raise TooManyInvitesError,
+            format(s_("AddMember|Too many users specified (limit is %{user_limit})"), user_limit: user_limit)
+    end
+
+    def blank_invites_message
+      s_('AddMember|No users specified.')
+    end
 
+    def add_members
       members = source.add_users(
-        user_ids,
+        invites,
         params[:access_level],
         expires_at: params[:expires_at],
         current_user: current_user
       )
 
-      errors = []
-
-      members.each do |member|
-        if member.invalid?
-          current_error =
-            # Invited users may not have an associated user
-            if member.user.present?
-              "#{member.user.username}: "
-            else
-              ""
+      members.each { |member| process_result(member) }
     end
 
-          current_error += member.errors.full_messages.to_sentence
-          errors << current_error
+    def process_result(member)
+      if member.invalid?
+        add_error_for_member(member)
       else
         after_execute(member: member)
+        @member_created_namespace_id ||= member.namespace_id
       end
     end
 
-      enqueue_onboarding_progress_action(source) if members.size > errors.size
+    def add_error_for_member(member)
+      prefix = "#{member.user.username}: " if member.user.present?
 
-      return success unless errors.any?
-
-      error(errors.to_sentence)
+      errors << "#{prefix}#{member.errors.full_messages.to_sentence}"
     end
 
-    private
+    def user_limit
+      limit = params.fetch(:limit, DEFAULT_INVITE_LIMIT)
 
-    def user_ids
-      strong_memoize(:user_ids) do
-        ids = params[:user_ids] || ''
-        ids.split(',').uniq.flatten
-      end
+      limit && limit < 0 ? nil : limit
     end
 
-    def user_limit
-      limit = params.fetch(:limit, DEFAULT_LIMIT)
+    def enqueue_onboarding_progress_action
+      return unless member_created_namespace_id
 
-      limit && limit < 0 ? nil : limit
+      Namespaces::OnboardingUserAddedWorker.perform_async(member_created_namespace_id)
+    end
+
+    def result
+      if errors.any?
+        error(formatted_errors)
+      else
+        success
+      end
     end
 
-    def enqueue_onboarding_progress_action(source)
-      namespace_id = source.is_a?(Project) ? source.namespace_id : source.id
-      Namespaces::OnboardingUserAddedWorker.perform_async(namespace_id)
+    def formatted_errors
+      errors.to_sentence
     end
   end
 end

app/services/members/invite_service.rb

 # frozen_string_literal: true
 
 module Members
-  class InviteService < Members::BaseService
-    BlankEmailsError = Class.new(StandardError)
-    TooManyEmailsError = Class.new(StandardError)
+  class InviteService < Members::CreateService
+    extend ::Gitlab::Utils::Override
 
     def initialize(*args)
       super
 
       @errors = {}
-      @emails = params[:email]&.split(',')&.uniq&.flatten
-      @source = params[:source]
-    end
-
-    def execute
-      validate_emails!
-
-      emails.each(&method(:process_email))
-      enqueue_onboarding_progress_action
-      result
-    rescue BlankEmailsError, TooManyEmailsError => e
-      error(e.message)
     end
 
     private
 
-    attr_reader :source, :errors, :emails, :member_created_namespace_id
-
-    def validate_emails!
-      raise BlankEmailsError, s_('AddMember|Email cannot be blank') if emails.blank?
-
-      if user_limit && emails.size > user_limit
-        raise TooManyEmailsError, s_("AddMember|Too many users specified (limit is %{user_limit})") % { user_limit: user_limit }
-      end
-    end
-
-    def user_limit
-      limit = params.fetch(:limit, Members::CreateService::DEFAULT_LIMIT)
-
-      limit < 0 ? nil : limit
-    end
-
-    def process_email(email)
-      return if existing_member?(email)
-      return if existing_invite?(email)
-      return if existing_request?(email)
-
-      add_member(email)
-    end
-
-    def existing_member?(email)
-      existing_member = source.members.with_user_by_email(email).exists?
+    alias_method :formatted_errors, :errors
 
-      if existing_member
-        errors[email] = s_("AddMember|Already a member of %{source_name}") % { source_name: source.name }
-        return true
+    def invites_from_params
+      params[:email]
     end
 
-      false
-    end
-
-    def existing_invite?(email)
-      existing_invite = source.members.search_invite_email(email).exists?
-
-      if existing_invite
-        errors[email] = s_("AddMember|Member already invited to %{source_name}") % { source_name: source.name }
-        return true
-      end
-
-      false
-    end
-
-    def existing_request?(email)
-      existing_request = source.requesters.with_user_by_email(email).exists?
+    def validate_invites!
+      super
 
-      if existing_request
-        errors[email] = s_("AddMember|Member cannot be invited because they already requested to join %{source_name}") % { source_name: source.name }
-        return true
-      end
+      # we need the below due to add_users hitting Member#parse_users_list and ignoring invalid emails
+      # ideally we wouldn't need this, but we can't really change the add_users method
+      valid, invalid = invites.partition { |email| Member.valid_email?(email) }
+      @invites = valid
 
-      false
+      invalid.each { |email| errors[email] = s_('AddMember|Invite email is invalid') }
     end
 
-    def add_member(email)
-      new_member = source.add_user(email, params[:access_level], current_user: current_user, expires_at: params[:expires_at])
-
-      if new_member.invalid?
-        errors[email] = new_member.errors.full_messages.to_sentence
-      else
-        after_execute(member: new_member)
-        @member_created_namespace_id ||= new_member.namespace_id
-      end
+    override :blank_invites_message
+    def blank_invites_message
+      s_('AddMember|Emails cannot be blank')
     end
 
-    def result
-      if errors.any?
-        error(errors)
-      else
-        success
+    override :add_error_for_member
+    def add_error_for_member(member)
+      errors[invite_email(member)] = member.errors.full_messages.to_sentence
     end
-    end
-
-    def enqueue_onboarding_progress_action
-      return unless member_created_namespace_id
 
-      Namespaces::OnboardingUserAddedWorker.perform_async(member_created_namespace_id)
+    def invite_email(member)
+      member.invite_email || member.user.email
     end
   end
 end
-
-Members::InviteService.prepend_if_ee('EE::Members::InviteService')

changelogs/unreleased/323078-combine-concepts-in-inviting-member-service-classes-5.yml

+---
+title: Refactor member/invitation services to share common code
+merge_request: 57618
+author:
+type: other

changelogs/unreleased/sh-update-mailroom-config.yml

+---
+title: Add config support for using Microsoft Graph with MailRoom
+merge_request: 58250
+author:
+type: added

config/gitlab.yml.example

@@ -210,6 +210,13 @@ production: &base
     # Whether to expunge (permanently remove) messages from the mailbox when they are deleted after delivery
     expunge_deleted: false
 
+    # For Microsoft Graph support
+    # inbox_method: microsoft_graph
+    # inbox_options:
+    #   tenant_id: "YOUR-TENANT-ID"
+    #   client_id: "YOUR-CLIENT-ID"
+    #   client_secret: "YOUR-CLIENT-SECRET"
+
   ## Consolidated object store config
   ## This will only take effect if the object_store sections are not defined
   ## within the types (e.g. artifacts, lfs, etc.).

config/mail_room.yml

@@ -19,6 +19,13 @@
       :delete_after_delivery: true
       :expunge_deleted: <%= config[:expunge_deleted].to_json %>
 
+      <% if config[:inbox_method] %>
+      :inbox_method: <%= config[:inbox_method] %>
+      <% end %>
+      <% if config[:inbox_options].is_a?(Hash) %>
+      <%= config.slice(:inbox_options).to_yaml(indentation: 8).gsub(/^---\n/, '') %>
+      <% end %>
+
       :delivery_method: sidekiq
       :delivery_options:
         :redis_url: <%= config[:redis_url].to_json %>

doc/api/invitations.md

@@ -61,8 +61,8 @@ When there was any error sending the email:
 {
   "status": "error",
   "message": {
-               "test@example.com": "Already invited",
-               "test2@example.com": "Member already exsists"
+               "test@example.com": "Invite email has already been taken",
+               "test2@example.com": "User already exists in source"
              }
 }
 ```

ee/app/assets/javascripts/pages/admin/cloud_licenses/components/subscription_details_user_info.vue

+<script>
+import { GlCard, GlLink, GlSprintf } from '@gitlab/ui';
+import { helpPagePath } from '~/helpers/help_page_helper';
+import {
+  billableUsersText,
+  billableUsersTitle,
+  maximumUsersText,
+  maximumUsersTitle,
+  usersInSubscriptionText,
+  usersInSubscriptionTitle,
+  usersOverSubscriptionText,
+  usersOverSubscriptionTitle,
+} from '../constants';
+
+export const billableUsersURL = helpPagePath('subscriptions/self_managed/index');
+export const trueUpURL = 'https://about.gitlab.com/license-faq/';
+
+export default {
+  i18n: {
+    billableUsersTitle,
+    maximumUsersTitle,
+    usersInSubscriptionTitle,
+    usersOverSubscriptionTitle,
+    billableUsersText,
+    maximumUsersText,
+    usersInSubscriptionText,
+    usersOverSubscriptionText,
+  },
+  links: {
+    billableUsersURL,
+    trueUpURL,
+  },
+  name: 'SubscriptionDetailsUserInfo',
+  components: {
+    GlCard,
+    GlLink,
+    GlSprintf,
+  },
+  props: {
+    subscription: {
+      type: Object,
+      required: true,
+    },
+  },
+  computed: {
+    usersInSubscription() {
+      return this.subscription.usersInLicense;
+    },
+    billableUsers() {
+      return this.subscription.billableUsers;
+    },
+    maximumUsers() {
+      return this.subscription.maximumUsers;
+    },
+    usersOverSubscription() {
+      return this.subscription.usersOverSubscription;
+    },
+  },
+};
+</script>
+
+<template>
+  <section class="row">
+    <div class="col-md-6 gl-mb-5">
+      <gl-card data-testid="users-in-license">
+        <header>
+          <h2>{{ usersInSubscription }}</h2>
+          <h5 class="gl-font-weight-normal text-uppercase">
+            {{ $options.i18n.usersInSubscriptionTitle }}
+          </h5>
+        </header>
+        <p>
+          {{ $options.i18n.usersInSubscriptionText }}
+        </p>
+      </gl-card>
+    </div>
+
+    <div class="col-md-6 gl-mb-5">
+      <gl-card data-testid="billable-users">
+        <header>
+          <h2>{{ billableUsers }}</h2>
+          <h5 class="gl-font-weight-normal text-uppercase">
+            {{ $options.i18n.billableUsersTitle }}
+          </h5>
+        </header>
+        <p>
+          <gl-sprintf :message="$options.i18n.billableUsersText">
+            <template #billableUsersLink="{ content }">
+              <gl-link :href="$options.links.billableUsersURL" target="_blank"
+                >{{ content }}
+              </gl-link>
+            </template>
+          </gl-sprintf>
+        </p>
+      </gl-card>
+    </div>
+
+    <div class="col-md-6 gl-mb-5">
+      <gl-card data-testid="maximum-users">
+        <header>
+          <h2>{{ maximumUsers }}</h2>
+          <h5 class="gl-font-weight-normal text-uppercase">
+            {{ $options.i18n.maximumUsersTitle }}
+          </h5>
+        </header>
+        <p>
+          {{ $options.i18n.maximumUsersText }}
+        </p>
+      </gl-card>
+    </div>
+
+    <div class="col-md-6 gl-mb-5">
+      <gl-card data-testid="users-over-subscription">
+        <header>
+          <h2>{{ usersOverSubscription }}</h2>
+          <h5 class="gl-font-weight-normal text-uppercase">
+            {{ $options.i18n.usersOverSubscriptionTitle }}
+          </h5>
+        </header>
+        <p>
+          <gl-sprintf :message="$options.i18n.usersOverSubscriptionText">
+            <template #trueUpLink="{ content }">
+              <gl-link :href="$options.links.trueUpURL">{{ content }}</gl-link>
+            </template>
+          </gl-sprintf>
+        </p>
+      </gl-card>
+    </div>
+  </section>
+</template>

ee/app/assets/javascripts/pages/admin/cloud_licenses/constants.js

@@ -16,3 +16,20 @@ export const detailsLabels = {
   startsAt: s__('CloudLicense|Started'),
   renews: s__('CloudLicense|Renews'),
 };
+
+export const billableUsersTitle = s__('CloudLicense|Billable users');
+export const maximumUsersTitle = s__('CloudLicense|Maximum users');
+export const usersInSubscriptionTitle = s__('CloudLicense|Users in subscription');
+export const usersOverSubscriptionTitle = s__('CloudLicense|Users over subscription');
+export const billableUsersText = s__(
+  'CloudLicense|This is the number of %{billableUsersLinkStart}billable users%{billableUsersLinkEnd} on your installation, and this is the minimum number you need to purchase when you renew your license.',
+);
+export const maximumUsersText = s__(
+  'CloudLicense|This is the highest peak of users on your installation since the license started.',
+);
+export const usersInSubscriptionText = s__(
+  `CloudLicense|Users with a Guest role or those who don't belong to a Project or Group will not use a seat from your license.`,
+);
+export const usersOverSubscriptionText = s__(
+  `CloudLicense|You'll be charged for %{trueUpLinkStart}users over license%{trueUpLinkEnd} on a quarterly or annual basis, depending on the terms of your agreement.`,
+);

ee/app/controllers/concerns/group_invite_members.rb

@@ -5,11 +5,12 @@ module GroupInviteMembers
 
   def invite_members(group)
     invite_params = {
+      source: group,
       user_ids: emails_param[:emails]&.reject(&:blank?)&.join(','),
       access_level: Gitlab::Access::DEVELOPER
     }
 
-    result = Members::CreateService.new(current_user, invite_params).execute(group)
+    result = Members::CreateService.new(current_user, invite_params).execute
 
     ::Gitlab::Tracking.event(self.class.name, 'invite_members', label: 'new_group_form') if result[:status] == :success
   end

ee/app/controllers/registrations/group_invites_controller.rb

@@ -12,7 +12,7 @@ module Registrations
     end
 
     def create
-      result = Members::CreateService.new(current_user, invite_params).execute(group)
+      result = Members::CreateService.new(current_user, invite_params).execute
 
       if result[:status] == :success
         experiment(:registrations_group_invite, actor: current_user)
@@ -37,6 +37,7 @@ module Registrations
 
     def invite_params
       {
+        source: group,
         user_ids: emails_param[:emails]&.reject(&:blank?)&.join(','),
         access_level: Gitlab::Access::DEVELOPER
       }

ee/app/services/ee/members/create_service.rb

@@ -3,15 +3,30 @@
 module EE
   module Members
     module CreateService
-      extend ::Gitlab::Utils::Override
+      private
+
+      def validate_invites!
+        super
 
-      override :execute
-      def execute(source)
-        if invite_quota_exceeded?(source, user_ids)
-          return error(s_("AddMember|Invite limit of %{daily_invites} per day exceeded") % { daily_invites: source.actual_limits.daily_invites })
+        check_quota!
       end
 
-        super(source)
+      def check_quota!
+        return unless invite_quota_exceeded?
+
+        raise ::Members::CreateService::TooManyInvitesError,
+              format(
+                s_("AddMember|Invite limit of %{daily_invites} per day exceeded"),
+                daily_invites: source.actual_limits.daily_invites
+              )
+      end
+
+      def invite_quota_exceeded?
+        return unless source.actual_limits.daily_invites
+
+        invite_count = ::Member.invite.created_today.in_hierarchy(source).count
+
+        source.actual_limits.exceeded?(:daily_invites, invite_count + invites.count)
       end
 
       def after_execute(member:)
@@ -20,8 +35,6 @@ module EE
         log_audit_event(member: member)
       end
 
-      private
-
       def log_audit_event(member:)
         ::AuditEventService.new(
           current_user,
@@ -29,14 +42,6 @@ module EE
           action: :create
         ).for_member(member).security_event
       end
-
-      def invite_quota_exceeded?(source, user_ids)
-        return unless source.actual_limits.daily_invites
-
-        invite_count = ::Member.invite.created_today.in_hierarchy(source).count
-
-        source.actual_limits.exceeded?(:daily_invites, invite_count + user_ids.count)
-      end
     end
   end
 end

ee/app/services/ee/members/invite_service.rb

-# frozen_string_literal: true
-
-module EE
-  module Members
-    module InviteService
-      private
-
-      def validate_emails!
-        super
-
-        if invite_quota_exceeded?
-          raise ::Members::InviteService::TooManyEmailsError,
-                s_("AddMember|Invite limit of %{daily_invites} per day exceeded") %
-                  { daily_invites: source.actual_limits.daily_invites }
-        end
-      end
-
-      def invite_quota_exceeded?
-        return unless source.actual_limits.daily_invites
-
-        invite_count = ::Member.invite.created_today.in_hierarchy(source).count
-
-        source.actual_limits.exceeded?(:daily_invites, invite_count + emails.count)
-      end
-
-      def after_execute(member:)
-        super
-
-        log_audit_event(member: member)
-      end
-
-      def log_audit_event(member:)
-        ::AuditEventService.new(
-          current_user,
-          member.source,
-          action: :create
-        ).for_member(member).security_event
-      end
-    end
-  end
-end

ee/spec/frontend/admin/cloud_licenses/components/subscription_details_user_info_spec.js

+import { GlCard, GlLink, GlSprintf } from '@gitlab/ui';
+import { shallowMount } from '@vue/test-utils';
+import SubscriptionDetailsUserInfo, {
+  billableUsersURL,
+  trueUpURL,
+} from 'ee/pages/admin/cloud_licenses/components/subscription_details_user_info.vue';
+import {
+  billableUsersText,
+  billableUsersTitle,
+  maximumUsersText,
+  maximumUsersTitle,
+  usersInSubscriptionText,
+  usersInSubscriptionTitle,
+  usersOverSubscriptionText,
+  usersOverSubscriptionTitle,
+} from 'ee/pages/admin/cloud_licenses/constants';
+import { extendedWrapper } from 'helpers/vue_test_utils_helper';
+import { license } from '../mock_data';
+
+describe('Subscription Details Card', () => {
+  let wrapper;
+
+  const itif = (condition) => (condition ? it : it.skip);
+
+  const createComponent = (props = {}, stubGlSprintf = false) => {
+    wrapper = extendedWrapper(
+      shallowMount(SubscriptionDetailsUserInfo, {
+        propsData: {
+          subscription: license.ULTIMATE,
+          ...props,
+        },
+        stubs: {
+          GlCard,
+          GlSprintf: stubGlSprintf ? GlSprintf : true,
+        },
+      }),
+    );
+  };
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  describe.each`
+    testId                       | info    | title                         | text                         | link
+    ${'users-in-license'}        | ${'10'} | ${usersInSubscriptionTitle}   | ${usersInSubscriptionText}   | ${false}
+    ${'billable-users'}          | ${'8'}  | ${billableUsersTitle}         | ${billableUsersText}         | ${billableUsersURL}
+    ${'maximum-users'}           | ${'8'}  | ${maximumUsersTitle}          | ${maximumUsersText}          | ${false}
+    ${'users-over-subscription'} | ${'0'}  | ${usersOverSubscriptionTitle} | ${usersOverSubscriptionText} | ${trueUpURL}
+  `('with data for $card', ({ testId, info, title, text, link }) => {
+    beforeEach(() => {
+      createComponent();
+    });
+
+    const findUseCard = () => wrapper.findByTestId(testId);
+
+    it(`displays the info`, () => {
+      expect(findUseCard().find('h2').text()).toBe(info);
+    });
+
+    it(`displays the title`, () => {
+      expect(findUseCard().find('h5').text()).toBe(title);
+    });
+
+    itif(link)(`displays the content with a link`, () => {
+      expect(findUseCard().findComponent(GlSprintf).attributes('message')).toBe(text);
+    });
+
+    itif(!link)('displays a simple content', () => {
+      expect(findUseCard().find('p').text()).toBe(text);
+    });
+
+    itif(link)(`has a link`, () => {
+      createComponent({}, true);
+      expect(findUseCard().findComponent(GlLink).attributes('href')).toBe(link);
+    });
+
+    itif(!link)(`has not a link`, () => {
+      createComponent({}, true);
+      expect(findUseCard().findComponent(GlLink).exists()).toBe(link);
+    });
+  });
+});

ee/spec/frontend/admin/cloud_licenses/mock_data.js

 export const license = {
   ULTIMATE: {
+    billableUsers: '8',
+    company: 'ACME Corp',
+    email: 'user@acmecorp.com',
     id: '1309188',
-    plan: 'Ultimate',
     lastSync: 'just now - actual date',
+    maximumUsers: '8',
+    name: 'Jane Doe',
+    plan: 'Ultimate',
     startsAt: '22 February',
     renews: 'in 11 months',
-    name: 'Jane Doe',
-    email: 'user@acmecorp.com',
-    company: 'ACME Corp',
+    usersInLicense: '10',
+    usersOverSubscription: '0',
   },
 };
 

ee/spec/services/ee/members/create_service_spec.rb

@@ -12,7 +12,7 @@ RSpec.describe Members::CreateService do
 
   let(:params) { { user_ids: project_users.map(&:id).join(','), access_level: Gitlab::Access::GUEST } }
 
-  subject { described_class.new(user, params).execute(project) }
+  subject { described_class.new(user, params.merge({ source: project })).execute }
 
   before_all do
     project.add_maintainer(user)

lib/api/members.rb

@@ -100,9 +100,9 @@ module API
           authorize_admin_source!(source_type, source)
 
           if params[:user_id].to_s.include?(',')
-            create_service_params = params.except(:user_id).merge({ user_ids: params[:user_id] })
+            create_service_params = params.except(:user_id).merge({ user_ids: params[:user_id], source: source })
 
-            ::Members::CreateService.new(current_user, create_service_params).execute(source)
+            ::Members::CreateService.new(current_user, create_service_params).execute
           elsif params[:user_id].present?
             member = source.members.find_by(user_id: params[:user_id])
             conflict!('Member already exists') if member

locale/gitlab.pot

@@ -2022,21 +2022,15 @@ msgstr ""
 msgid "AddContextCommits|Add/remove"
 msgstr ""
 
-msgid "AddMember|Already a member of %{source_name}"
+msgid "AddMember|Emails cannot be blank"
 msgstr ""
 
-msgid "AddMember|Email cannot be blank"
+msgid "AddMember|Invite email is invalid"
 msgstr ""
 
 msgid "AddMember|Invite limit of %{daily_invites} per day exceeded"
 msgstr ""
 
-msgid "AddMember|Member already invited to %{source_name}"
-msgstr ""
-
-msgid "AddMember|Member cannot be invited because they already requested to join %{source_name}"
-msgstr ""
-
 msgid "AddMember|No users specified."
 msgstr ""
 
@@ -6450,6 +6444,9 @@ msgstr ""
 msgid "CloudLicense|Activation code"
 msgstr ""
 
+msgid "CloudLicense|Billable users"
+msgstr ""
+
 msgid "CloudLicense|I agree that my use of the GitLab Software is subject to the Subscription Agreement located at the %{linkStart}Terms of Service%{linkEnd}, unless otherwise agreed to in writing with GitLab."
 msgstr ""
 
@@ -6468,6 +6465,9 @@ msgstr ""
 msgid "CloudLicense|Manage"
 msgstr ""
 
+msgid "CloudLicense|Maximum users"
+msgstr ""
+
 msgid "CloudLicense|Paste your activation code"
 msgstr ""
 
@@ -6489,6 +6489,24 @@ msgstr ""
 msgid "CloudLicense|This instance is currently using the %{planName} plan."
 msgstr ""
 
+msgid "CloudLicense|This is the highest peak of users on your installation since the license started."
+msgstr ""
+
+msgid "CloudLicense|This is the number of %{billableUsersLinkStart}billable users%{billableUsersLinkEnd} on your installation, and this is the minimum number you need to purchase when you renew your license."
+msgstr ""
+
+msgid "CloudLicense|Users in subscription"
+msgstr ""
+
+msgid "CloudLicense|Users over subscription"
+msgstr ""
+
+msgid "CloudLicense|Users with a Guest role or those who don't belong to a Project or Group will not use a seat from your license."
+msgstr ""
+
+msgid "CloudLicense|You'll be charged for %{trueUpLinkStart}users over license%{trueUpLinkEnd} on a quarterly or annual basis, depending on the terms of your agreement."
+msgstr ""
+
 msgid "CloudLicense|Your subscription"
 msgstr ""
 

spec/config/mail_room_spec.rb

@@ -16,7 +16,9 @@ RSpec.describe 'mail_room.yml' do
     }
     cmd = "puts ERB.new(File.read(#{absolute_path(mailroom_config_path).inspect})).result"
 
-    output, status = Gitlab::Popen.popen(%W(ruby -rerb -e #{cmd}), absolute_path('config'), vars)
+    result = Gitlab::Popen.popen_with_detail(%W(ruby -rerb -e #{cmd}), absolute_path('config'), vars)
+    output = result.stdout
+    status = result.status
     raise "Error interpreting #{mailroom_config_path}: #{output}" unless status == 0
 
     YAML.load(output)
@@ -68,6 +70,39 @@ RSpec.describe 'mail_room.yml' do
     end
   end
 
+  context 'when both incoming email and service desk email are enabled for Microsoft Graph' do
+    let(:gitlab_config_path) { 'spec/fixtures/config/mail_room_enabled_ms_graph.yml' }
+    let(:queues_config_path) { 'spec/fixtures/config/redis_queues_new_format_host.yml' }
+    let(:gitlab_redis_queues) { Gitlab::Redis::Queues.new(Rails.env) }
+
+    it 'contains the intended configuration' do
+      expected_mailbox = {
+        email: 'gitlab-incoming@gmail.com',
+        name: 'inbox',
+        idle_timeout: 60,
+        expunge_deleted: true
+      }
+      expected_options = {
+        redis_url: gitlab_redis_queues.url,
+        sentinels: gitlab_redis_queues.sentinels
+      }
+      expected_inbox_options = {
+        tenant_id: '12345',
+        client_id: 'MY-CLIENT-ID',
+        client_secret: 'MY-CLIENT-SECRET',
+        poll_interval: 60
+      }
+
+      expect(configuration[:mailboxes].length).to eq(2)
+      expect(configuration[:mailboxes]).to all(include(expected_mailbox))
+      expect(configuration[:mailboxes].map { |m| m[:inbox_method] }).to all(eq('microsoft_graph'))
+      expect(configuration[:mailboxes].map { |m| m[:inbox_options] }).to all(eq(expected_inbox_options))
+      expect(configuration[:mailboxes].map { |m| m[:delivery_options] }).to all(include(expected_options))
+      expect(configuration[:mailboxes].map { |m| m[:delivery_options] }).to all(include(expected_options))
+      expect(configuration[:mailboxes].map { |m| m[:arbitration_options] }).to all(include(expected_options))
+    end
+  end
+
   def clear_queues_raw_config
     Gitlab::Redis::Queues.remove_instance_variable(:@_raw_config)
   rescue NameError

spec/fixtures/config/mail_room_enabled_ms_graph.yml

+test:
+  incoming_email:
+    enabled: true
+    address: "gitlab-incoming+%{key}@gmail.com"
+    user: "gitlab-incoming@gmail.com"
+    mailbox: "inbox"
+    expunge_deleted: true
+    inbox_method: "microsoft_graph"
+    inbox_options:
+      tenant_id: "12345"
+      client_id: "MY-CLIENT-ID"
+      client_secret: "MY-CLIENT-SECRET"
+      poll_interval: 60
+
+  service_desk_email:
+    enabled: true
+    address: "gitlab-incoming+%{key}@gmail.com"
+    user: "gitlab-incoming@gmail.com"
+    mailbox: "inbox"
+    expunge_deleted: true
+    inbox_method: "microsoft_graph"
+    inbox_options:
+      tenant_id: "12345"
+      client_id: "MY-CLIENT-ID"
+      client_secret: "MY-CLIENT-SECRET"
+      poll_interval: 60

spec/models/member_spec.rb

@@ -438,6 +438,16 @@ RSpec.describe Member do
     it { is_expected.to respond_to(:user_email) }
   end
 
+  describe '.valid_email?' do
+    it 'is a valid email format' do
+      expect(described_class.valid_email?('foo')).to eq(false)
+    end
+
+    it 'is not a valid email format' do
+      expect(described_class.valid_email?('foo@example.com')).to eq(true)
+    end
+  end
+
   describe '.add_user' do
     %w[project group].each do |source_type|
       context "when source is a #{source_type}" do

spec/requests/api/invitations_spec.rb

@@ -102,7 +102,8 @@ RSpec.describe API::Invitations do
                params: { email: stranger.email, access_level: Member::REPORTER }
 
           expect(response).to have_gitlab_http_status(:created)
-          expect(json_response['message'][stranger.email]).to eq("Access level should be greater than or equal to Developer inherited membership from group #{parent.name}")
+          expect(json_response['message'][stranger.email])
+            .to eq("Access level should be greater than or equal to Developer inherited membership from group #{parent.name}")
         end
 
         it 'creates the member if group level is lower' do
@@ -153,10 +154,10 @@ RSpec.describe API::Invitations do
 
       it "returns a message if member already exists" do
         post api("/#{source_type.pluralize}/#{source.id}/invitations", maintainer),
-             params: { email: maintainer.email, access_level: Member::MAINTAINER }
+             params: { email: developer.email, access_level: Member::MAINTAINER }
 
         expect(response).to have_gitlab_http_status(:created)
-        expect(json_response['message'][maintainer.email]).to eq("Already a member of #{source.name}")
+        expect(json_response['message'][developer.email]).to eq("User already exists in source")
       end
 
       it 'returns 404 when the email is not valid' do
@@ -164,7 +165,7 @@ RSpec.describe API::Invitations do
              params: { email: '', access_level: Member::MAINTAINER }
 
         expect(response).to have_gitlab_http_status(:created)
-        expect(json_response['message']).to eq('Email cannot be blank')
+        expect(json_response['message']).to eq('Emails cannot be blank')
       end
 
       it 'returns 404 when the email list is not a valid format' do

spec/requests/api/members_spec.rb

@@ -273,7 +273,7 @@ RSpec.describe API::Members do
             user_ids = [stranger.id, access_requester.id].join(',')
 
             allow_next_instance_of(::Members::CreateService) do |service|
-              expect(service).to receive(:execute).with(source).and_return({ status: :error, message: error_message })
+              expect(service).to receive(:execute).and_return({ status: :error, message: error_message })
             end
 
             expect do

spec/services/members/create_service_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Members::CreateService, :clean_gitlab_redis_shared_state, :sidekiq_inline do
+RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_shared_state, :sidekiq_inline do
   let_it_be(:source) { create(:project) }
   let_it_be(:user) { create(:user) }
   let_it_be(:member) { create(:user) }
@@ -10,7 +10,7 @@ RSpec.describe Members::CreateService, :clean_gitlab_redis_shared_state, :sideki
   let_it_be(:access_level) { Gitlab::Access::GUEST }
   let(:params) { { user_ids: user_ids, access_level: access_level } }
 
-  subject(:execute_service) { described_class.new(user, params).execute(source) }
+  subject(:execute_service) { described_class.new(user, params.merge({ source: source })).execute }
 
   before do
     if source.is_a?(Project)

spec/services/members/invite_service_spec.rb

@@ -48,14 +48,24 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
 
     it 'returns an error' do
       expect_not_to_create_members
-      expect(result[:message]).to eq('Email cannot be blank')
+      expect(result[:message]).to eq('Emails cannot be blank')
     end
   end
 
   context 'when email param is not included' do
     it 'returns an error' do
       expect_not_to_create_members
-      expect(result[:message]).to eq('Email cannot be blank')
+      expect(result[:message]).to eq('Emails cannot be blank')
+    end
+  end
+
+  context 'when email is not a valid email format' do
+    let(:params) { { email: '_bogus_' } }
+
+    it 'returns an error' do
+      expect { result }.not_to change(ProjectMember, :count)
+      expect(result[:status]).to eq(:error)
+      expect(result[:message][params[:email]]).to eq("Invite email is invalid")
     end
   end
 
@@ -114,7 +124,8 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
 
     it 'returns an error' do
       expect_not_to_create_members
-      expect(result[:message][project_user.email]).to eq("Access level is not included in the list")
+      expect(result[:message][project_user.email])
+        .to eq("Access level is not included in the list")
     end
   end
 
@@ -125,7 +136,8 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
     it 'adds new email and returns an error for the already invited email' do
       expect_to_create_members(count: 1)
       expect(result[:status]).to eq(:error)
-      expect(result[:message][invited_member.invite_email]).to eq("Member already invited to #{project.name}")
+      expect(result[:message][invited_member.invite_email])
+        .to eq("Invite email has already been taken")
       expect(project.users).to include project_user
     end
   end
@@ -138,7 +150,7 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
       expect_to_create_members(count: 1)
       expect(result[:status]).to eq(:error)
       expect(result[:message][requested_member.user.email])
-        .to eq("Member cannot be invited because they already requested to join #{project.name}")
+        .to eq("User already exists in source")
       expect(project.users).to include project_user
     end
   end
@@ -150,7 +162,8 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
     it 'adds new email and returns an error for the already invited email' do
       expect_to_create_members(count: 1)
       expect(result[:status]).to eq(:error)
-      expect(result[:message][existing_member.user.email]).to eq("Already a member of #{project.name}")
+      expect(result[:message][existing_member.user.email])
+        .to eq("User already exists in source")
       expect(project.users).to include project_user
     end
   end