From df30b7eb500c4fd92abc58d1150b3434e1e54e40 Mon Sep 17 00:00:00 2001 From: Antonio Date: Mon, 27 Nov 2023 12:25:49 +0100 Subject: [PATCH 1/9] [Cases] Add scroll to columns popover. (#171912) ## Summary - Added vertical scroll to the column selection popover - Changed the fixed width of some columns in the cases table https://github.com/elastic/kibana/assets/1533137/08bccc40-792c-4bc0-8a4b-2a007b7257c2 --- .../components/all_cases/columns_popover.tsx | 7 +++- .../components/all_cases/use_actions.test.tsx | 1 + .../components/all_cases/use_actions.tsx | 1 + .../all_cases/use_cases_columns.test.tsx | 38 ++++++++++--------- .../all_cases/use_cases_columns.tsx | 8 ++-- 5 files changed, 33 insertions(+), 22 deletions(-) diff --git a/x-pack/plugins/cases/public/components/all_cases/columns_popover.tsx b/x-pack/plugins/cases/public/components/all_cases/columns_popover.tsx index e95afcc159e98..d16b1f20059a9 100644 --- a/x-pack/plugins/cases/public/components/all_cases/columns_popover.tsx +++ b/x-pack/plugins/cases/public/components/all_cases/columns_popover.tsx @@ -7,6 +7,7 @@ import type { ChangeEvent } from 'react'; import React, { useCallback, useMemo, useState } from 'react'; +import { css } from '@emotion/react'; import type { DropResult } from '@elastic/eui'; @@ -128,7 +129,11 @@ export const ColumnsPopover: React.FC = ({ diff --git a/x-pack/plugins/cases/public/components/all_cases/use_actions.test.tsx b/x-pack/plugins/cases/public/components/all_cases/use_actions.test.tsx index ecc9233fc327c..dcec2558aad46 100644 --- a/x-pack/plugins/cases/public/components/all_cases/use_actions.test.tsx +++ b/x-pack/plugins/cases/public/components/all_cases/use_actions.test.tsx @@ -43,6 +43,7 @@ describe('useActions', () => { "align": "right", "name": "Actions", "render": [Function], + "width": "100px", }, } `); diff --git a/x-pack/plugins/cases/public/components/all_cases/use_actions.tsx b/x-pack/plugins/cases/public/components/all_cases/use_actions.tsx index ea43f79b4954e..70a163bcd69a0 100644 --- a/x-pack/plugins/cases/public/components/all_cases/use_actions.tsx +++ b/x-pack/plugins/cases/public/components/all_cases/use_actions.tsx @@ -244,6 +244,7 @@ export const useActions = ({ disableActions }: UseBulkActionsProps): UseBulkActi ); }, + width: '100px', } : null, }; diff --git a/x-pack/plugins/cases/public/components/all_cases/use_cases_columns.test.tsx b/x-pack/plugins/cases/public/components/all_cases/use_cases_columns.test.tsx index b61e7548b089e..0577dcabeb67d 100644 --- a/x-pack/plugins/cases/public/components/all_cases/use_cases_columns.test.tsx +++ b/x-pack/plugins/cases/public/components/all_cases/use_cases_columns.test.tsx @@ -90,13 +90,12 @@ describe('useCasesColumns ', () => { "field": "assignees", "name": "Assignees", "render": [Function], - "width": "180px", }, Object { "field": "tags", "name": "Tags", "render": [Function], - "width": "15%", + "width": "12%", }, Object { "align": "right", @@ -110,13 +109,14 @@ describe('useCasesColumns ', () => { "field": "totalComment", "name": "Comments", "render": [Function], + "width": "90px", }, Object { "field": "category", "name": "Category", "render": [Function], "sortable": true, - "width": "100px", + "width": "120px", }, Object { "field": "createdAt", @@ -139,13 +139,13 @@ describe('useCasesColumns ', () => { Object { "name": "External incident", "render": [Function], - "width": undefined, }, Object { "field": "status", "name": "Status", "render": [Function], "sortable": true, + "width": "110px", }, Object { "field": "severity", @@ -158,6 +158,7 @@ describe('useCasesColumns ', () => { "align": "right", "name": "Actions", "render": [Function], + "width": "100px", }, ], "isLoadingColumns": false, @@ -190,13 +191,12 @@ describe('useCasesColumns ', () => { "field": "assignees", "name": "Assignees", "render": [Function], - "width": "180px", }, Object { "field": "tags", "name": "Tags", "render": [Function], - "width": "15%", + "width": "12%", }, Object { "align": "right", @@ -210,13 +210,14 @@ describe('useCasesColumns ', () => { "field": "totalComment", "name": "Comments", "render": [Function], + "width": "90px", }, Object { "field": "category", "name": "Category", "render": [Function], "sortable": true, - "width": "100px", + "width": "120px", }, Object { "field": "createdAt", @@ -233,13 +234,13 @@ describe('useCasesColumns ', () => { Object { "name": "External incident", "render": [Function], - "width": undefined, }, Object { "field": "status", "name": "Status", "render": [Function], "sortable": true, + "width": "110px", }, Object { "field": "severity", @@ -252,6 +253,7 @@ describe('useCasesColumns ', () => { "align": "right", "name": "Actions", "render": [Function], + "width": "100px", }, ], "isLoadingColumns": false, @@ -288,7 +290,7 @@ describe('useCasesColumns ', () => { "name": "Category", "render": [Function], "sortable": true, - "width": "100px", + "width": "120px", }, Object { "field": "createdAt", @@ -336,7 +338,7 @@ describe('useCasesColumns ', () => { "name": "Category", "render": [Function], "sortable": true, - "width": "100px", + "width": "120px", }, Object { "field": "createdAt", @@ -384,7 +386,7 @@ describe('useCasesColumns ', () => { "name": "Category", "render": [Function], "sortable": true, - "width": "100px", + "width": "120px", }, Object { "field": "createdAt", @@ -430,7 +432,7 @@ describe('useCasesColumns ', () => { "field": "tags", "name": "Tags", "render": [Function], - "width": "15%", + "width": "12%", }, Object { "align": "right", @@ -444,13 +446,14 @@ describe('useCasesColumns ', () => { "field": "totalComment", "name": "Comments", "render": [Function], + "width": "90px", }, Object { "field": "category", "name": "Category", "render": [Function], "sortable": true, - "width": "100px", + "width": "120px", }, Object { "field": "createdAt", @@ -467,13 +470,13 @@ describe('useCasesColumns ', () => { Object { "name": "External incident", "render": [Function], - "width": undefined, }, Object { "field": "status", "name": "Status", "render": [Function], "sortable": true, + "width": "110px", }, Object { "field": "severity", @@ -536,7 +539,7 @@ describe('useCasesColumns ', () => { "field": "tags", "name": "Tags", "render": [Function], - "width": "15%", + "width": "12%", }, Object { "align": "right", @@ -550,13 +553,14 @@ describe('useCasesColumns ', () => { "field": "totalComment", "name": "Comments", "render": [Function], + "width": "90px", }, Object { "field": "category", "name": "Category", "render": [Function], "sortable": true, - "width": "100px", + "width": "120px", }, Object { "field": "createdAt", @@ -573,13 +577,13 @@ describe('useCasesColumns ', () => { Object { "name": "External incident", "render": [Function], - "width": undefined, }, Object { "field": "status", "name": "Status", "render": [Function], "sortable": true, + "width": "110px", }, Object { "field": "severity", diff --git a/x-pack/plugins/cases/public/components/all_cases/use_cases_columns.tsx b/x-pack/plugins/cases/public/components/all_cases/use_cases_columns.tsx index 6ef5dcae9fe6b..de053bfb27fab 100644 --- a/x-pack/plugins/cases/public/components/all_cases/use_cases_columns.tsx +++ b/x-pack/plugins/cases/public/components/all_cases/use_cases_columns.tsx @@ -137,7 +137,6 @@ export const useCasesColumns = ({ render: (assignees: CaseUI['assignees']) => ( ), - width: '180px', }, tags: { field: casesColumnsConfig.tags.field, @@ -184,7 +183,7 @@ export const useCasesColumns = ({ } return getEmptyCellValue(); }, - width: '15%', + width: '12%', }, totalAlerts: { field: casesColumnsConfig.totalAlerts.field, @@ -204,6 +203,7 @@ export const useCasesColumns = ({ totalComment != null ? renderStringField(`${totalComment}`, `case-table-column-commentCount`) : getEmptyCellValue(), + width: '90px', }, category: { field: casesColumnsConfig.category.field, @@ -217,7 +217,7 @@ export const useCasesColumns = ({ } return getEmptyCellValue(); }, - width: '100px', + width: '120px', }, closedAt: { field: casesColumnsConfig.closedAt.field, @@ -273,7 +273,6 @@ export const useCasesColumns = ({ } return getEmptyCellValue(); }, - width: isSelectorView ? '80px' : undefined, }, status: { field: casesColumnsConfig.status.field, @@ -286,6 +285,7 @@ export const useCasesColumns = ({ return getEmptyCellValue(); }, + width: '110px', }, severity: { field: casesColumnsConfig.severity.field, From 619b8b2f1e5f0fa50307d58809e106a501db8628 Mon Sep 17 00:00:00 2001 From: Shahzad Date: Mon, 27 Nov 2023 12:30:51 +0100 Subject: [PATCH 2/9] [skip-ci] [OBS UX MG] Add team path label for changes (#171904) --- .github/paths-labeller.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/paths-labeller.yml b/.github/paths-labeller.yml index 1c1b2742001b5..4f4057935265a 100644 --- a/.github/paths-labeller.yml +++ b/.github/paths-labeller.yml @@ -21,3 +21,5 @@ - "x-pack/plugins/synthetics/**/*.*" - "x-pack/plugins/ux/**/*.*" - "x-pack/plugins/observability/public/components/shared/exploratory_view/**/*.*" + - "Team:obs-ux-management": + - "x-pack/plugins/observability/**/*.*" From 0942bcea0431053c7346a436aef6cf6cfc4e836f Mon Sep 17 00:00:00 2001 From: Maryam Saeidi Date: Mon, 27 Nov 2023 13:11:57 +0100 Subject: [PATCH 3/9] [Custom threshold] Fix not showing threshold line in lens preview (#171970) Closes #171843 ## Summary This PR fixes not showing the threshold line in the following case: --- .../preview_chart/preview_chart.test.tsx | 17 ++++++++++++++++- .../components/preview_chart/preview_chart.tsx | 5 ++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/observability/public/components/custom_threshold/components/preview_chart/preview_chart.test.tsx b/x-pack/plugins/observability/public/components/custom_threshold/components/preview_chart/preview_chart.test.tsx index 3adc5975e91a8..bc478e54135ba 100644 --- a/x-pack/plugins/observability/public/components/custom_threshold/components/preview_chart/preview_chart.test.tsx +++ b/x-pack/plugins/observability/public/components/custom_threshold/components/preview_chart/preview_chart.test.tsx @@ -13,7 +13,7 @@ import { Comparator, Aggregators } from '../../../../../common/custom_threshold_ import { useKibana } from '../../../../utils/kibana_react'; import { kibanaStartMock } from '../../../../utils/kibana_react.mock'; import { MetricExpression } from '../../types'; -import { PreviewChart } from './preview_chart'; +import { getBufferThreshold, PreviewChart } from './preview_chart'; jest.mock('../../../../utils/kibana_react'); @@ -70,3 +70,18 @@ describe('Preview chart', () => { expect(wrapper.find('[data-test-subj="thresholdRuleNoChartData"]').exists()).toBeTruthy(); }); }); + +describe('getBufferThreshold', () => { + const testData = [ + { threshold: undefined, buffer: '0.00' }, + { threshold: 0.1, buffer: '0.12' }, + { threshold: 0.01, buffer: '0.02' }, + { threshold: 0.001, buffer: '0.01' }, + { threshold: 0.00098, buffer: '0.01' }, + { threshold: 130, buffer: '143.00' }, + ]; + + it.each(testData)('getBufferThreshold($threshold) = $buffer', ({ threshold, buffer }) => { + expect(getBufferThreshold(threshold)).toBe(buffer); + }); +}); diff --git a/x-pack/plugins/observability/public/components/custom_threshold/components/preview_chart/preview_chart.tsx b/x-pack/plugins/observability/public/components/custom_threshold/components/preview_chart/preview_chart.tsx index 23d27a554a77c..1c25d22b3a595 100644 --- a/x-pack/plugins/observability/public/components/custom_threshold/components/preview_chart/preview_chart.tsx +++ b/x-pack/plugins/observability/public/components/custom_threshold/components/preview_chart/preview_chart.tsx @@ -44,6 +44,9 @@ const getOperationTypeFromRuleAggType = (aggType: AggType): OperationType => { return aggType; }; +export const getBufferThreshold = (threshold?: number): string => + (Math.ceil((threshold || 0) * 1.1 * 100) / 100).toFixed(2).toString(); + export function PreviewChart({ metricExpression, dataView, @@ -147,7 +150,7 @@ export function PreviewChart({ const bufferRefLine = new XYReferenceLinesLayer({ data: [ { - value: Math.round((threshold[0] || 0) * 1.1).toString(), + value: getBufferThreshold(threshold[0]), color: 'transparent', fill, format, From 037f68852b207bd428715bce5f8d9a599c541ee8 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Mon, 27 Nov 2023 14:30:32 +0200 Subject: [PATCH 4/9] [ES|QL] Allows searching in the documentation description (#171916) ## Summary Allows searching on the ES|QL reference markdown. This means that now the search will return more results. Examples: - If I search for keep it will return all the occurences of the word keep so the user will see the keep command but also all the other commands that the keep word is used in the examples. I think that this is very useful as the user can see more than 1 examples of a command - If I search for date it will return not only the commands that have the word date but also the commands that allow date in their arguments - As now it searches also to the description it can also return false positive results. I think is an accepted drawback. image Note: I am not allowing this for Lens formulas. I introduced a new property to disable it. The implementation works for formulas too but we haven't received any negative feedback so far so I would like to test it in the ES|QL reference first. ### Checklist - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- .../components/documentation_content.test.tsx | 28 ++++++++++++-- .../src/components/documentation_content.tsx | 13 ++++++- .../src/components/documentation_popover.tsx | 14 ++++++- .../src/utils/element_to_string.test.tsx | 37 +++++++++++++++++++ .../src/utils/element_to_string.ts | 35 ++++++++++++++++++ .../tsconfig.json | 1 + .../src/text_based_languages_editor.tsx | 2 + 7 files changed, 123 insertions(+), 7 deletions(-) create mode 100644 packages/kbn-language-documentation-popover/src/utils/element_to_string.test.tsx create mode 100644 packages/kbn-language-documentation-popover/src/utils/element_to_string.ts diff --git a/packages/kbn-language-documentation-popover/src/components/documentation_content.test.tsx b/packages/kbn-language-documentation-popover/src/components/documentation_content.test.tsx index 6d91cc403795e..e0d7e3c28dbea 100644 --- a/packages/kbn-language-documentation-popover/src/components/documentation_content.test.tsx +++ b/packages/kbn-language-documentation-popover/src/components/documentation_content.test.tsx @@ -9,6 +9,7 @@ import React from 'react'; import { mountWithIntl, findTestSubject } from '@kbn/test-jest-helpers'; import { act } from 'react-dom/test-utils'; +import { Markdown } from '@kbn/kibana-react-plugin/public'; import { LanguageDocumentationPopoverContent } from './documentation_content'; describe('###Documentation popover content', () => { @@ -24,11 +25,11 @@ describe('###Documentation popover content', () => { items: [ { label: 'Section two item 1', - description: Section 2 item 1 description, + description: , }, { label: 'Section two item 2', - description: Section 2 item 2 description, + description: , }, ], }, @@ -52,7 +53,7 @@ describe('###Documentation popover content', () => { }); }); - test('Documentation component should list all sections that match the search input', () => { + test('Documentation component should list all sections that match the search input when title matches', () => { const component = mountWithIntl( ); @@ -69,4 +70,25 @@ describe('###Documentation popover content', () => { expect(sectionsLabels.length).toBe(1); expect(sectionsLabels.text()).toEqual('Section one'); }); + + test('Documentation component should list all sections that match the search input when description matches', () => { + const component = mountWithIntl( + + ); + const searchBox = component.find('[data-test-subj="language-documentation-navigation-search"]'); + act(() => { + searchBox.at(0).prop('onChange')!({ + target: { value: 'item 2 description' }, + } as React.ChangeEvent); + }); + + component.update(); + + const sectionsLabels = findTestSubject(component, 'language-documentation-navigation-title'); + expect(sectionsLabels.length).toBe(1); + }); }); diff --git a/packages/kbn-language-documentation-popover/src/components/documentation_content.tsx b/packages/kbn-language-documentation-popover/src/components/documentation_content.tsx index b7c2e800bbaf5..0f24e233a4f28 100644 --- a/packages/kbn-language-documentation-popover/src/components/documentation_content.tsx +++ b/packages/kbn-language-documentation-popover/src/components/documentation_content.tsx @@ -20,6 +20,7 @@ import { EuiHighlight, EuiSpacer, } from '@elastic/eui'; +import { elementToString } from '../utils/element_to_string'; import './documentation.scss'; @@ -35,9 +36,11 @@ export interface LanguageDocumentationSections { interface DocumentationProps { language: string; sections?: LanguageDocumentationSections; + // if sets to true, allows searching in the markdown description + searchInDescription?: boolean; } -function DocumentationContent({ language, sections }: DocumentationProps) { +function DocumentationContent({ language, sections, searchInDescription }: DocumentationProps) { const [selectedSection, setSelectedSection] = useState(); const scrollTargets = useRef>({}); @@ -55,7 +58,13 @@ function DocumentationContent({ language, sections }: DocumentationProps) { .map((group) => { const items = group.items.filter((helpItem) => { return ( - !normalizedSearchText || helpItem.label.toLocaleLowerCase().includes(normalizedSearchText) + !normalizedSearchText || + helpItem.label.toLocaleLowerCase().includes(normalizedSearchText) || + // Converting the JSX element to a string first + (searchInDescription && + elementToString(helpItem.description) + ?.toLocaleLowerCase() + .includes(normalizedSearchText)) ); }); return { ...group, items }; diff --git a/packages/kbn-language-documentation-popover/src/components/documentation_popover.tsx b/packages/kbn-language-documentation-popover/src/components/documentation_popover.tsx index 8ff16737337aa..db66d69d7173f 100644 --- a/packages/kbn-language-documentation-popover/src/components/documentation_popover.tsx +++ b/packages/kbn-language-documentation-popover/src/components/documentation_popover.tsx @@ -17,9 +17,15 @@ interface DocumentationPopoverProps { language: string; sections?: LanguageDocumentationSections; buttonProps?: Omit; + searchInDescription?: boolean; } -function DocumentationPopover({ language, sections, buttonProps }: DocumentationPopoverProps) { +function DocumentationPopover({ + language, + sections, + buttonProps, + searchInDescription, +}: DocumentationPopoverProps) { const [isHelpOpen, setIsHelpOpen] = useState(false); const toggleDocumentationPopover = useCallback(() => { @@ -50,7 +56,11 @@ function DocumentationPopover({ language, sections, buttonProps }: Documentation } > - + ); } diff --git a/packages/kbn-language-documentation-popover/src/utils/element_to_string.test.tsx b/packages/kbn-language-documentation-popover/src/utils/element_to_string.test.tsx new file mode 100644 index 0000000000000..42ca61cca472b --- /dev/null +++ b/packages/kbn-language-documentation-popover/src/utils/element_to_string.test.tsx @@ -0,0 +1,37 @@ +/* + * 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 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ +import React from 'react'; +import { Markdown } from '@kbn/kibana-react-plugin/public'; +import { elementToString } from './element_to_string'; + +describe('elementToString', () => { + test('Should return empty string if no element is given', () => { + const text = elementToString(undefined); + expect(text).toEqual(''); + }); + + test('Should return empty string if no markdown is passed', () => { + const text = elementToString(Meow); + expect(text).toEqual(''); + }); + + test('Should convert to string if markdown is passed', () => { + const text = elementToString(); + expect(text).toEqual('## Markdown goes here '); + }); + + test('Should convert to string if children with markdown are passed', () => { + const text = elementToString( + <> +

Meow

+ + + ); + expect(text).toEqual('## Markdown goes here '); + }); +}); diff --git a/packages/kbn-language-documentation-popover/src/utils/element_to_string.ts b/packages/kbn-language-documentation-popover/src/utils/element_to_string.ts new file mode 100644 index 0000000000000..f13bb652f662a --- /dev/null +++ b/packages/kbn-language-documentation-popover/src/utils/element_to_string.ts @@ -0,0 +1,35 @@ +/* + * 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 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ +import React, { isValidElement } from 'react'; + +function nonNullable(v: T): v is NonNullable { + return v != null; +} + +/** + * Gets the JSX.Element as the input. It returns the markdown as string. + * If the children are not markdown it will return an empty string. + */ +export function elementToString(element?: JSX.Element): string { + if (!element) { + return ''; + } + const props = element.props; + if (props && 'markdown' in props) { + return String(props.markdown); + } else if (props && 'children' in props && Array.isArray(props.children)) { + return props.children.reduce((text: string, child: React.ReactNode): string => { + const validChildren = React.Children.toArray(child).filter(nonNullable); + if (isValidElement(child) && validChildren.length > 0) { + return text.concat(elementToString(child)); + } + return text; + }, ''); + } + return ''; +} diff --git a/packages/kbn-language-documentation-popover/tsconfig.json b/packages/kbn-language-documentation-popover/tsconfig.json index 82710b41d10b4..a0c043b8a15e5 100644 --- a/packages/kbn-language-documentation-popover/tsconfig.json +++ b/packages/kbn-language-documentation-popover/tsconfig.json @@ -14,6 +14,7 @@ "kbn_references": [ "@kbn/i18n", "@kbn/test-jest-helpers", + "@kbn/kibana-react-plugin", ], "exclude": [ "target/**/*", diff --git a/packages/kbn-text-based-editor/src/text_based_languages_editor.tsx b/packages/kbn-text-based-editor/src/text_based_languages_editor.tsx index a6cdea64704dc..9a5eae37ed097 100644 --- a/packages/kbn-text-based-editor/src/text_based_languages_editor.tsx +++ b/packages/kbn-text-based-editor/src/text_based_languages_editor.tsx @@ -621,6 +621,7 @@ export const TextBasedLanguagesEditor = memo(function TextBasedLanguagesEditor({ Date: Mon, 27 Nov 2023 12:39:12 +0000 Subject: [PATCH 5/9] [Project navigation] Improve performance (#171662) --- .../project_navigation_service.test.ts | 26 +- .../src/project_navigation/utils.test.ts | 123 +++--- .../src/project_navigation/utils.ts | 1 - .../src/project_navigation.ts | 7 +- .../project_navigation.test.tsx.snap | 351 ++++------------- .../navigation/__jest__/active_node.test.tsx | 35 +- .../__jest__/build_nav_tree.test.tsx | 150 ++------ .../chrome/navigation/__jest__/links.test.tsx | 15 +- .../chrome/navigation/__jest__/panel.test.tsx | 9 +- .../__jest__/project_navigation.test.tsx | 17 +- .../chrome/navigation/mocks/src/jest.ts | 15 +- .../chrome/navigation/mocks/src/storybook.ts | 4 +- .../chrome/navigation/src/cloud_links.tsx | 8 +- .../chrome/navigation/src/navnode_utils.ts | 171 +++++++++ .../chrome/navigation/src/services.tsx | 27 +- .../src/ui/components/navigation.tsx | 130 ++++--- .../src/ui/components/navigation_footer.tsx | 21 +- .../src/ui/components/navigation_group.tsx | 199 ++++++---- .../src/ui/components/navigation_item.tsx | 120 +++--- .../components/navigation_item_open_panel.tsx | 16 +- .../ui/components/navigation_section_ui.tsx | 172 ++++++--- .../src/ui/components/panel/context.tsx | 3 +- .../ui/components/panel/default_content.tsx | 2 +- .../ui/components/panel/navigation_panel.tsx | 4 +- .../navigation/src/ui/default_navigation.tsx | 81 +--- .../chrome/navigation/src/ui/hooks/index.ts | 9 - .../src/ui/hooks/use_init_navnode.ts | 356 ------------------ .../src/ui/hooks/use_register_tree_node.ts | 29 -- .../navigation/src/ui/navigation.stories.tsx | 329 +++++----------- .../chrome/navigation/src/ui/types.ts | 41 +- .../shared-ux/chrome/navigation/src/utils.ts | 50 ++- .../chrome/navigation/types/index.ts | 2 +- .../chrome_navigation_tree.test.ts | 22 +- .../navigation_tree/chrome_navigation_tree.ts | 8 +- .../project_navigation/project_navigation.tsx | 5 +- .../services/ml/observability_navigation.ts | 4 +- 36 files changed, 1002 insertions(+), 1560 deletions(-) create mode 100644 packages/shared-ux/chrome/navigation/src/navnode_utils.ts delete mode 100644 packages/shared-ux/chrome/navigation/src/ui/hooks/index.ts delete mode 100644 packages/shared-ux/chrome/navigation/src/ui/hooks/use_init_navnode.ts delete mode 100644 packages/shared-ux/chrome/navigation/src/ui/hooks/use_register_tree_node.ts diff --git a/packages/core/chrome/core-chrome-browser-internal/src/project_navigation/project_navigation_service.test.ts b/packages/core/chrome/core-chrome-browser-internal/src/project_navigation/project_navigation_service.test.ts index fded446938e75..473ec16369266 100644 --- a/packages/core/chrome/core-chrome-browser-internal/src/project_navigation/project_navigation_service.test.ts +++ b/packages/core/chrome/core-chrome-browser-internal/src/project_navigation/project_navigation_service.test.ts @@ -44,18 +44,18 @@ describe('breadcrumbs', () => { { id: 'root', title: 'Root', - path: ['root'], + path: 'root', breadcrumbStatus: 'hidden' as 'hidden', children: [ { id: 'subNav', - path: ['root', 'subNav'], + path: 'root.subNav', title: '', // intentionally empty to skip rendering children: [ { id: 'navItem1', title: 'Nav Item 1', - path: ['root', 'subNav', 'navItem1'], + path: 'root.subNav.navItem1', deepLink: { id: 'navItem1', title: 'Nav Item 1', @@ -318,12 +318,12 @@ describe('getActiveNodes$()', () => { { id: 'root', title: 'Root', - path: ['root'], + path: 'root', children: [ { id: 'item1', title: 'Item 1', - path: ['root', 'item1'], + path: 'root.item1', deepLink: { id: 'item1', title: 'Item 1', @@ -344,14 +344,12 @@ describe('getActiveNodes$()', () => { { id: 'root', title: 'Root', - isActive: true, - path: ['root'], + path: 'root', }, { id: 'item1', title: 'Item 1', - isActive: true, - path: ['root', 'item1'], + path: 'root.item1', deepLink: { id: 'item1', title: 'Item 1', @@ -375,12 +373,12 @@ describe('getActiveNodes$()', () => { { id: 'root', title: 'Root', - path: ['root'], + path: 'root', children: [ { id: 'item1', title: 'Item 1', - path: ['root', 'item1'], + path: 'root.item1', getIsActive: () => true, }, ], @@ -395,14 +393,12 @@ describe('getActiveNodes$()', () => { { id: 'root', title: 'Root', - isActive: true, - path: ['root'], + path: 'root', }, { id: 'item1', title: 'Item 1', - isActive: true, - path: ['root', 'item1'], + path: 'root.item1', getIsActive: expect.any(Function), }, ], diff --git a/packages/core/chrome/core-chrome-browser-internal/src/project_navigation/utils.test.ts b/packages/core/chrome/core-chrome-browser-internal/src/project_navigation/utils.test.ts index 93abfd5d5a1f7..a207162e060cb 100644 --- a/packages/core/chrome/core-chrome-browser-internal/src/project_navigation/utils.test.ts +++ b/packages/core/chrome/core-chrome-browser-internal/src/project_navigation/utils.test.ts @@ -23,27 +23,27 @@ describe('flattenNav', () => { { id: 'root', title: 'Root', - path: ['root'], + path: 'root', children: [ { id: 'item1', title: 'Item 1', - path: ['root', 'item1'], + path: 'root.item1', }, { id: 'item2', title: 'Item 2', - path: ['root', 'item2'], + path: 'root.item2', }, { id: 'group1', title: 'Group 1', - path: ['root', 'group1'], + path: 'root.group1', children: [ { id: 'item3', title: 'Item 3', - path: ['root', 'group1', 'item3'], + path: 'root.group1.item3', }, ], }, @@ -55,27 +55,27 @@ describe('flattenNav', () => { '[0]': { id: 'root', title: 'Root', - path: ['root'], + path: 'root', }, '[0][0]': { id: 'item1', title: 'Item 1', - path: ['root', 'item1'], + path: 'root.item1', }, '[0][1]': { id: 'item2', title: 'Item 2', - path: ['root', 'item2'], + path: 'root.item2', }, '[0][2]': { id: 'group1', title: 'Group 1', - path: ['root', 'group1'], + path: 'root.group1', }, '[0][2][0]': { id: 'item3', title: 'Item 3', - path: ['root', 'group1', 'item3'], + path: 'root.group1.item3', }, }; @@ -89,18 +89,18 @@ describe('findActiveNodes', () => { '[0]': { id: 'root', title: 'Root', - path: ['root'], + path: 'root', }, '[0][0]': { id: 'group1', title: 'Group 1', - path: ['root', 'group1'], + path: 'root.group1', }, '[0][0][0]': { id: 'item1', title: 'Item 1', deepLink: getDeepLink('item1', 'item1'), - path: ['root', 'group1', 'item1'], + path: 'root.group1.item1', }, }; @@ -109,21 +109,18 @@ describe('findActiveNodes', () => { { id: 'root', title: 'Root', - isActive: true, - path: ['root'], + path: 'root', }, { id: 'group1', title: 'Group 1', - isActive: true, - path: ['root', 'group1'], + path: 'root.group1', }, { id: 'item1', title: 'Item 1', - isActive: true, deepLink: getDeepLink('item1', 'item1'), - path: ['root', 'group1', 'item1'], + path: 'root.group1.item1', }, ], ]); @@ -134,35 +131,35 @@ describe('findActiveNodes', () => { '[0]': { id: 'root', title: 'Root', - path: ['root'], + path: 'root', }, '[0][0]': { id: 'group1', title: 'Group 1', deepLink: getDeepLink('group1', 'group1'), - path: ['root', 'group1'], + path: 'root.group1', }, '[0][0][0]': { id: 'group1A', title: 'Group 1A', - path: ['root', 'group1', 'group1A'], + path: 'root.group1.group1A', }, '[0][0][0][0]': { id: 'item1', title: 'Item 1', deepLink: getDeepLink('item1', 'item1'), - path: ['root', 'group1', 'group1A', 'item1'], + path: 'root.group1.group1A.item1', }, '[0][1]': { id: 'group2', title: 'Group 2', - path: ['root', 'group2'], + path: 'root.group2', }, '[0][1][0]': { id: 'item2', title: 'Item 2', deepLink: getDeepLink('item1', 'item1'), // Same link as above, should match both - path: ['root', 'group2', 'item2'], + path: 'root.group2.item2', }, }; @@ -172,49 +169,42 @@ describe('findActiveNodes', () => { { id: 'root', title: 'Root', - isActive: true, - path: ['root'], + path: 'root', }, { id: 'group1', title: 'Group 1', - isActive: true, deepLink: getDeepLink('group1', 'group1'), - path: ['root', 'group1'], + path: 'root.group1', }, { id: 'group1A', title: 'Group 1A', - isActive: true, - path: ['root', 'group1', 'group1A'], + path: 'root.group1.group1A', }, { id: 'item1', title: 'Item 1', - isActive: true, deepLink: getDeepLink('item1', 'item1'), - path: ['root', 'group1', 'group1A', 'item1'], + path: 'root.group1.group1A.item1', }, ], [ { id: 'root', title: 'Root', - isActive: true, - path: ['root'], + path: 'root', }, { id: 'group2', title: 'Group 2', - isActive: true, - path: ['root', 'group2'], + path: 'root.group2', }, { id: 'item2', title: 'Item 2', - isActive: true, deepLink: getDeepLink('item1', 'item1'), - path: ['root', 'group2', 'item2'], + path: 'root.group2.item2', }, ], ]); @@ -225,13 +215,13 @@ describe('findActiveNodes', () => { '[0]': { id: 'root', title: 'Root', - path: ['root'], + path: 'root', }, '[0][1]': { id: 'item1', title: 'Item 1', deepLink: getDeepLink('item1', `item1#/foo/bar`), - path: ['root', 'item1'], + path: 'root.item1', }, }; @@ -240,15 +230,13 @@ describe('findActiveNodes', () => { { id: 'root', title: 'Root', - isActive: true, - path: ['root'], + path: 'root', }, { id: 'item1', title: 'Item 1', - isActive: true, deepLink: getDeepLink('item1', `item1#/foo/bar`), - path: ['root', 'item1'], + path: 'root.item1', }, ], ]); @@ -260,7 +248,7 @@ describe('findActiveNodes', () => { id: 'root', title: 'Root', deepLink: getDeepLink('root', `root`), - path: ['root'], + path: 'root', }, }; @@ -269,9 +257,8 @@ describe('findActiveNodes', () => { { id: 'root', title: 'Root', - isActive: true, deepLink: getDeepLink('root', `root`), - path: ['root'], + path: 'root', }, ], ]); @@ -282,19 +269,19 @@ describe('findActiveNodes', () => { '[0]': { id: 'root', title: 'Root', - path: ['root'], + path: 'root', }, '[0][1]': { id: 'item1', title: 'Item 1', deepLink: getDeepLink('item1', `item1#/foo`), - path: ['root', 'item1'], + path: 'root.item1', }, '[0][2]': { id: 'item2', title: 'Item 2', deepLink: getDeepLink('item2', `item1#/foo/bar`), // Should match this one - path: ['root', 'item2'], + path: 'root.item2', }, }; @@ -303,15 +290,13 @@ describe('findActiveNodes', () => { { id: 'root', title: 'Root', - isActive: true, - path: ['root'], + path: 'root', }, { id: 'item2', title: 'Item 2', - isActive: true, deepLink: getDeepLink('item2', `item1#/foo/bar`), - path: ['root', 'item2'], + path: 'root.item2', }, ], ]); @@ -322,13 +307,13 @@ describe('findActiveNodes', () => { '[0]': { id: 'root', title: 'Root', - path: ['root'], + path: 'root', }, '[0][1]': { id: 'item1', title: 'Item 1', deepLink: getDeepLink('item1', `appRoot`), - path: ['root', 'item1'], + path: 'root.item1', }, }; @@ -337,15 +322,13 @@ describe('findActiveNodes', () => { { id: 'root', title: 'Root', - isActive: true, - path: ['root'], + path: 'root', }, { id: 'item1', title: 'Item 1', - isActive: true, deepLink: getDeepLink('item1', `appRoot`), - path: ['root', 'item1'], + path: 'root.item1', }, ], ]; @@ -362,19 +345,19 @@ describe('findActiveNodes', () => { '[0]': { id: 'root', title: 'Root', - path: ['root'], + path: 'root', }, '[0][1]': { id: 'item1', title: 'Item 1', - path: ['root', 'item1'], + path: 'root.item1', getIsActive: ({ location }) => location.pathname.startsWith('/foo'), // Should match }, '[0][2]': { id: 'item2', title: 'Item 2', deepLink: getDeepLink('item2', 'item2'), // Should match - path: ['root', 'item2'], + path: 'root.item2', }, }; @@ -393,30 +376,26 @@ describe('findActiveNodes', () => { { id: 'root', title: 'Root', - isActive: true, - path: ['root'], + path: 'root', }, { id: 'item1', title: 'Item 1', - isActive: true, getIsActive: expect.any(Function), - path: ['root', 'item1'], + path: 'root.item1', }, ], [ { id: 'root', title: 'Root', - isActive: true, - path: ['root'], + path: 'root', }, { id: 'item2', title: 'Item 2', - isActive: true, deepLink: getDeepLink('item2', 'item2'), - path: ['root', 'item2'], + path: 'root.item2', }, ], ]); diff --git a/packages/core/chrome/core-chrome-browser-internal/src/project_navigation/utils.ts b/packages/core/chrome/core-chrome-browser-internal/src/project_navigation/utils.ts index 48158025414cb..63f7f8e612c2e 100644 --- a/packages/core/chrome/core-chrome-browser-internal/src/project_navigation/utils.ts +++ b/packages/core/chrome/core-chrome-browser-internal/src/project_navigation/utils.ts @@ -114,7 +114,6 @@ export const findActiveNodes = ( const activeNodeFromKey = (key: string): ChromeProjectNavigationNode => ({ ...navTree[key], - isActive: true, }); Object.entries(navTree).forEach(([key, node]) => { diff --git a/packages/core/chrome/core-chrome-browser/src/project_navigation.ts b/packages/core/chrome/core-chrome-browser/src/project_navigation.ts index b879cf6f716d7..fe7c7bfc7c187 100644 --- a/packages/core/chrome/core-chrome-browser/src/project_navigation.ts +++ b/packages/core/chrome/core-chrome-browser/src/project_navigation.ts @@ -169,7 +169,7 @@ export interface ChromeProjectNavigationNode extends NodeDefinitionBase { /** Optional title. If not provided and a "link" is provided the title will be the Deep link title */ title: string; /** Path in the tree of the node */ - path: string[]; + path: string; /** App id or deeplink id */ deepLink?: ChromeNavLink; /** @@ -178,9 +178,10 @@ export interface ChromeProjectNavigationNode extends NodeDefinitionBase { */ children?: ChromeProjectNavigationNode[]; /** - * Flag to indicate if the node is currently active. + * Handler to render the node item with custom JSX. This handler is added to render the `children` of + * the Navigation.Item component when React components are used to declare the navigation tree. */ - isActive?: boolean; + renderItem?: () => React.ReactNode; } /** @public */ diff --git a/packages/shared-ux/chrome/navigation/__jest__/__snapshots__/project_navigation.test.tsx.snap b/packages/shared-ux/chrome/navigation/__jest__/__snapshots__/project_navigation.test.tsx.snap index 65ed8b80aa8a3..fd5e1b1af30e0 100644 --- a/packages/shared-ux/chrome/navigation/__jest__/__snapshots__/project_navigation.test.tsx.snap +++ b/packages/shared-ux/chrome/navigation/__jest__/__snapshots__/project_navigation.test.tsx.snap @@ -5,21 +5,14 @@ Array [ Object { "children": Array [ Object { - "children": undefined, "deepLink": undefined, "href": undefined, "id": "item1", - "isActive": false, - "isGroup": false, - "path": Array [ - "group1", - "item1", - ], + "path": "group1.item1", "sideNavStatus": "visible", "title": "Item 1", }, Object { - "children": undefined, "deepLink": Object { "baseUrl": "", "href": "", @@ -27,19 +20,13 @@ Array [ "title": "Title from deeplink!", "url": "", }, - "href": undefined, + "href": "", "id": "item2", - "isActive": false, - "isGroup": false, - "path": Array [ - "group1", - "item2", - ], + "path": "group1.item2", "sideNavStatus": "visible", "title": "Title from deeplink!", }, Object { - "children": undefined, "deepLink": Object { "baseUrl": "", "href": "", @@ -47,14 +34,9 @@ Array [ "title": "Title from deeplink!", "url": "", }, - "href": undefined, + "href": "", "id": "item3", - "isActive": false, - "isGroup": false, - "path": Array [ - "group1", - "item3", - ], + "path": "group1.item3", "sideNavStatus": "visible", "title": "Deeplink title overriden", }, @@ -62,11 +44,7 @@ Array [ "deepLink": undefined, "href": undefined, "id": "group1", - "isActive": false, - "isGroup": true, - "path": Array [ - "group1", - ], + "path": "group1", "sideNavStatus": "visible", "title": "Group 1", "type": "navGroup", @@ -74,7 +52,6 @@ Array [ Object { "children": Array [ Object { - "children": undefined, "deepLink": Object { "baseUrl": "/mocked", "href": "http://mocked/discover", @@ -82,19 +59,13 @@ Array [ "title": "Deeplink discover", "url": "/mocked/discover", }, - "href": undefined, + "href": "/mocked/discover", "id": "discover", - "isActive": false, - "isGroup": false, - "path": Array [ - "rootNav:analytics", - "discover", - ], + "path": "rootNav:analytics.discover", "sideNavStatus": "visible", "title": "Deeplink discover", }, Object { - "children": undefined, "deepLink": Object { "baseUrl": "/mocked", "href": "http://mocked/dashboards", @@ -102,19 +73,13 @@ Array [ "title": "Deeplink dashboards", "url": "/mocked/dashboards", }, - "href": undefined, + "href": "/mocked/dashboards", "id": "dashboards", - "isActive": false, - "isGroup": false, - "path": Array [ - "rootNav:analytics", - "dashboards", - ], + "path": "rootNav:analytics.dashboards", "sideNavStatus": "visible", "title": "Deeplink dashboards", }, Object { - "children": undefined, "deepLink": Object { "baseUrl": "/mocked", "href": "http://mocked/visualize", @@ -122,14 +87,9 @@ Array [ "title": "Deeplink visualize", "url": "/mocked/visualize", }, - "href": undefined, + "href": "/mocked/visualize", "id": "visualize", - "isActive": false, - "isGroup": false, - "path": Array [ - "rootNav:analytics", - "visualize", - ], + "path": "rootNav:analytics.visualize", "sideNavStatus": "visible", "title": "Deeplink visualize", }, @@ -138,11 +98,7 @@ Array [ "href": undefined, "icon": "stats", "id": "rootNav:analytics", - "isActive": false, - "isGroup": true, - "path": Array [ - "rootNav:analytics", - ], + "path": "rootNav:analytics", "renderAs": "accordion", "sideNavStatus": "visible", "title": "Data exploration", @@ -151,7 +107,6 @@ Array [ Object { "children": Array [ Object { - "children": undefined, "deepLink": Object { "baseUrl": "/mocked", "href": "http://mocked/ml:overview", @@ -159,19 +114,13 @@ Array [ "title": "Deeplink ml:overview", "url": "/mocked/ml:overview", }, - "href": undefined, + "href": "/mocked/ml:overview", "id": "ml:overview", - "isActive": false, - "isGroup": false, - "path": Array [ - "rootNav:ml", - "ml:overview", - ], + "path": "rootNav:ml.ml:overview", "sideNavStatus": "visible", "title": "Deeplink ml:overview", }, Object { - "children": undefined, "deepLink": Object { "baseUrl": "/mocked", "href": "http://mocked/ml:notifications", @@ -179,19 +128,13 @@ Array [ "title": "Deeplink ml:notifications", "url": "/mocked/ml:notifications", }, - "href": undefined, + "href": "/mocked/ml:notifications", "id": "ml:notifications", - "isActive": false, - "isGroup": false, - "path": Array [ - "rootNav:ml", - "ml:notifications", - ], + "path": "rootNav:ml.ml:notifications", "sideNavStatus": "visible", "title": "Deeplink ml:notifications", }, Object { - "children": undefined, "deepLink": Object { "baseUrl": "/mocked", "href": "http://mocked/ml:memoryUsage", @@ -199,14 +142,9 @@ Array [ "title": "Deeplink ml:memoryUsage", "url": "/mocked/ml:memoryUsage", }, - "href": undefined, + "href": "/mocked/ml:memoryUsage", "id": "ml:memoryUsage", - "isActive": false, - "isGroup": false, - "path": Array [ - "rootNav:ml", - "ml:memoryUsage", - ], + "path": "rootNav:ml.ml:memoryUsage", "sideNavStatus": "visible", "title": "Deeplink ml:memoryUsage", }, @@ -214,7 +152,6 @@ Array [ "children": Array [ Object { "breadcrumbStatus": "hidden", - "children": undefined, "deepLink": Object { "baseUrl": "/mocked", "href": "http://mocked/ml:anomalyDetection", @@ -222,20 +159,13 @@ Array [ "title": "Deeplink ml:anomalyDetection", "url": "/mocked/ml:anomalyDetection", }, - "href": undefined, + "href": "/mocked/ml:anomalyDetection", "id": "ml:anomalyDetection", - "isActive": false, - "isGroup": false, - "path": Array [ - "rootNav:ml", - "ml:anomalyDetection", - "ml:anomalyDetection", - ], + "path": "rootNav:ml.ml:anomalyDetection.ml:anomalyDetection", "sideNavStatus": "visible", "title": "Jobs", }, Object { - "children": undefined, "deepLink": Object { "baseUrl": "/mocked", "href": "http://mocked/ml:anomalyExplorer", @@ -243,20 +173,13 @@ Array [ "title": "Deeplink ml:anomalyExplorer", "url": "/mocked/ml:anomalyExplorer", }, - "href": undefined, + "href": "/mocked/ml:anomalyExplorer", "id": "ml:anomalyExplorer", - "isActive": false, - "isGroup": false, - "path": Array [ - "rootNav:ml", - "ml:anomalyDetection", - "ml:anomalyExplorer", - ], + "path": "rootNav:ml.ml:anomalyDetection.ml:anomalyExplorer", "sideNavStatus": "visible", "title": "Deeplink ml:anomalyExplorer", }, Object { - "children": undefined, "deepLink": Object { "baseUrl": "/mocked", "href": "http://mocked/ml:singleMetricViewer", @@ -264,20 +187,13 @@ Array [ "title": "Deeplink ml:singleMetricViewer", "url": "/mocked/ml:singleMetricViewer", }, - "href": undefined, + "href": "/mocked/ml:singleMetricViewer", "id": "ml:singleMetricViewer", - "isActive": false, - "isGroup": false, - "path": Array [ - "rootNav:ml", - "ml:anomalyDetection", - "ml:singleMetricViewer", - ], + "path": "rootNav:ml.ml:anomalyDetection.ml:singleMetricViewer", "sideNavStatus": "visible", "title": "Deeplink ml:singleMetricViewer", }, Object { - "children": undefined, "deepLink": Object { "baseUrl": "/mocked", "href": "http://mocked/ml:settings", @@ -285,15 +201,9 @@ Array [ "title": "Deeplink ml:settings", "url": "/mocked/ml:settings", }, - "href": undefined, + "href": "/mocked/ml:settings", "id": "ml:settings", - "isActive": false, - "isGroup": false, - "path": Array [ - "rootNav:ml", - "ml:anomalyDetection", - "ml:settings", - ], + "path": "rootNav:ml.ml:anomalyDetection.ml:settings", "sideNavStatus": "visible", "title": "Deeplink ml:settings", }, @@ -305,14 +215,9 @@ Array [ "title": "Deeplink ml:anomalyDetection", "url": "/mocked/ml:anomalyDetection", }, - "href": undefined, + "href": "/mocked/ml:anomalyDetection", "id": "ml:anomalyDetection", - "isActive": false, - "isGroup": true, - "path": Array [ - "rootNav:ml", - "ml:anomalyDetection", - ], + "path": "rootNav:ml.ml:anomalyDetection", "renderAs": "accordion", "sideNavStatus": "visible", "title": "Anomaly Detection", @@ -321,7 +226,6 @@ Array [ "children": Array [ Object { "breadcrumbStatus": "hidden", - "children": undefined, "deepLink": Object { "baseUrl": "/mocked", "href": "http://mocked/ml:dataFrameAnalytics", @@ -329,20 +233,13 @@ Array [ "title": "Deeplink ml:dataFrameAnalytics", "url": "/mocked/ml:dataFrameAnalytics", }, - "href": undefined, + "href": "/mocked/ml:dataFrameAnalytics", "id": "ml:dataFrameAnalytics", - "isActive": false, - "isGroup": false, - "path": Array [ - "rootNav:ml", - "ml:dataFrameAnalytics", - "ml:dataFrameAnalytics", - ], + "path": "rootNav:ml.ml:dataFrameAnalytics.ml:dataFrameAnalytics", "sideNavStatus": "visible", "title": "Jobs", }, Object { - "children": undefined, "deepLink": Object { "baseUrl": "/mocked", "href": "http://mocked/ml:resultExplorer", @@ -350,20 +247,13 @@ Array [ "title": "Deeplink ml:resultExplorer", "url": "/mocked/ml:resultExplorer", }, - "href": undefined, + "href": "/mocked/ml:resultExplorer", "id": "ml:resultExplorer", - "isActive": false, - "isGroup": false, - "path": Array [ - "rootNav:ml", - "ml:dataFrameAnalytics", - "ml:resultExplorer", - ], + "path": "rootNav:ml.ml:dataFrameAnalytics.ml:resultExplorer", "sideNavStatus": "visible", "title": "Deeplink ml:resultExplorer", }, Object { - "children": undefined, "deepLink": Object { "baseUrl": "/mocked", "href": "http://mocked/ml:analyticsMap", @@ -371,15 +261,9 @@ Array [ "title": "Deeplink ml:analyticsMap", "url": "/mocked/ml:analyticsMap", }, - "href": undefined, + "href": "/mocked/ml:analyticsMap", "id": "ml:analyticsMap", - "isActive": false, - "isGroup": false, - "path": Array [ - "rootNav:ml", - "ml:dataFrameAnalytics", - "ml:analyticsMap", - ], + "path": "rootNav:ml.ml:dataFrameAnalytics.ml:analyticsMap", "sideNavStatus": "visible", "title": "Deeplink ml:analyticsMap", }, @@ -391,14 +275,9 @@ Array [ "title": "Deeplink ml:dataFrameAnalytics", "url": "/mocked/ml:dataFrameAnalytics", }, - "href": undefined, + "href": "/mocked/ml:dataFrameAnalytics", "id": "ml:dataFrameAnalytics", - "isActive": false, - "isGroup": true, - "path": Array [ - "rootNav:ml", - "ml:dataFrameAnalytics", - ], + "path": "rootNav:ml.ml:dataFrameAnalytics", "renderAs": "accordion", "sideNavStatus": "visible", "title": "Data Frame Analytics", @@ -406,7 +285,6 @@ Array [ Object { "children": Array [ Object { - "children": undefined, "deepLink": Object { "baseUrl": "/mocked", "href": "http://mocked/ml:nodesOverview", @@ -414,20 +292,13 @@ Array [ "title": "Deeplink ml:nodesOverview", "url": "/mocked/ml:nodesOverview", }, - "href": undefined, + "href": "/mocked/ml:nodesOverview", "id": "ml:nodesOverview", - "isActive": false, - "isGroup": false, - "path": Array [ - "rootNav:ml", - "model_management", - "ml:nodesOverview", - ], + "path": "rootNav:ml.model_management.ml:nodesOverview", "sideNavStatus": "visible", "title": "Deeplink ml:nodesOverview", }, Object { - "children": undefined, "deepLink": Object { "baseUrl": "/mocked", "href": "http://mocked/ml:nodes", @@ -435,15 +306,9 @@ Array [ "title": "Deeplink ml:nodes", "url": "/mocked/ml:nodes", }, - "href": undefined, + "href": "/mocked/ml:nodes", "id": "ml:nodes", - "isActive": false, - "isGroup": false, - "path": Array [ - "rootNav:ml", - "model_management", - "ml:nodes", - ], + "path": "rootNav:ml.model_management.ml:nodes", "sideNavStatus": "visible", "title": "Deeplink ml:nodes", }, @@ -451,12 +316,7 @@ Array [ "deepLink": undefined, "href": undefined, "id": "model_management", - "isActive": false, - "isGroup": true, - "path": Array [ - "rootNav:ml", - "model_management", - ], + "path": "rootNav:ml.model_management", "renderAs": "accordion", "sideNavStatus": "visible", "title": "Model Management", @@ -464,7 +324,6 @@ Array [ Object { "children": Array [ Object { - "children": undefined, "deepLink": Object { "baseUrl": "/mocked", "href": "http://mocked/ml:fileUpload", @@ -472,20 +331,13 @@ Array [ "title": "Deeplink ml:fileUpload", "url": "/mocked/ml:fileUpload", }, - "href": undefined, + "href": "/mocked/ml:fileUpload", "id": "ml:fileUpload", - "isActive": false, - "isGroup": false, - "path": Array [ - "rootNav:ml", - "data_visualizer", - "ml:fileUpload", - ], + "path": "rootNav:ml.data_visualizer.ml:fileUpload", "sideNavStatus": "visible", "title": "File", }, Object { - "children": undefined, "deepLink": Object { "baseUrl": "/mocked", "href": "http://mocked/ml:indexDataVisualizer", @@ -494,20 +346,13 @@ Array [ "url": "/mocked/ml:indexDataVisualizer", }, "getIsActive": [Function], - "href": undefined, + "href": "/mocked/ml:indexDataVisualizer", "id": "ml:indexDataVisualizer", - "isActive": false, - "isGroup": false, - "path": Array [ - "rootNav:ml", - "data_visualizer", - "ml:indexDataVisualizer", - ], + "path": "rootNav:ml.data_visualizer.ml:indexDataVisualizer", "sideNavStatus": "visible", "title": "Data view", }, Object { - "children": undefined, "deepLink": Object { "baseUrl": "/mocked", "href": "http://mocked/ml:dataDrift", @@ -516,15 +361,9 @@ Array [ "url": "/mocked/ml:dataDrift", }, "getIsActive": [Function], - "href": undefined, + "href": "/mocked/ml:dataDrift", "id": "ml:dataDrift", - "isActive": false, - "isGroup": false, - "path": Array [ - "rootNav:ml", - "data_visualizer", - "ml:dataDrift", - ], + "path": "rootNav:ml.data_visualizer.ml:dataDrift", "sideNavStatus": "visible", "title": "Data drift", }, @@ -532,12 +371,7 @@ Array [ "deepLink": undefined, "href": undefined, "id": "data_visualizer", - "isActive": false, - "isGroup": true, - "path": Array [ - "rootNav:ml", - "data_visualizer", - ], + "path": "rootNav:ml.data_visualizer", "renderAs": "accordion", "sideNavStatus": "visible", "title": "Data Visualizer", @@ -545,7 +379,6 @@ Array [ Object { "children": Array [ Object { - "children": undefined, "deepLink": Object { "baseUrl": "/mocked", "href": "http://mocked/ml:logRateAnalysis", @@ -554,20 +387,13 @@ Array [ "url": "/mocked/ml:logRateAnalysis", }, "getIsActive": [Function], - "href": undefined, + "href": "/mocked/ml:logRateAnalysis", "id": "ml:logRateAnalysis", - "isActive": false, - "isGroup": false, - "path": Array [ - "rootNav:ml", - "aiops_labs", - "ml:logRateAnalysis", - ], + "path": "rootNav:ml.aiops_labs.ml:logRateAnalysis", "sideNavStatus": "visible", "title": "Deeplink ml:logRateAnalysis", }, Object { - "children": undefined, "deepLink": Object { "baseUrl": "/mocked", "href": "http://mocked/ml:logPatternAnalysis", @@ -576,20 +402,13 @@ Array [ "url": "/mocked/ml:logPatternAnalysis", }, "getIsActive": [Function], - "href": undefined, + "href": "/mocked/ml:logPatternAnalysis", "id": "ml:logPatternAnalysis", - "isActive": false, - "isGroup": false, - "path": Array [ - "rootNav:ml", - "aiops_labs", - "ml:logPatternAnalysis", - ], + "path": "rootNav:ml.aiops_labs.ml:logPatternAnalysis", "sideNavStatus": "visible", "title": "Deeplink ml:logPatternAnalysis", }, Object { - "children": undefined, "deepLink": Object { "baseUrl": "/mocked", "href": "http://mocked/ml:changePointDetections", @@ -598,15 +417,9 @@ Array [ "url": "/mocked/ml:changePointDetections", }, "getIsActive": [Function], - "href": undefined, + "href": "/mocked/ml:changePointDetections", "id": "ml:changePointDetections", - "isActive": false, - "isGroup": false, - "path": Array [ - "rootNav:ml", - "aiops_labs", - "ml:changePointDetections", - ], + "path": "rootNav:ml.aiops_labs.ml:changePointDetections", "sideNavStatus": "visible", "title": "Deeplink ml:changePointDetections", }, @@ -614,12 +427,7 @@ Array [ "deepLink": undefined, "href": undefined, "id": "aiops_labs", - "isActive": false, - "isGroup": true, - "path": Array [ - "rootNav:ml", - "aiops_labs", - ], + "path": "rootNav:ml.aiops_labs", "renderAs": "accordion", "sideNavStatus": "visible", "title": "AIOps labs", @@ -629,17 +437,12 @@ Array [ "href": undefined, "icon": "machineLearningApp", "id": "rootNav:ml", - "isActive": false, - "isGroup": true, - "path": Array [ - "rootNav:ml", - ], + "path": "rootNav:ml", "sideNavStatus": "visible", "title": "Machine Learning", "type": "navGroup", }, Object { - "children": undefined, "deepLink": Object { "baseUrl": "/mocked", "href": "http://mocked/dev_tools", @@ -647,14 +450,10 @@ Array [ "title": "Deeplink dev_tools", "url": "/mocked/dev_tools", }, - "href": undefined, + "href": "/mocked/dev_tools", "icon": "editorCodeBlock", "id": "devTools", - "isActive": false, - "isGroup": false, - "path": Array [ - "devTools", - ], + "path": "devTools", "sideNavStatus": "visible", "title": "Developer tools", "type": "navItem", @@ -663,7 +462,6 @@ Array [ "breadcrumbStatus": "hidden", "children": Array [ Object { - "children": undefined, "deepLink": Object { "baseUrl": "/mocked", "href": "http://mocked/management", @@ -671,42 +469,25 @@ Array [ "title": "Deeplink management", "url": "/mocked/management", }, - "href": undefined, + "href": "/mocked/management", "id": "management", - "isActive": false, - "isGroup": false, - "path": Array [ - "project_settings_project_nav", - "management", - ], + "path": "project_settings_project_nav.management", "sideNavStatus": "visible", "title": "Management", }, Object { - "children": undefined, "deepLink": undefined, "href": "https://cloud.elastic.co/deployments/123456789/security/users", "id": "cloudLinkUserAndRoles", - "isActive": false, - "isGroup": false, - "path": Array [ - "project_settings_project_nav", - "cloudLinkUserAndRoles", - ], + "path": "project_settings_project_nav.cloudLinkUserAndRoles", "sideNavStatus": "visible", "title": "Mock Users & Roles", }, Object { - "children": undefined, "deepLink": undefined, "href": "https://cloud.elastic.co/account/billing", "id": "cloudLinkBilling", - "isActive": false, - "isGroup": false, - "path": Array [ - "project_settings_project_nav", - "cloudLinkBilling", - ], + "path": "project_settings_project_nav.cloudLinkBilling", "sideNavStatus": "visible", "title": "Mock Billing & Subscriptions", }, @@ -715,11 +496,7 @@ Array [ "href": undefined, "icon": "gear", "id": "project_settings_project_nav", - "isActive": false, - "isGroup": true, - "path": Array [ - "project_settings_project_nav", - ], + "path": "project_settings_project_nav", "sideNavStatus": "visible", "title": "Project settings", "type": "navGroup", diff --git a/packages/shared-ux/chrome/navigation/__jest__/active_node.test.tsx b/packages/shared-ux/chrome/navigation/__jest__/active_node.test.tsx index a0c35ace442e4..5a82d4ccd509b 100644 --- a/packages/shared-ux/chrome/navigation/__jest__/active_node.test.tsx +++ b/packages/shared-ux/chrome/navigation/__jest__/active_node.test.tsx @@ -8,36 +8,35 @@ import './setup_jest_mocks'; import React from 'react'; import { type RenderResult, act } from '@testing-library/react'; -import { type Observable, of, BehaviorSubject } from 'rxjs'; +import { of, BehaviorSubject } from 'rxjs'; import type { - ChromeNavLink, ChromeProjectNavigation, ChromeProjectNavigationNode, } from '@kbn/core-chrome-browser'; import { Navigation } from '../src/ui/components/navigation'; import type { RootNavigationItemDefinition } from '../src/ui/types'; - +import { NavigationServices } from '../types'; import { renderNavigation, errorHandler, TestType } from './utils'; describe('Active node', () => { test('should set the active node', async () => { - const navLinks$: Observable = of([ - { + const deepLinks$: NavigationServices['deepLinks$'] = of({ + item1: { id: 'item1', title: 'Item 1', baseUrl: '', url: '', href: '', }, - { + item2: { id: 'item2', title: 'Item 2', baseUrl: '', url: '', href: '', }, - ]); + }); let activeNodes$: BehaviorSubject; @@ -47,12 +46,12 @@ describe('Active node', () => { { id: 'group1', title: 'Group 1', - path: ['group1'], + path: 'group1', }, { id: 'item1', title: 'Item 1', - path: ['group1', 'item1'], + path: 'group1.item1', }, ], ]); @@ -75,12 +74,12 @@ describe('Active node', () => { { id: 'group1', title: 'Group 1', - path: ['group1'], + path: 'group1', }, { id: 'item2', title: 'Item 2', - path: ['group1', 'item2'], + path: 'group1.item2', }, ], ]); @@ -112,7 +111,7 @@ describe('Active node', () => { const renderResult = renderNavigation({ navTreeDef: { body: navigationBody }, - services: { navLinks$, activeNodes$: getActiveNodes$() }, + services: { deepLinks$, activeNodes$: getActiveNodes$() }, }); await runTests('treeDef', renderResult); @@ -131,7 +130,7 @@ describe('Active node', () => { ), - services: { navLinks$, activeNodes$: getActiveNodes$() }, + services: { deepLinks$, activeNodes$: getActiveNodes$() }, }); await runTests('uiComponents', renderResult); @@ -139,15 +138,15 @@ describe('Active node', () => { }); test('should override the URL location to set the active node', async () => { - const navLinks$: Observable = of([ - { + const deepLinks$: NavigationServices['deepLinks$'] = of({ + item1: { id: 'item1', title: 'Item 1', baseUrl: '', url: '', href: '', }, - ]); + }); let activeNodes$: BehaviorSubject; @@ -197,7 +196,7 @@ describe('Active node', () => { const renderResult = renderNavigation({ navTreeDef: { body: navigationBody }, - services: { navLinks$, activeNodes$: getActiveNodes$() }, + services: { deepLinks$, activeNodes$: getActiveNodes$() }, onProjectNavigationChange, }); @@ -223,7 +222,7 @@ describe('Active node', () => { ), onProjectNavigationChange, - services: { navLinks$, activeNodes$: getActiveNodes$() }, + services: { deepLinks$, activeNodes$: getActiveNodes$() }, }); await runTests('uiComponents', renderResult); diff --git a/packages/shared-ux/chrome/navigation/__jest__/build_nav_tree.test.tsx b/packages/shared-ux/chrome/navigation/__jest__/build_nav_tree.test.tsx index df8246df69d5e..2487df25c0e64 100644 --- a/packages/shared-ux/chrome/navigation/__jest__/build_nav_tree.test.tsx +++ b/packages/shared-ux/chrome/navigation/__jest__/build_nav_tree.test.tsx @@ -8,13 +8,12 @@ import './setup_jest_mocks'; import React from 'react'; import { type RenderResult } from '@testing-library/react'; -import { type Observable, of } from 'rxjs'; +import { of } from 'rxjs'; import type { ChromeNavLink } from '@kbn/core-chrome-browser'; import { navLinksMock } from '../mocks/src/navlinks'; import { Navigation } from '../src/ui/components/navigation'; import type { RootNavigationItemDefinition } from '../src/ui/types'; - import { getMockFn, renderNavigation, @@ -23,6 +22,7 @@ import { type ProjectNavigationChangeListener, } from './utils'; import { getServicesMock } from '../mocks/src/jest'; +import { NavigationServices } from '../types'; const { cloudLinks: mockCloudLinks } = getServicesMock(); @@ -110,65 +110,38 @@ describe('builds navigation tree', () => { Object { "children": Array [ Object { - "children": undefined, "deepLink": undefined, "href": "https://foo", "id": "item1", - "isActive": false, - "isGroup": false, - "path": Array [ - "group1", - "item1", - ], + "path": "group1.item1", "sideNavStatus": "visible", "title": "Item 1", }, Object { - "children": undefined, "deepLink": undefined, "href": "https://foo", "id": "item2", - "isActive": false, - "isGroup": false, - "path": Array [ - "group1", - "item2", - ], + "path": "group1.item2", "sideNavStatus": "visible", "title": "Item 2", }, Object { "children": Array [ Object { - "children": undefined, "deepLink": undefined, "href": "https://foo", "id": "item1", - "isActive": false, - "isGroup": false, - "path": Array [ - "group1", - "group1A", - "item1", - ], + "path": "group1.group1A.item1", "sideNavStatus": "visible", "title": "Group 1A Item 1", }, Object { "children": Array [ Object { - "children": undefined, "deepLink": undefined, "href": "https://foo", "id": "item1", - "isActive": false, - "isGroup": false, - "path": Array [ - "group1", - "group1A", - "group1A_1", - "item1", - ], + "path": "group1.group1A.group1A_1.item1", "sideNavStatus": "visible", "title": "Group 1A_1 Item 1", }, @@ -176,38 +149,25 @@ describe('builds navigation tree', () => { "deepLink": undefined, "href": undefined, "id": "group1A_1", - "isActive": false, - "isGroup": true, - "path": Array [ - "group1", - "group1A", - "group1A_1", - ], + "path": "group1.group1A.group1A_1", "sideNavStatus": "visible", "title": "Group1A_1", }, ], "deepLink": undefined, + "defaultIsCollapsed": false, "href": undefined, "id": "group1A", - "isActive": true, - "isGroup": true, - "path": Array [ - "group1", - "group1A", - ], + "path": "group1.group1A", "sideNavStatus": "visible", "title": "Group1A", }, ], "deepLink": undefined, + "defaultIsCollapsed": false, "href": undefined, "id": "group1", - "isActive": true, - "isGroup": true, - "path": Array [ - "group1", - ], + "path": "group1", "sideNavStatus": "visible", "title": "", "type": "navGroup", @@ -246,65 +206,38 @@ describe('builds navigation tree', () => { Object { "children": Array [ Object { - "children": undefined, "deepLink": undefined, "href": "https://foo", "id": "item1", - "isActive": false, - "isGroup": false, - "path": Array [ - "group1", - "item1", - ], + "path": "group1.item1", "sideNavStatus": "visible", "title": "Item 1", }, Object { - "children": undefined, "deepLink": undefined, "href": "https://foo", "id": "item2", - "isActive": false, - "isGroup": false, - "path": Array [ - "group1", - "item2", - ], + "path": "group1.item2", "sideNavStatus": "visible", "title": "Item 2", }, Object { "children": Array [ Object { - "children": undefined, "deepLink": undefined, "href": "https://foo", "id": "item1", - "isActive": false, - "isGroup": false, - "path": Array [ - "group1", - "group1A", - "item1", - ], + "path": "group1.group1A.item1", "sideNavStatus": "visible", "title": "Group 1A Item 1", }, Object { "children": Array [ Object { - "children": undefined, "deepLink": undefined, "href": "https://foo", "id": "item1", - "isActive": false, - "isGroup": false, - "path": Array [ - "group1", - "group1A", - "group1A_1", - "item1", - ], + "path": "group1.group1A.group1A_1.item1", "sideNavStatus": "visible", "title": "Group 1A_1 Item 1", }, @@ -312,13 +245,7 @@ describe('builds navigation tree', () => { "deepLink": undefined, "href": undefined, "id": "group1A_1", - "isActive": false, - "isGroup": true, - "path": Array [ - "group1", - "group1A", - "group1A_1", - ], + "path": "group1.group1A.group1A_1", "sideNavStatus": "visible", "title": "Group1A_1", }, @@ -326,24 +253,16 @@ describe('builds navigation tree', () => { "deepLink": undefined, "href": undefined, "id": "group1A", - "isActive": false, - "isGroup": true, - "path": Array [ - "group1", - "group1A", - ], + "path": "group1.group1A", "sideNavStatus": "visible", "title": "Group1A", }, ], "deepLink": undefined, + "defaultIsCollapsed": false, "href": undefined, "id": "group1", - "isActive": true, - "isGroup": true, - "path": Array [ - "group1", - ], + "path": "group1", "sideNavStatus": "visible", "title": "", }, @@ -353,16 +272,19 @@ describe('builds navigation tree', () => { }); test('should read the title from deeplink, prop or React children', async () => { - const navLinks$: Observable = of([ - ...navLinksMock, - { + const deepLinks$: NavigationServices['deepLinks$'] = of({ + ...navLinksMock.reduce>((acc, navLink) => { + acc[navLink.id] = navLink; + return acc; + }, {}), + item1: { id: 'item1', title: 'Title from deeplink', baseUrl: '', url: '', href: '', }, - ]); + }); const onProjectNavigationChange = getMockFn(); @@ -425,7 +347,7 @@ describe('builds navigation tree', () => { const renderResult = renderNavigation({ navTreeDef: { body: navigationBody }, - services: { navLinks$ }, + services: { deepLinks$ }, onProjectNavigationChange, }); @@ -454,7 +376,7 @@ describe('builds navigation tree', () => { ), - services: { navLinks$ }, + services: { deepLinks$ }, onProjectNavigationChange, }); @@ -466,15 +388,15 @@ describe('builds navigation tree', () => { }); test('should not render the group if it does not have children AND no href or deeplink', async () => { - const navLinks$: Observable = of([ - { + const deepLinks$: NavigationServices['deepLinks$'] = of({ + item1: { id: 'item1', title: 'Title from deeplink', baseUrl: '', url: '', href: '', }, - ]); + }); const onProjectNavigationChange = getMockFn(); const runTests = (type: TestType, { queryByTestId }: RenderResult) => { @@ -523,7 +445,7 @@ describe('builds navigation tree', () => { const renderResult = renderNavigation({ navTreeDef: { body: navigationBody }, - services: { navLinks$ }, + services: { deepLinks$ }, onProjectNavigationChange, }); @@ -548,7 +470,7 @@ describe('builds navigation tree', () => { ), - services: { navLinks$ }, + services: { deepLinks$ }, onProjectNavigationChange, }); @@ -663,11 +585,7 @@ describe('builds navigation tree', () => { const renderResult = renderNavigation({ navigationElement: ( - - - - - + ), services: { recentlyAccessed$ }, diff --git a/packages/shared-ux/chrome/navigation/__jest__/links.test.tsx b/packages/shared-ux/chrome/navigation/__jest__/links.test.tsx index 56da3d4494c89..c52f56e3075d4 100644 --- a/packages/shared-ux/chrome/navigation/__jest__/links.test.tsx +++ b/packages/shared-ux/chrome/navigation/__jest__/links.test.tsx @@ -8,12 +8,11 @@ import './setup_jest_mocks'; import React from 'react'; import { type RenderResult } from '@testing-library/react'; -import { type Observable, of } from 'rxjs'; -import type { ChromeNavLink } from '@kbn/core-chrome-browser'; +import { of } from 'rxjs'; import { Navigation } from '../src/ui/components/navigation'; import type { RootNavigationItemDefinition } from '../src/ui/types'; - +import { NavigationServices } from '../types'; import { getMockFn, renderNavigation, @@ -27,15 +26,15 @@ describe('Links', () => { const onProjectNavigationChange = getMockFn(); const unknownLinkId = 'unknown'; - const navLinks$: Observable = of([ - { + const deepLinks$: NavigationServices['deepLinks$'] = of({ + item1: { id: 'item1', title: 'Title from deeplink', baseUrl: '', url: '', href: '', }, - ]); + }); const runTests = async (type: TestType, { findByTestId, queryByTestId }: RenderResult) => { try { @@ -84,7 +83,7 @@ describe('Links', () => { const renderResult = renderNavigation({ navTreeDef: { body: navigationBody }, onProjectNavigationChange, - services: { navLinks$ }, + services: { deepLinks$ }, }); await runTests('treeDef', renderResult); @@ -108,7 +107,7 @@ describe('Links', () => { ), onProjectNavigationChange, - services: { navLinks$ }, + services: { deepLinks$ }, }); await runTests('uiComponents', renderResult); diff --git a/packages/shared-ux/chrome/navigation/__jest__/panel.test.tsx b/packages/shared-ux/chrome/navigation/__jest__/panel.test.tsx index 40641eb31d2d2..e588ed384c112 100644 --- a/packages/shared-ux/chrome/navigation/__jest__/panel.test.tsx +++ b/packages/shared-ux/chrome/navigation/__jest__/panel.test.tsx @@ -13,7 +13,7 @@ import type { ChromeProjectNavigationNode } from '@kbn/core-chrome-browser'; import { Navigation } from '../src/ui/components/navigation'; import type { RootNavigationItemDefinition } from '../src/ui/types'; - +import { PanelContentProvider } from '../src/ui'; import { renderNavigation, errorHandler, @@ -21,7 +21,6 @@ import { getMockFn, ProjectNavigationChangeListener, } from './utils'; -import { PanelContentProvider } from '../src/ui'; describe('Panel', () => { test('should render group as panel opener', async () => { @@ -187,7 +186,7 @@ describe('Panel', () => { const [path0 = []] = activeNodes; return (
-

{selectedNode.id}

+

{selectedNode.path}

    {path0.map((node) => (
  • {node.id}
    • @@ -207,12 +206,12 @@ describe('Panel', () => { { id: 'activeGroup1', title: 'Group 1', - path: ['activeGroup1'], + path: 'activeGroup1', }, { id: 'activeItem1', title: 'Item 1', - path: ['activeGroup1', 'activeItem1'], + path: 'activeGroup1.activeItem1', }, ], ]); diff --git a/packages/shared-ux/chrome/navigation/__jest__/project_navigation.test.tsx b/packages/shared-ux/chrome/navigation/__jest__/project_navigation.test.tsx index 7b47c466b838b..a707d1e84b192 100644 --- a/packages/shared-ux/chrome/navigation/__jest__/project_navigation.test.tsx +++ b/packages/shared-ux/chrome/navigation/__jest__/project_navigation.test.tsx @@ -6,12 +6,12 @@ * Side Public License, v 1. */ import './setup_jest_mocks'; -import { type Observable, of } from 'rxjs'; +import { of } from 'rxjs'; import type { ChromeNavLink } from '@kbn/core-chrome-browser'; import { navLinksMock } from '../mocks/src/navlinks'; +import { NavigationServices } from '../types'; import type { ProjectNavigationTreeDefinition } from '../src/ui/types'; - import { getMockFn, renderNavigation, type ProjectNavigationChangeListener } from './utils'; describe('Default navigation', () => { @@ -22,16 +22,19 @@ describe('Default navigation', () => { */ test('builds the full navigation tree when only the project is provided', async () => { const onProjectNavigationChange = getMockFn(); - const navLinks$: Observable = of([ - ...navLinksMock, - { + const deepLinks$: NavigationServices['deepLinks$'] = of({ + ...navLinksMock.reduce>((acc, navLink) => { + acc[navLink.id] = navLink; + return acc; + }, {}), + item2: { id: 'item2', title: 'Title from deeplink!', baseUrl: '', url: '', href: '', }, - ]); + }); const projectNavigationTree: ProjectNavigationTreeDefinition = [ { @@ -62,7 +65,7 @@ describe('Default navigation', () => { renderNavigation({ projectNavigationTree, onProjectNavigationChange, - services: { navLinks$ }, + services: { deepLinks$ }, }); expect(onProjectNavigationChange).toHaveBeenCalled(); diff --git a/packages/shared-ux/chrome/navigation/mocks/src/jest.ts b/packages/shared-ux/chrome/navigation/mocks/src/jest.ts index 9ae6c2b2a6952..750b3faada7b1 100644 --- a/packages/shared-ux/chrome/navigation/mocks/src/jest.ts +++ b/packages/shared-ux/chrome/navigation/mocks/src/jest.ts @@ -13,18 +13,25 @@ import { navLinksMock } from './navlinks'; const activeNodes: ChromeProjectNavigationNode[][] = []; +const defaultDeepLinks = { + ...navLinksMock.reduce>((acc, navLink) => { + acc[navLink.id] = navLink; + return acc; + }, {}), +}; + export const getServicesMock = ({ - navLinks = navLinksMock, -}: { navLinks?: ChromeNavLink[] } = {}): NavigationServices => { + deepLinks = defaultDeepLinks, +}: { deepLinks?: Readonly> } = {}): NavigationServices => { const navigateToUrl = jest.fn().mockResolvedValue(undefined); const basePath = { prepend: jest.fn((path: string) => `/base${path}`) }; const recentlyAccessed$ = new BehaviorSubject([]); - const navLinks$ = new BehaviorSubject(navLinks); + const deepLinks$ = new BehaviorSubject(deepLinks); return { basePath, recentlyAccessed$, - navLinks$, + deepLinks$, navIsOpen: true, navigateToUrl, onProjectNavigationChange: jest.fn(), diff --git a/packages/shared-ux/chrome/navigation/mocks/src/storybook.ts b/packages/shared-ux/chrome/navigation/mocks/src/storybook.ts index df1416ec0f793..7a7d0e5fbe3ea 100644 --- a/packages/shared-ux/chrome/navigation/mocks/src/storybook.ts +++ b/packages/shared-ux/chrome/navigation/mocks/src/storybook.ts @@ -14,7 +14,7 @@ import { NavigationServices } from '../../types'; type Arguments = NavigationServices; export type Params = Pick< Arguments, - 'navIsOpen' | 'recentlyAccessed$' | 'activeNodes$' | 'navLinks$' | 'onProjectNavigationChange' + 'navIsOpen' | 'recentlyAccessed$' | 'activeNodes$' | 'deepLinks$' | 'onProjectNavigationChange' >; export class StorybookMock extends AbstractStorybookMock<{}, NavigationServices> { @@ -41,7 +41,7 @@ export class StorybookMock extends AbstractStorybookMock<{}, NavigationServices> basePath: { prepend: (suffix: string) => `/basepath${suffix}` }, navigateToUrl, recentlyAccessed$: params.recentlyAccessed$ ?? new BehaviorSubject([]), - navLinks$: params.navLinks$ ?? new BehaviorSubject([]), + deepLinks$: params.deepLinks$ ?? new BehaviorSubject({}), onProjectNavigationChange: params.onProjectNavigationChange ?? (() => undefined), activeNodes$: params.activeNodes$ ?? new BehaviorSubject([]), isSideNavCollapsed: true, diff --git a/packages/shared-ux/chrome/navigation/src/cloud_links.tsx b/packages/shared-ux/chrome/navigation/src/cloud_links.tsx index ce9d9e5990aab..b900daa340b44 100644 --- a/packages/shared-ux/chrome/navigation/src/cloud_links.tsx +++ b/packages/shared-ux/chrome/navigation/src/cloud_links.tsx @@ -7,7 +7,6 @@ */ import { i18n } from '@kbn/i18n'; import type { CloudLinkId } from '@kbn/core-chrome-browser'; -import type { CloudStart } from '@kbn/cloud-plugin/public'; export interface CloudLink { title: string; @@ -18,7 +17,12 @@ export type CloudLinks = { [id in CloudLinkId]?: CloudLink; }; -export const getCloudLinks = (cloud: CloudStart): CloudLinks => { +export const getCloudLinks = (cloud: { + billingUrl?: string; + deploymentUrl?: string; + performanceUrl?: string; + usersAndRolesUrl?: string; +}): CloudLinks => { const { billingUrl, deploymentUrl, performanceUrl, usersAndRolesUrl } = cloud; const links: CloudLinks = {}; diff --git a/packages/shared-ux/chrome/navigation/src/navnode_utils.ts b/packages/shared-ux/chrome/navigation/src/navnode_utils.ts new file mode 100644 index 0000000000000..daabf3af54d9c --- /dev/null +++ b/packages/shared-ux/chrome/navigation/src/navnode_utils.ts @@ -0,0 +1,171 @@ +/* + * 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 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import type { + AppDeepLinkId, + ChromeNavLink, + ChromeProjectNavigationNode, + CloudLinkId, + SideNavNodeStatus, +} from '@kbn/core-chrome-browser'; + +import type { CloudLinks } from './cloud_links'; +import type { NodeProps } from './ui/types'; +import { getNavigationNodeHref, getNavigationNodeId, isAbsoluteLink } from './utils'; + +/** + * We don't have currently a way to know if a user has access to a Cloud section. + * TODO: This function will have to be revisited once we have an API from Cloud to know the user + * permissions. + */ +function hasUserAccessToCloudLink(): boolean { + return true; +} + +function getNodeStatus( + { + link, + deepLink, + cloudLink, + sideNavStatus, + }: { + link?: string; + deepLink?: ChromeNavLink; + cloudLink?: CloudLinkId; + sideNavStatus?: SideNavNodeStatus; + }, + { cloudLinks }: { cloudLinks: CloudLinks } +): SideNavNodeStatus | 'remove' { + if (link && !deepLink) { + // If a link is provided, but no deepLink is found, don't render anything + return 'remove'; + } + + if (cloudLink) { + if (!cloudLinks[cloudLink]) { + // Invalid cloudLinkId or link url has not been set in kibana.yml + return 'remove'; + } + if (!hasUserAccessToCloudLink()) return 'remove'; + } + + if (deepLink && deepLink.hidden) return 'hidden'; + + return sideNavStatus ?? 'visible'; +} + +function getTitleForNode< + LinkId extends AppDeepLinkId = AppDeepLinkId, + Id extends string = string, + ChildrenId extends string = Id +>( + navNode: NodeProps, + { deepLink, cloudLinks }: { deepLink?: ChromeNavLink; cloudLinks: CloudLinks } +): string { + const { children } = navNode; + if (navNode.title) { + return navNode.title; + } + + if (typeof children === 'string') { + return children; + } + + if (deepLink?.title) { + return deepLink.title; + } + + if (navNode.cloudLink) { + return cloudLinks[navNode.cloudLink]?.title ?? ''; + } + + return ''; +} + +function validateNodeProps< + LinkId extends AppDeepLinkId = AppDeepLinkId, + Id extends string = string, + ChildrenId extends string = Id +>({ id, link, href, cloudLink, renderAs }: NodeProps) { + if (link && cloudLink) { + throw new Error( + `[Chrome navigation] Error in node [${id}]. Only one of "link" or "cloudLink" can be provided.` + ); + } + if (href && cloudLink) { + throw new Error( + `[Chrome navigation] Error in node [${id}]. Only one of "href" or "cloudLink" can be provided.` + ); + } + if (renderAs === 'panelOpener' && !link) { + throw new Error( + `[Chrome navigation] Error in node [${id}]. If renderAs is set to "panelOpener", a "link" must also be provided.` + ); + } + if (renderAs === 'item' && !link) { + throw new Error( + `[Chrome navigation] Error in node [${id}]. If renderAs is set to "item", a "link" must also be provided.` + ); + } +} + +export const initNavNode = < + LinkId extends AppDeepLinkId = AppDeepLinkId, + Id extends string = string, + ChildrenId extends string = Id +>( + node: NodeProps, + { cloudLinks, deepLinks }: { cloudLinks: CloudLinks; deepLinks: Record } +): ChromeProjectNavigationNode | null => { + validateNodeProps(node); + + const { + cloudLink, + link, + parentNodePath, + rootIndex = 0, + treeDepth = 0, + index = 0, + children, + ...navNodeFromProps + } = node; + const deepLink = link !== undefined ? deepLinks[link] : undefined; + const sideNavStatus = getNodeStatus( + { + link, + deepLink, + cloudLink, + sideNavStatus: navNodeFromProps.sideNavStatus, + }, + { cloudLinks } + ); + if (sideNavStatus === 'remove') { + return null; + } + + const id = getNavigationNodeId(node, () => `node-${rootIndex}-${treeDepth}-${index}`) as Id; + const title = getTitleForNode(node, { deepLink, cloudLinks }); + const href = cloudLink ? cloudLinks[cloudLink]?.href : node.href; + const path = parentNodePath ? `${parentNodePath}.${id}` : id; + + if (href && !isAbsoluteLink(href)) { + throw new Error(`href must be an absolute URL. Node id [${id}].`); + } + + const navNode: ChromeProjectNavigationNode = { + ...navNodeFromProps, + id, + href: getNavigationNodeHref({ href, deepLink }), + path, + title, + deepLink, + sideNavStatus, + }; + + return navNode; +}; diff --git a/packages/shared-ux/chrome/navigation/src/services.tsx b/packages/shared-ux/chrome/navigation/src/services.tsx index f77a288160a3d..54e45ede100e4 100644 --- a/packages/shared-ux/chrome/navigation/src/services.tsx +++ b/packages/shared-ux/chrome/navigation/src/services.tsx @@ -8,6 +8,9 @@ import React, { FC, useContext, useMemo } from 'react'; import useObservable from 'react-use/lib/useObservable'; +import { map } from 'rxjs'; +import type { ChromeNavLink } from '@kbn/core-chrome-browser'; + import { NavigationKibanaDependencies, NavigationServices } from '../types'; import { CloudLink, CloudLinks, getCloudLinks } from './cloud_links'; @@ -61,17 +64,31 @@ export const NavigationKibanaProvider: FC = ({ const { chrome, http } = core; const { basePath } = http; const { navigateToUrl } = core.application; + const { billingUrl, deploymentUrl, performanceUrl, usersAndRolesUrl } = cloud; - const cloudLinks: CloudLinks = useMemo( - () => (cloud ? parseCloudURLs(getCloudLinks(cloud)) : {}), - [cloud] - ); + const cloudLinks: CloudLinks = useMemo(() => { + return parseCloudURLs( + getCloudLinks({ billingUrl, deploymentUrl, performanceUrl, usersAndRolesUrl }) + ); + }, [billingUrl, deploymentUrl, performanceUrl, usersAndRolesUrl]); const isSideNavCollapsed = useObservable(chrome.getIsSideNavCollapsed$(), true); + const navLinks$ = useMemo(() => chrome.navLinks.getNavLinks$(), [chrome.navLinks]); + const deepLinks$ = useMemo(() => { + return navLinks$.pipe( + map((navLinks) => { + return navLinks.reduce((acc, navLink) => { + acc[navLink.id] = navLink; + return acc; + }, {} as Record); + }) + ); + }, [navLinks$]); + const value: NavigationServices = { basePath, recentlyAccessed$: chrome.recentlyAccessed.get$(), - navLinks$: chrome.navLinks.getNavLinks$(), + deepLinks$, navigateToUrl, navIsOpen: true, onProjectNavigationChange: serverless.setNavigation, diff --git a/packages/shared-ux/chrome/navigation/src/ui/components/navigation.tsx b/packages/shared-ux/chrome/navigation/src/ui/components/navigation.tsx index 8f74abee6a110..d56495b810f4a 100644 --- a/packages/shared-ux/chrome/navigation/src/ui/components/navigation.tsx +++ b/packages/shared-ux/chrome/navigation/src/ui/components/navigation.tsx @@ -8,19 +8,18 @@ import React, { createContext, - useState, useCallback, ReactNode, useMemo, - useEffect, useContext, useRef, + Children, } from 'react'; import type { ChromeProjectNavigationNode } from '@kbn/core-chrome-browser'; -import useDebounce from 'react-use/lib/useDebounce'; import useObservable from 'react-use/lib/useObservable'; import { useNavigation as useNavigationServices } from '../../services'; +import { getChildType } from '../../utils'; import { RegisterFunction, UnRegisterFunction } from '../types'; import { NavigationFooter } from './navigation_footer'; import { NavigationGroup } from './navigation_group'; @@ -31,17 +30,12 @@ import { PanelProvider, type ContentProvider } from './panel'; interface Context { register: RegisterFunction; - updateFooterChildren: (children: ReactNode) => void; unstyled: boolean; activeNodes: ChromeProjectNavigationNode[][]; } const NavigationContext = createContext({ - register: () => ({ - unregister: () => {}, - path: [], - }), - updateFooterChildren: () => {}, + register: () => () => {}, unstyled: false, activeNodes: [], }); @@ -77,83 +71,105 @@ export function Navigation({ const idx = useRef(0); const activeNodes = useObservable(activeNodes$, []); - const [navigationItems, setNavigationItems] = useState< - Record - >({}); - const [debouncedNavigationItems, setDebouncedNavigationItems] = useState< - Record - >({}); - const [footerChildren, setFooterChildren] = useState(null); - - const unregister: UnRegisterFunction = useCallback((id: string) => { - setNavigationItems((prevItems) => { - const updatedItems = { ...prevItems }; - delete updatedItems[id]; - return updatedItems; + const navigationItemsRef = useRef>({}); + + const onNavigationItemsChange = useCallback(() => { + const navigationTree = Object.values(navigationItemsRef.current).sort((a, b) => { + const aOrder = orderChildrenRef.current[a.id]; + const bOrder = orderChildrenRef.current[b.id]; + return aOrder - bOrder; }); - }, []); + + // This will update the navigation tree in the Chrome service (calling the serverless.setNavigation()) + onProjectNavigationChange({ navigationTree }); + }, [onProjectNavigationChange]); + + const unregister = useCallback( + (id: string) => { + const updatedItems = { ...navigationItemsRef.current }; + delete updatedItems[id]; + navigationItemsRef.current = updatedItems; + + onNavigationItemsChange(); + }, + [onNavigationItemsChange] + ); const register = useCallback( - (navNode) => { + (navNode, order): UnRegisterFunction => { if (orderChildrenRef.current[navNode.id] === undefined) { - orderChildrenRef.current[navNode.id] = idx.current++; + orderChildrenRef.current[navNode.id] = order ?? idx.current++; } - setNavigationItems((prevItems) => { - return { - ...prevItems, - [navNode.id]: navNode, - }; - }); + const updatedRef = { ...navigationItemsRef.current, [navNode.id]: navNode }; + navigationItemsRef.current = updatedRef; + + onNavigationItemsChange(); - return { - unregister, - path: [navNode.id], - }; + return () => unregister(navNode.id); }, - [unregister] + [unregister, onNavigationItemsChange] ); const contextValue = useMemo( () => ({ register, - updateFooterChildren: setFooterChildren, unstyled, activeNodes, }), [register, unstyled, activeNodes] ); - useDebounce( - () => { - setDebouncedNavigationItems(navigationItems); - }, - 100, - [navigationItems] - ); + const childrenParsed = useMemo(() => { + let footerChildren: ReactNode; + let rootIndex = 0; + + const parseChildren = (_children: ReactNode, wrapperComponent = '') => { + const parsed: ReactNode[] = []; + Children.forEach(_children, (child, i) => { + if (!React.isValidElement(child)) { + return; + } + + const childType = getChildType(child); + if (childType === 'unknown' && unstyled === false) { + throw new Error( + `${wrapperComponent} only accepts , and as children. Received ${child.type}` + ); + } + + if (childType === 'footer') { + if (footerChildren) { + throw new Error('Only one is allowed'); + } + footerChildren = parseChildren(child.props.children, ''); + return; + } + + // We add a "rootIndex" prop to each child to keep track of the order of the children + // and correctly set the order of nodes in the navigation tree independently of when a + // node register itself (it could be after a deepLink is being activated). + parsed.push({ ...child, props: { ...child.props, rootIndex } }); + rootIndex += 1; + }); - useEffect(() => { - const navigationTree = Object.values(debouncedNavigationItems).sort((a, b) => { - const aOrder = orderChildrenRef.current[a.id]; - const bOrder = orderChildrenRef.current[b.id]; - return aOrder - bOrder; - }); + return parsed; + }; - // This will update the navigation tree in the Chrome service (calling the serverless.setNavigation()) - onProjectNavigationChange({ - navigationTree, - }); - }, [debouncedNavigationItems, onProjectNavigationChange]); + const bodyChildren: ReactNode[] = parseChildren(children); + + return { body: bodyChildren, footer: footerChildren }; + }, [children, unstyled]); return ( - {children} + {childrenParsed.body} diff --git a/packages/shared-ux/chrome/navigation/src/ui/components/navigation_footer.tsx b/packages/shared-ux/chrome/navigation/src/ui/components/navigation_footer.tsx index d4b78c1e93053..083e8e66ee55a 100644 --- a/packages/shared-ux/chrome/navigation/src/ui/components/navigation_footer.tsx +++ b/packages/shared-ux/chrome/navigation/src/ui/components/navigation_footer.tsx @@ -6,23 +6,14 @@ * Side Public License, v 1. */ -import React, { useEffect } from 'react'; -import { useNavigation } from './navigation'; +import { type FC } from 'react'; export interface Props { - children?: React.ReactNode; + children?: JSX.Element[]; } -function NavigationFooterComp({ children }: Props) { - const { updateFooterChildren } = useNavigation(); - - useEffect(() => { - if (children) { - updateFooterChildren(children); - } - }, [children, updateFooterChildren]); - +// Note: this component is only used to detect which children are part of the body and which +// are part of the footer. See the "childrenParsed" value of the component. +export const NavigationFooter: FC = () => { return null; -} - -export const NavigationFooter = React.memo(NavigationFooterComp); +}; diff --git a/packages/shared-ux/chrome/navigation/src/ui/components/navigation_group.tsx b/packages/shared-ux/chrome/navigation/src/ui/components/navigation_group.tsx index 2f3ecbd381b45..11beb966963a1 100644 --- a/packages/shared-ux/chrome/navigation/src/ui/components/navigation_group.tsx +++ b/packages/shared-ux/chrome/navigation/src/ui/components/navigation_group.tsx @@ -6,31 +6,98 @@ * Side Public License, v 1. */ -import React, { createContext, useCallback, useMemo, useContext } from 'react'; -import type { AppDeepLinkId, ChromeProjectNavigationNode } from '@kbn/core-chrome-browser'; +import React, { useMemo, Children, ReactNode, useEffect, useRef } from 'react'; +import useObservable from 'react-use/lib/useObservable'; +import deepEqual from 'react-fast-compare'; +import type { + AppDeepLinkId, + ChromeProjectNavigationNode, + ChromeNavLink, +} from '@kbn/core-chrome-browser'; import { useNavigation as useNavigationServices } from '../../services'; -import { useInitNavNode } from '../hooks'; -import type { NodeProps, NodePropsEnhanced, RegisterFunction } from '../types'; +import type { NodeProps } from '../types'; import { NavigationSectionUI } from './navigation_section_ui'; import { useNavigation } from './navigation'; import { NavigationBucket, type Props as NavigationBucketProps } from './navigation_bucket'; +import { generateUniqueNodeId, getChildType } from '../../utils'; +import { initNavNode } from '../../navnode_utils'; +import { CloudLinks } from '../../cloud_links'; + +/** + * Handler to convert the JSX children of the NavigationGroup to the ChromeProjectNavigationNode + * interface. We do that by parsing the children with the React.Children func, read their prop + * and initiate the node objects. + */ +const jsxChildrenToNavigationNode = ( + { + parentNodePath, + jsxChildren, + rootIndex, + treeDepth, + }: { parentNodePath: string; jsxChildren?: ReactNode; rootIndex: number; treeDepth: number }, + { cloudLinks, deepLinks }: { cloudLinks: CloudLinks; deepLinks: Record } +): ChromeProjectNavigationNode[] | undefined => { + if (!jsxChildren) return undefined; + + const navigationNodes: ChromeProjectNavigationNode[] = []; + + Children.forEach(jsxChildren, (child, index) => { + if (!React.isValidElement(child)) { + return; + } + const title = + typeof child.props.children === 'string' ? child.props.children : child.props.title; + const childNode = initNavNode( + { ...child.props, title, rootIndex, treeDepth, index, parentNodePath }, + { cloudLinks, deepLinks } + ); -interface Context { - register: RegisterFunction; -} + if (!childNode) return; + + const childType = getChildType(child); + + if (childType !== 'group') { + if (childType === 'item') { + if (child.props.children && typeof child.props.children !== 'string') { + // Render the node item + childNode.renderItem = () => child.props.children; + } + navigationNodes.push(childNode); + } else { + // This is a custom JSX node, render it "as is" in the nav. + navigationNodes.push({ + id: generateUniqueNodeId(), + title: '', + path: '', + renderItem: () => child, + }); + } + return; + } -export const NavigationGroupContext = createContext(undefined); + if (child.props?.children) { + navigationNodes.push({ + ...childNode, + // Recursively add all the children of the group + children: jsxChildrenToNavigationNode( + { + parentNodePath: childNode.path, + jsxChildren: child.props.children, + rootIndex, + treeDepth: treeDepth + 1, + }, + { cloudLinks, deepLinks } + ), + }); + return; + } -export function useNavigationGroup( - throwIfNotFound: T = true as T -): T extends true ? Context : Context | undefined { - const context = useContext(NavigationGroupContext); - if (!context && throwIfNotFound) { - throw new Error('useNavigationGroup must be used within a NavigationGroup provider'); - } - return context as T extends true ? Context : Context | undefined; -} + navigationNodes.push(childNode); + }); + + return navigationNodes.length > 0 ? navigationNodes : undefined; +}; export interface Props< LinkId extends AppDeepLinkId = AppDeepLinkId, @@ -45,82 +112,60 @@ function NavigationGroupInternalComp< Id extends string = string, ChildrenId extends string = Id >(props: Props) { - const { cloudLinks } = useNavigationServices(); - const navigationContext = useNavigation(); - - const { children, node } = useMemo(() => { - const { children: _children, defaultIsCollapsed, ...rest } = props; - const nodeEnhanced: Omit, 'children'> = { - ...rest, - isActive: defaultIsCollapsed !== undefined ? defaultIsCollapsed === false : undefined, - isGroup: true, - }; - return { - children: _children, - node: nodeEnhanced, - }; - }, [props]); - - const { navNode, registerChildNode, path, childrenNodes } = useInitNavNode(node, { cloudLinks }); + const { cloudLinks, deepLinks$ } = useNavigationServices(); + const { register } = useNavigation(); + const deepLinks = useObservable(deepLinks$, {}); + const { rootIndex = 0 } = props; - // We add to the nav node the children that have mounted and registered themselves. - // Those children render in the UI inside the NavigationSectionUI -> EuiCollapsibleNavItem -> items - const navNodeWithChildren = useMemo(() => { - if (!navNode) return null; + const navNodeRef = useRef(); + const childrenNodesRef = useRef(); - const hasChildren = Object.keys(childrenNodes).length > 0; - const withChildren: ChromeProjectNavigationNode = { - ...navNode, - children: hasChildren ? Object.values(childrenNodes) : undefined, - }; + const navNode = useMemo(() => { + const _navNode = initNavNode(props, { cloudLinks, deepLinks }); - return withChildren; - }, [navNode, childrenNodes]); + if (!_navNode) return null; - const unstyled = props.unstyled ?? navigationContext.unstyled; + const childrenNodes = jsxChildrenToNavigationNode( + { parentNodePath: _navNode.path, jsxChildren: props.children, rootIndex, treeDepth: 1 }, + { cloudLinks, deepLinks } + ); - const renderContent = useCallback(() => { - if (!path || !navNodeWithChildren) { - return null; + const childrenChanged = deepEqual(childrenNodes, childrenNodesRef.current) === false; + if (childrenChanged) { + childrenNodesRef.current = childrenNodes; } - if (navNodeWithChildren.sideNavStatus === 'hidden') return null; + const nextValue = { + ..._navNode, + children: childrenNodesRef.current, + }; - if (unstyled) { - // No UI for unstyled groups - return children; + const hasChanged = deepEqual(nextValue, navNodeRef.current) === false; + if (hasChanged) { + navNodeRef.current = nextValue; } - // We will only render the component for root groups. The nested group - // are handled by the EuiCollapsibleNavItem component through its "items" prop. - const isRootLevel = path && path.length === 1; + if (navNodeRef.current === undefined) { + // Adding this check for TS purpose, it should never be undefined. + throw new Error('Navnode ref is undefined.'); + } - return ( - <> - {isRootLevel && } - {/* We render the children so they mount and can **register** themselves but - visually they don't appear here in the DOM. They are rendered inside the - "items" prop (see ) */} - {children} - - ); - }, [navNodeWithChildren, path, children, unstyled]); + return navNodeRef.current; + }, [props, cloudLinks, deepLinks, rootIndex]); - const contextValue = useMemo(() => { - return { - register: registerChildNode, - }; - }, [registerChildNode]); + /** Register when mounting and whenever the internal nav node changes */ + useEffect(() => { + if (navNode) { + return register(navNode, rootIndex); + } + return undefined; + }, [register, navNode, rootIndex]); - if (!navNode) { + if (!navNode || navNode.sideNavStatus === 'hidden') { return null; } - return ( - - {renderContent()} - - ); + return ; } function NavigationGroupComp< diff --git a/packages/shared-ux/chrome/navigation/src/ui/components/navigation_item.tsx b/packages/shared-ux/chrome/navigation/src/ui/components/navigation_item.tsx index dae8cef6f4eee..068f12d7ccd95 100644 --- a/packages/shared-ux/chrome/navigation/src/ui/components/navigation_item.tsx +++ b/packages/shared-ux/chrome/navigation/src/ui/components/navigation_item.tsx @@ -6,16 +6,18 @@ * Side Public License, v 1. */ -import React, { Fragment, useEffect, useMemo } from 'react'; +import React, { Fragment, useEffect, useMemo, useRef } from 'react'; import type { AppDeepLinkId, ChromeProjectNavigationNode } from '@kbn/core-chrome-browser'; import { EuiCollapsibleNavItem } from '@elastic/eui'; import classNames from 'classnames'; +import deepEqual from 'react-fast-compare'; +import useObservable from 'react-use/lib/useObservable'; import { useNavigation as useNavigationServices } from '../../services'; -import { useInitNavNode } from '../hooks'; -import type { NodeProps, NodePropsEnhanced } from '../types'; +import type { NodeProps } from '../types'; import { useNavigation } from './navigation'; -import { getNavigationNodeHref } from '../../utils'; +import { isActiveFromUrl } from '../../utils'; +import { initNavNode } from '../../navnode_utils'; export interface Props< LinkId extends AppDeepLinkId = AppDeepLinkId, @@ -30,31 +32,55 @@ function NavigationItemComp< Id extends string = string, ChildrenId extends string = Id >(props: Props) { - const { cloudLinks, navigateToUrl } = useNavigationServices(); - const navigationContext = useNavigation(); - const navNodeRef = React.useRef(null); + const { cloudLinks, navigateToUrl, deepLinks$ } = useNavigationServices(); + const { unstyled: unstyledFromContext, register, activeNodes } = useNavigation(); + const deepLinks = useObservable(deepLinks$, {}); + const navNodeRef = useRef(); + const { rootIndex, appendHorizontalRule } = props; const { children, node } = useMemo(() => { const { children: _children, ...rest } = props; - const nodeEnhanced: Omit, 'children'> = { - ...rest, - isGroup: false, - }; + if (typeof _children === 'string') { - nodeEnhanced.title = nodeEnhanced.title ?? _children; + rest.title = rest.title ?? _children; } + return { children: _children, - node: nodeEnhanced, + node: rest, }; }, [props]); - const unstyled = props.unstyled ?? navigationContext.unstyled; + const unstyled = props.unstyled ?? unstyledFromContext; + + const navNode = useMemo(() => { + const _navNode = initNavNode(node, { cloudLinks, deepLinks }); + if (!_navNode) return null; + + const hasChanged = deepEqual(_navNode, navNodeRef.current) === false; + if (hasChanged) { + navNodeRef.current = _navNode; + } + + if (navNodeRef.current === undefined) { + // Adding this check for TS purpose, it should never be undefined. + throw new Error('Navnode ref is undefined.'); + } - const { navNode } = useInitNavNode(node, { cloudLinks }); + return navNodeRef.current; + }, [node, cloudLinks, deepLinks]); + + if (navNode && appendHorizontalRule) { + throw new Error( + `[Chrome navigation] Error in node [${navNode.id}]. "appendHorizontalRule" can only be added for group with children.` + ); + } useEffect(() => { - navNodeRef.current = navNode; - }, [navNode]); + if (navNode) { + return register(navNode, rootIndex); + } + return undefined; + }, [register, navNode, rootIndex]); if (!navNode) { return null; @@ -71,40 +97,34 @@ function NavigationItemComp< return {navNode.title}; } - const isRootLevel = navNode.path.length === 1; - - if (isRootLevel) { - const href = getNavigationNodeHref(navNode); - const dataTestSubj = classNames(`nav-item`, { - [`nav-item-deepLinkId-${navNode.deepLink?.id}`]: !!navNode.deepLink, - [`nav-item-isActive`]: navNode.isActive, - }); - - return ( - { - e.preventDefault(); - e.stopPropagation(); - if (href) { - navigateToUrl(href); - } - }, - }} - /> - ); - } + const isActive = isActiveFromUrl(navNode.path, activeNodes); + + const { href } = navNode; + const dataTestSubj = classNames(`nav-item`, { + [`nav-item-deepLinkId-${navNode.deepLink?.id}`]: !!navNode.deepLink, + [`nav-item-isActive`]: isActive, + }); - // We don't render anything in the UI for non root item as those register themselves on the parent (Group) - // updating its "childrenNodes" state which are then converted to "items" for the EuiCollapsibleNavItem component. - return null; + return ( + { + e.preventDefault(); + e.stopPropagation(); + if (href) { + navigateToUrl(href); + } + }, + }} + /> + ); } export const NavigationItem = React.memo(NavigationItemComp) as typeof NavigationItemComp; diff --git a/packages/shared-ux/chrome/navigation/src/ui/components/navigation_item_open_panel.tsx b/packages/shared-ux/chrome/navigation/src/ui/components/navigation_item_open_panel.tsx index eb12759eb09d8..8e4475a979a06 100644 --- a/packages/shared-ux/chrome/navigation/src/ui/components/navigation_item_open_panel.tsx +++ b/packages/shared-ux/chrome/navigation/src/ui/components/navigation_item_open_panel.tsx @@ -23,9 +23,9 @@ import { } from '@elastic/eui'; import type { ChromeProjectNavigationNode } from '@kbn/core-chrome-browser'; import type { NavigateToUrlFn } from '../../../types/internal'; -import { nodePathToString } from '../../utils'; import { useNavigation as useServices } from '../../services'; import { usePanel } from './panel'; +import { isActiveFromUrl } from '../../utils'; const getStyles = (euiTheme: EuiThemeComputed<{}>) => css` * { @@ -47,17 +47,19 @@ const getStyles = (euiTheme: EuiThemeComputed<{}>) => css` interface Props { item: ChromeProjectNavigationNode; navigateToUrl: NavigateToUrlFn; + activeNodes: ChromeProjectNavigationNode[][]; } -export const NavigationItemOpenPanel: FC = ({ item, navigateToUrl }: Props) => { +export const NavigationItemOpenPanel: FC = ({ item, navigateToUrl, activeNodes }: Props) => { const { euiTheme } = useEuiTheme(); const { open: openPanel, close: closePanel, selectedNode } = usePanel(); const { isSideNavCollapsed } = useServices(); - const { title, deepLink, isActive, children } = item; - const id = nodePathToString(item); + const { title, deepLink, children } = item; + const { id, path } = item; const href = deepLink?.url ?? item.href; const isNotMobile = useIsWithinMinBreakpoint('s'); const isIconVisible = isNotMobile && !isSideNavCollapsed && !!children && children.length > 0; + const isActive = isActiveFromUrl(item.path, activeNodes); const itemClassNames = classNames( 'sideNavItem', @@ -65,12 +67,12 @@ export const NavigationItemOpenPanel: FC = ({ item, navigateToUrl }: Prop getStyles(euiTheme) ); - const dataTestSubj = classNames(`nav-item`, `nav-item-${id}`, { + const dataTestSubj = classNames(`nav-item`, `nav-item-${path}`, { [`nav-item-deepLinkId-${deepLink?.id}`]: !!deepLink, [`nav-item-id-${id}`]: id, [`nav-item-isActive`]: isActive, }); - const buttonDataTestSubj = classNames(`panelOpener`, `panelOpener-${id}`, { + const buttonDataTestSubj = classNames(`panelOpener`, `panelOpener-${path}`, { [`panelOpener-deepLinkId-${deepLink?.id}`]: !!deepLink, }); @@ -113,7 +115,7 @@ export const NavigationItemOpenPanel: FC = ({ item, navigateToUrl }: Prop {isIconVisible && ( Boolean(navNod const itemIsVisible = (item: ChromeProjectNavigationNode) => { if (item.sideNavStatus === 'hidden') return false; + if (item.renderItem) return true; + if (nodeHasLink(item)) { return true; } @@ -61,8 +66,8 @@ const getRenderAs = (navNode: ChromeProjectNavigationNode): RenderAs => { }; const getTestSubj = (navNode: ChromeProjectNavigationNode, isActive = false): string => { - const { id, deepLink } = navNode; - return classnames(`nav-item`, `nav-item-${id}`, { + const { id, path, deepLink } = navNode; + return classnames(`nav-item`, `nav-item-${path}`, { [`nav-item-deepLinkId-${deepLink?.id}`]: !!deepLink, [`nav-item-id-${id}`]: id, [`nav-item-isActive`]: isActive, @@ -79,9 +84,7 @@ const filterChildren = ( const serializeNavNode = (navNode: ChromeProjectNavigationNode) => { const serialized: ChromeProjectNavigationNode = { ...navNode, - id: nodePathToString(navNode), children: filterChildren(navNode.children), - href: getNavigationNodeHref(navNode), }; serialized.renderAs = getRenderAs(serialized); @@ -130,7 +133,7 @@ const renderBlockTitle: ( const renderGroup = ( navGroup: ChromeProjectNavigationNode, - groupItems: Array, + groupItems: Array, { spaceBefore = DEFAULT_SPACE_BETWEEN_LEVEL_1_GROUPS }: { spaceBefore?: EuiThemeSize | null } = {} ): Required['items'] => { let itemPrepend: EuiCollapsibleNavItemProps | EuiCollapsibleNavSubItemProps | null = null; @@ -163,26 +166,30 @@ const nodeToEuiCollapsibleNavProps = ( closePanel, isSideNavCollapsed, treeDepth, - itemsState, + itemsAccordionState, + activeNodes, }: { navigateToUrl: NavigateToUrlFn; openPanel: PanelContext['open']; closePanel: PanelContext['close']; isSideNavCollapsed: boolean; treeDepth: number; - itemsState: AccordionItemsState; + itemsAccordionState: AccordionItemsState; + activeNodes: ChromeProjectNavigationNode[][]; } ): { - items: Array; + items: Array; isVisible: boolean; } => { const { navNode, isItem, hasChildren, hasLink } = serializeNavNode(_navNode); + const isActive = isActiveFromUrl(navNode.path, activeNodes); - const { id, title, href, icon, renderAs, isActive, spaceBefore: _spaceBefore } = navNode; + const { id, path, title, href, icon, renderAs, spaceBefore: _spaceBefore } = navNode; const isExternal = Boolean(href) && isAbsoluteLink(href!); const isAccordion = hasChildren && !isItem; - const isAccordionExpanded = (itemsState[id]?.isCollapsed ?? DEFAULT_IS_COLLAPSED) === false; + const isAccordionExpanded = + (itemsAccordionState[path]?.isCollapsed ?? DEFAULT_IS_COLLAPSED) === false; const isSelected = isAccordion && isAccordionExpanded ? false : isActive; const dataTestSubj = getTestSubj(navNode, isSelected); @@ -195,9 +202,15 @@ const nodeToEuiCollapsibleNavProps = ( } if (renderAs === 'panelOpener') { - const items: EuiCollapsibleNavSubItemProps[] = [ + const items: EuiCollapsibleNavSubItemPropsEnhanced[] = [ { - renderItem: () => , + renderItem: () => ( + + ), }, ]; if (spaceBefore) { @@ -227,7 +240,8 @@ const nodeToEuiCollapsibleNavProps = ( closePanel, isSideNavCollapsed, treeDepth: treeDepth + 1, - itemsState, + itemsAccordionState, + activeNodes, }) ) .filter(({ isVisible }) => isVisible) @@ -264,9 +278,21 @@ const nodeToEuiCollapsibleNavProps = ( // Render as an accordion or a link (handled by EUI) depending if // "items" is undefined or not. If it is undefined --> a link, otherwise an // accordion is rendered. - const items: Array = [ + if (navNode.renderItem) { + return { + items: [ + { + renderItem: navNode.renderItem, + }, + ], + isVisible: true, + }; + } + + const items: Array = [ { id, + path, title, isSelected, linkProps, @@ -317,19 +343,22 @@ interface Props { navNode: ChromeProjectNavigationNode; } -export const NavigationSectionUI: FC = ({ navNode }) => { +export const NavigationSectionUI: FC = React.memo(({ navNode: _navNode }) => { + const { activeNodes } = useNavigation(); const { navigateToUrl, isSideNavCollapsed } = useServices(); + + const { navNode } = useMemo(() => serializeNavNode(_navNode), [_navNode]); const { open: openPanel, close: closePanel } = usePanel(); const navNodesById = useMemo(() => { const byId = { - [nodePathToString(navNode)]: navNode, + [navNode.path]: navNode, }; const parse = (navNodes?: ChromeProjectNavigationNode[]) => { if (!navNodes) return; navNodes.forEach((childNode) => { - byId[nodePathToString(childNode)] = childNode; + byId[childNode.path] = childNode; parse(childNode.children); }); }; @@ -338,13 +367,21 @@ export const NavigationSectionUI: FC = ({ navNode }) => { return byId; }, [navNode]); - const [itemsState, setItemsState] = useState(() => { + const [itemsAccordionState, setItemsAccordionState] = useState(() => { return Object.entries(navNodesById).reduce((acc, [_id, node]) => { if (node.children) { + let isCollapsed = DEFAULT_IS_COLLAPSED; + let doCollapseFromActiveState = true; + + if (node.defaultIsCollapsed !== undefined) { + isCollapsed = node.defaultIsCollapsed; + doCollapseFromActiveState = false; + } + acc[_id] = { - isCollapsed: !node.isActive ?? DEFAULT_IS_COLLAPSED, + isCollapsed, isCollapsible: node.isCollapsible ?? DEFAULT_IS_COLLAPSIBLE, - doCollapseFromActiveState: true, + doCollapseFromActiveState, }; } return acc; @@ -354,7 +391,8 @@ export const NavigationSectionUI: FC = ({ navNode }) => { const [subItems, setSubItems] = useState(); const toggleAccordion = useCallback((id: string) => { - setItemsState((prev) => { + setItemsAccordionState((prev) => { + // if (prev[id]?.isCollapsed === undefined) return prev; const prevValue = prev[id]?.isCollapsed ?? DEFAULT_IS_COLLAPSED; return { ...prev, @@ -367,13 +405,15 @@ export const NavigationSectionUI: FC = ({ navNode }) => { }); }, []); - const setAccordionProps = useCallback( + const getAccordionProps = useCallback( ( id: string, _accordionProps?: Partial ): Partial | undefined => { - const isCollapsed = itemsState[id]?.isCollapsed ?? DEFAULT_IS_COLLAPSED; - const isCollapsible = itemsState[id]?.isCollapsible ?? DEFAULT_IS_COLLAPSIBLE; + const isCollapsed = itemsAccordionState[id]?.isCollapsed; + const isCollapsible = itemsAccordionState[id]?.isCollapsible; + + if (isCollapsed === undefined) return _accordionProps; // No state set yet let forceState: EuiAccordionProps['forceState'] = isCollapsed ? 'closed' : 'open'; if (!isCollapsible) forceState = 'open'; // Allways open if the accordion is not collapsible @@ -394,7 +434,7 @@ export const NavigationSectionUI: FC = ({ navNode }) => { return updated; }, - [itemsState, toggleAccordion] + [itemsAccordionState, toggleAccordion] ); const { items, isVisible } = useMemo(() => { @@ -404,9 +444,18 @@ export const NavigationSectionUI: FC = ({ navNode }) => { closePanel, isSideNavCollapsed, treeDepth: 0, - itemsState, + itemsAccordionState, + activeNodes, }); - }, [closePanel, isSideNavCollapsed, navNode, navigateToUrl, openPanel, itemsState]); + }, [ + navNode, + navigateToUrl, + openPanel, + closePanel, + isSideNavCollapsed, + itemsAccordionState, + activeNodes, + ]); const [props] = items; const { items: accordionItems } = props; @@ -416,47 +465,74 @@ export const NavigationSectionUI: FC = ({ navNode }) => { } /** - * Effect to set our internal state of each of the accordions (isCollapsed) based on the - * "isActive" state of the navNode. + * Effect to set the internal state of each of the accordions (isCollapsed) based on the + * "isActive" state of the navNode or if its path matches the URL location */ useEffect(() => { - setItemsState((prev) => { - return Object.entries(navNodesById).reduce((acc, [_id, node]) => { - if (node.children && (!prev[_id] || prev[_id].doCollapseFromActiveState)) { - acc[_id] = { - isCollapsed: !node.isActive ?? DEFAULT_IS_COLLAPSED, - isCollapsible: node.isCollapsible ?? DEFAULT_IS_COLLAPSIBLE, - doCollapseFromActiveState: true, - }; - } - return acc; - }, prev); + setItemsAccordionState((prev) => { + return Object.entries(navNodesById).reduce( + (acc, [_id, node]) => { + const prevState = prev[_id]; + + if ( + node.children && + node.renderAs !== 'item' && + (!prevState || prevState.doCollapseFromActiveState === true) + ) { + let nextIsActive = false; + let doCollapseFromActiveState = true; + + if (!prevState && node.defaultIsCollapsed !== undefined) { + nextIsActive = !node.defaultIsCollapsed; + doCollapseFromActiveState = false; + } else { + if (prevState?.doCollapseFromActiveState !== false) { + nextIsActive = isActiveFromUrl(node.path, activeNodes); + } else if (nextIsActive === undefined) { + nextIsActive = !DEFAULT_IS_COLLAPSED; + } + } + + acc[_id] = { + ...prevState, + isCollapsed: !nextIsActive, + isCollapsible: node.isCollapsible ?? DEFAULT_IS_COLLAPSIBLE, + doCollapseFromActiveState, + }; + } + return acc; + }, + { ...prev } + ); }); - }, [navNodesById]); + }, [navNodesById, activeNodes]); useEffect(() => { // Serializer to add recursively the accordionProps to each of the items // that will control its "open"/"closed" state + handler to toggle the state. const serializeAccordionItems = ( - _items?: EuiCollapsibleNavSubItemProps[] + _items?: EuiCollapsibleNavSubItemPropsEnhanced[] ): EuiCollapsibleNavSubItemProps[] | undefined => { if (!_items) return; - return _items.map((item: EuiCollapsibleNavSubItemProps) => { + return _items.map((item) => { if (item.renderItem) { return item; } const parsed: EuiCollapsibleNavSubItemProps = { ...item, items: serializeAccordionItems(item.items), - accordionProps: setAccordionProps(item.id!, item.accordionProps), + accordionProps: + item.items !== undefined + ? getAccordionProps(item.path ?? item.id!, item.accordionProps) + : undefined, }; return parsed; }); }; setSubItems(serializeAccordionItems(accordionItems)); - }, [accordionItems, setAccordionProps]); + }, [accordionItems, getAccordionProps]); if (!isVisible) { return null; @@ -467,7 +543,7 @@ export const NavigationSectionUI: FC = ({ navNode }) => { {...props} className={className} items={subItems} - accordionProps={setAccordionProps(navNode.id)} + accordionProps={getAccordionProps(navNode.path)} /> ); -}; +}); diff --git a/packages/shared-ux/chrome/navigation/src/ui/components/panel/context.tsx b/packages/shared-ux/chrome/navigation/src/ui/components/panel/context.tsx index f9720a38a6655..7eee3515c49c3 100644 --- a/packages/shared-ux/chrome/navigation/src/ui/components/panel/context.tsx +++ b/packages/shared-ux/chrome/navigation/src/ui/components/panel/context.tsx @@ -9,7 +9,6 @@ import React, { type FC, useCallback, useContext, useMemo, useState, ReactNode } from 'react'; import type { ChromeProjectNavigationNode } from '@kbn/core-chrome-browser'; -import { nodePathToString } from '../../../utils'; import { DefaultContent } from './default_content'; import { ContentProvider, PanelNavNode } from './types'; @@ -54,7 +53,7 @@ export const PanelProvider: FC = ({ children, contentProvider, activeNode return null; } - const provided = contentProvider?.(nodePathToString(selectedNode)); + const provided = contentProvider?.(selectedNode.path); if (!provided) { return ; diff --git a/packages/shared-ux/chrome/navigation/src/ui/components/panel/default_content.tsx b/packages/shared-ux/chrome/navigation/src/ui/components/panel/default_content.tsx index a678bad3aeaee..e3a45c7602257 100644 --- a/packages/shared-ux/chrome/navigation/src/ui/components/panel/default_content.tsx +++ b/packages/shared-ux/chrome/navigation/src/ui/components/panel/default_content.tsx @@ -39,7 +39,7 @@ function serializeChildren(node: PanelNavNode): ChromeProjectNavigationNode[] | { id: 'root', title: '', - path: [...node.path, 'root'], + path: `${node.path}.root`, children: [...node.children], }, ]; diff --git a/packages/shared-ux/chrome/navigation/src/ui/components/panel/navigation_panel.tsx b/packages/shared-ux/chrome/navigation/src/ui/components/panel/navigation_panel.tsx index ca118249d123b..ff79824176268 100644 --- a/packages/shared-ux/chrome/navigation/src/ui/components/panel/navigation_panel.tsx +++ b/packages/shared-ux/chrome/navigation/src/ui/components/panel/navigation_panel.tsx @@ -38,7 +38,9 @@ export const NavigationPanel: FC = () => { ({ target }: Event) => { // Only close if we are not clicking on the currently selected nav node if ( - !(target as HTMLButtonElement).dataset.testSubj?.includes(`panelOpener-${selectedNode?.id}`) + !(target as HTMLButtonElement).dataset.testSubj?.includes( + `panelOpener-${selectedNode?.path}` + ) ) { close(); } diff --git a/packages/shared-ux/chrome/navigation/src/ui/default_navigation.tsx b/packages/shared-ux/chrome/navigation/src/ui/default_navigation.tsx index f423ba98d04a7..e7d20a812d58f 100644 --- a/packages/shared-ux/chrome/navigation/src/ui/default_navigation.tsx +++ b/packages/shared-ux/chrome/navigation/src/ui/default_navigation.tsx @@ -8,9 +8,8 @@ import React, { FC, useCallback, useMemo } from 'react'; import { i18n } from '@kbn/i18n'; -import type { AppDeepLinkId, NodeDefinition } from '@kbn/core-chrome-browser'; +import type { NodeDefinition } from '@kbn/core-chrome-browser'; -import { getNavigationNodeId } from '../utils'; import { Navigation } from './components'; import type { GroupDefinition, @@ -116,55 +115,12 @@ const getDefaultNavigationTree = ( }; }; -/** - * Serialize a navigation node. Currently this handler only adds an autogenerated id if it's missing - * - * @param item The navigation node - * @returns The navigation node serialized - */ -function serializeNode< - T extends { - id?: string; - link?: LinkId; - children?: Array<{ id?: string; link?: LinkId }>; - }, - LinkId extends AppDeepLinkId = AppDeepLinkId ->(item: T, depth: number, index: number): T & { id: string } { - const id = getNavigationNodeId(item, () => `node-${depth}-${index}`); - const children = item.children?.map((_item, i) => serializeNode(_item, depth + 1, i)); - - return { - ...item, - id, - children, - }; -} - -const serializeNavigationTree = (navTree: NavigationTreeDefinition): NavigationTreeDefinition => { - const serialized: NavigationTreeDefinition = { ...navTree }; - - const serialize = (item: RootNavigationItemDefinition, index: number) => { - if (item.type === 'recentlyAccessed') return item; - return serializeNode(item, 0, index); - }; - - if (navTree.body) { - serialized.body = navTree.body.map(serialize); - } - - if (navTree.footer) { - serialized.footer = navTree.footer.map(serialize); - } - - return serialized; -}; - interface Props { dataTestSubj?: string; panelContentProvider?: ContentProvider; } -export const DefaultNavigation: FC = ({ +const DefaultNavigationComp: FC = ({ projectNavigationTree, navigationTree, dataTestSubj, @@ -174,14 +130,6 @@ export const DefaultNavigation: FC = ({ throw new Error('One of navigationTree or projectNavigationTree must be defined'); } - const navigationDefinition = useMemo(() => { - const definition = !navigationTree - ? getDefaultNavigationTree(projectNavigationTree!) - : navigationTree; - - return serializeNavigationTree(definition); - }, [navigationTree, projectNavigationTree]); - const renderNodes = useCallback( (nodes: Array = []) => { return nodes.map((navNode, i) => { @@ -194,28 +142,35 @@ export const DefaultNavigation: FC = ({ } if (isGroupDefinition(navNode)) { + // Recursively build the tree return ( - - {/* Recursively build the tree */} + {renderNodes(navNode.children)} ); } - return ; + return ; }); }, [] ); + const definitionToJSX = useMemo(() => { + const definition = !navigationTree + ? getDefaultNavigationTree(projectNavigationTree!) + : navigationTree; + + const { body, footer } = definition; + return { body: renderNodes(body), footer: Boolean(footer) ? renderNodes(footer) : null }; + }, [navigationTree, projectNavigationTree, renderNodes]); + return ( - <> - {renderNodes(navigationDefinition.body)} - {navigationDefinition.footer && ( - {renderNodes(navigationDefinition.footer)} - )} - + {definitionToJSX.body} + {definitionToJSX.footer && {definitionToJSX.footer}} ); }; + +export const DefaultNavigation = React.memo(DefaultNavigationComp) as typeof DefaultNavigationComp; diff --git a/packages/shared-ux/chrome/navigation/src/ui/hooks/index.ts b/packages/shared-ux/chrome/navigation/src/ui/hooks/index.ts deleted file mode 100644 index 631ad5f590ce4..0000000000000 --- a/packages/shared-ux/chrome/navigation/src/ui/hooks/index.ts +++ /dev/null @@ -1,9 +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 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -export { useInitNavNode } from './use_init_navnode'; diff --git a/packages/shared-ux/chrome/navigation/src/ui/hooks/use_init_navnode.ts b/packages/shared-ux/chrome/navigation/src/ui/hooks/use_init_navnode.ts deleted file mode 100644 index f8d56bc785269..0000000000000 --- a/packages/shared-ux/chrome/navigation/src/ui/hooks/use_init_navnode.ts +++ /dev/null @@ -1,356 +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 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; -import useObservable from 'react-use/lib/useObservable'; -import type { - AppDeepLinkId, - ChromeNavLink, - ChromeProjectNavigationNode, - CloudLinkId, - SideNavNodeStatus, -} from '@kbn/core-chrome-browser'; -import { CloudLinks } from '../../cloud_links'; - -import { useNavigation as useNavigationServices } from '../../services'; -import { getNavigationNodeId, isAbsoluteLink } from '../../utils'; -import { useNavigation } from '../components/navigation'; -import { NodePropsEnhanced, RegisterFunction, UnRegisterFunction } from '../types'; -import { useRegisterTreeNode } from './use_register_tree_node'; - -/** - * We don't have currently a way to know if a user has access to a Cloud section. - * TODO: This function will have to be revisited once we have an API from Cloud to know the user - * permissions. - */ -function hasUserAccessToCloudLink(): boolean { - return true; -} - -function getNodeStatus( - { - link, - deepLink, - cloudLink, - sideNavStatus, - }: { - link?: string; - deepLink?: ChromeNavLink; - cloudLink?: CloudLinkId; - sideNavStatus?: SideNavNodeStatus; - }, - { cloudLinks }: { cloudLinks: CloudLinks } -): SideNavNodeStatus | 'remove' { - if (link && !deepLink) { - // If a link is provided, but no deepLink is found, don't render anything - return 'remove'; - } - - if (cloudLink) { - if (!cloudLinks[cloudLink]) { - // Invalid cloudLinkId or link url has not been set in kibana.yml - return 'remove'; - } - if (!hasUserAccessToCloudLink()) return 'remove'; - } - - if (deepLink && deepLink.hidden) return 'hidden'; - - return sideNavStatus ?? 'visible'; -} - -function getTitleForNode< - LinkId extends AppDeepLinkId = AppDeepLinkId, - Id extends string = string, - ChildrenId extends string = Id ->( - navNode: NodePropsEnhanced, - { deepLink, cloudLinks }: { deepLink?: ChromeNavLink; cloudLinks: CloudLinks } -): string { - const { children } = navNode; - if (navNode.title) { - return navNode.title; - } - - if (typeof children === 'string') { - return children; - } - - if (deepLink?.title) { - return deepLink.title; - } - - if (navNode.cloudLink) { - return cloudLinks[navNode.cloudLink]?.title ?? ''; - } - - return ''; -} - -function validateNodeProps< - LinkId extends AppDeepLinkId = AppDeepLinkId, - Id extends string = string, - ChildrenId extends string = Id ->({ - id, - link, - href, - cloudLink, - renderAs, - appendHorizontalRule, - isGroup, -}: Omit, 'children'>) { - if (link && cloudLink) { - throw new Error( - `[Chrome navigation] Error in node [${id}]. Only one of "link" or "cloudLink" can be provided.` - ); - } - if (href && cloudLink) { - throw new Error( - `[Chrome navigation] Error in node [${id}]. Only one of "href" or "cloudLink" can be provided.` - ); - } - if (renderAs === 'panelOpener' && !link) { - throw new Error( - `[Chrome navigation] Error in node [${id}]. If renderAs is set to "panelOpener", a "link" must also be provided.` - ); - } - if (renderAs === 'item' && !link) { - throw new Error( - `[Chrome navigation] Error in node [${id}]. If renderAs is set to "item", a "link" must also be provided.` - ); - } - if (appendHorizontalRule && !isGroup) { - throw new Error( - `[Chrome navigation] Error in node [${id}]. "appendHorizontalRule" can only be added for group with children.` - ); - } -} - -function createInternalNavNode< - LinkId extends AppDeepLinkId = AppDeepLinkId, - Id extends string = string, - ChildrenId extends string = Id ->( - id: string, - _navNode: NodePropsEnhanced, - deepLinks: Readonly, - path: string[] | null, - isActive: boolean, - { cloudLinks }: { cloudLinks: CloudLinks } -): ChromeProjectNavigationNode | null { - validateNodeProps(_navNode); - - const { children, link, cloudLink, ...navNode } = _navNode; - const deepLink = deepLinks.find((dl) => dl.id === link); - const sideNavStatus = getNodeStatus( - { - link, - deepLink, - cloudLink, - sideNavStatus: navNode.sideNavStatus, - }, - { cloudLinks } - ); - const title = getTitleForNode(_navNode, { deepLink, cloudLinks }); - const href = cloudLink ? cloudLinks[cloudLink]?.href : _navNode.href; - - if (href && !isAbsoluteLink(href)) { - throw new Error(`href must be an absolute URL. Node id [${id}].`); - } - - if (sideNavStatus === 'remove') { - return null; - } - - return { - ...navNode, - id, - path: path ?? [], - title: title ?? '', - deepLink, - href, - isActive, - sideNavStatus, - }; -} - -function isSamePath(pathA: string[] | null, pathB: string[] | null) { - if (pathA === null || pathB === null) { - return false; - } - const pathAToString = pathA.join('.'); - const pathBToString = pathB.join('.'); - return pathAToString === pathBToString; -} - -export const useInitNavNode = < - LinkId extends AppDeepLinkId = AppDeepLinkId, - Id extends string = string, - ChildrenId extends string = Id ->( - node: Omit, 'children'>, - { cloudLinks }: { cloudLinks: CloudLinks } -) => { - const { isActive: isActiveControlled } = node; - - /** - * Map of children nodes - */ - const [childrenNodes, setChildrenNodes] = useState>( - {} - ); - - const isMounted = useRef(false); - - /** - * Reference to the unregister function - */ - const unregisterRef = useRef(); - - /** - * Map to keep track of the order of the children when they mount. - * This allows us to keep in sync the nav tree sent to the Chrome service - * with the order of the DOM elements - */ - const orderChildrenRef = useRef>({}); - - /** - * Index to keep track of the order of the children when they mount. - */ - const idx = useRef(0); - - /** - * The current node path, including all of its parents. We'll use it to match it against - * the list of active routes based on current URL location (passed by the Chrome service) - */ - const [nodePath, setNodePath] = useState(null); - const [isActiveState, setIsActive] = useState(false); - const isActive = isActiveControlled ?? isActiveState; - - const { navLinks$ } = useNavigationServices(); - const deepLinks = useObservable(navLinks$, []); - const { register: registerNodeOnParent } = useRegisterTreeNode(); - const { activeNodes } = useNavigation(); - - const id = getNavigationNodeId(node); - - const internalNavNode = useMemo( - () => createInternalNavNode(id, node, deepLinks, nodePath, isActive, { cloudLinks }), - [node, id, deepLinks, nodePath, isActive, cloudLinks] - ); - - // Register the node on the parent whenever its properties change or whenever - // a child node is registered. - const register = useCallback(() => { - if (!internalNavNode) { - return; - } - - const children = Object.values(childrenNodes).sort((a, b) => { - const aOrder = orderChildrenRef.current[a.id]; - const bOrder = orderChildrenRef.current[b.id]; - return aOrder - bOrder; - }); - - const { unregister, path } = registerNodeOnParent({ - ...internalNavNode, - children: children.length ? children : undefined, - }); - - setNodePath((prev) => { - if (!isSamePath(prev, path)) { - return path; - } - return prev; - }); - - unregisterRef.current = unregister; - }, [internalNavNode, childrenNodes, registerNodeOnParent]); - - // Un-register from the parent. This will happen when the node is unmounted or if the deeplink - // is not active anymore. - const unregister = useCallback(() => { - if (unregisterRef.current) { - unregisterRef.current(id); - unregisterRef.current = undefined; - } - }, [id]); - - const registerChildNode = useCallback( - (childNode) => { - if (orderChildrenRef.current[childNode.id] === undefined) { - orderChildrenRef.current[childNode.id] = idx.current++; - } - - const childPath = nodePath ? [...nodePath, childNode.id] : []; - - setChildrenNodes((prev) => { - return { - ...prev, - [childNode.id]: { - ...childNode, - path: childPath, - }, - }; - }); - - return { - unregister: (childId: string) => { - setChildrenNodes((prev) => { - const updatedItems = { ...prev }; - delete updatedItems[childId]; - return updatedItems; - }); - }, - path: childPath, - }; - }, - [nodePath] - ); - - useEffect(() => { - const updatedIsActive = activeNodes.reduce((acc, nodesBranch) => { - return acc === true ? acc : nodesBranch.some((_node) => isSamePath(_node.path, nodePath)); - }, false); - - setIsActive(updatedIsActive); - }, [activeNodes, nodePath]); - - /** Register when mounting and whenever the internal nav node changes */ - useEffect(() => { - if (!isMounted.current) { - return; - } - - if (internalNavNode) { - register(); - } else { - unregister(); - } - }, [unregister, register, internalNavNode]); - - /** Unregister when unmounting */ - useEffect(() => { - isMounted.current = true; - return () => { - isMounted.current = false; - unregister(); - }; - }, [unregister]); - - return useMemo( - () => ({ - navNode: internalNavNode, - path: nodePath, - registerChildNode, - childrenNodes, - }), - [internalNavNode, registerChildNode, nodePath, childrenNodes] - ); -}; diff --git a/packages/shared-ux/chrome/navigation/src/ui/hooks/use_register_tree_node.ts b/packages/shared-ux/chrome/navigation/src/ui/hooks/use_register_tree_node.ts deleted file mode 100644 index 5ad690b2de2e1..0000000000000 --- a/packages/shared-ux/chrome/navigation/src/ui/hooks/use_register_tree_node.ts +++ /dev/null @@ -1,29 +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 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -import { useMemo } from 'react'; - -import { useNavigation } from '../components/navigation'; -import { useNavigationGroup } from '../components/navigation_group'; - -/** - * Helper hook that will proxy the correct "register" handler. - * It first tries to the closest parent group, if not found it will use the root register. - */ -export const useRegisterTreeNode = () => { - const root = useNavigation(); - const group = useNavigationGroup(false); - const register = group ? group.register : root.register; - - return useMemo( - () => ({ - register, - }), - [register] - ); -}; diff --git a/packages/shared-ux/chrome/navigation/src/ui/navigation.stories.tsx b/packages/shared-ux/chrome/navigation/src/ui/navigation.stories.tsx index be749a5cac9bb..def69d3c9df52 100644 --- a/packages/shared-ux/chrome/navigation/src/ui/navigation.stories.tsx +++ b/packages/shared-ux/chrome/navigation/src/ui/navigation.stories.tsx @@ -7,26 +7,22 @@ */ import { action } from '@storybook/addon-actions'; -import { useState as useStateStorybook } from '@storybook/addons'; import { ComponentMeta } from '@storybook/react'; import React, { EventHandler, FC, MouseEvent, useState, useEffect } from 'react'; -import { BehaviorSubject, of } from 'rxjs'; +import { of } from 'rxjs'; import { EuiButton, EuiCollapsibleNavBeta, EuiCollapsibleNavBetaProps, - EuiFlexGroup, - EuiFlexItem, EuiHeader, EuiHeaderSection, EuiLink, EuiPageTemplate, EuiText, - EuiTitle, } from '@elastic/eui'; -import type { ChromeNavLink, ChromeProjectNavigationNode } from '@kbn/core-chrome-browser'; +import type { ChromeNavLink } from '@kbn/core-chrome-browser'; import { NavigationStorybookMock, navLinksMock } from '../../mocks'; import mdx from '../../README.mdx'; import type { NavigationServices } from '../../types'; @@ -34,7 +30,7 @@ import { NavigationProvider } from '../services'; import { Navigation } from './components'; import { DefaultNavigation } from './default_navigation'; import { getPresets } from './nav_tree_presets'; -import type { GroupDefinition, ProjectNavigationDefinition } from './types'; +import type { ProjectNavigationDefinition } from './types'; import { ContentProvider } from './components/panel'; const storybookMock = new NavigationStorybookMock(); @@ -127,6 +123,13 @@ const deepLinks: ChromeNavLink[] = [ createDeepLink('group:settings.tracing'), ]; +const deepLinks$ = of({ + ...[...navLinksMock, ...deepLinks].reduce>((acc, navLink) => { + acc[navLink.id] = navLink; + return acc; + }, {}), +}); + const simpleNavigationDefinition: ProjectNavigationDefinition = { projectNavigationTree: [ { @@ -182,7 +185,7 @@ const simpleNavigationDefinition: ProjectNavigationDefinition = { export const SimpleObjectDefinition = (args: NavigationServices) => { const services = storybookMock.getServices({ ...args, - navLinks$: of([...navLinksMock, ...deepLinks]), + deepLinks$, onProjectNavigationChange: (updated) => { action('Update chrome navigation')(JSON.stringify(updated, null, 2)); }, @@ -330,7 +333,7 @@ const groupExamplesDefinition: ProjectNavigationDefinition = { export const GroupsExamples = (args: NavigationServices) => { const services = storybookMock.getServices({ ...args, - navLinks$: of([...navLinksMock, ...deepLinks]), + deepLinks$, onProjectNavigationChange: (updated) => { action('Update chrome navigation')(JSON.stringify(updated, null, 2)); }, @@ -342,9 +345,11 @@ export const GroupsExamples = (args: NavigationServices) => { return ( - - - + {({ isCollapsed }) => ( + + + + )} ); }; @@ -401,6 +406,7 @@ const navigationDefinition: ProjectNavigationDefinition = { { id: 'group:settings-2', title: 'Settings as nav Item', + link: 'item1', renderAs: 'item', // Render just like any other item, even if it has children children: [ { @@ -520,6 +526,7 @@ const navigationDefinition: ProjectNavigationDefinition = { id: 'test_all_hidden', title: 'Test group render as Item', renderAs: 'item', + link: 'item1', children: [ { id: 'test.item1', @@ -576,7 +583,7 @@ const navigationDefinition: ProjectNavigationDefinition = { export const ComplexObjectDefinition = (args: NavigationServices) => { const services = storybookMock.getServices({ ...args, - navLinks$: of([...navLinksMock, ...deepLinks]), + deepLinks$, onProjectNavigationChange: (updated) => { action('Update chrome navigation')(JSON.stringify(updated, null, 2)); }, @@ -891,6 +898,7 @@ const navigationDefinitionWithPanel: ProjectNavigationDefinition = { { title: 'Group renders as "item" (2)', id: 'group2.renderAsItem', + link: 'item1', renderAs: 'item', children: [ { @@ -945,6 +953,7 @@ const navigationDefinitionWithPanel: ProjectNavigationDefinition = { children: [ { id: 'group2-B', + link: 'item1', title: 'Group renders as "item" (3)', renderAs: 'item', // This group renders as a normal item children: [ @@ -978,6 +987,7 @@ const navigationDefinitionWithPanel: ProjectNavigationDefinition = { }, { title: 'Yet another group as item', + link: 'item1', renderAs: 'item', children: [ { @@ -1076,7 +1086,7 @@ const navigationDefinitionWithPanel: ProjectNavigationDefinition = { export const ObjectDefinitionWithPanel = (args: NavigationServices) => { const services = storybookMock.getServices({ ...args, - navLinks$: of([...navLinksMock, ...deepLinks]), + deepLinks$, onProjectNavigationChange: (updated) => { action('Update chrome navigation')(JSON.stringify(updated, null, 2)); }, @@ -1100,10 +1110,69 @@ export const ObjectDefinitionWithPanel = (args: NavigationServices) => { ); }; +export const WithUIComponentsTiny = (args: NavigationServices) => { + const services = storybookMock.getServices({ + ...args, + deepLinks$, + onProjectNavigationChange: (updated) => { + action('Update chrome navigation')(JSON.stringify(updated, null, 2)); + }, + recentlyAccessed$: of([ + { label: 'This is an example', link: '/app/example/39859', id: '39850' }, + { label: 'Another example', link: '/app/example/5235', id: '5235' }, + ]), + }); + + return ( + + {({ isCollapsed }) => ( + + + + + id="item1" link="item1" /> + + + id="item2" link="item1" title="YEAH!!" icon="launch" /> + + + + + + + + + + + + + + + )} + + ); +}; + export const WithUIComponents = (args: NavigationServices) => { const services = storybookMock.getServices({ ...args, - navLinks$: of([...navLinksMock, ...deepLinks]), + deepLinks$, onProjectNavigationChange: (updated) => { action('Update chrome navigation')(JSON.stringify(updated, null, 2)); }, @@ -1127,7 +1196,7 @@ export const WithUIComponents = (args: NavigationServices) => { defaultIsCollapsed={false} > id="item1" link="item1" /> - + {/* {(navNode) => { return (
    @@ -1135,7 +1204,7 @@ export const WithUIComponents = (args: NavigationServices) => {
    ); }} -     +     */}
    Title in ReactNode @@ -1224,7 +1293,7 @@ export const WithUIComponents = (args: NavigationServices) => { export const MinimalUI = (args: NavigationServices) => { const services = storybookMock.getServices({ ...args, - navLinks$: of([...navLinksMock, ...deepLinks]), + deepLinks$, onProjectNavigationChange: (updated) => { action('Update chrome navigation')(JSON.stringify(updated, null, 2)); }, @@ -1272,230 +1341,6 @@ export const MinimalUI = (args: NavigationServices) => { ); }; -export const CreativeUI = (args: NavigationServices) => { - const services = storybookMock.getServices({ - ...args, - navLinks$: of([...navLinksMock, ...deepLinks]), - onProjectNavigationChange: (updated) => { - action('Update chrome navigation')(JSON.stringify(updated, null, 2)); - }, - recentlyAccessed$: of([ - { label: 'This is an example', link: '/app/example/39859', id: '39850' }, - { label: 'Another example', link: '/app/example/5235', id: '5235' }, - ]), - }); - - return ( - - - - - - - - -

    Hello!

    -     - - - - -

    - As you can see there is really no limit in what UI you can create! -
    -

    -

    - Have fun! -

    -     - - -     -     -     -     -     -     - ); -}; - -export const UpdatingState = (args: NavigationServices) => { - const simpleGroupDef: GroupDefinition = { - type: 'navGroup', - id: 'observability_project_nav', - title: 'Observability', - icon: 'logoObservability', - children: [ - { - id: 'aiops', - title: 'AIOps', - icon: 'branch', - children: [ - { - title: 'Anomaly detection', - id: 'ml:anomalyDetection', - link: 'ml:anomalyDetection', - }, - { - title: 'Log Rate Analysis', - id: 'ml:logRateAnalysis', - link: 'ml:logRateAnalysis', - }, - { - title: 'Change Point Detections', - link: 'ml:changePointDetections', - id: 'ml:changePointDetections', - }, - { - title: 'Job Notifications', - link: 'ml:notifications', - id: 'ml:notifications', - }, - ], - }, - { - id: 'project_settings_project_nav', - title: 'Project settings', - icon: 'gear', - children: [ - { id: 'management', link: 'management' }, - { id: 'integrations', link: 'integrations' }, - { id: 'fleet', link: 'fleet' }, - ], - }, - ], - }; - const firstSection = simpleGroupDef.children![0]; - const firstSectionFirstChild = firstSection.children![0]; - const secondSection = simpleGroupDef.children![1]; - const secondSectionFirstChild = secondSection.children![0]; - - const activeNodeSets: ChromeProjectNavigationNode[][][] = [ - [ - [ - { - ...simpleGroupDef, - path: [simpleGroupDef.id], - } as unknown as ChromeProjectNavigationNode, - { - ...firstSection, - path: [simpleGroupDef.id, firstSection.id], - } as unknown as ChromeProjectNavigationNode, - { - ...firstSectionFirstChild, - path: [simpleGroupDef.id, firstSection.id, firstSectionFirstChild.id], - } as unknown as ChromeProjectNavigationNode, - ], - ], - [ - [ - { - ...simpleGroupDef, - path: [simpleGroupDef.id], - } as unknown as ChromeProjectNavigationNode, - { - ...secondSection, - path: [simpleGroupDef.id, secondSection.id], - } as unknown as ChromeProjectNavigationNode, - { - ...secondSectionFirstChild, - path: [simpleGroupDef.id, secondSection.id, secondSectionFirstChild.id], - } as unknown as ChromeProjectNavigationNode, - ], - ], - ]; - - // use state to track which element of activeNodeSets is active - const [activeNodeIndex, setActiveNodeIndex] = useStateStorybook(0); - const changeActiveNode = () => { - const value = (activeNodeIndex + 1) % 2; // toggle between 0 and 1 - setActiveNodeIndex(value); - }; - - const activeNodes$ = new BehaviorSubject([]); - activeNodes$.next(activeNodeSets[activeNodeIndex]); - - const services = storybookMock.getServices({ - ...args, - activeNodes$, - navLinks$: of([...navLinksMock, ...deepLinks]), - onProjectNavigationChange: (updated) => { - action('Update chrome navigation')(JSON.stringify(updated, null, 2)); - }, - }); - - return ( - - - - - - ); -}; - export default { title: 'Chrome/Navigation', description: 'Navigation container to render items for cross-app linking', diff --git a/packages/shared-ux/chrome/navigation/src/ui/types.ts b/packages/shared-ux/chrome/navigation/src/ui/types.ts index 9051cc3747bf0..768155ced3050 100644 --- a/packages/shared-ux/chrome/navigation/src/ui/types.ts +++ b/packages/shared-ux/chrome/navigation/src/ui/types.ts @@ -30,26 +30,15 @@ export interface NodeProps< * Children of the node. For Navigation.Item (only) it allows a function to be set. * This function will receive the ChromeProjectNavigationNode object */ - children?: ((navNode: ChromeProjectNavigationNode) => ReactNode) | ReactNode; -} - -/** - * @internal - * - * Internally we enhance the Props passed to the Navigation.Item component. - */ -export interface NodePropsEnhanced< - LinkId extends AppDeepLinkId = AppDeepLinkId, - Id extends string = string, - ChildrenId extends string = Id -> extends NodeProps { - /** - * Forces the node to be active. This is used to force a collapisble nav group to be open - * even if the URL does not match any of the nodes in the group. - */ - isActive?: boolean; - /** Flag to indicate if the navigation node is a group or not */ - isGroup: boolean; + children?: ReactNode; + /** @internal - Prop internally controlled, don't use it. */ + parentNodePath?: string; + /** @internal - Prop internally controlled, don't use it. */ + rootIndex?: number; + /** @internal - Prop internally controlled, don't use it. */ + treeDepth?: number; + /** @internal - Prop internally controlled, don't use it. */ + index?: number; } /** The preset that can be pass to the NavigationBucket component */ @@ -181,16 +170,14 @@ export interface ProjectNavigationDefinition< * * Function to unregister a navigation node from its parent. */ -export type UnRegisterFunction = (id: string) => void; +export type UnRegisterFunction = () => void; /** * @internal * * A function to register a navigation node on its parent. */ -export type RegisterFunction = (navNode: ChromeProjectNavigationNode) => { - /** The function to unregister the node. */ - unregister: UnRegisterFunction; - /** The full path of the node in the navigation tree. */ - path: string[]; -}; +export type RegisterFunction = ( + navNode: ChromeProjectNavigationNode, + order?: number +) => UnRegisterFunction; diff --git a/packages/shared-ux/chrome/navigation/src/utils.ts b/packages/shared-ux/chrome/navigation/src/utils.ts index 8322fe797590b..efbf3c0e2a42e 100644 --- a/packages/shared-ux/chrome/navigation/src/utils.ts +++ b/packages/shared-ux/chrome/navigation/src/utils.ts @@ -6,11 +6,16 @@ * Side Public License, v 1. */ +import React, { type ReactNode } from 'react'; import type { ChromeProjectNavigationNode, NodeDefinition } from '@kbn/core-chrome-browser'; +import { NavigationFooter } from './ui/components/navigation_footer'; +import { NavigationGroup } from './ui/components/navigation_group'; +import { NavigationItem } from './ui/components/navigation_item'; +import { RecentlyAccessed } from './ui/components/recently_accessed'; let uniqueId = 0; -function generateUniqueNodeId() { +export function generateUniqueNodeId() { const id = `node${uniqueId++}`; return id; } @@ -19,15 +24,6 @@ export function isAbsoluteLink(link: string) { return link.startsWith('http://') || link.startsWith('https://'); } -export function nodePathToString( - node?: T -): T extends { path?: string[]; id: string } ? string : undefined { - if (!node) return undefined as T extends { path?: string[]; id: string } ? string : undefined; - return (node.path ? node.path.join('.') : node.id) as T extends { path?: string[]; id: string } - ? string - : undefined; -} - export function isGroupNode({ children }: Pick) { return children !== undefined; } @@ -50,3 +46,37 @@ export function getNavigationNodeHref({ }: Pick): string | undefined { return deepLink?.url ?? href; } + +function isSamePath(pathA: string | null, pathB: string | null) { + if (pathA === null || pathB === null) { + return false; + } + return pathA === pathB; +} + +export function isActiveFromUrl(nodePath: string, activeNodes: ChromeProjectNavigationNode[][]) { + return activeNodes.reduce((acc, nodesBranch) => { + return acc === true ? acc : nodesBranch.some((branch) => isSamePath(branch.path, nodePath)); + }, false); +} + +type ChildType = 'item' | 'group' | 'recentlyAccessed' | 'footer' | 'unknown'; + +export const getChildType = (child: ReactNode): ChildType => { + if (!React.isValidElement(child)) { + return 'unknown'; + } + + switch (child.type) { + case NavigationItem: + return 'item'; + case NavigationGroup: + return 'group'; + case RecentlyAccessed: + return 'recentlyAccessed'; + case NavigationFooter: + return 'footer'; + default: + return 'unknown'; + } +}; diff --git a/packages/shared-ux/chrome/navigation/types/index.ts b/packages/shared-ux/chrome/navigation/types/index.ts index e162e395362a9..43ff4083ba414 100644 --- a/packages/shared-ux/chrome/navigation/types/index.ts +++ b/packages/shared-ux/chrome/navigation/types/index.ts @@ -24,7 +24,7 @@ import type { CloudLinks } from '../src/cloud_links'; export interface NavigationServices { basePath: BasePathService; recentlyAccessed$: Observable; - navLinks$: Observable>; + deepLinks$: Observable>>; navIsOpen: boolean; navigateToUrl: NavigateToUrlFn; onProjectNavigationChange: (chromeProjectNavigation: ChromeProjectNavigation) => void; diff --git a/x-pack/plugins/security_solution_serverless/public/navigation/navigation_tree/chrome_navigation_tree.test.ts b/x-pack/plugins/security_solution_serverless/public/navigation/navigation_tree/chrome_navigation_tree.test.ts index 5b3502225e769..74979b0549b91 100644 --- a/x-pack/plugins/security_solution_serverless/public/navigation/navigation_tree/chrome_navigation_tree.test.ts +++ b/x-pack/plugins/security_solution_serverless/public/navigation/navigation_tree/chrome_navigation_tree.test.ts @@ -119,7 +119,7 @@ describe('formatChromeProjectNavNodes', () => { { id: chromeNavLink1.id, title: link1.title, - path: [chromeNavLink1.id], + path: chromeNavLink1.id, deepLink: chromeNavLink1, }, ]); @@ -132,7 +132,7 @@ describe('formatChromeProjectNavNodes', () => { { id: chromeNavLink3.id, title: chromeNavLink3.title, - path: [chromeNavLink3.id], + path: chromeNavLink3.id, deepLink: chromeNavLink3, }, ]); @@ -145,13 +145,13 @@ describe('formatChromeProjectNavNodes', () => { { id: chromeNavLink1.id, title: link1.title, - path: [chromeNavLink1.id], + path: chromeNavLink1.id, deepLink: chromeNavLink1, children: [ { id: chromeNavLink2.id, title: link2.title, - path: [chromeNavLink1.id, chromeNavLink2.id], + path: [chromeNavLink1.id, chromeNavLink2.id].join('.'), deepLink: chromeNavLink2, }, ], @@ -173,24 +173,24 @@ describe('formatChromeProjectNavNodes', () => { { id: chromeNavLinkTest.id, title: link1.title, - path: [chromeNavLinkTest.id], + path: chromeNavLinkTest.id, deepLink: chromeNavLinkTest, children: [ { id: chromeNavLinkMl1.id, title: chromeNavLinkMl1.title, - path: [chromeNavLinkTest.id, chromeNavLinkMl1.id], + path: [chromeNavLinkTest.id, chromeNavLinkMl1.id].join('.'), deepLink: chromeNavLinkMl1, }, { id: defaultNavCategory1.id, title: defaultNavCategory1.title, - path: [chromeNavLinkTest.id, defaultNavCategory1.id], + path: [chromeNavLinkTest.id, defaultNavCategory1.id].join('.'), children: [ { id: chromeNavLinkMl2.id, title: 'Overridden ML SubLink 2', - path: [chromeNavLinkTest.id, defaultNavCategory1.id, chromeNavLinkMl2.id], + path: [chromeNavLinkTest.id, defaultNavCategory1.id, chromeNavLinkMl2.id].join('.'), deepLink: chromeNavLinkMl2, }, ], @@ -208,7 +208,7 @@ describe('formatChromeProjectNavNodes', () => { { id: chromeNavLink2.id, title: link2.title, - path: [chromeNavLink2.id], + path: chromeNavLink2.id, deepLink: chromeNavLink2, }, ]); @@ -230,14 +230,14 @@ describe('formatChromeProjectNavNodes', () => { { id: chromeNavLinkTest.id, title: link1.title, - path: [chromeNavLinkTest.id], + path: chromeNavLinkTest.id, deepLink: chromeNavLinkTest, breadcrumbStatus: 'hidden', }, { id: chromeNavLink2.id, title: link2.title, - path: [chromeNavLink2.id], + path: chromeNavLink2.id, deepLink: chromeNavLink2, }, ]); diff --git a/x-pack/plugins/security_solution_serverless/public/navigation/navigation_tree/chrome_navigation_tree.ts b/x-pack/plugins/security_solution_serverless/public/navigation/navigation_tree/chrome_navigation_tree.ts index 0dc8a1140aaab..b787aeb927361 100644 --- a/x-pack/plugins/security_solution_serverless/public/navigation/navigation_tree/chrome_navigation_tree.ts +++ b/x-pack/plugins/security_solution_serverless/public/navigation/navigation_tree/chrome_navigation_tree.ts @@ -19,7 +19,7 @@ import { isBreadcrumbHidden } from './utils'; export const getFormatChromeProjectNavNodes = (services: Services) => { const formatChromeProjectNavNodes = ( projectNavLinks: ProjectNavigationLink[], - path: string[] = [] + path?: string ): ChromeProjectNavigationNode[] => { const { chrome } = services; @@ -31,7 +31,7 @@ export const getFormatChromeProjectNavNodes = (services: Services) => { const link: ChromeProjectNavigationNode = { id: navLinkId, title, - path: [...path, navLinkId], + path: path ? [path, navLinkId].join('.') : navLinkId, deepLink: chrome.navLinks.get(navLinkId), ...(isBreadcrumbHidden(id) && { breadcrumbStatus: 'hidden' }), }; @@ -63,7 +63,7 @@ export const getFormatChromeProjectNavNodes = (services: Services) => { const processDefaultNav = ( children: NodeDefinition[], - path: string[] + path: string ): ChromeProjectNavigationNode[] => { const { chrome } = services; return children.reduce((navNodes, node) => { @@ -80,7 +80,7 @@ export const getFormatChromeProjectNavNodes = (services: Services) => { const navNode: ChromeProjectNavigationNode = { id, title: node.title || '', - path: [...path, id], + path: [path, id].join('.'), breadcrumbStatus: node.breadcrumbStatus, getIsActive: node.getIsActive, }; diff --git a/x-pack/plugins/security_solution_serverless/public/navigation/project_navigation/project_navigation.tsx b/x-pack/plugins/security_solution_serverless/public/navigation/project_navigation/project_navigation.tsx index b26700eb8e4b3..2532589d1c994 100644 --- a/x-pack/plugins/security_solution_serverless/public/navigation/project_navigation/project_navigation.tsx +++ b/x-pack/plugins/security_solution_serverless/public/navigation/project_navigation/project_navigation.tsx @@ -13,7 +13,7 @@ import type { import { SolutionSideNavPanelContent } from '@kbn/security-solution-side-nav/panel'; import useObservable from 'react-use/lib/useObservable'; import { useKibana } from '../../common/services'; -import type { ProjectNavigationLink, ProjectPageName } from '../links/types'; +import type { ProjectNavigationLink } from '../links/types'; import { useFormattedSideNavItems } from '../side_navigation/use_side_nav_items'; import { CATEGORIES, FOOTER_CATEGORIES } from '../categories'; import { formatNavigationTree } from '../navigation_tree/navigation_tree'; @@ -21,8 +21,7 @@ import { formatNavigationTree } from '../navigation_tree/navigation_tree'; const getPanelContentProvider = ( projectNavLinks: ProjectNavigationLink[] ): React.FC => - React.memo(function PanelContentProvider({ selectedNode: { path }, closePanel }) { - const linkId = path[path.length - 1] as ProjectPageName; + React.memo(function PanelContentProvider({ selectedNode: { id: linkId }, closePanel }) { const currentPanelItem = projectNavLinks.find((item) => item.id === linkId); const { title = '', links = [], categories } = currentPanelItem ?? {}; diff --git a/x-pack/test_serverless/functional/services/ml/observability_navigation.ts b/x-pack/test_serverless/functional/services/ml/observability_navigation.ts index 3dd18587a9140..91c149dab37ac 100644 --- a/x-pack/test_serverless/functional/services/ml/observability_navigation.ts +++ b/x-pack/test_serverless/functional/services/ml/observability_navigation.ts @@ -16,10 +16,10 @@ export function MachineLearningNavigationProviderObservability({ async function navigateToArea(id: string) { await svlCommonNavigation.sidenav.openSection('observability_project_nav.aiops'); - await testSubjects.existOrFail(`~nav-item-id-observability_project_nav.aiops.ml:${id}`, { + await testSubjects.existOrFail(`~nav-item-observability_project_nav.aiops.ml:${id}`, { timeout: 60 * 1000, }); - await testSubjects.click(`~nav-item-id-observability_project_nav.aiops.ml:${id}`); + await testSubjects.click(`~nav-item-observability_project_nav.aiops.ml:${id}`); } return { From 903149af5f8dd4cd1a799443e2347710758307c2 Mon Sep 17 00:00:00 2001 From: Antonio Date: Mon, 27 Nov 2023 14:02:17 +0100 Subject: [PATCH 6/9] [ResponseOps][Connectors] Pager duty connector UI (#171748) Fixes #170048 ## Summary This PR adds support in the `UI` for the `custom_details` and links attributes in the Pagerduty connector. ### Release Notes PagerDuty connector now supports the links and custom_details attributes. --- .../pagerduty/links_list.test.tsx | 92 ++++++++++++ .../connector_types/pagerduty/links_list.tsx | 138 +++++++++++++++++ .../pagerduty/pagerduty.test.tsx | 140 ++++++++++++++++++ .../connector_types/pagerduty/pagerduty.tsx | 27 ++++ .../pagerduty/pagerduty_params.test.tsx | 10 +- .../pagerduty/pagerduty_params.tsx | 67 ++++++++- .../public/connector_types/types.ts | 2 + 7 files changed, 468 insertions(+), 8 deletions(-) create mode 100644 x-pack/plugins/stack_connectors/public/connector_types/pagerduty/links_list.test.tsx create mode 100644 x-pack/plugins/stack_connectors/public/connector_types/pagerduty/links_list.tsx diff --git a/x-pack/plugins/stack_connectors/public/connector_types/pagerduty/links_list.test.tsx b/x-pack/plugins/stack_connectors/public/connector_types/pagerduty/links_list.test.tsx new file mode 100644 index 0000000000000..10c78d4029d9a --- /dev/null +++ b/x-pack/plugins/stack_connectors/public/connector_types/pagerduty/links_list.test.tsx @@ -0,0 +1,92 @@ +/* + * 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 React from 'react'; +import { screen, render } from '@testing-library/react'; +import { LinksList } from './links_list'; +import userEvent from '@testing-library/user-event'; + +describe('LinksList', () => { + const editAction = jest.fn(); + + const options = { + index: 0, + errors: { + links: [], + }, + editAction, + links: [], + }; + + beforeEach(() => jest.clearAllMocks()); + + it('the list is empty by default', () => { + render(); + + expect(screen.queryByTestId('linksListItemRow')).not.toBeInTheDocument(); + }); + + it('clicking add button calls editAction with correct params', async () => { + render(); + + userEvent.click(await screen.findByTestId('pagerDutyAddLinkButton')); + + expect(editAction).toHaveBeenCalledWith('links', [{ href: '', text: '' }], 0); + }); + + it('clicking remove link button calls editAction with correct params', async () => { + render( + + ); + + expect(await screen.findAllByTestId('linksListItemRow', { exact: false })).toHaveLength(3); + + userEvent.click((await screen.findAllByTestId('pagerDutyRemoveLinkButton'))[1]); + + expect(editAction).toHaveBeenCalledWith( + 'links', + [ + { href: '1', text: 'foobar' }, + { href: '3', text: 'foobar' }, + ], + 0 + ); + }); + + it('editing a link href field calls editAction with correct params', async () => { + render(); + + expect(await screen.findByTestId('linksListItemRow', { exact: false })).toBeInTheDocument(); + + userEvent.paste(await screen.findByTestId('linksHrefInput'), 'newHref'); + + expect(editAction).toHaveBeenCalledWith('links', [{ href: 'newHref', text: 'foobar' }], 0); + }); + + it('editing a link text field calls editAction with correct params', async () => { + render(); + + expect(await screen.findByTestId('linksListItemRow', { exact: false })).toBeInTheDocument(); + + userEvent.paste(await screen.findByTestId('linksTextInput'), 'newText'); + + expect(editAction).toHaveBeenCalledWith('links', [{ href: 'foobar', text: 'newText' }], 0); + }); + + it('correctly displays error messages', async () => { + render(); + + expect(await screen.findByText('FoobarError')); + }); +}); diff --git a/x-pack/plugins/stack_connectors/public/connector_types/pagerduty/links_list.tsx b/x-pack/plugins/stack_connectors/public/connector_types/pagerduty/links_list.tsx new file mode 100644 index 0000000000000..70477fc03683a --- /dev/null +++ b/x-pack/plugins/stack_connectors/public/connector_types/pagerduty/links_list.tsx @@ -0,0 +1,138 @@ +/* + * 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 React from 'react'; +import { + EuiButton, + EuiButtonIcon, + EuiFlexGroup, + EuiFlexItem, + EuiFormRow, + EuiSpacer, +} from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; +import { + ActionParamsProps, + TextFieldWithMessageVariables, +} from '@kbn/triggers-actions-ui-plugin/public'; +import { PagerDutyActionParams } from '../types'; + +type LinksListProps = Pick< + ActionParamsProps, + 'index' | 'editAction' | 'errors' | 'messageVariables' +> & + Pick; + +export const LinksList: React.FC = ({ + editAction, + errors, + index, + links, + messageVariables, +}) => { + const areLinksInvalid = Array.isArray(errors.links) && errors.links.length > 0; + + return ( + + + {links && + links.map((link, currentLinkIndex) => ( + + + + + + { + const newLinks = [...links]; + newLinks[currentLinkIndex] = { text: link.text, href: value }; + editAction('links', newLinks, actionIndex); + }} + messageVariables={messageVariables} + paramsProperty={'linksHref'} + inputTargetValue={link.href} + /> + + + + + { + const newLinks = [...links]; + newLinks[currentLinkIndex] = { href: link.href, text: value }; + editAction('links', newLinks, actionIndex); + }} + messageVariables={messageVariables} + paramsProperty={'linksText'} + inputTargetValue={link.text} + /> + + + + { + links.splice(currentLinkIndex, 1); + editAction('links', links, index); + }} + iconType="minusInCircle" + css={{ marginTop: 28 }} + data-test-subj="pagerDutyRemoveLinkButton" + /> + + + + ))} + +
    + + editAction( + 'links', + links ? [...links, { href: '', text: '' }] : [{ href: '', text: '' }], + index + ) + } + data-test-subj="pagerDutyAddLinkButton" + > + {i18n.translate('xpack.stackConnectors.components.pagerDuty.addLinkButtonLabel', { + defaultMessage: 'Add Link', + })} + +
    +     +     +     + ); +}; diff --git a/x-pack/plugins/stack_connectors/public/connector_types/pagerduty/pagerduty.test.tsx b/x-pack/plugins/stack_connectors/public/connector_types/pagerduty/pagerduty.test.tsx index 311d4bfac8679..4df68dee15880 100644 --- a/x-pack/plugins/stack_connectors/public/connector_types/pagerduty/pagerduty.test.tsx +++ b/x-pack/plugins/stack_connectors/public/connector_types/pagerduty/pagerduty.test.tsx @@ -43,6 +43,8 @@ describe('pagerduty action params validation', () => { component: 'test', group: 'group', class: 'test class', + customDetails: '{}', + links: [], }; expect(await connectorTypeModel.validateParams(actionParams)).toEqual({ @@ -50,6 +52,8 @@ describe('pagerduty action params validation', () => { dedupKey: [], summary: [], timestamp: [], + links: [], + customDetails: [], }, }); }); @@ -74,6 +78,142 @@ describe('pagerduty action params validation', () => { dedupKey: [], summary: [], timestamp: expect.arrayContaining(expected), + links: [], + customDetails: [], + }, + }); + }); + + test('action params validation fails when customDetails are not valid JSON', async () => { + const actionParams = { + eventAction: 'trigger', + dedupKey: 'test', + summary: '2323', + source: 'source', + severity: 'critical', + timestamp: new Date().toISOString(), + component: 'test', + group: 'group', + class: 'test class', + customDetails: '{foo:bar}', + links: [], + }; + + expect(await connectorTypeModel.validateParams(actionParams)).toEqual({ + errors: { + dedupKey: [], + summary: [], + timestamp: [], + links: [], + customDetails: ['Custom details must be a valid JSON.'], + }, + }); + }); + + test('action params validation does not fail when customDetails are not JSON but have mustache templates inside', async () => { + const actionParams = { + eventAction: 'trigger', + dedupKey: 'test', + summary: '2323', + source: 'source', + severity: 'critical', + timestamp: new Date().toISOString(), + component: 'test', + group: 'group', + class: 'test class', + customDetails: '{"details": {{alert.flapping}}}', + links: [], + }; + + expect(await connectorTypeModel.validateParams(actionParams)).toEqual({ + errors: { + dedupKey: [], + summary: [], + timestamp: [], + links: [], + customDetails: [], + }, + }); + }); + + test('action params validation fails when a link is missing the href field', async () => { + const actionParams = { + eventAction: 'trigger', + dedupKey: 'test', + summary: '2323', + source: 'source', + severity: 'critical', + timestamp: new Date().toISOString(), + component: 'test', + group: 'group', + class: 'test class', + customDetails: '{}', + links: [{ href: '', text: 'foobar' }], + }; + + expect(await connectorTypeModel.validateParams(actionParams)).toEqual({ + errors: { + dedupKey: [], + summary: [], + timestamp: [], + links: ['Link properties cannot be empty.'], + customDetails: [], + }, + }); + }); + + test('action params validation fails when a link is missing the text field', async () => { + const actionParams = { + eventAction: 'trigger', + dedupKey: 'test', + summary: '2323', + source: 'source', + severity: 'critical', + timestamp: new Date().toISOString(), + component: 'test', + group: 'group', + class: 'test class', + customDetails: '{}', + links: [{ href: 'foobar', text: '' }], + }; + + expect(await connectorTypeModel.validateParams(actionParams)).toEqual({ + errors: { + dedupKey: [], + summary: [], + timestamp: [], + links: ['Link properties cannot be empty.'], + customDetails: [], + }, + }); + }); + + test('action params validation does not throw the same error multiple times for links', async () => { + const actionParams = { + eventAction: 'trigger', + dedupKey: 'test', + summary: '2323', + source: 'source', + severity: 'critical', + timestamp: new Date().toISOString(), + component: 'test', + group: 'group', + class: 'test class', + customDetails: '{}', + links: [ + { href: 'foobar', text: '' }, + { href: '', text: 'foobar' }, + { href: '', text: '' }, + ], + }; + + expect(await connectorTypeModel.validateParams(actionParams)).toEqual({ + errors: { + dedupKey: [], + summary: [], + timestamp: [], + links: ['Link properties cannot be empty.'], + customDetails: [], }, }); }); diff --git a/x-pack/plugins/stack_connectors/public/connector_types/pagerduty/pagerduty.tsx b/x-pack/plugins/stack_connectors/public/connector_types/pagerduty/pagerduty.tsx index 0dd1513ac55ac..b02665eec66be 100644 --- a/x-pack/plugins/stack_connectors/public/connector_types/pagerduty/pagerduty.tsx +++ b/x-pack/plugins/stack_connectors/public/connector_types/pagerduty/pagerduty.tsx @@ -50,6 +50,8 @@ export function getConnectorType(): ConnectorTypeModel< summary: new Array(), timestamp: new Array(), dedupKey: new Array(), + links: new Array(), + customDetails: new Array(), }; const validationResult = { errors }; if ( @@ -79,6 +81,31 @@ export function getConnectorType(): ConnectorTypeModel< ); } } + if (Array.isArray(actionParams.links)) { + actionParams.links.forEach(({ href, text }) => { + if ((!href || !text) && errors.links.length === 0) { + errors.links.push( + i18n.translate('xpack.stackConnectors.components.pagerDuty.error.invalidLink', { + defaultMessage: 'Link properties cannot be empty.', + }) + ); + } + }); + } + if (actionParams.customDetails?.length && !hasMustacheTokens(actionParams.customDetails)) { + try { + JSON.parse(actionParams.customDetails); + } catch { + errors.customDetails.push( + i18n.translate( + 'xpack.stackConnectors.components.pagerDuty.error.invalidCustomDetails', + { + defaultMessage: 'Custom details must be a valid JSON.', + } + ) + ); + } + } return validationResult; }, actionConnectorFields: lazy(() => import('./pagerduty_connectors')), diff --git a/x-pack/plugins/stack_connectors/public/connector_types/pagerduty/pagerduty_params.test.tsx b/x-pack/plugins/stack_connectors/public/connector_types/pagerduty/pagerduty_params.test.tsx index 19f47166b2726..aa1e6be9bebe0 100644 --- a/x-pack/plugins/stack_connectors/public/connector_types/pagerduty/pagerduty_params.test.tsx +++ b/x-pack/plugins/stack_connectors/public/connector_types/pagerduty/pagerduty_params.test.tsx @@ -11,7 +11,7 @@ import { EventActionOptions, SeverityActionOptions } from '../types'; import PagerDutyParamsFields from './pagerduty_params'; describe('PagerDutyParamsFields renders', () => { - test('all params fields is rendered', () => { + test('all params fields are rendered', () => { const actionParams = { eventAction: EventActionOptions.TRIGGER, dedupKey: 'test', @@ -22,6 +22,11 @@ describe('PagerDutyParamsFields renders', () => { component: 'test', group: 'group', class: 'test class', + customDetails: '{"foo":"bar"}', + links: [ + { href: 'foo', text: 'bar' }, + { href: 'foo', text: 'bar' }, + ], }; const wrapper = mountWithIntl( @@ -55,6 +60,9 @@ describe('PagerDutyParamsFields renders', () => { expect(wrapper.find('[data-test-subj="sourceInput"]').length > 0).toBeTruthy(); expect(wrapper.find('[data-test-subj="summaryInput"]').length > 0).toBeTruthy(); expect(wrapper.find('[data-test-subj="dedupKeyAddVariableButton"]').length > 0).toBeTruthy(); + expect(wrapper.find('[data-test-subj="customDetailsJsonEditor"]').length > 0).toBeTruthy(); + expect(wrapper.find('[data-test-subj="linksList"]').length > 0).toBeTruthy(); + expect(wrapper.find('[data-test-subj="pagerDutyAddLinkButton"]').length > 0).toBeTruthy(); }); test('params select fields do not auto set values eventActionSelect', () => { diff --git a/x-pack/plugins/stack_connectors/public/connector_types/pagerduty/pagerduty_params.tsx b/x-pack/plugins/stack_connectors/public/connector_types/pagerduty/pagerduty_params.tsx index 970ef6ae1ecbf..8505adfee17f8 100644 --- a/x-pack/plugins/stack_connectors/public/connector_types/pagerduty/pagerduty_params.tsx +++ b/x-pack/plugins/stack_connectors/public/connector_types/pagerduty/pagerduty_params.tsx @@ -6,12 +6,23 @@ */ import React from 'react'; -import { EuiFlexGroup, EuiFlexItem, EuiFormRow, EuiSelect, EuiSpacer } from '@elastic/eui'; +import { + EuiFlexGroup, + EuiFlexItem, + EuiFormRow, + EuiSelect, + EuiSpacer, + useEuiTheme, +} from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { isUndefined } from 'lodash'; -import type { ActionParamsProps } from '@kbn/triggers-actions-ui-plugin/public'; +import { + ActionParamsProps, + JsonEditorWithMessageVariables, +} from '@kbn/triggers-actions-ui-plugin/public'; import { TextFieldWithMessageVariables } from '@kbn/triggers-actions-ui-plugin/public'; import { PagerDutyActionParams } from '../types'; +import { LinksList } from './links_list'; const PagerDutyParamsFields: React.FunctionComponent> = ({ actionParams, @@ -20,8 +31,20 @@ const PagerDutyParamsFields: React.FunctionComponent { - const { eventAction, dedupKey, summary, source, severity, timestamp, component, group } = - actionParams; + const { euiTheme } = useEuiTheme(); + + const { + eventAction, + dedupKey, + summary, + source, + severity, + timestamp, + component, + group, + customDetails, + links, + } = actionParams; const severityOptions = [ { value: 'critical', @@ -125,7 +148,7 @@ const PagerDutyParamsFields: React.FunctionComponent - + - {isTriggerPagerDutyEvent ? ( + {isTriggerPagerDutyEvent && ( <> + + { + editAction('customDetails', json, index); + }} + onBlur={() => { + if (!customDetails) { + editAction('customDetails', '', index); + } + }} + data-test-subj="customDetailsJsonEditor" + /> + + - ) : null} + )} ); }; diff --git a/x-pack/plugins/stack_connectors/public/connector_types/types.ts b/x-pack/plugins/stack_connectors/public/connector_types/types.ts index 72319df375e1f..eb0cb9927dce1 100644 --- a/x-pack/plugins/stack_connectors/public/connector_types/types.ts +++ b/x-pack/plugins/stack_connectors/public/connector_types/types.ts @@ -39,6 +39,8 @@ export interface PagerDutyActionParams { component?: string; group?: string; class?: string; + customDetails?: string; + links?: Array<{ href: string; text: string }>; } export interface IndexActionParams { From 808990a78d0044dc775cdfb030b286eba0aa742b Mon Sep 17 00:00:00 2001 From: Dima Arnautov Date: Mon, 27 Nov 2023 14:08:22 +0100 Subject: [PATCH 7/9] [ML] Validate and limit threading parameters for starting model deployment (#171921) ## Summary Closes #171883 This PR adds: - Limiting options for "Threads per allocation" control for a model deployment based on the `max_single_ml_node_processors` limit - Validation of the number of allocation according to the `total_ml_processors` field image ### Checklist - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [x] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) --- .../plugins/ml/common/types/ml_server_info.ts | 2 + .../model_management/deployment_setup.tsx | 55 ++++++++++++++----- 2 files changed, 43 insertions(+), 14 deletions(-) diff --git a/x-pack/plugins/ml/common/types/ml_server_info.ts b/x-pack/plugins/ml/common/types/ml_server_info.ts index e5141d6f2e78f..215f46c26bef8 100644 --- a/x-pack/plugins/ml/common/types/ml_server_info.ts +++ b/x-pack/plugins/ml/common/types/ml_server_info.ts @@ -20,6 +20,8 @@ export interface MlServerDefaults { export interface MlServerLimits { max_model_memory_limit?: string; effective_max_model_memory_limit?: string; + max_single_ml_node_processors?: number; + total_ml_processors?: number; } export interface MlInfoResponse { diff --git a/x-pack/plugins/ml/public/application/model_management/deployment_setup.tsx b/x-pack/plugins/ml/public/application/model_management/deployment_setup.tsx index 3ac6960af55ca..102af34d3e95d 100644 --- a/x-pack/plugins/ml/public/application/model_management/deployment_setup.tsx +++ b/x-pack/plugins/ml/public/application/model_management/deployment_setup.tsx @@ -31,7 +31,7 @@ import type { I18nStart, OverlayStart, ThemeServiceStart } from '@kbn/core/publi import { css } from '@emotion/react'; import { numberValidator } from '@kbn/ml-agg-utils'; import { toMountPoint } from '@kbn/react-kibana-mount'; -import { isCloudTrial } from '../services/ml_server_info'; +import { getNewJobLimits, isCloudTrial } from '../services/ml_server_info'; import { composeValidators, dictionaryValidator, @@ -42,7 +42,7 @@ import { ModelItem } from './models_list'; interface DeploymentSetupProps { config: ThreadingParams; onConfigChange: (config: ThreadingParams) => void; - errors: Partial>; + errors: Partial>>; isUpdate?: boolean; deploymentsParams?: Record; } @@ -66,6 +66,11 @@ export const DeploymentSetup: FC = ({ isUpdate, deploymentsParams, }) => { + const { + total_ml_processors: totalMlProcessors, + max_single_ml_node_processors: maxSingleMlNodeProcessors, + } = getNewJobLimits(); + const numOfAllocation = config.numOfAllocations; const threadsPerAllocations = config.threadsPerAllocations; @@ -76,17 +81,20 @@ export const DeploymentSetup: FC = ({ const threadsPerAllocationsOptions = useMemo( () => - new Array(THREADS_MAX_EXPONENT).fill(null).map((v, i) => { - const value = Math.pow(2, i); - const id = value.toString(); - - return { - id, - label: id, - value, - }; - }), - [] + new Array(THREADS_MAX_EXPONENT) + .fill(null) + .map((v, i) => Math.pow(2, i)) + .filter(maxSingleMlNodeProcessors ? (v) => v <= maxSingleMlNodeProcessors : (v) => true) + .map((value) => { + const id = value.toString(); + + return { + id, + label: id, + value, + }; + }), + [maxSingleMlNodeProcessors] ); const disableThreadingControls = config.priority === 'low'; @@ -251,11 +259,28 @@ export const DeploymentSetup: FC = ({ } hasChildLabel={false} isDisabled={disableThreadingControls} + isInvalid={!!errors.numOfAllocations} + error={ + errors?.numOfAllocations?.min ? ( + + ) : errors?.numOfAllocations?.max ? ( + + ) : null + } > = ({ }) => { const isUpdate = !!initialParams; + const { total_ml_processors: totalMlProcessors } = getNewJobLimits(); + const [config, setConfig] = useState( initialParams ?? { numOfAllocations: 1, @@ -373,7 +400,7 @@ export const StartUpdateDeploymentModal: FC = ({ const numOfAllocationsValidator = composeValidators( requiredValidator(), - numberValidator({ min: 1, integerOnly: true }) + numberValidator({ min: 1, max: totalMlProcessors, integerOnly: true }) ); const numOfAllocationsErrors = numOfAllocationsValidator(config.numOfAllocations); From 8d7816ca4994bed4ab4c1d3387ea0ec42fd64c6b Mon Sep 17 00:00:00 2001 From: Cristina Amico Date: Mon, 27 Nov 2023 14:26:44 +0100 Subject: [PATCH 8/9] [Fleet] Fix epm endpoints return errors (#171722) ## Summary [Fleet] Improve error handling on epm endpoints. Currently most errors occurring when doing any operation with packages will throw and result in a `500` in the correspondent endpoint. This PR is an attempts to handle those errors in a more comprehensive way and to return meaningful responses. Where I can I'm replacing the generic `Error` with `FleetError`; it calls `Logger.error` and checks if the error belongs to a specific type, if not defaults to 500. The error described in https://github.com/elastic/integrations/pull/8268 will now return a 400: https://github.com/elastic/kibana/pull/171722/files#diff-952b3c1842d5d24d9e70833cae1683e2d78df7b489dc99665dab723cc10927c1R349-R352 ### Checklist - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../plugins/fleet/server/errors/handlers.ts | 78 +++++++++++++------ x-pack/plugins/fleet/server/errors/index.ts | 18 +++-- .../fleet/server/services/epm/agent/agent.ts | 15 ++-- .../server/services/epm/archive/storage.ts | 7 +- .../services/epm/elasticsearch/ilm/install.ts | 3 +- .../epm/elasticsearch/template/template.ts | 23 ++++-- .../services/epm/kibana/assets/install.ts | 5 +- .../server/services/epm/package_service.ts | 4 +- .../services/epm/packages/bundled_packages.ts | 8 +- .../fleet/server/services/epm/packages/get.ts | 4 +- .../server/services/epm/packages/install.ts | 18 +++-- .../server/services/epm/packages/reinstall.ts | 8 +- .../services/epm/packages/remove.test.ts | 3 +- .../server/services/epm/packages/remove.ts | 18 ++--- .../server/services/epm/packages/update.ts | 4 +- .../server/services/epm/registry/index.ts | 6 +- 16 files changed, 144 insertions(+), 78 deletions(-) diff --git a/x-pack/plugins/fleet/server/errors/handlers.ts b/x-pack/plugins/fleet/server/errors/handlers.ts index edc34d50598ec..7bd380af258e2 100644 --- a/x-pack/plugins/fleet/server/errors/handlers.ts +++ b/x-pack/plugins/fleet/server/errors/handlers.ts @@ -34,6 +34,13 @@ import { PackagePolicyNotFoundError, FleetUnauthorizedError, PackagePolicyNameExistsError, + PackageOutdatedError, + PackageInvalidArchiveError, + BundledPackageLocationNotFoundError, + PackageRemovalError, + PackageESError, + KibanaSOReferenceError, + PackageAlreadyInstalledError, } from '.'; type IngestErrorHandler = ( @@ -47,30 +54,30 @@ interface IngestErrorHandlerParams { } // unsure if this is correct. would prefer to use something "official" // this type is based on BadRequest values observed while debugging https://github.com/elastic/kibana/issues/75862 - const getHTTPResponseCode = (error: FleetError): number => { - if (error instanceof RegistryResponseError) { - // 4xx/5xx's from EPR - return 500; + // Bad Request + if (error instanceof PackageFailedVerificationError) { + return 400; } - if (error instanceof RegistryConnectionError || error instanceof RegistryError) { - // Connection errors (ie. RegistryConnectionError) / fallback (RegistryError) from EPR - return 502; // Bad Gateway + if (error instanceof PackageOutdatedError) { + return 400; } - if (error instanceof PackageNotFoundError || error instanceof PackagePolicyNotFoundError) { - return 404; // Not Found + if (error instanceof PackageInvalidArchiveError) { + return 400; } - if (error instanceof AgentPolicyNameExistsError) { - return 409; // Conflict + if (error instanceof PackageRemovalError) { + return 400; } - if (error instanceof PackageUnsupportedMediaTypeError) { - return 415; // Unsupported Media Type + if (error instanceof KibanaSOReferenceError) { + return 400; } - if (error instanceof PackageFailedVerificationError) { - return 400; // Bad Request + // Unauthorized + if (error instanceof FleetUnauthorizedError) { + return 403; } - if (error instanceof ConcurrentInstallOperationError) { - return 409; // Conflict + // Not Found + if (error instanceof PackageNotFoundError || error instanceof PackagePolicyNotFoundError) { + return 404; } if (error instanceof AgentNotFoundError) { return 404; @@ -78,14 +85,41 @@ const getHTTPResponseCode = (error: FleetError): number => { if (error instanceof AgentActionNotFoundError) { return 404; } - if (error instanceof FleetUnauthorizedError) { - return 403; // Unauthorized + // Conflict + if (error instanceof AgentPolicyNameExistsError) { + return 409; + } + if (error instanceof ConcurrentInstallOperationError) { + return 409; } if (error instanceof PackagePolicyNameExistsError) { - return 409; // Conflict + return 409; + } + if (error instanceof PackageAlreadyInstalledError) { + return 409; + } + // Unsupported Media Type + if (error instanceof PackageUnsupportedMediaTypeError) { + return 415; } + // Internal Server Error if (error instanceof UninstallTokenError) { - return 500; // Internal Error + return 500; + } + if (error instanceof BundledPackageLocationNotFoundError) { + return 500; + } + if (error instanceof PackageESError) { + return 500; + } + if (error instanceof RegistryResponseError) { + // 4xx/5xx's from EPR + return 500; + } + // Bad Gateway + if (error instanceof RegistryConnectionError || error instanceof RegistryError) { + // Connection errors (ie. RegistryConnectionError) / fallback (RegistryError) from EPR + return 502; } return 400; // Bad Request }; @@ -115,7 +149,7 @@ export function fleetErrorToResponseOptions(error: IngestErrorHandlerParams['err }; } - // not sure what type of error this is. log as much as possible + // default response is 500 logger.error(error); return { statusCode: 500, diff --git a/x-pack/plugins/fleet/server/errors/index.ts b/x-pack/plugins/fleet/server/errors/index.ts index 0b2c6b0fc5e93..7f607f4692774 100644 --- a/x-pack/plugins/fleet/server/errors/index.ts +++ b/x-pack/plugins/fleet/server/errors/index.ts @@ -26,8 +26,9 @@ export class RegistryResponseError extends RegistryError { super(message); } } + +// Package errors export class PackageNotFoundError extends FleetError {} -export class PackageKeyInvalidError extends FleetError {} export class PackageOutdatedError extends FleetError {} export class PackageFailedVerificationError extends FleetError { constructor(pkgName: string, pkgVersion: string) { @@ -37,22 +38,25 @@ export class PackageFailedVerificationError extends FleetError { }; } } +export class PackageUnsupportedMediaTypeError extends FleetError {} +export class PackageInvalidArchiveError extends FleetError {} +export class PackageRemovalError extends FleetError {} +export class PackageESError extends FleetError {} +export class ConcurrentInstallOperationError extends FleetError {} +export class BundledPackageLocationNotFoundError extends FleetError {} +export class KibanaSOReferenceError extends FleetError {} +export class PackageAlreadyInstalledError extends FleetError {} + export class AgentPolicyError extends FleetError {} export class AgentPolicyNotFoundError extends FleetError {} export class AgentNotFoundError extends FleetError {} export class AgentActionNotFoundError extends FleetError {} export class AgentPolicyNameExistsError extends AgentPolicyError {} -export class PackageUnsupportedMediaTypeError extends FleetError {} -export class PackageInvalidArchiveError extends FleetError {} -export class PackageCacheError extends FleetError {} -export class PackageOperationNotSupportedError extends FleetError {} -export class ConcurrentInstallOperationError extends FleetError {} export class AgentReassignmentError extends FleetError {} export class PackagePolicyIneligibleForUpgradeError extends FleetError {} export class PackagePolicyValidationError extends FleetError {} export class PackagePolicyNameExistsError extends FleetError {} export class PackagePolicyNotFoundError extends FleetError {} -export class BundledPackageNotFoundError extends FleetError {} export class HostedAgentPolicyRestrictionRelatedError extends FleetError { constructor(message = 'Cannot perform that action') { super( diff --git a/x-pack/plugins/fleet/server/services/epm/agent/agent.ts b/x-pack/plugins/fleet/server/services/epm/agent/agent.ts index 077aa720b96d8..0bc220a500fb1 100644 --- a/x-pack/plugins/fleet/server/services/epm/agent/agent.ts +++ b/x-pack/plugins/fleet/server/services/epm/agent/agent.ts @@ -10,17 +10,18 @@ import { safeLoad, safeDump } from 'js-yaml'; import type { PackagePolicyConfigRecord } from '../../../../common/types'; import { toCompiledSecretRef } from '../../secrets'; +import { PackageInvalidArchiveError } from '../../../errors'; const handlebars = Handlebars.create(); export function compileTemplate(variables: PackagePolicyConfigRecord, templateStr: string) { - const { vars, yamlValues } = buildTemplateVariables(variables, templateStr); + const { vars, yamlValues } = buildTemplateVariables(variables); let compiledTemplate: string; try { const template = handlebars.compile(templateStr, { noEscape: true }); compiledTemplate = template(vars); } catch (err) { - throw new Error(`Error while compiling agent template: ${err.message}`); + throw new PackageInvalidArchiveError(`Error while compiling agent template: ${err.message}`); } compiledTemplate = replaceRootLevelYamlVariables(yamlValues, compiledTemplate); @@ -64,7 +65,7 @@ function replaceVariablesInYaml(yamlVariables: { [k: string]: any }, yaml: any) return yaml; } -function buildTemplateVariables(variables: PackagePolicyConfigRecord, templateStr: string) { +function buildTemplateVariables(variables: PackagePolicyConfigRecord) { const yamlValues: { [k: string]: any } = {}; const vars = Object.entries(variables).reduce((acc, [key, recordEntry]) => { // support variables with . like key.patterns @@ -72,13 +73,17 @@ function buildTemplateVariables(variables: PackagePolicyConfigRecord, templateSt const lastKeyPart = keyParts.pop(); if (!lastKeyPart || !isValidKey(lastKeyPart)) { - throw new Error('Invalid key'); + throw new PackageInvalidArchiveError( + `Error while compiling agent template: Invalid key ${lastKeyPart}` + ); } let varPart = acc; for (const keyPart of keyParts) { if (!isValidKey(keyPart)) { - throw new Error('Invalid key'); + throw new PackageInvalidArchiveError( + `Error while compiling agent template: Invalid key ${keyPart}` + ); } if (!varPart[keyPart]) { varPart[keyPart] = {}; diff --git a/x-pack/plugins/fleet/server/services/epm/archive/storage.ts b/x-pack/plugins/fleet/server/services/epm/archive/storage.ts index cfa110589a010..81d55c5fd3138 100644 --- a/x-pack/plugins/fleet/server/services/epm/archive/storage.ts +++ b/x-pack/plugins/fleet/server/services/epm/archive/storage.ts @@ -19,6 +19,7 @@ import type { InstallSource, PackageAssetReference, } from '../../../../common/types'; +import { PackageInvalidArchiveError, PackageNotFoundError } from '../../../errors'; import { appContextService } from '../../app_context'; @@ -70,13 +71,13 @@ export async function archiveEntryToESDocument(opts: { // validation: filesize? asset type? anything else if (dataUtf8.length > currentMaxAssetBytes) { - throw new Error( + throw new PackageInvalidArchiveError( `File at ${path} is larger than maximum allowed size of ${currentMaxAssetBytes}` ); } if (dataBase64.length > currentMaxAssetBytes) { - throw new Error( + throw new PackageInvalidArchiveError( `After base64 encoding file at ${path} is larger than maximum allowed size of ${currentMaxAssetBytes}` ); } @@ -113,7 +114,7 @@ export async function saveArchiveEntries(opts: { const bulkBody = await Promise.all( paths.map((path) => { const buffer = getArchiveEntry(path); - if (!buffer) throw new Error(`Could not find ArchiveEntry at ${path}`); + if (!buffer) throw new PackageNotFoundError(`Could not find ArchiveEntry at ${path}`); const { name, version } = packageInfo; return archiveEntryToBulkCreateObject({ path, buffer, name, version, installSource }); }) diff --git a/x-pack/plugins/fleet/server/services/epm/elasticsearch/ilm/install.ts b/x-pack/plugins/fleet/server/services/epm/elasticsearch/ilm/install.ts index 3aa86b526addd..61a75d28b7999 100644 --- a/x-pack/plugins/fleet/server/services/epm/elasticsearch/ilm/install.ts +++ b/x-pack/plugins/fleet/server/services/epm/elasticsearch/ilm/install.ts @@ -14,6 +14,7 @@ import { getAsset, getPathParts } from '../../archive'; import { updateEsAssetReferences } from '../../packages/install'; import { getESAssetMetadata } from '../meta'; import { retryTransientEsErrors } from '../retry'; +import { PackageInvalidArchiveError } from '../../../../errors'; export async function installILMPolicy( packageInfo: InstallablePackage, @@ -57,7 +58,7 @@ export async function installILMPolicy( { logger } ); } catch (err) { - throw new Error(err.message); + throw new PackageInvalidArchiveError(`Couldn't install ilm policies: ${err.message}`); } }) ); diff --git a/x-pack/plugins/fleet/server/services/epm/elasticsearch/template/template.ts b/x-pack/plugins/fleet/server/services/epm/elasticsearch/template/template.ts index eb0ea4f62c73e..a26aa2c8e0114 100644 --- a/x-pack/plugins/fleet/server/services/epm/elasticsearch/template/template.ts +++ b/x-pack/plugins/fleet/server/services/epm/elasticsearch/template/template.ts @@ -33,6 +33,7 @@ import { } from '../../../../constants'; import { getESAssetMetadata } from '../meta'; import { retryTransientEsErrors } from '../retry'; +import { PackageESError, PackageInvalidArchiveError } from '../../../../errors'; import { getDefaultProperties, histogram, keyword, scaledFloat } from './mappings'; @@ -102,7 +103,9 @@ export function getTemplate({ isIndexModeTimeSeries, }); if (template.template.settings.index.final_pipeline) { - throw new Error(`Error template for ${templateIndexPattern} contains a final_pipeline`); + throw new PackageInvalidArchiveError( + `Error template for ${templateIndexPattern} contains a final_pipeline` + ); } const esBaseComponents = getBaseEsComponents(type, !!isIndexModeTimeSeries); @@ -427,8 +430,8 @@ function _generateMappings( matchingType = field.object_type_mapping_type ?? 'object'; break; default: - throw new Error( - `no dynamic mapping generated for field ${path} of type ${field.object_type}` + throw new PackageInvalidArchiveError( + `No dynamic mapping generated for field ${path} of type ${field.object_type}` ); } @@ -908,7 +911,9 @@ const rolloverDataStream = (dataStreamName: string, esClient: ElasticsearchClien alias: dataStreamName, }); } catch (error) { - throw new Error(`cannot rollover data stream [${dataStreamName}] due to error: ${error}`); + throw new PackageESError( + `Cannot rollover data stream [${dataStreamName}] due to error: ${error}` + ); } }; @@ -1055,7 +1060,11 @@ const updateExistingDataStream = async ({ { logger } ); } catch (err) { - throw new Error(`could not update lifecycle settings for ${dataStreamName}: ${err.message}`); + // Check if this error can happen because of invalid settings; + // We are returning a 500 but in that case it should be a 400 instead + throw new PackageESError( + `Could not update lifecycle settings for ${dataStreamName}: ${err.message}` + ); } } @@ -1078,6 +1087,8 @@ const updateExistingDataStream = async ({ { logger } ); } catch (err) { - throw new Error(`could not update index template settings for ${dataStreamName}`); + // Same as above - Check if this error can happen because of invalid settings; + // We are returning a 500 but in that case it should be a 400 instead + throw new PackageESError(`Could not update index template settings for ${dataStreamName}`); } }; diff --git a/x-pack/plugins/fleet/server/services/epm/kibana/assets/install.ts b/x-pack/plugins/fleet/server/services/epm/kibana/assets/install.ts index 20a0484c77a4a..23327a2253f86 100644 --- a/x-pack/plugins/fleet/server/services/epm/kibana/assets/install.ts +++ b/x-pack/plugins/fleet/server/services/epm/kibana/assets/install.ts @@ -35,6 +35,7 @@ import { savedObjectTypes } from '../../packages'; import { indexPatternTypes, getIndexPatternSavedObjects } from '../index_pattern/install'; import { saveKibanaAssetsRefs } from '../../packages/install'; import { deleteKibanaSavedObjectsAssets } from '../../packages/remove'; +import { KibanaSOReferenceError } from '../../../../errors'; import { withPackageSpan } from '../../packages/utils'; @@ -340,7 +341,7 @@ export async function installKibanaSavedObjects({ ); if (otherErrors?.length) { - throw new Error( + throw new KibanaSOReferenceError( `Encountered ${ otherErrors.length } errors creating saved objects: ${formatImportErrorsForLog(otherErrors)}` @@ -383,7 +384,7 @@ export async function installKibanaSavedObjects({ }); if (resolveErrors?.length) { - throw new Error( + throw new KibanaSOReferenceError( `Encountered ${ resolveErrors.length } errors resolving reference errors: ${formatImportErrorsForLog(resolveErrors)}` diff --git a/x-pack/plugins/fleet/server/services/epm/package_service.ts b/x-pack/plugins/fleet/server/services/epm/package_service.ts index 39ca950af93db..e6f71cb7cb96c 100644 --- a/x-pack/plugins/fleet/server/services/epm/package_service.ts +++ b/x-pack/plugins/fleet/server/services/epm/package_service.ts @@ -29,7 +29,7 @@ import type { } from '../../types'; import type { FleetAuthzRouteConfig } from '../security/types'; import { checkSuperuser, getAuthzFromRequest, doesNotHaveRequiredFleetAuthz } from '../security'; -import { FleetUnauthorizedError } from '../../errors'; +import { FleetUnauthorizedError, FleetError } from '../../errors'; import { INSTALL_PACKAGES_AUTHZ, READ_PACKAGE_INFO_AUTHZ } from '../../routes/epm'; import { installTransforms, isTransform } from './elasticsearch/transform/install'; @@ -208,7 +208,7 @@ class PackageClientImpl implements PackageClient { const transformPaths = assetPaths.filter(isTransform); if (transformPaths.length !== assetPaths.length) { - throw new Error('reinstallEsAssets is currently only implemented for transform assets'); + throw new FleetError('reinstallEsAssets is currently only implemented for transform assets'); } if (transformPaths.length) { diff --git a/x-pack/plugins/fleet/server/services/epm/packages/bundled_packages.ts b/x-pack/plugins/fleet/server/services/epm/packages/bundled_packages.ts index 7078761a4a583..92f9674656103 100644 --- a/x-pack/plugins/fleet/server/services/epm/packages/bundled_packages.ts +++ b/x-pack/plugins/fleet/server/services/epm/packages/bundled_packages.ts @@ -9,7 +9,7 @@ import fs from 'fs/promises'; import path from 'path'; import type { BundledPackage, Installation } from '../../../types'; -import { FleetError } from '../../../errors'; +import { BundledPackageLocationNotFoundError } from '../../../errors'; import { appContextService } from '../../app_context'; import { splitPkgKey, pkgToPkgKey } from '../registry'; @@ -19,7 +19,9 @@ export async function getBundledPackages(): Promise { const bundledPackageLocation = config?.developer?.bundledPackageLocation; if (!bundledPackageLocation) { - throw new FleetError('xpack.fleet.developer.bundledPackageLocation is not configured'); + throw new BundledPackageLocationNotFoundError( + 'xpack.fleet.developer.bundledPackageLocation is not configured' + ); } // If the bundled package directory is missing, we log a warning during setup, @@ -51,7 +53,7 @@ export async function getBundledPackages(): Promise { return result; } catch (err) { const logger = appContextService.getLogger(); - logger.debug(`Unable to read bundled packages from ${bundledPackageLocation}`); + logger.warn(`Unable to read bundled packages from ${bundledPackageLocation}`); return []; } diff --git a/x-pack/plugins/fleet/server/services/epm/packages/get.ts b/x-pack/plugins/fleet/server/services/epm/packages/get.ts index 68a884fd5f198..a3ed5f62d7f1e 100644 --- a/x-pack/plugins/fleet/server/services/epm/packages/get.ts +++ b/x-pack/plugins/fleet/server/services/epm/packages/get.ts @@ -44,7 +44,6 @@ import type { } from '../../../../common/types'; import type { Installation, PackageInfo, PackagePolicySOAttributes } from '../../../types'; import { - FleetError, PackageFailedVerificationError, PackageNotFoundError, RegistryResponseError, @@ -575,6 +574,7 @@ export async function getPackageFromSource(options: { logger.debug(`retrieved installed package ${pkgName}-${pkgVersion}`); } catch (error) { if (error instanceof PackageFailedVerificationError) { + logger.error(`package ${pkgName}-${pkgVersion} failed verification`); throw error; } // treating this is a 404 as no status code returned @@ -600,7 +600,7 @@ export async function getPackageFromSource(options: { } } if (!res) { - throw new FleetError(`package info for ${pkgName}-${pkgVersion} does not exist`); + throw new PackageNotFoundError(`Package info for ${pkgName}-${pkgVersion} does not exist`); } return { paths: res.paths, diff --git a/x-pack/plugins/fleet/server/services/epm/packages/install.ts b/x-pack/plugins/fleet/server/services/epm/packages/install.ts index a3919bbdb20e7..40db0c76dd66a 100644 --- a/x-pack/plugins/fleet/server/services/epm/packages/install.ts +++ b/x-pack/plugins/fleet/server/services/epm/packages/install.ts @@ -57,11 +57,13 @@ import { DATASET_VAR_NAME, } from '../../../../common/constants'; import { - type FleetError, + FleetError, PackageOutdatedError, PackagePolicyValidationError, ConcurrentInstallOperationError, FleetUnauthorizedError, + PackageInvalidArchiveError, + PackageNotFoundError, } from '../../../errors'; import { PACKAGES_SAVED_OBJECT_TYPE, MAX_TIME_COMPLETE_INSTALL } from '../../../constants'; import { dataStreamService, licenseService } from '../..'; @@ -202,7 +204,7 @@ export async function ensureInstalledPackage(options: { } const installation = await getInstallation({ savedObjectsClient, pkgName }); - if (!installation) throw new Error(`could not get installation ${pkgName}`); + if (!installation) throw new FleetError(`Could not get installation for ${pkgName}`); return installation; } @@ -714,7 +716,7 @@ export type InstallPackageParams = { export async function installPackage(args: InstallPackageParams): Promise { if (!('installSource' in args)) { - throw new Error('installSource is required'); + throw new FleetError('installSource is required'); } const logger = appContextService.getLogger(); @@ -805,7 +807,7 @@ export async function installPackage(args: InstallPackageParams): Promise { getLogger: jest.fn().mockReturnValue({ info: jest.fn(), error: jest.fn(), + warn: jest.fn(), }), }, packagePolicyService: { @@ -78,7 +79,7 @@ describe('removeInstallation', () => { force: false, }) ).rejects.toThrowError( - `unable to remove package with existing package policy(s) in use by agent(s)` + `Unable to remove package with existing package policy(s) in use by agent(s)` ); }); diff --git a/x-pack/plugins/fleet/server/services/epm/packages/remove.ts b/x-pack/plugins/fleet/server/services/epm/packages/remove.ts index c65a4d165cf7f..ba9bace6a0dee 100644 --- a/x-pack/plugins/fleet/server/services/epm/packages/remove.ts +++ b/x-pack/plugins/fleet/server/services/epm/packages/remove.ts @@ -7,8 +7,6 @@ import type { ElasticsearchClient, SavedObjectsClientContract } from '@kbn/core/server'; -import Boom from '@hapi/boom'; - import type { SavedObject } from '@kbn/core/server'; import { SavedObjectsClient } from '@kbn/core/server'; @@ -42,6 +40,7 @@ import { deleteIlms } from '../elasticsearch/datastream_ilm/remove'; import { removeArchiveEntries } from '../archive/storage'; import { auditLoggingService } from '../../audit_logging'; +import { FleetError, PackageRemovalError } from '../../../errors'; import { populatePackagePolicyAssignedAgentsCount } from '../../package_policies/populate_package_policy_assigned_agents_count'; @@ -56,7 +55,7 @@ export async function removeInstallation(options: { }): Promise { const { savedObjectsClient, pkgName, pkgVersion, esClient } = options; const installation = await getInstallation({ savedObjectsClient, pkgName }); - if (!installation) throw Boom.badRequest(`${pkgName} is not installed`); + if (!installation) throw new PackageRemovalError(`${pkgName} is not installed`); const { total, items } = await packagePolicyService.list(savedObjectsClient, { kuery: `${PACKAGE_POLICY_SAVED_OBJECT_TYPE}.package.name:${pkgName}`, @@ -72,17 +71,12 @@ export async function removeInstallation(options: { if (options.force || items.every((item) => (item.agents ?? 0) === 0)) { // delete package policies const ids = items.map((item) => item.id); - appContextService - .getLogger() - .info( - `deleting package policies of ${pkgName} package because not used by agents or force flag was enabled: ${ids}` - ); await packagePolicyService.delete(savedObjectsClient, esClient, ids, { force: options.force, }); } else { - throw Boom.badRequest( - `unable to remove package with existing package policy(s) in use by agent(s)` + throw new PackageRemovalError( + `Unable to remove package with existing package policy(s) in use by agent(s)` ); } } @@ -242,7 +236,7 @@ async function deleteIndexTemplate(esClient: ElasticsearchClient, name: string): try { await esClient.indices.deleteIndexTemplate({ name }, { ignore: [404] }); } catch { - throw new Error(`error deleting index template ${name}`); + throw new FleetError(`Error deleting index template ${name}`); } } } @@ -253,7 +247,7 @@ async function deleteComponentTemplate(esClient: ElasticsearchClient, name: stri try { await esClient.cluster.deleteComponentTemplate({ name }, { ignore: [404] }); } catch (error) { - throw new Error(`error deleting component template ${name}`); + throw new FleetError(`Error deleting component template ${name}`); } } } diff --git a/x-pack/plugins/fleet/server/services/epm/packages/update.ts b/x-pack/plugins/fleet/server/services/epm/packages/update.ts index 3072dfed86636..72c43b6dc688a 100644 --- a/x-pack/plugins/fleet/server/services/epm/packages/update.ts +++ b/x-pack/plugins/fleet/server/services/epm/packages/update.ts @@ -12,7 +12,7 @@ import type { ExperimentalIndexingFeature } from '../../../../common/types'; import { PACKAGES_SAVED_OBJECT_TYPE } from '../../../constants'; import type { Installation, UpdatePackageRequestSchema } from '../../../types'; -import { FleetError } from '../../../errors'; +import { PackageNotFoundError } from '../../../errors'; import { auditLoggingService } from '../../audit_logging'; @@ -29,7 +29,7 @@ export async function updatePackage( const installedPackage = await getInstallationObject({ savedObjectsClient, pkgName }); if (!installedPackage) { - throw new FleetError(`package ${pkgName} is not installed`); + throw new PackageNotFoundError(`Error while updating package: ${pkgName} is not installed`); } auditLoggingService.writeCustomSoAuditLog({ diff --git a/x-pack/plugins/fleet/server/services/epm/registry/index.ts b/x-pack/plugins/fleet/server/services/epm/registry/index.ts index 487faf55730bd..24c81dd023244 100644 --- a/x-pack/plugins/fleet/server/services/epm/registry/index.ts +++ b/x-pack/plugins/fleet/server/services/epm/registry/index.ts @@ -43,6 +43,7 @@ import { PackageNotFoundError, RegistryResponseError, PackageFailedVerificationError, + PackageUnsupportedMediaTypeError, } from '../../../errors'; import { getBundledPackageByName } from '../packages/bundled_packages'; @@ -364,8 +365,11 @@ export async function getPackage( function ensureContentType(archivePath: string) { const contentType = mime.lookup(archivePath); + if (!contentType) { - throw new Error(`Unknown compression format for '${archivePath}'. Please use .zip or .gz`); + throw new PackageUnsupportedMediaTypeError( + `Unknown compression format for '${archivePath}'. Please use .zip or .gz` + ); } return contentType; } From 5f5c92a5a346907468d0d8227b2a15d9d7f544ac Mon Sep 17 00:00:00 2001 From: Antonio Date: Mon, 27 Nov 2023 14:28:54 +0100 Subject: [PATCH 9/9] [Cases] Display required badge in custom field configuration. (#171975) Fixes #167767 ## Summary I added information about whether the custom field is required or not. Additionally, I followed @mdefazio 's comment and changed how we display these to use an`EuiBadge` instead. The color is custom. Screenshot 2023-11-27 at 11 57 52 --- .../custom_fields_list/index.test.tsx | 55 +++++++++++++------ .../custom_fields_list/index.tsx | 11 +++- .../components/custom_fields/translations.ts | 4 ++ 3 files changed, 52 insertions(+), 18 deletions(-) diff --git a/x-pack/plugins/cases/public/components/custom_fields/custom_fields_list/index.test.tsx b/x-pack/plugins/cases/public/components/custom_fields/custom_fields_list/index.test.tsx index f9b913af4d429..b7c87f3356d38 100644 --- a/x-pack/plugins/cases/public/components/custom_fields/custom_fields_list/index.test.tsx +++ b/x-pack/plugins/cases/public/components/custom_fields/custom_fields_list/index.test.tsx @@ -37,11 +37,21 @@ describe('CustomFieldsList', () => { it('shows CustomFieldsList correctly', async () => { appMockRender.render(); - expect(screen.getByTestId('custom-fields-list')).toBeInTheDocument(); + expect(await screen.findByTestId('custom-fields-list')).toBeInTheDocument(); - for (const field of customFieldsConfigurationMock) { - expect(screen.getByTestId(`custom-field-${field.key}-${field.type}`)).toBeInTheDocument(); - } + expect( + await screen.findByTestId( + `custom-field-${customFieldsConfigurationMock[0].key}-${customFieldsConfigurationMock[0].type}` + ) + ).toBeInTheDocument(); + expect(await screen.findByText('Text')).toBeInTheDocument(); + expect(await screen.findByText('Required')).toBeInTheDocument(); + expect( + await screen.findByTestId( + `custom-field-${customFieldsConfigurationMock[1].key}-${customFieldsConfigurationMock[1].type}` + ) + ).toBeInTheDocument(); + expect(await screen.findByText('Toggle')).toBeInTheDocument(); }); it('shows single CustomFieldsList correctly', async () => { @@ -49,16 +59,21 @@ describe('CustomFieldsList', () => { ); - const list = screen.getByTestId('custom-fields-list'); + const list = await screen.findByTestId('custom-fields-list'); expect(list).toBeInTheDocument(); expect( - screen.getByTestId( + await screen.findByTestId( `custom-field-${customFieldsConfigurationMock[0].key}-${customFieldsConfigurationMock[0].type}` ) ).toBeInTheDocument(); + expect(await screen.findByText('Text')).toBeInTheDocument(); + expect(await screen.findByText('Required')).toBeInTheDocument(); + expect( + await within(list).findByTestId(`${customFieldsConfigurationMock[0].key}-custom-field-edit`) + ).toBeInTheDocument(); expect( - within(list).getByTestId(`${customFieldsConfigurationMock[0].key}-custom-field-delete`) + await within(list).findByTestId(`${customFieldsConfigurationMock[0].key}-custom-field-delete`) ).toBeInTheDocument(); }); @@ -76,10 +91,12 @@ describe('CustomFieldsList', () => { it('shows confirmation modal when deleting a field ', async () => { appMockRender.render(); - const list = screen.getByTestId('custom-fields-list'); + const list = await screen.findByTestId('custom-fields-list'); userEvent.click( - within(list).getByTestId(`${customFieldsConfigurationMock[0].key}-custom-field-delete`) + await within(list).findByTestId( + `${customFieldsConfigurationMock[0].key}-custom-field-delete` + ) ); expect(await screen.findByTestId('confirm-delete-custom-field-modal')).toBeInTheDocument(); @@ -88,15 +105,17 @@ describe('CustomFieldsList', () => { it('calls onDeleteCustomField when confirm', async () => { appMockRender.render(); - const list = screen.getByTestId('custom-fields-list'); + const list = await screen.findByTestId('custom-fields-list'); userEvent.click( - within(list).getByTestId(`${customFieldsConfigurationMock[0].key}-custom-field-delete`) + await within(list).findByTestId( + `${customFieldsConfigurationMock[0].key}-custom-field-delete` + ) ); expect(await screen.findByTestId('confirm-delete-custom-field-modal')).toBeInTheDocument(); - userEvent.click(screen.getByText('Delete')); + userEvent.click(await screen.findByText('Delete')); await waitFor(() => { expect(screen.queryByTestId('confirm-delete-custom-field-modal')).not.toBeInTheDocument(); @@ -109,15 +128,17 @@ describe('CustomFieldsList', () => { it('does not call onDeleteCustomField when cancel', async () => { appMockRender.render(); - const list = screen.getByTestId('custom-fields-list'); + const list = await screen.findByTestId('custom-fields-list'); userEvent.click( - within(list).getByTestId(`${customFieldsConfigurationMock[0].key}-custom-field-delete`) + await within(list).findByTestId( + `${customFieldsConfigurationMock[0].key}-custom-field-delete` + ) ); expect(await screen.findByTestId('confirm-delete-custom-field-modal')).toBeInTheDocument(); - userEvent.click(screen.getByText('Cancel')); + userEvent.click(await screen.findByText('Cancel')); await waitFor(() => { expect(screen.queryByTestId('confirm-delete-custom-field-modal')).not.toBeInTheDocument(); @@ -134,10 +155,10 @@ describe('CustomFieldsList', () => { it('calls onEditCustomField correctly', async () => { appMockRender.render(); - const list = screen.getByTestId('custom-fields-list'); + const list = await screen.findByTestId('custom-fields-list'); userEvent.click( - within(list).getByTestId(`${customFieldsConfigurationMock[0].key}-custom-field-edit`) + await within(list).findByTestId(`${customFieldsConfigurationMock[0].key}-custom-field-edit`) ); await waitFor(() => { diff --git a/x-pack/plugins/cases/public/components/custom_fields/custom_fields_list/index.tsx b/x-pack/plugins/cases/public/components/custom_fields/custom_fields_list/index.tsx index 649b0ec5d339f..cfccb53e48db3 100644 --- a/x-pack/plugins/cases/public/components/custom_fields/custom_fields_list/index.tsx +++ b/x-pack/plugins/cases/public/components/custom_fields/custom_fields_list/index.tsx @@ -13,7 +13,10 @@ import { EuiSpacer, EuiText, EuiButtonIcon, + useEuiTheme, + EuiBadge, } from '@elastic/eui'; +import * as i18n from '../translations'; import type { CustomFieldTypes, CustomFieldsConfiguration } from '../../../../common/types/domain'; import { builderMap } from '../builder'; @@ -28,6 +31,7 @@ export interface Props { const CustomFieldsListComponent: React.FC = (props) => { const { customFields, onDeleteCustomField, onEditCustomField } = props; const [selectedItem, setSelectedItem] = useState(null); + const { euiTheme } = useEuiTheme(); const renderTypeLabel = (type?: CustomFieldTypes) => { const createdBuilder = type && builderMap[type]; @@ -69,7 +73,12 @@ const CustomFieldsListComponent: React.FC = (props) => {

    {customField.label}

    - {renderTypeLabel(customField.type)} + + {renderTypeLabel(customField.type)} + + {customField.required && ( + {i18n.REQUIRED} + )}     diff --git a/x-pack/plugins/cases/public/components/custom_fields/translations.ts b/x-pack/plugins/cases/public/components/custom_fields/translations.ts index ac7f99f191373..a5ac6da2fa5f3 100644 --- a/x-pack/plugins/cases/public/components/custom_fields/translations.ts +++ b/x-pack/plugins/cases/public/components/custom_fields/translations.ts @@ -66,6 +66,10 @@ export const FIELD_OPTION_REQUIRED = i18n.translate( } ); +export const REQUIRED = i18n.translate('xpack.cases.customFields.required', { + defaultMessage: 'Required', +}); + export const REQUIRED_FIELD = (fieldName: string): string => i18n.translate('xpack.cases.customFields.requiredField', { values: { fieldName },