From a88c14c24fe1e04d97bf1df22288c944141c3ed3 Mon Sep 17 00:00:00 2001 From: "D. Ror" Date: Tue, 30 Jan 2024 07:51:00 -0500 Subject: [PATCH] [MergeDups] Refactor MergeTreeSense (#2901) Rather than having `MergeTreeSense extends Sense`, have it include `sense: Sense` as a field. --- .../MergeDupsStep/MergeDragDrop/DragSense.tsx | 20 ++++---- .../MergeDupsStep/MergeDragDrop/DropWord.tsx | 2 +- .../MergeDragDrop/SidebarDragSense.tsx | 10 ++-- .../MergeDragDrop/SidebarDrop.tsx | 4 +- .../MergeDupsStep/MergeDragDrop/index.tsx | 6 +-- .../MergeDragDrop/tests/index.test.tsx | 3 +- .../MergeDuplicates/MergeDupsTreeTypes.ts | 24 ++++----- .../MergeDuplicates/Redux/MergeDupsReducer.ts | 49 ++++++++++--------- .../Redux/tests/MergeDupsActions.test.tsx | 9 ++-- .../Redux/tests/MergeDupsDataMock.ts | 19 ++----- .../Redux/tests/MergeDupsReducer.test.tsx | 2 +- 11 files changed, 71 insertions(+), 77 deletions(-) diff --git a/src/goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/DragSense.tsx b/src/goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/DragSense.tsx index ee90b14ada..48df5a3d94 100644 --- a/src/goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/DragSense.tsx +++ b/src/goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/DragSense.tsx @@ -14,7 +14,7 @@ interface DragSenseProps { index: number; wordId: string; mergeSenseId: string; - senses: MergeTreeSense[]; + mergeSenses: MergeTreeSense[]; isOnlySenseInProtectedWord: boolean; isProtectedSense: boolean; } @@ -47,12 +47,12 @@ export default function DragSense(props: DragSenseProps): ReactElement { const isInSidebar = sidebar.wordId === props.wordId && sidebar.mergeSenseId === props.mergeSenseId && - sidebar.senses.length > 1; + sidebar.mergeSenses.length > 1; const updateSidebar = useCallback(() => { dispatch( setSidebar({ - senses: props.senses, + mergeSenses: props.mergeSenses, wordId: props.wordId, mergeSenseId: props.mergeSenseId, }) @@ -68,19 +68,19 @@ export default function DragSense(props: DragSenseProps): ReactElement { }; useEffect(() => { - if (props.senses.length !== duplicateCount) { - if (props.senses.length > duplicateCount) { + if (props.mergeSenses.length !== duplicateCount) { + if (props.mergeSenses.length > duplicateCount) { updateSidebar(); } - setDuplicateCount(props.senses.length); + setDuplicateCount(props.mergeSenses.length); } - }, [props.senses.length, duplicateCount, updateSidebar]); + }, [props.mergeSenses.length, duplicateCount, updateSidebar]); if ( isInSidebar && !arraysEqual( - sidebar.senses.map((s) => s.guid), - props.senses.map((s) => s.guid) + sidebar.mergeSenses.map((m) => m.sense.guid), + props.mergeSenses.map((m) => m.sense.guid) ) ) { updateSidebar(); @@ -125,7 +125,7 @@ export default function DragSense(props: DragSenseProps): ReactElement { }} > s.sense)} languages={analysisLangs} toggleFunction={toggleSidebar} /> diff --git a/src/goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/DropWord.tsx b/src/goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/DropWord.tsx index 672de4a3c3..b1166bf2cb 100644 --- a/src/goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/DropWord.tsx +++ b/src/goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/DropWord.tsx @@ -71,7 +71,7 @@ export default function DropWord(props: DropWordProps): ReactElement { index={index} wordId={props.wordId} mergeSenseId={id} - senses={senses} + mergeSenses={senses} isOnlySenseInProtectedWord={protectedWithOneChild} isProtectedSense={senses[0].protected} /> diff --git a/src/goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/SidebarDragSense.tsx b/src/goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/SidebarDragSense.tsx index 27de4beb24..cb2ccc9013 100644 --- a/src/goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/SidebarDragSense.tsx +++ b/src/goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/SidebarDragSense.tsx @@ -13,7 +13,7 @@ import { useAppSelector } from "types/hooks"; import theme from "types/theme"; interface SidebarDragSenseProps { - sense: MergeTreeSense; + mergeSense: MergeTreeSense; index: number; } @@ -29,10 +29,10 @@ export default function SidebarDragSense( return ( {(provided, snapshot): ReactElement => (
- +
)} diff --git a/src/goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/SidebarDrop.tsx b/src/goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/SidebarDrop.tsx index 4a556c9fc3..c67a2ec81a 100644 --- a/src/goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/SidebarDrop.tsx +++ b/src/goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/SidebarDrop.tsx @@ -38,8 +38,8 @@ export default function SidebarDrop(): ReactElement { {vernacular} - {sidebar.senses.map((sense: MergeTreeSense, index: number) => ( - + {sidebar.mergeSenses.map((ms: MergeTreeSense, i: number) => ( + ))} {providedDroppable.placeholder} diff --git a/src/goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/index.tsx b/src/goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/index.tsx index 96a6df527a..30165ea245 100644 --- a/src/goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/index.tsx +++ b/src/goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/index.tsx @@ -26,11 +26,11 @@ export default function MergeDragDrop(): ReactElement { const dispatch = useAppDispatch(); const sidebarOpen = useAppSelector( (state: StoreState) => - state.mergeDuplicateGoal.tree.sidebar.senses.length > 1 + state.mergeDuplicateGoal.tree.sidebar.mergeSenses.length > 1 ); const sidebarProtected = useAppSelector((state: StoreState) => { - const senses = state.mergeDuplicateGoal.tree.sidebar.senses; - return senses.length && senses[0].protected; + const ms = state.mergeDuplicateGoal.tree.sidebar.mergeSenses; + return ms.length && ms[0].protected; }); const words = useAppSelector( (state: StoreState) => state.mergeDuplicateGoal.tree.words diff --git a/src/goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/tests/index.test.tsx b/src/goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/tests/index.test.tsx index a381da3a3a..82735cbdb2 100644 --- a/src/goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/tests/index.test.tsx +++ b/src/goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/tests/index.test.tsx @@ -12,6 +12,7 @@ import DragSense from "goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/DragSen import DropWord from "goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/DropWord"; import { convertSenseToMergeTreeSense, + defaultSidebar, newMergeTreeWord, } from "goals/MergeDuplicates/MergeDupsTreeTypes"; import { MergeTreeState } from "goals/MergeDuplicates/Redux/MergeDupsReduxTypes"; @@ -99,7 +100,7 @@ const mockTwoWordState = (): MergeTreeState => ({ words: { [wordFoo1.id]: wordFoo1, [wordFoo2.id]: wordFoo2 }, }, tree: { - sidebar: { senses: [], wordId: "", mergeSenseId: "" }, + sidebar: defaultSidebar, words: { [wordFoo1.id]: newMergeTreeWord(wordFoo1.vernacular, { word1_senseA: [senseBah.guid, senseBaj.guid], diff --git a/src/goals/MergeDuplicates/MergeDupsTreeTypes.ts b/src/goals/MergeDuplicates/MergeDupsTreeTypes.ts index f333343797..09c03f3823 100644 --- a/src/goals/MergeDuplicates/MergeDupsTreeTypes.ts +++ b/src/goals/MergeDuplicates/MergeDupsTreeTypes.ts @@ -4,10 +4,11 @@ import { Flag, Sense, Status, Word } from "api/models"; import { Hash } from "types/hash"; import { newFlag, newSense } from "types/word"; -export interface MergeTreeSense extends Sense { - srcWordId: string; +export interface MergeTreeSense { order: number; protected: boolean; + srcWordId: string; + sense: Sense; } export interface MergeData { @@ -30,15 +31,16 @@ export interface MergeTreeWord { } export function newMergeTreeSense( - gloss = "", - srcWordId = "", - order = 0 + gloss: string, + srcWordId: string, + order: number, + guid?: string ): MergeTreeSense { return { - ...newSense(gloss), - srcWordId, order, protected: false, + srcWordId, + sense: guid ? { ...newSense(gloss), guid } : newSense(gloss), }; } @@ -60,10 +62,10 @@ export function convertSenseToMergeTreeSense( order = 0 ): MergeTreeSense { return { - ...sense, - srcWordId, order, protected: sense?.accessibility === Status.Protected, + srcWordId, + sense, }; } @@ -78,13 +80,13 @@ export function convertWordToMergeTreeWord(word: Word): MergeTreeWord { } export interface Sidebar { - senses: MergeTreeSense[]; + mergeSenses: MergeTreeSense[]; wordId: string; mergeSenseId: string; } export const defaultSidebar: Sidebar = { - senses: [], + mergeSenses: [], wordId: "", mergeSenseId: "", }; diff --git a/src/goals/MergeDuplicates/Redux/MergeDupsReducer.ts b/src/goals/MergeDuplicates/Redux/MergeDupsReducer.ts index d6c6daac0a..27bd01e34d 100644 --- a/src/goals/MergeDuplicates/Redux/MergeDupsReducer.ts +++ b/src/goals/MergeDuplicates/Redux/MergeDupsReducer.ts @@ -302,13 +302,15 @@ function buildSenses( senses[wordId] = []; for (const sense of dbWord.senses) { senses[wordId].push({ - ...sense, - srcWordId: wordId, order: senses[wordId].length, - accessibility: nonDeletedSenses.includes(sense.guid) - ? Status.Separate - : Status.Deleted, protected: sense.accessibility === Status.Protected, + srcWordId: wordId, + sense: { + ...sense, + accessibility: nonDeletedSenses.includes(sense.guid) + ? Status.Separate + : Status.Deleted, + }, }); } } @@ -347,7 +349,8 @@ function createMergeWords( onlyChild[0].srcWordId === wordId && onlyChild.length === word.senses.length && !onlyChild.find( - (s) => ![Status.Active, Status.Protected].includes(s.accessibility) + (ms) => + ![Status.Active, Status.Protected].includes(ms.sense.accessibility) ) && compareFlags(mergeWord.flag, word.flag) === 0 ) { @@ -365,21 +368,20 @@ function createMergeWords( parent.vernacular = mergeWord.vern; } const children: MergeSourceWord[] = Object.values(mergeSenses).map( - (sList) => { - sList.forEach((sense) => { - if ([Status.Active, Status.Protected].includes(sense.accessibility)) { - parent.senses.push({ - guid: sense.guid, - definitions: sense.definitions, - glosses: sense.glosses, - semanticDomains: sense.semanticDomains, - accessibility: sense.accessibility, - grammaticalInfo: sense.grammaticalInfo, - }); + (msList) => { + msList.forEach((mergeSense) => { + if ( + [Status.Active, Status.Protected].includes( + mergeSense.sense.accessibility + ) + ) { + parent.senses.push(mergeSense.sense); } }); - const getAudio = !sList.find((s) => s.accessibility === Status.Separate); - return { srcWordId: sList[0].srcWordId, getAudio }; + const getAudio = !msList.find( + (ms) => ms.sense.accessibility === Status.Separate + ); + return { srcWordId: msList[0].srcWordId, getAudio }; } ); @@ -390,17 +392,18 @@ function createMergeWords( * - change the accessibility of the first one from Separate to Active/Protected, * - change the accessibility of the rest to Duplicate, * - merge select content from duplicates into main sense */ -function combineIntoFirstSense(senses: MergeTreeSense[]): void { +function combineIntoFirstSense(mergeSenses: MergeTreeSense[]): void { // Set the first sense to be merged as Active/Protected. // This was the top sense when the sidebar was opened. - const mainSense = senses[0]; - mainSense.accessibility = mainSense.protected + const mainSense = mergeSenses[0].sense; + mainSense.accessibility = mergeSenses[0].protected ? Status.Protected : Status.Active; // Merge the rest as duplicates. // These were senses dropped into another sense. - senses.slice(1).forEach((dupSense) => { + mergeSenses.slice(1).forEach((mergeDupSense) => { + const dupSense = mergeDupSense.sense; dupSense.accessibility = Status.Duplicate; // Merge the duplicate's definitions into the main sense. const sep = "; "; diff --git a/src/goals/MergeDuplicates/Redux/tests/MergeDupsActions.test.tsx b/src/goals/MergeDuplicates/Redux/tests/MergeDupsActions.test.tsx index c5fcccc0b2..8032e10d1f 100644 --- a/src/goals/MergeDuplicates/Redux/tests/MergeDupsActions.test.tsx +++ b/src/goals/MergeDuplicates/Redux/tests/MergeDupsActions.test.tsx @@ -80,13 +80,12 @@ const S3 = senses["S3"].guid; const S4 = senses["S4"].guid; const data: MergeData = { words: { WA: wordA, WB: wordB }, senses: {} }; data.senses[S1] = { - ...newMergeTreeSense("S1", idA, 0), - guid: S1, + ...newMergeTreeSense("S1", idA, 0, S1), protected: true, }; -data.senses[S2] = { ...newMergeTreeSense("S2", idA, 1), guid: S2 }; -data.senses[S3] = { ...newMergeTreeSense("S3", idB, 0), guid: S3 }; -data.senses[S4] = { ...newMergeTreeSense("S4", idB, 1), guid: S4 }; +data.senses[S2] = newMergeTreeSense("S2", idA, 1, S2); +data.senses[S3] = newMergeTreeSense("S3", idB, 0, S3); +data.senses[S4] = newMergeTreeSense("S4", idB, 1, S4); beforeEach(jest.clearAllMocks); diff --git a/src/goals/MergeDuplicates/Redux/tests/MergeDupsDataMock.ts b/src/goals/MergeDuplicates/Redux/tests/MergeDupsDataMock.ts index 3837d6c216..cfe62e0b3c 100644 --- a/src/goals/MergeDuplicates/Redux/tests/MergeDupsDataMock.ts +++ b/src/goals/MergeDuplicates/Redux/tests/MergeDupsDataMock.ts @@ -5,6 +5,7 @@ import { defaultState } from "components/App/DefaultState"; import { convertSenseToMergeTreeSense, convertWordToMergeTreeWord, + defaultSidebar, newMergeTreeWord, } from "goals/MergeDuplicates/MergeDupsTreeTypes"; import { type MergeDupsData } from "goals/MergeDuplicates/MergeDupsTypes"; @@ -145,11 +146,7 @@ export const mergeTwoWordsScenario: GetMergeWordsScenario = { }, }, tree: { - sidebar: { - senses: [], - wordId: "", - mergeSenseId: "", - }, + sidebar: defaultSidebar, words: { [wordFoo2.id]: convertWordToMergeTreeWord({ ...wordFoo2, @@ -211,11 +208,7 @@ export const mergeTwoSensesScenario: GetMergeWordsScenario = { }, }, tree: { - sidebar: { - senses: [], - wordId: "", - mergeSenseId: "", - }, + sidebar: defaultSidebar, words: { [wordFoo2.id]: newMergeTreeWord(wordFoo2.vernacular, { word2_senseA: [senseBar.guid], @@ -277,11 +270,7 @@ export const mergeTwoDefinitionsScenario: GetMergeWordsScenario = { }, }, tree: { - sidebar: { - senses: [], - wordId: "", - mergeSenseId: "", - }, + sidebar: defaultSidebar, words: { [wordFoo2.id]: newMergeTreeWord(wordFoo2.vernacular, { word2_senseA: [senseBar.guid], diff --git a/src/goals/MergeDuplicates/Redux/tests/MergeDupsReducer.test.tsx b/src/goals/MergeDuplicates/Redux/tests/MergeDupsReducer.test.tsx index 1960d90679..5785df9d05 100644 --- a/src/goals/MergeDuplicates/Redux/tests/MergeDupsReducer.test.tsx +++ b/src/goals/MergeDuplicates/Redux/tests/MergeDupsReducer.test.tsx @@ -609,7 +609,7 @@ describe("MergeDupsReducer", () => { ); // check that this sense is somewhere in the tree expect( - getRefByGuid(treeSense.guid, treeState.tree.words) + getRefByGuid(treeSense.sense.guid, treeState.tree.words) ).toBeDefined(); } }