From 324a5101e22d4a2c5d08c4e9663a601adeec2833 Mon Sep 17 00:00:00 2001 From: seaerchin <44049504+seaerchin@users.noreply.github.com> Date: Wed, 7 Apr 2021 17:22:59 +0800 Subject: [PATCH] refactor: migrate admin-console client service to ts (#1531) * feat(public/services/billingservice): added billing service * feat(public/services/exampleservice): added example service * test(public/services/billingservice): adds tests for getBillingInfo * test(public/services/exampleservice): adds tests for getSingleExampleForm and getExampleForms * refactor(billingservice): added dto types to fe/be and type annnotation on get for fe * style(public/exampleservice): added curly braces; added type annotation on methods * refactor(examples): adds dto to params and updated fe file naming to fit be * test(examplesservice): updated tests to use new dto shape * refactor(billing): updated billing fe/be to use dto * test(billingservice): updated tests to use dto for params * refactor(examples-list/controller): updated to use ExamplesService * refactor(billing/client/controller): replaced AdminService with new BillingService * refactor(forms/client/routes): replaced AdminConsole with ExamplesService * chore(main.js): deletes old admin-console.client.service * test(billingservice): updated test blocks * test(exampleservice): updated test blocks * style(types/billing): imported LoginStatistic rather than redefining --- .../__tests__/billing.controller.spec.ts | 4 +- .../billing/__tests__/billing.routes.spec.ts | 4 +- src/app/modules/billing/billing.controller.ts | 11 +-- .../modules/examples/examples.controller.ts | 20 ++-- src/public/main.js | 1 - .../forms/config/forms.client.routes.js | 10 +- .../controllers/billing.client.controller.js | 21 ++--- .../examples-list.client.controller.js | 20 ++-- .../services/admin-console.client.service.js | 68 -------------- src/public/services/BillingService.ts | 21 +++++ src/public/services/ExamplesService.ts | 42 +++++++++ .../services/__tests__/BillingService.test.ts | 52 +++++++++++ .../__tests__/ExamplesService.test.ts | 91 +++++++++++++++++++ src/types/api/billing.ts | 12 +++ src/types/api/examples.ts | 20 ++++ src/types/api/index.ts | 2 + 16 files changed, 291 insertions(+), 108 deletions(-) delete mode 100644 src/public/modules/users/services/admin-console.client.service.js create mode 100644 src/public/services/BillingService.ts create mode 100644 src/public/services/ExamplesService.ts create mode 100644 src/public/services/__tests__/BillingService.test.ts create mode 100644 src/public/services/__tests__/ExamplesService.test.ts create mode 100644 src/types/api/billing.ts create mode 100644 src/types/api/examples.ts diff --git a/src/app/modules/billing/__tests__/billing.controller.spec.ts b/src/app/modules/billing/__tests__/billing.controller.spec.ts index 374c936c7b..6b421a3de1 100644 --- a/src/app/modules/billing/__tests__/billing.controller.spec.ts +++ b/src/app/modules/billing/__tests__/billing.controller.spec.ts @@ -92,7 +92,9 @@ describe('billing.controller', () => { ...EXPECTED_SERVICE_CALL_ARGS, ) expect(mockRes.status).toBeCalledWith(500) - expect(mockRes.json).toBeCalledWith('Error in retrieving billing records') + expect(mockRes.json).toBeCalledWith({ + message: 'Error in retrieving billing records', + }) }) }) }) diff --git a/src/app/modules/billing/__tests__/billing.routes.spec.ts b/src/app/modules/billing/__tests__/billing.routes.spec.ts index 3dfa904ba7..5bc76cde85 100644 --- a/src/app/modules/billing/__tests__/billing.routes.spec.ts +++ b/src/app/modules/billing/__tests__/billing.routes.spec.ts @@ -206,7 +206,9 @@ describe('billing.routes', () => { // Assert expect(retrieveStatsSpy).toBeCalled() expect(response.status).toEqual(500) - expect(response.body).toEqual('Error in retrieving billing records') + expect(response.body).toEqual({ + message: 'Error in retrieving billing records', + }) }) }) }) diff --git a/src/app/modules/billing/billing.controller.ts b/src/app/modules/billing/billing.controller.ts index 09d0032c34..77d9e6fb3b 100644 --- a/src/app/modules/billing/billing.controller.ts +++ b/src/app/modules/billing/billing.controller.ts @@ -4,6 +4,7 @@ import { StatusCodes } from 'http-status-codes' import moment from 'moment-timezone' import { createLoggerWithLabel } from '../../../config/logger' +import { BillingInfoDto, BillingQueryDto, ErrorDto } from '../../../types/api' import { createReqMeta } from '../../utils/request' import { BillingFactory } from './billing.factory' @@ -20,13 +21,9 @@ const logger = createLoggerWithLabel(module) */ export const handleGetBillInfo: RequestHandler< ParamsDictionary, + ErrorDto | BillingInfoDto, unknown, - unknown, - { - esrvcId: string - yr: string - mth: string - } + BillingQueryDto > = async (req, res) => { const { esrvcId, mth, yr } = req.query const authedUser = (req.session as Express.AuthedSession).user @@ -54,7 +51,7 @@ export const handleGetBillInfo: RequestHandler< }) return res .status(StatusCodes.INTERNAL_SERVER_ERROR) - .json('Error in retrieving billing records') + .json({ message: 'Error in retrieving billing records' }) } // Retrieved login stats successfully. diff --git a/src/app/modules/examples/examples.controller.ts b/src/app/modules/examples/examples.controller.ts index 0de411868e..df080efa23 100644 --- a/src/app/modules/examples/examples.controller.ts +++ b/src/app/modules/examples/examples.controller.ts @@ -3,10 +3,15 @@ import { ParamsDictionary, Query } from 'express-serve-static-core' import { StatusCodes } from 'http-status-codes' import { createLoggerWithLabel } from '../../../config/logger' +import { + ErrorDto, + ExampleFormsQueryDto, + ExampleFormsResult, + ExampleSingleFormResult, +} from '../../../types/api' import { createReqMeta } from '../../utils/request' import { ExamplesFactory } from './examples.factory' -import { ExamplesQueryParams } from './examples.types' import { mapRouteError } from './examples.utils' const logger = createLoggerWithLabel(module) @@ -14,15 +19,16 @@ const logger = createLoggerWithLabel(module) /** * Handler for GET /examples endpoint. * @security session + * @param exampleFormsQuery The search terms to find forms for * @returns 200 with an array of forms to be listed on the examples page * @returns 401 when user does not exist in session * @returns 500 when error occurs whilst querying the database */ export const handleGetExamples: RequestHandler< ParamsDictionary, + ErrorDto | ExampleFormsResult, unknown, - unknown, - Query & ExamplesQueryParams + Query & ExampleFormsQueryDto > = (req, res) => { return ExamplesFactory.getExampleForms(req.query) .map((result) => res.status(StatusCodes.OK).json(result)) @@ -44,14 +50,16 @@ export const handleGetExamples: RequestHandler< /** * Handler for GET /examples/:formId endpoint. * @security session + * @param formId The id of the example form * @returns 200 with the retrieved form example * @returns 401 when user does not exist in session * @returns 404 when the form with given formId does not exist in the database * @returns 500 when error occurs whilst querying the database */ -export const handleGetExampleByFormId: RequestHandler<{ - formId: string -}> = (req, res) => { +export const handleGetExampleByFormId: RequestHandler< + { formId: string }, + ExampleSingleFormResult | ErrorDto +> = (req, res) => { const { formId } = req.params return ExamplesFactory.getSingleExampleForm(formId) diff --git a/src/public/main.js b/src/public/main.js index 1871c9ff2a..1bdfeacf96 100644 --- a/src/public/main.js +++ b/src/public/main.js @@ -283,7 +283,6 @@ require('./modules/users/config/users.client.routes.js') // User services require('./modules/users/services/auth.client.service.js') -require('./modules/users/services/admin-console.client.service.js') // User controllers require('./modules/users/controllers/authentication.client.controller.js') diff --git a/src/public/modules/forms/config/forms.client.routes.js b/src/public/modules/forms/config/forms.client.routes.js index edfc03eaac..5f5467c213 100644 --- a/src/public/modules/forms/config/forms.client.routes.js +++ b/src/public/modules/forms/config/forms.client.routes.js @@ -1,5 +1,7 @@ 'use strict' +const ExamplesService = require('../../../services/ExamplesService') + // Setting up route angular.module('forms').config([ '$stateProvider', @@ -72,22 +74,22 @@ angular.module('forms').config([ url: '/{formId:[0-9a-fA-F]{24}}/use-template', templateUrl: 'modules/users/views/examples.client.view.html', resolve: { - AdminConsole: 'AdminConsole', Auth: 'Auth', FormErrorService: 'FormErrorService', // If the user is logged in, this field will contain the form data of the provided formId, // otherwise it will only contain the formId itself. FormData: [ - 'AdminConsole', + '$q', 'Auth', 'FormErrorService', '$stateParams', - function (AdminConsole, Auth, FormErrorService, $stateParams) { + function ($q, Auth, FormErrorService, $stateParams) { if (!Auth.getUser()) { return $stateParams.formId } - return AdminConsole.getSingleExampleForm($stateParams.formId) + return $q + .when(ExamplesService.getSingleExampleForm($stateParams.formId)) .then(function (response) { response.form.isTemplate = true return response.form diff --git a/src/public/modules/users/controllers/billing.client.controller.js b/src/public/modules/users/controllers/billing.client.controller.js index fa1d7aaf1e..48388f2115 100644 --- a/src/public/modules/users/controllers/billing.client.controller.js +++ b/src/public/modules/users/controllers/billing.client.controller.js @@ -1,25 +1,20 @@ 'use strict' const CsvGenerator = require('../../forms/helpers/CsvGenerator') +const BillingService = require('../../../services/BillingService') angular .module('users') .controller('BillingController', [ + '$q', '$state', '$timeout', - 'AdminConsole', 'Auth', 'NgTableParams', BillingController, ]) -function BillingController( - $state, - $timeout, - AdminConsole, - Auth, - NgTableParams, -) { +function BillingController($q, $state, $timeout, Auth, NgTableParams) { const vm = this vm.user = Auth.getUser() @@ -102,10 +97,12 @@ function BillingController( esrvcId = esrvcId || vm.esrvcId vm.loading = true vm.searchError = false - AdminConsole.getBillingInfo( - vm.selectedTimePeriod.yr, - vm.selectedTimePeriod.mth, - esrvcId, + $q.when( + BillingService.getBillingInfo({ + yr: vm.selectedTimePeriod.yr, + mth: vm.selectedTimePeriod.mth, + esrvcId, + }), ).then( function (response) { // Remove loader diff --git a/src/public/modules/users/controllers/examples-list.client.controller.js b/src/public/modules/users/controllers/examples-list.client.controller.js index ee9918975a..c1a6c6047c 100644 --- a/src/public/modules/users/controllers/examples-list.client.controller.js +++ b/src/public/modules/users/controllers/examples-list.client.controller.js @@ -1,10 +1,12 @@ 'use strict' +const ExamplesService = require('../../../services/ExamplesService') + const PAGE_SIZE = 16 angular.module('users').controller('ExamplesController', [ + '$q', '$scope', - 'AdminConsole', 'Auth', 'GTag', '$state', @@ -17,8 +19,8 @@ angular.module('users').controller('ExamplesController', [ ]) function ExamplesController( + $q, $scope, - AdminConsole, Auth, GTag, $state, @@ -129,12 +131,14 @@ function ExamplesController( const shouldGetTotalNumResults = pageNo === 0 && searchTerm !== '' // Get next page of forms and add to ui - AdminConsole.getExampleForms({ - pageNo, - searchTerm, - agency, - shouldGetTotalNumResults, - }).then( + $q.when( + ExamplesService.getExampleForms({ + pageNo, + searchTerm, + agency, + shouldGetTotalNumResults, + }), + ).then( function (response) { /* Form Properties ------------------ diff --git a/src/public/modules/users/services/admin-console.client.service.js b/src/public/modules/users/services/admin-console.client.service.js deleted file mode 100644 index c50aa97418..0000000000 --- a/src/public/modules/users/services/admin-console.client.service.js +++ /dev/null @@ -1,68 +0,0 @@ -angular.module('users').factory('AdminConsole', ['$q', '$http', AdminConsole]) - -function AdminConsole($q, $http) { - return { - getBillingInfo: function (yr, mth, esrvcId) { - let deferred = $q.defer() - $http({ - url: '/billing', - method: 'GET', - params: { yr, mth, esrvcId }, - }).then( - function (response) { - deferred.resolve(response.data) - }, - function () { - deferred.reject('Billing information could not be obtained.') - }, - ) - return deferred.promise - }, - /** - * Retrieve example forms for listing - */ - getExampleForms: function ({ - pageNo, - searchTerm, - agency, - shouldGetTotalNumResults, - }) { - let deferred = $q.defer() - $http({ - url: '/examples', - method: 'GET', - params: { pageNo, searchTerm, agency, shouldGetTotalNumResults }, - headers: { 'If-Modified-Since': '0' }, - // disable IE ajax request caching (so search requests don't get cached) - }).then( - function (response) { - deferred.resolve(response.data) - }, - function (error) { - deferred.reject(error) - }, - ) - return deferred.promise - }, - /** - * Retrieve a single form for examples - */ - getSingleExampleForm: function (formId) { - let deferred = $q.defer() - $http({ - url: `/examples/${formId}`, - method: 'GET', - headers: { 'If-Modified-Since': '0' }, - // disable IE ajax request caching (so search requests don't get cached) - }).then( - function (response) { - deferred.resolve(response.data) - }, - function (error) { - deferred.reject(error) - }, - ) - return deferred.promise - }, - } -} diff --git a/src/public/services/BillingService.ts b/src/public/services/BillingService.ts new file mode 100644 index 0000000000..1a77743300 --- /dev/null +++ b/src/public/services/BillingService.ts @@ -0,0 +1,21 @@ +import axios from 'axios' + +import { BillingInfoDto, BillingQueryDto } from '../../types/api/billing' + +// Exported for testing +export const BILLING_ENDPOINT = '/billing' + +/** + * Gets the billing information for the given month and year + * @param billingQueryParams The formId and the specific month to get the information for + * @returns Promise The billing statistics of the given month + */ +export const getBillingInfo = ( + billingQueryParams: BillingQueryDto, +): Promise => { + return axios + .get(BILLING_ENDPOINT, { + params: billingQueryParams, + }) + .then(({ data }) => data) +} diff --git a/src/public/services/ExamplesService.ts b/src/public/services/ExamplesService.ts new file mode 100644 index 0000000000..0575cdc711 --- /dev/null +++ b/src/public/services/ExamplesService.ts @@ -0,0 +1,42 @@ +import axios from 'axios' + +import { + ExampleFormsQueryDto, + ExampleFormsResult, + ExampleSingleFormResult, +} from '../../types/api' + +export const EXAMPLES_ENDPOINT = '/examples' + +/** + * Gets example forms that matches the specified parameters for listing + * @param exampleFormsSearchParams The search terms to query the backend for + * @returns The list of retrieved examples if `shouldGetTotalNumResults` is false + * @returns The list of retrieved examples with the total results if `shouldGetTotalNumResults` is true + */ +export const getExampleForms = ( + exampleFormsSearchParams: ExampleFormsQueryDto, +): Promise => { + return axios + .get(EXAMPLES_ENDPOINT, { + params: exampleFormsSearchParams, + // disable IE ajax request caching (so search requests don't get cached) + headers: { 'If-Modified-Since': '0' }, + }) + .then(({ data }) => data) +} +/** + * Gets a single form for examples + * @param formId The id of the form to search for + * @returns The information of the example form + */ +export const getSingleExampleForm = ( + formId: string, +): Promise => { + return axios + .get(`${EXAMPLES_ENDPOINT}/${formId}`, { + // disable IE ajax request caching (so search requests don't get cached) + headers: { 'If-Modified-Since': '0' }, + }) + .then(({ data }) => data) +} diff --git a/src/public/services/__tests__/BillingService.test.ts b/src/public/services/__tests__/BillingService.test.ts new file mode 100644 index 0000000000..8aa0bb7473 --- /dev/null +++ b/src/public/services/__tests__/BillingService.test.ts @@ -0,0 +1,52 @@ +import MockAxios from 'jest-mock-axios' + +import * as BillingService from '../BillingService' + +jest.mock('axios', () => MockAxios) + +describe('BillingService', () => { + describe('getBillingInfo', () => { + const MOCK_DATA = { + adminEmail: 'Big Mock', + formName: 'Mock Form', + total: 0, + formId: 'Mock', + authType: 'NIL', + } + const MOCK_PARAMS = { yr: '2020', mth: '12', esrvcId: 'mock' } + it('should return the billing information when the GET request succeeds', async () => { + // Arrange + MockAxios.get.mockResolvedValueOnce({ data: MOCK_DATA }) + + // Act + const actual = await BillingService.getBillingInfo(MOCK_PARAMS) + + // Assert + expect(MockAxios.get).toHaveBeenCalledWith( + BillingService.BILLING_ENDPOINT, + { + params: MOCK_PARAMS, + }, + ) + expect(actual).toEqual(MOCK_DATA) + }) + + it('should reject with the provided error message when the GET request fails', async () => { + // Arrange + const expected = new Error('Mock Error') + MockAxios.get.mockRejectedValueOnce(expected) + + // Act + const actualPromise = BillingService.getBillingInfo(MOCK_PARAMS) + + // Assert + await expect(actualPromise).rejects.toEqual(expected) + expect(MockAxios.get).toHaveBeenCalledWith( + BillingService.BILLING_ENDPOINT, + { + params: MOCK_PARAMS, + }, + ) + }) + }) +}) diff --git a/src/public/services/__tests__/ExamplesService.test.ts b/src/public/services/__tests__/ExamplesService.test.ts new file mode 100644 index 0000000000..b50f0edcc5 --- /dev/null +++ b/src/public/services/__tests__/ExamplesService.test.ts @@ -0,0 +1,91 @@ +import MockAxios from 'jest-mock-axios' + +import * as ExamplesService from '../ExamplesService' + +jest.mock('axios', () => MockAxios) + +describe('ExamplesService', () => { + afterEach(() => jest.clearAllMocks()) + describe('getExampleForms', () => { + const MOCK_PARAMS = { + pageNo: 1, + searchTerm: 'mock', + agency: 'Mock Gov', + shouldGetTotalNumResults: false, + } + const MOCK_HEADERS = { 'If-Modified-Since': '0' } + it('should return example forms data when the GET request succeeds', async () => { + // Arrange + const expected = {} + MockAxios.get.mockResolvedValueOnce({ data: {} }) + + // Act + const actual = await ExamplesService.getExampleForms(MOCK_PARAMS) + + // Assert + expect(MockAxios.get).toHaveBeenCalledWith( + ExamplesService.EXAMPLES_ENDPOINT, + { + params: MOCK_PARAMS, + headers: MOCK_HEADERS, + }, + ) + expect(actual).toEqual(expected) + }) + + it('should reject with the provided error message when the GET request fails', async () => { + // Arrange + const expected = new Error('Mock Error') + MockAxios.get.mockRejectedValueOnce(expected) + + // Act + const actualPromise = ExamplesService.getExampleForms(MOCK_PARAMS) + + // Assert + await expect(actualPromise).rejects.toEqual(expected) + expect(MockAxios.get).toHaveBeenCalledWith( + ExamplesService.EXAMPLES_ENDPOINT, + { + params: MOCK_PARAMS, + headers: MOCK_HEADERS, + }, + ) + }) + }) + + describe('getSingleExampleForm', () => { + const MOCK_FORM_ID = 'MOCK' + const MOCK_HEADERS = { 'If-Modified-Since': '0' } + it('should return example single form data when the GET request succeeds', async () => { + // Arrange + const expected = {} + const expectedMockEndpoint = `${ExamplesService.EXAMPLES_ENDPOINT}/${MOCK_FORM_ID}` + MockAxios.get.mockResolvedValueOnce({ data: {} }) + + // Act + const actual = await ExamplesService.getSingleExampleForm(MOCK_FORM_ID) + + // Assert + expect(MockAxios.get).toHaveBeenCalledWith(expectedMockEndpoint, { + headers: MOCK_HEADERS, + }) + expect(actual).toEqual(expected) + }) + + it('should reject with the provided error message when the GET request fails', async () => { + // Arrange + const expected = new Error('Mock Error') + const expectedMockEndpoint = `${ExamplesService.EXAMPLES_ENDPOINT}/${MOCK_FORM_ID}` + MockAxios.get.mockRejectedValueOnce(expected) + + // Act + const actualPromise = ExamplesService.getSingleExampleForm(MOCK_FORM_ID) + + // Assert + await expect(actualPromise).rejects.toEqual(expected) + expect(MockAxios.get).toHaveBeenCalledWith(expectedMockEndpoint, { + headers: MOCK_HEADERS, + }) + }) + }) +}) diff --git a/src/types/api/billing.ts b/src/types/api/billing.ts new file mode 100644 index 0000000000..60e3f03fcb --- /dev/null +++ b/src/types/api/billing.ts @@ -0,0 +1,12 @@ +import { LoginStatistic } from '../../types' + +// yr: The year to get the billing information for +// mth: The month to get the billing information for +// esrvcId: The id of the form +export type BillingQueryDto = { + esrvcId: string + yr: string + mth: string +} + +export type BillingInfoDto = { loginStats: LoginStatistic[] } diff --git a/src/types/api/examples.ts b/src/types/api/examples.ts new file mode 100644 index 0000000000..7cda15f7b1 --- /dev/null +++ b/src/types/api/examples.ts @@ -0,0 +1,20 @@ +import { + QueryPageResult, + QueryPageResultWithTotal, + SingleFormResult, +} from '../../app/modules/examples/examples.types' + +// pageNo: The page to render +// searchTerm: The term to search on +// agency: The agency to search on - this can be all agencies or the user's agency +// shouldGetTotalNumResults: Whether to return all the results or not +export type ExampleFormsQueryDto = { + pageNo: number + searchTerm?: string + agency?: string + shouldGetTotalNumResults?: boolean +} + +export type ExampleFormsResult = QueryPageResult | QueryPageResultWithTotal +// NOTE: Renaming for clarity that this type refers to an example +export type ExampleSingleFormResult = SingleFormResult diff --git a/src/types/api/index.ts b/src/types/api/index.ts index 977a0c0c32..20d19312a1 100644 --- a/src/types/api/index.ts +++ b/src/types/api/index.ts @@ -1,2 +1,4 @@ export * from './core' export * from './form' +export * from './billing' +export * from './examples'