From 249614ba5db81c4b33a24ac08a68d10ead01e608 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Tue, 29 Sep 2026 14:13:15 +0200 Subject: [PATCH] Critical #1: Optimize grid email queries --- .../notification/notification.routes.spec.ts | 104 ++++++++++++++++++ .../query/customTemplates.query.spec.ts | 80 ++++++++++++++ src/routes/notification/index.ts | 38 ++++++- src/schema/query/customTemplates.query.ts | 20 +++- .../query/emailDistributionList.query.ts | 4 +- 5 files changed, 235 insertions(+), 11 deletions(-) create mode 100644 __tests__/unit-tests/routes/notification/notification.routes.spec.ts create mode 100644 __tests__/unit-tests/schema/query/customTemplates.query.spec.ts diff --git a/__tests__/unit-tests/routes/notification/notification.routes.spec.ts b/__tests__/unit-tests/routes/notification/notification.routes.spec.ts new file mode 100644 index 000000000..736baec4e --- /dev/null +++ b/__tests__/unit-tests/routes/notification/notification.routes.spec.ts @@ -0,0 +1,104 @@ +import notificationRoutes from '@routes/notification'; +import axios from 'axios'; +import config from 'config'; +import express, { NextFunction, Request, Response } from 'express'; +import supertest from 'supertest'; + +jest.mock('@models', () => ({ + EmailNotification: { + findById: jest.fn(), + }, +})); +jest.mock('@services/logger.service'); +jest.mock('axios', () => ({ + __esModule: true, + default: { + get: jest.fn(), + post: jest.fn(), + isAxiosError: jest.fn(), + }, +})); +jest.mock('config', () => ({ + get: jest.fn(), + util: { + getEnv: jest.fn(() => 'test'), + }, +})); + +/** + * Build a minimal application containing the notification proxy. + * + * @returns Express application + */ +const buildApp = () => { + const app = express(); + app.use(express.json()); + app.use((req: Request, _res: Response, next: NextFunction) => { + req.t = ((key: string) => key) as unknown as typeof req.t; + next(); + }); + app.use('/notification', notificationRoutes); + return app; +}; + +const request = supertest(buildApp()); + +describe('Notification proxy routes', () => { + beforeEach(() => { + jest.resetAllMocks(); + (config.get as jest.Mock).mockImplementation((key: string) => { + if (key === 'email.serverless.url') { + return 'http://localhost:7071/api'; + } + if (key === 'email.serverless.key') { + return ''; + } + return undefined; + }); + }); + + it('preserves an Azure Function error status and body', async () => { + (axios.isAxiosError as unknown as jest.Mock).mockReturnValue(true); + (axios.post as jest.Mock).mockRejectedValue({ + response: { + status: 400, + data: { message: 'No emails sent' }, + }, + }); + + const response = await request + .post('/notification/send-quick-email') + .send({}); + + expect(response.status).toBe(400); + expect(response.body).toEqual({ message: 'No emails sent' }); + }); + + it('returns an internal server error for failures without a response', async () => { + (axios.isAxiosError as unknown as jest.Mock).mockReturnValue(false); + (axios.post as jest.Mock).mockRejectedValue(new Error('Connection failed')); + + const response = await request + .post('/notification/send-quick-email') + .send({}); + + expect(response.status).toBe(500); + expect(response.text).toBe('common.errors.internalServerError'); + }); + + it('preserves plain-text Azure Function errors on GET requests', async () => { + (axios.isAxiosError as unknown as jest.Mock).mockReturnValue(true); + (axios.get as jest.Mock).mockRejectedValue({ + response: { + status: 404, + data: 'common.errors.dataNotFound', + }, + }); + + const response = await request.get('/notification/preview-email'); + + expect(response.status).toBe(404); + expect(response.type).toBe('text/plain'); + expect(response.text).toBe('common.errors.dataNotFound'); + }); +}); diff --git a/__tests__/unit-tests/schema/query/customTemplates.query.spec.ts b/__tests__/unit-tests/schema/query/customTemplates.query.spec.ts new file mode 100644 index 000000000..00475c5e2 --- /dev/null +++ b/__tests__/unit-tests/schema/query/customTemplates.query.spec.ts @@ -0,0 +1,80 @@ +import { CustomTemplate } from '@models/customTemplate.model'; +import { graphQLAuthCheck } from '@schema/shared'; +import resolver from '@schema/query/customTemplates.query'; +import { Context } from '@server/apollo/context'; + +jest.mock('@models/customTemplate.model', () => ({ + CustomTemplate: { + find: jest.fn(), + countDocuments: jest.fn(), + }, +})); +jest.mock('@schema/shared'); +jest.mock('@services/logger.service'); + +describe('CustomTemplates Query Resolver', () => { + const context = { + i18next: { + t: jest.fn((key: string) => key), + }, + } as unknown as Context; + + beforeEach(() => { + jest.resetAllMocks(); + (graphQLAuthCheck as jest.Mock).mockImplementation(() => undefined); + }); + + it('filters templates by the requested IDs', async () => { + const templates = [ + { + _id: 'template-1', + createdAt: new Date('2026-01-01T00:00:00Z'), + }, + ]; + const limit = jest.fn().mockResolvedValue(templates); + const skip = jest.fn().mockReturnValue({ limit }); + const sort = jest.fn().mockReturnValue({ skip }); + (CustomTemplate.find as jest.Mock).mockReturnValue({ sort }); + (CustomTemplate.countDocuments as jest.Mock).mockResolvedValue(1); + + const args = { + applicationId: 'application-1', + ids: ['template-1', 'template-2'], + isFromEmailNotification: false, + limit: 0, + skip: 0, + }; + const result = await resolver.resolve(null, args, context); + const expectedQuery = { + _id: { $in: args.ids }, + applicationId: args.applicationId, + isDeleted: { $ne: 1 }, + isFromEmailNotification: { $ne: true }, + }; + + expect(CustomTemplate.find).toHaveBeenCalledWith(expectedQuery); + expect(CustomTemplate.countDocuments).toHaveBeenCalledWith(expectedQuery); + expect(result.edges).toHaveLength(1); + expect(result.totalCount).toBe(1); + }); + + it('returns no templates when an empty ID list is requested', async () => { + const limit = jest.fn().mockResolvedValue([]); + const skip = jest.fn().mockReturnValue({ limit }); + const sort = jest.fn().mockReturnValue({ skip }); + (CustomTemplate.find as jest.Mock).mockReturnValue({ sort }); + (CustomTemplate.countDocuments as jest.Mock).mockResolvedValue(0); + + const args = { + ids: [], + limit: 0, + skip: 0, + }; + const result = await resolver.resolve(null, args, context); + + expect(CustomTemplate.find).toHaveBeenCalledWith( + expect.objectContaining({ _id: { $in: [] } }) + ); + expect(result.pageInfo.hasNextPage).toBe(false); + }); +}); diff --git a/src/routes/notification/index.ts b/src/routes/notification/index.ts index a8d29a304..462967144 100644 --- a/src/routes/notification/index.ts +++ b/src/routes/notification/index.ts @@ -12,6 +12,38 @@ import { getErrorMessage, getErrorStack } from '@utils/error'; */ const router = express.Router(); +/** + * Return an Azure Function error without hiding its status or response body. + * + * @param err Proxy request error + * @param req Express request + * @param res Express response + * @returns Express response + */ +const handleServerlessError = ( + err: unknown, + req: express.Request, + res: express.Response +) => { + logger.error(getErrorMessage(err), { stack: getErrorStack(err) }); + if (axios.isAxiosError(err) && err.response) { + const { data, status } = err.response; + if (typeof data === 'string') { + return res.status(status).type('text/plain').send(data); + } + if ( + typeof data === 'object' && + data !== null && + 'message' in data && + typeof data.message === 'string' + ) { + return res.status(status).json({ message: data.message }); + } + return res.status(status).type('text/plain').send('Request failed'); + } + return res.status(500).send(req.t('common.errors.internalServerError')); +}; + router.post('/add-subscription', async (req, res) => { let configId = ''; let userEmail = ''; @@ -146,8 +178,7 @@ router.post('/:functionName/:configId?', async (req, res) => { ); res.status(200).send(response.data); } catch (err) { - logger.error(getErrorMessage(err), { stack: getErrorStack(err) }); - res.status(500).send(req.t('common.errors.internalServerError')); + return handleServerlessError(err, req, res); } }); @@ -172,8 +203,7 @@ router.get('/:functionName/:configId?', async (req, res) => { res.status(200).send(response.data); } catch (err) { - logger.error(getErrorMessage(err), { stack: getErrorStack(err) }); - res.status(500).send(req.t('common.errors.internalServerError')); + return handleServerlessError(err, req, res); } }); diff --git a/src/schema/query/customTemplates.query.ts b/src/schema/query/customTemplates.query.ts index 4a176cb94..61c51fead 100644 --- a/src/schema/query/customTemplates.query.ts +++ b/src/schema/query/customTemplates.query.ts @@ -1,6 +1,13 @@ import { graphQLAuthCheck } from '@schema/shared'; import { logger } from '@services/logger.service'; -import { GraphQLBoolean, GraphQLError, GraphQLID, GraphQLInt } from 'graphql'; +import { + GraphQLBoolean, + GraphQLError, + GraphQLID, + GraphQLInt, + GraphQLList, + GraphQLNonNull, +} from 'graphql'; import { Context } from '@server/apollo/context'; import { decodeCursor, encodeCursor } from '@schema/types'; import getSortOrder from '@utils/schema/resolvers/Query/getSortOrder'; @@ -42,17 +49,20 @@ export default { limit: { type: GraphQLInt, defaultValue: 0 }, skip: { type: GraphQLInt, defaultValue: 0 }, isFromEmailNotification: { type: GraphQLBoolean }, + ids: { type: new GraphQLList(new GraphQLNonNull(GraphQLID)) }, }, async resolve(_, args, context: Context) { graphQLAuthCheck(context); try { - const customTemplates = await CustomTemplate.find({ + const query = { isDeleted: { $ne: 1 }, + ...(args.ids && { _id: { $in: args.ids } }), ...(args.applicationId && { applicationId: args.applicationId }), ...(!args.isFromEmailNotification && { isFromEmailNotification: { $ne: true }, }), - }) + }; + const customTemplates = await CustomTemplate.find(query) .sort(SORT_FIELDS[0].sort('desc')) .skip(args.skip) .limit(args.limit); @@ -63,12 +73,12 @@ export default { return { pageInfo: { - hasNextPage: edges.length === args.limit, + hasNextPage: args.limit > 0 && edges.length === args.limit, startCursor: edges.length > 0 ? edges[0].cursor : null, endCursor: edges.length > 0 ? edges[edges.length - 1].cursor : null, }, edges, - totalCount: await CustomTemplate.countDocuments(), + totalCount: await CustomTemplate.countDocuments(query), }; } catch (err) { logger.error(getErrorMessage(err), { stack: getErrorStack(err) }); diff --git a/src/schema/query/emailDistributionList.query.ts b/src/schema/query/emailDistributionList.query.ts index 0980377bd..252faf214 100644 --- a/src/schema/query/emailDistributionList.query.ts +++ b/src/schema/query/emailDistributionList.query.ts @@ -67,12 +67,12 @@ export default { return { pageInfo: { - hasNextPage: edges.length === args.limit, + hasNextPage: args.limit > 0 && edges.length === args.limit, startCursor: edges.length > 0 ? edges[0].cursor : null, endCursor: edges.length > 0 ? edges[edges.length - 1].cursor : null, }, edges, - totalCount: await EmailDistributionList.countDocuments(), + totalCount: await EmailDistributionList.countDocuments(query), }; } catch (err) { logger.error(getErrorMessage(err), { stack: getErrorStack(err) });