Skip to content

Commit

Permalink
feat(api): add handling empty metadata suggestions (#3608)
Browse files Browse the repository at this point in the history 
* feat: add handling empty metadata suggestions

* fix: const value naming

---------

Co-authored-by: Jakub Kida <jkida@box.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: greg-in-a-box <103291617+greg-in-a-box@users.noreply.github.com>
  • Loading branch information
@JakubKida @mergify @greg-in-a-box
4 people authored Aug 14, 2024
1 parent 4bfceaa commit ecd4c80
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 28 deletions.
8 changes: 8 additions & 0 deletions src/api/Metadata.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import getProp from 'lodash/get';
import uniqueId from 'lodash/uniqueId';
import isEmpty from 'lodash/isEmpty';
import { getBadItemError, getBadPermissionsError, isUserCorrectableError } from '../utils/error';
import { getTypedFileId } from '../utils/file';
import File from './File';
Expand All @@ -28,6 +29,7 @@ import {
ERROR_CODE_FETCH_METADATA_TEMPLATES,
ERROR_CODE_FETCH_SKILLS,
ERROR_CODE_FETCH_METADATA_SUGGESTIONS,
ERROR_CODE_EMPTY_METADATA_SUGGESTIONS,
TYPE_FILE,
} from '../constants';

Expand Down Expand Up @@ -786,6 +788,12 @@ class Metadata extends File {
throw e;
}
}

if (!isEmpty(suggestionsResponse) && getProp(suggestionsResponse, 'data.suggestions').length === 0) {
this.errorCode = ERROR_CODE_EMPTY_METADATA_SUGGESTIONS;
throw new Error('No suggestions found.');
}

return getProp(suggestionsResponse, 'data.suggestions', []);
}
}
Expand Down
66 changes: 38 additions & 28 deletions src/api/__tests__/Metadata.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
METADATA_TEMPLATE_CLASSIFICATION,
METADATA_TEMPLATE_PROPERTIES,
TYPE_FILE,
ERROR_CODE_EMPTY_METADATA_SUGGESTIONS,
} from '../../constants';

