Preserve more content when merging senses

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.

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]);

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<Sense>): 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}`
+    );
+  });
+});