Merge branch '327737-fix-add-draft-image-comment-shows-in-overview' into 'master'

Fix draft image comments showing in MR overview

See merge request gitlab-org/gitlab!59426

app/assets/javascripts/notes/stores/getters.js

@@ -8,7 +8,7 @@ const getDraftComments = (state) => {
   }
 
   return state.batchComments.drafts
-    .filter((draft) => !draft.line_code && !draft.discussion_id)
+    .filter((draft) => !draft.file_path && !draft.discussion_id)
     .map((x) => ({
       ...x,
       // Treat a top-level draft note as individual_note so it's not included in

spec/factories/draft_note.rb

@@ -9,17 +9,30 @@ FactoryBot.define do
       transient do
         line_number { 14 }
         diff_refs { merge_request.try(:diff_refs) }
+        path { "files/ruby/popen.rb" }
       end
 
       position do
         Gitlab::Diff::Position.new(
-          old_path: "files/ruby/popen.rb",
-          new_path: "files/ruby/popen.rb",
+          old_path: path,
+          new_path: path,
           old_line: nil,
           new_line: line_number,
           diff_refs: diff_refs
         )
       end
+
+      factory :draft_note_on_image_diff do
+        transient do
+          path { "files/images/any_image.png" }
+        end
+
+        position do
+          association(:image_diff_position,
+                file: path,
+                diff_refs: diff_refs)
+        end
+      end
     end
 
     factory :draft_note_on_discussion, traits: [:on_discussion]

spec/features/merge_request/batch_comments_spec.rb

@@ -86,6 +86,19 @@ RSpec.describe 'Merge request > Batch comments', :js do
       expect(page).to have_selector('.draft-note-component', text: 'Testing update')
     end
 
+    context 'with image and file draft note' do
+      let(:merge_request) { create(:merge_request_with_diffs, :with_image_diffs, source_project: project) }
+      let!(:draft_on_text) { create(:draft_note_on_text_diff, merge_request: merge_request, author: user, path: 'README.md', note: 'Lorem ipsum on text...') }
+      let!(:draft_on_image) { create(:draft_note_on_image_diff, merge_request: merge_request, author: user, path: 'files/images/ee_repo_logo.png', note: 'Lorem ipsum on an image...') }
+
+      it 'does not show in overview' do
+        visit_overview
+
+        expect(page).to have_no_text(draft_on_text.note)
+        expect(page).to have_no_text(draft_on_image.note)
+      end
+    end
+
     context 'adding single comment to review' do
       before do
         visit_overview

spec/frontend/notes/components/notes_app_spec.js

@@ -408,10 +408,9 @@ describe('note_app', () => {
       store = createStore();
       store.registerModule('batchComments', batchComments());
       store.state.batchComments.drafts = [
-        { line_code: 1, isDraft: true },
-        { discussion_id: 1, isDraft: true },
-        { note: 'A', isDraft: true },
-        { note: 'B', isDraft: true },
+        mockData.draftDiffDiscussion,
+        mockData.draftReply,
+        ...mockData.draftComments,
       ];
       store.state.isLoading = false;
       wrapper = shallowMount(NotesApp, {
@@ -426,10 +425,9 @@ describe('note_app', () => {
     it('correctly finds only draft comments', () => {
       const drafts = wrapper.findAll(DraftNote).wrappers;
 
-      expect(drafts.map((x) => x.props('draft'))).toEqual([
-        expect.objectContaining({ note: 'A' }),
-        expect.objectContaining({ note: 'B' }),
-      ]);
+      expect(drafts.map((x) => x.props('draft'))).toEqual(
+        mockData.draftComments.map(({ note }) => expect.objectContaining({ note })),
+      );
     });
   });
 });

spec/frontend/notes/mock_data.js

@@ -1273,10 +1273,16 @@ export const batchSuggestionsInfoMock = [
 ];
 
 export const draftComments = [
-  { id: 7, note: 'test draft note' },
-  { id: 9, note: 'draft note 2' },
+  { id: 7, note: 'test draft note', isDraft: true },
+  { id: 9, note: 'draft note 2', isDraft: true },
 ];
 
-export const draftReply = { id: 8, note: 'draft reply', discussion_id: 1 };
+export const draftReply = { id: 8, note: 'draft reply', discussion_id: 1, isDraft: true };
 
-export const draftDiffDiscussion = { id: 6, note: 'draft diff discussion', line_code: 1 };
+export const draftDiffDiscussion = {
+  id: 6,
+  note: 'draft diff discussion',
+  line_code: 1,
+  file_path: 'lib/foo.rb',
+  isDraft: true,
+};