From 0b54d316ef5b413793e95c846ee7f9bd04f18480 Mon Sep 17 00:00:00 2001 From: mosiac1 <88427079+mosiac1@users.noreply.github.com> Date: Tue, 21 Feb 2023 14:35:07 +0000 Subject: [PATCH] Move search to URL query string parameter rather than path (#10088) * [WiP] Fix encoding of '%' in search query * Use search query parameter for search instead of path * Fix tests --------- Co-authored-by: Chirag Madlani <12962843+chirag-madlani@users.noreply.github.com> --- .../constants/redirections.constants.js | 2 +- .../GlobalSearchProvider.tsx | 27 ++++++----- .../MyAssetStats/MyAssetStats.component.tsx | 12 ++--- .../MyAssetStats/MyAssetStats.test.tsx | 10 ++-- .../RecentSearchedTermsAntd.tsx | 4 +- .../ui/src/components/app-bar/Appbar.tsx | 48 +++++++++++-------- .../src/components/app-bar/SearchOptions.tsx | 4 +- .../router/AuthenticatedAppRouter.tsx | 1 - .../resources/ui/src/constants/constants.ts | 38 +++++++++++---- .../ui/src/pages/database-details/index.tsx | 20 ++++---- .../pages/explore/ExplorePage.component.tsx | 22 +++++---- .../resources/ui/src/pages/tags/index.tsx | 18 +++---- .../resources/ui/src/utils/RouterUtils.ts | 21 -------- 13 files changed, 124 insertions(+), 103 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/cypress/constants/redirections.constants.js b/openmetadata-ui/src/main/resources/ui/cypress/constants/redirections.constants.js index dcbcdf8858c..0845ec6c7b0 100644 --- a/openmetadata-ui/src/main/resources/ui/cypress/constants/redirections.constants.js +++ b/openmetadata-ui/src/main/resources/ui/cypress/constants/redirections.constants.js @@ -55,7 +55,7 @@ export const LEFT_PANEL_DETAILS = { export const NAVBAR_DETAILS = { explore: { testid: '[data-testid="appbar-item-explore"]', - url: `${BASE_URL}/explore/tables/?page=1`, + url: `${BASE_URL}/explore/tables?page=1`, }, quality: { testid: '[data-testid="appbar-item-data-quality"]', diff --git a/openmetadata-ui/src/main/resources/ui/src/components/GlobalSearchProvider/GlobalSearchProvider.tsx b/openmetadata-ui/src/main/resources/ui/src/components/GlobalSearchProvider/GlobalSearchProvider.tsx index e5bb933a13e..9f6b9f94b10 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/GlobalSearchProvider/GlobalSearchProvider.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/GlobalSearchProvider/GlobalSearchProvider.tsx @@ -13,6 +13,7 @@ import { Modal } from 'antd'; import { debounce } from 'lodash'; +import Qs from 'qs'; import { BaseSelectRef } from 'rc-select'; import React, { FC, @@ -23,8 +24,7 @@ import React, { useState, } from 'react'; import { useHistory } from 'react-router-dom'; -import AppState from '../../AppState'; -import { getExplorePathWithSearch, ROUTES } from '../../constants/constants'; +import { getExplorePath, ROUTES } from '../../constants/constants'; import { addToRecentSearched } from '../../utils/CommonUtils'; import { isCommandKeyPress, Keys } from '../../utils/KeyboardUtil'; import GlobalSearchSuggestions from './GlobalSearchSuggestions/GlobalSearchSuggestions'; @@ -77,15 +77,20 @@ const GlobalSearchProvider: FC = ({ children }: Props) => { const searchHandler = (value: string) => { addToRecentSearched(value); - history.push({ - pathname: getExplorePathWithSearch( - value, - location.pathname.startsWith(ROUTES.EXPLORE) - ? AppState.explorePageTab - : 'tables' - ), - search: location.search, - }); + if (location.pathname.startsWith(ROUTES.EXPLORE)) { + // Already on explore page, only push search change + const paramsObject: Record = Qs.parse( + location.search.startsWith('?') + ? location.search.substr(1) + : location.search + ); + history.push({ + search: Qs.stringify({ ...paramsObject, search: value }), + }); + } else { + // Outside Explore page + history.push(getExplorePath({ search: value })); + } }; const handleKeyDown = (e: React.KeyboardEvent) => { diff --git a/openmetadata-ui/src/main/resources/ui/src/components/MyAssetStats/MyAssetStats.component.tsx b/openmetadata-ui/src/main/resources/ui/src/components/MyAssetStats/MyAssetStats.component.tsx index 7b1aad09586..497d3e6cc66 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/MyAssetStats/MyAssetStats.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/MyAssetStats/MyAssetStats.component.tsx @@ -16,7 +16,7 @@ import { isNil } from 'lodash'; import React, { FunctionComponent, useMemo } from 'react'; import { useTranslation } from 'react-i18next'; import { Link } from 'react-router-dom'; -import { getExplorePathWithSearch, ROUTES } from '../../constants/constants'; +import { getExplorePath, ROUTES } from '../../constants/constants'; import { GlobalSettingOptions, GlobalSettingsMenuCategory, @@ -41,35 +41,35 @@ const MyAssetStats: FunctionComponent = ({ icon: Icons.TABLE_GREY, data: t('label.table-plural'), count: entityCounts.tableCount, - link: getExplorePathWithSearch(undefined, 'tables'), + link: getExplorePath({ tab: 'tables' }), dataTestId: 'tables', }, topics: { icon: Icons.TOPIC_GREY, data: t('label.topic-plural'), count: entityCounts.topicCount, - link: getExplorePathWithSearch(undefined, 'topics'), + link: getExplorePath({ tab: 'topics' }), dataTestId: 'topics', }, dashboards: { icon: Icons.DASHBOARD_GREY, data: t('label.dashboard-plural'), count: entityCounts.dashboardCount, - link: getExplorePathWithSearch(undefined, 'dashboards'), + link: getExplorePath({ tab: 'dashboards' }), dataTestId: 'dashboards', }, pipelines: { icon: Icons.PIPELINE_GREY, data: t('label.pipeline-plural'), count: entityCounts.pipelineCount, - link: getExplorePathWithSearch(undefined, 'pipelines'), + link: getExplorePath({ tab: 'pipelines' }), dataTestId: 'pipelines', }, mlModal: { icon: Icons.MLMODAL, data: t('label.ml-model-plural'), count: entityCounts.mlmodelCount, - link: getExplorePathWithSearch(undefined, 'mlmodels'), + link: getExplorePath({ tab: 'mlmodels' }), dataTestId: 'mlmodels', }, testSuite: { diff --git a/openmetadata-ui/src/main/resources/ui/src/components/MyAssetStats/MyAssetStats.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/MyAssetStats/MyAssetStats.test.tsx index 2a0c8ff12ba..8aace049905 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/MyAssetStats/MyAssetStats.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/MyAssetStats/MyAssetStats.test.tsx @@ -68,11 +68,11 @@ describe('Test MyDataHeader Component', () => { const user = getByTestId(container, 'user'); const terms = getByTestId(container, 'terms'); - expect(tables).toHaveAttribute('href', '/explore/tables/'); - expect(topics).toHaveAttribute('href', '/explore/topics/'); - expect(dashboards).toHaveAttribute('href', '/explore/dashboards/'); - expect(pipelines).toHaveAttribute('href', '/explore/pipelines/'); - expect(mlmodel).toHaveAttribute('href', '/explore/mlmodels/'); + expect(tables).toHaveAttribute('href', '/explore/tables'); + expect(topics).toHaveAttribute('href', '/explore/topics'); + expect(dashboards).toHaveAttribute('href', '/explore/dashboards'); + expect(pipelines).toHaveAttribute('href', '/explore/pipelines'); + expect(mlmodel).toHaveAttribute('href', '/explore/mlmodels'); expect(service).toHaveAttribute('href', '/settings/services/databases'); expect(user).toHaveAttribute('href', '/settings/members/users'); expect(terms).toHaveAttribute( diff --git a/openmetadata-ui/src/main/resources/ui/src/components/RecentSearchedTerms/RecentSearchedTermsAntd.tsx b/openmetadata-ui/src/main/resources/ui/src/components/RecentSearchedTerms/RecentSearchedTermsAntd.tsx index 459178dc60c..a4558e708aa 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/RecentSearchedTerms/RecentSearchedTermsAntd.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/RecentSearchedTerms/RecentSearchedTermsAntd.tsx @@ -16,7 +16,7 @@ import { RecentlySearchedData } from 'Models'; import React, { FunctionComponent, useEffect, useState } from 'react'; import { useTranslation } from 'react-i18next'; import { Link } from 'react-router-dom'; -import { getExplorePathWithSearch } from '../../constants/constants'; +import { getExplorePath } from '../../constants/constants'; import { getRecentlySearchedData, removeRecentSearchTerm, @@ -74,7 +74,7 @@ const RecentSearchedTermsAntd: FunctionComponent = () => {
+ to={getExplorePath({ search: item.term })}>