[ReviewEntries] Allow senses w/o gloss but w/ definition (#3536)

public/locales/en/translation.json

@@ -334,6 +334,7 @@
     "deleteDisabled": "This was imported with data that The Combine doesn't handle, so it may not be deleted.",
     "error": {
       "gloss": "Glosses cannot be left blank",
+      "glossAndDefinition": "Glosses and definitions cannot both be left blank",
       "senses": "Cannot save an entry with no senses",
       "vernacular": "Vernacular cannot be left blank"
     },

src/goals/ReviewEntries/ReviewEntriesTable/Cells/EditCell/EditDialog.tsx

@@ -120,6 +120,9 @@ export default function EditDialog(props: EditDialogProps): ReactElement {
     (state: StoreState) =>
       state.currentProjectState.project.analysisWritingSystems
   );
+  const definitionsEnabled = useAppSelector(
+    (state: StoreState) => state.currentProjectState.project.definitionsEnabled
+  );
   const vernLang = useAppSelector(
     (state: StoreState) =>
       state.currentProjectState.project.vernacularWritingSystem.bcp47
@@ -276,7 +279,10 @@ export default function EditDialog(props: EditDialogProps): ReactElement {
     }
 
     // Remove empty/deleted senses; confirm nonempty vernacular and senses
-    const cleanedWord = cleanWord(newWord, true);
+    const cleanedWord = cleanWord(newWord, {
+      definitionsEnabled,
+      exemptProtected: true,
+    });
     if (typeof cleanedWord === "string") {
       toast.error(t(cleanedWord));
       return Promise.reject(t(cleanedWord));

src/goals/ReviewEntries/ReviewEntriesTable/Cells/EditCell/EditSenseDialog.tsx

@@ -66,8 +66,8 @@ export enum EditSenseField {
 type EditSenseFieldChanged = Record<EditSenseField, boolean>;
 const defaultEditSenseFieldChanged: EditSenseFieldChanged = {
   [EditSenseField.Definitions]: false,
-  [EditSenseField.GrammaticalInfo]: false,
   [EditSenseField.Glosses]: false,
+  [EditSenseField.GrammaticalInfo]: false,
   [EditSenseField.SemanticDomains]: false,
 };
 
@@ -85,17 +85,18 @@ export default function EditSenseDialog(
     (state: StoreState) =>
       state.currentProjectState.project.analysisWritingSystems
   );
-  const showDefinitions = useAppSelector(
+  const definitionsEnabled = useAppSelector(
     (state: StoreState) => state.currentProjectState.project.definitionsEnabled
   );
-  const showGrammaticalInfo = useAppSelector(
+  const grammaticalInfoEnabled = useAppSelector(
     (state: StoreState) =>
       state.currentProjectState.project.grammaticalInfoEnabled
   );
 
   const [newSense, setNewSense] = useState(props.sense);
   const [cancelDialog, setCancelDialog] = useState(false);
   const [changes, setChanges] = useState(defaultEditSenseFieldChanged);
+  const [noDefinitionOrGloss, setNoDefinitionOrGloss] = useState(false);
 
   const { t } = useTranslation();
 
@@ -115,6 +116,10 @@ export default function EditSenseDialog(
         props.sense.semanticDomains
       ),
     });
+    setNoDefinitionOrGloss(
+      newSense.definitions.every((d) => !d.text.trim()) &&
+        newSense.glosses.every((g) => !g.def.trim())
+    );
   }, [newSense, props.sense]);
 
   const bgStyle = (field: EditSenseField): CSSProperties => ({
@@ -138,7 +143,7 @@ export default function EditSenseDialog(
     }
 
     // Confirm nonempty senses
-    const cleanedSense = cleanSense(newSense);
+    const cleanedSense = cleanSense(newSense, { definitionsEnabled });
     if (!cleanedSense || typeof cleanedSense === "string") {
       toast.error(t(cleanedSense ?? ""));
       return;
@@ -205,14 +210,15 @@ export default function EditSenseDialog(
             spacing={3}
           >
             {/* Definitions */}
-            {showDefinitions && (
+            {definitionsEnabled && (
               <Grid item>
                 <Card sx={bgStyle(EditSenseField.Definitions)}>
                   <CardHeader title={t("reviewEntries.columns.definitions")} />
                   <CardContent>
                     <DefinitionList
                       defaultLang={analysisWritingSystems[0]}
                       definitions={newSense.definitions}
+                      error={noDefinitionOrGloss}
                       onChange={updateDefinitions}
                       textFieldIdPrefix={
                         EditSenseDialogId.TextFieldDefinitionPrefix
@@ -230,6 +236,7 @@ export default function EditSenseDialog(
                 <CardContent>
                   <GlossList
                     defaultLang={analysisWritingSystems[0]}
+                    error={noDefinitionOrGloss}
                     glosses={newSense.glosses}
                     onChange={updateGlosses}
                     textFieldIdPrefix={EditSenseDialogId.TextFieldGlossPrefix}
@@ -239,7 +246,7 @@ export default function EditSenseDialog(
             </Grid>
 
             {/* Part of Speech */}
-            {showGrammaticalInfo && (
+            {grammaticalInfoEnabled && (
               <Grid item>
                 <Card sx={bgStyle(EditSenseField.GrammaticalInfo)}>
                   <CardHeader title={t("reviewEntries.columns.partOfSpeech")} />
@@ -286,6 +293,7 @@ export default function EditSenseDialog(
 interface DefinitionListProps {
   defaultLang: WritingSystem;
   definitions: Definition[];
+  error?: boolean;
   onChange: (definitions: Definition[]) => void;
   textFieldIdPrefix: string;
 }
@@ -302,6 +310,7 @@ function DefinitionList(props: DefinitionListProps): ReactElement {
       {definitions.map((d, i) => (
         <DefinitionTextField
           definition={d}
+          error={props.error}
           key={i}
           onChange={(definition: Definition) => {
             const updated = [...definitions];
@@ -317,13 +326,15 @@ function DefinitionList(props: DefinitionListProps): ReactElement {
 
 interface DefinitionTextFieldProps {
   definition: Definition;
+  error?: boolean;
   textFieldId: string;
   onChange: (definition: Definition) => void;
 }
 
 function DefinitionTextField(props: DefinitionTextFieldProps): ReactElement {
   return (
     <TextFieldWithFont
+      error={props.error}
       fullWidth
       id={props.textFieldId}
       label={props.definition.language}
@@ -343,6 +354,7 @@ function DefinitionTextField(props: DefinitionTextFieldProps): ReactElement {
 
 interface GlossListProps {
   defaultLang: WritingSystem;
+  error?: boolean;
   glosses: Gloss[];
   onChange: (glosses: Gloss[]) => void;
   textFieldIdPrefix: string;
@@ -359,6 +371,7 @@ function GlossList(props: GlossListProps): ReactElement {
     <>
       {glosses.map((g, i) => (
         <GlossTextField
+          error={props.error}
           gloss={g}
           key={i}
           onChange={(gloss: Gloss) => {
@@ -374,6 +387,7 @@ function GlossList(props: GlossListProps): ReactElement {
 }
 
 interface GlossTextFieldProps {
+  error?: boolean;
   gloss: Gloss;
   textFieldId: string;
   onChange: (gloss: Gloss) => void;
@@ -382,9 +396,9 @@ interface GlossTextFieldProps {
 function GlossTextField(props: GlossTextFieldProps): ReactElement {
   return (
     <TextFieldWithFont
+      error={props.error}
       fullWidth
       id={props.textFieldId}
-      error={!props.gloss.def.trim()}
       label={props.gloss.language}
       lang={props.gloss.language}
       margin="dense"

src/goals/ReviewEntries/ReviewEntriesTable/Cells/EditCell/tests/utilities.test.ts

@@ -210,15 +210,35 @@ describe("cleanSense", () => {
     sense.guid = "guid-does-not-matter";
     sense.accessibility = Status.Protected;
     expect(cleanSense(sense)).toBeUndefined();
-    expect(typeof cleanSense(sense, true)).toEqual("object");
+    expect(typeof cleanSense(sense, { exemptProtected: true })).toEqual(
+      "object"
+    );
   });
 
   it("returns error string for non-empty sense without gloss (unless protected and exempted)", () => {
     const sense = newSense();
     sense.grammaticalInfo.catGroup = GramCatGroup.Noun;
     sense.accessibility = Status.Protected;
     expect(typeof cleanSense(sense)).toEqual("string");
-    expect(typeof cleanSense(sense, true)).toEqual("object");
+    expect(typeof cleanSense(sense, { exemptProtected: true })).toEqual(
+      "object"
+    );
+  });
+
+  it("allow sense without gloss but with definition", () => {
+    const sense = newSense();
+    sense.definitions.push(newDefinition("¡Hola, Mundo!"));
+    expect(typeof cleanSense(sense)).toEqual("object");
+  });
+
+  it("returns different error if definitionsEnabled", () => {
+    const sense = newSense();
+    sense.grammaticalInfo.catGroup = GramCatGroup.Noun;
+    const disabledResult = cleanSense(sense);
+    const enabledResult = cleanSense(sense, { definitionsEnabled: true });
+    expect(typeof disabledResult).toEqual("string");
+    expect(typeof enabledResult).toEqual("string");
+    expect(disabledResult).not.toEqual(enabledResult);
   });
 });
 
@@ -242,7 +262,7 @@ describe("cleanWord", () => {
     sense.accessibility = Status.Protected;
     word.senses.push(sense);
     expect(typeof cleanWord(word)).toEqual("string");
-    expect(typeof cleanWord(word, true)).toEqual("object");
+    expect(typeof cleanWord(word, { exemptProtected: true })).toEqual("object");
   });
 
   it("cleans up note", () => {

src/goals/ReviewEntries/ReviewEntriesTable/Cells/EditCell/utilities.ts

@@ -72,14 +72,20 @@ export function isSenseChanged(oldSense: Sense, newSense: Sense): boolean {
   );
 }
 
+/** Common options for the cleanSense, cleanWord functions. */
+interface CleanOptions {
+  /** Allow no glosses if there are definitions. */
+  definitionsEnabled?: boolean;
+  /** Allow empty sense if protected and thus cannot be deleted. */
+  exemptProtected?: boolean;
+}
+
 /** Return a cleaned sense ready to be saved:
  * - If a sense is marked as deleted or is utterly blank, return undefined
- * - If a sense lacks gloss, return error string
- *
- * (If `exemptProtected = true`, protected senses are allowed to be without gloss.) */
+ * - If a sense lacks gloss, return error string */
 export function cleanSense(
   newSense: Sense,
-  exemptProtected = false
+  options?: CleanOptions
 ): Sense | string | undefined {
   // Ignore deleted senses.
   if (newSense.accessibility === Status.Deleted) {
@@ -96,7 +102,7 @@ export function cleanSense(
   );
 
   // Bypass the following checks on protected senses.
-  if (exemptProtected && newSense.accessibility === Status.Protected) {
+  if (options?.exemptProtected && newSense.accessibility === Status.Protected) {
     return newSense;
   }
 
@@ -110,20 +116,20 @@ export function cleanSense(
     return;
   }
 
-  // Don't allow senses without a gloss.
-  if (!newSense.glosses.length) {
-    return "reviewEntries.error.gloss";
+  // Don't allow senses without a gloss or definition.
+  if (!newSense.glosses.length && !newSense.definitions.length) {
+    return options?.definitionsEnabled
+      ? "reviewEntries.error.glossAndDefinition"
+      : "reviewEntries.error.gloss";
   }
 
   return newSense;
 }
 
 /** Clean a word. Return error string id if:
  * - the vernacular field is empty
- * - all senses are empty/deleted
- *
- * (If `exemptProtected = true`, protected senses are allowed to be empty.) */
-export function cleanWord(word: Word, exemptProtected = false): Word | string {
+ * - all senses are empty/deleted */
+export function cleanWord(word: Word, options?: CleanOptions): Word | string {
   // Make sure vernacular isn't empty.
   const vernacular = word.vernacular.trim();
   if (!vernacular.length) {
@@ -133,7 +139,7 @@ export function cleanWord(word: Word, exemptProtected = false): Word | string {
   // Clean senses and check for problems.
   const senses: Sense[] = [];
   for (const sense of word.senses) {
-    const cleanedSense = cleanSense(sense, exemptProtected);
+    const cleanedSense = cleanSense(sense, options);
     // Skip deleted or empty senses.
     if (!cleanedSense) {
       continue;