let metadata: Metadata;
Expand Down Expand Up @@ -578,10 +579,7 @@ describe('api/Metadata', () => {
metadata.getEditors = jest.fn().mockResolvedValueOnce('editors');
metadata.getCustomPropertiesTemplate = jest.fn().mockReturnValueOnce('custom');
metadata.getUserAddableTemplates = jest.fn().mockReturnValueOnce('templates');
metadata.getTemplates = jest
.fn()
.mockResolvedValueOnce('global')
.mockResolvedValueOnce('enterprise');
metadata.getTemplates = jest.fn().mockResolvedValueOnce('global').mockResolvedValueOnce('enterprise');

await metadata.getMetadata(file, jest.fn(), jest.fn(), true, { refreshCache: false });

Expand Down Expand Up @@ -618,10 +616,7 @@ describe('api/Metadata', () => {
metadata.getEditors = jest.fn().mockResolvedValueOnce('editors');
metadata.getCustomPropertiesTemplate = jest.fn().mockReturnValueOnce('custom');
metadata.getUserAddableTemplates = jest.fn().mockReturnValueOnce('templates');
metadata.getTemplates = jest
.fn()
.mockResolvedValueOnce('global')
.mockResolvedValueOnce('enterprise');
metadata.getTemplates = jest.fn().mockResolvedValueOnce('global').mockResolvedValueOnce('enterprise');

await metadata.getMetadata(file, jest.fn(), jest.fn(), true);

Expand Down Expand Up @@ -671,10 +666,7 @@ describe('api/Metadata', () => {
metadata.getEditors = jest.fn().mockResolvedValueOnce('editors');
metadata.getCustomPropertiesTemplate = jest.fn().mockReturnValueOnce('custom');
metadata.getUserAddableTemplates = jest.fn().mockReturnValueOnce('templates');
metadata.getTemplates = jest
.fn()
.mockResolvedValueOnce('global')
.mockResolvedValueOnce('enterprise');
metadata.getTemplates = jest.fn().mockResolvedValueOnce('global').mockResolvedValueOnce('enterprise');

await metadata.getMetadata(file, jest.fn(), jest.fn(), true, { refreshCache: true });

Expand Down Expand Up @@ -726,10 +718,7 @@ describe('api/Metadata', () => {
metadata.getEditors = jest.fn().mockResolvedValueOnce('editors');
metadata.getCustomPropertiesTemplate = jest.fn().mockReturnValueOnce('custom');
metadata.getUserAddableTemplates = jest.fn().mockReturnValueOnce('templates');
metadata.getTemplates = jest
.fn()
.mockResolvedValueOnce('global')
.mockResolvedValueOnce('enterprise');
metadata.getTemplates = jest.fn().mockResolvedValueOnce('global').mockResolvedValueOnce('enterprise');

await metadata.getMetadata(file, jest.fn(), jest.fn(), true, { forceFetch: true });

Expand Down Expand Up @@ -780,10 +769,7 @@ describe('api/Metadata', () => {
metadata.getEditors = jest.fn().mockResolvedValueOnce('editors');
metadata.getCustomPropertiesTemplate = jest.fn().mockReturnValueOnce('custom');
metadata.getUserAddableTemplates = jest.fn().mockReturnValueOnce('templates');
metadata.getTemplates = jest
.fn()
.mockResolvedValueOnce('global')
.mockResolvedValueOnce('enterprise');
metadata.getTemplates = jest.fn().mockResolvedValueOnce('global').mockResolvedValueOnce('enterprise');

await metadata.getMetadata(file, jest.fn(), jest.fn(), false);

Expand Down Expand Up @@ -826,10 +812,7 @@ describe('api/Metadata', () => {
metadata.getEditors = jest.fn().mockResolvedValueOnce('editors');
metadata.getCustomPropertiesTemplate = jest.fn().mockReturnValueOnce('custom');
metadata.getUserAddableTemplates = jest.fn().mockReturnValueOnce('templates');
metadata.getTemplates = jest
.fn()
.mockResolvedValueOnce('global')
.mockResolvedValueOnce('enterprise');
metadata.getTemplates = jest.fn().mockResolvedValueOnce('global').mockResolvedValueOnce('enterprise');

await metadata.getMetadata(file, jest.fn(), jest.fn(), true);

Expand Down Expand Up @@ -866,10 +849,7 @@ describe('api/Metadata', () => {
metadata.getEditors = jest.fn().mockResolvedValueOnce('editors');
metadata.getCustomPropertiesTemplate = jest.fn().mockReturnValueOnce('custom');
metadata.getUserAddableTemplates = jest.fn().mockReturnValueOnce('templates');
metadata.getTemplates = jest
.fn()
.mockResolvedValueOnce('global')
.mockResolvedValueOnce('enterprise');
metadata.getTemplates = jest.fn().mockResolvedValueOnce('global').mockResolvedValueOnce('enterprise');

await metadata.getMetadata(file, jest.fn(), jest.fn(), true, { forceFetch: true });

Expand Down Expand Up @@ -2107,6 +2087,36 @@ describe('api/Metadata', () => {
).rejects.toThrow(new Error(`Invalid confidence level: "high"`));
});

test('should throw an error if no suggestions were found', async () => {
const suggestionsFromServer = [];

metadata.getMetadataSuggestionsUrl = jest.fn().mockReturnValueOnce('suggestions_url');
metadata.xhr.get = jest.fn().mockReturnValueOnce({
data: {
$scope: 'enterprise',
$templateKey: 'templateKey',
suggestions: suggestionsFromServer,
},
});

await expect(() =>
metadata.getMetadataSuggestions('id', TYPE_FILE, 'enterprise', 'templateKey'),
).rejects.toThrow(new Error('No suggestions found.'));

expect(metadata.errorCode).toBe(ERROR_CODE_EMPTY_METADATA_SUGGESTIONS);
expect(metadata.getMetadataSuggestionsUrl).toHaveBeenCalled();
expect(metadata.xhr.get).toHaveBeenCalledWith({
url: 'suggestions_url',
id: 'file_id',
params: {
item: `file_id`,
scope: 'enterprise',
template_key: 'templateKey',
confidence: METADATA_SUGGESTIONS_CONFIDENCE_EXPERIMENTAL,
},
});
});

test('should return empty array of suggestions when error is 400', async () => {
const error = new Error();
error.status = 400;
Expand Down
1 change: 1 addition & 0 deletions src/constants.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ export const ERROR_CODE_FETCH_ACCESS_STATS = 'fetch_access_stats_error';
export const ERROR_CODE_FETCH_SKILLS = 'fetch_skills_error';
export const ERROR_CODE_FETCH_RECENTS = 'fetch_recents_error';
export const ERROR_CODE_FETCH_METADATA_SUGGESTIONS = 'fetch_metadata_suggestions_error';
export const ERROR_CODE_EMPTY_METADATA_SUGGESTIONS = 'empty_metadata_suggestions_error';
export const ERROR_CODE_EXECUTE_INTEGRATION = 'execute_integrations_error';
export const ERROR_CODE_EXTRACT_STRUCTURED = 'extract_structured_error';
export const ERROR_CODE_CREATE_ANNOTATION = 'create_annotation_error';
Expand Down

0 comments on commit ecd4c80

Please sign in to comment.