[MergeDups] Prevent merged sets from showing up again (#2869)

docs/user_guide/docs/goals.md

@@ -112,7 +112,7 @@ next set: "Save & Continue" and "Defer".
 ![Merge Duplicates Save and Continue button](images/mergeSaveAndContinue.png)
 
 The blue "Save and Continue" button does two things. First, it saves all changes made (i.e., all moved, merged, or
-deleted senses), updating the words in the database. Second, it saves any unmerged words as non-duplicates.
+deleted senses), updating the words in the database. Second, it saves the resulting set of words as non-duplicates.
 
 !!! tip "Tip"
 

src/goals/MergeDuplicates/Redux/MergeDupsActions.ts

@@ -104,20 +104,27 @@ export function deferMerge() {
   };
 }
 
+/** Dispatch function to construct all merges from the current merge tree.
+ * Each word with all senses deleted results in a `MergeWord` with `deleteOnly: true`.
+ * Each word column with no changes is ignored.
+ * Each word column with any changes results in a `MergeWord` with `deleteOnly: false`.
+ * The resulting `MergeWord` array is sent to the backend for merging.
+ * Also, the merges are added as changes to the current goal.
+ * Also, the new set of ids (for merge parents and unchanged words) is blacklisted. */
 export function mergeAll() {
   return async (dispatch: StoreStateDispatch, getState: () => StoreState) => {
-    const mergeTree = getState().mergeDuplicateGoal;
-
-    // Add to blacklist.
-    await backend.blacklistAdd(Object.keys(mergeTree.data.words));
-
-    // Merge words.
+    // Get MergeWord array from the state.
     dispatch(getMergeWords());
-
-    const mergeWordsArray = [...getState().mergeDuplicateGoal.mergeWords];
+    const mergeTree = getState().mergeDuplicateGoal;
+    const mergeWordsArray = [...mergeTree.mergeWords];
     dispatch(clearMergeWords());
+
+    let parentIds: string[] = [];
     if (mergeWordsArray.length) {
-      const parentIds = await backend.mergeWords(mergeWordsArray);
+      // Send merges to the backend.
+      parentIds = await backend.mergeWords(mergeWordsArray);
+
+      // Add merges as changes to the goal.
       const childIds = [
         ...new Set(
           mergeWordsArray.flatMap((m) => m.children).map((s) => s.srcWordId)
@@ -127,6 +134,16 @@ export function mergeAll() {
       dispatch(addCompletedMergeToGoal(completedMerge));
       await dispatch(asyncUpdateGoal());
     }
+
+    // Blacklist the set of words with updated ids.
+    const mergedIds = mergeWordsArray.map((mw) => mw.parent.id);
+    const unmergedIds = Object.keys(mergeTree.data.words).filter(
+      (id) => !mergedIds.includes(id)
+    );
+    const blacklistIds = [...unmergedIds, ...parentIds];
+    if (blacklistIds.length > 1) {
+      await backend.blacklistAdd(blacklistIds);
+    }
   };
 }
 

src/goals/MergeDuplicates/Redux/tests/MergeDupsActions.test.tsx

@@ -30,13 +30,14 @@ function wordAnyGuids(vern: string, senses: Sense[], id: string): Word {
   };
 }
 
+const mockBlacklistAdd = jest.fn();
 const mockGraylistAdd = jest.fn();
 const mockMergeWords = jest.fn();
 
 jest.mock("backend", () => ({
-  blacklistAdd: jest.fn(),
+  blacklistAdd: (ids: string[]) => mockBlacklistAdd(ids),
   getWord: jest.fn(),
-  graylistAdd: () => mockGraylistAdd(),
+  graylistAdd: (ids: string[]) => mockGraylistAdd(ids),
   mergeWords: (mergeWordsArray: MergeWords[]) =>
     mockMergeWords(mergeWordsArray),
 }));
@@ -89,7 +90,12 @@ const data: MergeData = {
   },
 };
 
-beforeEach(jest.clearAllMocks);
+beforeEach(() => {
+  jest.clearAllMocks();
+  mockMergeWords.mockImplementation((mwArray: MergeWords[]) =>
+    mwArray.filter((mw) => !mw.deleteOnly).map((mw) => mw.parent.id + "+")
+  );
+});
 
 describe("MergeDupActions", () => {
   describe("mergeAll", () => {
@@ -105,6 +111,12 @@ describe("MergeDupActions", () => {
       await store.dispatch(mergeAll());
 
       expect(mockMergeWords).not.toHaveBeenCalled();
+
+      expect(mockBlacklistAdd).toHaveBeenCalledTimes(1);
+      const blacklist = mockBlacklistAdd.mock.calls[0][0];
+      expect(blacklist).toHaveLength(2);
+      expect(blacklist).toContain(idA);
+      expect(blacklist).toContain(idB);
     });
 
     // Merge sense 3 from B as duplicate into sense 1 from A
@@ -130,6 +142,12 @@ describe("MergeDupActions", () => {
       for (const mergeWords of mockMerges) {
         expect(mockMergeWords.mock.calls[0][0]).toContainEqual(mergeWords);
       }
+
+      expect(mockBlacklistAdd).toHaveBeenCalledTimes(1);
+      const blacklist = mockBlacklistAdd.mock.calls[0][0];
+      expect(blacklist).toHaveLength(2);
+      expect(blacklist).not.toContain(idA);
+      expect(blacklist).not.toContain(idB);
     });
 
     // Move sense 3 from B to A
@@ -159,6 +177,12 @@ describe("MergeDupActions", () => {
       for (const mergeWords of mockMerges) {
         expect(mockMergeWords.mock.calls[0][0]).toContainEqual(mergeWords);
       }
+
+      expect(mockBlacklistAdd).toHaveBeenCalledTimes(1);
+      const blacklist = mockBlacklistAdd.mock.calls[0][0];
+      expect(blacklist).toHaveLength(2);
+      expect(blacklist).not.toContain(idA);
+      expect(blacklist).not.toContain(idB);
     });
 
     // Merge sense 1 and 2 in A as duplicates
@@ -178,6 +202,12 @@ describe("MergeDupActions", () => {
       const child = { srcWordId: idA, getAudio: true };
       const mockMerge = newMergeWords(parent, [child]);
       expect(mockMergeWords).toHaveBeenCalledWith([mockMerge]);
+
+      expect(mockBlacklistAdd).toHaveBeenCalledTimes(1);
+      const blacklist = mockBlacklistAdd.mock.calls[0][0];
+      expect(blacklist).toHaveLength(2);
+      expect(blacklist).not.toContain(idA);
+      expect(blacklist).toContain(idB);
     });
 
     // Delete sense 2 from A
@@ -196,6 +226,12 @@ describe("MergeDupActions", () => {
       const child = { srcWordId: idA, getAudio: true };
       const mockMerge = newMergeWords(parent, [child]);
       expect(mockMergeWords).toHaveBeenCalledWith([mockMerge]);
+
+      expect(mockBlacklistAdd).toHaveBeenCalledTimes(1);
+      const blacklist = mockBlacklistAdd.mock.calls[0][0];
+      expect(blacklist).toHaveLength(2);
+      expect(blacklist).not.toContain(idA);
+      expect(blacklist).toContain(idB);
     });
 
     // Delete both senses from B
@@ -212,6 +248,9 @@ describe("MergeDupActions", () => {
       const child = { srcWordId: idB, getAudio: false };
       const mockMerge = newMergeWords(wordB, [child], true);
       expect(mockMergeWords).toHaveBeenCalledWith([mockMerge]);
+
+      // No blacklist entry added for only 1 resulting word.
+      expect(mockBlacklistAdd).not.toHaveBeenCalled();
     });
 
     // Performs a merge when a word is flagged
@@ -233,6 +272,12 @@ describe("MergeDupActions", () => {
       const child = { srcWordId: idA, getAudio: true };
       const mockMerge = newMergeWords(parent, [child]);
       expect(mockMergeWords).toHaveBeenCalledWith([mockMerge]);
+
+      expect(mockBlacklistAdd).toHaveBeenCalledTimes(1);
+      const blacklist = mockBlacklistAdd.mock.calls[0][0];
+      expect(blacklist).toHaveLength(2);
+      expect(blacklist).not.toContain(idA);
+      expect(blacklist).toContain(idB);
     });
   });