Merge branch '225309-add-invite_email-quick-action' into 'master'

Add multiple email participants via a single quick action [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!49264

app/models/issue.rb

@@ -438,6 +438,10 @@ class Issue < ApplicationRecord
     issue_type_supports?(:assignee)
   end
 
+  def email_participants_downcase
+    issue_email_participants.pluck(IssueEmailParticipant.arel_table[:email].lower)
+  end
+
   private
 
   def ensure_metrics

app/models/issue_email_participant.rb

@@ -3,7 +3,7 @@
 class IssueEmailParticipant < ApplicationRecord
   belongs_to :issue
 
-  validates :email, presence: true, uniqueness: { scope: [:issue_id] }
+  validates :email, uniqueness: { scope: [:issue_id], case_sensitive: false }
   validates :issue, presence: true
   validate :validate_email_format
 

app/services/system_note_service.rb

@@ -241,6 +241,10 @@ module SystemNoteService
     ::SystemNotes::IssuablesService.new(noteable: noteable, project: project, author: author).mark_canonical_issue_of_duplicate(duplicate_issue)
   end
 
+  def add_email_participants(noteable, project, author, body)
+    ::SystemNotes::IssuablesService.new(noteable: noteable, project: project, author: author).add_email_participants(body)
+  end
+
   def discussion_lock(issuable, author)
     ::SystemNotes::IssuablesService.new(noteable: issuable, project: issuable.project, author: author).discussion_lock
   end

app/services/system_notes/issuables_service.rb

@@ -354,6 +354,10 @@ module SystemNotes
       create_note(NoteSummary.new(noteable, project, author, body, action: 'duplicate'))
     end
 
+    def add_email_participants(body)
+      create_note(NoteSummary.new(noteable, project, author, body))
+    end
+
     def discussion_lock
       action = noteable.discussion_locked? ? 'locked' : 'unlocked'
       body = "#{action} this #{noteable.class.to_s.titleize.downcase}"

changelogs/unreleased/225309-add-invite_email-quick-action.yml

+---
+title: Add invite_email Quick Action
+merge_request: 49264
+author: Lee Tickett
+type: added

config/feature_flags/development/issue_email_participants.yml

+---
+name: issue_email_participants
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/49264
+rollout_issue_url: 
+milestone: '13.8'
+type: development
+group: group::product planning
+default_enabled: false

config/metrics/counts_28d/20210222041219_i_quickactions_invite_email_single_monthly.yml

+---
+# See Usage Ping metrics dictionary docs https://docs.gitlab.com/ee/development/usage_ping/metrics_dictionary.html
+key_path: redis_hll_counters.quickactions.i_quickactions_invite_email_single_monthly
+description:
+product_section: dev
+product_stage: plan
+product_group: group::product planning
+product_category: issue_tracking
+value_type: number
+status: implemented
+milestone: 13.10
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/49264
+time_frame: 28d
+data_source: redis_hll
+distribution:
+- ce
+tier:
+- free
+skip_validation: true

config/metrics/counts_28d/20210222041235_i_quickactions_invite_email_multiple_monthly.yml

+---
+# See Usage Ping metrics dictionary docs https://docs.gitlab.com/ee/development/usage_ping/metrics_dictionary.html
+key_path: redis_hll_counters.quickactions.i_quickactions_invite_email_multiple_monthly
+description:
+product_section: dev
+product_stage: plan
+product_group: group::product planning
+product_category: issue_tracking
+value_type: number
+status: implemented
+milestone: 13.10
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/49264
+time_frame: 28d
+data_source: redis_hll
+distribution:
+- ce
+tier:
+- free
+skip_validation: true

db/migrate/20201209163113_recreate_index_issue_email_participants_on_issue_id_and_email.rb

+# frozen_string_literal: true
+
+class RecreateIndexIssueEmailParticipantsOnIssueIdAndEmail < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  OLD_INDEX_NAME = 'index_issue_email_participants_on_issue_id_and_email'
+  NEW_INDEX_NAME = 'index_issue_email_participants_on_issue_id_and_lower_email'
+
+  def up
+    # This table is currently empty, so no need to worry about unique index violations
+    add_concurrent_index :issue_email_participants, 'issue_id, lower(email)', unique: true, name: NEW_INDEX_NAME
+    remove_concurrent_index_by_name :issue_email_participants, OLD_INDEX_NAME
+  end
+
+  def down
+    add_concurrent_index :issue_email_participants, [:issue_id, :email], unique: true, name: OLD_INDEX_NAME
+    remove_concurrent_index_by_name :issue_email_participants, NEW_INDEX_NAME
+  end
+end

db/schema_migrations/20201209163113

+a7397b8f5d00b85f8c963f04ae062393b21313d97c9b9875a88a76608b98f826
\ No newline at end of file

db/structure.sql

@@ -22421,7 +22421,7 @@ CREATE UNIQUE INDEX index_issuable_slas_on_issue_id ON issuable_slas USING btree
 
 CREATE INDEX index_issue_assignees_on_user_id ON issue_assignees USING btree (user_id);
 
-CREATE UNIQUE INDEX index_issue_email_participants_on_issue_id_and_email ON issue_email_participants USING btree (issue_id, email);
+CREATE UNIQUE INDEX index_issue_email_participants_on_issue_id_and_lower_email ON issue_email_participants USING btree (issue_id, lower(email));
 
 CREATE INDEX index_issue_links_on_source_id ON issue_links USING btree (source_id);
 

doc/development/usage_ping/dictionary.md

@@ -17641,6 +17641,48 @@ Missing description
 | `tier` |  |
 | `skip_validation` | true |
 
+## `redis_hll_counters.quickactions.i_quickactions_invite_email_multiple_monthly`
+
+Missing description
+
+| field | value |
+| --- | --- |
+| `key_path` | **`redis_hll_counters.quickactions.i_quickactions_invite_email_multiple_monthly`** |
+| `product_section` | dev |
+| `product_stage` | plan |
+| `product_group` | `group::product planning` |
+| `product_category` | `issue_tracking` |
+| `value_type` | number |
+| `status` | implemented |
+| `milestone` | 13.1 |
+| `introduced_by_url` | [Introduced by](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/49264) |
+| `time_frame` | 28d |
+| `data_source` | Redis_hll |
+| `distribution` | ce |
+| `tier` | free |
+| `skip_validation` | true |
+
+## `redis_hll_counters.quickactions.i_quickactions_invite_email_single_monthly`
+
+Missing description
+
+| field | value |
+| --- | --- |
+| `key_path` | **`redis_hll_counters.quickactions.i_quickactions_invite_email_single_monthly`** |
+| `product_section` | dev |
+| `product_stage` | plan |
+| `product_group` | `group::product planning` |
+| `product_category` | `issue_tracking` |
+| `value_type` | number |
+| `status` | implemented |
+| `milestone` | 13.1 |
+| `introduced_by_url` | [Introduced by](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/49264) |
+| `time_frame` | 28d |
+| `data_source` | Redis_hll |
+| `distribution` | ce |
+| `tier` | free |
+| `skip_validation` | true |
+
 ## `redis_hll_counters.quickactions.i_quickactions_iteration_monthly`
 
 Missing description
@@ -19463,7 +19505,7 @@ Calculated unique users to perform Basic or Advanced searches by week
 
 ## `redis_hll_counters.search.search_total_unique_counts_monthly`
 
-Calculated unique users to perform Basic or Advanced searches by month
+Total unique users for i_search_total, i_search_advanced, i_search_paid for recent 28 days. This metric is redundant because advanced will be a subset of paid and paid will be a subset of total. i_search_total is more appropriate if you just want the total
 
 | field | value |
 | --- | --- |

doc/user/project/quick_actions.md

@@ -52,6 +52,7 @@ The following quick actions are applicable to descriptions, discussions and thre
 | `/duplicate <#issue>`                 | ✓     |               |      | Close this issue and mark as a duplicate of another issue. **(FREE)** Also, mark both as related. **(STARTER)**                                          |
 | `/epic <epic>`                        | ✓     |               |      | Add to epic `<epic>`. The `<epic>` value should be in the format of `&epic`, `group&epic`, or a URL to an epic. **(PREMIUM)**  |
 | `/estimate <<W>w <DD>d <hh>h <mm>m>`  | ✓     | ✓             |      | Set time estimate. For example, `/estimate 1w 3d 2h 14m`.                                                                        |
+| `/invite_email email1 email2`         | ✓     |               |      | Add up to 6 e-mail participants. This action is behind feature flag `issue_email_participants` |
 | `/iteration *iteration:"iteration name"`     | ✓     |               |      | Set iteration. For example, to set the `Late in July` iteration: `/iteration *iteration:"Late in July"` ([introduced in GitLab 13.1](https://gitlab.com/gitlab-org/gitlab/-/issues/196795)). **(STARTER)** |
 | `/label ~label1 ~label2`              | ✓     | ✓             | ✓    | Add one or more labels. Label names can also start without a tilde (`~`), but mixed syntax is not supported.                      |
 | `/lock`                               | ✓     | ✓             |      | Lock the discussions.                                                                                                                 |

lib/gitlab/quick_actions/issue_actions.rb

@@ -235,6 +235,35 @@ module Gitlab
           @execution_message[:remove_zoom] = result.message
         end
 
+        desc _('Add email participant(s)')
+        explanation _('Adds email participant(s)')
+        params 'email1@example.com email2@example.com (up to 6 emails)'
+        types Issue
+        condition do
+          Feature.enabled?(:issue_email_participants, parent) &&
+            current_user.can?(:"admin_#{quick_action_target.to_ability_name}", quick_action_target)
+        end
+        command :invite_email do |emails = ""|
+          MAX_NUMBER_OF_EMAILS = 6
+
+          existing_emails = quick_action_target.email_participants_downcase
+          emails_to_add = emails.split(' ').index_by { |email| [email.downcase, email] }.except(*existing_emails).each_value.first(MAX_NUMBER_OF_EMAILS)
+          added_emails = []
+
+          emails_to_add.each do |email|
+            new_participant = quick_action_target.issue_email_participants.create(email: email)
+            added_emails << email if new_participant.persisted?
+          end
+
+          if added_emails.any?
+            message = _("added %{emails}") % { emails: added_emails.to_sentence }
+            SystemNoteService.add_email_participants(quick_action_target, quick_action_target.project, current_user, message)
+            @execution_message[:invite_email] = message.upcase_first << "."
+          else
+            @execution_message[:invite_email] = _("No email participants were added. Either none were provided, or they already exist.")
+          end
+        end
+
         private
 
         def zoom_link_service

lib/gitlab/usage_data_counters/known_events/quickactions.yml

@@ -324,3 +324,13 @@
   redis_slot: quickactions
   aggregation: weekly
   feature_flag: usage_data_track_quickactions
+- name: i_quickactions_invite_email_single
+  category: quickactions
+  redis_slot: quickactions
+  aggregation: weekly
+  feature_flag: usage_data_track_quickactions
+- name: i_quickactions_invite_email_multiple
+  category: quickactions
+  redis_slot: quickactions
+  aggregation: weekly
+  feature_flag: usage_data_track_quickactions

lib/gitlab/usage_data_counters/quick_action_activity_unique_counter.rb

@@ -34,6 +34,8 @@ module Gitlab
             event_name_for_unassign(args)
           when 'unlabel', 'remove_label'
             event_name_for_unlabel(args)
+          when 'invite_email'
+            'invite_email' + event_name_quantifier(args.split)
           else
             name
           end
@@ -44,10 +46,8 @@ module Gitlab
 
           if args.count == 1 && args.first == 'me'
             'assign_self'
-          elsif args.count == 1
-            'assign_single'
           else
-            'assign_multiple'
+            'assign' + event_name_quantifier(args)
           end
         end
 
@@ -82,6 +82,14 @@ module Gitlab
             'unlabel_all'
           end
         end
+
+        def event_name_quantifier(args)
+          if args.count == 1
+            '_single'
+          else
+            '_multiple'
+          end
+        end
       end
     end
   end

locale/gitlab.pot

@@ -1838,6 +1838,9 @@ msgstr ""
 msgid "Add email address"
 msgstr ""
 
+msgid "Add email participant(s)"
+msgstr ""
+
 msgid "Add environment"
 msgstr ""
 
@@ -1997,6 +2000,9 @@ msgstr ""
 msgid "Adds an issue to an epic."
 msgstr ""
 
+msgid "Adds email participant(s)"
+msgstr ""
+
 msgid "Adjust your filters/search criteria above. If you believe this may be an error, please refer to the %{linkStart}Geo Troubleshooting%{linkEnd} documentation for more information."
 msgstr ""
 
@@ -20208,6 +20214,9 @@ msgstr ""
 msgid "No due date"
 msgstr ""
 
+msgid "No email participants were added. Either none were provided, or they already exist."
+msgstr ""
+
 msgid "No endpoint provided"
 msgstr ""
 
@@ -34520,6 +34529,9 @@ msgstr ""
 msgid "added %{created_at_timeago}"
 msgstr ""
 
+msgid "added %{emails}"
+msgstr ""
+
 msgid "added a Zoom call to this issue"
 msgstr ""
 

spec/db/schema_spec.rb

@@ -102,7 +102,12 @@ RSpec.describe 'Database schema' do
         context 'all foreign keys' do
           # for index to be effective, the FK constraint has to be at first place
           it 'are indexed' do
-            first_indexed_column = indexes.map(&:columns).map(&:first)
+            first_indexed_column = indexes.map(&:columns).map do |columns|
+              # In cases of complex composite indexes, a string is returned eg:
+              # "lower((extern_uid)::text), group_id"
+              columns = columns.split(',') if columns.is_a?(String)
+              columns.first.chomp
+            end
             foreign_keys_columns = foreign_keys.map(&:column)
 
             # Add the primary key column to the list of indexed columns because

spec/lib/gitlab/usage_data_counters/quick_action_activity_unique_counter_spec.rb

@@ -160,4 +160,24 @@ RSpec.describe Gitlab::UsageDataCounters::QuickActionActivityUniqueCounter, :cle
       end
     end
   end
+
+  context 'tracking invite_email' do
+    let(:quickaction_name) { 'invite_email' }
+
+    context 'single email' do
+      let(:args) { 'someone@gitlab.com' }
+
+      it_behaves_like 'a tracked quick action unique event' do
+        let(:action) { 'i_quickactions_invite_email_single' }
+      end
+    end
+
+    context 'multiple emails' do
+      let(:args) { 'someone@gitlab.com another@gitlab.com' }
+
+      it_behaves_like 'a tracked quick action unique event' do
+        let(:action) { 'i_quickactions_invite_email_multiple' }
+      end
+    end
+  end
 end

spec/models/application_setting_spec.rb

@@ -377,7 +377,7 @@ RSpec.describe ApplicationSetting do
       end
     end
 
-    it_behaves_like 'an object with email-formated attributes', :abuse_notification_email do
+    it_behaves_like 'an object with email-formatted attributes', :abuse_notification_email do
       subject { setting }
     end
 

spec/models/email_spec.rb

@@ -10,7 +10,7 @@ RSpec.describe Email do
   end
 
   describe 'validations' do
-    it_behaves_like 'an object with RFC3696 compliant email-formated attributes', :email do
+    it_behaves_like 'an object with RFC3696 compliant email-formatted attributes', :email do
       subject { build(:email) }
     end
   end

spec/models/issue_email_participant_spec.rb

@@ -11,9 +11,14 @@ RSpec.describe IssueEmailParticipant do
     subject { build(:issue_email_participant) }
 
     it { is_expected.to validate_presence_of(:issue) }
-    it { is_expected.to validate_presence_of(:email) }
-    it { is_expected.to validate_uniqueness_of(:email).scoped_to([:issue_id]) }
+    it { is_expected.to validate_uniqueness_of(:email).scoped_to([:issue_id]).ignoring_case_sensitivity }
 
-    it_behaves_like 'an object with RFC3696 compliant email-formated attributes', :email
+    it_behaves_like 'an object with RFC3696 compliant email-formatted attributes', :email
+
+    it 'is invalid if the email is nil' do
+      subject.email = nil
+
+      expect(subject).to be_invalid
+    end
   end
 end

spec/models/issue_spec.rb

@@ -1258,4 +1258,12 @@ RSpec.describe Issue do
       expect { issue.issue_type_supports?(:unkown_feature) }.to raise_error(ArgumentError)
     end
   end
+
+  describe '#email_participants_downcase' do
+    it 'returns a list of emails with all uppercase letters replaced with their lowercase counterparts' do
+      participant = create(:issue_email_participant, email: 'SomEoNe@ExamPLe.com')
+
+      expect(participant.issue.email_participants_downcase).to match([participant.email.downcase])
+    end
+  end
 end

spec/models/member_spec.rb

@@ -24,7 +24,7 @@ RSpec.describe Member do
       it { is_expected.to allow_value(nil).for(:expires_at) }
     end
 
-    it_behaves_like 'an object with email-formated attributes', :invite_email do
+    it_behaves_like 'an object with email-formatted attributes', :invite_email do
       subject { build(:project_member) }
     end
 

spec/models/user_spec.rb

@@ -381,11 +381,11 @@ RSpec.describe User do
     it { is_expected.not_to allow_value(-1).for(:projects_limit) }
     it { is_expected.not_to allow_value(Gitlab::Database::MAX_INT_VALUE + 1).for(:projects_limit) }
 
-    it_behaves_like 'an object with email-formated attributes', :email do
+    it_behaves_like 'an object with email-formatted attributes', :email do
       subject { build(:user) }
     end
 
-    it_behaves_like 'an object with RFC3696 compliant email-formated attributes', :public_email, :notification_email do
+    it_behaves_like 'an object with RFC3696 compliant email-formatted attributes', :public_email, :notification_email do
       subject { create(:user).tap { |user| user.emails << build(:email, email: email_value, confirmed_at: Time.current) } }
     end
 

spec/services/quick_actions/interpret_service_spec.rb

@@ -1949,6 +1949,100 @@ RSpec.describe QuickActions::InterpretService do
         end
       end
     end
+
+    context 'invite_email command' do
+      let_it_be(:issuable) { issue }
+
+      it_behaves_like 'empty command', "No email participants were added. Either none were provided, or they already exist." do
+        let(:content) { '/invite_email' }
+      end
+
+      context 'with existing email participant' do
+        let(:content) { '/invite_email a@gitlab.com' }
+
+        before do
+          issuable.issue_email_participants.create!(email: "a@gitlab.com")
+        end
+
+        it_behaves_like 'empty command', "No email participants were added. Either none were provided, or they already exist."
+      end
+
+      context 'with new email participants' do
+        let(:content) { '/invite_email a@gitlab.com b@gitlab.com' }
+
+        subject(:add_emails) { service.execute(content, issuable) }
+
+        it 'returns message' do
+          _, _, message = add_emails
+
+          expect(message).to eq('Added a@gitlab.com and b@gitlab.com.')
+        end
+
+        it 'adds 2 participants' do
+          expect { add_emails }.to change { issue.issue_email_participants.count }.by(2)
+        end
+
+        context 'with mixed case email' do
+          let(:content) { '/invite_email FirstLast@GitLab.com' }
+
+          it 'returns correctly cased message' do
+            _, _, message = add_emails
+
+            expect(message).to eq('Added FirstLast@GitLab.com.')
+          end
+        end
+
+        context 'with invalid email' do
+          let(:content) { '/invite_email a@gitlab.com bad_email' }
+
+          it 'only adds valid emails' do
+            expect { add_emails }.to change { issue.issue_email_participants.count }.by(1)
+          end
+        end
+
+        context 'with existing email' do
+          let(:content) { '/invite_email a@gitlab.com existing@gitlab.com' }
+
+          it 'only adds new emails' do
+            issue.issue_email_participants.create!(email: 'existing@gitlab.com')
+
+            expect { add_emails }.to change { issue.issue_email_participants.count }.by(1)
+          end
+
+          it 'only adds new (case insensitive) emails' do
+            issue.issue_email_participants.create!(email: 'EXISTING@gitlab.com')
+
+            expect { add_emails }.to change { issue.issue_email_participants.count }.by(1)
+          end
+        end
+
+        context 'with duplicate email' do
+          let(:content) { '/invite_email a@gitlab.com a@gitlab.com' }
+
+          it 'only adds unique new emails' do
+            expect { add_emails }.to change { issue.issue_email_participants.count }.by(1)
+          end
+        end
+
+        context 'with more than 6 emails' do
+          let(:content) { '/invite_email a@gitlab.com b@gitlab.com c@gitlab.com d@gitlab.com e@gitlab.com f@gitlab.com g@gitlab.com' }
+
+          it 'only adds 6 new emails' do
+            expect { add_emails }.to change { issue.issue_email_participants.count }.by(6)
+          end
+        end
+
+        context 'with feature flag disabled' do
+          before do
+            stub_feature_flags(issue_email_participants: false)
+          end
+
+          it 'does not add any participants' do
+            expect { add_emails }.not_to change { issue.issue_email_participants.count }
+          end
+        end
+      end
+    end
   end
 
   describe '#explain' do

spec/support/shared_examples/models/email_format_shared_examples.rb

@@ -6,7 +6,7 @@
 #   Note: You have access to `email_value` which is the email address value
 #         being currently tested).
 
-RSpec.shared_examples 'an object with email-formated attributes' do |*attributes|
+RSpec.shared_examples 'an object with email-formatted attributes' do |*attributes|
   attributes.each do |attribute|
     describe "specifically its :#{attribute} attribute" do
       %w[
@@ -45,7 +45,7 @@ RSpec.shared_examples 'an object with email-formated attributes' do |*attributes
   end
 end
 
-RSpec.shared_examples 'an object with RFC3696 compliant email-formated attributes' do |*attributes|
+RSpec.shared_examples 'an object with RFC3696 compliant email-formatted attributes' do |*attributes|
   attributes.each do |attribute|
     describe "specifically its :#{attribute} attribute" do
       %w[