address review feedback

elastic · Apr 28, 2021 · 52fe1fd · 52fe1fd
1 parent 138cd07
commit 52fe1fd
Show file tree

Hide file tree

Showing 10 changed files with 66 additions and 106 deletions.
diff --git a/...grade_assistant/public/application/components/es_deprecations/deprecation_tab_content.tsx b/...grade_assistant/public/application/components/es_deprecations/deprecation_tab_content.tsx
@@ -34,16 +34,17 @@ export interface CheckupTabProps extends UpgradeAssistantTabProps {
   checkupLabel: string;
 }
 
-export const filterDeps = (level: LevelFilterOption, search: string = '') => {
+export const createDependenciesFilter = (level: LevelFilterOption, search: string = '') => {
   const conditions: Array<(dep: EnrichedDeprecationInfo) => boolean> = [];
 
-  if (level !== LevelFilterOption.all) {
+  if (level !== 'all') {
     conditions.push((dep: EnrichedDeprecationInfo) => dep.level === level);
   }
 
   if (search.length > 0) {
     conditions.push((dep) => {
       try {
+        // 'i' is used for case-insensitive matching
         const searchReg = new RegExp(search, 'i');
         return searchReg.test(dep.message);
       } catch (e) {
@@ -57,37 +58,29 @@ export const filterDeps = (level: LevelFilterOption, search: string = '') => {
   return (dep: EnrichedDeprecationInfo) => conditions.map((c) => c(dep)).every((t) => t);
 };
 
-/**
- * Collection of calculated fields based on props
- */
-const calcFields = {
-  filteredDeprecations(props: {
-    deprecations: EnrichedDeprecationInfo[];
-    currentFilter: LevelFilterOption;
-    search: string;
-  }) {
-    const { deprecations = [], currentFilter, search } = props;
-    return deprecations.filter(filterDeps(currentFilter, search));
-  },
-
-  groups(props: {
-    deprecations: EnrichedDeprecationInfo[];
-    currentFilter: LevelFilterOption;
-    search: string;
-    currentGroupBy: GroupByOption;
-  }) {
-    return groupBy(calcFields.filteredDeprecations(props), props.currentGroupBy);
-  },
-
-  numPages(props: {
-    deprecations: EnrichedDeprecationInfo[];
-    currentFilter: LevelFilterOption;
-    search: string;
-    currentGroupBy: GroupByOption;
-  }) {
-    return Math.ceil(Object.keys(calcFields.groups(props)).length / DEPRECATIONS_PER_PAGE);
-  },
-};
+const filterDeprecations = (
+  deprecations: EnrichedDeprecationInfo[] = [],
+  currentFilter: LevelFilterOption,
+  search: string
+) => deprecations.filter(createDependenciesFilter(currentFilter, search));
+
+const groupDeprecations = (
+  deprecations: EnrichedDeprecationInfo[],
+  currentFilter: LevelFilterOption,
+  search: string,
+  currentGroupBy: GroupByOption
+) => groupBy(filterDeprecations(deprecations, currentFilter, search), currentGroupBy);
+
+const getPageCount = (
+  deprecations: EnrichedDeprecationInfo[],
+  currentFilter: LevelFilterOption,
+  search: string,
+  currentGroupBy: GroupByOption
+) =>
+  Math.ceil(
+    Object.keys(groupDeprecations(deprecations, currentFilter, search, currentGroupBy)).length /
+      DEPRECATIONS_PER_PAGE
+  );
 
 /**
  * Displays a list of deprecations that are filterable and groupable. Can be used for cluster,
@@ -101,7 +94,7 @@ export const DeprecationTabContent: FunctionComponent<CheckupTabProps> = ({
   refreshCheckupData,
   navigateToOverviewPage,
 }) => {
-  const [currentFilter, setCurrentFilter] = useState<LevelFilterOption>(LevelFilterOption.all);
+  const [currentFilter, setCurrentFilter] = useState<LevelFilterOption>('all');
   const [search, setSearch] = useState<string>('');
   const [currentGroupBy, setCurrentGroupBy] = useState<GroupByOption>(GroupByOption.message);
   const [expandState, setExpandState] = useState({
@@ -124,12 +117,7 @@ export const DeprecationTabContent: FunctionComponent<CheckupTabProps> = ({
 
   useEffect(() => {
     if (deprecations) {
-      const pageCount = calcFields.numPages({
-        deprecations,
-        currentFilter,
-        search,
-        currentGroupBy,
-      });
+      const pageCount = getPageCount(deprecations, currentFilter, search, currentGroupBy);
 
       if (currentPage >= pageCount) {
         setCurrentPage(0);
@@ -154,23 +142,14 @@ export const DeprecationTabContent: FunctionComponent<CheckupTabProps> = ({
     content = <SectionLoading>{i18nTexts.isLoading}</SectionLoading>;
   } else if (deprecations?.length) {
     const levelGroups = groupBy(deprecations, 'level');
-    const deprecationLevelsCount = Object.keys(levelGroups).reduce((counts, level) => {
+    const levelToDeprecationCountMap = Object.keys(levelGroups).reduce((counts, level) => {
       counts[level] = levelGroups[level].length;
       return counts;
     }, {} as Record<string, number>);
 
-    const filteredDeprecations = calcFields.filteredDeprecations({
-      deprecations,
-      currentFilter,
-      search,
-    });
+    const filteredDeprecations = filterDeprecations(deprecations, currentFilter, search);
 
-    const groups = calcFields.groups({
-      deprecations,
-      currentFilter,
-      search,
-      currentGroupBy,
-    });
+    const groups = groupDeprecations(deprecations, currentFilter, search, currentGroupBy);
 
     content = (
       <div data-test-subj="deprecationsContainer">
@@ -182,7 +161,7 @@ export const DeprecationTabContent: FunctionComponent<CheckupTabProps> = ({
           onFilterChange={setCurrentFilter}
           onSearchChange={setSearch}
           totalDeprecationsCount={deprecations.length}
-          deprecationLevelsCount={deprecationLevelsCount}
+          levelToDeprecationCountMap={levelToDeprecationCountMap}
           groupByFilterProps={{
             availableGroupByOptions: getAvailableGroupByOptions(),
             currentGroupBy,
@@ -218,7 +197,6 @@ export const DeprecationTabContent: FunctionComponent<CheckupTabProps> = ({
                 />
                 <EuiHorizontalRule margin="s" />
               </div>,
-              ,
             ])}
 
           {/* Only show pagination if we have more than DEPRECATIONS_PER_PAGE. */}
@@ -227,12 +205,7 @@ export const DeprecationTabContent: FunctionComponent<CheckupTabProps> = ({
               <EuiSpacer />
 
               <DeprecationPagination
-                pageCount={calcFields.numPages({
-                  deprecations,
-                  currentFilter,
-                  search,
-                  currentGroupBy,
-                })}
+                pageCount={getPageCount(deprecations, currentFilter, search, currentGroupBy)}
                 activePage={currentPage}
                 setPage={setCurrentPage}
               />

diff --git a/...ins/upgrade_assistant/public/application/components/es_deprecations/deprecations/cell.tsx b/...ins/upgrade_assistant/public/application/components/es_deprecations/deprecations/cell.tsx
@@ -61,8 +61,8 @@ export const DeprecationCell: FunctionComponent<DeprecationCellProps> = ({
           </EuiTitle>
         )}
 
-        {items.map((item) => (
-          <EuiText key={item.title || item.body}>
+        {items.map((item, index) => (
+          <EuiText key={`deprecation-item-${index}`}>
             {item.title && <h6>{item.title}</h6>}
             <p>{item.body}</p>
           </EuiText>

diff --git a/.../upgrade_assistant/public/application/components/kibana_deprecations/deprecation_list.tsx b/.../upgrade_assistant/public/application/components/kibana_deprecations/deprecation_list.tsx
@@ -13,7 +13,7 @@ import type { DomainDeprecationDetails } from 'kibana/public';
 
 import { LevelFilterOption } from '../types';
 import { SearchBar, DeprecationListBar, DeprecationPagination } from '../shared';
-import { LEVEL_MAP, DEPRECATIONS_PER_PAGE } from '../constants';
+import { DEPRECATIONS_PER_PAGE } from '../constants';
 import { KibanaDeprecationAccordion } from './deprecation_item';
 import { StepsModalContent } from './steps_modal';
 import { KibanaDeprecationErrors } from './kibana_deprecation_errors';
@@ -28,7 +28,7 @@ interface Props {
 
 const getFilteredDeprecations = (
   deprecations: DomainDeprecationDetails[],
-  level: string,
+  level: LevelFilterOption,
   search: string
 ) => {
   return deprecations
@@ -38,6 +38,7 @@ const getFilteredDeprecations = (
     .filter((filteredDep) => {
       if (search.length > 0) {
         try {
+          // 'i' is used for case-insensitive matching
           const searchReg = new RegExp(search, 'i');
           return searchReg.test(filteredDep.message);
         } catch (e) {
@@ -49,18 +50,14 @@ const getFilteredDeprecations = (
     });
 };
 
-const sortByLevelDesc = (a: DomainDeprecationDetails, b: DomainDeprecationDetails) => {
-  return -1 * (LEVEL_MAP[a.level] - LEVEL_MAP[b.level]);
-};
-
 export const KibanaDeprecationList: FunctionComponent<Props> = ({
   deprecations,
   showStepsModal,
   showResolveModal,
   reloadDeprecations,
   isLoading,
 }) => {
-  const [currentFilter, setCurrentFilter] = useState<LevelFilterOption>(LevelFilterOption.all);
+  const [currentFilter, setCurrentFilter] = useState<LevelFilterOption>('all');
   const [search, setSearch] = useState('');
   const [expandState, setExpandState] = useState({
     forceExpand: false,
@@ -73,7 +70,7 @@ export const KibanaDeprecationList: FunctionComponent<Props> = ({
   };
 
   const levelGroups = groupBy(deprecations, 'level');
-  const deprecationLevelsCount = Object.keys(levelGroups).reduce((counts, level) => {
+  const levelToDeprecationCountMap = Object.keys(levelGroups).reduce((counts, level) => {
     counts[level] = levelGroups[level].length;
     return counts;
   }, {} as { [level: string]: number });
@@ -98,7 +95,7 @@ export const KibanaDeprecationList: FunctionComponent<Props> = ({
         onFilterChange={setCurrentFilter}
         onSearchChange={setSearch}
         totalDeprecationsCount={deprecations.length}
-        deprecationLevelsCount={deprecationLevelsCount}
+        levelToDeprecationCountMap={levelToDeprecationCountMap}
       />
 
       {deprecationsWithErrors.length > 0 && (
@@ -119,7 +116,6 @@ export const KibanaDeprecationList: FunctionComponent<Props> = ({
       <>
         {filteredDeprecations
           .slice(currentPage * DEPRECATIONS_PER_PAGE, (currentPage + 1) * DEPRECATIONS_PER_PAGE)
-          .sort(sortByLevelDesc)
           .map((deprecation, index) => [
             <div key={`kibana-deprecation-${index}`} data-test-subj="kibanaDeprecationItem">
               <KibanaDeprecationAccordion

diff --git a/...assistant/public/application/components/kibana_deprecations/kibana_deprecation_errors.tsx b/...assistant/public/application/components/kibana_deprecations/kibana_deprecation_errors.tsx
@@ -16,7 +16,7 @@ interface Props {
 const i18nTexts = {
   pluginError: i18n.translate('xpack.upgradeAssistant.kibanaDeprecationErrors.pluginErrorMessage', {
     defaultMessage:
-      'Not all Kibana deprecations were retrieved successfully. This list may be incomplete.',
+      'Not all Kibana deprecations were retrieved successfully. This list may be incomplete. Check the Kibana server logs for errors.',
   }),
   loadingError: i18n.translate(
     'xpack.upgradeAssistant.kibanaDeprecationErrors.loadingErrorMessage',

diff --git a/...grade_assistant/public/application/components/kibana_deprecations/kibana_deprecations.tsx b/...grade_assistant/public/application/components/kibana_deprecations/kibana_deprecations.tsx
@@ -26,6 +26,7 @@ import { KibanaDeprecationList } from './deprecation_list';
 import { StepsModal, StepsModalContent } from './steps_modal';
 import { KibanaDeprecationErrors } from './kibana_deprecation_errors';
 import { ResolveDeprecationModal } from './resolve_deprecation_modal';
+import { LEVEL_MAP } from '../constants';
 
 const i18nTexts = {
   pageTitle: i18n.translate('xpack.upgradeAssistant.kibanaDeprecations.pageTitle', {
@@ -51,6 +52,10 @@ const i18nTexts = {
   }),
 };
 
+const sortByLevelDesc = (a: DomainDeprecationDetails, b: DomainDeprecationDetails) => {
+  return -1 * (LEVEL_MAP[a.level] - LEVEL_MAP[b.level]);
+};
+
 export const KibanaDeprecationsContent = withRouter(({ history }: RouteComponentProps) => {
   const [kibanaDeprecations, setKibanaDeprecations] = useState<
     DomainDeprecationDetails[] | undefined
@@ -72,7 +77,8 @@ export const KibanaDeprecationsContent = withRouter(({ history }: RouteComponent
 
     try {
       const response = await deprecations.getAllDeprecations();
-      setKibanaDeprecations(response);
+      const sortedDeprecations = response.sort(sortByLevelDesc);
+      setKibanaDeprecations(sortedDeprecations);
     } catch (e) {
       setError(e);
     }
@@ -81,18 +87,10 @@ export const KibanaDeprecationsContent = withRouter(({ history }: RouteComponent
   }, [deprecations]);
 
   const toggleStepsModal = (newStepsModalContent?: StepsModalContent) => {
-    if (typeof newStepsModalContent === 'undefined') {
-      setStepsModalContent(undefined);
-    }
-
     setStepsModalContent(newStepsModalContent);
   };
 
   const toggleResolveModal = (newResolveModalContent?: DomainDeprecationDetails) => {
-    if (typeof newResolveModalContent === 'undefined') {
-      setResolveModalContent(undefined);
-    }
-
     setResolveModalContent(newResolveModalContent);
   };
 
@@ -115,7 +113,7 @@ export const KibanaDeprecationsContent = withRouter(({ history }: RouteComponent
 
     notifications.toasts.addSuccess(i18nTexts.successMessage);
     // Refetch deprecations
-    await getAllDeprecations();
+    getAllDeprecations();
   };
 
   useEffect(() => {

diff --git a/...s/upgrade_assistant/public/application/components/shared/search_bar/level_filter.test.tsx b/...s/upgrade_assistant/public/application/components/shared/search_bar/level_filter.test.tsx
@@ -7,16 +7,16 @@
 
 import { mount, shallow } from 'enzyme';
 import React from 'react';
-
 import { LevelFilterOption } from '../../types';
+
 import { DeprecationLevelFilter } from './level_filter';
 
 const defaultProps = {
   levelsCount: {
     warning: 4,
     critical: 1,
   },
-  currentFilter: LevelFilterOption.all,
+  currentFilter: 'all' as LevelFilterOption,
   onFilterChange: jest.fn(),
 };
 
@@ -29,6 +29,6 @@ describe('DeprecationLevelFilter', () => {
     const wrapper = mount(<DeprecationLevelFilter {...defaultProps} />);
     wrapper.find('button[data-test-subj="criticalLevelFilter"]').simulate('click');
     expect(defaultProps.onFilterChange).toHaveBeenCalledTimes(1);
-    expect(defaultProps.onFilterChange.mock.calls[0][0]).toEqual(LevelFilterOption.critical);
+    expect(defaultProps.onFilterChange.mock.calls[0][0]).toEqual('critical');
   });
 });
diff --git a/...lugins/upgrade_assistant/public/application/components/shared/search_bar/level_filter.tsx b/...lugins/upgrade_assistant/public/application/components/shared/search_bar/level_filter.tsx
@@ -41,19 +41,15 @@ export const DeprecationLevelFilter: React.FunctionComponent<DeprecationLevelPro
     <EuiFlexItem grow={false}>
       <EuiFilterGroup>
         <EuiFilterButton
-          key={LevelFilterOption.critical}
+          key="critical"
           onClick={() => {
-            onFilterChange(
-              currentFilter !== LevelFilterOption.critical
-                ? LevelFilterOption.critical
-                : LevelFilterOption.all
-            );
+            onFilterChange(currentFilter !== 'critical' ? 'critical' : 'all');
           }}
-          hasActiveFilters={currentFilter === LevelFilterOption.critical}
-          numFilters={levelsCount[LevelFilterOption.critical] || undefined}
+          hasActiveFilters={currentFilter === 'critical'}
+          numFilters={levelsCount.critical || undefined}
           data-test-subj="criticalLevelFilter"
         >
-          {LocalizedOptions[LevelFilterOption.critical]}
+          {LocalizedOptions.critical}
         </EuiFilterButton>
       </EuiFilterGroup>
     </EuiFlexItem>

diff --git a/.../plugins/upgrade_assistant/public/application/components/shared/search_bar/search_bar.tsx b/.../plugins/upgrade_assistant/public/application/components/shared/search_bar/search_bar.tsx
@@ -31,7 +31,7 @@ interface SearchBarProps {
   onFilterChange: (filter: LevelFilterOption) => void;
   onSearchChange: (filter: string) => void;
   totalDeprecationsCount: number;
-  deprecationLevelsCount: {
+  levelToDeprecationCountMap: {
     [key: string]: number;
   };
   groupByFilterProps?: {
@@ -67,7 +67,7 @@ const i18nTexts = {
 
 export const SearchBar: FunctionComponent<SearchBarProps> = ({
   totalDeprecationsCount,
-  deprecationLevelsCount,
+  levelToDeprecationCountMap,
   isLoading,
   loadData,
   currentFilter,
@@ -108,7 +108,7 @@ export const SearchBar: FunctionComponent<SearchBarProps> = ({
             <DeprecationLevelFilter
               {...{
                 totalDeprecationsCount,
-                levelsCount: deprecationLevelsCount,
+                levelsCount: levelToDeprecationCountMap,
                 currentFilter,
                 onFilterChange,
               }}