From 551bfe7b8bf4bb797dd1a038aa4cd93fea4690c8 Mon Sep 17 00:00:00 2001 From: "D. Ror" Date: Fri, 28 Jun 2024 17:48:46 -0400 Subject: [PATCH] Preserve more content when merging senses (#3181) --- .../MergeDuplicates/Redux/reducerUtilities.ts | 35 +++- .../Redux/tests/MergeDupsActions.test.tsx | 38 ++-- .../Redux/tests/reducerUtilities.test.ts | 175 ++++++++++++++++++ 3 files changed, 230 insertions(+), 18 deletions(-) create mode 100644 src/goals/MergeDuplicates/Redux/tests/reducerUtilities.test.ts diff --git a/src/goals/MergeDuplicates/Redux/reducerUtilities.ts b/src/goals/MergeDuplicates/Redux/reducerUtilities.ts index 0fc98d95d4..d85627d902 100644 --- a/src/goals/MergeDuplicates/Redux/reducerUtilities.ts +++ b/src/goals/MergeDuplicates/Redux/reducerUtilities.ts @@ -112,14 +112,15 @@ export function createMergeParent( export function combineIntoFirstSense(mergeSenses: MergeTreeSense[]): void { // Set the main sense to the first sense (the top one when the sidebar was opened). const mainSense = mergeSenses[0].sense; + const sep = "; "; // Merge the rest as duplicates. // These were senses dropped into another sense. mergeSenses.slice(1).forEach((mergeDupSense) => { const dupSense = mergeDupSense.sense; dupSense.accessibility = Status.Duplicate; + // Merge the duplicate's definitions into the main sense. - const sep = "; "; dupSense.definitions.forEach((def) => { const newText = def.text.trim(); if (newText) { @@ -142,9 +143,41 @@ export function combineIntoFirstSense(mergeSenses: MergeTreeSense[]): void { } }); + // Merge the duplicate's glosses into the main sense. + dupSense.glosses.forEach((gloss) => { + const newDef = gloss.def.trim(); + if (newDef) { + // Check if glosses array already has entry with the same language. + const oldGloss = mainSense.glosses.find( + (g) => g.language === gloss.language + ); + if (!oldGloss) { + // If not, add this one to the array. + mainSense.glosses.push({ ...gloss, def: newDef }); + } else { + // If so, check whether this one's text is already present. + const oldDef = oldGloss.def.trim(); + if (!oldDef) { + oldGloss.def = newDef; + } else if (!oldDef.includes(newDef)) { + oldGloss.def = `${oldDef}${sep}${newDef}`; + } + } + } + }); + // Use the duplicate's part of speech if not specified in the main sense. if (mainSense.grammaticalInfo.catGroup === GramCatGroup.Unspecified) { mainSense.grammaticalInfo = { ...dupSense.grammaticalInfo }; + } else if ( + mainSense.grammaticalInfo.catGroup === dupSense.grammaticalInfo.catGroup + ) { + const oldCat = mainSense.grammaticalInfo.grammaticalCategory.trim(); + const oldCats = oldCat.split(sep).map((cat) => cat.trim()); + const newCat = dupSense.grammaticalInfo.grammaticalCategory.trim(); + if (newCat && !oldCats.includes(newCat)) { + mainSense.grammaticalInfo.grammaticalCategory = `${oldCat}${sep}${newCat}`; + } } // Put the duplicate's domains in the main sense if the id is new. diff --git a/src/goals/MergeDuplicates/Redux/tests/MergeDupsActions.test.tsx b/src/goals/MergeDuplicates/Redux/tests/MergeDupsActions.test.tsx index d603e468fc..73c5cab2a1 100644 --- a/src/goals/MergeDuplicates/Redux/tests/MergeDupsActions.test.tsx +++ b/src/goals/MergeDuplicates/Redux/tests/MergeDupsActions.test.tsx @@ -91,11 +91,19 @@ const senses = { S3: wordB.senses[0], S4: wordB.senses[1], }; -senses["S1"].accessibility = Status.Protected; -const S1 = senses["S1"].guid; -const S2 = senses["S2"].guid; -const S3 = senses["S3"].guid; -const S4 = senses["S4"].guid; +senses.S1.accessibility = Status.Protected; +const sense12: Sense = { + ...senses.S1, + glosses: [{ ...senses.S1.glosses[0], def: "S1; S2" }], +}; +const sense13: Sense = { + ...senses.S1, + glosses: [{ ...senses.S1.glosses[0], def: "S1; S3" }], +}; +const S1 = senses.S1.guid; +const S2 = senses.S2.guid; +const S3 = senses.S3.guid; +const S4 = senses.S4.guid; const data: MergeData = { words: { WA: wordA, WB: wordB }, senses: { @@ -147,8 +155,8 @@ describe("MergeDupActions", () => { await store.dispatch(mergeAll()); expect(mockMergeWords).toHaveBeenCalledTimes(1); - const parentA = wordAnyGuids(vernA, [senses["S1"], senses["S2"]], idA); - const parentB = wordAnyGuids(vernB, [senses["S4"]], idB); + const parentA = wordAnyGuids(vernA, [sense13, senses.S2], idA); + const parentB = wordAnyGuids(vernB, [senses.S4], idB); const childA = newMergeSourceWord(idA); const childB = newMergeSourceWord(idB); const mockMerges = [ @@ -180,10 +188,10 @@ describe("MergeDupActions", () => { expect(mockMergeWords).toHaveBeenCalledTimes(1); const parentA = wordAnyGuids( vernA, - [senses["S1"], senses["S3"], senses["S2"]], + [senses.S1, senses.S3, senses.S2], idA ); - const parentB = wordAnyGuids(vernB, [senses["S4"]], idB); + const parentB = wordAnyGuids(vernB, [senses.S4], idB); const childA = newMergeSourceWord(idA); const childB = newMergeSourceWord(idB); const mockMerges = [ @@ -214,7 +222,7 @@ describe("MergeDupActions", () => { expect(mockMergeWords).toHaveBeenCalledTimes(1); - const parent = wordAnyGuids(vernA, [senses["S1"]], idA); + const parent = wordAnyGuids(vernA, [sense12], idA); const child = newMergeSourceWord(idA); const mockMerge = newMergeWords(parent, [child]); expect(mockMergeWords).toHaveBeenCalledWith([mockMerge]); @@ -242,7 +250,7 @@ describe("MergeDupActions", () => { await store.dispatch(mergeAll()); expect(mockMergeWords).toHaveBeenCalledTimes(1); - const parent = wordAnyGuids(vernA, [senses["S1"]], idA); + const parent = wordAnyGuids(vernA, [senses.S1], idA); const child = newMergeSourceWord(idA); const mockMerge = newMergeWords(parent, [child]); expect(mockMergeWords).toHaveBeenCalledWith([mockMerge]); @@ -297,11 +305,7 @@ describe("MergeDupActions", () => { await store.dispatch(mergeAll()); expect(mockMergeWords).toHaveBeenCalledTimes(1); - const parentA = wordAnyGuids( - vernA, - [senses["S1"], senses["S2"], senses["S4"]], - idA - ); + const parentA = wordAnyGuids(vernA, [sense13, senses.S2, senses.S4], idA); const childA = newMergeSourceWord(idA); const childB = newMergeSourceWord(idB, true); const mockMerge = newMergeWords(parentA, [childA, childB]); @@ -325,7 +329,7 @@ describe("MergeDupActions", () => { expect(mockMergeWords).toHaveBeenCalledTimes(1); - const parent = wordAnyGuids(vernA, [senses["S1"], senses["S2"]], idA); + const parent = wordAnyGuids(vernA, [senses.S1, senses.S2], idA); parent.flag = WA.flag; const child = newMergeSourceWord(idA); const mockMerge = newMergeWords(parent, [child]); diff --git a/src/goals/MergeDuplicates/Redux/tests/reducerUtilities.test.ts b/src/goals/MergeDuplicates/Redux/tests/reducerUtilities.test.ts new file mode 100644 index 0000000000..84db5a6167 --- /dev/null +++ b/src/goals/MergeDuplicates/Redux/tests/reducerUtilities.test.ts @@ -0,0 +1,175 @@ +import { type Sense, Status, GramCatGroup } from "api/models"; +import { + type MergeTreeSense, + newMergeTreeSense, +} from "goals/MergeDuplicates/MergeDupsTreeTypes"; +import { combineIntoFirstSense } from "goals/MergeDuplicates/Redux/reducerUtilities"; +import { newSemanticDomain } from "types/semanticDomain"; +import { newDefinition, newGloss } from "types/word"; +import { randomIntString } from "utilities/utilities"; + +function mockMergeTreeSense(partialSense?: Partial): MergeTreeSense { + const treeSense = newMergeTreeSense("", randomIntString(), 0); + return { ...treeSense, sense: { ...treeSense.sense, ...partialSense } }; +} + +describe("combineIntoFirstSense", () => { + it("marks all but the first sense as Status.Duplicate", () => { + const senses = [ + mockMergeTreeSense(), + mockMergeTreeSense(), + mockMergeTreeSense(), + mockMergeTreeSense(), + ]; + combineIntoFirstSense(senses); + senses.map((ts, i) => { + expect(ts.sense.accessibility).toEqual( + i ? Status.Duplicate : Status.Active + ); + }); + }); + + it("adds non-duplicate-id semantic domains to first sense", () => { + const senses = [ + mockMergeTreeSense({ + semanticDomains: [newSemanticDomain("5.5", "Fire", "en")], + }), + mockMergeTreeSense({ + semanticDomains: [ + newSemanticDomain("5.5", "Fuego", "es"), + newSemanticDomain("5.5.4", "Quemar", "es"), + ], + }), + mockMergeTreeSense({ + semanticDomains: [ + newSemanticDomain("5.5.4", "Burn", "es"), + newSemanticDomain("5.5.6", "Fuel", "en"), + ], + }), + ]; + combineIntoFirstSense(senses); + const semDoms = senses[0].sense.semanticDomains; + // Check that 2 more domains were added + expect(semDoms).toHaveLength(3); + // Check that the domains have the expected ids + expect(new Set(["5.5", "5.5.4", "5.5.6"])).toEqual( + new Set(semDoms.map((dom) => dom.id)) + ); + // Check that the initial domain wasn't replaced + expect(semDoms.find((dom) => dom.id === "5.5")?.lang).toEqual("en"); + }); + + describe("grammatical info", () => { + it("inherits the first not-Unspecified catGroup", () => { + const senses = [ + mockMergeTreeSense(), + mockMergeTreeSense({ + grammaticalInfo: { + catGroup: GramCatGroup.Unspecified, + grammaticalCategory: "", + }, + }), + mockMergeTreeSense({ + grammaticalInfo: { + catGroup: GramCatGroup.Verb, + grammaticalCategory: "", + }, + }), + mockMergeTreeSense({ + grammaticalInfo: { + catGroup: GramCatGroup.Noun, + grammaticalCategory: "", + }, + }), + ]; + combineIntoFirstSense(senses); + const catGroup = senses[0].sense.grammaticalInfo.catGroup; + expect(catGroup).toEqual(GramCatGroup.Verb); + }); + + it("merges grammaticalCategory of the same catGroup", () => { + const senses = [ + mockMergeTreeSense({ + grammaticalInfo: { + catGroup: GramCatGroup.Verb, + grammaticalCategory: "vt", + }, + }), + mockMergeTreeSense({ + grammaticalInfo: { + catGroup: GramCatGroup.Verb, + grammaticalCategory: "vi", + }, + }), + mockMergeTreeSense({ + grammaticalInfo: { + catGroup: GramCatGroup.Verb, + grammaticalCategory: "vt", + }, + }), + mockMergeTreeSense({ + grammaticalInfo: { + catGroup: GramCatGroup.Verb, + grammaticalCategory: "v", + }, + }), + ]; + combineIntoFirstSense(senses); + const gramCat = senses[0].sense.grammaticalInfo.grammaticalCategory; + expect(gramCat).toEqual("vt; vi; v"); + }); + }); + + it("combines non-duplicate definitions", () => { + const textEn1 = "a flying fish"; + const textEn1Contained = "flying fish"; + const textEn2 = "a species of flying fish"; + const textEs = "un pez volando"; + const senses = [ + mockMergeTreeSense({ definitions: [newDefinition(textEn1, "en")] }), + mockMergeTreeSense({ + definitions: [newDefinition(textEn1Contained, "en")], + }), + mockMergeTreeSense({ + definitions: [ + newDefinition(textEs, "es"), + newDefinition(textEn2, "en"), + ], + }), + ]; + combineIntoFirstSense(senses); + const defs = senses[0].sense.definitions; + // Check that the non-English definition was added + expect(defs).toHaveLength(2); + expect(defs.map((d) => d.language)).toEqual(["en", "es"]); + expect(defs.find((d) => d.language === "es")?.text).toEqual(textEs); + // Check that the English definition text was extended + expect(defs.find((d) => d.language === "en")?.text).toEqual( + `${textEn1}; ${textEn2}` + ); + }); + + it("combines non-duplicate glosses", () => { + const defEn1 = "a flying fish"; + const defEn1Contained = "flying fish"; + const defEn2 = "a species of flying fish"; + const defEs = "un pez volando"; + const senses = [ + mockMergeTreeSense({ glosses: [newGloss(defEn1, "en")] }), + mockMergeTreeSense({ glosses: [newGloss(defEn1Contained, "en")] }), + mockMergeTreeSense({ + glosses: [newGloss(defEs, "es"), newGloss(defEn2, "en")], + }), + ]; + combineIntoFirstSense(senses); + const glosses = senses[0].sense.glosses; + // Check that the non-English gloss was added + expect(glosses).toHaveLength(2); + expect(glosses.map((g) => g.language)).toEqual(["en", "es"]); + expect(glosses.find((g) => g.language === "es")?.def).toEqual(defEs); + // Check that the English gloss def was extended + expect(glosses.find((g) => g.language === "en")?.def).toEqual( + `${defEn1}; ${defEn2}` + ); + }); +});