Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Onboarding] Introduce search index details page locator and update all reference #205005

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

[Onboarding] Introduce search index details page locator and update all reference #205005

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
761d919
add locator, make index_management use this locator when active solut…
saarikabhasi Dec 12, 2024
031ad33
inital commit for connectors, crawler
saarikabhasi Dec 16, 2024
acdd102
update search application references
saarikabhasi Dec 17, 2024
11de762
clean up
saarikabhasi Dec 18, 2024
d5c621e
use extensionService for index_management details page view
saarikabhasi Dec 19, 2024
fd10893
clean up for ci
saarikabhasi Dec 19, 2024
adba282
Merge branch 'main' into onboarding/search-index-details-locator
saarikabhasi Dec 19, 2024
a813e6d
add new route for document_search in searchIndices plugin
saarikabhasi Dec 19, 2024
7237fb1
Merge branch 'onboarding/search-index-details-locator' of github.com:…
saarikabhasi Dec 19, 2024
adf7a89
update FTR tests for functional_search index management
saarikabhasi Jan 6, 2025
a504b82
[CI] Auto-commit changed files from 'node scripts/yarn_deduplicate'
kibanamachine Jan 6, 2025
4bd6ca6
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine Jan 6, 2025
0eafbd9
fix merge conflict
saarikabhasi Jan 6, 2025
468f3e6
Merge branch 'onboarding/search-index-details-locator' of github.com:…
saarikabhasi Jan 6, 2025
4023011
remove unintended changes
saarikabhasi Jan 6, 2025
ca8dc1d
Merge branch 'main' into onboarding/search-index-details-locator
saarikabhasi Jan 7, 2025
5df9816
add classic nav FTR
saarikabhasi Jan 7, 2025
601f509
Merge branch 'onboarding/search-index-details-locator' of github.com:…
saarikabhasi Jan 7, 2025
8116d7d
enable connector detail page external link to new tab
saarikabhasi Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 49 additions & 27 deletions ...ic/applications/applications/components/search_application/search_application_indices.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import React, { useState } from 'react';
import React, { useCallback, useMemo, useState } from 'react';

import { useActions, useValues } from 'kea';

