From c702bd1a6d5646a30b69fff4f58fc6ef0058c58a Mon Sep 17 00:00:00 2001 From: "D. Ror" Date: Tue, 16 Jan 2024 18:33:14 -0500 Subject: [PATCH] [DataEntry] Auto merge new entry with empty selected dup sense (#2866) --- .../DataEntryTable/NewEntry/SenseDialog.tsx | 6 +- .../DataEntryTable/NewEntry/index.tsx | 33 ++-- .../NewEntry/tests/index.test.tsx | 1 + .../DataEntry/DataEntryTable/index.tsx | 163 ++++++++++++++---- .../DataEntryTable/tests/index.test.tsx | 105 ++++++++++- src/types/word.ts | 10 +- 6 files changed, 256 insertions(+), 62 deletions(-) diff --git a/src/components/DataEntry/DataEntryTable/NewEntry/SenseDialog.tsx b/src/components/DataEntry/DataEntryTable/NewEntry/SenseDialog.tsx index 121cbff0f7..eec313cd2c 100644 --- a/src/components/DataEntry/DataEntryTable/NewEntry/SenseDialog.tsx +++ b/src/components/DataEntry/DataEntryTable/NewEntry/SenseDialog.tsx @@ -22,7 +22,7 @@ interface SenseDialogProps { selectedWord: Word; open: boolean; // Call handleClose with no input to indicate no selection was made. - handleClose: (gloss?: string) => void; + handleClose: (guid?: string) => void; analysisLang: string; } @@ -50,7 +50,7 @@ export default function SenseDialog(props: SenseDialogProps): ReactElement { interface SenseListProps { selectedWord: Word; - closeDialog: (gloss?: string) => void; + closeDialog: (guid?: string) => void; analysisLang: string; } @@ -71,7 +71,7 @@ export function SenseList(props: SenseListProps): ReactElement { props.closeDialog(gloss)} + onClick={() => props.closeDialog(sense.guid)} > Promise; resetNewEntry: () => void; - updateWordWithNewGloss: (wordId: string) => Promise; + updateWordWithNewGloss: () => Promise; newAudio: Pronunciation[]; addNewAudio: (file: FileWithSpeakerId) => void; delNewAudio: (url: string) => void; @@ -59,7 +59,9 @@ interface NewEntryProps { vernInput: RefObject; // Parent component handles vern suggestion state: selectedDup?: Word; + selectedSenseGuid?: string; setSelectedDup: (id?: string) => void; + setSelectedSense: (guid?: string) => void; suggestedVerns: string[]; suggestedDups: Word[]; } @@ -88,7 +90,9 @@ export default function NewEntry(props: NewEntryProps): ReactElement { vernInput, // Parent component handles vern suggestion state: selectedDup, + selectedSenseGuid, setSelectedDup, + setSelectedSense, suggestedVerns, suggestedDups, } = props; @@ -136,6 +140,11 @@ export default function NewEntry(props: NewEntryProps): ReactElement { } }, [isTreeOpen, resetState, wasTreeClosed]); + /** Update whether the Sense dialog should be open. */ + useEffect(() => { + setSenseOpen(!!selectedDup?.id && selectedSenseGuid === undefined); + }, [selectedDup, selectedSenseGuid]); + /** When the vern/sense dialogs are closed, focus needs to return to text fields. * The following sets a flag (shouldFocus) to be triggered by conditionalFocus(), * which is passed to each input component to call on update. */ @@ -172,7 +181,7 @@ export default function NewEntry(props: NewEntryProps): ReactElement { setVernOpen(true); } else if (selectedDup.id) { // Case 1b: User has selected an entry to modify - await updateWordWithNewGloss(selectedDup.id); + await updateWordWithNewGloss(); resetState(); } else { // Case 1c: User has selected new entry @@ -203,25 +212,19 @@ export default function NewEntry(props: NewEntryProps): ReactElement { if (id !== undefined) { setSelectedDup(id); } - - if (id) { - setSenseOpen(true); - } - setVernOpen(false); }; - const handleCloseSenseDialog = (gloss?: string): void => { - if (gloss) { - setNewGloss(gloss); - } else if (gloss === undefined) { - // If gloss is undefined, the user exited the dialog without a selection. + const handleCloseSenseDialog = (guid?: string): void => { + if (guid === undefined) { + // If undefined, the user exited the dialog without a selection. setSelectedDup(); setVernOpen(true); + } else { + // Set the selected dup sense to the one with the specified guid; + // an empty string indicates new sense for the selectedDup. + setSelectedSense(guid); } - // else: an empty string indicates new sense for the selectedWord. - - setSenseOpen(false); }; return ( diff --git a/src/components/DataEntry/DataEntryTable/NewEntry/tests/index.test.tsx b/src/components/DataEntry/DataEntryTable/NewEntry/tests/index.test.tsx index 3f98cd71c1..113d5bfdeb 100644 --- a/src/components/DataEntry/DataEntryTable/NewEntry/tests/index.test.tsx +++ b/src/components/DataEntry/DataEntryTable/NewEntry/tests/index.test.tsx @@ -39,6 +39,7 @@ describe("NewEntry", () => { vernInput={createRef()} // Parent component handles vern suggestion state: setSelectedDup={jest.fn()} + setSelectedSense={jest.fn()} suggestedVerns={[]} suggestedDups={[]} /> diff --git a/src/components/DataEntry/DataEntryTable/index.tsx b/src/components/DataEntry/DataEntryTable/index.tsx index c644c99e72..5b80b340c9 100644 --- a/src/components/DataEntry/DataEntryTable/index.tsx +++ b/src/components/DataEntry/DataEntryTable/index.tsx @@ -37,6 +37,7 @@ import { useAppSelector } from "types/hooks"; import theme from "types/theme"; import { FileWithSpeakerId, + newGloss, newNote, newPronunciation, newSense, @@ -77,7 +78,8 @@ enum DefunctStatus { Retire = "RETIRE", } -/** Add current semantic domain to specified sense within a word. */ +/** Add current semantic domain to specified sense within a word. + * Return the word unchanged if the sense already has the domain. */ export function addSemanticDomainToSense( semDom: SemanticDomain, word: Word, @@ -87,6 +89,9 @@ export function addSemanticDomainToSense( if (!sense) { throw new Error("Word has no sense with specified guid"); } + if (sense.semanticDomains.find((s) => s.id == semDom.id)) { + return word; + } sense.semanticDomains.push(makeSemDomCurrent(semDom)); const senses = word.senses.map((s) => (s.guid === senseGuid ? sense : s)); return { ...word, senses }; @@ -187,6 +192,7 @@ interface NewEntryState { newNote: string; newVern: string; selectedDup?: Word; + selectedSenseGuid?: string; suggestedVerns: string[]; suggestedDups: Word[]; } @@ -197,6 +203,7 @@ const defaultNewEntryState = (): NewEntryState => ({ newNote: "", newVern: "", selectedDup: undefined, + selectedSenseGuid: undefined, suggestedDups: [], suggestedVerns: [], }); @@ -329,11 +336,23 @@ export default function DataEntryTable( ); /** Add one-sense word to the display of recent entries. */ - const addToDisplay = (wordAccess: WordAccess, insertIndex?: number): void => { + const addToDisplay = (wordAccess: WordAccess, insertIndex = -1): void => { setState((prevState) => { const recentWords = [...prevState.recentWords]; - if (insertIndex !== undefined && insertIndex < recentWords.length) { - recentWords.splice(insertIndex, 0, wordAccess); + + // If wordAccess has senseGuid matching a recent word, + // replace it instead of just inserting. + let deleteCount = 0; + const replaceIndex = recentWords.findIndex( + (wa) => wa.senseGuid == wordAccess.senseGuid + ); + if (replaceIndex > -1) { + deleteCount = 1; + insertIndex = replaceIndex; + } + + if (insertIndex > -1 && insertIndex < recentWords.length) { + recentWords.splice(insertIndex, deleteCount, wordAccess); } else { recentWords.push(wordAccess); } @@ -425,14 +444,42 @@ export default function DataEntryTable( /** Set or clear the selected vern-duplicate word. */ const setSelectedDup = (id?: string): void => { - setState((prev) => ({ - ...prev, - selectedDup: id + setState((prev) => { + const selectedDup = id ? prev.suggestedDups.find((w) => w.id === id) : id === "" ? newWord(prev.newVern) - : undefined, - })); + : undefined; + + // If selected duplicate has one empty sense, automatically select it. + const soloSense = + selectedDup?.senses.length === 1 ? selectedDup.senses[0] : undefined; + const emptySense = + soloSense?.definitions.find((d) => d.text) || + soloSense?.glosses.find((g) => g.def) + ? undefined + : soloSense; + + return { + ...prev, + selectedDup, + selectedSenseGuid: emptySense?.guid, + }; + }); + }; + + /** Set or clear the selected duplicate word's sense. */ + const setSelectedSense = (guid?: string): void => { + setState((prev) => { + const selectedSense = guid + ? prev.selectedDup?.senses.find((s) => s.guid === guid) + : undefined; + return { + ...prev, + newGloss: selectedSense ? firstGlossText(selectedSense) : "", + selectedSenseGuid: guid, + }; + }); }; /** Reset things specific to the current data entry session in the current semantic domain. */ @@ -670,8 +717,6 @@ export default function DataEntryTable( audio: Pronunciation[], insertIndex?: number ): Promise => { - wordToAdd.note.language = analysisLang.bcp47; - // Check if word is duplicate to existing word. const dupId = await backend.getDuplicateId(wordToAdd); if (dupId) { @@ -685,7 +730,7 @@ export default function DataEntryTable( } addToDisplay({ word, senseGuid: word.senses[0].guid }, insertIndex); }, - [addAudiosToBackend, addDuplicateWord, analysisLang.bcp47] + [addAudiosToBackend, addDuplicateWord] ); /** Update the word in the backend and the frontend. */ @@ -704,17 +749,12 @@ export default function DataEntryTable( /** Reset the entry table. If there is an un-submitted word then submit it. */ const handleExit = async (): Promise => { - // Check if there is a new word, but user exited without pressing enter. + // Check if there is a dup selected, but user exited without pressing enter. if (state.newVern) { - const oldWord = state.allWords.find( - (w) => w.vernacular === state.newVern - ); - if (!oldWord) { - // Existing word not found, so create a new word. - await addNewEntry(); + if (state.selectedDup?.id) { + await updateWordWithNewEntry(); } else { - // Found an existing word, so add a sense to it. - await updateWordWithNewEntry(oldWord.id); + await addNewEntry(); } } resetEverything(); @@ -724,10 +764,10 @@ export default function DataEntryTable( // Async functions for handling changes of the NewEntry. ///////////////////////////////// - /** Assemble a word from the new entry state and add it. */ + /** Build a word from the new entry state and add it. */ const addNewEntry = async (): Promise => { - const word = newWord(state.newVern); const lang = analysisLang.bcp47; + const word = newWord(state.newVern, lang); word.senses.push( newSense(state.newGloss, lang, makeSemDomCurrent(props.semanticDomain)) ); @@ -735,19 +775,64 @@ export default function DataEntryTable( await addNewWord(word, state.newAudio); }; - /** Checks if sense already exists with this gloss and semantic domain. */ - const updateWordWithNewEntry = async (wordId: string): Promise => { - const oldWord = state.allWords.find((w: Word) => w.id === wordId); - if (!oldWord) { + /** Update the selected duplicate with the new entry. + * (Only considers the first gloss, `.glosses[0]`, of each sense.) */ + const updateWordWithNewEntry = async (): Promise => { + const oldWord = state.selectedDup; + if (!oldWord || !oldWord.id) { throw new Error("You are trying to update a nonexistent word"); } + const semDom = makeSemDomCurrent(props.semanticDomain); - // If this gloss matches a sense on the word, update that sense. + // If a dup sense is selected, update it. + if (state.selectedSenseGuid) { + const oldSense = oldWord.senses.find( + (s) => s.guid === state.selectedSenseGuid + ); + if (!oldSense) { + throw new Error("You are trying to update a nonexistent sense"); + } + if (!oldSense.glosses.length) { + oldSense.glosses.push(newGloss()); + } + + // If selected sense already has this domain, add audio without updating first. + if ( + oldSense.glosses[0].def === state.newGloss && + oldSense.semanticDomains.find((d) => d.id === semDom.id) + ) { + enqueueSnackbar( + t("addWords.senseInWord", { + val1: oldWord.vernacular, + val2: state.newGloss, + }) + ); + if (state.newAudio.length) { + await addAudiosToBackend(oldWord.id, state.newAudio); + } + return; + } + + // Only update the selected sense if the old gloss is blank or matches the new gloss. + if (!oldSense.glosses[0].def.trim()) { + oldSense.glosses[0] = newGloss(state.newGloss, analysisLang.bcp47); + } + if (oldSense.glosses[0].def === state.newGloss) { + await updateWordBackAndFront( + addSemanticDomainToSense(semDom, oldWord, state.selectedSenseGuid), + state.selectedSenseGuid, + state.newAudio + ); + return; + } + } + + // Otherwise, if new gloss matches a sense, update that sense. for (const sense of oldWord.senses) { if (sense.glosses?.length && sense.glosses[0].def === state.newGloss) { if (sense.semanticDomains.find((d) => d.id === semDom.id)) { - // User is trying to add a sense that already exists + // User is trying to add a sense that already exists. enqueueSnackbar( t("addWords.senseInWord", { val1: oldWord.vernacular, @@ -755,7 +840,7 @@ export default function DataEntryTable( }) ); if (state.newAudio.length) { - await addAudiosToBackend(wordId, state.newAudio); + await addAudiosToBackend(oldWord.id, state.newAudio); } return; } else { @@ -837,17 +922,27 @@ export default function DataEntryTable( } if (oldSenses.length === 1 && oldSense.semanticDomains.length === 1) { - // The word can simply be updated as it stand + // The word can simply be updated as it stands. await updateWordInBackend({ ...oldEntry.word, vernacular }); } else { // Retract and replaced with a new entry. - const word = simpleWord(vernacular, firstGlossText(oldSense)); + const word = simpleWord( + vernacular, + firstGlossText(oldSense), + analysisLang.bcp47 + ); word.id = ""; await undoRecentEntry(index); await addNewWord(word, [], index); } }, - [addNewWord, state.recentWords, undoRecentEntry, updateWordInBackend] + [ + addNewWord, + analysisLang.bcp47, + state.recentWords, + undoRecentEntry, + updateWordInBackend, + ] ); /** Update the gloss def in a recent entry. */ @@ -967,7 +1062,9 @@ export default function DataEntryTable( vernInput={newVernInput} // Parent handles vern suggestion state of child: selectedDup={state.selectedDup} + selectedSenseGuid={state.selectedSenseGuid} setSelectedDup={setSelectedDup} + setSelectedSense={setSelectedSense} suggestedDups={state.suggestedDups} suggestedVerns={state.suggestedVerns} /> diff --git a/src/components/DataEntry/DataEntryTable/tests/index.test.tsx b/src/components/DataEntry/DataEntryTable/tests/index.test.tsx index 08422b9c2c..32244a77a1 100644 --- a/src/components/DataEntry/DataEntryTable/tests/index.test.tsx +++ b/src/components/DataEntry/DataEntryTable/tests/index.test.tsx @@ -51,12 +51,13 @@ jest.mock("backend", () => ({ jest.mock("backend/localStorage", () => ({ getUserId: () => mockUserId, })); +jest.mock("components/DataEntry/DataEntryTable/NewEntry/SenseDialog"); +jest.mock("components/DataEntry/DataEntryTable/NewEntry/VernDialog"); jest.mock( "components/DataEntry/DataEntryTable/RecentEntry", () => MockRecentEntry ); jest.mock("components/Pronunciations/PronunciationsFrontend", () => "div"); -jest.mock("utilities/utilities"); jest.spyOn(window, "alert").mockImplementation(() => {}); @@ -180,7 +181,7 @@ describe("DataEntryTable", () => { it("throws error when word doesn't have sense with specified guid", () => { expect(() => addSemanticDomainToSense(newSemanticDomain(), mockWord(), "gibberish") - ).toThrowError(); + ).toThrow(); }); it("adds a semantic domain to existing sense", () => { @@ -242,7 +243,7 @@ describe("DataEntryTable", () => { describe("updateEntryGloss", () => { it("throws error when entry doesn't have sense with specified guid", () => { const entry: WordAccess = { word: newWord(), senseGuid: "gibberish" }; - expect(() => updateEntryGloss(entry, "def", "semDomId")).toThrowError(); + expect(() => updateEntryGloss(entry, "def", "semDomId")).toThrow(); }); it("directly updates a sense with no other semantic domains", () => { @@ -288,6 +289,14 @@ describe("DataEntryTable", () => { return { ...word, senses }; }; + it("throws error if no selected dup", async () => { + await renderTable(); + testHandle = testRenderer.root.findByType(NewEntry); + await expect( + async () => await testHandle.props.updateWordWithNewGloss() + ).rejects.toThrow(); + }); + it("doesn't update word in backend if sense is a duplicate", async () => { const word = changeSemDoms(mockMultiWord, [ newSemanticDomain("someSemDomId"), @@ -297,8 +306,10 @@ describe("DataEntryTable", () => { await renderTable(); testHandle = testRenderer.root.findByType(NewEntry); await act(async () => { + await testHandle.props.setNewVern(word.vernacular); + await testHandle.props.setSelectedDup(word.id); await testHandle.props.setNewGloss(firstGlossText(word.senses[0])); - await testHandle.props.updateWordWithNewGloss(word.id); + await testHandle.props.updateWordWithNewGloss(); }); expect(mockUpdateWord).not.toHaveBeenCalled(); }); @@ -316,8 +327,10 @@ describe("DataEntryTable", () => { testHandle = testRenderer.root.findByType(NewEntry); const glossText = firstGlossText(word.senses[senseIndex]); await act(async () => { + await testHandle.props.setNewVern(word.vernacular); + await testHandle.props.setSelectedDup(word.id); await testHandle.props.setNewGloss(glossText); - await testHandle.props.updateWordWithNewGloss(word.id); + await testHandle.props.updateWordWithNewGloss(); }); expect(mockUpdateWord).toHaveBeenCalledTimes(1); @@ -331,14 +344,94 @@ describe("DataEntryTable", () => { }); it("updates word in backend if gloss doesn't exist", async () => { + mockGetFrontierWords.mockResolvedValue([mockMultiWord]); await renderTable(); testHandle = testRenderer.root.findByType(NewEntry); await act(async () => { + await testHandle.props.setNewVern(mockMultiWord.vernacular); + await testHandle.props.setSelectedDup(mockMultiWord.id); await testHandle.props.setNewGloss("differentGloss"); - await testHandle.props.updateWordWithNewGloss(mockMultiWord.id); + await testHandle.props.updateWordWithNewGloss(); }); expect(mockUpdateWord).toHaveBeenCalledTimes(1); }); + + describe("with selected sense", () => { + it("throws error if selected sense not in dup", async () => { + mockGetFrontierWords.mockResolvedValue([mockMultiWord]); + await renderTable(); + testHandle = testRenderer.root.findByType(NewEntry); + await act(async () => { + await testHandle.props.setNewVern(mockMultiWord.vernacular); + await testHandle.props.setSelectedDup(mockMultiWord.id); + await testHandle.props.setSelectedSense("non-existent-guid"); + }); + await expect( + async () => await testHandle.props.updateWordWithNewGloss() + ).rejects.toThrow(); + }); + + it("updates word if selected sense has empty gloss", async () => { + const word = changeSemDoms(mockMultiWord, [ + newSemanticDomain("someSemDomId"), + newSemanticDomain(mockSemDomId), + ]); + word.senses[0].glosses[0].def = ""; + mockGetFrontierWords.mockResolvedValue([word]); + await renderTable(); + testHandle = testRenderer.root.findByType(NewEntry); + await act(async () => { + await testHandle.props.setNewVern(word.vernacular); + await testHandle.props.setSelectedDup(word.id); + await testHandle.props.setSelectedSense(word.senses[0].guid); + await testHandle.props.setNewGloss("new gloss"); + await testHandle.props.updateWordWithNewGloss(); + }); + expect(mockUpdateWord).toHaveBeenCalledTimes(1); + }); + + it("doesn't update word if selected sense has domain", async () => { + const word = changeSemDoms(mockMultiWord, [ + newSemanticDomain("someSemDomId"), + newSemanticDomain(mockSemDomId), + ]); + mockGetFrontierWords.mockResolvedValue([word]); + await renderTable(); + testHandle = testRenderer.root.findByType(NewEntry); + await act(async () => { + await testHandle.props.setNewVern(word.vernacular); + await testHandle.props.setSelectedDup(word.id); + await testHandle.props.setSelectedSense(word.senses[0].guid); + await testHandle.props.setNewGloss(firstGlossText(word.senses[0])); + await testHandle.props.updateWordWithNewGloss(); + }); + expect(mockUpdateWord).not.toHaveBeenCalled(); + }); + + it("updates word if selected sense has different semantic domain", async () => { + const word = changeSemDoms(mockMultiWord, [ + newSemanticDomain("someSemDomId"), + newSemanticDomain("anotherSemDomId"), + ]); + mockGetFrontierWords.mockResolvedValue([word]); + await renderTable(); + + testHandle = testRenderer.root.findByType(NewEntry); + await act(async () => { + await testHandle.props.setNewVern(word.vernacular); + await testHandle.props.setSelectedDup(word.id); + await testHandle.props.setSelectedSense(word.senses[0].guid); + await testHandle.props.setNewGloss(firstGlossText(word.senses[0])); + await testHandle.props.updateWordWithNewGloss(); + }); + expect(mockUpdateWord).toHaveBeenCalledTimes(1); + + // Confirm the semantic domain was added. + const wordUpdated: Word = mockUpdateWord.mock.calls[0][0]; + const doms = wordUpdated.senses[0].semanticDomains; + expect(doms.find((d) => d.id === mockSemDomId)).toBeTruthy(); + }); + }); }); describe("addNewWord", () => { diff --git a/src/types/word.ts b/src/types/word.ts index b160306620..b4f8624a1a 100644 --- a/src/types/word.ts +++ b/src/types/word.ts @@ -85,7 +85,7 @@ export function newGrammaticalInfo(): GrammaticalInfo { return { catGroup: GramCatGroup.Unspecified, grammaticalCategory: "" }; } -export function newWord(vernacular = ""): Word { +export function newWord(vernacular = "", lang?: string): Word { return { id: "", guid: v4(), @@ -97,7 +97,7 @@ export function newWord(vernacular = ""): Word { accessibility: Status.Active, history: [], projectId: "", - note: newNote(), + note: newNote(undefined, lang), flag: newFlag(), }; } @@ -118,11 +118,11 @@ export class DomainWord { } } -export function simpleWord(vern: string, gloss: string): Word { +export function simpleWord(vern: string, gloss: string, lang?: string): Word { return { - ...newWord(vern), + ...newWord(vern, lang), id: randomIntString(), - senses: [newSense(gloss)], + senses: [newSense(gloss, lang)], }; }