Address CR changes

Author: sabarasaba

x-pack/plugins/upgrade_assistant/__jest__/client_integration/helpers/app_context.mock.ts

@@ -35,7 +35,7 @@ const idToUrlMap = {
 };
 type IdKey = keyof typeof idToUrlMap;
 
-const convertObjectValuesToString = (params: Record<string, any>) => {
+const stringifySearchParams = (params: Record<string, any>) => {
   return Object.keys(params).reduce((list, key) => {
     const value = typeof params[key] === 'object' ? JSON.stringify(params[key]) : params[key];
 
@@ -48,7 +48,7 @@ const shareMock = sharePluginMock.createSetupContract();
 shareMock.url.locators.get = (id: IdKey) => ({
   useUrl: (): string | undefined => idToUrlMap[id],
   getUrl: (params: Record<string, any>): string | undefined =>
-    `${idToUrlMap[id]}?${new URLSearchParams(convertObjectValuesToString(params)).toString()}`,
+    `${idToUrlMap[id]}?${new URLSearchParams(stringifySearchParams(params)).toString()}`,
 });
 
 export const getAppContextMock = () => ({

x-pack/plugins/upgrade_assistant/__jest__/client_integration/overview/fix_logs_step/fix_logs_step.test.tsx

@@ -7,14 +7,17 @@
 
 import { act } from 'react-dom/test-utils';
 
+// Once the logs team register the kibana locators in their app, we should be able
+// to remove this mock and follow a similar approach to how discover link is tested.
+// See: https://github.com/elastic/kibana/issues/104855
 const MOCKED_TIME = '2021-09-05T10:49:01.805Z';
-jest.mock('../../../../public/application/lib/utils', () => {
-  const originalModule = jest.requireActual('../../../../public/application/lib/utils');
+jest.mock('../../../../public/application/lib/logs_checkpoint', () => {
+  const originalModule = jest.requireActual('../../../../public/application/lib/logs_checkpoint');
 
   return {
     __esModule: true,
     ...originalModule,
-    getLastCheckpointFromLS: jest.fn().mockReturnValue('2021-09-05T10:49:01.805Z'),
+    loadLogsCheckpoint: jest.fn().mockReturnValue('2021-09-05T10:49:01.805Z'),
   };
 });
 

x-pack/plugins/upgrade_assistant/public/application/components/overview/fix_logs_step/deprecations_count_checkpoint/deprecations_count_checkpoint.tsx

@@ -48,19 +48,19 @@ const i18nTexts = {
 };
 
 interface Props {
-  lastCheckpoint: string;
-  resetLastCheckpoint: (value: string) => void;
+  checkpoint: string;
+  setCheckpoint: (value: string) => void;
 }
 
 export const DeprecationsCountCheckpoint: FunctionComponent<Props> = ({
-  lastCheckpoint,
-  resetLastCheckpoint,
+  checkpoint,
+  setCheckpoint,
 }) => {
   const {
     services: { api },
   } = useAppContext();
   const { data, error, isLoading, resendRequest, isInitialRequest } = api.getDeprecationLogsCount(
-    lastCheckpoint
+    checkpoint
   );
 
   const warningsCount = data?.count || 0;
@@ -70,7 +70,7 @@ export const DeprecationsCountCheckpoint: FunctionComponent<Props> = ({
 
   const onResetClick = () => {
     const now = moment().toISOString();
-    resetLastCheckpoint(now);
+    setCheckpoint(now);
   };
 
   if (isInitialRequest && isLoading) {
@@ -97,7 +97,7 @@ export const DeprecationsCountCheckpoint: FunctionComponent<Props> = ({
 
   return (
     <EuiCallOut
-      title={i18nTexts.calloutTitle(warningsCount, lastCheckpoint)}
+      title={i18nTexts.calloutTitle(warningsCount, checkpoint)}
       color={calloutTint}
       iconType={calloutIcon}
       data-test-subj={calloutTestId}

x-pack/plugins/upgrade_assistant/public/application/components/overview/fix_logs_step/external_links.tsx

@@ -19,7 +19,7 @@ import {
 } from '../../../../../common/constants';
 
 interface Props {
-  lastCheckpoint: string;
+  checkpoint: string;
 }
 
 const getDeprecationIndexPatternId = async (dataService: DataPublicPluginStart) => {
@@ -41,7 +41,7 @@ const getDeprecationIndexPatternId = async (dataService: DataPublicPluginStart)
   }
 };
 
-const DiscoverAppLink: FunctionComponent<Props> = ({ lastCheckpoint }) => {
+const DiscoverAppLink: FunctionComponent<Props> = ({ checkpoint }) => {
   const {
     services: { data: dataService },
     plugins: { share },
@@ -62,15 +62,15 @@ const DiscoverAppLink: FunctionComponent<Props> = ({ lastCheckpoint }) => {
         indexPatternId,
         query: {
           language: 'kuery',
-          query: `@timestamp > "${lastCheckpoint}"`,
+          query: `@timestamp > "${checkpoint}"`,
         },
       });
 
       setDiscoveryUrl(url);
     };
 
     getDiscoveryUrl();
-  }, [dataService, lastCheckpoint, share.url.locators]);
+  }, [dataService, checkpoint, share.url.locators]);
 
   return (
     <EuiLink href={discoveryUrl} data-test-subj="viewDiscoverLogs">
@@ -82,15 +82,15 @@ const DiscoverAppLink: FunctionComponent<Props> = ({ lastCheckpoint }) => {
   );
 };
 
-const ObservabilityAppLink: FunctionComponent<Props> = ({ lastCheckpoint }) => {
+const ObservabilityAppLink: FunctionComponent<Props> = ({ checkpoint }) => {
   const {
     services: {
       core: { http },
     },
   } = useAppContext();
   const logStreamUrl = http?.basePath?.prepend(
     `/app/logs/stream?sourceId=${DEPRECATION_LOGS_SOURCE_ID}&logPosition=(end:now,start:${encode(
-      lastCheckpoint
+      checkpoint
     )})`
   );
 
@@ -104,7 +104,7 @@ const ObservabilityAppLink: FunctionComponent<Props> = ({ lastCheckpoint }) => {
   );
 };
 
-export const ExternalLinks: FunctionComponent<Props> = ({ lastCheckpoint }) => {
+export const ExternalLinks: FunctionComponent<Props> = ({ checkpoint }) => {
   return (
     <EuiFlexGroup>
       <EuiFlexItem>
@@ -118,7 +118,7 @@ export const ExternalLinks: FunctionComponent<Props> = ({ lastCheckpoint }) => {
             </p>
           </EuiText>
           <EuiSpacer size="m" />
-          <ObservabilityAppLink lastCheckpoint={lastCheckpoint} />
+          <ObservabilityAppLink checkpoint={checkpoint} />
         </EuiPanel>
       </EuiFlexItem>
       <EuiFlexItem>
@@ -132,7 +132,7 @@ export const ExternalLinks: FunctionComponent<Props> = ({ lastCheckpoint }) => {
             </p>
           </EuiText>
           <EuiSpacer size="m" />
-          <DiscoverAppLink lastCheckpoint={lastCheckpoint} />
+          <DiscoverAppLink checkpoint={checkpoint} />
         </EuiPanel>
       </EuiFlexItem>
     </EuiFlexGroup>

x-pack/plugins/upgrade_assistant/public/application/components/overview/fix_logs_step/fix_logs_step.tsx

@@ -5,7 +5,7 @@
  * 2.0.
  */
 
-import React, { FunctionComponent, useState, useCallback } from 'react';
+import React, { FunctionComponent, useState, useEffect } from 'react';
 
 import { i18n } from '@kbn/i18n';
 import { EuiText, EuiSpacer, EuiPanel, EuiCallOut } from '@elastic/eui';
@@ -15,7 +15,7 @@ import { ExternalLinks } from './external_links';
 import { DeprecationsCountCheckpoint } from './deprecations_count_checkpoint';
 import { useDeprecationLogging } from './use_deprecation_logging';
 import { DeprecationLoggingToggle } from './deprecation_logging_toggle';
-import { getLastCheckpointFromLS, setLastCheckpointToLS } from '../../../lib/utils';
+import { loadLogsCheckpoint, saveLogsCheckpoint } from '../../../lib/logs_checkpoint';
 
 const i18nTexts = {
   identifyStepTitle: i18n.translate('xpack.upgradeAssistant.overview.identifyStepTitle', {
@@ -50,12 +50,11 @@ const i18nTexts = {
 
 const FixLogsStep: FunctionComponent = () => {
   const state = useDeprecationLogging();
-  const [lastCheckpoint, setLastCheckpoint] = useState(getLastCheckpointFromLS());
+  const [checkpoint, setCheckpoint] = useState(loadLogsCheckpoint());
 
-  const resetLastCheckpoint = useCallback((newValue: string) => {
-    setLastCheckpoint(newValue);
-    setLastCheckpointToLS(newValue);
-  }, []);
+  useEffect(() => {
+    saveLogsCheckpoint(checkpoint);
+  }, [checkpoint]);
 
   return (
     <>
@@ -88,17 +87,14 @@ const FixLogsStep: FunctionComponent = () => {
             <h4>{i18nTexts.analyzeTitle}</h4>
           </EuiText>
           <EuiSpacer size="m" />
-          <ExternalLinks lastCheckpoint={lastCheckpoint} />
+          <ExternalLinks checkpoint={checkpoint} />
 
           <EuiSpacer size="xl" />
           <EuiText data-test-subj="deprecationsCountTitle">
             <h4>{i18nTexts.deprecationsCountCheckpointTitle}</h4>
           </EuiText>
           <EuiSpacer size="m" />
-          <DeprecationsCountCheckpoint
-            lastCheckpoint={lastCheckpoint}
-            resetLastCheckpoint={resetLastCheckpoint}
-          />
+          <DeprecationsCountCheckpoint checkpoint={checkpoint} setCheckpoint={setCheckpoint} />
         </>
       )}
     </>

x-pack/plugins/upgrade_assistant/public/application/lib/logs_checkpoint.ts

@@ -0,0 +1,30 @@
+/*
+ * 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 moment from 'moment-timezone';
+
+import { Storage } from '../../shared_imports';
+
+const SETTING_ID = 'kibana.upgradeAssistant.lastCheckpoint';
+const localStorage = new Storage(window.localStorage);
+
+export const loadLogsCheckpoint = () => {
+  const storedValue = moment(localStorage.get(SETTING_ID));
+
+  if (storedValue.isValid()) {
+    return storedValue.toISOString();
+  }
+
+  const now = moment().toISOString();
+  localStorage.set(SETTING_ID, now);
+
+  return now;
+};
+
+export const saveLogsCheckpoint = (value: string) => {
+  localStorage.set(SETTING_ID, value);
+};

x-pack/plugins/upgrade_assistant/public/application/lib/utils.ts

@@ -5,12 +5,10 @@
  * 2.0.
  */
 
-import moment from 'moment-timezone';
 import { pipe } from 'fp-ts/lib/pipeable';
 import { tryCatch, fold } from 'fp-ts/lib/Either';
 
 import { DEPRECATION_WARNING_UPPER_LIMIT } from '../../../common/constants';
-import { Storage } from '../../shared_imports';
 
 export const validateRegExpString = (s: string) =>
   pipe(
@@ -36,23 +34,3 @@ export const getDeprecationsUpperLimit = (count: number) => {
 
   return count.toString();
 };
-
-const LS_SETTING_ID = 'kibana.upgradeAssistant.lastCheckpoint';
-const localStorage = new Storage(window.localStorage);
-
-export const getLastCheckpointFromLS = () => {
-  const storedValue = moment(localStorage.get(LS_SETTING_ID));
-
-  if (storedValue.isValid()) {
-    return storedValue.toISOString();
-  }
-
-  const now = moment().toISOString();
-  localStorage.set(LS_SETTING_ID, now);
-
-  return now;
-};
-
-export const setLastCheckpointToLS = (value: string) => {
-  localStorage.set(LS_SETTING_ID, value);
-};