Expand All @@ -15,22 +15,19 @@ import {
EuiConfirmModal,
EuiIcon,
EuiInMemoryTable,
EuiLink,
EuiSpacer,
EuiTableActionsColumnType,
EuiText,
useEuiBackgroundColor,
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';

import { ENTERPRISE_SEARCH_CONTENT_PLUGIN } from '../../../../../common/constants';
import { EnterpriseSearchApplicationIndex } from '../../../../../common/types/search_applications';

import { SEARCH_INDEX_PATH } from '../../../enterprise_search_content/routes';
import { CANCEL_BUTTON_LABEL } from '../../../shared/constants';
import { indexHealthToHealthColor } from '../../../shared/constants/health_colors';
import { generateEncodedPath } from '../../../shared/encode_path_params';
import { KibanaLogic } from '../../../shared/kibana';
import { EuiLinkTo } from '../../../shared/react_router_helpers';
import { TelemetryLogic } from '../../../shared/telemetry/telemetry_logic';

import { SearchApplicationIndicesLogic } from './search_application_indices_logic';
Expand All @@ -40,8 +37,40 @@ export const SearchApplicationIndices: React.FC = () => {
const { sendEnterpriseSearchTelemetry } = useActions(TelemetryLogic);
const { searchApplicationData } = useValues(SearchApplicationIndicesLogic);
const { removeIndexFromSearchApplication } = useActions(SearchApplicationIndicesLogic);
const { navigateToUrl } = useValues(KibanaLogic);
const { navigateToUrl, share } = useValues(KibanaLogic);
const [removeIndexConfirm, setConfirmRemoveIndex] = useState<string | null>(null);
const searchIndicesLocator = useMemo(
() => share?.url.locators.get('SEARCH_INDEX_DETAILS_LOCATOR_ID'),
[share]
);

const SearchIndicesLinkProps = useCallback(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ This is not a good way to use useCallback(). That should be used if you want to create a onClick handler for the EuiLink. This is being used to generate props at render time, which feels like a code smell to me.

What you likely want is something more like this:

const onSearchIndicesClick = useCallback(
  (event: React.MouseEvent<HTMLAnchorElement>, indexName: string) => {
    if (!searchIndicesLocator) return;
    e.prevenDefault();
    searchIndicesLocator.getUrl({ indexName }).then((url) => {
      navigateToUrl(url, {
        shouldNotCreateHref: true,
        shouldNotPrepend: true,
      });
    });            
  },
  [navigateToUrl, searchIndicesLocator]
);
...
health === 'unknown' ? name : <EuiLink data-test-subj='search-application-index-link' href={searchIndicesLocator.getRedirectUrl({})} onClick={(e) => onSearchIndicesClick(e, name)}>{name}</EuiLink>

but the promise in the useCallback is problematic, but likely the best option if we do this here. A plus would be this function could be shared between the link and and the action instead of having two handlers.

BUT a better option might be to create a link component that takes the index name as a prop and handles all this logic with the locator and share plugin start in that component which is rendered in the column render() it could then be used in this file and the flyout

(indexName: string) => {
const viewIndicesDefaultProps = {
'data-test-subj': 'search-application-index-link',
};
if (searchIndicesLocator) {
return {
...viewIndicesDefaultProps,
href: searchIndicesLocator.getRedirectUrl({}),
onClick: async (event: React.MouseEvent<HTMLAnchorElement>) => {
event.preventDefault();
const url = await searchIndicesLocator.getUrl({ indexName });
navigateToUrl(url, {
shouldNotCreateHref: true,
shouldNotPrepend: true,
});
},
};
} else {
return {
...viewIndicesDefaultProps,
disabled: true,
};
}
},
[navigateToUrl, searchIndicesLocator]
);

if (!searchApplicationData) return null;
const { indices } = searchApplicationData;
Expand Down Expand Up @@ -91,19 +120,7 @@ export const SearchApplicationIndices: React.FC = () => {
}
),
render: ({ health, name }: EnterpriseSearchApplicationIndex) =>
health === 'unknown' ? (
name
) : (
<EuiLinkTo
data-test-subj="search-application-index-link"
to={`${ENTERPRISE_SEARCH_CONTENT_PLUGIN.URL}${generateEncodedPath(SEARCH_INDEX_PATH, {
indexName: name,
})}`}
shouldNotCreateHref
>
{name}
</EuiLinkTo>
),
health === 'unknown' ? name : <EuiLink {...SearchIndicesLinkProps(name)}>{name}</EuiLink>,
sortable: ({ name }: EnterpriseSearchApplicationIndex) => name,
truncateText: true,
width: '40%',
Expand Down Expand Up @@ -148,6 +165,7 @@ export const SearchApplicationIndices: React.FC = () => {
{
actions: [
{
enabled: () => searchIndicesLocator !== undefined,
available: (index) => index.health !== 'unknown',
'data-test-subj': 'search-application-view-index-btn',
description: i18n.translate(
Expand All @@ -168,15 +186,19 @@ export const SearchApplicationIndices: React.FC = () => {
},
}
),
onClick: (index) =>
navigateToUrl(
`${ENTERPRISE_SEARCH_CONTENT_PLUGIN.URL}/${generateEncodedPath(SEARCH_INDEX_PATH, {
indexName: index.name,
})}`,
{

onClick: async (index) => {
if (searchIndicesLocator) {
const url = await searchIndicesLocator.getUrl({ indexName: index.name });
navigateToUrl(url, {
shouldNotCreateHref: true,
}
),
shouldNotPrepend: true,
});
} else {
return undefined;
}
},

type: 'icon',
},
...(indices.length > 1 ? [removeIndexAction] : []),
Expand Down
52 changes: 35 additions & 17 deletions ...cations/applications/components/search_applications/search_application_indices_flyout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import React from 'react';
import React, { useCallback, useMemo } from 'react';

import { useValues, useActions } from 'kea';

Expand All @@ -16,6 +16,7 @@ import {
EuiFlyoutBody,
EuiFlyoutHeader,
EuiIcon,
EuiLink,
EuiSpacer,
EuiText,
EuiTitle,
Expand All @@ -25,14 +26,11 @@ import { i18n } from '@kbn/i18n';

import { FormattedMessage } from '@kbn/i18n-react';

import { ENTERPRISE_SEARCH_CONTENT_PLUGIN } from '../../../../../common/constants';

import { EnterpriseSearchApplicationIndex } from '../../../../../common/types/search_applications';

import { SEARCH_INDEX_PATH } from '../../../enterprise_search_content/routes';
import { healthColorsMap } from '../../../shared/constants/health_colors';
import { generateEncodedPath } from '../../../shared/encode_path_params';
import { EuiLinkTo } from '../../../shared/react_router_helpers';

import { KibanaLogic } from '../../../shared/kibana';

import { SearchApplicationIndicesFlyoutLogic } from './search_application_indices_flyout_logic';

Expand All @@ -44,7 +42,36 @@ export const SearchApplicationIndicesFlyout: React.FC = () => {
isFlyoutVisible,
} = useValues(SearchApplicationIndicesFlyoutLogic);
const { closeFlyout } = useActions(SearchApplicationIndicesFlyoutLogic);

const { navigateToUrl, share } = useValues(KibanaLogic);
const searchIndicesLocator = useMemo(
() => share?.url.locators.get('SEARCH_INDEX_DETAILS_LOCATOR_ID'),
[share]
);
const SearchIndicesLinkProps = useCallback(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see other comment.

(indexName: string) => {
const viewIndicesLinkDefaultProps = {
'data-test-subj': 'search-application-index-link',
'data-telemetry-id': 'entSearchApplications-list-viewIndex',
};
if (searchIndicesLocator) {
return {
...viewIndicesLinkDefaultProps,
href: searchIndicesLocator.getRedirectUrl({}),
onClick: async (event: React.MouseEvent<HTMLAnchorElement>) => {
event.preventDefault();
const url = await searchIndicesLocator.getUrl({ indexName });
navigateToUrl(url, {
shouldNotCreateHref: true,
shouldNotPrepend: true,
});
},
};
} else {
return { ...viewIndicesLinkDefaultProps, disabled: true };
}
},
[navigateToUrl, searchIndicesLocator]
);
if (!searchApplicationData) return null;
const { indices } = searchApplicationData;

Expand All @@ -58,16 +85,7 @@ export const SearchApplicationIndicesFlyout: React.FC = () => {
}
),
render: (indexName: string) => (
<EuiLinkTo
data-test-subj="search-application-index-link"
data-telemetry-id="entSearchApplications-list-viewIndex"
to={`${ENTERPRISE_SEARCH_CONTENT_PLUGIN.URL}${generateEncodedPath(SEARCH_INDEX_PATH, {
indexName,
})}`}
shouldNotCreateHref
>
{indexName}
</EuiLinkTo>
<EuiLink {...SearchIndicesLinkProps(indexName)}>{indexName}</EuiLink>
),
sortable: true,
truncateText: true,
Expand Down
46 changes: 34 additions & 12 deletions ...erprise_search_content/components/connector_detail/components/generated_config_fields.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
* 2.0.
*/

import React, { useState } from 'react';
import React, { useMemo, useState } from 'react';

import { useValues } from 'kea';

import {
EuiButtonIcon,
Expand All @@ -28,10 +30,11 @@ import { Connector } from '@kbn/search-connectors';

import { MANAGE_API_KEYS_URL } from '../../../../../../common/constants';
import { generateEncodedPath } from '../../../../shared/encode_path_params';
import { KibanaLogic } from '../../../../shared/kibana';
import { EuiLinkTo } from '../../../../shared/react_router_helpers';

import { ApiKey } from '../../../api/connector/generate_connector_api_key_api_logic';
import { CONNECTOR_DETAIL_PATH, SEARCH_INDEX_PATH } from '../../../routes';
import { CONNECTOR_DETAIL_PATH } from '../../../routes';

export interface GeneratedConfigFieldsProps {
apiKey?: ApiKey;
Expand Down Expand Up @@ -84,7 +87,7 @@ export const GeneratedConfigFields: React.FC<GeneratedConfigFieldsProps> = ({
isGenerateLoading,
}) => {
const [isModalVisible, setIsModalVisible] = useState(false);

const { navigateToUrl, share } = useValues(KibanaLogic);
const refreshButtonClick = () => {
setIsModalVisible(true);
};
Expand All @@ -96,6 +99,33 @@ export const GeneratedConfigFields: React.FC<GeneratedConfigFieldsProps> = ({
if (generateApiKey) generateApiKey();
setIsModalVisible(false);
};
const searchIndexDetailsUrl = share?.url.locators
.get('SEARCH_INDEX_DETAILS_LOCATOR_ID')
?.useUrl({ indexName: connector?.index_name });
Comment on lines +102 to +104
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this usage of the locator so different from the others? if we can make a sync call that returns the value or undefined we should likely prefer this pattern.


const SearchIndicesLinkProps = useMemo(() => {
const props = {
target: '_blank',
};
if (searchIndexDetailsUrl) {
return {
...props,
href: searchIndexDetailsUrl,
onClick: async (event: React.MouseEvent<HTMLAnchorElement>) => {
event.stopImmediatePropagation();
navigateToUrl(searchIndexDetailsUrl, {
shouldNotCreateHref: true,
shouldNotPrepend: true,
});
},
};
} else {
return {
...props,
disabled: true,
};
}
}, [navigateToUrl, searchIndexDetailsUrl]);

const showApiKeyInfoForSelfManagedConnector = !connector.is_native;
const showApiKeyBanner = showApiKeyInfoForSelfManagedConnector && apiKey?.encoded;
Expand Down Expand Up @@ -184,15 +214,7 @@ export const GeneratedConfigFields: React.FC<GeneratedConfigFieldsProps> = ({
</EuiFlexItem>
<EuiFlexItem>
{connector.index_name && (
<EuiLinkTo
external
target="_blank"
to={generateEncodedPath(SEARCH_INDEX_PATH, {
indexName: connector.index_name,
})}
>
{connector.index_name}
</EuiLinkTo>
<EuiLink {...SearchIndicesLinkProps}>{connector.index_name}</EuiLink>
)}
</EuiFlexItem>
<EuiFlexItem />
Expand Down
39 changes: 32 additions & 7 deletions .../public/applications/enterprise_search_content/components/connectors/connectors_table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import React from 'react';
import React, { useCallback, useMemo } from 'react';

import { useValues } from 'kea';

Expand All @@ -17,6 +17,7 @@ import {
EuiFlexGroup,
EuiFlexItem,
EuiText,
EuiLink,
} from '@elastic/eui';

import { i18n } from '@kbn/i18n';
Expand Down Expand Up @@ -57,7 +58,33 @@ export const ConnectorsTable: React.FC<ConnectorsTableProps> = ({
isLoading,
onDelete,
}) => {
const { navigateToUrl } = useValues(KibanaLogic);
const { navigateToUrl, share } = useValues(KibanaLogic);
const searchIndicesLocator = useMemo(
() => share?.url.locators.get('SEARCH_INDEX_DETAILS_LOCATOR_ID'),
[share]
);

const SearchIndicesLinkProps = useCallback(
(indexName: string) => {
if (searchIndicesLocator) {
return {
href: searchIndicesLocator.getRedirectUrl({}),
onClick: async (event: React.MouseEvent<HTMLAnchorElement>) => {
event.preventDefault();
const url = await searchIndicesLocator.getUrl({ indexName });
navigateToUrl(url, {
shouldNotCreateHref: true,
shouldNotPrepend: true,
});
},
};
} else {
return null;
}
},
[navigateToUrl, searchIndicesLocator]
);

const columns: Array<EuiBasicTableColumn<ConnectorViewItem>> = [
...(!isCrawler
? [
Expand Down Expand Up @@ -88,12 +115,10 @@ export const ConnectorsTable: React.FC<ConnectorsTableProps> = ({
),
render: (connector: ConnectorViewItem) =>
connector.index_name ? (
connector.indexExists ? (
<EuiLinkTo
to={generateEncodedPath(SEARCH_INDEX_PATH, { indexName: connector.index_name })}
>
connector.indexExists && searchIndicesLocator ? (
<EuiLink {...SearchIndicesLinkProps(connector.index_name)}>
{connector.index_name}
</EuiLinkTo>
</EuiLink>
) : (
connector.index_name
)
Expand Down
1 change: 1 addition & 0 deletions x-pack/solutions/search/plugins/search_indices/common/routes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ export const GET_USER_PRIVILEGES_ROUTE = '/internal/search_indices/start_privile
export const POST_CREATE_INDEX_ROUTE = '/internal/search_indices/indices/create';

export const INDEX_DOCUMENT_ROUTE = '/internal/search_indices/{indexName}/documents/{id}';
export const SEARCH_DOCUMENTS_ROUTE = '/internal/search_indices/{indexName}/documents/search';
2 changes: 1 addition & 1 deletion x-pack/solutions/search/plugins/search_indices/public/hooks/api/use_document_search.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export const useIndexDocumentSearch = (indexName: string) => {
refetchIntervalInBackground: true,
refetchOnWindowFocus: 'always',
queryFn: async ({ signal }) =>
http.post<IndexDocuments>(`/internal/serverless_search/indices/${indexName}/search`, {
http.post<IndexDocuments>(`/internal/search_indices/${indexName}/documents/search`, {
body: JSON.stringify({
searchQuery: '',
trackTotalHits: true,
Expand Down
Loading