Skip to content

Commit ef36dbb

Browse files
authored
fix(bulk-import): correct dataFetcher return type and replace unsafe Response casts (#3004)
The dataFetcher method returns a raw Response on error but the type declared only OrgAndRepoResponse. Fixed the return type union and replaced all unsafe `as Response` casts with `instanceof` narrowing. Also fixed three no-op test assertions that never validated anything. Signed-off-by: Jon Koops <jonkoops@gmail.com>
1 parent 46a666e commit ef36dbb

7 files changed

Lines changed: 35 additions & 16 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@red-hat-developer-hub/backstage-plugin-bulk-import': patch
3+
---
4+
5+
Corrected `dataFetcher` return type to include `Response` and replaced unsafe type casts with `instanceof` narrowing.

workspaces/bulk-import/plugins/bulk-import/src/api/BulkImportBackendClient.test.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ describe('BulkImportBackendClient with open-pull-requests', () => {
348348

349349
await expect(
350350
bulkImportApi.dataFetcher(1, 2, '', ApprovalTool.Git),
351-
).resolves.toEqual(expect.objectContaining([]));
351+
).resolves.toBeInstanceOf(Response);
352352
});
353353
});
354354

@@ -427,7 +427,7 @@ describe('BulkImportBackendClient with open-pull-requests', () => {
427427
bulkImportApi.dataFetcher(1, 2, '', ApprovalTool.Git, {
428428
fetchOrganizations: true,
429429
}),
430-
).resolves.toEqual(expect.objectContaining([]));
430+
).resolves.toBeInstanceOf(Response);
431431
});
432432
});
433433

@@ -459,6 +459,12 @@ describe('BulkImportBackendClient with open-pull-requests', () => {
459459
});
460460

461461
it('getImportJobs should handle non-200/204 responses correctly', async () => {
462+
server.use(
463+
rest.get(`${LOCAL_ADDR}/api/bulk-import/imports`, (_req, res, ctx) => {
464+
return res(ctx.status(404));
465+
}),
466+
);
467+
462468
await expect(
463469
bulkImportApi.getImportJobs(
464470
1,
@@ -467,7 +473,7 @@ describe('BulkImportBackendClient with open-pull-requests', () => {
467473
AddedRepositoryColumnNameEnum.repoName,
468474
SortingOrderEnum.Asc,
469475
),
470-
).resolves.toEqual(expect.objectContaining([]));
476+
).resolves.toBeInstanceOf(Response);
471477
});
472478
});
473479

workspaces/bulk-import/plugins/bulk-import/src/api/BulkImportBackendClient.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ export type BulkImportAPI = {
4747
searchString: string,
4848
approvalTool: ApprovalTool,
4949
options?: APITypes,
50-
) => Promise<OrgAndRepoResponse>;
50+
) => Promise<OrgAndRepoResponse | Response>;
5151
getImportJobs: (
5252
page: number,
5353
size: number,

workspaces/bulk-import/plugins/bulk-import/src/components/PreviewFile/PreviewFileSidebar.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,14 +95,14 @@ export const PreviewFileSidebar = ({
9595
branch || 'main',
9696
approvalTool,
9797
);
98-
if ((result as Response)?.statusText) {
98+
if (result instanceof Response) {
9999
setStatus({
100100
...status,
101101
errors: {
102102
...(status?.errors || {}),
103103
[id]: {
104104
error: {
105-
title: (result as Response)?.statusText,
105+
title: result.statusText,
106106
message: [t('previewFile.failedToFetchPR')],
107107
},
108108
},

workspaces/bulk-import/plugins/bulk-import/src/components/Repositories/CatalogInfoAction.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -103,11 +103,11 @@ const CatalogInfoAction = ({ data }: { data: AddRepositoryData }) => {
103103
value?.status === RepositoryStatus.WAIT_PR_APPROVAL &&
104104
values.repositories[(value as ImportJobStatus)?.repository?.id];
105105

106-
if ((value as Response)?.statusText) {
106+
if (value instanceof Response) {
107107
setOpenDrawer(false);
108108
setStatus({
109-
title: (value as Response)?.statusText,
110-
url: (value as Response)?.url,
109+
title: value.statusText,
110+
url: value.url,
111111
});
112112
removeQueryParams();
113113
} else if (shouldOpenPanel) {

workspaces/bulk-import/plugins/bulk-import/src/hooks/useAddedRepositories.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,10 +120,10 @@ export const useAddedRepositories = (
120120
loading: isLoadingTable,
121121
error: {
122122
...(error ?? {}),
123-
...((value as Response)?.statusText
123+
...(value instanceof Response
124124
? {
125125
name: 'Error',
126-
message: (value as Response)?.statusText,
126+
message: value.statusText,
127127
}
128128
: {}),
129129
},

workspaces/bulk-import/plugins/bulk-import/src/hooks/useRepositories.ts

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -158,16 +158,24 @@ export const useRepositories = (
158158
);
159159
}, [options?.showOrganizations, value, user, baseUrl]);
160160

161+
const errors: string[] = [];
162+
163+
if (tokenFetchError) {
164+
errors.push(tokenFetchError.message);
165+
}
166+
167+
if (value instanceof Response) {
168+
errors.push(value.statusText);
169+
} else if (value?.errors && value.errors.length > 0) {
170+
errors.push(...value.errors);
171+
}
172+
161173
return {
162174
loading: tokenLoading || isQueryLoading,
163175
data: prepareData,
164176
error: {
165-
...(tokenFetchError ? { errors: [tokenFetchError.message] } : {}),
166177
...(error ?? {}),
167-
...((value?.errors && value.errors.length > 0) ||
168-
(value as any as Response)?.statusText
169-
? { errors: value?.errors || (value as any as Response)?.statusText }
170-
: {}),
178+
...(errors.length > 0 ? { errors } : {}),
171179
} as RepositoriesError,
172180
};
173181
};

0 commit comments

Comments
 (0)