diff --git a/src/goals/MergeDupGoal/MergeDupStep/MergeDupStepActions.tsx b/src/goals/MergeDupGoal/MergeDupStep/MergeDupStepActions.tsx index f3967aed7f..939bb6b4d9 100644 --- a/src/goals/MergeDupGoal/MergeDupStep/MergeDupStepActions.tsx +++ b/src/goals/MergeDupGoal/MergeDupStep/MergeDupStepActions.tsx @@ -19,79 +19,82 @@ import { MergeDups, MergeStepData } from "../MergeDups"; import { Hash, MergeTreeReference, TreeDataSense } from "./MergeDupsTree"; export enum MergeTreeActions { - SET_VERNACULAR = "SET_VERNACULAR", - SET_PLURAL = "SET_PLURAL", + CLEAR_TREE = "CLEAR_TREE", MOVE_SENSE = "MOVE_SENSE", + ORDER_DUPLICATE = "ORDER_DUPLICATE", ORDER_SENSE = "ORDER_SENSE", - SET_SENSE = "SET_SENSE", SET_DATA = "SET_DATA", - CLEAR_TREE = "CLEAR_TREE", - ORDER_DUPLICATE = "ORDER_DUPLICATE", + SET_PLURAL = "SET_PLURAL", + SET_SENSE = "SET_SENSE", + SET_VERNACULAR = "SET_VERNACULAR", } -interface MergeDataAction { - type: MergeTreeActions.SET_DATA; - payload: Word[]; +interface ClearTreeMergeAction { + type: MergeTreeActions.CLEAR_TREE; } -interface MergeTreeMoveAction { +interface MoveSenseMergeAction { type: MergeTreeActions.MOVE_SENSE; payload: { src: MergeTreeReference[]; dest: MergeTreeReference[] }; } -interface MergeTreeSetAction { - type: MergeTreeActions.SET_SENSE; - payload: { ref: MergeTreeReference; data: number | undefined }; +interface OrderDuplicateMergeAction { + type: MergeTreeActions.ORDER_DUPLICATE; + payload: { ref: MergeTreeReference; order: number }; } -interface MergeOrderAction { +interface OrderSenseMergeAction { type: MergeTreeActions.ORDER_SENSE; - wordID: string; - senseID: string; - order: number; + payload: { + wordID: string; + senseID: string; + order: number; + }; } -interface MergeTreeWordAction { - type: MergeTreeActions.SET_VERNACULAR | MergeTreeActions.SET_PLURAL; +interface SetDataMergeAction { + type: MergeTreeActions.SET_DATA; + payload: Word[]; +} + +interface SetSenseMergeAction { + type: MergeTreeActions.SET_SENSE; + payload: { ref: MergeTreeReference; data: number | undefined }; +} + +interface SetWordStringMergeAction { + type: MergeTreeActions.SET_PLURAL | MergeTreeActions.SET_VERNACULAR; payload: { wordID: string; data: string }; } export type MergeTreeAction = - | MergeTreeWordAction - | MergeTreeMoveAction - | MergeTreeSetAction - | MergeDataAction - | MergeOrderAction - | { - type: MergeTreeActions.ORDER_DUPLICATE; - ref: MergeTreeReference; - order: number; - } - | { type: MergeTreeActions.CLEAR_TREE }; + | ClearTreeMergeAction + | MoveSenseMergeAction + | OrderDuplicateMergeAction + | OrderSenseMergeAction + | SetDataMergeAction + | SetSenseMergeAction + | SetWordStringMergeAction; // action creators -export function setVern(wordID: string, vern: string): MergeTreeAction { +export function setVern( + wordID: string, + vern: string +): SetWordStringMergeAction { return { type: MergeTreeActions.SET_VERNACULAR, payload: { wordID, data: vern }, }; } -export function setPlural(wordID: string, plural: string): MergeTreeAction { - return { - type: MergeTreeActions.SET_PLURAL, - payload: { wordID, data: plural }, - }; -} - -export function clearTree(): MergeTreeAction { +export function clearTree(): ClearTreeMergeAction { return { type: MergeTreeActions.CLEAR_TREE }; } export function moveSenses( src: MergeTreeReference[], dest: MergeTreeReference[] -): MergeTreeAction { +): MoveSenseMergeAction { return { type: MergeTreeActions.MOVE_SENSE, payload: { src, dest }, @@ -102,25 +105,25 @@ export function moveSenses( export function moveSense( src: MergeTreeReference, dest: MergeTreeReference -): MergeTreeAction { +): MoveSenseMergeAction { return moveSenses([src], [dest]); } export function setSense( ref: MergeTreeReference, data: number | undefined -): MergeTreeAction { +): SetSenseMergeAction { return { type: MergeTreeActions.SET_SENSE, payload: { ref, data }, }; } -export function removeSense(ref: MergeTreeReference): MergeTreeAction { +export function removeSense(ref: MergeTreeReference): SetSenseMergeAction { return setSense(ref, undefined); } -export function setWordData(words: Word[]): MergeDataAction { +export function setWordData(words: Word[]): SetDataMergeAction { return { type: MergeTreeActions.SET_DATA, payload: words, @@ -131,23 +134,20 @@ export function orderSense( wordID: string, senseID: string, order: number -): MergeOrderAction { +): OrderSenseMergeAction { return { type: MergeTreeActions.ORDER_SENSE, - wordID: wordID, - senseID: senseID, - order: order, + payload: { wordID, senseID, order }, }; } export function orderDuplicate( ref: MergeTreeReference, order: number -): MergeTreeAction { +): OrderDuplicateMergeAction { return { type: MergeTreeActions.ORDER_DUPLICATE, - ref, - order, + payload: { ref, order }, }; } @@ -333,13 +333,16 @@ export async function mergeWord( }; }); - // a merge is an identity if all of its senses come from parent - // and it has the same number of senses as parent - if (!children.find((val) => val.wordID !== wordID)) { - if (children.length === data.words[wordID].senses.length) { - // if the merge is an identity don't bother sending a merge - return mapping; - } + // a merge is an identity if the only child is the parent word + // and it has the same number of senses as parent (all with State.Sense) + if ( + children.length === 1 && + children[0].wordID === wordID && + children[0].senses.length === data.words[wordID].senses.length && + !children[0].senses.find((s) => s !== State.Sense) + ) { + // if the merge is an identity don't bother sending a merge + return mapping; } // send database call @@ -396,7 +399,7 @@ export function mergeAll() { const hash: string = wordIDs .sort() .reduce((val, acc) => `${acc}:${val}`, ""); - let blacklist: Hash = LocalStorage.getMergeDupsBlacklist(); + const blacklist: Hash = LocalStorage.getMergeDupsBlacklist(); blacklist[hash] = true; LocalStorage.setMergeDupsBlacklist(blacklist); // merge words diff --git a/src/goals/MergeDupGoal/MergeDupStep/MergeDupStepComponent.tsx b/src/goals/MergeDupGoal/MergeDupStep/MergeDupStepComponent.tsx index 5241527818..97608f5d4d 100644 --- a/src/goals/MergeDupGoal/MergeDupStep/MergeDupStepComponent.tsx +++ b/src/goals/MergeDupGoal/MergeDupStep/MergeDupStepComponent.tsx @@ -296,21 +296,22 @@ class MergeDupStep extends React.Component< @@ -318,5 +319,4 @@ class MergeDupStep extends React.Component< } } -//export class as default export default withLocalize(MergeDupStep); diff --git a/src/goals/MergeDupGoal/MergeDupStep/MergeDupStepReducer.tsx b/src/goals/MergeDupGoal/MergeDupStep/MergeDupStepReducer.tsx index 1d9c0f8e19..d7f15d2365 100644 --- a/src/goals/MergeDupGoal/MergeDupStep/MergeDupStepReducer.tsx +++ b/src/goals/MergeDupGoal/MergeDupStep/MergeDupStepReducer.tsx @@ -1,16 +1,16 @@ +import { StoreAction, StoreActions } from "../../../rootActions"; +import { Word } from "../../../types/word"; +import { uuid } from "../../../utilities"; import { MergeTreeAction, MergeTreeActions } from "./MergeDupStepActions"; import { - MergeTree, defaultData, defaultTree, + Hash, MergeData, + MergeTree, MergeTreeWord, - Hash, TreeDataSense, } from "./MergeDupsTree"; -import { Word } from "../../../types/word"; -import { uuid } from "../../../utilities"; -import { StoreAction, StoreActions } from "../../../rootActions"; export const defaultState: MergeTreeState = { data: defaultData, @@ -27,24 +27,33 @@ const mergeDupStepReducer = ( action: StoreAction | MergeTreeAction ): MergeTreeState => { switch (action.type) { - case MergeTreeActions.SET_VERNACULAR: + case MergeTreeActions.SET_VERNACULAR: { state.tree.words[action.payload.wordID].vern = action.payload.data; state.tree.words = { ...state.tree.words }; state.tree = { ...state.tree }; return { ...state }; - case MergeTreeActions.SET_PLURAL: - return state; + } + + case MergeTreeActions.SET_PLURAL: { + state.tree.words[action.payload.wordID].plural = action.payload.data; + state.tree.words = { ...state.tree.words }; + state.tree = { ...state.tree }; + return { ...state }; + } + case MergeTreeActions.ORDER_SENSE: { // reorder sense - let word = JSON.parse(JSON.stringify(state.tree.words[action.wordID])); + let word = JSON.parse( + JSON.stringify(state.tree.words[action.payload.wordID]) + ); let senses = Object.entries(word.senses); - let sense = { ...word.senses[action.senseID] }; + let sense = { ...word.senses[action.payload.senseID] }; senses.splice( - senses.findIndex((s) => s[0] === action.senseID), + senses.findIndex((s) => s[0] === action.payload.senseID), 1 ); - senses.splice(action.order, 0, [action.senseID, sense]); + senses.splice(action.payload.order, 0, [action.payload.senseID, sense]); word.senses = {}; for (let sense of senses) { @@ -54,13 +63,14 @@ const mergeDupStepReducer = ( let treeWords: Hash = JSON.parse( JSON.stringify(state.tree.words) ); - treeWords[action.wordID] = word; + treeWords[action.payload.wordID] = word; state = { ...state, tree: { ...state.tree, words: treeWords } }; return state; } + case MergeTreeActions.ORDER_DUPLICATE: { - let ref = action.ref; + let ref = action.payload.ref; let dups = Object.entries(state.tree.words[ref.word].senses[ref.sense]); let dup = state.tree.words[ref.word].senses[ref.sense][ref.duplicate]; @@ -68,7 +78,7 @@ const mergeDupStepReducer = ( dups.findIndex((s) => s[0] === ref.duplicate), 1 ); - dups.splice(action.order, 0, [ref.duplicate, dup]); + dups.splice(action.payload.order, 0, [ref.duplicate, dup]); let newDups: Hash = {}; @@ -86,9 +96,11 @@ const mergeDupStepReducer = ( state.tree.words = { ...state.tree.words }; state.tree = { ...state.tree }; state = { ...state }; + return state; } - case MergeTreeActions.MOVE_SENSE: + + case MergeTreeActions.MOVE_SENSE: { let treeState: MergeTree = JSON.parse(JSON.stringify(state.tree)); for (let op in action.payload.src) { let src = action.payload.src[op]; @@ -120,8 +132,7 @@ const mergeDupStepReducer = ( // cleanup src delete treeState.words[src.word].senses[src.sense][src.duplicate]; - // check if we removed last dup in a sense if so remove the sense from the word - + // if we removed last dup in a sense, remove sense from word if ( Object.keys(treeState.words[src.word].senses[src.sense]).length === 0 @@ -129,8 +140,7 @@ const mergeDupStepReducer = ( delete treeState.words[src.word].senses[src.sense]; } - // check if we removed last sense in a word if so remove the word from the tree - + // if we removed last sense in a word, remove word from tree if (Object.keys(treeState.words[src.word].senses).length === 0) { delete treeState.words[src.word]; } @@ -138,7 +148,9 @@ const mergeDupStepReducer = ( } return { ...state, tree: treeState }; - case MergeTreeActions.SET_DATA: + } + + case MergeTreeActions.SET_DATA: { let words: Hash = {}; let senses: Hash = {}; let wordsTree: Hash = {}; @@ -163,12 +175,20 @@ const mergeDupStepReducer = ( tree: { words: wordsTree }, data: { senses, words }, }; - case MergeTreeActions.CLEAR_TREE: + } + + case MergeTreeActions.CLEAR_TREE: { return { tree: { ...defaultTree }, data: { ...defaultData } }; - case StoreActions.RESET: + } + + case StoreActions.RESET: { return defaultState; - default: + } + + default: { return state; + } } }; + export default mergeDupStepReducer; diff --git a/src/goals/MergeDupGoal/MergeDupStep/tests/MergeDupStepActions.test.tsx b/src/goals/MergeDupGoal/MergeDupStep/tests/MergeDupStepActions.test.tsx index 77f631d0eb..b51f140d28 100644 --- a/src/goals/MergeDupGoal/MergeDupStep/tests/MergeDupStepActions.test.tsx +++ b/src/goals/MergeDupGoal/MergeDupStep/tests/MergeDupStepActions.test.tsx @@ -1,13 +1,20 @@ import configureMockStore from "redux-mock-store"; import thunk from "redux-thunk"; + import * as backend from "../../../../backend"; -import { multiGlossWord, State, Word, MergeWord } from "../../../../types/word"; +import { + MergeWord, + multiGlossWord, + Sense, + State, + Word, +} from "../../../../types/word"; import { MergeDups } from "../../MergeDups"; import { mergeAll } from "../MergeDupStepActions"; import { MergeData, MergeTree, Hash } from "../MergeDupsTree"; import { goalDataMock } from "./MockMergeDupData"; -type mockWordListIndices = "WA" | "WB" | "WA2" | "WB2"; +type mockWordListIndex = "WA" | "WB" | "WA2" | "WB2" | "WA3" | "WA4"; const mockWordList = { WA: { ...multiGlossWord("AAA", ["Sense 1", "Sense 2"]), id: "WA" }, WB: { ...multiGlossWord("BBB", ["Sense 3", "Sense 4"]), id: "WB" }, @@ -21,64 +28,83 @@ const mockWordList = { id: "WB2", history: ["WB"], }, + WA3: { + ...multiGlossWord("AAA", ["Sense 1", "Sense 2", "Sense 3"]), + id: "WA3", + history: ["WA", "WB"], + }, + WA4: { + ...multiGlossWord("AAA", ["Sense 1"]), + id: "WA4", + history: ["WA"], + }, +}; + +// mockMergeN is for test treeN below +interface parentWithMergeChildren { + parent: Word; + children: MergeWord[]; +} +const mockMerge2a: parentWithMergeChildren = { + parent: { ...mockWordList["WA2"], id: "WA", history: [] }, + children: [ + { wordID: "WA", senses: [State.Sense, State.Sense] }, + { wordID: "WB", senses: [State.Duplicate, State.Separate] }, + ], +}; +const mockMerge2b: parentWithMergeChildren = { + parent: { ...mockWordList["WB2"], id: "WB", history: [] }, + children: [{ wordID: "WB2", senses: [State.Sense] }], }; +const mockMerge3a: parentWithMergeChildren = { + parent: { ...mockWordList["WA3"], id: "WA", history: [] }, + children: [ + { wordID: "WA", senses: [State.Sense, State.Sense] }, + { wordID: "WB", senses: [State.Sense, State.Separate] }, + ], +}; +const mockMerge3b = mockMerge2b; +const mockMerge4a: parentWithMergeChildren = { + parent: { ...mockWordList["WA4"], id: "WA", history: [] }, + children: [{ wordID: "WA", senses: [State.Sense, State.Duplicate] }], +}; + +const mockMerges = [mockMerge2a, mockMerge2b, mockMerge3a, mockMerge4a]; +const mergeResults = [["WA2", "WB2"], ["WB2"], ["WA3", "WB2"], ["WA4"]]; +const mergeList: Hash = {}; +for (let i = 0; i <= mockMerges.length; i++) { + mergeList[JSON.stringify(mockMerges[i])] = mergeResults[i]; +} + +function mockMergeWords(parent: Word, children: MergeWord[]) { + expect(mockMerges).toContainEqual({ parent, children }); + const args = JSON.stringify({ parent, children }); + return Promise.resolve(mergeList[args]); +} jest.mock("../../../../backend", () => { const realBackend = jest.requireActual("../../../../backend"); return { ...realBackend, - mergeWords: jest.fn((parent: Word, children: MergeWord[]) => { - const { State } = jest.requireActual("../../../../types/word"); - // Setup data needed to mock - const M0: { parent: Word; children: MergeWord[] } = { - parent: { ...mockWordList["WA2"], id: "WA", history: [] }, - children: [ - { wordID: "WA", senses: [State.Sense, State.Sense] }, - { wordID: "WB", senses: [State.Duplicate, State.Separate] }, - ], - }; - - const M1: { parent: Word; children: MergeWord[] } = { - parent: { ...mockWordList["WB2"], history: [], id: "WB" }, - children: [{ wordID: "WB2", senses: [State.Sense] }], - }; - - expect([M0, M1]).toContainEqual({ parent, children }); - - let mergeList: Hash = {}; - mergeList[JSON.stringify(M0)] = ["WA2", "WB2"]; - mergeList[JSON.stringify(M1)] = ["WB2"]; - let args = JSON.stringify({ parent, children }); - - return Promise.resolve(mergeList[args]); - }), - getWord: jest.fn((id: mockWordListIndices) => { + mergeWords: jest.fn((parent: Word, children: MergeWord[]) => + mockMergeWords(parent, children) + ), + getWord: jest.fn((id: mockWordListIndex) => { return Promise.resolve(mockWordList[id]); }), }; }); -const mockGoal: MergeDups = new MergeDups(); +const mockGoal = new MergeDups(); mockGoal.data = goalDataMock; -mockGoal.steps = [ - { - words: [], - }, - { - words: [], - }, -]; +mockGoal.steps = [{ words: [] }, { words: [] }]; const createMockStore = configureMockStore([thunk]); const mockStoreState = { goalsState: { - historyState: { - history: [mockGoal], - }, + historyState: { history: [mockGoal] }, allPossibleGoals: [], - suggestionsState: { - suggestions: [], - }, + suggestionsState: { suggestions: [] }, }, mergeDuplicateGoal: { mergeTreeState: { data: {}, tree: {} } }, }; @@ -90,36 +116,47 @@ const data: { data: MergeData } = { WB: { ...multiGlossWord("BBB", ["Sense 3", "Sense 4"]), id: "WB" }, }, senses: { - S1: { - glosses: [{ language: "", def: "Sense 1" }], - semanticDomains: [], - srcWord: "WA", - order: 0, - }, - S2: { - glosses: [{ language: "", def: "Sense 2" }], - semanticDomains: [], - srcWord: "WA", - order: 1, - }, - S3: { - glosses: [{ language: "", def: "Sense 3" }], - semanticDomains: [], - srcWord: "WB", - order: 0, + S1: { ...new Sense("Sense 1"), srcWord: "WA", order: 0 }, + S2: { ...new Sense("Sense 2"), srcWord: "WA", order: 1 }, + S3: { ...new Sense("Sense 3"), srcWord: "WB", order: 0 }, + S4: { ...new Sense("Sense 4"), srcWord: "WB", order: 1 }, + }, + }, +}; + +beforeEach(() => { + jest.clearAllMocks(); +}); + +// Don't move or merge anything +const tree1: { tree: MergeTree } = { + tree: { + words: { + WA: { + senses: { ID1: { ID1: "S1" }, ID2: { ID1: "S2" } }, + vern: "AAA", + plural: "AAAS", }, - S4: { - glosses: [{ language: "", def: "Sense 4" }], - semanticDomains: [], - srcWord: "WB", - order: 1, + WB: { + senses: { ID1: { ID1: "S3" }, ID2: { ID1: "S4" } }, + vern: "BBB", + plural: "BBBS", }, }, }, }; +test("no merge", async () => { + const mockStore = createMockStore({ + ...mockStoreState, + mergeDuplicateGoal: { mergeTreeState: { ...data, ...tree1 } }, + }); + await mockStore.dispatch(mergeAll()); -// Merge sense 1 and 3 as duplicates -const treeA: { tree: MergeTree } = { + expect(backend.mergeWords).toHaveBeenCalledTimes(0); +}); + +// Merge sense 3 from B as duplicate into sense 1 from A +const tree2: { tree: MergeTree } = { tree: { words: { WA: { @@ -131,20 +168,86 @@ const treeA: { tree: MergeTree } = { }, }, }; +test("merge senses from different words", async () => { + const mockStore = createMockStore({ + ...mockStoreState, + mergeDuplicateGoal: { mergeTreeState: { ...data, ...tree2 } }, + }); + await mockStore.dispatch(mergeAll()); -test("test merge", async () => { - let parent = { ...data.data.words.WA }; - let children = [ - { wordID: "WA", senses: [State.Sense, State.Sense] }, - { wordID: "WB", senses: [State.Duplicate, State.Separate] }, - ]; + expect(backend.mergeWords).toHaveBeenCalledTimes(2); + expect(backend.mergeWords).toHaveBeenCalledWith( + mockMerge2a.parent, + mockMerge2a.children + ); + expect(backend.mergeWords).toHaveBeenCalledWith( + mockMerge2b.parent, + mockMerge2b.children + ); +}); +// Move sense 3 from B to A +const tree3: { tree: MergeTree } = { + tree: { + words: { + WA: { + senses: { ID1: { ID1: "S1" }, ID2: { ID1: "S2" }, ID3: { ID1: "S3" } }, + vern: "AAA", + plural: "AAAS", + }, + WB: { + senses: { ID1: { ID1: "S4" } }, + vern: "BBB", + plural: "BBBS", + }, + }, + }, +}; +test("move sense between words", async () => { const mockStore = createMockStore({ ...mockStoreState, - mergeDuplicateGoal: { mergeTreeState: { ...data, ...treeA } }, + mergeDuplicateGoal: { mergeTreeState: { ...data, ...tree3 } }, }); + await mockStore.dispatch(mergeAll()); + expect(backend.mergeWords).toHaveBeenCalledTimes(2); + expect(backend.mergeWords).toHaveBeenCalledWith( + mockMerge3a.parent, + mockMerge3a.children + ); + expect(backend.mergeWords).toHaveBeenCalledWith( + mockMerge3b.parent, + mockMerge3b.children + ); +}); + +// Merge sense 1 and 2 in A as duplicates +const tree4: { tree: MergeTree } = { + tree: { + words: { + WA: { + senses: { ID1: { ID1: "S1", ID2: "S2" } }, + vern: "AAA", + plural: "AAAS", + }, + WB: { + senses: { ID1: { ID1: "S3" }, ID2: { ID1: "S4" } }, + vern: "BBB", + plural: "BBBS", + }, + }, + }, +}; +test("merge senses within a word", async () => { + const mockStore = createMockStore({ + ...mockStoreState, + mergeDuplicateGoal: { mergeTreeState: { ...data, ...tree4 } }, + }); await mockStore.dispatch(mergeAll()); - expect(backend.mergeWords).toHaveBeenCalledWith(parent, children); + expect(backend.mergeWords).toHaveBeenCalledTimes(1); + expect(backend.mergeWords).toHaveBeenCalledWith( + mockMerge4a.parent, + mockMerge4a.children + ); }); diff --git a/src/resources/translations.json b/src/resources/translations.json index 8056eb07f0..528316427a 100644 --- a/src/resources/translations.json +++ b/src/resources/translations.json @@ -377,14 +377,6 @@ "progress": { "step": ["Step", "Paso", "Étape"], "of": ["of", "de", "sur"] - }, - "mergeDups": { - "done": [ - "Save & Continue", - "Ahorrar y Continuar", - "Enregistrer & Continuer" - ], - "skip": ["Skip", "omitir", "Sauter"] } }, "createStrWordInv": { @@ -584,7 +576,7 @@ "Arrastra un nuevo sentido aquí", "Faites glisser un nouveau sens ici" ], - "next": [ + "saveAndContinue": [ "Merge all these words and load a new set of words", "Combina todas estas palabras y carga un nuevo conjunto de palabras", "Fusionner tous ces mots et charger un nouvel ensemble de mots" @@ -630,6 +622,12 @@ "rejected": ["Rejected", "Rechazado", "Rejeté"], "restore": ["Restore", "Restaurar", "Restaurer"], "save": ["Save", "Guardar", "Enregistrer"], + "saveAndContinue": [ + "Save & Continue", + "Ahorrar y continuar", + "Enregistrer et continuer" + ], + "skip": ["Skip", "Omitir", "Sauter"], "undecided": ["Undecided", "Indeciso", "Incertain"], "upload": ["Upload", "Cargar", "Télécharger"] },