Skip to content

Commit

Permalink
[MergeDups] Refactor MergeTreeSense (#2901)
Browse files Browse the repository at this point in the history
Rather than having `MergeTreeSense extends Sense`, have it include `sense: Sense` as a field.
  • Loading branch information
imnasnainaec authored Jan 30, 2024
1 parent e3d90e6 commit a88c14c
Show file tree
Hide file tree
Showing 11 changed files with 71 additions and 77 deletions.
20 changes: 10 additions & 10 deletions src/goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/DragSense.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ interface DragSenseProps {
index: number;
wordId: string;
mergeSenseId: string;
senses: MergeTreeSense[];
mergeSenses: MergeTreeSense[];
isOnlySenseInProtectedWord: boolean;
isProtectedSense: boolean;
}
Expand Down Expand Up @@ -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,
})
Expand All @@ -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();
Expand Down Expand Up @@ -125,7 +125,7 @@ export default function DragSense(props: DragSenseProps): ReactElement {
}}
>
<SenseCardContent
senses={props.senses}
senses={props.mergeSenses.map((s) => s.sense)}
languages={analysisLangs}
toggleFunction={toggleSidebar}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { useAppSelector } from "types/hooks";
import theme from "types/theme";

interface SidebarDragSenseProps {
sense: MergeTreeSense;
mergeSense: MergeTreeSense;
index: number;
}

Expand All @@ -29,10 +29,10 @@ export default function SidebarDragSense(

return (
<Draggable
key={props.sense.guid}
key={props.mergeSense.sense.guid}
draggableId={draggableId}
index={props.index}
isDragDisabled={props.sense.protected}
isDragDisabled={props.mergeSense.protected}
>
{(provided, snapshot): ReactElement => (
<div
Expand All @@ -54,14 +54,14 @@ export default function SidebarDragSense(
? "red"
: snapshot.isDragging
? "lightgreen"
: props.sense.protected
: props.mergeSense.protected
? "lightyellow"
: props.index === 0
? "white"
: "lightgrey",
}}
>
<SenseCardContent senses={[props.sense]} sidebar />
<SenseCardContent senses={[props.mergeSense.sense]} sidebar />
</Card>
</div>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ export default function SidebarDrop(): ReactElement {
<ArrowForwardIos />
</IconButton>
<Typography variant="h5">{vernacular}</Typography>
{sidebar.senses.map((sense: MergeTreeSense, index: number) => (
<SidebarDragSense key={index} index={index} sense={sense} />
{sidebar.mergeSenses.map((ms: MergeTreeSense, i: number) => (
<SidebarDragSense key={ms.sense.guid} index={i} mergeSense={ms} />
))}
{providedDroppable.placeholder}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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],
Expand Down
24 changes: 13 additions & 11 deletions src/goals/MergeDuplicates/MergeDupsTreeTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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),
};
}

Expand All @@ -60,10 +62,10 @@ export function convertSenseToMergeTreeSense(
order = 0
): MergeTreeSense {
return {
...sense,
srcWordId,
order,
protected: sense?.accessibility === Status.Protected,
srcWordId,
sense,
};
}

Expand All @@ -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: "",
};
Expand Down
49 changes: 26 additions & 23 deletions src/goals/MergeDuplicates/Redux/MergeDupsReducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
});
}
}
Expand Down Expand Up @@ -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
) {
Expand All @@ -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 };
}
);

Expand All @@ -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 = "; ";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
19 changes: 4 additions & 15 deletions src/goals/MergeDuplicates/Redux/tests/MergeDupsDataMock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -145,11 +146,7 @@ export const mergeTwoWordsScenario: GetMergeWordsScenario = {
},
},
tree: {
sidebar: {
senses: [],
wordId: "",
mergeSenseId: "",
},
sidebar: defaultSidebar,
words: {
[wordFoo2.id]: convertWordToMergeTreeWord({
...wordFoo2,
Expand Down Expand Up @@ -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],
Expand Down Expand Up @@ -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],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
Expand Down

0 comments on commit a88c14c

Please sign in to comment.