Merge branch 'alert-system-notes-tool-tip' into 'master'

Alert assignee feature flag removal

Closes #218409

See merge request gitlab-org/gitlab!34360

app/assets/javascripts/alert_management/components/alert_details.vue

@@ -15,7 +15,8 @@ import { s__ } from '~/locale';
 import query from '../graphql/queries/details.query.graphql';
 import { fetchPolicies } from '~/lib/graphql';
 import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
-import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
+import highlightCurrentUser from '~/behaviors/markdown/highlight_current_user';
+import initUserPopovers from '~/user_popovers';
 import { ALERTS_SEVERITY_LABELS, trackAlertsDetailsViewsOptions } from '../constants';
 import createIssueQuery from '../graphql/mutations/create_issue_from_alert.graphql';
 import { visitUrl, joinPaths } from '~/lib/utils/url_utility';
@@ -51,7 +52,6 @@ export default {
     AlertSidebar,
     SystemNote,
   },
-  mixins: [glFeatureFlagsMixin()],
   props: {
     alertId: {
       type: String,
@@ -116,6 +116,12 @@ export default {
       'right-sidebar-expanded': true,
     });
   },
+  updated() {
+    this.$nextTick(() => {
+      highlightCurrentUser(this.$el.querySelectorAll('.gfm-project_member'));
+      initUserPopovers(this.$el.querySelectorAll('.js-user-link'));
+    });
+  },
   methods: {
     dismissError() {
       this.isErrorDismissed = true;
@@ -187,7 +193,7 @@ export default {
     <div
       v-if="alert"
       class="alert-management-details gl-relative"
-      :class="{ 'pr-8': sidebarCollapsed }"
+      :class="{ 'pr-sm-8': sidebarCollapsed }"
     >
       <div
         class="gl-display-flex gl-justify-content-space-between gl-align-items-baseline gl-px-1 py-3 py-md-4 gl-border-b-1 gl-border-b-gray-200 gl-border-b-solid flex-column flex-sm-row"
@@ -294,8 +300,8 @@ export default {
             </div>
             <div class="gl-pl-2" data-testid="service">{{ alert.service }}</div>
           </div>
-          <template v-if="glFeatures.alertAssignee">
-            <div v-if="alert.notes" class="issuable-discussion">
+          <template>
+            <div v-if="alert.notes.nodes" class="issuable-discussion py-5">
               <ul class="notes main-notes-list timeline">
                 <system-note v-for="note in alert.notes.nodes" :key="note.id" :note="note" />
               </ul>

app/assets/javascripts/alert_management/components/alert_sidebar.vue

 <script>
-import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import SidebarHeader from './sidebar/sidebar_header.vue';
 import SidebarTodo from './sidebar/sidebar_todo.vue';
 import SidebarStatus from './sidebar/sidebar_status.vue';
@@ -12,7 +11,6 @@ export default {
     SidebarTodo,
     SidebarStatus,
   },
-  mixins: [glFeatureFlagsMixin()],
   props: {
     sidebarCollapsed: {
       type: Boolean,
@@ -50,14 +48,13 @@ export default {
         @alert-sidebar-error="$emit('alert-sidebar-error', $event)"
       />
       <sidebar-assignees
-        v-if="glFeatures.alertAssignee"
         :project-path="projectPath"
         :alert="alert"
+        :sidebar-collapsed="sidebarCollapsed"
         @alert-refresh="$emit('alert-refresh')"
         @toggle-sidebar="$emit('toggle-sidebar')"
         @alert-sidebar-error="$emit('alert-sidebar-error', $event)"
       />
-      <!-- TODO: Remove after adding extra attribute blocks to sidebar -->
       <div class="block"></div>
     </div>
   </aside>

app/assets/javascripts/alert_management/components/sidebar/sidebar_assignees.vue

@@ -51,6 +51,10 @@ export default {
       required: false,
       default: true,
     },
+    sidebarCollapsed: {
+      type: Boolean,
+      required: false,
+    },
   },
   data() {
     return {
@@ -62,10 +66,19 @@ export default {
     };
   },
   computed: {
-    assignedUsers() {
-      return this.alert.assignees.nodes.length > 0
-        ? this.alert.assignees.nodes[0].username
-        : s__('AlertManagement|Unassigned');
+    currentUser() {
+      return gon?.current_username;
+    },
+    userName() {
+      return this.alert?.assignees?.nodes[0]?.username;
+    },
+    assignedUser() {
+      return this.userName || s__('AlertManagement|None');
+    },
+    sortedUsers() {
+      return this.users
+        .map(user => ({ ...user, active: this.isActive(user.username) }))
+        .sort((a, b) => (a.active === b.active ? 0 : a.active ? -1 : 1)); // eslint-disable-line no-nested-ternary
     },
     dropdownClass() {
       return this.isDropdownShowing ? 'show' : 'gl-display-none';
@@ -115,7 +128,7 @@ export default {
             per_page: 20,
             active: true,
             current_user: true,
-            project_id: gon.current_project_id,
+            project_id: gon?.current_project_id,
           },
         })
         .then(({ data }) => {
@@ -159,12 +172,11 @@ export default {
     <div ref="status" class="sidebar-collapsed-icon" @click="$emit('toggle-sidebar')">
       <gl-icon name="user" :size="14" />
       <gl-loading-icon v-if="isUpdating" />
-      <p v-else class="collapse-truncated-title px-1">{{ assignedUsers }}</p>
     </div>
     <gl-tooltip :target="() => $refs.status" boundary="viewport" placement="left">
       <gl-sprintf :message="s__('AlertManagement|Alert assignee(s): %{assignees}')">
         <template #assignees>
-          {{ assignedUsers }}
+          {{ assignedUser }}
         </template>
       </gl-sprintf>
     </gl-tooltip>
@@ -187,7 +199,7 @@ export default {
       <div class="dropdown dropdown-menu-selectable" :class="dropdownClass">
         <gl-dropdown
           ref="dropdown"
-          :text="assignedUsers"
+          :text="assignedUser"
           class="w-100"
           toggle-class="dropdown-menu-toggle"
           variant="outline-default"
@@ -195,7 +207,7 @@ export default {
           @hide="hideDropdown"
         >
           <div class="dropdown-title">
-            <span class="alert-title">{{ s__('AlertManagement|Assign Assignees') }}</span>
+            <span class="alert-title">{{ s__('AlertManagement|Assign To') }}</span>
             <gl-button
               :aria-label="__('Close')"
               variant="link"
@@ -215,34 +227,25 @@ export default {
           </div>
           <div class="dropdown-content dropdown-body">
             <template v-if="userListValid">
-              <gl-dropdown-item @click="updateAlertAssignees('')">
+              <gl-dropdown-item
+                :active="!userName"
+                active-class="is-active"
+                @click="updateAlertAssignees('')"
+              >
                 {{ s__('AlertManagement|Unassigned') }}
               </gl-dropdown-item>
               <gl-dropdown-divider />
 
               <gl-dropdown-header class="mt-0">
-                {{ s__('AlertManagement|Assignee(s)') }}
+                {{ s__('AlertManagement|Assignee') }}
               </gl-dropdown-header>
-
-              <template v-for="user in users">
-                <sidebar-assignee
-                  v-if="isActive(user.username)"
-                  :key="user.username"
-                  :user="user"
-                  :active="true"
-                  @update-alert-assignees="updateAlertAssignees"
-                />
-              </template>
-              <gl-dropdown-divider />
-              <template v-for="user in users">
-                <sidebar-assignee
-                  v-if="!isActive(user.username)"
-                  :key="user.username"
-                  :user="user"
-                  :active="false"
-                  @update-alert-assignees="updateAlertAssignees"
-                />
-              </template>
+              <sidebar-assignee
+                v-for="user in sortedUsers"
+                :key="user.username"
+                :user="user"
+                :active="user.active"
+                @update-alert-assignees="updateAlertAssignees"
+              />
             </template>
             <gl-dropdown-item v-else-if="userListEmpty">
               {{ s__('AlertManagement|No Matching Results') }}
@@ -253,16 +256,21 @@ export default {
       </div>
 
       <gl-loading-icon v-if="isUpdating" :inline="true" />
-      <p
-        v-else-if="!isDropdownShowing"
-        class="value gl-m-0"
-        :class="{ 'no-value': !alert.assignees.nodes }"
-      >
-        <span v-if="alert.assignees.nodes" class="gl-text-gray-700" data-testid="assigned-users">{{
-          assignedUsers
+      <p v-else-if="!isDropdownShowing" class="value gl-m-0" :class="{ 'no-value': !userName }">
+        <span v-if="userName" class="gl-text-gray-700" data-testid="assigned-users">{{
+          assignedUser
         }}</span>
-        <span v-else>
-          {{ s__('AlertManagement|None') }}
+        <span v-else class="gl-display-flex gl-align-items-center">
+          {{ s__('AlertManagement|None -') }}
+          <gl-button
+            class="gl-pl-2"
+            href="#"
+            variant="link"
+            data-testid="unassigned-users"
+            @click="updateAlertAssignees(currentUser)"
+          >
+            {{ s__('AlertManagement| assign yourself') }}
+          </gl-button>
         </span>
       </p>
     </div>

app/assets/javascripts/alert_management/components/system_notes/system_note.vue

@@ -16,6 +16,13 @@ export default {
     noteAnchorId() {
       return `note_${this.note?.id?.split('/').pop()}`;
     },
+    noteAuthor() {
+      const {
+        author,
+        author: { id },
+      } = this.note;
+      return { ...author, id: id?.split('/').pop() };
+    },
     iconHtml() {
       return spriteIcon('user');
     },
@@ -29,7 +36,7 @@ export default {
       <div class="timeline-icon" v-html="iconHtml"></div>
       <div class="timeline-content">
         <div class="note-header">
-          <note-header :author="note.author" :created-at="note.createdAt" :note-id="note.id">
+          <note-header :author="noteAuthor" :created-at="note.createdAt" :note-id="note.id">
             <span v-html="note.bodyHtml"></span>
           </note-header>
         </div>

app/assets/stylesheets/pages/alert_management/details.scss

@@ -51,13 +51,23 @@
   }
 
   .assignee-dropdown-item {
-    button {
+    .dropdown-item {
       display: flex;
       align-items: center;
 
       &::before {
         top: 50% !important;
       }
+
+      &.is-active {
+        &:last-child {
+          border-bottom: 1px solid $gray-200;
+        }
+      }
     }
   }
+
+  .note-header-info {
+    margin-top: 1px;
+  }
 }

app/controllers/projects/alert_management_controller.rb

@@ -2,9 +2,6 @@
 
 class Projects::AlertManagementController < Projects::ApplicationController
   before_action :authorize_read_alert_management_alert!
-  before_action do
-    push_frontend_feature_flag(:alert_assignee, project)
-  end
 
   def index
   end

app/graphql/resolvers/alert_management/alert_resolver.rb

@@ -33,7 +33,8 @@ module Resolvers
 
       def preloads
         {
-          assignees: [:assignees]
+          assignees: [:assignees],
+          notes: [:ordered_notes, { ordered_notes: [:system_note_metadata, :project, :noteable] }]
         }
       end
     end

app/graphql/types/alert_management/alert_type.rb

@@ -91,10 +91,8 @@ module Types
             null: true,
             description: 'Assignees of the alert'
 
-      def assignees
-        return User.none unless Feature.enabled?(:alert_assignee, object.project)
-
-        object.assignees
+      def notes
+        object.ordered_notes
       end
     end
   end

app/models/alert_management/alert.rb

@@ -32,6 +32,7 @@ module AlertManagement
     has_many :assignees, through: :alert_assignees
 
     has_many :notes, as: :noteable, inverse_of: :noteable, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
+    has_many :ordered_notes, -> { fresh }, as: :noteable, class_name: 'Note'
     has_many :user_mentions, class_name: 'AlertManagement::AlertUserMention', foreign_key: :alert_management_alert_id
 
     has_internal_id :iid, scope: :project, init: ->(s) { s.project.alert_management_alerts.maximum(:iid) }

changelogs/unreleased/alert-system-notes-tool-tip.yml

+---
+title: Enable ability to assign alerts to users with corresponding system notes and todos
+merge_request: 34360
+author:
+type: added

locale/gitlab.pot

@@ -1852,6 +1852,9 @@ msgid_plural "Alerts"
 msgstr[0] ""
 msgstr[1] ""
 
+msgid "AlertManagement| assign yourself"
+msgstr ""
+
 msgid "AlertManagement|Acknowledged"
 msgstr ""
 
@@ -1876,7 +1879,7 @@ msgstr ""
 msgid "AlertManagement|All alerts"
 msgstr ""
 
-msgid "AlertManagement|Assign Assignees"
+msgid "AlertManagement|Assign To"
 msgstr ""
 
 msgid "AlertManagement|Assign status"
@@ -1885,9 +1888,6 @@ msgstr ""
 msgid "AlertManagement|Assignee"
 msgstr ""
 
-msgid "AlertManagement|Assignee(s)"
-msgstr ""
-
 msgid "AlertManagement|Assignees"
 msgstr ""
 
@@ -1942,6 +1942,9 @@ msgstr ""
 msgid "AlertManagement|None"
 msgstr ""
 
+msgid "AlertManagement|None -"
+msgstr ""
+
 msgid "AlertManagement|Open"
 msgstr ""
 

spec/frontend/alert_management/components/alert_managment_sidebar_assignees_spec.js

@@ -127,7 +127,7 @@ describe('Alert Details Sidebar Assignees', () => {
     it('stops updating and cancels loading when the request fails', () => {
       jest.spyOn(wrapper.vm.$apollo, 'mutate').mockReturnValue(Promise.reject(new Error()));
       wrapper.vm.updateAlertAssignees('root');
-      expect(wrapper.find('[data-testid="assigned-users"]').text()).toBe('Unassigned');
+      expect(wrapper.find('[data-testid="unassigned-users"]').text()).toBe('assign yourself');
     });
   });
 });

spec/frontend/alert_management/components/alert_sidebar_spec.js

@@ -14,7 +14,6 @@ describe('Alert Details Sidebar', () => {
   function mountComponent({
     sidebarCollapsed = true,
     mountMethod = shallowMount,
-    alertAssignee = false,
     stubs = {},
     alert = {},
   } = {}) {
@@ -24,9 +23,6 @@ describe('Alert Details Sidebar', () => {
         sidebarCollapsed,
         projectPath: 'projectPath',
       },
-      provide: {
-        glFeatures: { alertAssignee },
-      },
       stubs,
     });
   }
@@ -48,14 +44,9 @@ describe('Alert Details Sidebar', () => {
       expect(wrapper.props('sidebarCollapsed')).toBe(true);
     });
 
-    it('should not render side bar assignee dropdown by default', () => {
-      expect(wrapper.find(SidebarAssignees).exists()).toBe(false);
-    });
-
-    it('should render side bar assignee dropdown if feature flag enabled', () => {
+    it('should render side bar assignee dropdown', () => {
       mountComponent({
         mountMethod: mount,
-        alertAssignee: true,
         alert: mockAlert,
       });
       expect(wrapper.find(SidebarAssignees).exists()).toBe(true);

spec/models/alert_management/alert_spec.rb

@@ -8,6 +8,7 @@ describe AlertManagement::Alert do
     it { is_expected.to belong_to(:issue) }
     it { is_expected.to have_many(:assignees).through(:alert_assignees) }
     it { is_expected.to have_many(:notes) }
+    it { is_expected.to have_many(:ordered_notes) }
     it { is_expected.to have_many(:user_mentions) }
   end
 

spec/requests/api/graphql/project/alert_management/alert/assignees_spec.rb

@@ -75,17 +75,4 @@ describe 'getting Alert Management Alert Assignees' do
     expect(third_assignees.length).to eq(1)
     expect(third_assignees.first).to include('username' => current_user.username)
   end
-
-  context 'with alert_assignee flag disabled' do
-    before do
-      stub_feature_flags(alert_assignee: false)
-    end
-
-    it 'excludes assignees' do
-      post_graphql(query, current_user: current_user)
-
-      expect(first_assignees).to be_empty
-      expect(second_assignees).to be_empty
-    end
-  end
 end

spec/requests/api/graphql/project/alert_management/alert/notes_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'getting Alert Management Alert Notes' do
+  include GraphqlHelpers
+
+  let_it_be(:project) { create(:project) }
+  let_it_be(:current_user) { create(:user) }
+  let_it_be(:first_alert) { create(:alert_management_alert, project: project, assignees: [current_user]) }
+  let_it_be(:second_alert) { create(:alert_management_alert, project: project) }
+  let_it_be(:first_system_note) { create(:note_on_alert, noteable: first_alert, project: project) }
+  let_it_be(:second_system_note) { create(:note_on_alert, noteable: first_alert, project: project) }
+
+  let(:params) { {} }
+
+  let(:fields) do
+    <<~QUERY
+      nodes {
+        iid
+        notes {
+          nodes {
+            id
+          }
+        }
+      }
+    QUERY
+  end
+
+  let(:query) do
+    graphql_query_for(
+      'project',
+      { 'fullPath' => project.full_path },
+      query_graphql_field('alertManagementAlerts', params, fields)
+    )
+  end
+
+  let(:alerts_result) { graphql_data.dig('project', 'alertManagementAlerts', 'nodes') }
+  let(:notes_result) { alerts_result.map { |alert| [alert['iid'], alert['notes']['nodes']] }.to_h }
+  let(:first_notes_result) { notes_result[first_alert.iid.to_s] }
+  let(:second_notes_result) { notes_result[second_alert.iid.to_s] }
+
+  before do
+    project.add_developer(current_user)
+  end
+
+  it 'returns the notes ordered by createdAt' do
+    post_graphql(query, current_user: current_user)
+
+    expect(first_notes_result.length).to eq(2)
+    expect(first_notes_result.first).to include('id' => first_system_note.to_global_id.to_s)
+    expect(first_notes_result.second).to include('id' => second_system_note.to_global_id.to_s)
+    expect(second_notes_result).to be_empty
+  end
+
+  it 'avoids N+1 queries' do
+    base_count = ActiveRecord::QueryRecorder.new do
+      post_graphql(query, current_user: current_user)
+    end
+
+    # An N+1 would mean a new alert would increase the query count
+    create(:alert_management_alert, project: project)
+
+    expect { post_graphql(query, current_user: current_user) }.not_to exceed_query_limit(base_count)
+    expect(alerts_result.length).to eq(3)
+  end
+end

spec/requests/api/graphql/project/alert_management/alerts_spec.rb

@@ -10,7 +10,6 @@ describe 'getting Alert Management Alerts' do
   let_it_be(:resolved_alert) { create(:alert_management_alert, :all_fields, :resolved, project: project, issue: nil, severity: :low) }
   let_it_be(:triggered_alert) { create(:alert_management_alert, :all_fields, project: project, severity: :critical, payload: payload) }
   let_it_be(:other_project_alert) { create(:alert_management_alert, :all_fields) }
-  let_it_be(:system_note) { create(:note_on_alert, noteable: triggered_alert, project: project) }
 
   let(:params) { {} }
 
@@ -77,8 +76,6 @@ describe 'getting Alert Management Alerts' do
           'updatedAt' => triggered_alert.updated_at.strftime('%Y-%m-%dT%H:%M:%SZ')
         )
 
-        expect(first_alert['notes']['nodes'].first).to include('id' => system_note.to_global_id.to_s)
-
         expect(second_alert).to include(
           'iid' => resolved_alert.iid.to_s,
           'issueIid' => nil,