Skip to content

Commit

Permalink
feat(metadata-sidebar): Add clear error callback
Browse files Browse the repository at this point in the history
  • Loading branch information
@JChan106
JChan106 committed Feb 12, 2025
1 parent 982ded2 commit f2e1b8a
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 14 deletions.
2 changes: 1 addition & 1 deletion src/elements/content-sidebar/MetadataInstanceEditor.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ const MetadataInstanceEditor: React.FC<MetadataInstanceEditorProps> = ({
return (
<MetadataInstanceForm
areAiSuggestionsAvailable={areAiSuggestionsAvailable}
errorCode={errorCode}
isAiSuggestionsFeatureEnabled={isBoxAiSuggestionsEnabled}
isBetaLanguageEnabled={isBetaLanguageEnabled}
isDeleteButtonDisabled={isDeleteButtonDisabled}
Expand All @@ -65,7 +66,6 @@ const MetadataInstanceEditor: React.FC<MetadataInstanceEditorProps> = ({
selectedTemplateInstance={template}
setIsUnsavedChangesModalOpen={setIsUnsavedChangesModalOpen}
taxonomyOptionsFetcher={taxonomyOptionsFetcher}
errorCode={errorCode}
/>
);
};
Expand Down
7 changes: 6 additions & 1 deletion src/elements/content-sidebar/MetadataSidebarRedesign.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ function MetadataSidebarRedesign({
getStructuredTextRep,
}: MetadataSidebarRedesignProps) {
const {
clearExtractError,
extractSuggestions,
file,
handleCreateMetadataInstance,
Expand Down Expand Up @@ -157,6 +158,8 @@ function MetadataSidebarRedesign({
}, [editingTemplate, templateInstances, templateInstances.length]);

const handleTemplateSelect = (selectedTemplate: MetadataTemplate) => {
clearExtractError();

if (editingTemplate) {
setPendingTemplateToEdit(convertTemplateToTemplateInstance(file, selectedTemplate));
setIsUnsavedChangesModalOpen(true);
Expand All @@ -167,6 +170,7 @@ function MetadataSidebarRedesign({
};

const handleCancel = () => {
clearExtractError();
setEditingTemplate(null);
};

Expand All @@ -190,6 +194,7 @@ function MetadataSidebarRedesign({
} catch {
// ignore error, handled in useSidebarMetadataFetcher
}
clearExtractError();
setEditingTemplate(null);
};

Expand Down Expand Up @@ -280,6 +285,7 @@ function MetadataSidebarRedesign({
{editingTemplate && (
<MetadataInstanceEditor
areAiSuggestionsAvailable={areAiSuggestionsAvailable}
errorCode={extractErrorCode}
isBetaLanguageEnabled={isBetaLanguageEnabled}
isBoxAiSuggestionsEnabled={isBoxAiSuggestionsEnabled}
isDeleteButtonDisabled={isDeleteButtonDisabled}
Expand All @@ -291,7 +297,6 @@ function MetadataSidebarRedesign({
setIsUnsavedChangesModalOpen={setIsUnsavedChangesModalOpen}
taxonomyOptionsFetcher={taxonomyOptionsFetcher}
template={editingTemplate}
errorCode={extractErrorCode}
/>
)}
{showList && (
Expand Down
11 changes: 11 additions & 0 deletions src/elements/content-sidebar/__tests__/MetadataSidebarRedesign.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ describe('elements/content-sidebar/Metadata/MetadataSidebarRedesign', () => {

beforeEach(() => {
mockUseSidebarMetadataFetcher.mockReturnValue({
clearExtractError: jest.fn(),
extractSuggestions: jest.fn(),
handleCreateMetadataInstance: jest.fn(),
handleDeleteMetadataInstance: jest.fn(),
Expand All @@ -143,6 +144,7 @@ describe('elements/content-sidebar/Metadata/MetadataSidebarRedesign', () => {

test('should have accessible "All templates" combobox trigger button', () => {
mockUseSidebarMetadataFetcher.mockReturnValue({
clearExtractError: jest.fn(),
extractSuggestions: jest.fn(),
handleCreateMetadataInstance: jest.fn(),
handleDeleteMetadataInstance: jest.fn(),
Expand Down Expand Up @@ -186,6 +188,7 @@ describe('elements/content-sidebar/Metadata/MetadataSidebarRedesign', () => {

test('should have accessible "All templates" combobox trigger button', () => {
mockUseSidebarMetadataFetcher.mockReturnValue({
clearExtractError: jest.fn(),
extractSuggestions: jest.fn(),
handleCreateMetadataInstance: jest.fn(),
handleDeleteMetadataInstance: jest.fn(),
Expand All @@ -207,6 +210,7 @@ describe('elements/content-sidebar/Metadata/MetadataSidebarRedesign', () => {

test('should render metadata sidebar with error', async () => {
mockUseSidebarMetadataFetcher.mockReturnValue({
clearExtractError: jest.fn(),
extractSuggestions: jest.fn(),
handleCreateMetadataInstance: jest.fn(),
handleDeleteMetadataInstance: jest.fn(),
Expand All @@ -231,6 +235,7 @@ describe('elements/content-sidebar/Metadata/MetadataSidebarRedesign', () => {

test('should render metadata sidebar with loading indicator', async () => {
mockUseSidebarMetadataFetcher.mockReturnValue({
clearExtractError: jest.fn(),
extractSuggestions: jest.fn(),
handleCreateMetadataInstance: jest.fn(),
handleDeleteMetadataInstance: jest.fn(),
Expand Down Expand Up @@ -271,6 +276,7 @@ describe('elements/content-sidebar/Metadata/MetadataSidebarRedesign', () => {

test('should render empty state when no visible template instances are present', () => {
mockUseSidebarMetadataFetcher.mockReturnValue({
clearExtractError: jest.fn(),
extractSuggestions: jest.fn(),
handleCreateMetadataInstance: jest.fn(),
handleDeleteMetadataInstance: jest.fn(),
Expand All @@ -294,6 +300,7 @@ describe('elements/content-sidebar/Metadata/MetadataSidebarRedesign', () => {

test('should render metadata instance list when templates are present', () => {
mockUseSidebarMetadataFetcher.mockReturnValue({
clearExtractError: jest.fn(),
extractSuggestions: jest.fn(),
handleCreateMetadataInstance: jest.fn(),
handleDeleteMetadataInstance: jest.fn(),
Expand All @@ -318,6 +325,7 @@ describe('elements/content-sidebar/Metadata/MetadataSidebarRedesign', () => {

test('should render filter dropdown when more than one templates are present', () => {
mockUseSidebarMetadataFetcher.mockReturnValue({
clearExtractError: jest.fn(),
extractSuggestions: jest.fn(),
handleCreateMetadataInstance: jest.fn(),
handleDeleteMetadataInstance: jest.fn(),
Expand Down Expand Up @@ -345,6 +353,7 @@ describe('elements/content-sidebar/Metadata/MetadataSidebarRedesign', () => {
'should not render filter dropdown when only one or none visible template is present',
(templateInstances: MetadataTemplateInstance[]) => {
mockUseSidebarMetadataFetcher.mockReturnValue({
clearExtractError: jest.fn(),
extractSuggestions: jest.fn(),
handleCreateMetadataInstance: jest.fn(),
handleDeleteMetadataInstance: jest.fn(),
Expand All @@ -365,6 +374,7 @@ describe('elements/content-sidebar/Metadata/MetadataSidebarRedesign', () => {

test('should render metadata filterd instance list when fileterd templates are present and matching', () => {
mockUseSidebarMetadataFetcher.mockReturnValue({
clearExtractError: jest.fn(),
extractSuggestions: jest.fn(),
handleCreateMetadataInstance: jest.fn(),
handleDeleteMetadataInstance: jest.fn(),
Expand All @@ -388,6 +398,7 @@ describe('elements/content-sidebar/Metadata/MetadataSidebarRedesign', () => {

test('should render metadata unfiltered instance list when fileterd templates are present and do not match existing templates', () => {
mockUseSidebarMetadataFetcher.mockReturnValue({
clearExtractError: jest.fn(),
extractSuggestions: jest.fn(),
handleCreateMetadataInstance: jest.fn(),
handleDeleteMetadataInstance: jest.fn(),
Expand Down
17 changes: 10 additions & 7 deletions src/elements/content-sidebar/__tests__/useSidebarMetadataFetcher.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -370,15 +370,15 @@ describe('useSidebarMetadataFetcher', () => {
});

test('should handle user correctable error during suggestions extraction', async () => {
mockAPI.extractStructured.mockRejectedValue(mockRateLimitError);
mockAPI.extractStructured.mockRejectedValue({ response: mockRateLimitError });

const { result } = setupHook();
const suggestions = await result.current.extractSuggestions('templateKey', 'global');

expect(suggestions).toEqual([]);
expect(onSuccessMock).not.toHaveBeenCalled();
expect(onErrorMock).toHaveBeenCalledWith(
mockRateLimitError,
{ response: mockRateLimitError },
ERROR_CODE_FETCH_METADATA_SUGGESTIONS,
expect.objectContaining({
showNotification: true,
Expand All @@ -388,11 +388,11 @@ describe('useSidebarMetadataFetcher', () => {
});

test.each`
description | error | expectedErrorCode
${'metadata autofill timeout error'} | ${mockTimeoutError} | ${ERROR_CODE_METADATA_AUTOFILL_TIMEOUT}
${'metadata pre-condition failure error'} | ${mockPreconditionFailedError} | ${ERROR_CODE_METADATA_PRECONDITION_FAILED}
${'internal server error'} | ${mockInternalServerError} | ${ERROR_CODE_UNKNOWN}
`('should set extract error code for $description', async ({ error, expectedErrorCode }) => {
description | error | expectedErrorCode
${'metadata autofill timeout error'} | ${{ response: mockTimeoutError }} | ${ERROR_CODE_METADATA_AUTOFILL_TIMEOUT}
${'metadata pre-condition failure error'} | ${{ response: mockPreconditionFailedError }} | ${ERROR_CODE_METADATA_PRECONDITION_FAILED}
${'internal server error'} | ${{ response: mockInternalServerError }} | ${ERROR_CODE_UNKNOWN}
`('should set extract error code for $description and get cleared', async ({ error, expectedErrorCode }) => {
mockAPI.extractStructured.mockRejectedValue(error);

const { result } = setupHook();
Expand All @@ -402,6 +402,9 @@ describe('useSidebarMetadataFetcher', () => {
expect(onSuccessMock).not.toHaveBeenCalled();
expect(onErrorMock).toHaveBeenCalledWith(error, expectedErrorCode);
await waitFor(() => expect(result.current.extractErrorCode).toEqual(expectedErrorCode));

await result.current.clearExtractError();
await waitFor(() => expect(result.current.extractErrorCode).toBeNull());
});

test('should handle empty suggestions', async () => {
Expand Down
13 changes: 8 additions & 5 deletions src/elements/content-sidebar/hooks/useSidebarMetadataFetcher.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export enum STATUS {
}

interface DataFetcher {
clearExtractError: () => void;
errorMessage: MessageDescriptor | null;
extractErrorCode:
| ERROR_CODE_METADATA_AUTOFILL_TIMEOUT
Expand Down Expand Up @@ -224,19 +225,20 @@ function useSidebarMetadataFetcher(
metadata_template: { template_key: templateKey, scope, type: 'metadata_template' },
})) as Record<string, MetadataFieldValue>;
} catch (error) {
if (error.status === 408) {
// Axios makes the status code nested under the response object
if (error.response.status === 408) {
onError(error, ERROR_CODE_METADATA_AUTOFILL_TIMEOUT);
setExtractErrorCode(ERROR_CODE_METADATA_AUTOFILL_TIMEOUT);
} else if (error.status === 412) {
} else if (error.response.status === 412) {
onError(error, ERROR_CODE_METADATA_PRECONDITION_FAILED);
setExtractErrorCode(ERROR_CODE_METADATA_PRECONDITION_FAILED);
} else if (error.status === 500) {
} else if (error.response.status === 500) {
onError(error, ERROR_CODE_UNKNOWN);
setExtractErrorCode(ERROR_CODE_UNKNOWN);
} else if (isUserCorrectableError(error.status)) {
} else if (isUserCorrectableError(error.response.status)) {
onError(error, ERROR_CODE_FETCH_METADATA_SUGGESTIONS, { showNotification: true });
} else {
onError(error, ERROR_CODE_UNKNOWN);
onError(error, ERROR_CODE_UNKNOWN, { showNotification: true });
// react way of throwing errors from async callbacks - https://github.com/facebook/react/issues/14981#issuecomment-468460187
setError(() => {
throw error;
Expand Down Expand Up @@ -280,6 +282,7 @@ function useSidebarMetadataFetcher(
}, [api, fetchFileErrorCallback, fetchFileSuccessCallback, fileId, status]);

return {
clearExtractError: () => setExtractErrorCode(null),
extractSuggestions,
handleCreateMetadataInstance,
handleDeleteMetadataInstance,
Expand Down

0 comments on commit f2e1b8a

Please sign in to comment.