updated types to remove casting

elastic · Feb 24, 2021 · 7825c7b · 7825c7b
1 parent a4fd2f3
commit 7825c7b
Show file tree

Hide file tree

Showing 9 changed files with 51 additions and 62 deletions.
diff --git a/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.ts b/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.ts
@@ -7,10 +7,13 @@
 
 import { Dispatch, useEffect, useState } from 'react';
 
-import { UpdateExceptionListItemSchema } from '../../../common/schemas';
+import {
+  CreateExceptionListItemSchema,
+  UpdateExceptionListItemSchema,
+} from '../../../common/schemas';
 import { addExceptionListItem, updateExceptionListItem } from '../api';
-import { transformOutput } from '../transforms';
-import { AddExceptionListItem, PersistHookProps } from '../types';
+import { transformNewItemOutput, transformOutput } from '../transforms';
+import { PersistHookProps } from '../types';
 
 interface PersistReturnExceptionItem {
   isLoading: boolean;
@@ -19,7 +22,7 @@ interface PersistReturnExceptionItem {
 
 export type ReturnPersistExceptionItem = [
   PersistReturnExceptionItem,
-  Dispatch<AddExceptionListItem | null>
+  Dispatch<CreateExceptionListItemSchema | UpdateExceptionListItemSchema | null>
 ];
 
 /**
@@ -33,7 +36,9 @@ export const usePersistExceptionItem = ({
   http,
   onError,
 }: PersistHookProps): ReturnPersistExceptionItem => {
-  const [exceptionListItem, setExceptionItem] = useState<AddExceptionListItem | null>(null);
+  const [exceptionListItem, setExceptionItem] = useState<
+    CreateExceptionListItemSchema | UpdateExceptionListItemSchema | null
+  >(null);
   const [isSaved, setIsSaved] = useState(false);
   const [isLoading, setIsLoading] = useState(false);
   const isUpdateExceptionItem = (item: unknown): item is UpdateExceptionListItemSchema =>
@@ -48,17 +53,22 @@ export const usePersistExceptionItem = ({
       if (exceptionListItem != null) {
         try {
           setIsLoading(true);
-          // Please see `x-pack/plugins/lists/public/exceptions/transforms.ts` doc notes
-          // for context around the temporary `id`
-          const transformedList = transformOutput(exceptionListItem);
 
-          if (isUpdateExceptionItem(transformedList)) {
+          if (isUpdateExceptionItem(exceptionListItem)) {
+            // Please see `x-pack/plugins/lists/public/exceptions/transforms.ts` doc notes
+            // for context around the temporary `id`
+            const transformedList = transformOutput(exceptionListItem);
+
             await updateExceptionListItem({
               http,
               listItem: transformedList,
               signal: abortCtrl.signal,
             });
           } else {
+            // Please see `x-pack/plugins/lists/public/exceptions/transforms.ts` doc notes
+            // for context around the temporary `id`
+            const transformedList = transformNewItemOutput(exceptionListItem);
+
             await addExceptionListItem({
               http,
               listItem: transformedList,

diff --git a/x-pack/plugins/lists/public/exceptions/hooks/translations.ts b/x-pack/plugins/lists/public/exceptions/hooks/translations.ts
diff --git a/x-pack/plugins/lists/public/exceptions/hooks/use_api.ts b/x-pack/plugins/lists/public/exceptions/hooks/use_api.ts
@@ -14,13 +14,11 @@ import {
   ExceptionListItemSchema,
   ExceptionListSchema,
   UpdateExceptionListItemSchema,
-  createExceptionListItemSchema,
 } from '../../../common/schemas';
 import { ApiCallFindListsItemsMemoProps, ApiCallMemoProps, ApiListExportProps } from '../types';
 import { getIdsAndNamespaces } from '../utils';
-import { transformInput, transformOutput } from '../transforms';
+import { transformInput, transformNewItemOutput, transformOutput } from '../transforms';
 
-import * as i18n from './translations';
 export interface ExceptionsApi {
   addExceptionListItem: (arg: {
     listItem: CreateExceptionListItemSchema;
@@ -49,22 +47,13 @@ export const useApi = (http: HttpStart): ExceptionsApi => {
         listItem: CreateExceptionListItemSchema;
       }): Promise<ExceptionListItemSchema> {
         const abortCtrl = new AbortController();
-        const sanitizedItem = transformOutput(listItem);
+        const sanitizedItem: CreateExceptionListItemSchema = transformNewItemOutput(listItem);
 
-        // This was added to satisfy Typescript, which I don't love,
-        // but felt better than doing an `as` cast. Wasn't able to
-        // figure out how to do a generic to specify that input is of
-        // type A or B, and if it is A returns A, if B returns B. Can
-        // delete this if/else statement if figured out
-        if (createExceptionListItemSchema.is(sanitizedItem)) {
-          return Api.addExceptionListItem({
-            http,
-            listItem: sanitizedItem,
-            signal: abortCtrl.signal,
-          });
-        } else {
-          throw new Error(i18n.ADD_EXCEPTION_ITEM_TYPE_ERROR);
-        }
+        return Api.addExceptionListItem({
+          http,
+          listItem: sanitizedItem,
+          signal: abortCtrl.signal,
+        });
       },
       async deleteExceptionItem({
         id,
@@ -232,7 +221,7 @@ export const useApi = (http: HttpStart): ExceptionsApi => {
         listItem: UpdateExceptionListItemSchema;
       }): Promise<ExceptionListItemSchema> {
         const abortCtrl = new AbortController();
-        const sanitizedItem = transformOutput(listItem);
+        const sanitizedItem: UpdateExceptionListItemSchema = transformOutput(listItem);
 
         return Api.updateExceptionListItem({
           http,

diff --git a/x-pack/plugins/lists/public/exceptions/transforms.ts b/x-pack/plugins/lists/public/exceptions/transforms.ts
@@ -34,13 +34,14 @@ import { addIdToItem, removeIdFromItem } from '../../common/shared_imports';
  * @returns The exceptionItem transformed from the output
  */
 export const transformOutput = (
-  exceptionItem:
-    | CreateExceptionListItemSchema
-    | UpdateExceptionListItemSchema
-    | ExceptionListItemSchema
-): CreateExceptionListItemSchema | UpdateExceptionListItemSchema | ExceptionListItemSchema =>
+  exceptionItem: UpdateExceptionListItemSchema | ExceptionListItemSchema
+): UpdateExceptionListItemSchema | ExceptionListItemSchema =>
   flow(removeIdFromExceptionItemsEntries)(exceptionItem);
 
+export const transformNewItemOutput = (
+  exceptionItem: CreateExceptionListItemSchema
+): CreateExceptionListItemSchema => flow(removeIdFromExceptionItemsEntries)(exceptionItem);
+
 /**
  * Transforms the output of rules to compensate for technical debt or UI concerns such as
  * ReactJS preferences for having ids within arrays if the data is not modeled that way.
@@ -95,13 +96,10 @@ export const addIdToExceptionItemEntries = (
  * @param exceptionItem The exceptionItem to remove an id from the entries.
  * @returns exceptionItem The exceptionItem but with id removed from the entries
  */
-export const removeIdFromExceptionItemsEntries = (
-  exceptionItem:
-    | CreateExceptionListItemSchema
-    | UpdateExceptionListItemSchema
-    | ExceptionListItemSchema
-): CreateExceptionListItemSchema | UpdateExceptionListItemSchema | ExceptionListItemSchema => {
-  const { entries, ...itemInfo } = exceptionItem;
+export const removeIdFromExceptionItemsEntries = <T extends { entries: EntriesArray }>(
+  exceptionItem: T
+): T => {
+  const { entries } = exceptionItem;
   const entriesNoId = entries.map((entry) => {
     if (entry.type === 'nested') {
       return removeIdFromItem({
@@ -112,5 +110,5 @@ export const removeIdFromExceptionItemsEntries = (
       return removeIdFromItem<Entry>(entry);
     }
   });
-  return { ...itemInfo, entries: entriesNoId as EntriesArray };
+  return { ...exceptionItem, entries: entriesNoId };
 };
diff --git a/x-pack/plugins/lists/public/exceptions/types.ts b/x-pack/plugins/lists/public/exceptions/types.ts
@@ -33,8 +33,6 @@ export interface Pagination {
 
 export type AddExceptionList = UpdateExceptionListSchema | CreateExceptionListSchema;
 
-export type AddExceptionListItem = CreateExceptionListItemSchema | UpdateExceptionListItemSchema;
-
 export interface PersistHookProps {
   http: HttpStart;
   onError: (arg: Error) => void;

diff --git a/x-pack/plugins/security_solution/cypress/integration/exceptions/exceptions_modal.spec.ts b/x-pack/plugins/security_solution/cypress/integration/exceptions/exceptions_modal.spec.ts
@@ -28,6 +28,7 @@ import {
   FIELD_INPUT,
   LOADING_SPINNER,
   EXCEPTION_ITEM_CONTAINER,
+  ADD_EXCEPTIONS_BTN,
 } from '../../screens/exceptions';
 
 import { DETECTIONS_URL } from '../../urls/navigation';
@@ -62,7 +63,8 @@ describe('Exceptions modal', () => {
   });
 
   it('Does not overwrite values and-ed together', () => {
-    // VALID VALUES
+    cy.get(ADD_EXCEPTIONS_BTN).click({ force: true });
+
     // add multiple entries with invalid field values
     addExceptionEntryFieldValue('agent.name', 0);
     cy.get(ADD_AND_BTN).click();
@@ -79,6 +81,8 @@ describe('Exceptions modal', () => {
   });
 
   it('Does not overwrite values or-ed together', () => {
+    cy.get(ADD_EXCEPTIONS_BTN).click({ force: true });
+
     // exception item 1
     addExceptionEntryFieldValueOfItemX('agent.name', 0, 0);
     cy.get(ADD_AND_BTN).click();
@@ -90,7 +94,7 @@ describe('Exceptions modal', () => {
     cy.get(ADD_AND_BTN).click();
     addExceptionEntryFieldValueOfItemX('user.last', 1, 1);
     cy.get(ADD_AND_BTN).click();
-    addExceptionEntryFieldValueOfItemX('e', 0, 2);
+    addExceptionEntryFieldValueOfItemX('e', 1, 2);
 
     // delete single entry from exception item 2
     cy.get(ENTRY_DELETE_BTN).eq(3).click();

diff --git a/x-pack/plugins/security_solution/cypress/screens/exceptions.ts b/x-pack/plugins/security_solution/cypress/screens/exceptions.ts
@@ -37,6 +37,7 @@ export const FIELD_INPUT_LIST_BTN = '[data-test-subj="comboBoxToggleListButton"]
 export const CANCEL_BTN = '[data-test-subj="cancelExceptionAddButton"]';
 
 export const BUILDER_MODAL_BODY = '[data-test-subj="exceptionsBuilderWrapper"]';
+
 export const EXCEPTIONS_TABLE_TAB = '[data-test-subj="allRulesTableTab-exceptions"]';
 
 export const EXCEPTIONS_TABLE = '[data-test-subj="exceptions-table"]';

diff --git a/...ugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx b/...ugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx
@@ -466,7 +466,9 @@ export const AddExceptionModal = memo(function AddExceptionModal({
         )}
       {fetchOrCreateListError == null && (
         <EuiModalFooter>
-          <EuiButtonEmpty onClick={onCancel}>{i18n.CANCEL}</EuiButtonEmpty>
+          <EuiButtonEmpty data-test-subj="cancelExceptionAddButton" onClick={onCancel}>
+            {i18n.CANCEL}
+          </EuiButtonEmpty>
 
           <EuiButton
             data-test-subj="add-exception-confirm-button"

diff --git a/...gins/security_solution/public/common/components/exceptions/edit_exception_modal/index.tsx b/...gins/security_solution/public/common/components/exceptions/edit_exception_modal/index.tsx
@@ -382,7 +382,9 @@ export const EditExceptionModal = memo(function EditExceptionModal({
       )}
       {updateError == null && (
         <EuiModalFooter>
-          <EuiButtonEmpty onClick={onCancel}>{i18n.CANCEL}</EuiButtonEmpty>
+          <EuiButtonEmpty data-test-subj="cancelExceptionAddButton" onClick={onCancel}>
+            {i18n.CANCEL}
+          </EuiButtonEmpty>
 
           <EuiButton
             data-test-subj="edit-exception-confirm-button"