Infrastructure/logto
0
Fork 0
mirror of https://github.com/logto-io/logto.git synced 2025-01-13 21:30:30 -05:00
Code Issues Activity

refactor(core): remove duplicate findEntityById query before delete

Browse source 
remveo duplication findEntityById query before delete
This commit is contained in:
simeng-li 2023-02-20 11:37:39 +08:00
parent 794405bc1d
commit aa1cd52069
No known key found for this signature in database
GPG key ID: 14EA7BB1541E8075
6 changed files with 13 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
packages/core/src/routes/admin-user.test.ts
View file

@ -374,17 +374,17 @@ describe('adminUserRoutes', () => {
it('DELETE /users/:userId should throw if user cannot be found', async () => { it('DELETE /users/:userId should throw if user cannot be found', async () => {
const notExistedUserId = 'notExistedUserId'; const notExistedUserId = 'notExistedUserId';
const mockedFindUserById = findUserById as jest.Mock;
mockedFindUserById.mockImplementationOnce((userId) => { deleteUserById.mockImplementationOnce((userId) => {
if (userId === notExistedUserId) { if (userId === notExistedUserId) {
throw new Error(' '); throw new Error(' ');
} }
}); });
await expect(userRequest.delete(`/users/${notExistedUserId}`)).resolves.toHaveProperty( await expect(userRequest.delete(`/users/${notExistedUserId}`)).resolves.toHaveProperty(
'status', 'status',
500 500
); );
expect(deleteUserById).not.toHaveBeenCalled();
}); });
it('DELETE /users/:userId/identities/:target should throw if user cannot be found', async () => { it('DELETE /users/:userId/identities/:target should throw if user cannot be found', async () => {

2
packages/core/src/routes/admin-user.ts
View file

@ -277,8 +277,6 @@ export default function adminUserRoutes<T extends AuthedRouter>(
throw new RequestError('user.cannot_delete_self'); throw new RequestError('user.cannot_delete_self');
} }
await findUserById(userId);
await deleteUserById(userId); await deleteUserById(userId);
ctx.status = 204; ctx.status = 204;

6
packages/core/src/routes/application.test.ts
View file

@ -9,6 +9,7 @@ const { jest } = import.meta;
const { mockEsmWithActual } = createMockUtils(jest); const { mockEsmWithActual } = createMockUtils(jest);
const findApplicationById = jest.fn(async () => mockApplication); const findApplicationById = jest.fn(async () => mockApplication);
const deleteApplicationById = jest.fn();
await mockEsmWithActual('@logto/core-kit', () => ({ await mockEsmWithActual('@logto/core-kit', () => ({
// eslint-disable-next-line unicorn/consistent-function-scoping // eslint-disable-next-line unicorn/consistent-function-scoping
@ -21,7 +22,7 @@ const tenantContext = new MockTenant(undefined, {
findTotalNumberOfApplications: jest.fn(async () => ({ count: 10 })), findTotalNumberOfApplications: jest.fn(async () => ({ count: 10 })),
findAllApplications: jest.fn(async () => [mockApplication]), findAllApplications: jest.fn(async () => [mockApplication]),
findApplicationById, findApplicationById,
deleteApplicationById: jest.fn(), deleteApplicationById,
insertApplication: jest.fn( insertApplication: jest.fn(
async (body: CreateApplication): Promise<Application> => ({ async (body: CreateApplication): Promise<Application> => ({
...mockApplication, ...mockApplication,
@ -224,8 +225,7 @@ describe('application route', () => {
}); });
it('DELETE /applications/:applicationId should throw if application not found', async () => { it('DELETE /applications/:applicationId should throw if application not found', async () => {
const mockFindApplicationById = findApplicationById as jest.Mock; deleteApplicationById.mockRejectedValueOnce(new Error(' '));
mockFindApplicationById.mockRejectedValueOnce(new Error(' '));
await expect(applicationRequest.delete('/applications/foo')).resolves.toHaveProperty( await expect(applicationRequest.delete('/applications/foo')).resolves.toHaveProperty(
'status', 'status',

1
packages/core/src/routes/application.ts
View file

@ -132,7 +132,6 @@ export default function applicationRoutes<T extends AuthedRouter>(
async (ctx, next) => { async (ctx, next) => {
const { id } = ctx.guard.params; const { id } = ctx.guard.params;
// Note: will need delete cascade when application is joint with other tables // Note: will need delete cascade when application is joint with other tables
await findApplicationById(id);
await deleteApplicationById(id); await deleteApplicationById(id);
ctx.status = 204; ctx.status = 204;

7
packages/core/src/routes/resource.test.ts
View file

@ -150,6 +150,13 @@ describe('resource routes', () => {
await expect(resourceRequest.delete('/resources/foo')).resolves.toHaveProperty('status', 204); await expect(resourceRequest.delete('/resources/foo')).resolves.toHaveProperty('status', 204);
}); });
it('DELETE /resources/:id should throw with invalid id', async () => {
const { deleteResourceById } = resources;
deleteResourceById.mockRejectedValueOnce(new Error('not found'));
await expect(resourceRequest.delete('/resources/foo')).resolves.toHaveProperty('status', 500);
});
it('GET /resources/:id/scopes', async () => { it('GET /resources/:id/scopes', async () => {
const response = await resourceRequest.get('/resources/foo/scopes'); const response = await resourceRequest.get('/resources/foo/scopes');
expect(response.status).toEqual(200); expect(response.status).toEqual(200);

1
packages/core/src/routes/resource.ts
View file

@ -127,7 +127,6 @@ export default function resourceRoutes<T extends AuthedRouter>(
koaGuard({ params: object({ id: string().min(1) }) }), koaGuard({ params: object({ id: string().min(1) }) }),
async (ctx, next) => { async (ctx, next) => {
const { id } = ctx.guard.params; const { id } = ctx.guard.params;
await findResourceById(id);
await deleteResourceById(id); await deleteResourceById(id);
ctx.status = 204; ctx.status = 204;