From 7825c7b1227e99bf3ff2495188bd733817432d7f Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Wed, 24 Feb 2021 12:05:17 -0800 Subject: [PATCH] updated types to remove casting --- .../hooks/persist_exception_item.ts | 28 +++++++++++++------ .../public/exceptions/hooks/translations.ts | 15 ---------- .../lists/public/exceptions/hooks/use_api.ts | 27 ++++++------------ .../lists/public/exceptions/transforms.ts | 24 ++++++++-------- .../plugins/lists/public/exceptions/types.ts | 2 -- .../exceptions/exceptions_modal.spec.ts | 8 ++++-- .../cypress/screens/exceptions.ts | 1 + .../exceptions/add_exception_modal/index.tsx | 4 ++- .../exceptions/edit_exception_modal/index.tsx | 4 ++- 9 files changed, 51 insertions(+), 62 deletions(-) delete mode 100644 x-pack/plugins/lists/public/exceptions/hooks/translations.ts 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 index f08d53d83ca505..6135d14aef6a49 100644 --- 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 + Dispatch ]; /** @@ -33,7 +36,9 @@ export const usePersistExceptionItem = ({ http, onError, }: PersistHookProps): ReturnPersistExceptionItem => { - const [exceptionListItem, setExceptionItem] = useState(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 deleted file mode 100644 index 41c64216159626..00000000000000 --- a/x-pack/plugins/lists/public/exceptions/hooks/translations.ts +++ /dev/null @@ -1,15 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -import { i18n } from '@kbn/i18n'; - -export const ADD_EXCEPTION_ITEM_TYPE_ERROR = i18n.translate( - 'xpack.lists.exceptions.hooks.addExceptionItemTypeError', - { - defaultMessage: 'Unable to create exception item. Item malformed.', - } -); diff --git a/x-pack/plugins/lists/public/exceptions/hooks/use_api.ts b/x-pack/plugins/lists/public/exceptions/hooks/use_api.ts index 878b5e3836afe8..9e4e338b09dbf3 100644 --- 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 { 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 { 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 index 92c2cdb3ba337d..0791760611bf51 100644 --- 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 = ( + 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); } }); - 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 index e37c03978c9f6d..03cae387711f85 100644 --- 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 index 14b5cd5cddc59f..154e90d509c612 100644 --- 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 index 3d7fe58fc0c776..2479b76cf1de47 100644 --- 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/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx index b083ecbafeb2c1..3a2170d126a241 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx @@ -466,7 +466,9 @@ export const AddExceptionModal = memo(function AddExceptionModal({ )} {fetchOrCreateListError == null && ( - {i18n.CANCEL} + + {i18n.CANCEL} + - {i18n.CANCEL} + + {i18n.CANCEL} +