From bf90f646416d6428dc68394955055d99bf9a203b Mon Sep 17 00:00:00 2001 From: Marek Libra Date: Fri, 8 Mar 2024 10:41:54 +0100 Subject: [PATCH] fix: use orderField instead of sort,sortOrder for notifications To unify with the Catalog API. Signed-off-by: Marek Libra --- plugins/notifications-backend/package.json | 1 + .../DatabaseNotificationsStore.test.ts | 18 ++++------- .../database/DatabaseNotificationsStore.ts | 16 +++++----- .../src/database/NotificationsStore.ts | 9 ++++-- .../src/service/router.ts | 30 ++----------------- .../src/api/NotificationsClient.ts | 8 ++--- 6 files changed, 30 insertions(+), 52 deletions(-) diff --git a/plugins/notifications-backend/package.json b/plugins/notifications-backend/package.json index 68c7b9b22b..d8e7c75d17 100644 --- a/plugins/notifications-backend/package.json +++ b/plugins/notifications-backend/package.json @@ -37,6 +37,7 @@ "@backstage/config": "workspace:^", "@backstage/errors": "workspace:^", "@backstage/plugin-auth-node": "workspace:^", + "@backstage/plugin-catalog-backend": "workspace:^", "@backstage/plugin-events-node": "workspace:^", "@backstage/plugin-notifications-common": "workspace:^", "@backstage/plugin-notifications-node": "workspace:^", diff --git a/plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts b/plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts index e8cb9daca5..6204629821 100644 --- a/plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts +++ b/plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts @@ -405,8 +405,7 @@ describe.each(databases.eachSupportedId())( it('should sort created asc', async () => { const notificationsCreatedAsc = await storage.getNotifications({ user, - sort: 'created', - sortOrder: 'asc', + orderField: [{ field: 'created', order: 'asc' }], }); expect(notificationsCreatedAsc.map(idOnly)).toEqual([id1, id3, id2]); }); @@ -414,8 +413,7 @@ describe.each(databases.eachSupportedId())( it('should sort created desc', async () => { const notificationsCreatedDesc = await storage.getNotifications({ user, - sort: 'created', - sortOrder: 'desc', + orderField: [{ field: 'created', order: 'desc' }], }); expect(notificationsCreatedDesc.map(idOnly)).toEqual([id2, id3, id1]); }); @@ -423,8 +421,7 @@ describe.each(databases.eachSupportedId())( it('should sort topic asc', async () => { const notificationsTopicAsc = await storage.getNotifications({ user, - sort: 'topic', - sortOrder: 'asc', + orderField: [{ field: 'topic', order: 'asc' }], }); expect(notificationsTopicAsc.map(idOnly)).toEqual([id1, id3, id2]); }); @@ -432,8 +429,7 @@ describe.each(databases.eachSupportedId())( it('should sort topic desc', async () => { const notificationsTopicDesc = await storage.getNotifications({ user, - sort: 'topic', - sortOrder: 'desc', + orderField: [{ field: 'topic', order: 'desc' }], }); expect(notificationsTopicDesc.map(idOnly)).toEqual([id2, id3, id1]); }); @@ -441,8 +437,7 @@ describe.each(databases.eachSupportedId())( it('should sort origin asc', async () => { const notificationsOrigin = await storage.getNotifications({ user, - sort: 'origin', - sortOrder: 'asc', + orderField: [{ field: 'origin', order: 'asc' }], limit: 2, offset: 0, }); @@ -452,8 +447,7 @@ describe.each(databases.eachSupportedId())( it('should sort origin desc', async () => { const notificationsOriginNext = await storage.getNotifications({ user, - sort: 'origin', - sortOrder: 'desc', + orderField: [{ field: 'origin', order: 'desc' }], limit: 2, offset: 2, }); diff --git a/plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts b/plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts index 722489add4..f4f1f174be 100644 --- a/plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts +++ b/plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts @@ -160,7 +160,7 @@ export class DatabaseNotificationsStore implements NotificationsStore { private getNotificationsBaseQuery = ( options: NotificationGetOptions | NotificationModifyOptions, ) => { - const { user } = options; + const { user, orderField } = options; const subQuery = this.db('notification') .select(NOTIFICATION_COLUMNS) @@ -171,10 +171,12 @@ export class DatabaseNotificationsStore implements NotificationsStore { q.where('user', user).orWhereNull('user'); }); - if (options.sort !== undefined && options.sort !== null) { - query.orderBy(options.sort, options.sortOrder ?? 'desc'); - } else if (options.sort !== null) { - query.orderBy('created', options.sortOrder ?? 'desc'); + if (orderField && orderField.length > 0) { + orderField.forEach(orderBy => { + query.orderBy(orderBy.field, orderBy.order); + }); + } else if (!orderField) { + query.orderBy('created', 'desc'); } if (options.createdAfter) { @@ -235,7 +237,7 @@ export class DatabaseNotificationsStore implements NotificationsStore { const countOptions: NotificationGetOptions = { ...options }; countOptions.limit = undefined; countOptions.offset = undefined; - countOptions.sort = null; + countOptions.orderField = []; const notificationQuery = this.getNotificationsBaseQuery(countOptions); const response = await notificationQuery.count('id as CNT'); return Number(response[0].CNT); @@ -266,7 +268,7 @@ export class DatabaseNotificationsStore implements NotificationsStore { async getStatus(options: NotificationGetOptions) { const notificationQuery = this.getNotificationsBaseQuery({ ...options, - sort: null, + orderField: [], }); const readSubQuery = notificationQuery .clone() diff --git a/plugins/notifications-backend/src/database/NotificationsStore.ts b/plugins/notifications-backend/src/database/NotificationsStore.ts index 77f7df61df..901ea1695d 100644 --- a/plugins/notifications-backend/src/database/NotificationsStore.ts +++ b/plugins/notifications-backend/src/database/NotificationsStore.ts @@ -20,6 +20,12 @@ import { NotificationStatus, } from '@backstage/plugin-notifications-common'; +/** @internal */ +export type EntityOrder = { + field: string; + order: 'asc' | 'desc'; +}; + // TODO: reuse the common part of the type with front-end /** @internal */ export type NotificationGetOptions = { @@ -28,8 +34,7 @@ export type NotificationGetOptions = { offset?: number; limit?: number; search?: string; - sort?: 'created' | 'topic' | 'origin' | null; - sortOrder?: 'asc' | 'desc'; + orderField?: EntityOrder[]; read?: boolean; saved?: boolean; createdAfter?: Date; diff --git a/plugins/notifications-backend/src/service/router.ts b/plugins/notifications-backend/src/service/router.ts index 4a24763e40..959cd7e280 100644 --- a/plugins/notifications-backend/src/service/router.ts +++ b/plugins/notifications-backend/src/service/router.ts @@ -45,6 +45,7 @@ import { Notification, NotificationReadSignal, } from '@backstage/plugin-notifications-common'; +import { parseEntityOrderFieldParams } from '@backstage/plugin-catalog-backend'; /** @internal */ export interface RouterOptions { @@ -59,28 +60,6 @@ export interface RouterOptions { processors?: NotificationProcessor[]; } -const getSort = (input: string): NotificationGetOptions['sort'] | undefined => { - const valid: NotificationGetOptions['sort'][] = [ - 'created', - 'topic', - 'origin', - ]; - - if ((valid as string[]).includes(input)) { - return input as NotificationGetOptions['sort']; - } - return undefined; -}; - -const getSortOrder = (input: string): NotificationGetOptions['sortOrder'] => { - const valid: NotificationGetOptions['sortOrder'][] = ['asc', 'desc']; - - if ((valid as string[]).includes(input)) { - return input as NotificationGetOptions['sortOrder']; - } - return 'desc'; -}; - /** @internal */ export async function createRouter( options: RouterOptions, @@ -209,11 +188,8 @@ export async function createRouter( if (req.query.limit) { opts.limit = Number.parseInt(req.query.limit.toString(), 10); } - if (req.query.sort) { - opts.sort = getSort(req.query.sort.toString()); - } - if (req.query.sort_order) { - opts.sortOrder = getSortOrder(req.query.sort_order.toString()); + if (req.query.orderField) { + opts.orderField = parseEntityOrderFieldParams(req.query); } if (req.query.search) { opts.search = req.query.search.toString(); diff --git a/plugins/notifications/src/api/NotificationsClient.ts b/plugins/notifications/src/api/NotificationsClient.ts index 1e2e289bbe..047c594530 100644 --- a/plugins/notifications/src/api/NotificationsClient.ts +++ b/plugins/notifications/src/api/NotificationsClient.ts @@ -50,10 +50,10 @@ export class NotificationsClient implements NotificationsApi { queryString.append('offset', options.offset.toString(10)); } if (options?.sort !== undefined) { - queryString.append('sort', options.sort); - } - if (options?.sortOrder !== undefined) { - queryString.append('sort_order', options.sortOrder); + queryString.append( + 'orderField', + `${options.sort},${options?.sortOrder ?? 'desc'}`, + ); } if (options?.search) { queryString.append('search', options.search);