Create system notes for group milestone changes

app/services/issuable_base_service.rb

@@ -2,11 +2,8 @@ class IssuableBaseService < BaseService
   private
 
   def create_milestone_note(issuable)
-    milestone = issuable.milestone
-    return if milestone && milestone.is_group_milestone?
-
     SystemNoteService.change_milestone(
-      issuable, issuable.project, current_user, milestone)
+      issuable, issuable.project, current_user, issuable.milestone)
   end
 
   def create_labels_note(issuable, old_labels)

app/services/system_note_service.rb

@@ -142,7 +142,8 @@ module SystemNoteService
   #
   # Returns the created Note object
   def change_milestone(noteable, project, author, milestone)
-    body = milestone.nil? ? 'removed milestone' : "changed milestone to #{milestone.to_reference(project)}"
+    format = milestone&.is_group_milestone? ? :name : :iid
+    body = milestone.nil? ? 'removed milestone' : "changed milestone to #{milestone.to_reference(project, format: format)}"
 
     create_note(NoteSummary.new(noteable, project, author, body, action: 'milestone'))
   end

spec/services/system_note_service_spec.rb

@@ -3,7 +3,8 @@ require 'spec_helper'
 describe SystemNoteService do
   include Gitlab::Routing
 
-  let(:project)  { create(:project) }
+  let(:group)    { create(:group) }
+  let(:project)  { create(:project, group: group) }
   let(:author)   { create(:user) }
   let(:noteable) { create(:issue, project: project) }
   let(:issue)    { noteable }
@@ -242,25 +243,51 @@ describe SystemNoteService do
   end
 
   describe '.change_milestone' do
-    subject { described_class.change_milestone(noteable, project, author, milestone) }
+    context 'for a project milestone' do
+      subject { described_class.change_milestone(noteable, project, author, milestone) }
 
-    let(:milestone) { create(:milestone, project: project) }
+      let(:milestone) { create(:milestone, project: project) }
 
-    it_behaves_like 'a system note' do
-      let(:action) { 'milestone' }
-    end
+      it_behaves_like 'a system note' do
+        let(:action) { 'milestone' }
+      end
 
-    context 'when milestone added' do
-      it 'sets the note text' do
-        expect(subject.note).to eq "changed milestone to #{milestone.to_reference}"
+      context 'when milestone added' do
+        it 'sets the note text' do
+          expect(subject.note).to eq "changed milestone to #{milestone.to_reference}"
+        end
+      end
+
+      context 'when milestone removed' do
+        let(:milestone) { nil }
+
+        it 'sets the note text' do
+          expect(subject.note).to eq 'removed milestone'
+        end
       end
     end
 
-    context 'when milestone removed' do
-      let(:milestone) { nil }
+    context 'for a group milestone' do
+      subject { described_class.change_milestone(noteable, project, author, milestone) }
 
-      it 'sets the note text' do
-        expect(subject.note).to eq 'removed milestone'
+      let(:milestone) { create(:milestone, group: group) }
+
+      it_behaves_like 'a system note' do
+        let(:action) { 'milestone' }
+      end
+
+      context 'when milestone added' do
+        it 'sets the note text to use the milestone name' do
+          expect(subject.note).to eq "changed milestone to #{milestone.to_reference(format: :name)}"
+        end
+      end
+
+      context 'when milestone removed' do
+        let(:milestone) { nil }
+
+        it 'sets the note text' do
+          expect(subject.note).to eq 'removed milestone'
+        end
       end
     end
   end

spec/support/issuable_shared_examples.rb

@@ -21,15 +21,15 @@ shared_examples 'system notes for milestones' do
       create(:group_member, group: group, user: user)
     end
 
-    it 'does not create system note' do
+    it 'creates a system note' do
       expect do
         update_issuable(milestone: group_milestone)
-      end.not_to change { Note.system.count }
+      end.to change { Note.system.count }.by(1)
     end
   end
 
   context 'project milestones' do
-    it 'creates system note' do
+    it 'creates a system note' do
       expect do
         update_issuable(milestone: create(:milestone))
       end.to change { Note.system.count }.by(1)