[Cases] Align cases lint rules with security solution #117177

cnasikas · 2021-11-02T16:46:48Z

Summary

This PR aligns cases eslint rules with security solution eslint rules.

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2021-11-02T16:46:51Z

Pinging @elastic/security-threat-hunting-cases (Team:Threat Hunting:Cases)

cnasikas · 2021-11-03T18:49:35Z

@elasticmachine merge upstream

cnasikas · 2021-11-04T12:31:37Z

x-pack/plugins/cases/public/common/lib/kibana/kibana_react.mock.ts

@@ -5,6 +5,8 @@
 * 2.0.
 */

+/* eslint-disable react/display-name */


Disabling as it is a mock file

cnasikas · 2021-11-04T12:33:53Z

x-pack/plugins/cases/public/components/all_cases/expanded_row.tsx

 import styled from 'styled-components';
 import { Case, SubCase } from '../../containers/types';
 import { CasesColumns } from './columns';
 import { AssociationType } from '../../../common';

 type ExpandedRowMap = Record<string, Element> | {};

-const EuiBasicTable: any = _EuiBasicTable;
+// @ts-expect-error TS2769


I could disable the linting rule about any but I found it better to use @ts-expect-error because we will be warned if it will be fixed and we do not have to declare _EuiBasicTable (better readability)

cnasikas · 2021-11-04T12:34:22Z

x-pack/plugins/cases/public/components/all_cases/index.test.tsx

-        (i, key) => i.name != null && !i.hasOwnProperty('actions') && checkIt(`${i.name}`, key)
+        (i, key) =>
+          i.name != null &&
+          !Object.prototype.hasOwnProperty.call(i, 'actions') &&


Replace hasOwnProperty with Object.prototype.hasOwnProperty.call

cnasikas · 2021-11-04T12:34:36Z

x-pack/plugins/cases/public/components/all_cases/index.test.tsx

