diff --git a/docs/user_guide/docs/goals.md b/docs/user_guide/docs/goals.md index e3253fd19c..2a91976807 100644 --- a/docs/user_guide/docs/goals.md +++ b/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" diff --git a/src/goals/MergeDuplicates/Redux/MergeDupsActions.ts b/src/goals/MergeDuplicates/Redux/MergeDupsActions.ts index ea0929456a..44781642c0 100644 --- a/src/goals/MergeDuplicates/Redux/MergeDupsActions.ts +++ b/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); + } }; } diff --git a/src/goals/MergeDuplicates/Redux/tests/MergeDupsActions.test.tsx b/src/goals/MergeDuplicates/Redux/tests/MergeDupsActions.test.tsx index 31e36f5c1d..87c17dd840 100644 --- a/src/goals/MergeDuplicates/Redux/tests/MergeDupsActions.test.tsx +++ b/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); }); });