Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { render, screen, waitFor } from '@testing-library/react';
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
import { AxiosError } from 'axios';
import { DEFAULT_DOMAIN_VALUE } from '../../../constants/constants';
import { EntityType } from '../../../enums/entity.enum';
import { Domain, DomainType } from '../../../generated/entity/domains/domain';
import { EntityReference } from '../../../generated/entity/type';
import * as domainAPI from '../../../rest/domainAPI';
import { convertDomainsToTreeOptions } from '../../../utils/DomainUtils';
import { showErrorToast } from '../../../utils/ToastUtils';
import DomainSelectableTree from './DomainSelectableTree';

const mockDomains: Domain[] = [
Expand Down Expand Up @@ -108,6 +110,11 @@ jest.mock('../../../utils/EntityReferenceUtils', () => ({
jest.mock('../../../utils/StringUtils', () => ({
escapeESReservedCharacters: jest.fn().mockImplementation((value) => value),
getEncodedFqn: jest.fn().mockImplementation((value) => value),
getErrorText: jest
.fn()
.mockImplementation(
(error, fallback) => error?.response?.data?.message ?? fallback
),
}));

jest.mock('../../../utils/ToastUtils', () => ({
Expand Down Expand Up @@ -433,4 +440,111 @@ describe('DomainSelectableTree', () => {
expect(screen.getByText('label.no-entity-available')).toBeInTheDocument();
});
});

it('should show an inline error inside the list when a search fails', async () => {
const error = new AxiosError('Request failed with status code 400');
jest.spyOn(domainAPI, 'searchDomains').mockRejectedValueOnce(error);

renderComponent();

await waitFor(() => {
expect(screen.queryByText('Loader')).not.toBeInTheDocument();
});

fireEvent.change(screen.getByTestId('searchbar'), {
target: { value: 'a||||b' },
});

await waitFor(() => {
expect(screen.getByTestId('domain-search-error')).toBeInTheDocument();
});

expect(screen.getByTestId('retry-domain-search')).toBeInTheDocument();
expect(screen.queryByText('Loader')).not.toBeInTheDocument();
expect(showErrorToast).not.toHaveBeenCalled();
// no message on the error, so the static fallback is shown
expect(screen.getByText('server.entity-fetch-error')).toBeInTheDocument();
});

it("should show the server's own message when the response carries one", async () => {
const error = new AxiosError('Request failed with status code 400');
error.response = {
status: 400,
data: { code: 400, message: 'Invalid field name childrenCount' },
} as never;
jest.spyOn(domainAPI, 'searchDomains').mockRejectedValueOnce(error);

renderComponent();

fireEvent.change(screen.getByTestId('searchbar'), {
target: { value: 'eng' },
});

await waitFor(() => {
expect(
screen.getByText('Invalid field name childrenCount')
).toBeInTheDocument();
});

expect(
screen.queryByText('server.entity-fetch-error')
).not.toBeInTheDocument();
});

it('should retry the search from the inline error and recover', async () => {
jest
.spyOn(domainAPI, 'searchDomains')
.mockRejectedValueOnce(
new AxiosError('Request failed with status code 400')
)
.mockResolvedValueOnce([mockDomains[0]]);

renderComponent();

fireEvent.change(screen.getByTestId('searchbar'), {
target: { value: 'Engineering' },
});

await waitFor(() => {
expect(screen.getByTestId('domain-search-error')).toBeInTheDocument();
});

fireEvent.click(screen.getByTestId('retry-domain-search'));

await waitFor(() => {
expect(
screen.queryByTestId('domain-search-error')
).not.toBeInTheDocument();
});

expect(domainAPI.searchDomains).toHaveBeenCalledTimes(2);
});

it('should clear the inline error when the search box is cleared', async () => {
jest
.spyOn(domainAPI, 'searchDomains')
.mockRejectedValueOnce(
new AxiosError('Request failed with status code 400')
);

renderComponent();

fireEvent.change(screen.getByTestId('searchbar'), {
target: { value: 'a||||b' },
});

await waitFor(() => {
expect(screen.getByTestId('domain-search-error')).toBeInTheDocument();
});

fireEvent.change(screen.getByTestId('searchbar'), {
target: { value: '' },
});

await waitFor(() => {
expect(
screen.queryByTestId('domain-search-error')
).not.toBeInTheDocument();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import {
escapeESReservedCharacters,
getEncodedFqn,
getErrorText,
} from '../../../utils/StringUtils';
import { showErrorToast } from '../../../utils/ToastUtils';
import Loader from '../Loader/Loader';
Expand Down Expand Up @@ -100,6 +101,7 @@
>
>({});
const [domainMapper, setDomainMapper] = useState<Record<string, Domain>>({});
const [searchError, setSearchError] = useState<AxiosError>();

const { activeDomain } = useDomainStore();
const pagingRef = useRef(INITIAL_PAGING_STATE);
Expand Down Expand Up @@ -352,6 +354,7 @@
async (isLoadMore = false) => {
const setLoadingState = isLoadMore ? setIsLoadingMore : setIsLoading;
setLoadingState(true);
setSearchError(undefined);

if (!isLoadMore) {
setPaging(INITIAL_PAGING_STATE);
Expand Down Expand Up @@ -399,10 +402,10 @@
setLoadingState(false);
}
},
[domains, isMultiple, initialDomains, restrictedDomains]

Check warning on line 405 in openmetadata-ui/src/main/resources/ui/src/components/common/DomainSelectableTree/DomainSelectableTree.tsx

View workflow job for this annotation

GitHub Actions / checkstyle

React Hook useCallback has an unnecessary dependency: 'initialDomains'. Either exclude it or remove the dependency array
);

const onSelect = (selectedKeys: React.Key[]) => {

Check warning on line 408 in openmetadata-ui/src/main/resources/ui/src/components/common/DomainSelectableTree/DomainSelectableTree.tsx

View workflow job for this annotation

GitHub Actions / checkstyle

The 'onSelect' function makes the dependencies of useMemo Hook (at line 626) change on every render. Move it inside the useMemo callback. Alternatively, wrap the definition of 'onSelect' in its own useCallback() Hook
if (!isMultiple) {
if (selectedKeys.length === 0 && !isClearable) {
return;
Expand All @@ -421,7 +424,7 @@
}
};

const onCheck = (

Check warning on line 427 in openmetadata-ui/src/main/resources/ui/src/components/common/DomainSelectableTree/DomainSelectableTree.tsx

View workflow job for this annotation

GitHub Actions / checkstyle

The 'onCheck' function makes the dependencies of useMemo Hook (at line 626) change on every render. Move it inside the useMemo callback. Alternatively, wrap the definition of 'onCheck' in its own useCallback() Hook
checked: Key[] | { checked: Key[]; halfChecked: Key[] }
): void => {
if (Array.isArray(checked)) {
Expand All @@ -443,12 +446,13 @@
}
};

const onSearch = useCallback(

Check warning on line 449 in openmetadata-ui/src/main/resources/ui/src/components/common/DomainSelectableTree/DomainSelectableTree.tsx

View workflow job for this annotation

GitHub Actions / checkstyle

React Hook useCallback received a function whose dependencies are unknown. Pass an inline function instead
debounce(async (value: string) => {
setSearchTerm(value);
if (value) {
try {
setIsLoading(true);
setSearchError(undefined);
const encodedValue = getEncodedFqn(escapeESReservedCharacters(value));
const results: Domain[] = await searchDomains(encodedValue);
const filteredResults = restrictedDomains?.length
Expand All @@ -475,6 +479,9 @@
);
setTreeData(updatedTreeData);
setDomains(uniqueData);
} catch (error) {
setSearchError(error as AxiosError);
setTreeData([]);
} finally {
setIsLoading(false);
}
Expand Down Expand Up @@ -525,12 +532,40 @@
fetchAPI(true);
}
},
[hasMore, isLoadingMore, isLoading, searchTerm, fetchAPI, domains.length]

Check warning on line 535 in openmetadata-ui/src/main/resources/ui/src/components/common/DomainSelectableTree/DomainSelectableTree.tsx

View workflow job for this annotation

GitHub Actions / checkstyle

React Hook useCallback has an unnecessary dependency: 'domains.length'. Either exclude it or remove the dependency array
);

const treeContent = useMemo(() => {
if (isLoading) {
return <Loader />;
} else if (searchError) {
return (
<Box
align="center"
className="tw:py-6"
data-testid="domain-search-error"
direction="col"
gap={2}
justify="center">
<Typography
className="tw:text-text-tertiary tw:px-4 tw:text-center tw:break-words tw:line-clamp-4"
size="text-sm">
{getErrorText(
searchError,
t('server.entity-fetch-error', {
entity: t('label.domain-plural'),
})
)}
Comment thread
harsh-vador marked this conversation as resolved.
</Typography>
<Button
color="link-color"
data-testid="retry-domain-search"
size="sm"
onClick={() => onSearch(searchTerm)}>
{t('label.try-again')}
</Button>
</Box>
);
} else if (treeData.length === 0) {
return (
<Box
Expand Down Expand Up @@ -589,9 +624,11 @@
);
}
}, [
searchError,
isLoading,
isLoadingMore,
isSubmitLoading,
onSearch,
treeData,
value,
onSelect,
Expand All @@ -609,7 +646,7 @@
setSearchTerm('');
fetchAPI();
}
}, [visible]);

Check warning on line 649 in openmetadata-ui/src/main/resources/ui/src/components/common/DomainSelectableTree/DomainSelectableTree.tsx

View workflow job for this annotation

GitHub Actions / checkstyle

React Hook useEffect has a missing dependency: 'fetchAPI'. Either include it or remove the dependency array

useEffect(() => {
const loadSelectedDomainChildren = async () => {
Expand All @@ -633,7 +670,7 @@
};

loadSelectedDomainChildren();
}, [value, visible, domainMapper]);

Check warning on line 673 in openmetadata-ui/src/main/resources/ui/src/components/common/DomainSelectableTree/DomainSelectableTree.tsx

View workflow job for this annotation

GitHub Actions / checkstyle

React Hook useEffect has missing dependencies: 'loadChildDomains' and 'loadingChildren'. Either include them or remove the dependency array

const handleAllDomainKeyPress = (e: React.KeyboardEvent<HTMLDivElement>) => {
// To pass Sonar test
Expand All @@ -646,7 +683,7 @@
return (
<div className="p-sm" data-testid="domain-selectable-tree">
<Input
autoFocus

Check warning on line 686 in openmetadata-ui/src/main/resources/ui/src/components/common/DomainSelectableTree/DomainSelectableTree.tsx

View workflow job for this annotation

GitHub Actions / checkstyle

The autoFocus prop should not be used, as it can reduce usability and accessibility for users
className="tw:mb-2"
icon={SearchLg}
inputDataTestId="searchbar"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import {
escapeESReservedCharacters,
getEncodedFqn,
getErrorText,
} from '../../../utils/StringUtils';
import { showErrorToast } from '../../../utils/ToastUtils';
import Loader from '../Loader/Loader';
Expand Down Expand Up @@ -99,6 +100,7 @@
>
>({});
const [domainMapper, setDomainMapper] = useState<Record<string, Domain>>({});
const [searchError, setSearchError] = useState<AxiosError>();

const pagingRef = useRef(INITIAL_PAGING_STATE);
const scrollContainerRef = useRef<HTMLDivElement | null>(null);
Expand Down Expand Up @@ -244,7 +246,7 @@
disabled: true,
className: 'load-more-node',
title: (
<div onClick={(e) => e.stopPropagation()}>

Check warning on line 249 in openmetadata-ui/src/main/resources/ui/src/components/common/DomainSelectableTree/DomainSelectableTreeNew.tsx

View workflow job for this annotation

GitHub Actions / checkstyle

Avoid non-native interactive elements. If using native HTML is not possible, add an appropriate role and support for tabbing, mouse, keyboard, and touch inputs to an interactive content element

Check warning on line 249 in openmetadata-ui/src/main/resources/ui/src/components/common/DomainSelectableTree/DomainSelectableTreeNew.tsx

View workflow job for this annotation

GitHub Actions / checkstyle

Visible, non-interactive elements with click handlers must have at least one keyboard listener
<Button
color="link-color"
iconLeading={isLoadingMore ? undefined : Plus}
Expand Down Expand Up @@ -346,6 +348,7 @@
async (isLoadMore = false) => {
const setLoadingState = isLoadMore ? setIsLoadingMore : setIsLoading;
setLoadingState(true);
setSearchError(undefined);

if (!isLoadMore) {
setPaging(INITIAL_PAGING_STATE);
Expand Down Expand Up @@ -451,6 +454,7 @@
if (value) {
try {
setIsLoading(true);
setSearchError(undefined);
const encodedValue = getEncodedFqn(escapeESReservedCharacters(value));
const results: Domain[] = await searchDomains(encodedValue);

Expand All @@ -473,6 +477,9 @@
);
setTreeData(updatedTreeData);
setDomains(uniqueData);
} catch (error) {
setSearchError(error as AxiosError);
setTreeData([]);
} finally {
setIsLoading(false);
}
Expand Down Expand Up @@ -523,6 +530,29 @@
const treeContent = useMemo(() => {
if (isLoading) {
return <Loader />;
} else if (searchError) {
return (
<div
className="tw:py-4 tw:text-center tw:text-sm tw:text-tertiary"
data-testid="domain-search-error">
<div className="tw:px-4 tw:break-words tw:line-clamp-4">
{getErrorText(
searchError,
t('server.entity-fetch-error', {
entity: t('label.domain-plural'),
})
)}
</div>
<Button
className="tw:mt-2"
color="link-color"
data-testid="retry-domain-search"
size="sm"
onClick={() => onSearch(searchTerm)}>
{t('label.try-again')}
</Button>
</div>
);
} else if (treeData.length === 0) {
return (
<div className="tw:py-4 tw:text-center tw:text-sm tw:text-tertiary">
Expand Down Expand Up @@ -564,8 +594,10 @@
);
}
}, [
searchError,
isLoading,
isLoadingMore,
onSearch,
treeData,
selectedKeys,
onSelect,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ jest.mock('./i18next/LocalUtil', () => ({
import { AxiosError } from 'axios';
import {
decodeHtmlEntities,
escapeESReservedCharacters,
formatJsonString,
getDecodedFqn,
getEncodedFqn,
Expand Down Expand Up @@ -452,4 +453,33 @@ describe('StringUtils', () => {
expect(result).not.toContain('line-four');
});
});

describe('escapeESReservedCharacters', () => {
it('should escape every Lucene reserved character', () => {
expect(escapeESReservedCharacters('a+b-c=d&e')).toBe(
String.raw`a\+b\-c\=d\&e`
);
expect(escapeESReservedCharacters('(a){b}[c]')).toBe(
String.raw`\(a\)\{b\}\[c\]`
);
expect(escapeESReservedCharacters('a*b?c:d/e')).toBe(
String.raw`a\*b\?c\:d\/e`
);
});

it('should escape a single pipe so consecutive pipes cannot form an OR operator', () => {
expect(escapeESReservedCharacters('a|b')).toBe(String.raw`a\|b`);
expect(escapeESReservedCharacters('a||||b')).toBe(String.raw`a\|\|\|\|b`);
});

it('should leave a plain term untouched', () => {
expect(escapeESReservedCharacters('Customer Support')).toBe(
'Customer Support'
);
});

it('should return an empty string for an undefined term', () => {
expect(escapeESReservedCharacters()).toBe('');
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ export const ES_RESERVED_CHARACTERS: Record<string, string> = {
'=': String.raw`\=`,
'&': String.raw`\&`,
'&&': String.raw`\&&`,
'|': String.raw`\|`,
'||': String.raw`\||`,
'>': String.raw`\>`,
'<': String.raw`\<`,
Expand Down
Loading