Merge branch 'sy-expose-icon-name' into 'master'

Use server-defined icon for alert system notes

See merge request gitlab-org/gitlab!35902

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

@@ -24,7 +24,7 @@ export default {
       return { ...author, id: id?.split('/').pop() };
     },
     iconHtml() {
-      return spriteIcon('user');
+      return spriteIcon(this.note?.systemNoteIconName);
     },
   },
 };

app/assets/javascripts/alert_management/graphql/fragments/alert_note.fragment.graphql

@@ -13,4 +13,5 @@ fragment AlertNote on Note {
   discussion {
     id
   }
+  systemNoteIconName
 }

app/graphql/types/notes/note_type.rb

@@ -27,6 +27,8 @@ module Types
       field :system, GraphQL::BOOLEAN_TYPE,
             null: false,
             description: 'Indicates whether this note was created by the system or by a user'
+      field :system_note_icon_name, GraphQL::STRING_TYPE, null: true,
+            description: 'Name of the icon corresponding to a system note'
 
       field :body, GraphQL::STRING_TYPE,
             null: false,
@@ -46,6 +48,10 @@ module Types
       field :confidential, GraphQL::BOOLEAN_TYPE, null: true,
             description: 'Indicates if this note is confidential',
             method: :confidential?
+
+      def system_note_icon_name
+        SystemNoteHelper.system_note_icon_name(object) if object.system?
+      end
     end
   end
 end

doc/api/graphql/reference/gitlab_schema.graphql

@@ -8255,6 +8255,11 @@ type Note implements ResolvableInterface {
   """
   system: Boolean!
 
+  """
+  Name of the icon corresponding to a system note
+  """
+  systemNoteIconName: String
+
   """
   Timestamp of the note's last activity
   """

doc/api/graphql/reference/gitlab_schema.json

@@ -24601,6 +24601,20 @@
               "isDeprecated": false,
               "deprecationReason": null
             },
+            {
+              "name": "systemNoteIconName",
+              "description": "Name of the icon corresponding to a system note",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "SCALAR",
+                "name": "String",
+                "ofType": null
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
             {
               "name": "updatedAt",
               "description": "Timestamp of the note's last activity",

doc/api/graphql/reference/index.md

@@ -1262,6 +1262,7 @@ Contains statistics about a milestone
 | `resolvedAt` | Time | Timestamp of when the object was resolved |
 | `resolvedBy` | User | User who resolved the object |
 | `system` | Boolean! | Indicates whether this note was created by the system or by a user |
+| `systemNoteIconName` | String | Name of the icon corresponding to a system note |
 | `updatedAt` | Time! | Timestamp of the note's last activity |
 | `userPermissions` | NotePermissions! | Permissions for the current user on the resource |
 

spec/factories/notes.rb

@@ -159,6 +159,11 @@ FactoryBot.define do
       system { true }
     end
 
+    trait :with_system_note_metadata do
+      system
+      system_note_metadata
+    end
+
     trait :downvote do
       note { "thumbsdown" }
     end

spec/frontend/alert_management/components/system_notes/alert_management_system_note_spec.js

@@ -28,7 +28,11 @@ describe('Alert Details System Note', () => {
     });
 
     it('renders the correct system note', () => {
-      expect(wrapper.find('.note-wrapper').attributes('id')).toBe('note_1628');
+      const noteId = wrapper.find('.note-wrapper').attributes('id');
+      const iconRoute = wrapper.find('use').attributes('href');
+
+      expect(noteId).toBe('note_1628');
+      expect(iconRoute.includes('user')).toBe(true);
     });
   });
 });

spec/frontend/alert_management/mocks/alerts.json

@@ -33,7 +33,8 @@
             "name": "Administrator",
             "username": "root",
             "webUrl": "http://192.168.1.4:3000/root"
-          }
+          },
+          "systemNoteIconName": "user"
         }
       ]
     }

spec/graphql/types/notes/note_type_spec.rb

@@ -19,6 +19,7 @@ RSpec.describe GitlabSchema.types['Note'] do
       resolved_at
       resolved_by
       system
+      system_note_icon_name
       updated_at
       user_permissions
     ]

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

@@ -9,8 +9,8 @@ RSpec.describe 'getting Alert Management Alert Notes' do
   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_it_be(:first_system_note) { create(:note_on_alert, :with_system_note_metadata, noteable: first_alert, project: project) }
+  let_it_be(:second_system_note) { create(:note_on_alert, :with_system_note_metadata, noteable: first_alert, project: project) }
 
   let(:params) { {} }
 
@@ -21,6 +21,8 @@ RSpec.describe 'getting Alert Management Alert Notes' do
         notes {
           nodes {
             id
+            body
+            systemNoteIconName
           }
         }
       }
@@ -44,7 +46,17 @@ RSpec.describe 'getting Alert Management Alert Notes' do
     project.add_developer(current_user)
   end
 
-  it 'returns the notes ordered by createdAt' do
+  it 'includes expected data' do
+    post_graphql(query, current_user: current_user)
+
+    expect(first_notes_result.first).to include(
+      'id' => first_system_note.to_global_id.to_s,
+      'systemNoteIconName' => 'git-merge',
+      'body' => first_system_note.note
+    )
+  end
+
+  it 'returns the notes ordered by createdAt with sufficient content' do
     post_graphql(query, current_user: current_user)
 
     expect(first_notes_result.length).to eq(2)
@@ -64,4 +76,18 @@ RSpec.describe 'getting Alert Management Alert Notes' do
     expect { post_graphql(query, current_user: current_user) }.not_to exceed_query_limit(base_count)
     expect(alerts_result.length).to eq(3)
   end
+
+  context 'for non-system notes' do
+    let_it_be(:user_note) { create(:note_on_alert, noteable: second_alert, project: project) }
+
+    it 'includes expected data' do
+      post_graphql(query, current_user: current_user)
+
+      expect(second_notes_result.first).to include(
+        'id' => user_note.to_global_id.to_s,
+        'systemNoteIconName' => nil,
+        'body' => user_note.note
+      )
+    end
+  end
 end