From f7122b6104a159d45e2a78a5a771074221bc6bc8 Mon Sep 17 00:00:00 2001 From: Antariksh Date: Wed, 9 Jun 2021 18:21:19 +0800 Subject: [PATCH 1/4] feat: tear out FeatureNames.AggregateStats --- .../feature-manager/aggregate-stats.config.ts | 16 -- src/app/config/feature-manager/index.ts | 2 - src/app/config/feature-manager/types.ts | 6 - .../modules/examples/examples.controller.ts | 6 +- src/app/modules/examples/examples.factory.ts | 30 ---- src/app/modules/examples/examples.queries.ts | 88 +-------- src/app/modules/examples/examples.service.ts | 169 +++++++----------- src/app/modules/examples/examples.types.ts | 12 +- src/app/modules/examples/examples.utils.ts | 32 ---- 9 files changed, 71 insertions(+), 290 deletions(-) delete mode 100644 src/app/config/feature-manager/aggregate-stats.config.ts delete mode 100644 src/app/modules/examples/examples.factory.ts diff --git a/src/app/config/feature-manager/aggregate-stats.config.ts b/src/app/config/feature-manager/aggregate-stats.config.ts deleted file mode 100644 index c9793a833d..0000000000 --- a/src/app/config/feature-manager/aggregate-stats.config.ts +++ /dev/null @@ -1,16 +0,0 @@ -import { FeatureNames, RegisterableFeature } from './types' - -const aggregateCollectionFeature: RegisterableFeature = - { - name: FeatureNames.AggregateStats, - schema: { - aggregateCollection: { - doc: 'Has to be defined (i.e. =true) if FormStats collection is to be used', - format: '*', - default: null, - env: 'AGGREGATE_COLLECTION', - }, - }, - } - -export default aggregateCollectionFeature diff --git a/src/app/config/feature-manager/index.ts b/src/app/config/feature-manager/index.ts index a3fec885b4..b01e4a5973 100644 --- a/src/app/config/feature-manager/index.ts +++ b/src/app/config/feature-manager/index.ts @@ -1,5 +1,4 @@ import FeatureManager from './util/FeatureManager.class' -import aggregateStats from './aggregate-stats.config' import captcha from './captcha.config' import googleAnalytics from './google-analytics.config' import { intranetFeature } from './intranet.config' @@ -17,7 +16,6 @@ const featureManager = new FeatureManager() featureManager.register(captcha) featureManager.register(sentry) featureManager.register(googleAnalytics) -featureManager.register(aggregateStats) featureManager.register(spcpMyInfo) featureManager.register(webhookVerifiedContent) featureManager.register(sms) diff --git a/src/app/config/feature-manager/types.ts b/src/app/config/feature-manager/types.ts index 6e2b3ac337..61eb5841fb 100644 --- a/src/app/config/feature-manager/types.ts +++ b/src/app/config/feature-manager/types.ts @@ -2,7 +2,6 @@ import { MyInfoMode } from '@opengovsg/myinfo-gov-client' import { Schema } from 'convict' export enum FeatureNames { - AggregateStats = 'aggregate-stats', Captcha = 'captcha', GoogleAnalytics = 'google-analytics', Sentry = 'sentry', @@ -13,10 +12,6 @@ export enum FeatureNames { Intranet = 'intranet', } -export interface IAggregateStats { - aggregateCollection: string -} - export interface ICaptcha { captchaPrivateKey: string captchaPublicKey: string @@ -87,7 +82,6 @@ export interface IIntranet { } export interface IFeatureManager { - [FeatureNames.AggregateStats]: IAggregateStats [FeatureNames.Captcha]: ICaptcha [FeatureNames.GoogleAnalytics]: IGoogleAnalytics [FeatureNames.Sentry]: ISentry diff --git a/src/app/modules/examples/examples.controller.ts b/src/app/modules/examples/examples.controller.ts index 6a54eaa96b..cc86860784 100644 --- a/src/app/modules/examples/examples.controller.ts +++ b/src/app/modules/examples/examples.controller.ts @@ -10,7 +10,7 @@ import { createLoggerWithLabel } from '../../config/logger' import { createReqMeta } from '../../utils/request' import { ControllerHandler } from '../core/core.types' -import { ExamplesFactory } from './examples.factory' +import * as ExamplesService from './examples.service' import { mapRouteError } from './examples.utils' const logger = createLoggerWithLabel(module) @@ -29,7 +29,7 @@ export const handleGetExamples: ControllerHandler< unknown, ExampleFormsQueryDto > = (req, res) => { - return ExamplesFactory.getExampleForms(req.query) + return ExamplesService.getExampleForms(req.query) .map((result) => res.status(StatusCodes.OK).json(result)) .mapErr((error) => { logger.error({ @@ -61,7 +61,7 @@ export const handleGetExampleByFormId: ControllerHandler< > = (req, res) => { const { formId } = req.params - return ExamplesFactory.getSingleExampleForm(formId) + return ExamplesService.getSingleExampleForm(formId) .map((result) => res.status(StatusCodes.OK).json(result)) .mapErr((error) => { logger.error({ diff --git a/src/app/modules/examples/examples.factory.ts b/src/app/modules/examples/examples.factory.ts deleted file mode 100644 index 44ea0cfa3b..0000000000 --- a/src/app/modules/examples/examples.factory.ts +++ /dev/null @@ -1,30 +0,0 @@ -import FeatureManager, { - FeatureNames, - RegisteredFeature, -} from '../../config/feature-manager' - -import * as ExamplesService from './examples.service' -import { RetrievalType } from './examples.types' - -interface IExamplesFactory { - getExampleForms: ReturnType - getSingleExampleForm: ReturnType -} - -const aggregateFeature = FeatureManager.get(FeatureNames.AggregateStats) - -// Exported for testing. -export const createExamplesFactory = ({ - isEnabled, -}: RegisteredFeature): IExamplesFactory => { - // Set retrieval type to use statistics collection if feature is enabled. - const retrievalType = isEnabled - ? RetrievalType.Stats - : RetrievalType.Submissions - return { - getExampleForms: ExamplesService.getExampleForms(retrievalType), - getSingleExampleForm: ExamplesService.getSingleExampleForm(retrievalType), - } -} - -export const ExamplesFactory = createExamplesFactory(aggregateFeature) diff --git a/src/app/modules/examples/examples.queries.ts b/src/app/modules/examples/examples.queries.ts index 7d654368ad..4d4e46ca4f 100644 --- a/src/app/modules/examples/examples.queries.ts +++ b/src/app/modules/examples/examples.queries.ts @@ -114,8 +114,7 @@ export const filterByAgencyId = ( /** * Precondition: A group stage that produced a count field must be executed - * beforehand, which can be done with groupSubmissionsByFormId or - * lookupSubmissionInfo. + * beforehand, which can be done with lookupSubmissionInfo. * Aggregation step to filter forms with less than the minimum number of * submissions for the examples page. @@ -162,18 +161,6 @@ export const sortByLastSubmitted: Record[] = [ }, ] -/** - * Precondition: `created` field must have already been retrieved from the - * submissions collection via searchSubmissionsForForm. - * - * Aggregation step to sort forms by the creation date. - */ -export const sortByCreated = [ - { - $sort: { created: 1 }, - }, -] - /** * Precondition: `_id` field corresponding to the forms' ids must be retrieved * beforehand, which can be done using groupSubmissionsByFormId or @@ -250,63 +237,6 @@ export const lookupFormStatisticsInfo: Record[] = [ }, ] -/** - * Precondition: `_id` field corresponding to forms' ids must be retrieved - * beforehand, which can be done using groupSubmissionsByFormId or - * searchFormsForText. - * - * Aggregation step to retrieve submissionInfo by looking up, sorting and - * grouping submissions with form ids specified. - * - */ -export const lookupSubmissionInfo = [ - { - $lookup: { - from: 'submissions', - localField: '_id', - foreignField: 'form', - as: 'submissionInfo', - }, - }, - // Unwind results in multiple copies of each form, where each copy has its own submissionInfo - { - $unwind: '$submissionInfo', - }, - { - $sort: { 'submissionInfo.created': 1 }, - }, - // Retrieve only the necessary information from the submissionInfo - { - $group: { - _id: '$_id', - count: { $sum: 1 }, - formInfo: { $first: '$formInfo' }, - agencyInfo: { $first: '$agencyInfo' }, - lastSubmission: { $last: '$submissionInfo.created' }, - textScore: { $first: { $meta: 'textScore' } }, // Used to sort by relevance - }, - }, -] - -/** - * Precondition: `_id` field corresponding to forms' ids must be retrieved - * beforehand. - * - * !Note: Can only used on pipelines working with the Submissions collection. - * - * Aggregation step to group submissions by form id, count the number of - * submissions, and get the last submission date. - */ -export const groupSubmissionsByFormId = [ - { - $group: { - _id: '$form', - count: { $sum: 1 }, - lastSubmission: { $last: '$created' }, - }, - }, -] - /** * Precondition: `_id` field corresponding to forms' ids must be retrieved * beforehand. @@ -420,19 +350,3 @@ export const searchSubmissionsForForm = ( }, }, ] - -/** - * Precondition: `formFeedbackInfo` must have been retrieved in a previous step, - * which can be done using lookupFormFeedback. - * - * Aggregation step to add the average feedback field. - */ -export const addAvgFeedback = [ - { - $addFields: { - avgFeedback: { - $avg: '$formFeedbackInfo.rating', - }, - }, - }, -] diff --git a/src/app/modules/examples/examples.service.ts b/src/app/modules/examples/examples.service.ts index d6c7c907c2..ddd3ef6281 100644 --- a/src/app/modules/examples/examples.service.ts +++ b/src/app/modules/examples/examples.service.ts @@ -6,28 +6,23 @@ import { Except, Merge } from 'type-fest' import { createLoggerWithLabel } from '../../config/logger' import getFormModel from '../../models/form.server.model' import getFormStatisticsTotalModel from '../../models/form_statistics_total.server.model' -import getSubmissionModel from '../../models/submission.server.model' import { DatabaseError } from '../core/core.errors' import { MIN_SUB_COUNT, PAGE_SIZE } from './examples.constants' import { ResultsNotFoundError } from './examples.errors' import { - groupSubmissionsByFormId, lookupFormStatisticsInfo, - lookupSubmissionInfo, projectSubmissionInfo, selectAndProjectCardInfo, } from './examples.queries' import { ExamplesQueryParams, FormInfo, - QueryData, QueryDataMap, QueryExecResult, QueryExecResultWithTotal, QueryPageResult, QueryPageResultWithTotal, - RetrievalType, RetrieveSubmissionsExecResult, SingleFormInfoQueryResult, SingleFormResult, @@ -37,35 +32,14 @@ import { createGeneralQueryPipeline, createSearchQueryPipeline, createSingleSearchStatsPipeline, - createSingleSearchSubmissionPipeline, formatToRelativeString, } from './examples.utils' const FormModel = getFormModel(mongoose) const FormStatisticsModel = getFormStatisticsTotalModel(mongoose) -const SubmissionModel = getSubmissionModel(mongoose) const logger = createLoggerWithLabel(module) -/** - * Maps retrieval type to the middlewares and query model used for general - * queries to use when creating the aggregation pipeline - */ -const RETRIEVAL_TO_QUERY_DATA_MAP: QueryData = { - [RetrievalType.Stats]: { - generalQueryModel: FormStatisticsModel, - lookUpMiddleware: lookupFormStatisticsInfo, - groupByMiddleware: projectSubmissionInfo, - singleSearchPipeline: createSingleSearchStatsPipeline, - }, - [RetrievalType.Submissions]: { - generalQueryModel: SubmissionModel, - lookUpMiddleware: lookupSubmissionInfo, - groupByMiddleware: groupSubmissionsByFormId, - singleSearchPipeline: createSingleSearchSubmissionPipeline, - }, -} - /** * Creates and returns the query builder to execute some example fetch query. */ @@ -239,28 +213,23 @@ const getFormInfo = ( * @returns ok(list of retrieved example forms) if `shouldGetTotalNumResults` is not of string `"true"` * @returns err(DatabaseError) if any errors occurs whilst running the pipeline on the database */ -export const getExampleForms = - (type: RetrievalType) => - ( - query: ExamplesQueryParams, - ): ResultAsync => { - const { lookUpMiddleware, groupByMiddleware, generalQueryModel } = - RETRIEVAL_TO_QUERY_DATA_MAP[type] - - const queryBuilder = getExamplesQueryBuilder({ - query, - lookUpMiddleware, - groupByMiddleware, - generalQueryModel, - }) +export const getExampleForms = ( + query: ExamplesQueryParams, +): ResultAsync => { + const queryBuilder = getExamplesQueryBuilder({ + query, + lookUpMiddleware: lookupFormStatisticsInfo, + groupByMiddleware: projectSubmissionInfo, + generalQueryModel: FormStatisticsModel, + }) - const { pageNo, shouldGetTotalNumResults } = query - const offset = pageNo * PAGE_SIZE || 0 + const { pageNo, shouldGetTotalNumResults } = query + const offset = pageNo * PAGE_SIZE || 0 - return shouldGetTotalNumResults - ? execExamplesQueryWithTotal(queryBuilder, offset) - : execExamplesQuery(queryBuilder, offset) - } + return shouldGetTotalNumResults + ? execExamplesQueryWithTotal(queryBuilder, offset) + : execExamplesQuery(queryBuilder, offset) +} /** * Retrieves a single form for examples from either the FormStatisticsTotal @@ -272,63 +241,57 @@ export const getExampleForms = * @returns err(DatabaseError) if any errors occurs whilst running the pipeline on the database * @returns err(ResultsNotFoundError) if form info cannot be retrieved with the given form id */ -export const getSingleExampleForm = - (type: RetrievalType) => - ( - formId: string, - ): ResultAsync => { - const { singleSearchPipeline, generalQueryModel } = - RETRIEVAL_TO_QUERY_DATA_MAP[type] - - return ( - // Step 1: Retrieve base form info to augment. - getFormInfo(formId) - // Step 2a: Execute aggregate query with relevant single search pipeline. - .andThen((formInfo) => - ResultAsync.fromPromise( - generalQueryModel - .aggregate(singleSearchPipeline(formId)) - .read('secondary') - .exec() as Promise, - (error) => { - logger.error({ - message: 'Failed to retrieve a single example form', - meta: { - action: 'getSingleExampleForm', - }, - error, - }) - - return new DatabaseError() - }, - // Step 2b: Augment the initial base form info with the retrieved - // statistics from the aggregate pipeline. - ).map((queryResult) => { - // Process result depending on whether search pipeline returned - // results. - // If the statistics cannot be found, add default "null" fields. - if (!queryResult || queryResult.length === 0) { - const emptyStatsExampleInfo: FormInfo = { - ...formInfo, - count: 0, - lastSubmission: null, - timeText: '-', - avgFeedback: null, - } - return { form: emptyStatsExampleInfo } - } +export const getSingleExampleForm = ( + formId: string, +): ResultAsync => { + return ( + // Step 1: Retrieve base form info to augment. + getFormInfo(formId) + // Step 2a: Execute aggregate query with relevant single search pipeline. + .andThen((formInfo) => + ResultAsync.fromPromise( + FormStatisticsModel.aggregate(createSingleSearchStatsPipeline(formId)) + .read('secondary') + .exec() as Promise, + (error) => { + logger.error({ + message: 'Failed to retrieve a single example form', + meta: { + action: 'getSingleExampleForm', + }, + error, + }) - // Statistics can be found. - const [statistics] = queryResult - const processedExampleInfo: FormInfo = { + return new DatabaseError() + }, + // Step 2b: Augment the initial base form info with the retrieved + // statistics from the aggregate pipeline. + ).map((queryResult) => { + // Process result depending on whether search pipeline returned + // results. + // If the statistics cannot be found, add default "null" fields. + if (!queryResult || queryResult.length === 0) { + const emptyStatsExampleInfo: FormInfo = { ...formInfo, - count: statistics.count, - lastSubmission: statistics.lastSubmission, - avgFeedback: statistics.avgFeedback, - timeText: formatToRelativeString(statistics.lastSubmission), + count: 0, + lastSubmission: null, + timeText: '-', + avgFeedback: null, } - return { form: processedExampleInfo } - }), - ) - ) - } + return { form: emptyStatsExampleInfo } + } + + // Statistics can be found. + const [statistics] = queryResult + const processedExampleInfo: FormInfo = { + ...formInfo, + count: statistics.count, + lastSubmission: statistics.lastSubmission, + avgFeedback: statistics.avgFeedback, + timeText: formatToRelativeString(statistics.lastSubmission), + } + return { form: processedExampleInfo } + }), + ) + ) +} diff --git a/src/app/modules/examples/examples.types.ts b/src/app/modules/examples/examples.types.ts index 4293fe0d44..132d406eb9 100644 --- a/src/app/modules/examples/examples.types.ts +++ b/src/app/modules/examples/examples.types.ts @@ -3,26 +3,16 @@ import { IForm, IFormFeedbackSchema, IFormStatisticsTotalModel, - ISubmissionModel, StartPage, } from 'src/types' -export enum RetrievalType { - Stats = 'statistics', - Submissions = 'submissions', -} - export type QueryDataMap = { - generalQueryModel: IFormStatisticsTotalModel | ISubmissionModel + generalQueryModel: IFormStatisticsTotalModel lookUpMiddleware: Record[] groupByMiddleware: Record[] singleSearchPipeline: (formId: string) => Record[] } -export type QueryData = { - [k in RetrievalType]: QueryDataMap -} - export type QueryExecResult = { _id: string count: number diff --git a/src/app/modules/examples/examples.utils.ts b/src/app/modules/examples/examples.utils.ts index 5024e043fb..6a8fe883db 100644 --- a/src/app/modules/examples/examples.utils.ts +++ b/src/app/modules/examples/examples.utils.ts @@ -7,11 +7,9 @@ import { DatabaseError } from '../core/core.errors' import { ResultsNotFoundError } from './examples.errors' import { - addAvgFeedback, filterByAgencyId, filterBySubmissionCount, filterInactiveAndUnlistedForms, - groupSubmissionsByFormId, lookupAgencyInfo, lookupFormFeedback, lookupFormInfo, @@ -21,7 +19,6 @@ import { searchFormsById, searchFormsWithText, searchSubmissionsForForm, - sortByCreated, sortByLastSubmitted, sortByRelevance, } from './examples.queries' @@ -169,35 +166,6 @@ export const createFormIdInfoPipeline = ( ) } -/** - * Creates a query pipeline that can be used to retrieve a single example form - * for the /examples page using the submission collection. - * - * This pipeline will return the average feedback for the form id referenced to - * be shown in the example form. - * @param formId. The id of the form to retrieve data for - */ -export const createSingleSearchSubmissionPipeline = ( - formId: string, -): Record[] => { - // Retrieve all submissions with the specified formId. - // This pipeline using the submission collection, and `form` is the foreign - // key of the form collection in that collection. - return searchSubmissionsForForm('form', formId).concat( - // Sort forms by the creation date. - sortByCreated, - // Group submissions by form id, count the number of submissions, and get - // the last submission date. - groupSubmissionsByFormId, - // Sort all submissions by their last submission date. - sortByLastSubmitted, - // Retrieve form feedbacks for the submissions. - lookupFormFeedback, - // Calculate and add the average feedback. - addAvgFeedback, - ) -} - /** * Creates a query pipeline that can be used to retrieve a single example form * for the /examples page using the formStatisticsTotal collection. From cc207db84a89bc5b10fc80cabc7b2e2e9185f803 Mon Sep 17 00:00:00 2001 From: Antariksh Date: Wed, 9 Jun 2021 18:33:04 +0800 Subject: [PATCH 2/4] test: update tests to not rely on RetrievalType --- .../__tests__/examples.controller.spec.ts | 26 +- .../__tests__/examples.factory.spec.ts | 87 ------- .../__tests__/examples.routes.spec.ts | 230 +----------------- .../__tests__/examples.service.spec.ts | 208 +--------------- 4 files changed, 29 insertions(+), 522 deletions(-) delete mode 100644 src/app/modules/examples/__tests__/examples.factory.spec.ts diff --git a/src/app/modules/examples/__tests__/examples.controller.spec.ts b/src/app/modules/examples/__tests__/examples.controller.spec.ts index 5e1efc357b..1d60951145 100644 --- a/src/app/modules/examples/__tests__/examples.controller.spec.ts +++ b/src/app/modules/examples/__tests__/examples.controller.spec.ts @@ -8,11 +8,11 @@ import expressHandler from 'tests/unit/backend/helpers/jest-express' import { DatabaseError } from '../../core/core.errors' import * as ExamplesController from '../examples.controller' import { ResultsNotFoundError } from '../examples.errors' -import { ExamplesFactory } from '../examples.factory' +import * as ExamplesService from '../examples.service' import { ExamplesQueryParams, SingleFormResult } from '../examples.types' -jest.mock('../examples.factory') -const MockExamplesFactory = mocked(ExamplesFactory) +jest.mock('../examples.service') +const MockExamplesService = mocked(ExamplesService) describe('examples.controller', () => { beforeEach(() => jest.clearAllMocks()) @@ -38,7 +38,7 @@ describe('examples.controller', () => { forms: [], totalNumResults: 0, } - MockExamplesFactory.getExampleForms.mockReturnValueOnce( + MockExamplesService.getExampleForms.mockReturnValueOnce( okAsync(mockResult), ) @@ -46,7 +46,7 @@ describe('examples.controller', () => { await ExamplesController.handleGetExamples(MOCK_REQ, mockRes, jest.fn()) // Assert - expect(MockExamplesFactory.getExampleForms).toHaveBeenCalledWith( + expect(MockExamplesService.getExampleForms).toHaveBeenCalledWith( MOCK_REQ_QUERY, ) expect(mockRes.status).toBeCalledWith(200) @@ -57,7 +57,7 @@ describe('examples.controller', () => { // Arrange const mockRes = expressHandler.mockResponse() // Mock getExampleForms to return error. - MockExamplesFactory.getExampleForms.mockReturnValueOnce( + MockExamplesService.getExampleForms.mockReturnValueOnce( errAsync(new DatabaseError()), ) @@ -65,7 +65,7 @@ describe('examples.controller', () => { await ExamplesController.handleGetExamples(MOCK_REQ, mockRes, jest.fn()) // Assert - expect(MockExamplesFactory.getExampleForms).toHaveBeenCalledWith( + expect(MockExamplesService.getExampleForms).toHaveBeenCalledWith( MOCK_REQ_QUERY, ) expect(mockRes.status).toBeCalledWith(500) @@ -106,7 +106,7 @@ describe('examples.controller', () => { title: 'mockTitle', }, } - MockExamplesFactory.getSingleExampleForm.mockReturnValueOnce( + MockExamplesService.getSingleExampleForm.mockReturnValueOnce( okAsync(mockResult), ) @@ -118,7 +118,7 @@ describe('examples.controller', () => { ) // Assert - expect(MockExamplesFactory.getSingleExampleForm).toHaveBeenCalledWith( + expect(MockExamplesService.getSingleExampleForm).toHaveBeenCalledWith( MOCK_REQ_PARAMS.formId, ) expect(mockRes.status).toBeCalledWith(200) @@ -130,7 +130,7 @@ describe('examples.controller', () => { const mockRes = expressHandler.mockResponse() // Mock getSingleExampleForm to return not found error. const mockErrorString = 'not found error!' - MockExamplesFactory.getSingleExampleForm.mockReturnValueOnce( + MockExamplesService.getSingleExampleForm.mockReturnValueOnce( errAsync(new ResultsNotFoundError(mockErrorString)), ) @@ -142,7 +142,7 @@ describe('examples.controller', () => { ) // Assert - expect(MockExamplesFactory.getSingleExampleForm).toHaveBeenCalledWith( + expect(MockExamplesService.getSingleExampleForm).toHaveBeenCalledWith( MOCK_REQ_PARAMS.formId, ) expect(mockRes.status).toBeCalledWith(404) @@ -154,7 +154,7 @@ describe('examples.controller', () => { const mockRes = expressHandler.mockResponse() // Mock getSingleExampleForm to return database error. const mockErrorString = 'database error!' - MockExamplesFactory.getSingleExampleForm.mockReturnValueOnce( + MockExamplesService.getSingleExampleForm.mockReturnValueOnce( errAsync(new DatabaseError(mockErrorString)), ) @@ -166,7 +166,7 @@ describe('examples.controller', () => { ) // Assert - expect(MockExamplesFactory.getSingleExampleForm).toHaveBeenCalledWith( + expect(MockExamplesService.getSingleExampleForm).toHaveBeenCalledWith( MOCK_REQ_PARAMS.formId, ) expect(mockRes.status).toBeCalledWith(500) diff --git a/src/app/modules/examples/__tests__/examples.factory.spec.ts b/src/app/modules/examples/__tests__/examples.factory.spec.ts deleted file mode 100644 index 4df4721198..0000000000 --- a/src/app/modules/examples/__tests__/examples.factory.spec.ts +++ /dev/null @@ -1,87 +0,0 @@ -import { mocked } from 'ts-jest/utils' - -import { - FeatureNames, - IAggregateStats, - RegisteredFeature, -} from 'src/app/config/feature-manager' - -import { createExamplesFactory } from '../examples.factory' -import * as ExamplesService from '../examples.service' -import { ExamplesQueryParams, RetrievalType } from '../examples.types' - -// Higher order function requires two level mocking. -jest.mock('../examples.service', () => ({ - getExampleForms: jest.fn().mockImplementation(() => jest.fn()), - getSingleExampleForm: jest.fn().mockImplementation(() => jest.fn()), -})) - -const MockExamplesService = mocked(ExamplesService) - -describe('examples.factory', () => { - describe('aggregate-stats feature disabled', () => { - const MOCK_DISABLED_FEATURE: RegisteredFeature = - { - isEnabled: false, - props: {} as IAggregateStats, - } - const ExamplesFactory = createExamplesFactory(MOCK_DISABLED_FEATURE) - - describe('getExampleForms', () => { - it('should invoke service method with RetrievalType.Submissions', async () => { - // Act - await ExamplesFactory.getExampleForms({} as ExamplesQueryParams) - - // Assert - expect(MockExamplesService.getExampleForms).toHaveBeenCalledWith( - RetrievalType.Submissions, - ) - }) - }) - - describe('getSingleExampleForm', () => { - it('should invoke service method with RetrievalType.Submissions', async () => { - // Act - await ExamplesFactory.getSingleExampleForm('anything') - - // Assert - expect(MockExamplesService.getSingleExampleForm).toHaveBeenCalledWith( - RetrievalType.Submissions, - ) - }) - }) - }) - - describe('aggregate-stats feature enabled', () => { - const MOCK_ENABLED_FEATURE: RegisteredFeature = - { - isEnabled: true, - props: {} as IAggregateStats, - } - const ExamplesFactory = createExamplesFactory(MOCK_ENABLED_FEATURE) - - describe('getExampleForms', () => { - it('should invoke service method with RetrievalType.Stats', async () => { - // Act - await ExamplesFactory.getExampleForms({} as ExamplesQueryParams) - - // Assert - expect(MockExamplesService.getExampleForms).toHaveBeenCalledWith( - RetrievalType.Stats, - ) - }) - }) - - describe('getSingleExampleForm', () => { - it('should invoke service method with RetrievalType.Stats', async () => { - // Act - await ExamplesFactory.getSingleExampleForm('anything') - - // Assert - expect(MockExamplesService.getSingleExampleForm).toHaveBeenCalledWith( - RetrievalType.Stats, - ) - }) - }) - }) -}) diff --git a/src/app/modules/examples/__tests__/examples.routes.spec.ts b/src/app/modules/examples/__tests__/examples.routes.spec.ts index d59e4224ca..3984e4ff9b 100644 --- a/src/app/modules/examples/__tests__/examples.routes.spec.ts +++ b/src/app/modules/examples/__tests__/examples.routes.spec.ts @@ -2,7 +2,6 @@ import { ObjectId } from 'bson-ext' import { keyBy } from 'lodash' import { errAsync } from 'neverthrow' import supertest, { Session } from 'supertest-session' -import { mocked } from 'ts-jest/utils' import { IAgencySchema, IUserSchema } from 'src/types' @@ -12,10 +11,9 @@ import { buildCelebrateError } from 'tests/unit/backend/helpers/celebrate' import dbHandler from 'tests/unit/backend/helpers/jest-db' import { DatabaseError } from '../../core/core.errors' -import { ExamplesFactory } from '../examples.factory' import { ExamplesRouter } from '../examples.routes' -import { getExampleForms, getSingleExampleForm } from '../examples.service' -import { FormInfo, RetrievalType } from '../examples.types' +import * as ExamplesService from '../examples.service' +import { FormInfo } from '../examples.types' import prepareTestData, { SearchTerm, @@ -28,9 +26,6 @@ jest.mock('../examples.constants', () => ({ MIN_SUB_COUNT: 0, })) -jest.mock('../examples.factory') -const MockExamplesFactory = mocked(ExamplesFactory) - const app = setupApp('/examples', ExamplesRouter, { setupWithAuth: true, }) @@ -72,173 +67,6 @@ describe('examples.routes', () => { describe('GET /examples', () => { describe('AggregateStats feature enabled', () => { - beforeAll(() => { - MockExamplesFactory.getExampleForms.mockImplementation( - getExampleForms(RetrievalType.Stats), - ) - }) - - it('should return 200 with array of example forms for all agencies when query.agency is missing', async () => { - // Arrange - const session = await createAuthedSession(defaultUser.email, request) - - // Act - const response = await session.get('/examples').query({ - pageNo: '0', - }) - - // Assert - // Should have both comTestData and orgTestData since searching by all - // agencies. - const expectedBody = keyBy( - [ - ...stringifyFormInfoArray(comTestData.total.expectedFormInfo), - ...stringifyFormInfoArray(orgTestData.total.expectedFormInfo), - ], - '_id', - ) - const actualBody = keyBy(response.body.forms, '_id') - expect(response.status).toEqual(200) - // Check shape. - expect(response.body).toEqual({ - forms: expect.any(Array), - }) - expect(actualBody).toEqual(expectedBody) - }) - - it('should return 200 with array of example forms for a particular agency when query.agency is provided', async () => { - // Arrange - const session = await createAuthedSession(defaultUser.email, request) - - // Act - const response = await session.get('/examples').query({ - pageNo: '0', - agency: orgAgency._id.toString(), - }) - - // Assert - // Should only have orgTestData since its agency id is provided. - const expectedBody = keyBy( - stringifyFormInfoArray(orgTestData.total.expectedFormInfo), - '_id', - ) - const actualBody = keyBy(response.body.forms, '_id') - // Check shape. - expect(response.status).toEqual(200) - expect(response.body).toEqual({ - forms: expect.any(Array), - }) - expect(actualBody).toEqual(expectedBody) - }) - - it('should return 200 with empty array when no forms match the criterias', async () => { - // Arrange - const session = await createAuthedSession(defaultUser.email, request) - - // Act - const response = await session.get('/examples').query({ - pageNo: '0', - agency: new ObjectId().toHexString(), - }) - - // Assert - expect(response.status).toEqual(200) - // Check shape. - expect(response.body).toEqual({ - forms: [], - }) - }) - - it('should return 200 with array of example forms that match given query.searchTerm when that is provided', async () => { - // Arrange - const session = await createAuthedSession(defaultUser.email, request) - - // Act - const response = await session.get('/examples').query({ - pageNo: '0', - agency: comAgency._id.toString(), - shouldGetTotalNumResults: 'true', - searchTerm: SearchTerm.First, - }) - - // Assert - // Should only have comTestData since its agency id is provided. - const expectedBody = keyBy( - stringifyFormInfoArray(comTestData.first.expectedFormInfo), - '_id', - ) - const actualBody = keyBy(response.body.forms, '_id') - // Check shape. - expect(response.status).toEqual(200) - expect(response.body).toEqual({ - forms: expect.any(Array), - // Should have total num results key value. - totalNumResults: comTestData.first.formCount, - }) - expect(actualBody).toEqual(expectedBody) - }) - - it('should return 200 with correctly offset example forms according to query.pageNo when that is provided', async () => { - // Arrange - const session = await createAuthedSession(defaultUser.email, request) - - // Act - const response = await session.get('/examples').query({ - pageNo: '1', - shouldGetTotalNumResults: 'true', - }) - - // Assert - const actualBody = keyBy(response.body.forms, '_id') - // Check shape. - expect(response.status).toEqual(200) - expect(response.body).toEqual({ - forms: expect.any(Array), - // Should have total num results key value. - totalNumResults: - comTestData.total.formCount + orgTestData.total.formCount, - }) - // Should have nothing since the number of results is less than the - // offset. - expect(actualBody).toEqual({}) - }) - - it('should return 200 with array of example forms and total count when query.shouldGetTotalNumResults is true', async () => { - // Arrange - const session = await createAuthedSession(defaultUser.email, request) - - // Act - const response = await session.get('/examples').query({ - pageNo: '0', - agency: comAgency._id.toString(), - shouldGetTotalNumResults: 'true', - }) - - // Assert - // Should only have comTestData since its agency id is provided. - const expectedBody = keyBy( - stringifyFormInfoArray(comTestData.total.expectedFormInfo), - '_id', - ) - const actualBody = keyBy(response.body.forms, '_id') - // Check shape. - expect(response.status).toEqual(200) - expect(response.body).toEqual({ - forms: expect.any(Array), - // Should have total num results key value. - totalNumResults: comTestData.total.formCount, - }) - expect(actualBody).toEqual(expectedBody) - }) - }) - - describe('AggregateStats feature disabled', () => { - beforeAll(() => { - MockExamplesFactory.getExampleForms.mockImplementation( - getExampleForms(RetrievalType.Submissions), - ) - }) - it('should return 200 with array of example forms for all agencies when query.agency is missing', async () => { // Arrange const session = await createAuthedSession(defaultUser.email, request) @@ -493,7 +321,7 @@ describe('examples.routes', () => { it('should return 500 when an error occurs whilst querying the database', async () => { // Arrange const getExamplesSpy = jest - .spyOn(ExamplesFactory, 'getExampleForms') + .spyOn(ExamplesService, 'getExampleForms') .mockReturnValueOnce(errAsync(new DatabaseError())) const session = await createAuthedSession(defaultUser.email, request) @@ -513,12 +341,6 @@ describe('examples.routes', () => { describe('GET /examples/:formId', () => { describe('AggregateStats feature enabled', () => { - beforeAll(() => { - MockExamplesFactory.getSingleExampleForm.mockImplementation( - getSingleExampleForm(RetrievalType.Stats), - ) - }) - it('should return 200 with the example information of the retrieved form', async () => { // Arrange const session = await createAuthedSession(defaultUser.email, request) @@ -554,46 +376,6 @@ describe('examples.routes', () => { }) }) - describe('AggregateStats feature disabled', () => { - beforeAll(() => { - MockExamplesFactory.getSingleExampleForm.mockImplementation( - getSingleExampleForm(RetrievalType.Submissions), - ) - }) - it('should return 200 with the example information of the retrieved form', async () => { - // Arrange - const session = await createAuthedSession(defaultUser.email, request) - const validFormId = comTestData.first.forms[1]._id - - // Act - const response = await session.get(`/examples/${validFormId}`) - - // Assert - const expectedFormInfo = stringifyFormInfo( - comTestData.first.expectedFormInfo[1], - ) - expect(response.status).toEqual(200) - expect(response.body).toEqual({ - form: expectedFormInfo, - }) - }) - - it('should return 404 when the form with the given formId does not exist in the database', async () => { - // Arrange - const session = await createAuthedSession(defaultUser.email, request) - const randomFormId = new ObjectId().toHexString() - - // Act - const response = await session.get(`/examples/${randomFormId}`) - - // Assert - expect(response.status).toEqual(404) - expect(response.body).toEqual({ - message: 'Error in retrieving template form - form not found.', - }) - }) - }) - it('should return 401 when user is not logged in', async () => { // Arrange const validFormId = comTestData.first.forms[0]._id @@ -611,9 +393,9 @@ describe('examples.routes', () => { const session = await createAuthedSession(defaultUser.email, request) const validFormId = comTestData.first.forms[0]._id const mockErrorString = 'database error' - MockExamplesFactory.getSingleExampleForm.mockReturnValueOnce( - errAsync(new DatabaseError(mockErrorString)), - ) + jest + .spyOn(ExamplesService, 'getSingleExampleForm') + .mockReturnValueOnce(errAsync(new DatabaseError(mockErrorString))) // Act const response = await session.get(`/examples/${validFormId}`) diff --git a/src/app/modules/examples/__tests__/examples.service.spec.ts b/src/app/modules/examples/__tests__/examples.service.spec.ts index 1a7f307f89..6d38bb2c52 100644 --- a/src/app/modules/examples/__tests__/examples.service.spec.ts +++ b/src/app/modules/examples/__tests__/examples.service.spec.ts @@ -2,19 +2,16 @@ import { ObjectId } from 'bson-ext' import mongoose from 'mongoose' import getFormStatisticsTotalModel from 'src/app/models/form_statistics_total.server.model' -import getSubmissionModel from 'src/app/models/submission.server.model' import dbHandler from 'tests/unit/backend/helpers/jest-db' import { PAGE_SIZE } from '../examples.constants' import { ResultsNotFoundError } from '../examples.errors' import * as ExamplesService from '../examples.service' -import { RetrievalType } from '../examples.types' import prepareTestData, { TestData } from './helpers/prepareTestData' const FormStatsModel = getFormStatisticsTotalModel(mongoose) -const SubmissionModel = getSubmissionModel(mongoose) // Mock min sub count so anything above 0 submissions will be counted. jest.mock('../examples.constants', () => ({ @@ -35,15 +32,11 @@ describe('examples.service', () => { describe('getExampleForms', () => { describe('with RetrievalType.Stats', () => { - const getExampleFormsUsingStats = ExamplesService.getExampleForms( - RetrievalType.Stats, - ) - describe('when query.searchTerm exists', () => { describe('when query.shouldGetTotalNumResults is true', () => { it('should return list of form info that match the search term with results count', async () => { // Act - const actualResults = await getExampleFormsUsingStats({ + const actualResults = await ExamplesService.getExampleForms({ searchTerm: testData.second.searchTerm, pageNo: 0, shouldGetTotalNumResults: true, @@ -59,7 +52,7 @@ describe('examples.service', () => { it('should return empty list if no forms match search term with 0 result count', async () => { // Act - const actualResults = await getExampleFormsUsingStats({ + const actualResults = await ExamplesService.getExampleForms({ searchTerm: INVALID_SEARCH_TERM, pageNo: 0, shouldGetTotalNumResults: true, @@ -77,7 +70,7 @@ describe('examples.service', () => { describe('when query.shouldGetTotalNumResults is false', () => { it('should return only list of form info that match the search term', async () => { // Act - const actualResults = await getExampleFormsUsingStats({ + const actualResults = await ExamplesService.getExampleForms({ searchTerm: testData.first.searchTerm, pageNo: 0, shouldGetTotalNumResults: false, @@ -92,7 +85,7 @@ describe('examples.service', () => { it('should return empty list if no forms match search term', async () => { // Act - const actualResults = await getExampleFormsUsingStats({ + const actualResults = await ExamplesService.getExampleForms({ searchTerm: INVALID_SEARCH_TERM, pageNo: 0, shouldGetTotalNumResults: false, @@ -111,7 +104,7 @@ describe('examples.service', () => { describe('when query.shouldGetTotalNumResults is true', () => { it('should return list of form info with results count', async () => { // Act - const actualResults = await getExampleFormsUsingStats({ + const actualResults = await ExamplesService.getExampleForms({ pageNo: 0, shouldGetTotalNumResults: true, }) @@ -129,7 +122,7 @@ describe('examples.service', () => { const overOffset = (await FormStatsModel.estimatedDocumentCount()) / PAGE_SIZE + 1 // Act - const actualResults = await getExampleFormsUsingStats({ + const actualResults = await ExamplesService.getExampleForms({ pageNo: overOffset, shouldGetTotalNumResults: true, }) @@ -146,7 +139,7 @@ describe('examples.service', () => { describe('when query.shouldGetTotalNumResults is false', () => { it('should return list of form info', async () => { // Act - const actualResults = await getExampleFormsUsingStats({ + const actualResults = await ExamplesService.getExampleForms({ pageNo: 0, shouldGetTotalNumResults: false, }) @@ -163,152 +156,7 @@ describe('examples.service', () => { const overOffset = (await FormStatsModel.estimatedDocumentCount()) / PAGE_SIZE + 1 // Act - const actualResults = await getExampleFormsUsingStats({ - pageNo: overOffset, - shouldGetTotalNumResults: false, - }) - - // Assert - expect(actualResults.isOk()).toEqual(true) - expect(actualResults._unsafeUnwrap()).toEqual({ - forms: [], - }) - }) - }) - }) - }) - - describe('with RetrievalType.Submissions', () => { - const getExampleFormsUsingSubs = ExamplesService.getExampleForms( - RetrievalType.Submissions, - ) - - describe('when query.searchTerm exists', () => { - describe('when query.shouldGetTotalNumResults is true', () => { - it('should return list of form info that match the search term with results count', async () => { - // Act - const actualResults = await getExampleFormsUsingSubs({ - searchTerm: testData.second.searchTerm, - pageNo: 0, - shouldGetTotalNumResults: true, - }) - - // Assert - expect(actualResults.isOk()).toEqual(true) - expect(actualResults._unsafeUnwrap()).toEqual({ - totalNumResults: testData.second.formCount, - forms: expect.arrayContaining(testData.second.expectedFormInfo), - }) - }) - - it('should return empty list if no forms match search term with 0 result count', async () => { - // Act - const actualResults = await getExampleFormsUsingSubs({ - searchTerm: INVALID_SEARCH_TERM, - pageNo: 0, - shouldGetTotalNumResults: true, - }) - - // Assert - expect(actualResults.isOk()).toEqual(true) - expect(actualResults._unsafeUnwrap()).toEqual({ - forms: [], - totalNumResults: 0, - }) - }) - }) - - describe('when query.shouldGetTotalNumResults is false', () => { - it('should return only list of form info that match the search term', async () => { - // Act - const actualResults = await getExampleFormsUsingSubs({ - searchTerm: testData.first.searchTerm, - pageNo: 0, - shouldGetTotalNumResults: false, - }) - - // Assert - expect(actualResults.isOk()).toEqual(true) - expect(actualResults._unsafeUnwrap().forms).toEqual( - expect.arrayContaining(testData.first.expectedFormInfo), - ) - }) - - it('should return empty list if no forms match search term', async () => { - // Act - const actualResults = await getExampleFormsUsingSubs({ - searchTerm: INVALID_SEARCH_TERM, - pageNo: 0, - shouldGetTotalNumResults: false, - }) - - // Assert - expect(actualResults.isOk()).toEqual(true) - expect(actualResults._unsafeUnwrap()).toEqual({ - forms: [], - }) - }) - }) - }) - - describe('when query.searchTerm does not exist', () => { - describe('when query.shouldGetTotalNumResults is true', () => { - it('should return list of form info with results count', async () => { - // Act - const actualResults = await getExampleFormsUsingSubs({ - pageNo: 0, - shouldGetTotalNumResults: true, - }) - - // Assert - expect(actualResults.isOk()).toEqual(true) - expect(actualResults._unsafeUnwrap()).toEqual({ - totalNumResults: testData.total.formCount, - forms: expect.arrayContaining(testData.total.expectedFormInfo), - }) - }) - - it('should return empty list with total number of submissions when offset is more than number of documents in collection', async () => { - // Arrange - const overOffset = - (await SubmissionModel.estimatedDocumentCount()) / PAGE_SIZE + 1 - // Act - const actualResults = await getExampleFormsUsingSubs({ - pageNo: overOffset, - shouldGetTotalNumResults: true, - }) - - // Assert - expect(actualResults.isOk()).toEqual(true) - expect(actualResults._unsafeUnwrap()).toEqual({ - forms: [], - totalNumResults: testData.total.formCount, - }) - }) - }) - - describe('when query.shouldGetTotalNumResults is false', () => { - it('should return only list of form info', async () => { - // Act - const actualResults = await getExampleFormsUsingSubs({ - pageNo: 0, - shouldGetTotalNumResults: false, - }) - - // Assert - expect(actualResults.isOk()).toEqual(true) - // Return should be all forms - expect(actualResults._unsafeUnwrap()).toEqual({ - forms: expect.arrayContaining(testData.total.expectedFormInfo), - }) - }) - - it('should return empty list when offset is more than number of documents', async () => { - // Arrange - const overOffset = - (await FormStatsModel.estimatedDocumentCount()) / PAGE_SIZE + 1 - // Act - const actualResults = await getExampleFormsUsingSubs({ + const actualResults = await ExamplesService.getExampleForms({ pageNo: overOffset, shouldGetTotalNumResults: false, }) @@ -326,48 +174,12 @@ describe('examples.service', () => { describe('getSingleExampleForm', () => { describe('with RetrievalType.Stats', () => { - const getSingleFormUsingSubs = ExamplesService.getSingleExampleForm( - RetrievalType.Submissions, - ) - it('should return form info of given formId when form exists in the database', async () => { // Arrange const expectedFormInfo = testData.first.expectedFormInfo[0] // Act - const actualResults = await getSingleFormUsingSubs(expectedFormInfo._id) - - // Assert - expect(actualResults.isOk()).toEqual(true) - expect(actualResults._unsafeUnwrap()).toEqual({ - form: expectedFormInfo, - }) - }) - - it('should return ResultsNotFoundError when form does not exist in the database', async () => { - // Act - const actualResults = await getSingleFormUsingSubs( - String(new ObjectId()), - ) - - // Assert - expect(actualResults.isErr()).toEqual(true) - expect(actualResults._unsafeUnwrapErr()).toBeInstanceOf( - ResultsNotFoundError, - ) - }) - }) - - describe('with RetrievalType.Submissions', () => { - const getSingleFormUsingStats = ExamplesService.getSingleExampleForm( - RetrievalType.Stats, - ) - it('should return form info of given formId when form exists in the database', async () => { - // Arrange - const expectedFormInfo = testData.second.expectedFormInfo[1] - - // Act - const actualResults = await getSingleFormUsingStats( + const actualResults = await ExamplesService.getSingleExampleForm( expectedFormInfo._id, ) @@ -380,7 +192,7 @@ describe('examples.service', () => { it('should return ResultsNotFoundError when form does not exist in the database', async () => { // Act - const actualResults = await getSingleFormUsingStats( + const actualResults = await ExamplesService.getSingleExampleForm( String(new ObjectId()), ) From e821d0b0abd317da25e1d81f7d64437d71e499f4 Mon Sep 17 00:00:00 2001 From: Antariksh Date: Wed, 9 Jun 2021 18:35:37 +0800 Subject: [PATCH 3/4] chore: remove unnecessary describe blocks --- .../__tests__/examples.routes.spec.ts | 338 +++++++++--------- .../__tests__/examples.service.spec.ts | 300 ++++++++-------- 2 files changed, 315 insertions(+), 323 deletions(-) diff --git a/src/app/modules/examples/__tests__/examples.routes.spec.ts b/src/app/modules/examples/__tests__/examples.routes.spec.ts index 3984e4ff9b..13f9a1a4f4 100644 --- a/src/app/modules/examples/__tests__/examples.routes.spec.ts +++ b/src/app/modules/examples/__tests__/examples.routes.spec.ts @@ -66,159 +66,157 @@ describe('examples.routes', () => { afterAll(async () => await dbHandler.closeDatabase()) describe('GET /examples', () => { - describe('AggregateStats feature enabled', () => { - it('should return 200 with array of example forms for all agencies when query.agency is missing', async () => { - // Arrange - const session = await createAuthedSession(defaultUser.email, request) - - // Act - const response = await session.get('/examples').query({ - pageNo: '0', - }) - - // Assert - // Should have both comTestData and orgTestData since searching by all - // agencies. - const expectedBody = keyBy( - [ - ...stringifyFormInfoArray(comTestData.total.expectedFormInfo), - ...stringifyFormInfoArray(orgTestData.total.expectedFormInfo), - ], - '_id', - ) - const actualBody = keyBy(response.body.forms, '_id') - expect(response.status).toEqual(200) - // Check shape. - expect(response.body).toEqual({ - forms: expect.any(Array), - }) - expect(actualBody).toEqual(expectedBody) + it('should return 200 with array of example forms for all agencies when query.agency is missing', async () => { + // Arrange + const session = await createAuthedSession(defaultUser.email, request) + + // Act + const response = await session.get('/examples').query({ + pageNo: '0', + }) + + // Assert + // Should have both comTestData and orgTestData since searching by all + // agencies. + const expectedBody = keyBy( + [ + ...stringifyFormInfoArray(comTestData.total.expectedFormInfo), + ...stringifyFormInfoArray(orgTestData.total.expectedFormInfo), + ], + '_id', + ) + const actualBody = keyBy(response.body.forms, '_id') + expect(response.status).toEqual(200) + // Check shape. + expect(response.body).toEqual({ + forms: expect.any(Array), + }) + expect(actualBody).toEqual(expectedBody) + }) + + it('should return 200 with array of example forms for a particular agency when query.agency is provided', async () => { + // Arrange + const session = await createAuthedSession(defaultUser.email, request) + + // Act + const response = await session.get('/examples').query({ + pageNo: '0', + agency: orgAgency._id.toString(), + }) + + // Assert + // Should only have orgTestData since its agency id is provided. + const expectedBody = keyBy( + stringifyFormInfoArray(orgTestData.total.expectedFormInfo), + '_id', + ) + const actualBody = keyBy(response.body.forms, '_id') + // Check shape. + expect(response.status).toEqual(200) + expect(response.body).toEqual({ + forms: expect.any(Array), + }) + expect(actualBody).toEqual(expectedBody) + }) + + it('should return 200 with empty array when no forms match the criterias', async () => { + // Arrange + const session = await createAuthedSession(defaultUser.email, request) + + // Act + const response = await session.get('/examples').query({ + pageNo: '0', + agency: new ObjectId().toHexString(), + }) + + // Assert + expect(response.status).toEqual(200) + // Check shape. + expect(response.body).toEqual({ + forms: [], + }) + }) + + it('should return 200 with array of example forms that match given query.searchTerm when that is provided', async () => { + // Arrange + const session = await createAuthedSession(defaultUser.email, request) + + // Act + const response = await session.get('/examples').query({ + pageNo: '0', + agency: comAgency._id.toString(), + shouldGetTotalNumResults: 'true', + searchTerm: SearchTerm.First, }) - it('should return 200 with array of example forms for a particular agency when query.agency is provided', async () => { - // Arrange - const session = await createAuthedSession(defaultUser.email, request) - - // Act - const response = await session.get('/examples').query({ - pageNo: '0', - agency: orgAgency._id.toString(), - }) - - // Assert - // Should only have orgTestData since its agency id is provided. - const expectedBody = keyBy( - stringifyFormInfoArray(orgTestData.total.expectedFormInfo), - '_id', - ) - const actualBody = keyBy(response.body.forms, '_id') - // Check shape. - expect(response.status).toEqual(200) - expect(response.body).toEqual({ - forms: expect.any(Array), - }) - expect(actualBody).toEqual(expectedBody) + // Assert + // Should only have comTestData since its agency id is provided. + const expectedBody = keyBy( + stringifyFormInfoArray(comTestData.first.expectedFormInfo), + '_id', + ) + const actualBody = keyBy(response.body.forms, '_id') + // Check shape. + expect(response.status).toEqual(200) + expect(response.body).toEqual({ + forms: expect.any(Array), + // Should have total num results key value. + totalNumResults: comTestData.first.formCount, }) + expect(actualBody).toEqual(expectedBody) + }) - it('should return 200 with empty array when no forms match the criterias', async () => { - // Arrange - const session = await createAuthedSession(defaultUser.email, request) - - // Act - const response = await session.get('/examples').query({ - pageNo: '0', - agency: new ObjectId().toHexString(), - }) - - // Assert - expect(response.status).toEqual(200) - // Check shape. - expect(response.body).toEqual({ - forms: [], - }) + it('should return 200 with correctly offset example forms according to query.pageNo when that is provided', async () => { + // Arrange + const session = await createAuthedSession(defaultUser.email, request) + + // Act + const response = await session.get('/examples').query({ + pageNo: '1', + shouldGetTotalNumResults: 'true', }) - it('should return 200 with array of example forms that match given query.searchTerm when that is provided', async () => { - // Arrange - const session = await createAuthedSession(defaultUser.email, request) - - // Act - const response = await session.get('/examples').query({ - pageNo: '0', - agency: comAgency._id.toString(), - shouldGetTotalNumResults: 'true', - searchTerm: SearchTerm.First, - }) - - // Assert - // Should only have comTestData since its agency id is provided. - const expectedBody = keyBy( - stringifyFormInfoArray(comTestData.first.expectedFormInfo), - '_id', - ) - const actualBody = keyBy(response.body.forms, '_id') - // Check shape. - expect(response.status).toEqual(200) - expect(response.body).toEqual({ - forms: expect.any(Array), - // Should have total num results key value. - totalNumResults: comTestData.first.formCount, - }) - expect(actualBody).toEqual(expectedBody) + // Assert + const actualBody = keyBy(response.body.forms, '_id') + // Check shape. + expect(response.status).toEqual(200) + expect(response.body).toEqual({ + forms: expect.any(Array), + // Should have total num results key value. + totalNumResults: + comTestData.total.formCount + orgTestData.total.formCount, }) + // Should have nothing since the number of results is less than the + // offset. + expect(actualBody).toEqual({}) + }) - it('should return 200 with correctly offset example forms according to query.pageNo when that is provided', async () => { - // Arrange - const session = await createAuthedSession(defaultUser.email, request) - - // Act - const response = await session.get('/examples').query({ - pageNo: '1', - shouldGetTotalNumResults: 'true', - }) - - // Assert - const actualBody = keyBy(response.body.forms, '_id') - // Check shape. - expect(response.status).toEqual(200) - expect(response.body).toEqual({ - forms: expect.any(Array), - // Should have total num results key value. - totalNumResults: - comTestData.total.formCount + orgTestData.total.formCount, - }) - // Should have nothing since the number of results is less than the - // offset. - expect(actualBody).toEqual({}) + it('should return 200 with array of example forms and total count when query.shouldGetTotalNumResults is true', async () => { + // Arrange + const session = await createAuthedSession(defaultUser.email, request) + + // Act + const response = await session.get('/examples').query({ + pageNo: '0', + agency: comAgency._id.toString(), + shouldGetTotalNumResults: 'true', }) - it('should return 200 with array of example forms and total count when query.shouldGetTotalNumResults is true', async () => { - // Arrange - const session = await createAuthedSession(defaultUser.email, request) - - // Act - const response = await session.get('/examples').query({ - pageNo: '0', - agency: comAgency._id.toString(), - shouldGetTotalNumResults: 'true', - }) - - // Assert - // Should only have comTestData since its agency id is provided. - const expectedBody = keyBy( - stringifyFormInfoArray(comTestData.total.expectedFormInfo), - '_id', - ) - const actualBody = keyBy(response.body.forms, '_id') - // Check shape. - expect(response.status).toEqual(200) - expect(response.body).toEqual({ - forms: expect.any(Array), - // Should have total num results key value. - totalNumResults: comTestData.total.formCount, - }) - expect(actualBody).toEqual(expectedBody) + // Assert + // Should only have comTestData since its agency id is provided. + const expectedBody = keyBy( + stringifyFormInfoArray(comTestData.total.expectedFormInfo), + '_id', + ) + const actualBody = keyBy(response.body.forms, '_id') + // Check shape. + expect(response.status).toEqual(200) + expect(response.body).toEqual({ + forms: expect.any(Array), + // Should have total num results key value. + totalNumResults: comTestData.total.formCount, }) + expect(actualBody).toEqual(expectedBody) }) it('should return 400 when query.pageNo is not provided', async () => { @@ -340,39 +338,37 @@ describe('examples.routes', () => { }) describe('GET /examples/:formId', () => { - describe('AggregateStats feature enabled', () => { - it('should return 200 with the example information of the retrieved form', async () => { - // Arrange - const session = await createAuthedSession(defaultUser.email, request) - const validFormId = comTestData.second.forms[0]._id - - // Act - const response = await session.get(`/examples/${validFormId}`) - - // Assert - const expectedFormInfo = stringifyFormInfo( - comTestData.second.expectedFormInfo[0], - ) - - expect(response.status).toEqual(200) - expect(response.body).toEqual({ - form: expectedFormInfo, - }) + it('should return 200 with the example information of the retrieved form', async () => { + // Arrange + const session = await createAuthedSession(defaultUser.email, request) + const validFormId = comTestData.second.forms[0]._id + + // Act + const response = await session.get(`/examples/${validFormId}`) + + // Assert + const expectedFormInfo = stringifyFormInfo( + comTestData.second.expectedFormInfo[0], + ) + + expect(response.status).toEqual(200) + expect(response.body).toEqual({ + form: expectedFormInfo, }) + }) - it('should return 404 when the form with the given formId does not exist in the database', async () => { - // Arrange - const session = await createAuthedSession(defaultUser.email, request) - const randomFormId = new ObjectId().toHexString() + it('should return 404 when the form with the given formId does not exist in the database', async () => { + // Arrange + const session = await createAuthedSession(defaultUser.email, request) + const randomFormId = new ObjectId().toHexString() - // Act - const response = await session.get(`/examples/${randomFormId}`) + // Act + const response = await session.get(`/examples/${randomFormId}`) - // Assert - expect(response.status).toEqual(404) - expect(response.body).toEqual({ - message: 'Error in retrieving template form - form not found.', - }) + // Assert + expect(response.status).toEqual(404) + expect(response.body).toEqual({ + message: 'Error in retrieving template form - form not found.', }) }) diff --git a/src/app/modules/examples/__tests__/examples.service.spec.ts b/src/app/modules/examples/__tests__/examples.service.spec.ts index 6d38bb2c52..35b7783009 100644 --- a/src/app/modules/examples/__tests__/examples.service.spec.ts +++ b/src/app/modules/examples/__tests__/examples.service.spec.ts @@ -31,177 +31,173 @@ describe('examples.service', () => { afterAll(async () => await dbHandler.closeDatabase()) describe('getExampleForms', () => { - describe('with RetrievalType.Stats', () => { - describe('when query.searchTerm exists', () => { - describe('when query.shouldGetTotalNumResults is true', () => { - it('should return list of form info that match the search term with results count', async () => { - // Act - const actualResults = await ExamplesService.getExampleForms({ - searchTerm: testData.second.searchTerm, - pageNo: 0, - shouldGetTotalNumResults: true, - }) - - // Assert - expect(actualResults.isOk()).toEqual(true) - expect(actualResults._unsafeUnwrap()).toEqual({ - totalNumResults: testData.second.formCount, - forms: expect.arrayContaining(testData.second.expectedFormInfo), - }) - }) - - it('should return empty list if no forms match search term with 0 result count', async () => { - // Act - const actualResults = await ExamplesService.getExampleForms({ - searchTerm: INVALID_SEARCH_TERM, - pageNo: 0, - shouldGetTotalNumResults: true, - }) - - // Assert - expect(actualResults.isOk()).toEqual(true) - expect(actualResults._unsafeUnwrap()).toEqual({ - forms: [], - totalNumResults: 0, - }) + describe('when query.searchTerm exists', () => { + describe('when query.shouldGetTotalNumResults is true', () => { + it('should return list of form info that match the search term with results count', async () => { + // Act + const actualResults = await ExamplesService.getExampleForms({ + searchTerm: testData.second.searchTerm, + pageNo: 0, + shouldGetTotalNumResults: true, + }) + + // Assert + expect(actualResults.isOk()).toEqual(true) + expect(actualResults._unsafeUnwrap()).toEqual({ + totalNumResults: testData.second.formCount, + forms: expect.arrayContaining(testData.second.expectedFormInfo), }) }) - describe('when query.shouldGetTotalNumResults is false', () => { - it('should return only list of form info that match the search term', async () => { - // Act - const actualResults = await ExamplesService.getExampleForms({ - searchTerm: testData.first.searchTerm, - pageNo: 0, - shouldGetTotalNumResults: false, - }) - - // Assert - expect(actualResults.isOk()).toEqual(true) - expect(actualResults._unsafeUnwrap()).toEqual({ - forms: expect.arrayContaining(testData.first.expectedFormInfo), - }) - }) - - it('should return empty list if no forms match search term', async () => { - // Act - const actualResults = await ExamplesService.getExampleForms({ - searchTerm: INVALID_SEARCH_TERM, - pageNo: 0, - shouldGetTotalNumResults: false, - }) - - // Assert - expect(actualResults.isOk()).toEqual(true) - expect(actualResults._unsafeUnwrap()).toEqual({ - forms: [], - }) + it('should return empty list if no forms match search term with 0 result count', async () => { + // Act + const actualResults = await ExamplesService.getExampleForms({ + searchTerm: INVALID_SEARCH_TERM, + pageNo: 0, + shouldGetTotalNumResults: true, + }) + + // Assert + expect(actualResults.isOk()).toEqual(true) + expect(actualResults._unsafeUnwrap()).toEqual({ + forms: [], + totalNumResults: 0, }) }) }) - describe('when query.searchTerm does not exist', () => { - describe('when query.shouldGetTotalNumResults is true', () => { - it('should return list of form info with results count', async () => { - // Act - const actualResults = await ExamplesService.getExampleForms({ - pageNo: 0, - shouldGetTotalNumResults: true, - }) - - // Assert - expect(actualResults.isOk()).toEqual(true) - expect(actualResults._unsafeUnwrap()).toEqual({ - totalNumResults: testData.total.formCount, - forms: expect.arrayContaining(testData.total.expectedFormInfo), - }) - }) - - it('should return empty list with number of forms with submissions when offset is more than number of documents in collection', async () => { - // Arrange - const overOffset = - (await FormStatsModel.estimatedDocumentCount()) / PAGE_SIZE + 1 - // Act - const actualResults = await ExamplesService.getExampleForms({ - pageNo: overOffset, - shouldGetTotalNumResults: true, - }) - - // Assert - expect(actualResults.isOk()).toEqual(true) - expect(actualResults._unsafeUnwrap()).toEqual({ - forms: [], - totalNumResults: testData.total.formCount, - }) + describe('when query.shouldGetTotalNumResults is false', () => { + it('should return only list of form info that match the search term', async () => { + // Act + const actualResults = await ExamplesService.getExampleForms({ + searchTerm: testData.first.searchTerm, + pageNo: 0, + shouldGetTotalNumResults: false, + }) + + // Assert + expect(actualResults.isOk()).toEqual(true) + expect(actualResults._unsafeUnwrap()).toEqual({ + forms: expect.arrayContaining(testData.first.expectedFormInfo), }) }) - describe('when query.shouldGetTotalNumResults is false', () => { - it('should return list of form info', async () => { - // Act - const actualResults = await ExamplesService.getExampleForms({ - pageNo: 0, - shouldGetTotalNumResults: false, - }) - - // Assert - expect(actualResults.isOk()).toEqual(true) - expect(actualResults._unsafeUnwrap()).toEqual({ - forms: expect.arrayContaining(testData.total.expectedFormInfo), - }) - }) - - it('should return empty list when offset is more than number of documents', async () => { - // Arrange - const overOffset = - (await FormStatsModel.estimatedDocumentCount()) / PAGE_SIZE + 1 - // Act - const actualResults = await ExamplesService.getExampleForms({ - pageNo: overOffset, - shouldGetTotalNumResults: false, - }) - - // Assert - expect(actualResults.isOk()).toEqual(true) - expect(actualResults._unsafeUnwrap()).toEqual({ - forms: [], - }) + it('should return empty list if no forms match search term', async () => { + // Act + const actualResults = await ExamplesService.getExampleForms({ + searchTerm: INVALID_SEARCH_TERM, + pageNo: 0, + shouldGetTotalNumResults: false, + }) + + // Assert + expect(actualResults.isOk()).toEqual(true) + expect(actualResults._unsafeUnwrap()).toEqual({ + forms: [], }) }) }) }) - }) - describe('getSingleExampleForm', () => { - describe('with RetrievalType.Stats', () => { - it('should return form info of given formId when form exists in the database', async () => { - // Arrange - const expectedFormInfo = testData.first.expectedFormInfo[0] - - // Act - const actualResults = await ExamplesService.getSingleExampleForm( - expectedFormInfo._id, - ) - - // Assert - expect(actualResults.isOk()).toEqual(true) - expect(actualResults._unsafeUnwrap()).toEqual({ - form: expectedFormInfo, + describe('when query.searchTerm does not exist', () => { + describe('when query.shouldGetTotalNumResults is true', () => { + it('should return list of form info with results count', async () => { + // Act + const actualResults = await ExamplesService.getExampleForms({ + pageNo: 0, + shouldGetTotalNumResults: true, + }) + + // Assert + expect(actualResults.isOk()).toEqual(true) + expect(actualResults._unsafeUnwrap()).toEqual({ + totalNumResults: testData.total.formCount, + forms: expect.arrayContaining(testData.total.expectedFormInfo), + }) + }) + + it('should return empty list with number of forms with submissions when offset is more than number of documents in collection', async () => { + // Arrange + const overOffset = + (await FormStatsModel.estimatedDocumentCount()) / PAGE_SIZE + 1 + // Act + const actualResults = await ExamplesService.getExampleForms({ + pageNo: overOffset, + shouldGetTotalNumResults: true, + }) + + // Assert + expect(actualResults.isOk()).toEqual(true) + expect(actualResults._unsafeUnwrap()).toEqual({ + forms: [], + totalNumResults: testData.total.formCount, + }) }) }) - it('should return ResultsNotFoundError when form does not exist in the database', async () => { - // Act - const actualResults = await ExamplesService.getSingleExampleForm( - String(new ObjectId()), - ) - - // Assert - expect(actualResults.isErr()).toEqual(true) - expect(actualResults._unsafeUnwrapErr()).toBeInstanceOf( - ResultsNotFoundError, - ) + describe('when query.shouldGetTotalNumResults is false', () => { + it('should return list of form info', async () => { + // Act + const actualResults = await ExamplesService.getExampleForms({ + pageNo: 0, + shouldGetTotalNumResults: false, + }) + + // Assert + expect(actualResults.isOk()).toEqual(true) + expect(actualResults._unsafeUnwrap()).toEqual({ + forms: expect.arrayContaining(testData.total.expectedFormInfo), + }) + }) + + it('should return empty list when offset is more than number of documents', async () => { + // Arrange + const overOffset = + (await FormStatsModel.estimatedDocumentCount()) / PAGE_SIZE + 1 + // Act + const actualResults = await ExamplesService.getExampleForms({ + pageNo: overOffset, + shouldGetTotalNumResults: false, + }) + + // Assert + expect(actualResults.isOk()).toEqual(true) + expect(actualResults._unsafeUnwrap()).toEqual({ + forms: [], + }) + }) + }) + }) + }) + + describe('getSingleExampleForm', () => { + it('should return form info of given formId when form exists in the database', async () => { + // Arrange + const expectedFormInfo = testData.first.expectedFormInfo[0] + + // Act + const actualResults = await ExamplesService.getSingleExampleForm( + expectedFormInfo._id, + ) + + // Assert + expect(actualResults.isOk()).toEqual(true) + expect(actualResults._unsafeUnwrap()).toEqual({ + form: expectedFormInfo, }) }) + + it('should return ResultsNotFoundError when form does not exist in the database', async () => { + // Act + const actualResults = await ExamplesService.getSingleExampleForm( + String(new ObjectId()), + ) + + // Assert + expect(actualResults.isErr()).toEqual(true) + expect(actualResults._unsafeUnwrapErr()).toBeInstanceOf( + ResultsNotFoundError, + ) + }) }) }) From 268fd08b66a62f77633b8c1c386789cb32b41f72 Mon Sep 17 00:00:00 2001 From: Antariksh Date: Wed, 9 Jun 2021 18:36:59 +0800 Subject: [PATCH 4/4] chore: remove mentions of AGGREGATE_COLLECTION --- .template-env | 1 - docker-compose.yml | 1 - docs/DEPLOYMENT_SETUP.md | 10 ---------- 3 files changed, 12 deletions(-) diff --git a/.template-env b/.template-env index 686a8748ab..2e6fcf10d0 100644 --- a/.template-env +++ b/.template-env @@ -31,7 +31,6 @@ FORMSG_SDK_MODE= # APP_NAME=FormSG # OTP_LIFE_SPAN=900000 # BOUNCE_LIFE_SPAN=86400000 -# AGGREGATE_COLLECTION= # If provided, a banner with the provided message will show up in every form. # IS_GENERAL_MAINTENANCE= diff --git a/docker-compose.yml b/docker-compose.yml index 63c317559d..bbe3a34860 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -71,7 +71,6 @@ services: - CORPPASS_IDP_ID - IS_SP_MAINTENANCE - IS_CP_MAINTENANCE - - AGGREGATE_COLLECTION mockpass: build: https://github.com/opengovsg/mockpass.git diff --git a/docs/DEPLOYMENT_SETUP.md b/docs/DEPLOYMENT_SETUP.md index 4c60788dcf..e1f9fb036e 100644 --- a/docs/DEPLOYMENT_SETUP.md +++ b/docs/DEPLOYMENT_SETUP.md @@ -251,16 +251,6 @@ If this feature is enabled, client-side error events will be piped to [sentry.io | `SENTRY_CONFIG_URL` | Sentry.io URL for configuring the Raven SDK. | | `CSP_REPORT_URI` | Reporting URL for Content Security Policy violdations. Can be configured to use a Sentry.io endpoint. | -#### Examples page Using Pre-Computed Results - -If this feature is enabled, the submission statistics associated with forms loaded on the examples page will be fetched from the pre-computed values in the FormStatisticsTotal collection. The FormStatisticsTotal collection only exists if the [batch jobs](https://github.com/datagovsg/formsg-batch-jobs/) needed to calculate the submission statistics are run daily. - -If this feature is not enabled, the submission statistics associated with forms loaded on the examples page will be calculated on the fly from the Submissions collection. This may be sub-optimal when submissions are in the millions. - -| Variable | Description | -| :--------------------- | --------------------------------------------------------------------- | -| `AGGREGATE_COLLECTION` | Has to be defined (i.e. =true) if FormStats collection is to be used. | - #### SMS with Twilio If this feature is enabled, the Mobile Number field will support form-fillers verifying their mobile numbers via a One-Time-Pin sent to their mobile phones and will also support an SMS confirmation of successful submission being sent out to their said mobile numbers. All messages are sent using [twilio](https://www.twilio.com/) messaging APIs.