From 42e9513ca14631573716c3f03869e8d15e5a85a6 Mon Sep 17 00:00:00 2001 From: simeng-li Date: Tue, 23 Jan 2024 11:22:26 +0800 Subject: [PATCH] fix(console,core): fix application count bug (#5270) fix application count bug --- .../hooks/use-application-data.ts | 1 + packages/core/src/queries/application.ts | 4 ++-- .../src/routes/applications/application.ts | 23 +++++++++++++++---- .../integration-tests/src/api/application.ts | 4 ++-- .../tests/api/application/application.test.ts | 23 +++++++++++++++++-- 5 files changed, 45 insertions(+), 10 deletions(-) diff --git a/packages/console/src/pages/Applications/hooks/use-application-data.ts b/packages/console/src/pages/Applications/hooks/use-application-data.ts index e1d333e02..584eb1bba 100644 --- a/packages/console/src/pages/Applications/hooks/use-application-data.ts +++ b/packages/console/src/pages/Applications/hooks/use-application-data.ts @@ -64,6 +64,7 @@ const useApplicationsData = (isThirdParty = false) => { buildUrl(applicationsEndpoint, { page: String(firstPartyApplicationPage), page_size: String(pageSize), + isThirdParty: 'false', }), [firstPartyApplicationPage] ); diff --git a/packages/core/src/queries/application.ts b/packages/core/src/queries/application.ts index 3c01fb217..ca1e100be 100644 --- a/packages/core/src/queries/application.ts +++ b/packages/core/src/queries/application.ts @@ -73,7 +73,7 @@ export const createApplicationQueries = (pool: CommonQueryMethods) => { ? sql`${fields.id} not in (${sql.join(excludeApplicationIds, sql`, `)})` : sql``, types && types.length > 0 ? sql`${fields.type} in (${sql.join(types, sql`, `)})` : sql``, - isThirdParty ? sql`${fields.isThirdParty} = true` : sql`${fields.isThirdParty} = false`, + typeof isThirdParty === 'boolean' ? sql`${fields.isThirdParty} = ${isThirdParty}` : sql``, buildApplicationConditions(search), ])} `); @@ -118,7 +118,7 @@ export const createApplicationQueries = (pool: CommonQueryMethods) => { ? sql`${fields.id} not in (${sql.join(excludeApplicationIds, sql`, `)})` : sql``, types && types.length > 0 ? sql`${fields.type} in (${sql.join(types, sql`, `)})` : sql``, - isThirdParty ? sql`${fields.isThirdParty} = true` : sql`${fields.isThirdParty} = false`, + typeof isThirdParty === 'boolean' ? sql`${fields.isThirdParty} = ${isThirdParty}` : sql``, buildApplicationConditions(search), ])} order by ${fields.createdAt} desc diff --git a/packages/core/src/routes/applications/application.ts b/packages/core/src/routes/applications/application.ts index 87371dc36..93dd438b8 100644 --- a/packages/core/src/routes/applications/application.ts +++ b/packages/core/src/routes/applications/application.ts @@ -56,6 +56,19 @@ export default function applicationRoutes( } = queries.applicationsRoles; const { findRoleByRoleName } = queries.roles; + const parseIsThirdPartQueryParam = (isThirdPartyQuery: 'true' | 'false ' | undefined) => { + // FIXME: @simeng-li Remove this guard once Logto as IdP is ready + if (!EnvSet.values.isDevFeaturesEnabled) { + return false; + } + + if (isThirdPartyQuery === undefined) { + return; + } + + return isThirdPartyQuery === 'true'; + }; + router.get( '/applications', koaPagination({ isOptional: true }), @@ -72,7 +85,9 @@ export default function applicationRoutes( excludeRoleId: string().optional(), // FIXME: @simeng-li Remove this guard once Logto as IdP is ready ...conditional( - EnvSet.values.isDevFeaturesEnabled && { isThirdParty: z.literal('true').optional() } + EnvSet.values.isDevFeaturesEnabled && { + isThirdParty: z.union([z.literal('true'), z.literal('false')]).optional(), + } ), }), response: z.array(applicationResponseGuard), @@ -81,10 +96,10 @@ export default function applicationRoutes( async (ctx, next) => { const { limit, offset, disabled: paginationDisabled } = ctx.pagination; const { searchParams } = ctx.URL; - const { types, excludeRoleId } = ctx.guard.query; + const { types, excludeRoleId, isThirdParty: isThirdPartyParam } = ctx.guard.query; - // FIXME: @simeng-li Remove this guard once Logto as IdP is ready - const isThirdParty = Boolean(ctx.guard.query.isThirdParty); + // @ts-expect-error FIXME: unknown type will be fixed once we have the isDevFeaturesEnabled guard removed + const isThirdParty = parseIsThirdPartQueryParam(isThirdPartyParam); // This will only parse the `search` query param, other params will be ignored. Please use query guard to validate them. const search = parseSearchParamsForSearch(searchParams); diff --git a/packages/integration-tests/src/api/application.ts b/packages/integration-tests/src/api/application.ts index 67d9e693a..70d059ffb 100644 --- a/packages/integration-tests/src/api/application.ts +++ b/packages/integration-tests/src/api/application.ts @@ -28,7 +28,7 @@ export const createApplication = async ( export const getApplications = async ( types?: ApplicationType[], searchParameters?: Record, - isThirdParty?: boolean + isThirdParty?: 'true' | 'false' ) => { const searchParams = new URLSearchParams([ ...(conditional(types && types.length > 0 && types.map((type) => ['types', type])) ?? []), @@ -37,7 +37,7 @@ export const getApplications = async ( Object.keys(searchParameters).length > 0 && Object.entries(searchParameters).map(([key, value]) => [key, value]) ) ?? []), - ...(conditional(isThirdParty && [['isThirdParty', 'true']]) ?? []), + ...(conditional(isThirdParty && [['isThirdParty', isThirdParty]]) ?? []), ]); return authedAdminApi.get('applications', { searchParams }).json(); diff --git a/packages/integration-tests/src/tests/api/application/application.test.ts b/packages/integration-tests/src/tests/api/application/application.test.ts index 54a49b629..8f2d56517 100644 --- a/packages/integration-tests/src/tests/api/application/application.test.ts +++ b/packages/integration-tests/src/tests/api/application/application.test.ts @@ -176,7 +176,7 @@ describe('admin console application', () => { }); it('should fetch all non-third party applications created above', async () => { - const applications = await getApplications(); + const applications = await getApplications(undefined, undefined, 'false'); const applicationNames = applications.map(({ name }) => name); expect(applicationNames).toContain('test-create-app'); @@ -194,13 +194,32 @@ describe('admin console application', () => { } ); - const applications = await getApplications(undefined, undefined, true); + const applications = await getApplications(undefined, undefined, 'true'); expect(applications.find(({ id }) => id === application.id)).toEqual(application); expect(applications.some(({ isThirdParty }) => !isThirdParty)).toBe(false); await deleteApplication(application.id); }); + it('should fetch all applications including third party applications', async () => { + const application = await createApplication( + 'test-third-party-app', + ApplicationType.Traditional, + { + isThirdParty: true, + } + ); + + const applications = await getApplications(); + const applicationNames = applications.map(({ name }) => name); + + expect(applicationNames).toContain('test-create-app'); + expect(applicationNames).toContain('test-update-app'); + expect(applicationNames).toContain('test-third-party-app'); + + await deleteApplication(application.id); + }); + it('should create m2m application successfully and can get only m2m applications by specifying types', async () => { await createApplication('test-m2m-app-1', ApplicationType.MachineToMachine); await createApplication('test-m2m-app-2', ApplicationType.MachineToMachine);