Automatic merge of gitlab-org/gitlab-ce master

GITALY_SERVER_VERSION

-1.47.0
+1.48.0

app/assets/javascripts/vue_shared/components/project_avatar/image.vue

@@ -17,13 +17,9 @@
 
 import defaultAvatarUrl from 'images/no_avatar.png';
 import { placeholderImage } from '../../../lazy_loader';
-import tooltip from '../../directives/tooltip';
 
 export default {
   name: 'ProjectAvatarImage',
-  directives: {
-    tooltip,
-  },
   props: {
     lazy: {
       type: Boolean,
@@ -50,16 +46,6 @@ export default {
       required: false,
       default: 20,
     },
-    tooltipText: {
-      type: String,
-      required: false,
-      default: '',
-    },
-    tooltipPlacement: {
-      type: String,
-      required: false,
-      default: 'top',
-    },
   },
   computed: {
     // API response sends null when gravatar is disabled and
@@ -71,9 +57,6 @@ export default {
     resultantSrcAttribute() {
       return this.lazy ? placeholderImage : this.sanitizedSource;
     },
-    tooltipContainer() {
-      return this.tooltipText ? 'body' : null;
-    },
     avatarSizeClass() {
       return `s${this.size}`;
     },
@@ -83,7 +66,6 @@ export default {
 
 <template>
   <img
-    v-tooltip
     :class="{
       lazy: lazy,
       [avatarSizeClass]: true,
@@ -94,9 +76,6 @@ export default {
     :height="size"
     :alt="imgAlt"
     :data-src="sanitizedSource"
-    :data-container="tooltipContainer"
-    :data-placement="tooltipPlacement"
-    :title="tooltipText"
     class="avatar"
   />
 </template>

app/mailers/emails/notes.rb

@@ -60,7 +60,7 @@ module Emails
       # `note_id` is a `Note` when originating in `NotifyPreview`
       @note = note_id.is_a?(Note) ? note_id : Note.find(note_id)
       @project = @note.project
-      @group = @note.noteable.try(:group)
+      @group = @project.try(:group) || @note.noteable.try(:group)
 
       if (@project || @group) && @note.persisted?
         @sent_notification = SentNotification.record_note(@note, recipient_id, reply_key)

app/workers/build_success_worker.rb

@@ -9,7 +9,6 @@ class BuildSuccessWorker
   # rubocop: disable CodeReuse/ActiveRecord
   def perform(build_id)
     Ci::Build.find_by(id: build_id).try do |build|
-      create_deployment(build) if build.has_environment?
       stop_environment(build) if build.stops_environment?
     end
   end
@@ -17,17 +16,6 @@ class BuildSuccessWorker
 
   private
 
-  ##
-  # Deprecated:
-  # As of 11.5, we started creating a deployment record when ci_builds record is created.
-  # Therefore we no longer need to create a deployment, after a build succeeded.
-  # We're leaving this code for the transition period, but we can remove this code in 11.6.
-  def create_deployment(build)
-    build.create_deployment.try do |deployment|
-      deployment.succeed
-    end
-  end
-
   ##
   # TODO: This should be processed in DeploymentSuccessWorker once we started storing `action` value in `deployments` records
   def stop_environment(build)

changelogs/unreleased/fix-notes-emails-with-group-settings.yml

+---
+title: Fix comment emails not respecting group-level notification email
+merge_request:
+author:
+type: fixed

changelogs/unreleased/gt-remove-tooltip-directive-on-project-avatar-image-component.yml

+---
+title: Remove tooltip directive on project avatar image component
+merge_request: 29631
+author: George Tsiolis
+type: performance

spec/mailers/emails/pages_domains_spec.rb

@@ -9,7 +9,7 @@ describe Emails::PagesDomains do
   set(:user) { project.creator }
 
   shared_examples 'a pages domain email' do
-    let(:test_recipient) { user }
+    let(:recipient) { user }
 
     it_behaves_like 'an email sent to a user'
     it_behaves_like 'an email sent from GitLab'

spec/mailers/notify_spec.rb

@@ -45,7 +45,7 @@ describe Notify do
 
   context 'for a project' do
     shared_examples 'an assignee email' do
-      let(:test_recipient) { assignee }
+      let(:recipient) { assignee }
 
       it_behaves_like 'an email sent to a user'
 
@@ -55,7 +55,7 @@ describe Notify do
         aggregate_failures do
           expect(sender.display_name).to eq(current_user.name)
           expect(sender.address).to eq(gitlab_sender)
-          expect(subject).to deliver_to(assignee.email)
+          expect(subject).to deliver_to(recipient.notification_email)
         end
       end
     end
@@ -547,12 +547,13 @@ describe Notify do
       let(:host) { Gitlab.config.gitlab.host }
 
       context 'in discussion' do
-        set(:first_note) { create(:discussion_note_on_issue) }
-        set(:second_note) { create(:discussion_note_on_issue, in_reply_to: first_note) }
-        set(:third_note) { create(:discussion_note_on_issue, in_reply_to: second_note) }
+        set(:first_note) { create(:discussion_note_on_issue, project: project) }
+        set(:second_note) { create(:discussion_note_on_issue, in_reply_to: first_note, project: project) }
+        set(:third_note) { create(:discussion_note_on_issue, in_reply_to: second_note, project: project) }
 
         subject { described_class.note_issue_email(recipient.id, third_note.id) }
 
+        it_behaves_like 'an email sent to a user'
         it_behaves_like 'appearance header and footer enabled'
         it_behaves_like 'appearance header and footer not enabled'
 
@@ -572,10 +573,11 @@ describe Notify do
       end
 
       context 'individual issue comments' do
-        set(:note) { create(:note_on_issue) }
+        set(:note) { create(:note_on_issue, project: project) }
 
         subject { described_class.note_issue_email(recipient.id, note.id) }
 
+        it_behaves_like 'an email sent to a user'
         it_behaves_like 'appearance header and footer enabled'
         it_behaves_like 'appearance header and footer not enabled'
 
@@ -604,13 +606,13 @@ describe Notify do
       it_behaves_like 'a user cannot unsubscribe through footer link'
 
       it 'has the correct subject and body' do
-        is_expected.to have_referable_subject(project_snippet, reply: true)
+        is_expected.to have_referable_subject(project_snippet, include_group: true, reply: true)
         is_expected.to have_body_text project_snippet_note.note
       end
     end
 
     describe 'project was moved' do
-      let(:test_recipient) { user }
+      let(:recipient) { user }
       subject { described_class.project_was_moved_email(project.id, user.id, "gitlab/gitlab") }
 
       it_behaves_like 'an email sent to a user'
@@ -811,7 +813,7 @@ describe Notify do
 
         it 'has the correct subject and body' do
           aggregate_failures do
-            is_expected.to have_subject("Re: #{project.name} | #{commit.title} (#{commit.short_id})")
+            is_expected.to have_subject("Re: #{project.name} | #{project.group.name} | #{commit.title} (#{commit.short_id})")
             is_expected.to have_body_text(commit.short_id)
           end
         end
@@ -837,7 +839,7 @@ describe Notify do
 
         it 'has the correct subject and body' do
           aggregate_failures do
-            is_expected.to have_referable_subject(merge_request, reply: true)
+            is_expected.to have_referable_subject(merge_request, include_group: true, reply: true)
             is_expected.to have_body_text note_on_merge_request_path
           end
         end
@@ -863,7 +865,7 @@ describe Notify do
 
         it 'has the correct subject and body' do
           aggregate_failures do
-            is_expected.to have_referable_subject(issue, reply: true)
+            is_expected.to have_referable_subject(issue, include_group: true, reply: true)
             is_expected.to have_body_text(note_on_issue_path)
           end
         end
@@ -929,7 +931,7 @@ describe Notify do
         it_behaves_like 'appearance header and footer not enabled'
 
         it 'has the correct subject' do
-          is_expected.to have_subject "Re: #{project.name} | #{commit.title} (#{commit.short_id})"
+          is_expected.to have_subject "Re: #{project.name} | #{project.group.name} | #{commit.title} (#{commit.short_id})"
         end
 
         it 'contains a link to the commit' do
@@ -957,7 +959,7 @@ describe Notify do
         it_behaves_like 'appearance header and footer not enabled'
 
         it 'has the correct subject' do
-          is_expected.to have_referable_subject(merge_request, reply: true)
+          is_expected.to have_referable_subject(merge_request, include_group: true, reply: true)
         end
 
         it 'contains a link to the merge request note' do
@@ -985,7 +987,7 @@ describe Notify do
         it_behaves_like 'appearance header and footer not enabled'
 
         it 'has the correct subject' do
-          is_expected.to have_referable_subject(issue, reply: true)
+          is_expected.to have_referable_subject(issue, include_group: true, reply: true)
         end
 
         it 'contains a link to the issue note' do

spec/support/helpers/email_helpers.rb

@@ -37,8 +37,19 @@ module EmailHelpers
     ActionMailer::Base.deliveries.find { |d| d.to.include?(user.notification_email) }
   end
 
-  def have_referable_subject(referable, include_project: true, reply: false)
-    prefix = (include_project && referable.project ? "#{referable.project.name} | " : '').freeze
+  def have_referable_subject(referable, include_project: true, include_group: false, reply: false)
+    context = []
+
+    context << referable.project.name if include_project && referable.project
+    context << referable.project.group.name if include_group && referable.project.group
+
+    prefix =
+      if context.any?
+        context.join(' | ') + ' | '
+      else
+        ''
+      end
+
     prefix = "Re: #{prefix}" if reply
 
     suffix = "#{referable.title} (#{referable.to_reference})"

spec/support/shared_examples/notify_shared_examples.rb

@@ -45,18 +45,18 @@ shared_examples 'an email sent to a user' do
   let(:group_notification_email) { 'user+group@example.com' }
 
   it 'is sent to user\'s global notification email address' do
-    expect(subject).to deliver_to(test_recipient.notification_email)
+    expect(subject).to deliver_to(recipient.notification_email)
   end
 
   context 'that is part of a project\'s group' do
     it 'is sent to user\'s group notification email address when set' do
-      create(:notification_setting, user: test_recipient, source: project.group, notification_email: group_notification_email)
+      create(:notification_setting, user: recipient, source: project.group, notification_email: group_notification_email)
       expect(subject).to deliver_to(group_notification_email)
     end
 
     it 'is sent to user\'s global notification email address when no group email set' do
-      create(:notification_setting, user: test_recipient, source: project.group, notification_email: '')
-      expect(subject).to deliver_to(test_recipient.notification_email)
+      create(:notification_setting, user: recipient, source: project.group, notification_email: '')
+      expect(subject).to deliver_to(recipient.notification_email)
     end
   end
 
@@ -67,17 +67,17 @@ shared_examples 'an email sent to a user' do
 
     it 'is sent to user\'s subgroup notification email address when set' do
       # Set top-level group notification email address to make sure it doesn't get selected
-      create(:notification_setting, user: test_recipient, source: group, notification_email: group_notification_email)
+      create(:notification_setting, user: recipient, source: group, notification_email: group_notification_email)
 
       subgroup_notification_email = 'user+subgroup@example.com'
-      create(:notification_setting, user: test_recipient, source: subgroup, notification_email: subgroup_notification_email)
+      create(:notification_setting, user: recipient, source: subgroup, notification_email: subgroup_notification_email)
 
       expect(subject).to deliver_to(subgroup_notification_email)
     end
 
     it 'is sent to user\'s group notification email address when set and subgroup email address not set' do
-      create(:notification_setting, user: test_recipient, source: subgroup, notification_email: '')
-      expect(subject).to deliver_to(test_recipient.notification_email)
+      create(:notification_setting, user: recipient, source: subgroup, notification_email: '')
+      expect(subject).to deliver_to(recipient.notification_email)
     end
   end
 end

spec/workers/build_success_worker_spec.rb

@@ -6,51 +6,7 @@ describe BuildSuccessWorker do
   describe '#perform' do
     subject { described_class.new.perform(build.id) }
 
-    before do
-      allow_any_instance_of(Deployment).to receive(:create_ref)
-    end
-
     context 'when build exists' do
-      context 'when deployment was not created with the build creation' do # An edge case during the transition period
-        let!(:build) { create(:ci_build, :deploy_to_production) }
-
-        before do
-          allow(Deployments::FinishedWorker).to receive(:perform_async)
-          Deployment.delete_all
-          build.reload
-        end
-
-        it 'creates a successful deployment' do
-          expect(build).not_to be_has_deployment
-
-          subject
-
-          build.reload
-          expect(build).to be_has_deployment
-          expect(build.deployment).to be_success
-        end
-      end
-
-      context 'when deployment was created with the build creation' do # Counter part of the above edge case
-        let!(:build) { create(:ci_build, :deploy_to_production) }
-
-        it 'does not create a new deployment' do
-          expect(build).to be_has_deployment
-
-          expect { subject }.not_to change { Deployment.count }
-        end
-      end
-
-      context 'when build is not associated with project' do
-        let!(:build) { create(:ci_build, project: nil) }
-
-        it 'does not create deployment' do
-          subject
-
-          expect(build.reload).not_to be_has_deployment
-        end
-      end
-
       context 'when the build will stop an environment' do
         let!(:build) { create(:ci_build, :stop_review_app, environment: environment.name, project: environment.project) }
         let(:environment) { create(:environment, state: :available) }