fix: pagination and ordering to getNotifications (#1016)

pnpm-workspace.yaml

@@ -1,3 +1,6 @@
+packages:
+  - '.'
+
 allowBuilds:
   '@apollo/protobufjs': true
   '@nestjs/core': true

src/common/services/pagination.service.ts

@@ -53,8 +53,11 @@ export class PaginationService {
     limit: number = 20,
     offset?: number,
     dateColumn: string = 'createdAt',
+    order: 'ASC' | 'DESC' = 'DESC',
   ): Promise<PaginationResult<T>> {
     const alias = qb.alias;
+    const normalizedOrder = order === 'ASC' ? 'ASC' : 'DESC';
+    const cursorOp = normalizedOrder === 'DESC' ? '<' : '>';
 
     if (cursor) {
       const decoded = decodeCursor(cursor);
@@ -63,12 +66,15 @@ export class PaginationService {
         const cursorId = decoded.id;
 
         qb.andWhere(
-          `(${alias}.${dateColumn} > :cursorDate OR (${alias}.${dateColumn} = :cursorDate AND ${alias}.id > :cursorId))`,
+          `(${alias}.${dateColumn} ${cursorOp} :cursorDate OR (${alias}.${dateColumn} = :cursorDate AND ${alias}.id ${cursorOp} :cursorId))`,
           { cursorDate, cursorId },
         );
       }
 
-      qb.orderBy(`${alias}.${dateColumn}`, 'ASC').addOrderBy(`${alias}.id`, 'ASC');
+      qb.orderBy(`${alias}.${dateColumn}`, normalizedOrder as 'ASC' | 'DESC').addOrderBy(
+        `${alias}.id`,
+        normalizedOrder as 'ASC' | 'DESC',
+      );
       qb.take(limit + 1);
 
       const items = await qb.getMany();
@@ -93,7 +99,10 @@ export class PaginationService {
       const page = offset !== undefined ? Math.floor(offset / limit) + 1 : 1;
       const skip = offset !== undefined ? offset : (page - 1) * limit;
 
-      qb.orderBy(`${alias}.${dateColumn}`, 'ASC').addOrderBy(`${alias}.id`, 'ASC');
+      qb.orderBy(`${alias}.${dateColumn}`, normalizedOrder as 'ASC' | 'DESC').addOrderBy(
+        `${alias}.id`,
+        normalizedOrder as 'ASC' | 'DESC',
+      );
       qb.skip(skip).take(limit);
 
       const [data, total] = await qb.getManyAndCount();

src/migrations/1810000000000-add-notifications-pagination-indexes.ts

@@ -0,0 +1,29 @@
+import { MigrationInterface, QueryRunner } from 'typeorm';
+
+/**
+ * Issue #1016 — Add pagination and ordering to NotificationsService.getNotifications
+ *
+ * Adds composite indexes to support bounded, newest-first pagination:
+ *  - idx_notifications_user_created          — (userId, createdAt DESC) for paginated listing
+ *  - idx_notifications_user_isread_created   — (userId, isRead, createdAt DESC) for unread filtering without full scan
+ *  - idx_notifications_user_status_created   — (userId, status, createdAt DESC) for status filtering
+ */
+export class AddNotificationsPaginationIndexes1810000000000 implements MigrationInterface {
+  public async up(queryRunner: QueryRunner): Promise<void> {
+    await queryRunner.query(
+      'CREATE INDEX IF NOT EXISTS "idx_notifications_user_created" ON "notifications" ("userId", "createdAt" DESC)',
+    );
+    await queryRunner.query(
+      'CREATE INDEX IF NOT EXISTS "idx_notifications_user_isread_created" ON "notifications" ("userId", "isRead", "createdAt" DESC)',
+    );
+    await queryRunner.query(
+      'CREATE INDEX IF NOT EXISTS "idx_notifications_user_status_created" ON "notifications" ("userId", "status", "createdAt" DESC)',
+    );
+  }
+
+  public async down(queryRunner: QueryRunner): Promise<void> {
+    await queryRunner.query('DROP INDEX IF EXISTS "idx_notifications_user_status_created"');
+    await queryRunner.query('DROP INDEX IF EXISTS "idx_notifications_user_isread_created"');
+    await queryRunner.query('DROP INDEX IF EXISTS "idx_notifications_user_created"');
+  }
+}

src/notifications/entities/notification.entity.ts

@@ -35,6 +35,9 @@ export enum NotificationStatus {
 
 @Entity('notifications')
 @Index('idx_notifications_dedup', ['userId', 'type', 'contentHash', 'createdAt'])
+@Index('idx_notifications_user_created', ['userId', 'createdAt'])
+@Index('idx_notifications_user_isread_created', ['userId', 'isRead', 'createdAt'])
+@Index('idx_notifications_user_status_created', ['userId', 'status', 'createdAt'])
 export class Notification {
   @PrimaryGeneratedColumn('uuid')
   id: string;

src/notifications/notifications.controller.ts

@@ -10,7 +10,13 @@ import {
   UseGuards,
   Req,
 } from '@nestjs/common';
-import { ApiTags, ApiOperation, ApiResponse, ApiBearerAuth } from '@nestjs/swagger';
+import {
+  ApiTags,
+  ApiOperation,
+  ApiResponse,
+  ApiBearerAuth,
+  ApiPropertyOptional,
+} from '@nestjs/swagger';
 import { JwtAuthGuard } from '../auth/guards/jwt-auth.guard';
 import { RolesGuard } from '../auth/guards/roles.guard';
 import { Roles } from '../auth/decorators/roles.decorator';
@@ -24,8 +30,27 @@ import {
   SendTemplatedNotificationDto,
 } from './dto/preferences.dto';
 import { PaginationQueryDto } from '../common/dto/pagination.dto';
-import { Notification } from './entities/notification.entity';
+import { Notification, NotificationStatus } from './entities/notification.entity';
 import { PaginatedSwaggerDto } from '../common/dto/paginated-response.dto';
+import { IsOptional, IsBoolean, IsEnum } from 'class-validator';
+import { Transform } from 'class-transformer';
+
+export class NotificationsQueryDto extends PaginationQueryDto {
+  @ApiPropertyOptional({ description: 'Filter by read status', type: Boolean })
+  @IsOptional()
+  @Transform(({ value }) => {
+    if (value === 'true' || value === '1') return true;
+    if (value === 'false' || value === '0') return false;
+    return value;
+  })
+  @IsBoolean({ message: 'isRead must be a boolean' })
+  isRead?: boolean;
+
+  @ApiPropertyOptional({ description: 'Filter by notification status', enum: NotificationStatus })
+  @IsOptional()
+  @IsEnum(NotificationStatus)
+  status?: NotificationStatus;
+}
 
 @ApiTags('notifications')
 @ApiBearerAuth()
@@ -86,7 +111,7 @@ export class NotificationsController {
     description: 'Paginated list of notifications',
     type: PaginatedSwaggerDto(Notification),
   })
-  list(@Req() req: any, @Query() query?: PaginationQueryDto) {
+  list(@Req() req: any, @Query() query?: NotificationsQueryDto) {
     return this.notificationsService.findForUser(req.user.id, query);
   }
 

src/notifications/notifications.service.ts

@@ -66,8 +66,41 @@ export class NotificationsService {
     return this.notificationRepository.save(notification);
   }
 
-  async getNotifications(userId: string) {
-    return this.notificationRepository.find({ where: { userId } });
+  async getNotifications(
+    userId: string,
+    query?: PaginationQueryDto & { status?: NotificationStatus; isRead?: boolean | string },
+  ) {
+    const limit = clampLimit(query?.limit);
+    const offset = query?.offset ?? (query?.cursor ? undefined : ((query?.page ?? 1) - 1) * limit);
+    const rawOrder = query?.order ? String(query.order).toUpperCase() : 'DESC';
+    const order = (rawOrder === 'ASC' ? 'ASC' : 'DESC') as 'ASC' | 'DESC';
+
+    const qb = this.notificationRepository
+      .createQueryBuilder('notification')
+      .where('notification.userId = :userId', { userId });
+
+    if (query?.isRead !== undefined) {
+      let isReadVal: boolean | undefined;
+      if (typeof query.isRead === 'string') {
+        if (query.isRead === 'true' || query.isRead === '1') isReadVal = true;
+        else if (query.isRead === 'false' || query.isRead === '0') isReadVal = false;
+        else isReadVal = undefined;
+      } else {
+        isReadVal = query.isRead;
+      }
+      if (isReadVal !== undefined) {
+        qb.andWhere('notification.isRead = :isRead', { isRead: isReadVal });
+      }
+    }
+
+    if (query?.status !== undefined) {
+      qb.andWhere('notification.status = :status', { status: query.status });
+    }
+
+    // Ensure deterministic ordering newest-first when no explicit order,
+    // using the composite index on (userId, createdAt DESC) and
+    // (userId, isRead, createdAt) / (userId, status, createdAt) for filtered queries.
+    return this.paginationService.paginate(qb, query?.cursor, limit, offset, 'createdAt', order);
   }
 
   async create(dto: CreateNotificationDto) {
@@ -193,15 +226,13 @@ export class NotificationsService {
     });
   }
 
-  async findForUser(userId: string, query?: PaginationQueryDto) {
-    const limit = clampLimit(query?.limit);
-    const offset = query?.offset ?? (query?.cursor ? undefined : ((query?.page ?? 1) - 1) * limit);
-
-    const qb = this.notificationRepository
-      .createQueryBuilder('notification')
-      .where('notification.userId = :userId', { userId });
-
-    return this.paginationService.paginate(qb, query?.cursor, limit, offset, 'createdAt');
+  async findForUser(
+    userId: string,
+    query?: PaginationQueryDto & { status?: NotificationStatus; isRead?: boolean | string },
+  ) {
+    // Delegate to getNotifications to ensure single source of truth for
+    // pagination, ordering (DESC newest-first), and indexed filtering.
+    return this.getNotifications(userId, query);
   }
 
   async markRead(id: string, userId: string) {