Merge branch '55138-fix-mr-discussions-count' into 'master'

Fix MR discussion counts being off

Closes #55138

See merge request gitlab-org/gitlab-ce!23710

3 files changed, 50 insertions, 28 deletions

diff --git a/app/assets/javascripts/notes/stores/mutations.js b/app/assets/javascripts/notes/stores/mutations.js
index a3228f2cfea..39ff0ff73d7 100644
--- a/app/assets/javascripts/notes/stores/mutations.js
+++ b/app/assets/javascripts/notes/stores/mutations.js
@@ -246,7 +246,7 @@ export default {
       discussion =>
         !discussion.individual_note &&
         discussion.resolvable &&
-        discussion.notes.some(note => !note.resolved),
+        discussion.notes.some(note => note.resolvable && !note.resolved),
     ).length;
     state.hasUnresolvedDiscussions = state.unresolvedDiscussionsCount > 1;
   },
diff --git a/changelogs/unreleased/55138-fix-mr-discussions-count.yml b/changelogs/unreleased/55138-fix-mr-discussions-count.yml
new file mode 100644
index 00000000000..667e9b971d8
--- /dev/null
+++ b/changelogs/unreleased/55138-fix-mr-discussions-count.yml
@@ -0,0 +1,5 @@
+---
+title: Fix MR resolved discussion counts being too low
+merge_request: 23710
+author:
+type: fixed
diff --git a/spec/javascripts/notes/stores/mutation_spec.js b/spec/javascripts/notes/stores/mutation_spec.js
index 52cdc16353a..3fbae82f16c 100644
--- a/spec/javascripts/notes/stores/mutation_spec.js
+++ b/spec/javascripts/notes/stores/mutation_spec.js
@@ -9,6 +9,11 @@ import {
   individualNote,
 } from '../mock_data';
 
+const RESOLVED_NOTE = { resolvable: true, resolved: true };
+const UNRESOLVED_NOTE = { resolvable: true, resolved: false };
+const SYSTEM_NOTE = { resolvable: false, resolved: false };
+const WEIRD_NOTE = { resolvable: false, resolved: true };
+
 describe('Notes Store mutations', () => {
   describe('ADD_NEW_NOTE', () => {
     let state;
@@ -449,49 +454,61 @@ describe('Notes Store mutations', () => {
   });
 
   describe('UPDATE_RESOLVABLE_DISCUSSIONS_COUNTS', () => {
-    it('updates resolvableDiscussionsCount', () => {
-      const state = {
-        discussions: [
-          { individual_note: false, resolvable: true, notes: [] },
-          { individual_note: true, resolvable: true, notes: [] },
-          { individual_note: false, resolvable: false, notes: [] },
-        ],
-        resolvableDiscussionsCount: 0,
-      };
-
-      mutations.UPDATE_RESOLVABLE_DISCUSSIONS_COUNTS(state);
-
-      expect(state.resolvableDiscussionsCount).toBe(1);
-    });
-
-    it('updates unresolvedDiscussionsCount', () => {
+    it('with unresolvable discussions, updates state', () => {
       const state = {
         discussions: [
-          { individual_note: false, resolvable: true, notes: [{ resolved: false }] },
-          { individual_note: true, resolvable: true, notes: [{ resolved: false }] },
-          { individual_note: false, resolvable: false, notes: [{ resolved: false }] },
+          { individual_note: false, resolvable: true, notes: [UNRESOLVED_NOTE] },
+          { individual_note: true, resolvable: true, notes: [UNRESOLVED_NOTE] },
+          { individual_note: false, resolvable: false, notes: [UNRESOLVED_NOTE] },
         ],
-        unresolvedDiscussionsCount: 0,
       };
 
       mutations.UPDATE_RESOLVABLE_DISCUSSIONS_COUNTS(state);
 
-      expect(state.unresolvedDiscussionsCount).toBe(1);
+      expect(state).toEqual(
+        jasmine.objectContaining({
+          resolvableDiscussionsCount: 1,
+          unresolvedDiscussionsCount: 1,
+          hasUnresolvedDiscussions: false,
+        }),
+      );
     });
 
-    it('updates hasUnresolvedDiscussions', () => {
+    it('with resolvable discussions, updates state', () => {
       const state = {
         discussions: [
-          { individual_note: false, resolvable: true, notes: [{ resolved: false }] },
-          { individual_note: false, resolvable: true, notes: [{ resolved: false }] },
-          { individual_note: false, resolvable: false, notes: [{ resolved: false }] },
+          {
+            individual_note: false,
+            resolvable: true,
+            notes: [RESOLVED_NOTE, SYSTEM_NOTE, RESOLVED_NOTE],
+          },
+          {
+            individual_note: false,
+            resolvable: true,
+            notes: [RESOLVED_NOTE, SYSTEM_NOTE, WEIRD_NOTE],
+          },
+          {
+            individual_note: false,
+            resolvable: true,
+            notes: [SYSTEM_NOTE, RESOLVED_NOTE, WEIRD_NOTE, UNRESOLVED_NOTE],
+          },
+          {
+            individual_note: false,
+            resolvable: true,
+            notes: [UNRESOLVED_NOTE],
+          },
         ],
-        hasUnresolvedDiscussions: 0,
       };
 
       mutations.UPDATE_RESOLVABLE_DISCUSSIONS_COUNTS(state);
 
-      expect(state.hasUnresolvedDiscussions).toBe(true);
+      expect(state).toEqual(
+        jasmine.objectContaining({
+          resolvableDiscussionsCount: 4,
+          unresolvedDiscussionsCount: 2,
+          hasUnresolvedDiscussions: true,
+        }),
+      );
     });
   });
 });