Merge pull request #23450 from mareklibra/FLPATH-1004.sorting.followup

fix: use orderField and createdAfter to unify for other plugins

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,
         });

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()

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;

plugins/notifications-backend/src/service/parseEntityOrderFieldParams.test.ts

@@ -0,0 +1,57 @@
+/*
+ * Copyright 2024 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { parseEntityOrderFieldParams } from './parseEntityOrderFieldParams';
+
+describe('parseEntityOrderFieldParams', () => {
+  it('supports no order fields', () => {
+    const result = parseEntityOrderFieldParams({});
+    expect(result).toEqual(undefined);
+  });
+
+  it('supports single orderField', () => {
+    const result = parseEntityOrderFieldParams({
+      orderField: ['metadata.name,desc'],
+    })!;
+    expect(result).toEqual([{ field: 'metadata.name', order: 'desc' }]);
+  });
+
+  it('supports single orderField without order', () => {
+    const result = parseEntityOrderFieldParams({
+      orderField: ['metadata.name'],
+    })!;
+    expect(result).toEqual([{ field: 'metadata.name' }]);
+  });
+
+  it('supports multiple order fields', () => {
+    const result = parseEntityOrderFieldParams({
+      orderField: ['metadata.name,desc', 'metadata.uid,asc'],
+    });
+
+    expect(result).toEqual([
+      { field: 'metadata.name', order: 'desc' },
+      { field: 'metadata.uid', order: 'asc' },
+    ]);
+  });
+
+  it('throws if orderField order is not valid', () => {
+    expect(() =>
+      parseEntityOrderFieldParams({
+        orderField: ['metadata.name,desc', 'metadata.uid,invalid'],
+      }),
+    ).toThrow(/Invalid order field order/);
+  });
+});

plugins/notifications-backend/src/service/parseEntityOrderFieldParams.ts

@@ -0,0 +1,62 @@
+/*
+ * Copyright 2024 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// This file is based on the plugins/catalog-backend
+
+import { InputError } from '@backstage/errors';
+import { EntityOrder } from '../database';
+
+/**
+ * Takes a single unknown parameter and makes sure that it's a single string or
+ * an array of strings, and returns as an array.
+ */
+export function parseStringsParam(
+  param: unknown,
+  ctx: string,
+): string[] | undefined {
+  if (param === undefined) {
+    return undefined;
+  }
+
+  const array = [param].flat();
+  if (array.some(p => typeof p !== 'string')) {
+    throw new InputError(`Invalid ${ctx}, not a string`);
+  }
+
+  return array as string[];
+}
+
+export function parseEntityOrderFieldParams(
+  params: Record<string, unknown>,
+): EntityOrder[] | undefined {
+  const orderFieldStrings = parseStringsParam(params.orderField, 'orderField');
+  if (!orderFieldStrings) {
+    return undefined;
+  }
+
+  return orderFieldStrings.map(orderFieldString => {
+    const [field, order] = orderFieldString.split(',');
+
+    if (order !== undefined && !isOrder(order)) {
+      throw new InputError('Invalid order field order, must be asc or desc');
+    }
+    return { field, order };
+  });
+}
+
+export function isOrder(order: string): order is 'asc' | 'desc' {
+  return ['asc', 'desc'].includes(order);
+}

plugins/notifications-backend/src/service/router.ts

@@ -45,6 +45,7 @@ import {
   Notification,
   NotificationReadSignal,
 } from '@backstage/plugin-notifications-common';
+import { parseEntityOrderFieldParams } from './parseEntityOrderFieldParams';
 
 /** @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();
@@ -230,8 +206,8 @@ export async function createRouter(
       opts.saved = false;
       // or keep undefined
     }
-    if (req.query.created_after) {
-      const sinceEpoch = Date.parse(String(req.query.created_after));
+    if (req.query.createdAfter) {
+      const sinceEpoch = Date.parse(String(req.query.createdAfter));
       if (isNaN(sinceEpoch)) {
         throw new InputError('Unexpected date format');
       }