fix: disable save buttons when the keyword is empty

graasp · Feb 21, 2024 · 3c024fe · 3c024fe
1 parent 70a19c4
commit 3c024fe
Show file tree

Hide file tree

Showing 7 changed files with 99 additions and 55 deletions.
diff --git a/cypress/e2e/builder/enterSettings.cy.ts b/cypress/e2e/builder/enterSettings.cy.ts
@@ -2,10 +2,8 @@ import { Context, PermissionLevel } from '@graasp/sdk';
 
 import { KeywordsData } from '../../../src/config/appSettingTypes';
 import {
-  ADD_KEYWORD_BUTTON_CY,
   BUILDER_VIEW_CY,
   CHATBOT_CONTAINER_CY,
-  ENTER_KEYWORD_FIELD_CY,
   INITIAL_CHATBOT_PROMPT_INPUT_FIELD_CY,
   INITIAL_PROMPT_INPUT_FIELD_CY,
   SETTINGS_SAVE_BUTTON_CY,
@@ -14,7 +12,6 @@ import {
   USE_CHATBOT_DATA_CY,
   buildDataCy,
   buildKeywordDefinitionTextInputCy,
-  buildKeywordNotExistWarningCy,
   buildKeywordTextInputCy,
 } from '../../../src/config/selectors';
 import {
@@ -80,55 +77,6 @@ describe('Enter Settings', () => {
     cy.get(buildDataCy(SETTINGS_SAVE_BUTTON_CY)).should('not.be.disabled');
   });
 
-  // Detected incomplete keywords in the text.
-  // 'wef' was found incomplete in 'wefwef hello'.
-  // Check that only complete words are detected in text.
-  it('only detect complete keywords', () => {
-    const PRBLEMATIC_TEXT = 'wefwef hello';
-    const PROBLEMATIC_KEYWORDS = ['wef', 'he'];
-
-    cy.get(buildDataCy(TEXT_INPUT_FIELD_CY))
-      .should('be.visible')
-      .type(PRBLEMATIC_TEXT);
-
-    PROBLEMATIC_KEYWORDS.forEach((k) => {
-      cy.get(buildDataCy(ENTER_KEYWORD_FIELD_CY)).should('be.visible').type(k);
-
-      cy.get(buildDataCy(ADD_KEYWORD_BUTTON_CY))
-        .should('be.visible')
-        .should('not.be.disabled')
-        .click()
-        .should('be.disabled');
-
-      cy.get(buildDataCy(buildKeywordNotExistWarningCy(k))).should(
-        'be.visible',
-      );
-    });
-
-    cy.get(buildDataCy(SETTINGS_SAVE_BUTTON_CY)).should('be.disabled');
-  });
-
-  it('detect keywords case insensitive', () => {
-    const TEXT = 'hello this is a Test';
-    const KEYWORDS = ['Hello', 'test'];
-
-    cy.get(buildDataCy(TEXT_INPUT_FIELD_CY)).should('be.visible').type(TEXT);
-
-    KEYWORDS.forEach((k) => {
-      cy.get(buildDataCy(ENTER_KEYWORD_FIELD_CY)).should('be.visible').type(k);
-
-      cy.get(buildDataCy(ADD_KEYWORD_BUTTON_CY))
-        .should('be.visible')
-        .should('not.be.disabled')
-        .click()
-        .should('be.disabled');
-
-      cy.get(buildDataCy(buildKeywordNotExistWarningCy(k))).should('not.exist');
-    });
-
-    cy.get(buildDataCy(SETTINGS_SAVE_BUTTON_CY)).should('be.disabled');
-  });
-
   it('does not use chatbot (by default)', () => {
     cy.get(buildDataCy(USE_CHATBOT_DATA_CY)).should('not.be.checked');
     cy.get(buildDataCy(CHATBOT_CONTAINER_CY)).should('not.exist');

diff --git a/cypress/e2e/builder/keywords.cy.ts b/cypress/e2e/builder/keywords.cy.ts
@@ -96,6 +96,55 @@ describe('Empty Keywords', () => {
       .click();
     cy.get(buildDataCy(EDITABLE_TABLE_NO_DATA_CY)).should('exist');
   });
+
+  // Detected incomplete keywords in the text.
+  // 'wef' was found incomplete in 'wefwef hello'.
+  // Check that only complete words are detected in text.
+  it('only detect complete keywords', () => {
+    const PRBLEMATIC_TEXT = 'wefwef hello';
+    const PROBLEMATIC_KEYWORDS = ['wef', 'he'];
+
+    cy.get(buildDataCy(TEXT_INPUT_FIELD_CY))
+      .should('be.visible')
+      .type(PRBLEMATIC_TEXT);
+
+    PROBLEMATIC_KEYWORDS.forEach((k) => {
+      cy.get(buildDataCy(ENTER_KEYWORD_FIELD_CY)).should('be.visible').type(k);
+
+      cy.get(buildDataCy(ADD_KEYWORD_BUTTON_CY))
+        .should('be.visible')
+        .should('not.be.disabled')
+        .click()
+        .should('be.disabled');
+
+      cy.get(buildDataCy(buildKeywordNotExistWarningCy(k))).should(
+        'be.visible',
+      );
+    });
+
+    cy.get(buildDataCy(SETTINGS_SAVE_BUTTON_CY)).should('be.disabled');
+  });
+
+  it('detect keywords case insensitive', () => {
+    const TEXT = 'hello this is a Test';
+    const KEYWORDS = ['Hello', 'test'];
+
+    cy.get(buildDataCy(TEXT_INPUT_FIELD_CY)).should('be.visible').type(TEXT);
+
+    KEYWORDS.forEach((k) => {
+      cy.get(buildDataCy(ENTER_KEYWORD_FIELD_CY)).should('be.visible').type(k);
+
+      cy.get(buildDataCy(ADD_KEYWORD_BUTTON_CY))
+        .should('be.visible')
+        .should('not.be.disabled')
+        .click()
+        .should('be.disabled');
+
+      cy.get(buildDataCy(buildKeywordNotExistWarningCy(k))).should('not.exist');
+    });
+
+    cy.get(buildDataCy(SETTINGS_SAVE_BUTTON_CY)).should('be.disabled');
+  });
 });
 
 describe.only('Existing Keywords', () => {
@@ -204,6 +253,26 @@ describe.only('Existing Keywords', () => {
   });
 
   describe('edit and delete single keyword', () => {
+    it('cannot save empty keyword', () => {
+      const KEYWORDS = getKeywords(MOCK_APP_SETTINGS_USING_CHATBOT).keywords;
+      const UPDATING_KEYWORD = KEYWORDS.at(0);
+
+      checkAllKeywords(KEYWORDS);
+
+      // edit the existing keyword to a keyword that are already in the table
+      cy.get(
+        buildDataCy(buildEditableTableEditButtonCy(UPDATING_KEYWORD.word)),
+      ).click();
+      cy.get(
+        buildDataCy(buildKeywordTextInputCy(UPDATING_KEYWORD.word, false)),
+      ).clear();
+
+      // should not be able to save the modifications
+      cy.get(
+        buildDataCy(buildEditableTableSaveButtonCy(UPDATING_KEYWORD.word)),
+      ).should('be.disabled');
+    });
+
     it('edit a keyword', () => {
       const KEYWORDS = getKeywords(MOCK_APP_SETTINGS_USING_CHATBOT).keywords;
       const UPDATING_KEYWORD = KEYWORDS.at(0);

diff --git a/src/components/common/table/EditableTable.tsx b/src/components/common/table/EditableTable.tsx
@@ -79,6 +79,11 @@ const EditableTable = <T extends RowType>({
   const getRow = (rowId: RowId): Row<T> | undefined =>
     rows.find((r) => isKeysEquals(r.rowId, rowId));
 
+  const hasMissingMandatoryValue = (row: Row<T>): boolean =>
+    columns.some((c) => !c.optional && !getColValue(row, c.key));
+
+  const hasMissingMandatoryValueInRows = rows.some(hasMissingMandatoryValue);
+
   const addInEditing = (row: Row<T>): void =>
     setEditingRows((currState) => new Map([...currState, [row.rowId, row]]));
 
@@ -213,6 +218,7 @@ const EditableTable = <T extends RowType>({
           isEditing={Boolean(editingRows.size)}
           isSelectable={isSelectable}
           isEditable={isEditable}
+          isValid={!hasMissingMandatoryValueInRows}
           totalColumns={totalColumns}
           isGlobalChecked={isGlobalChecked}
           isGlobalIndeterminate={isGlobalIndeterminate}
@@ -271,6 +277,7 @@ const EditableTable = <T extends RowType>({
                     <TableActions
                       rowId={r.rowId}
                       isEditing={isEditing(r.rowId)}
+                      isValid={!hasMissingMandatoryValue(r)}
                       onEvent={handleActionEvents}
                     />
                   </StyledTd>

diff --git a/src/components/common/table/TableActions.tsx b/src/components/common/table/TableActions.tsx
@@ -23,11 +23,17 @@ export enum TableActionEvent {
 
 type Props = {
   isEditing: boolean;
+  isValid: boolean;
   rowId: string;
   onEvent: (rowId: string, event: TableActionEvent) => void;
 };
 
-const TableActions = ({ isEditing, rowId, onEvent }: Props): JSX.Element => {
+const TableActions = ({
+  isEditing,
+  isValid,
+  rowId,
+  onEvent,
+}: Props): JSX.Element => {
   const handleEvent = (event: TableActionEvent): void => onEvent(rowId, event);
 
   return (
@@ -41,7 +47,12 @@ const TableActions = ({ isEditing, rowId, onEvent }: Props): JSX.Element => {
               data-cy={buildEditableTableSaveButtonCy(rowId)}
               aria-label="save-row-icon"
               color="success"
-              onClick={() => handleEvent(TableActionEvent.SAVE)}
+              disabled={!isValid}
+              onClick={() => {
+                if (isValid) {
+                  handleEvent(TableActionEvent.SAVE);
+                }
+              }}
             >
               <SaveIcon />
             </IconButton>

diff --git a/src/components/common/table/TableHeader.tsx b/src/components/common/table/TableHeader.tsx
@@ -42,6 +42,7 @@ type Props<T extends RowType> = {
   isEditing: boolean;
   isSelectable: boolean;
   isEditable: boolean;
+  isValid: boolean;
   totalColumns: number;
   isGlobalChecked: boolean;
   isGlobalIndeterminate: boolean;
@@ -57,6 +58,7 @@ const TableHeader = <T extends RowType>({
   isEditing,
   isSelectable,
   isEditable,
+  isValid,
   totalColumns,
   isGlobalChecked,
   isGlobalIndeterminate,
@@ -98,8 +100,13 @@ const TableHeader = <T extends RowType>({
               >
                 <IconButton
                   data-cy={EDITABLE_TABLE_SAVE_ALL_BUTTON_CY}
+                  disabled={!isValid}
                   aria-label="save-all-rows-icon"
-                  onClick={onSaveAll}
+                  onClick={() => {
+                    if (isValid) {
+                      onSaveAll();
+                    }
+                  }}
                 >
                   <SaveIcon />
                 </IconButton>

diff --git a/src/components/common/table/types.ts b/src/components/common/table/types.ts
@@ -11,6 +11,7 @@ export type Column<T extends RowType> = {
   key: RowKey<T>;
   displayColumn: string;
   multiline?: boolean;
+  optional?: boolean;
   renderAfter?: (row: Row<T>) => JSX.Element | null;
 };
 

diff --git a/src/components/views/admin/KeywordsTable.tsx b/src/components/views/admin/KeywordsTable.tsx
@@ -83,6 +83,7 @@ const KeywordsTable = ({
       key: 'word',
       displayColumn: t(TEXT_ANALYSIS.BUILDER_KEYWORDS_TABLE_KEYWORD_COLUMN),
       renderAfter: (content) => renderWarningIcon(content, text),
+      optional: false,
     },
     {
       key: 'def',