@@ -378,7 +381,9 @@ describe('AllCasesGeneric', () => {
      })
    );
    await waitFor(() => {
-      result.current.map((i) => i.name != null && !i.hasOwnProperty('actions'));
+      result.current.map(
+        (i) => i.name != null && !Object.prototype.hasOwnProperty.call(i, 'actions')


Replace hasOwnProperty with Object.prototype.hasOwnProperty.call

cnasikas · 2021-11-04T12:34:43Z

x-pack/plugins/cases/common/api/runtime_types.ts

@@ -60,7 +60,7 @@ export const decodeOrThrow =
 const getExcessProps = (props: rt.Props, r: Record<string, unknown>): string[] => {
  const ex: string[] = [];
  for (const k of Object.keys(r)) {
-    if (!props.hasOwnProperty(k)) {
+    if (!Object.prototype.hasOwnProperty.call(props, k)) {


Replace hasOwnProperty with Object.prototype.hasOwnProperty.call

~~Is this check really needed? I believe Object.keys() does not return inherited properties.~~
EDIT: now I see we are checking props, not r. 👍

cnasikas · 2021-11-04T12:35:20Z

x-pack/plugins/cases/public/components/connectors/connectors_registry.ts

@@ -9,8 +9,25 @@ import { i18n } from '@kbn/i18n';
 import { CaseConnector, CaseConnectorsRegistry } from './types';

 export const createCaseConnectorsRegistry = (): CaseConnectorsRegistry => {
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any


TS won this battle

cnasikas · 2021-11-04T12:36:13Z

x-pack/plugins/cases/public/components/connectors/connectors_registry.ts

-      }
-      return connectors.get(id)!;
+      const connector = connectors.get(id);
+      assertConnectorExists(connector, id);


Move the logic to an assertion so I can return the connector without having to put ! at the end of the return statement.

cnasikas · 2021-11-04T12:39:04Z

x-pack/plugins/cases/public/components/markdown_editor/plugins/lens/saved_objects_finder.tsx

                const fullName = currentSavedObjectMetaData.getTooltipForSavedObject
                  ? currentSavedObjectMetaData.getTooltipForSavedObject(item.savedObject)
-                  : `${item.title} (${currentSavedObjectMetaData!.name})`;
+                  : `${item.title} (${currentSavedObjectMetaData.name})`;


Remove the ! non-null assertion operator. The non-null check is done above

cnasikas · 2021-11-04T12:39:43Z

x-pack/plugins/cases/public/components/markdown_editor/plugins/lens/use_lens_button_toggle.ts

@@ -112,6 +114,7 @@ export const useLensButtonToggle = ({
            ) {
              if (child.type === 'text') break outer; // don't dive into `text` nodes
              node = child;
+              // eslint-disable-next-line no-continue


Trying to fix it may introduce bugs.

cnasikas · 2021-11-04T12:39:45Z

x-pack/plugins/cases/public/components/markdown_editor/plugins/lens/use_lens_button_toggle.ts

@@ -5,6 +5,8 @@
 * 2.0.
 */

+/* eslint-disable @typescript-eslint/no-non-null-assertion */


Trying to fix it may introduce bugs.

cnasikas · 2021-11-04T12:40:49Z

x-pack/plugins/cases/public/components/markdown_editor/renderer.tsx

@@ -16,16 +16,25 @@ interface Props {
  disableLinks?: boolean;
 }

+const withDisabledLinks = (disableLinks?: boolean): React.FC<EuiLinkAnchorProps> => {


Create a HOC that returns a MarkdownLink component with disableLinks so we can declare a displayName.

cnasikas · 2021-11-04T12:41:53Z

x-pack/plugins/cases/public/components/user_action_tree/index.tsx

@@ -165,7 +176,9 @@ export const UserActionTree = React.memo(
    const { isLoadingIds, patchComment } = useUpdateComment();
    const currentUser = useCurrentUser();
    const [manageMarkdownEditIds, setManageMarkdownEditIds] = useState<string[]>([]);
-    const commentRefs = useRef<Record<string, any>>({});
+    const commentRefs = useRef<


cnasikas · 2021-11-04T12:42:07Z

x-pack/plugins/cases/public/components/user_action_tree/index.tsx

@@ -129,6 +129,17 @@ const MyEuiCommentList = styled(EuiCommentList)`
 const DESCRIPTION_ID = 'description';
 const NEW_ID = 'newComment';

+const isAddCommentRef = (


Checks if the ref is of type AddCommentRefObject

cnasikas · 2021-11-04T12:56:01Z

x-pack/plugins/cases/public/components/user_action_tree/index.tsx

-          commentRefs.current &&
-          commentRefs.current[draftComment.commentId] &&
-          commentRefs.current[draftComment.commentId].editor?.textarea &&
-          commentRefs.current[draftComment.commentId].editor?.toolbar


toolbar is not part of the editor object. Nevertheless, openLensModal function checks for its existence.

cnasikas · 2021-11-04T12:58:10Z

x-pack/plugins/cases/server/authorization/authorization.ts

-                const owner = requiredPrivileges.get(privilege)!;
-                authorizedOwners.push(owner);
+                const owner = requiredPrivileges.get(privilege);
+                if (owner) {


Remove the ! non-null assertion operation and check if is empty before pushing.

cnasikas · 2021-11-04T12:58:48Z

x-pack/plugins/cases/server/connectors/case/schema.ts

@@ -83,6 +83,7 @@ const SwimlaneFieldsSchema = schema.object({

 const NoneFieldsSchema = schema.nullable(schema.object({}));

+// eslint-disable-next-line @typescript-eslint/no-explicit-any


Lost this battle with TS again

cnasikas · 2021-11-04T12:59:45Z

x-pack/plugins/cases/server/connectors/servicenow/sir_format.ts

        if (field && !manageDuplicate[alertFieldMapping[alertField].sirFieldKey].has(field)) {
          manageDuplicate[alertFieldMapping[alertField].sirFieldKey].add(field);
-          acc = {


Fix for the linting rule prohibiting the direct assignment of function arguments

cnasikas · 2021-11-04T13:00:37Z

x-pack/plugins/cases/server/common/error.ts

@@ -8,10 +8,14 @@
 import { Boom, isBoom } from '@hapi/boom';
 import { Logger } from 'src/core/server';

+export interface HTTPError extends Error {


Used in x-pack/plugins/cases/server/routes/api/utils.ts

cnasikas · 2021-11-04T13:01:14Z

x-pack/plugins/cases/server/routes/api/utils.ts

-export function wrapError(error: any): CustomHttpResponseOptions<ResponseError> {
+
+export function wrapError(
+  error: CaseError | Boom | HTTPError | Error


Type error instead of any

cnasikas · 2021-11-04T13:01:54Z

x-pack/plugins/cases/server/scripts/sub_cases/index.ts

@@ -4,7 +4,11 @@
 * 2.0; you may not use this file except in compliance with the Elastic License
 * 2.0.
 */
+
+/* eslint-disable @typescript-eslint/no-explicit-any */


As it is a script we do not care about TS errors

semd

LGTM! despite a couple of lost battles against TS (LOL), it is a big improvement.
Thanks Christos, this will help us all.

kibanamachine · 2021-11-04T15:00:39Z

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`cases`	303.9KB	304.2KB	+336.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`cases`	80.4KB	80.4KB	+53.0B

History

💚 Build #4127 succeeded 987abb5
💔 Build #4015 failed 03b8280
💔 Build #3943 failed a8f17ab9929049ed59f0031f7677ad5e97e48503
💔 Build #3740 failed d4cbfde

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @cnasikas

kibanamachine · 2021-11-04T15:51:29Z

💔 Backport failed

The backport operation could not be completed due to the following error:
There are no branches to backport to. Aborting.

The backport PRs will be merged automatically after passing CI.

To backport manually run:
node scripts/backport --pr 117177

kibanamachine · 2021-11-08T15:59:49Z