From 0c082d54f1aa272580d3f3482f250652c23469ec Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Fri, 4 Jun 2021 12:17:36 +0800 Subject: [PATCH 01/34] refactor: convert response classes to typescript and change response to abstract class --- .../ArrayAnswerResponse.class.js | 11 ------- .../ArrayAnswerResponse.class.ts | 17 +++++++++++ .../csv-response-classes/Response.class.js | 21 -------------- .../csv-response-classes/Response.class.ts | 29 +++++++++++++++++++ .../SingleAnswerResponse.class.js | 11 ------- .../SingleAnswerResponse.class.ts | 17 +++++++++++ .../TableResponse.class.js | 15 ---------- .../TableResponse.class.ts | 21 ++++++++++++++ .../helpers/csv-response-classes/index.js | 3 -- .../helpers/csv-response-classes/index.ts | 4 +++ src/types/response/index.ts | 20 +++++++++++++ 11 files changed, 108 insertions(+), 61 deletions(-) delete mode 100644 src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.js create mode 100644 src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.ts delete mode 100644 src/public/modules/forms/helpers/csv-response-classes/Response.class.js create mode 100644 src/public/modules/forms/helpers/csv-response-classes/Response.class.ts delete mode 100644 src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.js create mode 100644 src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.ts delete mode 100644 src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.js create mode 100644 src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.ts delete mode 100644 src/public/modules/forms/helpers/csv-response-classes/index.js create mode 100644 src/public/modules/forms/helpers/csv-response-classes/index.ts diff --git a/src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.js b/src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.js deleted file mode 100644 index 5b06db10ad..0000000000 --- a/src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.js +++ /dev/null @@ -1,11 +0,0 @@ -const Response = require('./Response.class') - -module.exports = class ArrayAnswerResponse extends Response { - getAnswer() { - return this._data.answerArray.join(';') - } - - get numCols() { - return 1 - } -} diff --git a/src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.ts b/src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.ts new file mode 100644 index 0000000000..e438775127 --- /dev/null +++ b/src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.ts @@ -0,0 +1,17 @@ +import { SingleDimResponse } from '../../../../../types/response' + +import { Response } from './Response.class' + +export class ArrayAnswerResponse extends Response { + constructor(responseData: SingleDimResponse) { + super(responseData) + } + + getAnswer(): string { + return this._data.answerArray.join(';') + } + + get numCols(): number { + return 1 + } +} diff --git a/src/public/modules/forms/helpers/csv-response-classes/Response.class.js b/src/public/modules/forms/helpers/csv-response-classes/Response.class.js deleted file mode 100644 index c10cb850f4..0000000000 --- a/src/public/modules/forms/helpers/csv-response-classes/Response.class.js +++ /dev/null @@ -1,21 +0,0 @@ -module.exports = class Response { - constructor(responseData) { - this._data = responseData - } - - get id() { - return this._data._id - } - - /** - * Gets the CSV header. - * @returns {string} - */ - get question() { - return this._data.question - } - - get isHeader() { - return this._data.isHeader - } -} diff --git a/src/public/modules/forms/helpers/csv-response-classes/Response.class.ts b/src/public/modules/forms/helpers/csv-response-classes/Response.class.ts new file mode 100644 index 0000000000..9c88badeb5 --- /dev/null +++ b/src/public/modules/forms/helpers/csv-response-classes/Response.class.ts @@ -0,0 +1,29 @@ +import { DisplayedResponse } from '../../../../../types/response' + +export abstract class Response { + _data: DisplayedResponse + + constructor(responseData: DisplayedResponse) { + this._data = responseData + } + + get id(): string { + return this._data._id + } + + /** + * Gets the CSV header. + * @returns {string} + */ + get question(): string { + return this._data.question + } + + get isHeader(): boolean { + return this._data.isHeader ?? false + } + + abstract get numCols(): number + + abstract getAnswer(colIndex?: number): string +} diff --git a/src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.js b/src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.js deleted file mode 100644 index fecb3d1d7e..0000000000 --- a/src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.js +++ /dev/null @@ -1,11 +0,0 @@ -const Response = require('./Response.class') - -module.exports = class SingleAnswerResponse extends Response { - getAnswer() { - return this._data.answer - } - - get numCols() { - return 1 - } -} diff --git a/src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.ts b/src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.ts new file mode 100644 index 0000000000..36faf971ec --- /dev/null +++ b/src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.ts @@ -0,0 +1,17 @@ +import { SingleDimResponse } from '../../../../../types/response' + +import { Response } from './Response.class' + +export class SingleAnswerResponse extends Response { + constructor(responseData: SingleDimResponse) { + super(responseData) + } + + getAnswer(): string { + return this._data.answer ?? '' + } + + get numCols(): number { + return 1 + } +} diff --git a/src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.js b/src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.js deleted file mode 100644 index 4352169a40..0000000000 --- a/src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.js +++ /dev/null @@ -1,15 +0,0 @@ -const Response = require('./Response.class') - -module.exports = class TableResponse extends Response { - getAnswer(colIndex) { - // Leave cell empty if number of rows is fewer than the index - if (colIndex >= this._data.answerArray.length) { - return '' - } - return this._data.answerArray[colIndex].join(';') - } - - get numCols() { - return this._data.answerArray.length - } -} diff --git a/src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.ts b/src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.ts new file mode 100644 index 0000000000..208fd2ca10 --- /dev/null +++ b/src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.ts @@ -0,0 +1,21 @@ +import { TwoDimResponse } from '../../../../../types/response' + +import { Response } from './Response.class' + +export class TableResponse extends Response { + constructor(responseData: TwoDimResponse) { + super(responseData) + } + + getAnswer(colIndex: number): string { + // Leave cell empty if number of rows is fewer than the index + if (colIndex >= this._data.answerArray.length) { + return '' + } + return (this._data as TwoDimResponse).answerArray[colIndex].join(';') + } + + get numCols(): number { + return this._data.answerArray.length + } +} diff --git a/src/public/modules/forms/helpers/csv-response-classes/index.js b/src/public/modules/forms/helpers/csv-response-classes/index.js deleted file mode 100644 index 837b2c4d38..0000000000 --- a/src/public/modules/forms/helpers/csv-response-classes/index.js +++ /dev/null @@ -1,3 +0,0 @@ -module.exports.SingleAnswerResponse = require('./SingleAnswerResponse.class') -module.exports.ArrayAnswerResponse = require('./ArrayAnswerResponse.class') -module.exports.TableResponse = require('./TableResponse.class') diff --git a/src/public/modules/forms/helpers/csv-response-classes/index.ts b/src/public/modules/forms/helpers/csv-response-classes/index.ts new file mode 100644 index 0000000000..b3ac7f159c --- /dev/null +++ b/src/public/modules/forms/helpers/csv-response-classes/index.ts @@ -0,0 +1,4 @@ +export { SingleAnswerResponse } from './SingleAnswerResponse.class' +export { ArrayAnswerResponse } from './ArrayAnswerResponse.class' +export { TableResponse } from './TableResponse.class' +export { Response } from './Response.class' diff --git a/src/types/response/index.ts b/src/types/response/index.ts index 06c9e6ceb6..6e019c6d97 100644 --- a/src/types/response/index.ts +++ b/src/types/response/index.ts @@ -56,3 +56,23 @@ export interface IClientEncryptSubmission extends IClientSubmission { encryptedContent: string version: number } + +export type DisplayedResponse = { + _id: string + question: string + answer?: string + answerArray: AnswerArray + fieldType: string + isHeader?: boolean +} + +type AnswerArray = string[] | string[][] + +export interface SingleDimResponse + extends Omit { + answerArray: Extract +} + +export interface TwoDimResponse extends Omit { + answerArray: Extract +} From 3d8c1d2cda25c123cdb668235997001bec57ffcb Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Fri, 4 Jun 2021 12:20:49 +0800 Subject: [PATCH 02/34] refactor: rearrange types --- .../csv-response-classes/ArrayAnswerResponse.class.ts | 6 +++++- .../SingleAnswerResponse.class.ts | 4 ++-- .../csv-response-classes/TableResponse.class.ts | 6 +++++- src/types/response/index.ts | 11 +---------- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.ts b/src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.ts index e438775127..f1960fd19b 100644 --- a/src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.ts +++ b/src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.ts @@ -1,7 +1,11 @@ -import { SingleDimResponse } from '../../../../../types/response' +import { AnswerArray, DisplayedResponse } from '../../../../../types/response' import { Response } from './Response.class' +interface SingleDimResponse extends Omit { + answerArray: Extract +} + export class ArrayAnswerResponse extends Response { constructor(responseData: SingleDimResponse) { super(responseData) diff --git a/src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.ts b/src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.ts index 36faf971ec..7fcd524881 100644 --- a/src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.ts +++ b/src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.ts @@ -1,9 +1,9 @@ -import { SingleDimResponse } from '../../../../../types/response' +import { DisplayedResponse } from '../../../../../types/response' import { Response } from './Response.class' export class SingleAnswerResponse extends Response { - constructor(responseData: SingleDimResponse) { + constructor(responseData: DisplayedResponse) { super(responseData) } diff --git a/src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.ts b/src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.ts index 208fd2ca10..e4d1c5379b 100644 --- a/src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.ts +++ b/src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.ts @@ -1,7 +1,11 @@ -import { TwoDimResponse } from '../../../../../types/response' +import { AnswerArray, DisplayedResponse } from '../../../../../types/response' import { Response } from './Response.class' +export interface TwoDimResponse extends Omit { + answerArray: Extract +} + export class TableResponse extends Response { constructor(responseData: TwoDimResponse) { super(responseData) diff --git a/src/types/response/index.ts b/src/types/response/index.ts index 6e019c6d97..48af311b60 100644 --- a/src/types/response/index.ts +++ b/src/types/response/index.ts @@ -66,13 +66,4 @@ export type DisplayedResponse = { isHeader?: boolean } -type AnswerArray = string[] | string[][] - -export interface SingleDimResponse - extends Omit { - answerArray: Extract -} - -export interface TwoDimResponse extends Omit { - answerArray: Extract -} +export type AnswerArray = string[] | string[][] // answer array can take in string arrays of different dimensions From bf5cb22bd0b8c94e6b952a2f4d74e0a998ec3396 Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Fri, 4 Jun 2021 12:21:27 +0800 Subject: [PATCH 03/34] refactor: convert CsvMergedHeadersGenerator to typescript --- ...erator.js => CsvMergedHeadersGenerator.ts} | 91 ++++++++++++------- 1 file changed, 59 insertions(+), 32 deletions(-) rename src/public/modules/forms/helpers/{CsvMergedHeadersGenerator.js => CsvMergedHeadersGenerator.ts} (68%) diff --git a/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.js b/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts similarity index 68% rename from src/public/modules/forms/helpers/CsvMergedHeadersGenerator.js rename to src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts index 9026cf3412..12198a0999 100644 --- a/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.js +++ b/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts @@ -1,21 +1,32 @@ -const moment = require('moment-timezone') -const keyBy = require('lodash/keyBy') -const { CsvGenerator } = require('./CsvGenerator') -const { getResponseInstance } = require('./response-factory') - -/** - * @typedef {{ - * _id: string, - * question: string, - * answer?: string, - * answerArray?: string[], - * fieldType: string, - * isHeader?: boolean, - * }} DisplayedResponse - */ +import { keyBy } from 'lodash' +import moment from 'moment-timezone' +import { Dictionary } from 'ts-essentials/dist/types' + +import { DisplayedResponse } from 'src/types/response' + +import { Response } from './csv-response-classes' +import { CsvGenerator } from './CsvGenerator' +import { getResponseInstance } from './response-factory' + +type UnprocessedRecord = { + created: string + submissionId: string + record: ReturnType +} class CsvMergedHeadersGenerator extends CsvGenerator { - constructor(expectedNumberOfRecords, numOfMetaDataRows) { + hasBeenProcessed: boolean + fieldIdToQuestion: Map< + string, + { + created: string + question: string + } + > + fieldIdToNumCols: Record + unprocessed: UnprocessedRecord[] + + constructor(expectedNumberOfRecords: number, numOfMetaDataRows: number) { super(expectedNumberOfRecords, numOfMetaDataRows) this.hasBeenProcessed = false @@ -38,7 +49,15 @@ class CsvMergedHeadersGenerator extends CsvGenerator { * @param {string} decryptedContent.created * @param {string} decryptedContent.submissionId */ - addRecord({ record, created, submissionId }) { + addRecord({ + record, + created, + submissionId, + }: { + record: DisplayedResponse[] + created: string + submissionId: string + }) { // First pass, create object with { [fieldId]: question } from // decryptedContent to get all the questions. const fieldRecords = record.map((content) => { @@ -80,9 +99,14 @@ class CsvMergedHeadersGenerator extends CsvGenerator { * Extracts the string representation from an unprocessed record. * @param {Object} unprocessedRecord * @param {string} fieldId + * @param {number} colIndex * @returns {string} */ - _extractAnswer(unprocessedRecord, fieldId, colIndex) { + _extractAnswer( + unprocessedRecord: Dictionary, + fieldId: string, + colIndex: number, + ) { const fieldRecord = unprocessedRecord[fieldId] if (!fieldRecord) return '' return fieldRecord.getAnswer(colIndex) @@ -97,7 +121,7 @@ class CsvMergedHeadersGenerator extends CsvGenerator { if (this.hasBeenProcessed) return // Create a header row in CSV using the fieldIdToQuestion map. - let headers = ['Reference number', 'Timestamp'] + const headers = ['Reference number', 'Timestamp'] this.fieldIdToQuestion.forEach((value, fieldId) => { for (let i = 0; i < this.fieldIdToNumCols[fieldId]; i++) { headers.push(value.question) @@ -108,17 +132,20 @@ class CsvMergedHeadersGenerator extends CsvGenerator { // Craft a new csv row for each unprocessed record // O(qn), where q = number of unique questions, n = number of submissions. this.unprocessed.forEach((up) => { - let createdAt = moment(up.created).tz('Asia/Singapore') - createdAt = createdAt.isValid() + const createdAt = moment(up.created).tz('Asia/Singapore') + const formattedDate = createdAt.isValid() ? createdAt.format('DD MMM YYYY hh:mm:ss A') - : createdAt - let row = [up.submissionId, createdAt] - for (let [fieldId] of this.fieldIdToQuestion) { - const numCols = this.fieldIdToNumCols[fieldId] - for (let colIndex = 0; colIndex < numCols; colIndex++) { - row.push(this._extractAnswer(up.record, fieldId, colIndex)) - } - } + : createdAt.toString() // just convert to string if given date is not valid + const row = [up.submissionId, formattedDate] + + this.fieldIdToQuestion.forEach( + (_question: { created: string; question: string }, fieldId: string) => { + const numCols = this.fieldIdToNumCols[fieldId] + for (let colIndex = 0; colIndex < numCols; colIndex++) { + row.push(this._extractAnswer(up.record, fieldId, colIndex)) + } + }, + ) this.addLine(row) }) @@ -129,8 +156,8 @@ class CsvMergedHeadersGenerator extends CsvGenerator { * Add meta-data as first three rows of the CSV. If there is already meta-data * added, it will be replaced by the latest counts. */ - addMetaDataFromSubmission(errorCount, unverifiedCount) { - let metaDataRows = [ + addMetaDataFromSubmission(errorCount: number, unverifiedCount: number) { + const metaDataRows = [ ['Expected total responses', this.expectedNumberOfRecords], ['Success count', this.length()], ['Error count', errorCount], @@ -144,7 +171,7 @@ class CsvMergedHeadersGenerator extends CsvGenerator { * Main method to call to retrieve a downloadable csv. * @param {string} filename */ - downloadCsv(filename) { + downloadCsv(filename: string) { this.process() this.triggerFileDownload(filename) } From 5483a87b49c3b83f8c98003c402811e3067b60ac Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Mon, 7 Jun 2021 10:35:48 +0800 Subject: [PATCH 04/34] refactor: edit export and import statements for CsvMergedHeadersGenerator --- .../helpers/CsvMergedHeadersGenerator.ts | 19 ++++++++----------- .../helpers/CsvMergedHeadersGenerator.test.js | 2 +- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts b/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts index 12198a0999..967bdaebad 100644 --- a/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts +++ b/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts @@ -14,7 +14,7 @@ type UnprocessedRecord = { record: ReturnType } -class CsvMergedHeadersGenerator extends CsvGenerator { +export class CsvMergedHeadersGenerator extends CsvGenerator { hasBeenProcessed: boolean fieldIdToQuestion: Map< string, @@ -38,12 +38,12 @@ class CsvMergedHeadersGenerator extends CsvGenerator { /** * Returns current length of CSV file excluding header and meta-data */ - length() { + length(): number { return this.unprocessed.length } /** - * + * Adds an UnprocessedRecord to this.unprocessed * @param {Object} decryptedContent * @param {DisplayedResponse[]} decryptedContent.record * @param {string} decryptedContent.created @@ -57,14 +57,13 @@ class CsvMergedHeadersGenerator extends CsvGenerator { record: DisplayedResponse[] created: string submissionId: string - }) { + }): void { // First pass, create object with { [fieldId]: question } from // decryptedContent to get all the questions. const fieldRecords = record.map((content) => { const fieldRecord = getResponseInstance(content) if (!fieldRecord.isHeader) { const currentMapping = this.fieldIdToQuestion.get(fieldRecord.id) - // Only set new mapping if it does not exist or this record is a later // submission. // Might need to differentiate the question headers if we allow @@ -106,7 +105,7 @@ class CsvMergedHeadersGenerator extends CsvGenerator { unprocessedRecord: Dictionary, fieldId: string, colIndex: number, - ) { + ): string { const fieldRecord = unprocessedRecord[fieldId] if (!fieldRecord) return '' return fieldRecord.getAnswer(colIndex) @@ -117,7 +116,7 @@ class CsvMergedHeadersGenerator extends CsvGenerator { * assigning each answer to their respective locations in each response row in * the csv data. */ - process() { + process(): void { if (this.hasBeenProcessed) return // Create a header row in CSV using the fieldIdToQuestion map. @@ -156,7 +155,7 @@ class CsvMergedHeadersGenerator extends CsvGenerator { * Add meta-data as first three rows of the CSV. If there is already meta-data * added, it will be replaced by the latest counts. */ - addMetaDataFromSubmission(errorCount: number, unverifiedCount: number) { + addMetaDataFromSubmission(errorCount: number, unverifiedCount: number): void { const metaDataRows = [ ['Expected total responses', this.expectedNumberOfRecords], ['Success count', this.length()], @@ -171,10 +170,8 @@ class CsvMergedHeadersGenerator extends CsvGenerator { * Main method to call to retrieve a downloadable csv. * @param {string} filename */ - downloadCsv(filename: string) { + downloadCsv(filename: string): void { this.process() this.triggerFileDownload(filename) } } - -module.exports = CsvMergedHeadersGenerator diff --git a/tests/unit/frontend/forms/helpers/CsvMergedHeadersGenerator.test.js b/tests/unit/frontend/forms/helpers/CsvMergedHeadersGenerator.test.js index ab72ebb532..0f43bff7f8 100644 --- a/tests/unit/frontend/forms/helpers/CsvMergedHeadersGenerator.test.js +++ b/tests/unit/frontend/forms/helpers/CsvMergedHeadersGenerator.test.js @@ -1,7 +1,7 @@ import { stringify } from 'csv-string' import moment from 'moment-timezone' -import CsvMergedHeadersGenerator from '../../../../../src/public/modules/forms/helpers/CsvMergedHeadersGenerator' +import { CsvMergedHeadersGenerator } from '../../../../../src/public/modules/forms/helpers/CsvMergedHeadersGenerator' const UTF8_BYTE_ORDER_MARK = '\uFEFF' const BOM_LENGTH = 1 From 2c0168c2d07b0f8ec987ddb680df1723470f9848 Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Mon, 7 Jun 2021 12:18:04 +0800 Subject: [PATCH 05/34] fix: edit import statement of generator in submissions.client.factory --- .../modules/forms/services/submissions.client.factory.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/public/modules/forms/services/submissions.client.factory.js b/src/public/modules/forms/services/submissions.client.factory.js index d1ed0e21ea..b71b3a9247 100644 --- a/src/public/modules/forms/services/submissions.client.factory.js +++ b/src/public/modules/forms/services/submissions.client.factory.js @@ -1,6 +1,8 @@ 'use strict' -const CsvMHGenerator = require('../helpers/CsvMergedHeadersGenerator') +const { + CsvMergedHeadersGenerator: CsvMHGenerator, +} = require('../helpers/CsvMergedHeadersGenerator') const DecryptionWorker = require('../helpers/decryption.worker.js') const { fixParamsToUrl, triggerFileDownload } = require('../helpers/util') const ndjsonStream = require('../helpers/ndjsonStream') From 2573fead3e2cd19ff06a9e98e6885105d2bddaec Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Tue, 8 Jun 2021 09:58:35 +0800 Subject: [PATCH 06/34] refactor: edit response types --- .../ArrayAnswerResponse.class.ts | 11 +++++++---- .../helpers/csv-response-classes/Response.class.ts | 6 +++--- .../SingleAnswerResponse.class.ts | 12 +++++++++--- .../csv-response-classes/TableResponse.class.ts | 13 ++++++++----- src/types/response/index.ts | 6 +----- 5 files changed, 28 insertions(+), 20 deletions(-) diff --git a/src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.ts b/src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.ts index f1960fd19b..fb50983517 100644 --- a/src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.ts +++ b/src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.ts @@ -1,14 +1,17 @@ -import { AnswerArray, DisplayedResponse } from '../../../../../types/response' +import { DisplayedResponseWithoutAnswer } from '../../../../../types/response' import { Response } from './Response.class' -interface SingleDimResponse extends Omit { - answerArray: Extract +interface ArrayResponse extends DisplayedResponseWithoutAnswer { + answerArray: string[] } export class ArrayAnswerResponse extends Response { - constructor(responseData: SingleDimResponse) { + _data: ArrayResponse + + constructor(responseData: ArrayResponse) { super(responseData) + this._data = responseData } getAnswer(): string { diff --git a/src/public/modules/forms/helpers/csv-response-classes/Response.class.ts b/src/public/modules/forms/helpers/csv-response-classes/Response.class.ts index 9c88badeb5..46726bf66a 100644 --- a/src/public/modules/forms/helpers/csv-response-classes/Response.class.ts +++ b/src/public/modules/forms/helpers/csv-response-classes/Response.class.ts @@ -1,9 +1,9 @@ -import { DisplayedResponse } from '../../../../../types/response' +import { DisplayedResponseWithoutAnswer } from '../../../../../types/response' export abstract class Response { - _data: DisplayedResponse + _data: DisplayedResponseWithoutAnswer - constructor(responseData: DisplayedResponse) { + constructor(responseData: DisplayedResponseWithoutAnswer) { this._data = responseData } diff --git a/src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.ts b/src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.ts index 7fcd524881..f2fed3188a 100644 --- a/src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.ts +++ b/src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.ts @@ -1,14 +1,20 @@ -import { DisplayedResponse } from '../../../../../types/response' +import { DisplayedResponseWithoutAnswer } from '../../../../../types/response' import { Response } from './Response.class' +interface SingleResponse extends DisplayedResponseWithoutAnswer { + answer: string +} export class SingleAnswerResponse extends Response { - constructor(responseData: DisplayedResponse) { + _data: SingleResponse + + constructor(responseData: SingleResponse) { super(responseData) + this._data = responseData } getAnswer(): string { - return this._data.answer ?? '' + return this._data.answer } get numCols(): number { diff --git a/src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.ts b/src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.ts index e4d1c5379b..5f91ddd7a1 100644 --- a/src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.ts +++ b/src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.ts @@ -1,14 +1,17 @@ -import { AnswerArray, DisplayedResponse } from '../../../../../types/response' +import { DisplayedResponseWithoutAnswer } from '../../../../../types/response' import { Response } from './Response.class' -export interface TwoDimResponse extends Omit { - answerArray: Extract +interface NestedResponse extends DisplayedResponseWithoutAnswer { + answerArray: string[][] } export class TableResponse extends Response { - constructor(responseData: TwoDimResponse) { + _data: NestedResponse + + constructor(responseData: NestedResponse) { super(responseData) + this._data = responseData } getAnswer(colIndex: number): string { @@ -16,7 +19,7 @@ export class TableResponse extends Response { if (colIndex >= this._data.answerArray.length) { return '' } - return (this._data as TwoDimResponse).answerArray[colIndex].join(';') + return this._data.answerArray[colIndex].join(';') } get numCols(): number { diff --git a/src/types/response/index.ts b/src/types/response/index.ts index 48af311b60..3b2980a2ff 100644 --- a/src/types/response/index.ts +++ b/src/types/response/index.ts @@ -57,13 +57,9 @@ export interface IClientEncryptSubmission extends IClientSubmission { version: number } -export type DisplayedResponse = { +export type DisplayedResponseWithoutAnswer = { _id: string question: string - answer?: string - answerArray: AnswerArray fieldType: string isHeader?: boolean } - -export type AnswerArray = string[] | string[][] // answer array can take in string arrays of different dimensions From cda7b2ed045ec9052adca4cbdf04cba69388ffe0 Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Tue, 8 Jun 2021 09:59:28 +0800 Subject: [PATCH 07/34] refactor: remove typing in jsdocs and edit types --- .../helpers/CsvMergedHeadersGenerator.ts | 43 +++++++++---------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts b/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts index 967bdaebad..71d2ce7dd3 100644 --- a/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts +++ b/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts @@ -1,8 +1,7 @@ -import { keyBy } from 'lodash' +import keyBy from 'lodash/keyBy' import moment from 'moment-timezone' -import { Dictionary } from 'ts-essentials/dist/types' -import { DisplayedResponse } from 'src/types/response' +import { DisplayedResponseWithoutAnswer } from '../../../../types/response' import { Response } from './csv-response-classes' import { CsvGenerator } from './CsvGenerator' @@ -11,7 +10,7 @@ import { getResponseInstance } from './response-factory' type UnprocessedRecord = { created: string submissionId: string - record: ReturnType + record: { [fieldId: string]: Response } } export class CsvMergedHeadersGenerator extends CsvGenerator { @@ -44,17 +43,17 @@ export class CsvMergedHeadersGenerator extends CsvGenerator { /** * Adds an UnprocessedRecord to this.unprocessed - * @param {Object} decryptedContent - * @param {DisplayedResponse[]} decryptedContent.record - * @param {string} decryptedContent.created - * @param {string} decryptedContent.submissionId + * @param decryptedContent + * @param decryptedContent.record + * @param decryptedContent.created + * @param decryptedContent.submissionId */ addRecord({ record, created, submissionId, }: { - record: DisplayedResponse[] + record: DisplayedResponseWithoutAnswer[] created: string submissionId: string }): void { @@ -96,13 +95,13 @@ export class CsvMergedHeadersGenerator extends CsvGenerator { /** * Extracts the string representation from an unprocessed record. - * @param {Object} unprocessedRecord - * @param {string} fieldId - * @param {number} colIndex - * @returns {string} + * @param unprocessedRecord + * @param fieldId + * @param colIndex + * @returns string representation of unprocessed record */ _extractAnswer( - unprocessedRecord: Dictionary, + unprocessedRecord: { [fieldId: string]: Response }, fieldId: string, colIndex: number, ): string { @@ -137,14 +136,12 @@ export class CsvMergedHeadersGenerator extends CsvGenerator { : createdAt.toString() // just convert to string if given date is not valid const row = [up.submissionId, formattedDate] - this.fieldIdToQuestion.forEach( - (_question: { created: string; question: string }, fieldId: string) => { - const numCols = this.fieldIdToNumCols[fieldId] - for (let colIndex = 0; colIndex < numCols; colIndex++) { - row.push(this._extractAnswer(up.record, fieldId, colIndex)) - } - }, - ) + this.fieldIdToQuestion.forEach((_question, fieldId) => { + const numCols = this.fieldIdToNumCols[fieldId] + for (let colIndex = 0; colIndex < numCols; colIndex++) { + row.push(this._extractAnswer(up.record, fieldId, colIndex)) + } + }) this.addLine(row) }) @@ -168,7 +165,7 @@ export class CsvMergedHeadersGenerator extends CsvGenerator { /** * Main method to call to retrieve a downloadable csv. - * @param {string} filename + * @param filename name of csv file */ downloadCsv(filename: string): void { this.process() From d11f18f0cf6c064a6babf7ae6f96b639a0c06554 Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Tue, 8 Jun 2021 11:19:20 +0800 Subject: [PATCH 08/34] refactor: export response subclasses input types --- .../csv-response-classes/ArrayAnswerResponse.class.ts | 6 +----- .../SingleAnswerResponse.class.ts | 5 +---- .../csv-response-classes/TableResponse.class.ts | 6 +----- src/types/response/index.ts | 11 +++++++++++ 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.ts b/src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.ts index fb50983517..63e5dad4a6 100644 --- a/src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.ts +++ b/src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.ts @@ -1,11 +1,7 @@ -import { DisplayedResponseWithoutAnswer } from '../../../../../types/response' +import { ArrayResponse } from '../../../../../types/response' import { Response } from './Response.class' -interface ArrayResponse extends DisplayedResponseWithoutAnswer { - answerArray: string[] -} - export class ArrayAnswerResponse extends Response { _data: ArrayResponse diff --git a/src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.ts b/src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.ts index f2fed3188a..1a681c6ce1 100644 --- a/src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.ts +++ b/src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.ts @@ -1,10 +1,7 @@ -import { DisplayedResponseWithoutAnswer } from '../../../../../types/response' +import { SingleResponse } from '../../../../../types/response' import { Response } from './Response.class' -interface SingleResponse extends DisplayedResponseWithoutAnswer { - answer: string -} export class SingleAnswerResponse extends Response { _data: SingleResponse diff --git a/src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.ts b/src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.ts index 5f91ddd7a1..e94684ab2e 100644 --- a/src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.ts +++ b/src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.ts @@ -1,11 +1,7 @@ -import { DisplayedResponseWithoutAnswer } from '../../../../../types/response' +import { NestedResponse } from '../../../../../types/response' import { Response } from './Response.class' -interface NestedResponse extends DisplayedResponseWithoutAnswer { - answerArray: string[][] -} - export class TableResponse extends Response { _data: NestedResponse diff --git a/src/types/response/index.ts b/src/types/response/index.ts index 3b2980a2ff..dff5a6850f 100644 --- a/src/types/response/index.ts +++ b/src/types/response/index.ts @@ -63,3 +63,14 @@ export type DisplayedResponseWithoutAnswer = { fieldType: string isHeader?: boolean } + +export interface ArrayResponse extends DisplayedResponseWithoutAnswer { + answerArray: string[] +} + +export interface NestedResponse extends DisplayedResponseWithoutAnswer { + answerArray: string[][] +} +export interface SingleResponse extends DisplayedResponseWithoutAnswer { + answer: string +} From fdc9549b8d540f6d629f967aac56663f17ceaed5 Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Tue, 8 Jun 2021 11:20:10 +0800 Subject: [PATCH 09/34] refactor: add ErrorResponse as default for response types --- .../ErrorResponse.class.ts | 17 ++++++ .../helpers/csv-response-classes/index.ts | 1 + .../modules/forms/helpers/response-factory.js | 18 ------- .../modules/forms/helpers/response-factory.ts | 54 +++++++++++++++++++ 4 files changed, 72 insertions(+), 18 deletions(-) create mode 100644 src/public/modules/forms/helpers/csv-response-classes/ErrorResponse.class.ts delete mode 100644 src/public/modules/forms/helpers/response-factory.js create mode 100644 src/public/modules/forms/helpers/response-factory.ts diff --git a/src/public/modules/forms/helpers/csv-response-classes/ErrorResponse.class.ts b/src/public/modules/forms/helpers/csv-response-classes/ErrorResponse.class.ts new file mode 100644 index 0000000000..3c694c37a5 --- /dev/null +++ b/src/public/modules/forms/helpers/csv-response-classes/ErrorResponse.class.ts @@ -0,0 +1,17 @@ +import { DisplayedResponseWithoutAnswer } from 'src/types' + +import { Response } from './Response.class' + +export class ErrorResponse extends Response { + constructor(responseData: DisplayedResponseWithoutAnswer) { + super(responseData) + } + + getAnswer(): string { + return 'error' + } + + get numCols(): number { + return 1 + } +} diff --git a/src/public/modules/forms/helpers/csv-response-classes/index.ts b/src/public/modules/forms/helpers/csv-response-classes/index.ts index b3ac7f159c..b9b89b35ed 100644 --- a/src/public/modules/forms/helpers/csv-response-classes/index.ts +++ b/src/public/modules/forms/helpers/csv-response-classes/index.ts @@ -1,4 +1,5 @@ export { SingleAnswerResponse } from './SingleAnswerResponse.class' export { ArrayAnswerResponse } from './ArrayAnswerResponse.class' export { TableResponse } from './TableResponse.class' +export { ErrorResponse } from './ErrorResponse.class' export { Response } from './Response.class' diff --git a/src/public/modules/forms/helpers/response-factory.js b/src/public/modules/forms/helpers/response-factory.js deleted file mode 100644 index e78ad6e820..0000000000 --- a/src/public/modules/forms/helpers/response-factory.js +++ /dev/null @@ -1,18 +0,0 @@ -const { - SingleAnswerResponse, - ArrayAnswerResponse, - TableResponse, -} = require('./csv-response-classes') - -const getResponseInstance = (fieldRecordData) => { - switch (fieldRecordData.fieldType) { - case 'table': - return new TableResponse(fieldRecordData) - case 'checkbox': - return new ArrayAnswerResponse(fieldRecordData) - default: - return new SingleAnswerResponse(fieldRecordData) - } -} - -module.exports = { getResponseInstance } diff --git a/src/public/modules/forms/helpers/response-factory.ts b/src/public/modules/forms/helpers/response-factory.ts new file mode 100644 index 0000000000..90649b00c6 --- /dev/null +++ b/src/public/modules/forms/helpers/response-factory.ts @@ -0,0 +1,54 @@ +import { + ArrayResponse, + DisplayedResponseWithoutAnswer, + NestedResponse, + SingleResponse, +} from '../../../../types/response' + +import { + ArrayAnswerResponse, + ErrorResponse, + Response, + SingleAnswerResponse, + TableResponse, +} from './csv-response-classes' + +export const getResponseInstance = ( + fieldRecordData: DisplayedResponseWithoutAnswer, +): Response => { + switch (fieldRecordData.fieldType) { + case 'table': + if (isNestedResponse(fieldRecordData)) { + return new TableResponse(fieldRecordData) + } + break + case 'checkbox': + if (isArrayResponse(fieldRecordData)) { + return new ArrayAnswerResponse(fieldRecordData) + } + break + default: + if (isSingleResponse(fieldRecordData)) { + return new SingleAnswerResponse(fieldRecordData) + } + break + } + return new ErrorResponse(fieldRecordData) +} + +function isNestedResponse( + response: DisplayedResponseWithoutAnswer, +): response is NestedResponse { + return 'answerArray' in response +} +function isArrayResponse( + response: DisplayedResponseWithoutAnswer, +): response is ArrayResponse { + return 'answerArray' in response +} + +function isSingleResponse( + response: DisplayedResponseWithoutAnswer, +): response is SingleResponse { + return 'answer' in response +} From 1f0659272c555fd22a5641ce36d2bc56e6d0d523 Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Wed, 9 Jun 2021 12:52:34 +0800 Subject: [PATCH 10/34] refactor: shift hasProps to shared --- src/app/modules/bounce/bounce.service.ts | 2 +- src/app/modules/myinfo/myinfo.util.ts | 2 +- src/app/modules/spcp/spcp.util.ts | 2 +- src/{app/utils => shared/util}/has-prop.ts | 0 4 files changed, 3 insertions(+), 3 deletions(-) rename src/{app/utils => shared/util}/has-prop.ts (100%) diff --git a/src/app/modules/bounce/bounce.service.ts b/src/app/modules/bounce/bounce.service.ts index fa31c17aba..e78b4dbc44 100644 --- a/src/app/modules/bounce/bounce.service.ts +++ b/src/app/modules/bounce/bounce.service.ts @@ -10,6 +10,7 @@ import { ResultAsync, } from 'neverthrow' +import { hasProp } from '../../../shared/util/has-prop' import { BounceType, IBounceSchema, @@ -25,7 +26,6 @@ import { EMAIL_HEADERS, EmailType } from '../../services/mail/mail.constants' import MailService from '../../services/mail/mail.service' import { SmsFactory } from '../../services/sms/sms.factory' import { transformMongoError } from '../../utils/handle-mongo-error' -import { hasProp } from '../../utils/has-prop' import { PossibleDatabaseError } from '../core/core.errors' import { getCollabEmailsWithPermission } from '../form/form.utils' import * as UserService from '../user/user.service' diff --git a/src/app/modules/myinfo/myinfo.util.ts b/src/app/modules/myinfo/myinfo.util.ts index 6f3febbaee..5c5a05b9dd 100644 --- a/src/app/modules/myinfo/myinfo.util.ts +++ b/src/app/modules/myinfo/myinfo.util.ts @@ -6,6 +6,7 @@ import { err, ok, Result } from 'neverthrow' import { v4 as uuidv4, validate as validateUUID } from 'uuid' import { types as myInfoTypes } from '../../../shared/resources/myinfo' +import { hasProp } from '../../../shared/util/has-prop' import { AuthType, BasicField, @@ -16,7 +17,6 @@ import { MapRouteError, } from '../../../types' import { createLoggerWithLabel } from '../../config/logger' -import { hasProp } from '../../utils/has-prop' import { DatabaseError, MissingFeatureError } from '../core/core.errors' import { AuthTypeMismatchError, diff --git a/src/app/modules/spcp/spcp.util.ts b/src/app/modules/spcp/spcp.util.ts index fde8713544..3aba24bbe6 100644 --- a/src/app/modules/spcp/spcp.util.ts +++ b/src/app/modules/spcp/spcp.util.ts @@ -3,6 +3,7 @@ import crypto from 'crypto' import { StatusCodes } from 'http-status-codes' import { err, ok, Result } from 'neverthrow' +import { hasProp } from '../../../shared/util/has-prop' import { AuthType, BasicField, @@ -11,7 +12,6 @@ import { SPCPFieldTitle, } from '../../../types' import { createLoggerWithLabel } from '../../config/logger' -import { hasProp } from '../../utils/has-prop' import { MissingFeatureError } from '../core/core.errors' import { AuthTypeMismatchError, diff --git a/src/app/utils/has-prop.ts b/src/shared/util/has-prop.ts similarity index 100% rename from src/app/utils/has-prop.ts rename to src/shared/util/has-prop.ts From 107e41f04c4346730ab727708808fba81ef2916b Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Wed, 9 Jun 2021 12:54:36 +0800 Subject: [PATCH 11/34] refactor: athrow error instead of returning ErrorResponse and edited typeguards --- .../helpers/CsvMergedHeadersGenerator.ts | 2 +- .../ErrorResponse.class.ts | 17 ------ .../helpers/csv-response-classes/index.ts | 1 - .../modules/forms/helpers/response-factory.ts | 57 ++++++++++--------- .../services/submissions.client.factory.js | 9 ++- 5 files changed, 38 insertions(+), 48 deletions(-) delete mode 100644 src/public/modules/forms/helpers/csv-response-classes/ErrorResponse.class.ts diff --git a/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts b/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts index ea0d214ee0..72b950ec3b 100644 --- a/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts +++ b/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts @@ -62,7 +62,7 @@ export class CsvMergedHeadersGenerator extends CsvGenerator { // First pass, create object with { [fieldId]: question } from // decryptedContent to get all the questions. const fieldRecords = record.map((content) => { - const fieldRecord = getResponseInstance(content) + const fieldRecord = getResponseInstance(content) // Could throw error, to be caught in submissions client factory if (!fieldRecord.isHeader) { const currentMapping = this.fieldIdToQuestion.get(fieldRecord.id) // Only set new mapping if it does not exist or this record is a later diff --git a/src/public/modules/forms/helpers/csv-response-classes/ErrorResponse.class.ts b/src/public/modules/forms/helpers/csv-response-classes/ErrorResponse.class.ts deleted file mode 100644 index 3c694c37a5..0000000000 --- a/src/public/modules/forms/helpers/csv-response-classes/ErrorResponse.class.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { DisplayedResponseWithoutAnswer } from 'src/types' - -import { Response } from './Response.class' - -export class ErrorResponse extends Response { - constructor(responseData: DisplayedResponseWithoutAnswer) { - super(responseData) - } - - getAnswer(): string { - return 'error' - } - - get numCols(): number { - return 1 - } -} diff --git a/src/public/modules/forms/helpers/csv-response-classes/index.ts b/src/public/modules/forms/helpers/csv-response-classes/index.ts index b9b89b35ed..b3ac7f159c 100644 --- a/src/public/modules/forms/helpers/csv-response-classes/index.ts +++ b/src/public/modules/forms/helpers/csv-response-classes/index.ts @@ -1,5 +1,4 @@ export { SingleAnswerResponse } from './SingleAnswerResponse.class' export { ArrayAnswerResponse } from './ArrayAnswerResponse.class' export { TableResponse } from './TableResponse.class' -export { ErrorResponse } from './ErrorResponse.class' export { Response } from './Response.class' diff --git a/src/public/modules/forms/helpers/response-factory.ts b/src/public/modules/forms/helpers/response-factory.ts index 90649b00c6..1b28611a7a 100644 --- a/src/public/modules/forms/helpers/response-factory.ts +++ b/src/public/modules/forms/helpers/response-factory.ts @@ -1,3 +1,5 @@ +import { hasProp } from 'src/shared/util/has-prop' + import { ArrayResponse, DisplayedResponseWithoutAnswer, @@ -7,7 +9,6 @@ import { import { ArrayAnswerResponse, - ErrorResponse, Response, SingleAnswerResponse, TableResponse, @@ -16,39 +17,41 @@ import { export const getResponseInstance = ( fieldRecordData: DisplayedResponseWithoutAnswer, ): Response => { - switch (fieldRecordData.fieldType) { - case 'table': - if (isNestedResponse(fieldRecordData)) { - return new TableResponse(fieldRecordData) - } - break - case 'checkbox': - if (isArrayResponse(fieldRecordData)) { - return new ArrayAnswerResponse(fieldRecordData) - } - break - default: - if (isSingleResponse(fieldRecordData)) { - return new SingleAnswerResponse(fieldRecordData) - } - break + if (isNestedResponse(fieldRecordData)) { + return new TableResponse(fieldRecordData) + } else if (isArrayResponse(fieldRecordData)) { + return new ArrayAnswerResponse(fieldRecordData) + } else if (isSingleResponse(fieldRecordData)) { + return new SingleAnswerResponse(fieldRecordData) + } else { + // eslint-disable-next-line typesafe/no-throw-sync-func + throw new Error('Response did not match any known type') // should be caught in submissions client factory } - return new ErrorResponse(fieldRecordData) } -function isNestedResponse( +const isNestedResponse = ( response: DisplayedResponseWithoutAnswer, -): response is NestedResponse { - return 'answerArray' in response +): response is NestedResponse => { + return ( + hasProp(response, 'answerArray') && + Array.isArray(response.answerArray) && + Array.isArray(response.answerArray[0]) && + typeof response.answerArray[0][0] === 'string' + ) } -function isArrayResponse( + +const isArrayResponse = ( response: DisplayedResponseWithoutAnswer, -): response is ArrayResponse { - return 'answerArray' in response +): response is ArrayResponse => { + return ( + hasProp(response, 'answerArray') && + Array.isArray(response.answerArray) && + typeof response.answerArray[0] === 'string' + ) } -function isSingleResponse( +const isSingleResponse = ( response: DisplayedResponseWithoutAnswer, -): response is SingleResponse { - return 'answer' in response +): response is SingleResponse => { + return hasProp(response, 'answer') && typeof response.answer === 'string' } diff --git a/src/public/modules/forms/services/submissions.client.factory.js b/src/public/modules/forms/services/submissions.client.factory.js index e804c97cf2..1b3eb410a4 100644 --- a/src/public/modules/forms/services/submissions.client.factory.js +++ b/src/public/modules/forms/services/submissions.client.factory.js @@ -181,8 +181,13 @@ function SubmissionsFactory($q, $http, $timeout, $window, GTag, FormSgSdk) { } if (csvRecord.submissionData) { - // accumulate dataset if it exists, since we may have status columns available - experimentalCsvGenerator.addRecord(csvRecord.submissionData) + try { + // accumulate dataset if it exists, since we may have status columns available + experimentalCsvGenerator.addRecord(csvRecord.submissionData) + } catch (error) { + errorCount++ + console.error('Error in getResponseInstance', error) + } } if (downloadAttachments && csvRecord.downloadBlob) { From 3dbe803ebc7621c62106fd6bc8bb1753f7d9a5f8 Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Wed, 9 Jun 2021 12:55:17 +0800 Subject: [PATCH 12/34] refactor: change interface to type --- src/types/response/index.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/types/response/index.ts b/src/types/response/index.ts index dff5a6850f..a9b75836d1 100644 --- a/src/types/response/index.ts +++ b/src/types/response/index.ts @@ -64,13 +64,13 @@ export type DisplayedResponseWithoutAnswer = { isHeader?: boolean } -export interface ArrayResponse extends DisplayedResponseWithoutAnswer { +export type ArrayResponse = DisplayedResponseWithoutAnswer & { answerArray: string[] } -export interface NestedResponse extends DisplayedResponseWithoutAnswer { +export type NestedResponse = DisplayedResponseWithoutAnswer & { answerArray: string[][] } -export interface SingleResponse extends DisplayedResponseWithoutAnswer { +export type SingleResponse = DisplayedResponseWithoutAnswer & { answer: string } From 2e54dd6c0aeefe34ea9cdbc6e1360a3e6b8d68cc Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Wed, 9 Jun 2021 14:58:22 +0800 Subject: [PATCH 13/34] fix: fix incorrect import statement --- src/public/modules/forms/helpers/response-factory.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/public/modules/forms/helpers/response-factory.ts b/src/public/modules/forms/helpers/response-factory.ts index 1b28611a7a..b533aea6bc 100644 --- a/src/public/modules/forms/helpers/response-factory.ts +++ b/src/public/modules/forms/helpers/response-factory.ts @@ -1,5 +1,4 @@ -import { hasProp } from 'src/shared/util/has-prop' - +import { hasProp } from '../../../../shared/util/has-prop' import { ArrayResponse, DisplayedResponseWithoutAnswer, From c99934f5f47f76eaa2cea6a9f24280cc091c9ab4 Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Wed, 9 Jun 2021 15:01:07 +0800 Subject: [PATCH 14/34] test: remove single header row test --- .../helpers/CsvMergedHeadersGenerator.test.js | 32 ------------------- 1 file changed, 32 deletions(-) diff --git a/tests/unit/frontend/forms/helpers/CsvMergedHeadersGenerator.test.js b/tests/unit/frontend/forms/helpers/CsvMergedHeadersGenerator.test.js index 0f43bff7f8..7b72aba76f 100644 --- a/tests/unit/frontend/forms/helpers/CsvMergedHeadersGenerator.test.js +++ b/tests/unit/frontend/forms/helpers/CsvMergedHeadersGenerator.test.js @@ -27,14 +27,6 @@ const generateRecord = (append, answerArray, fieldType) => { return generated } -const generateHeaderRow = (append) => { - return { - _id: `mock${append}`, - question: `mockQuestion${append}`, - fieldType: `mockFieldType${append}`, - isHeader: true, - } -} /** * Reshapes a mock record to match the expected shape in generator.unprocessed. @@ -173,30 +165,6 @@ describe('CsvMergedHeadersGenerator', () => { expect(Object.values(generator.fieldIdToNumCols)).toEqual([2]) }) - it('should handle adding of single header record', () => { - // Arrange - const mockDecryptedRecord = [generateHeaderRow(1)] - const mockRecord = { - record: mockDecryptedRecord, - created: mockCreatedEarly, - submissionId: 'mockSubmissionId', - } - expect(generator.unprocessed.length).toEqual(0) - - // Act - generator.addRecord(mockRecord) - - // Assert - // Generate new shape in unprocessed array - const expectedUnprocessed = [generateExpectedUnprocessed(mockRecord)] - - expect(generator.unprocessed.length).toEqual(1) - // Check shape - expect(generator.unprocessed).toEqual(expectedUnprocessed) - // Check headers - expect(generator.fieldIdToQuestion.size).toEqual(0) - }) - it('should override the question to the latest question', () => { // Arrange const expectedQuestionHeader = 'this should override the old question' From fdb303a292c81d45111822c039dc6475f91ad9a4 Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Wed, 9 Jun 2021 17:30:12 +0800 Subject: [PATCH 15/34] fix: fix bug caused by fieldtype guard and if else condition --- .../modules/forms/helpers/response-factory.ts | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/public/modules/forms/helpers/response-factory.ts b/src/public/modules/forms/helpers/response-factory.ts index b533aea6bc..a0e1d2157a 100644 --- a/src/public/modules/forms/helpers/response-factory.ts +++ b/src/public/modules/forms/helpers/response-factory.ts @@ -16,9 +16,16 @@ import { export const getResponseInstance = ( fieldRecordData: DisplayedResponseWithoutAnswer, ): Response => { - if (isNestedResponse(fieldRecordData)) { + console.log(fieldRecordData) + if ( + isNestedResponse(fieldRecordData) && + fieldRecordData.fieldType === 'table' + ) { return new TableResponse(fieldRecordData) - } else if (isArrayResponse(fieldRecordData)) { + } else if ( + isArrayResponse(fieldRecordData) && + fieldRecordData.fieldType === 'checkbox' + ) { return new ArrayAnswerResponse(fieldRecordData) } else if (isSingleResponse(fieldRecordData)) { return new SingleAnswerResponse(fieldRecordData) @@ -34,8 +41,10 @@ const isNestedResponse = ( return ( hasProp(response, 'answerArray') && Array.isArray(response.answerArray) && - Array.isArray(response.answerArray[0]) && - typeof response.answerArray[0][0] === 'string' + response.answerArray.every((value) => Array.isArray(value)) && // or has at least one element that is an array + response.answerArray.every((arr) => + arr.every((value: unknown) => typeof value === 'string'), + ) ) } @@ -45,7 +54,7 @@ const isArrayResponse = ( return ( hasProp(response, 'answerArray') && Array.isArray(response.answerArray) && - typeof response.answerArray[0] === 'string' + response.answerArray.every((value) => typeof value === 'string') ) } From bd3f94d37d45bc1d3894dc934e50f25d6bb1ef90 Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Thu, 10 Jun 2021 14:27:35 +0800 Subject: [PATCH 16/34] refactor: remove console.log --- src/public/modules/forms/helpers/response-factory.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/public/modules/forms/helpers/response-factory.ts b/src/public/modules/forms/helpers/response-factory.ts index a0e1d2157a..522473759e 100644 --- a/src/public/modules/forms/helpers/response-factory.ts +++ b/src/public/modules/forms/helpers/response-factory.ts @@ -16,7 +16,6 @@ import { export const getResponseInstance = ( fieldRecordData: DisplayedResponseWithoutAnswer, ): Response => { - console.log(fieldRecordData) if ( isNestedResponse(fieldRecordData) && fieldRecordData.fieldType === 'table' From ba1cd6a248276f7613dc4257f8ad77c75421c617 Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Thu, 10 Jun 2021 15:45:48 +0800 Subject: [PATCH 17/34] refactor: convert extractAnswer to private function --- .../helpers/CsvMergedHeadersGenerator.ts | 48 +++++++++++-------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts b/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts index 72b950ec3b..4ea3a73884 100644 --- a/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts +++ b/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts @@ -14,6 +14,15 @@ type UnprocessedRecord = { } export class CsvMergedHeadersGenerator extends CsvGenerator { + /** + * Extracts the string representation from a field response + */ + #extractAnswer: ( + unprocessedRecord: { [fieldId: string]: Response }, + fieldId: string, + colIndex: number, + ) => string + hasBeenProcessed: boolean hasBeenSorted: boolean fieldIdToQuestion: Map< @@ -34,6 +43,23 @@ export class CsvMergedHeadersGenerator extends CsvGenerator { this.fieldIdToQuestion = new Map() this.fieldIdToNumCols = {} this.unprocessed = [] + + /** + * Extracts the string representation from a field response + * @param unprocessedRecord + * @param fieldId + * @param colIndex + * @returns string representation of unprocessed record + */ + this.#extractAnswer = function ( + unprocessedRecord, + fieldId, + colIndex, + ): string { + const fieldRecord = unprocessedRecord[fieldId] + if (!fieldRecord) return '' + return fieldRecord.getAnswer(colIndex) + } } /** @@ -49,6 +75,7 @@ export class CsvMergedHeadersGenerator extends CsvGenerator { * @param decryptedContent.record * @param decryptedContent.created * @param decryptedContent.submissionId + * @throws Error when trying to convert record into a response instance. Should be caught in submissions client factory. */ addRecord({ record, @@ -62,7 +89,7 @@ export class CsvMergedHeadersGenerator extends CsvGenerator { // First pass, create object with { [fieldId]: question } from // decryptedContent to get all the questions. const fieldRecords = record.map((content) => { - const fieldRecord = getResponseInstance(content) // Could throw error, to be caught in submissions client factory + const fieldRecord = getResponseInstance(content) if (!fieldRecord.isHeader) { const currentMapping = this.fieldIdToQuestion.get(fieldRecord.id) // Only set new mapping if it does not exist or this record is a later @@ -95,23 +122,6 @@ export class CsvMergedHeadersGenerator extends CsvGenerator { }) } - /** - * Extracts the string representation from an unprocessed record. - * @param unprocessedRecord - * @param fieldId - * @param colIndex - * @returns string representation of unprocessed record - */ - _extractAnswer( - unprocessedRecord: { [fieldId: string]: Response }, - fieldId: string, - colIndex: number, - ): string { - const fieldRecord = unprocessedRecord[fieldId] - if (!fieldRecord) return '' - return fieldRecord.getAnswer(colIndex) - } - /** * Process the unprocessed records by creating the correct headers and * assigning each answer to their respective locations in each response row in @@ -141,7 +151,7 @@ export class CsvMergedHeadersGenerator extends CsvGenerator { this.fieldIdToQuestion.forEach((_question, fieldId) => { const numCols = this.fieldIdToNumCols[fieldId] for (let colIndex = 0; colIndex < numCols; colIndex++) { - row.push(this._extractAnswer(up.record, fieldId, colIndex)) + row.push(this.#extractAnswer(up.record, fieldId, colIndex)) } }) this.addLine(row) From 38eaf6bec2e5fc6cd33b4ad8dad5f4ef9e1f77d5 Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Thu, 10 Jun 2021 15:46:38 +0800 Subject: [PATCH 18/34] docs: add jsdocs for response factory --- src/public/modules/forms/helpers/response-factory.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/public/modules/forms/helpers/response-factory.ts b/src/public/modules/forms/helpers/response-factory.ts index 522473759e..97480fb4f4 100644 --- a/src/public/modules/forms/helpers/response-factory.ts +++ b/src/public/modules/forms/helpers/response-factory.ts @@ -13,6 +13,12 @@ import { TableResponse, } from './csv-response-classes' +/** + * Converts a field record into a custom response instance + * @param fieldRecordData Field record + * @returns Response instance + * @throws Error when data does not fit any known response type + */ export const getResponseInstance = ( fieldRecordData: DisplayedResponseWithoutAnswer, ): Response => { @@ -30,7 +36,7 @@ export const getResponseInstance = ( return new SingleAnswerResponse(fieldRecordData) } else { // eslint-disable-next-line typesafe/no-throw-sync-func - throw new Error('Response did not match any known type') // should be caught in submissions client factory + throw new Error('Response did not match any known type') } } @@ -40,7 +46,7 @@ const isNestedResponse = ( return ( hasProp(response, 'answerArray') && Array.isArray(response.answerArray) && - response.answerArray.every((value) => Array.isArray(value)) && // or has at least one element that is an array + response.answerArray.every((value) => Array.isArray(value)) && response.answerArray.every((arr) => arr.every((value: unknown) => typeof value === 'string'), ) From 1659012d003ef2d3ac37db7262e4ab2c9b4450d3 Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Thu, 10 Jun 2021 15:51:04 +0800 Subject: [PATCH 19/34] docs: edit jsdocs for addRecord --- src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts b/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts index 4ea3a73884..9b6681d619 100644 --- a/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts +++ b/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts @@ -70,7 +70,7 @@ export class CsvMergedHeadersGenerator extends CsvGenerator { } /** - * Adds an UnprocessedRecord to this.unprocessed + * Extracts information from input record, rearranges record and then adds an UnprocessedRecord to this.unprocessed * @param decryptedContent * @param decryptedContent.record * @param decryptedContent.created From 2b8501ffcdfc939bf169f7fba7ff3da40a67a576 Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Thu, 10 Jun 2021 16:03:06 +0800 Subject: [PATCH 20/34] refactor: extract acheck for answer array --- .../modules/forms/helpers/response-factory.ts | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/public/modules/forms/helpers/response-factory.ts b/src/public/modules/forms/helpers/response-factory.ts index 97480fb4f4..07854b4742 100644 --- a/src/public/modules/forms/helpers/response-factory.ts +++ b/src/public/modules/forms/helpers/response-factory.ts @@ -44,11 +44,12 @@ const isNestedResponse = ( response: DisplayedResponseWithoutAnswer, ): response is NestedResponse => { return ( - hasProp(response, 'answerArray') && - Array.isArray(response.answerArray) && + hasAnswerArray(response) && response.answerArray.every((value) => Array.isArray(value)) && - response.answerArray.every((arr) => - arr.every((value: unknown) => typeof value === 'string'), + response.answerArray.every( + (arr) => + Array.isArray(arr) && + arr.every((value: unknown) => typeof value === 'string'), ) ) } @@ -57,8 +58,7 @@ const isArrayResponse = ( response: DisplayedResponseWithoutAnswer, ): response is ArrayResponse => { return ( - hasProp(response, 'answerArray') && - Array.isArray(response.answerArray) && + hasAnswerArray(response) && response.answerArray.every((value) => typeof value === 'string') ) } @@ -68,3 +68,11 @@ const isSingleResponse = ( ): response is SingleResponse => { return hasProp(response, 'answer') && typeof response.answer === 'string' } + +type AnswerArrayObject = { + answerArray: Array +} + +const hasAnswerArray = (response: unknown): response is AnswerArrayObject => { + return hasProp(response, 'answerArray') && Array.isArray(response.answerArray) +} From 12afa33cd081df33546bb76d6398b4f816e22aa0 Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Thu, 10 Jun 2021 17:45:10 +0800 Subject: [PATCH 21/34] refactor: revert extractAnswer to non-private function --- .../helpers/CsvMergedHeadersGenerator.ts | 45 ++++++++----------- 1 file changed, 18 insertions(+), 27 deletions(-) diff --git a/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts b/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts index 9b6681d619..36b49853ac 100644 --- a/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts +++ b/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts @@ -14,15 +14,6 @@ type UnprocessedRecord = { } export class CsvMergedHeadersGenerator extends CsvGenerator { - /** - * Extracts the string representation from a field response - */ - #extractAnswer: ( - unprocessedRecord: { [fieldId: string]: Response }, - fieldId: string, - colIndex: number, - ) => string - hasBeenProcessed: boolean hasBeenSorted: boolean fieldIdToQuestion: Map< @@ -43,23 +34,6 @@ export class CsvMergedHeadersGenerator extends CsvGenerator { this.fieldIdToQuestion = new Map() this.fieldIdToNumCols = {} this.unprocessed = [] - - /** - * Extracts the string representation from a field response - * @param unprocessedRecord - * @param fieldId - * @param colIndex - * @returns string representation of unprocessed record - */ - this.#extractAnswer = function ( - unprocessedRecord, - fieldId, - colIndex, - ): string { - const fieldRecord = unprocessedRecord[fieldId] - if (!fieldRecord) return '' - return fieldRecord.getAnswer(colIndex) - } } /** @@ -151,7 +125,7 @@ export class CsvMergedHeadersGenerator extends CsvGenerator { this.fieldIdToQuestion.forEach((_question, fieldId) => { const numCols = this.fieldIdToNumCols[fieldId] for (let colIndex = 0; colIndex < numCols; colIndex++) { - row.push(this.#extractAnswer(up.record, fieldId, colIndex)) + row.push(this._extractAnswer(up.record, fieldId, colIndex)) } }) this.addLine(row) @@ -159,6 +133,23 @@ export class CsvMergedHeadersGenerator extends CsvGenerator { this.hasBeenProcessed = true } + /** + * Extracts the string representation from a field response + * @param unprocessedRecord + * @param fieldId + * @param colIndex + * @returns string representation of unprocessed record + */ + _extractAnswer( + unprocessedRecord: { [fieldId: string]: Response }, + fieldId: string, + colIndex: number, + ): string { + const fieldRecord = unprocessedRecord[fieldId] + if (!fieldRecord) return '' + return fieldRecord.getAnswer(colIndex) + } + /** * Sorts unprocessed records from oldest to newest */ From 33fe9471274f3345800803370724da5b16c192dd Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Mon, 14 Jun 2021 15:48:42 +0800 Subject: [PATCH 22/34] refactor: remove repeated check for type Array --- src/public/modules/forms/helpers/response-factory.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/public/modules/forms/helpers/response-factory.ts b/src/public/modules/forms/helpers/response-factory.ts index 07854b4742..3a28abb42a 100644 --- a/src/public/modules/forms/helpers/response-factory.ts +++ b/src/public/modules/forms/helpers/response-factory.ts @@ -45,7 +45,6 @@ const isNestedResponse = ( ): response is NestedResponse => { return ( hasAnswerArray(response) && - response.answerArray.every((value) => Array.isArray(value)) && response.answerArray.every( (arr) => Array.isArray(arr) && From cba1116f621cc239f5d2b88275721cad665c55ff Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Thu, 17 Jun 2021 10:40:44 +0800 Subject: [PATCH 23/34] refactor: create SubmissionRecord type --- .../forms/helpers/CsvMergedHeadersGenerator.ts | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts b/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts index 36b49853ac..70adfc8c08 100644 --- a/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts +++ b/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts @@ -13,6 +13,12 @@ type UnprocessedRecord = { record: { [fieldId: string]: Response } } +type SubmissionRecord = { + record: DisplayedResponseWithoutAnswer[] + created: string + submissionId: string +} + export class CsvMergedHeadersGenerator extends CsvGenerator { hasBeenProcessed: boolean hasBeenSorted: boolean @@ -51,15 +57,7 @@ export class CsvMergedHeadersGenerator extends CsvGenerator { * @param decryptedContent.submissionId * @throws Error when trying to convert record into a response instance. Should be caught in submissions client factory. */ - addRecord({ - record, - created, - submissionId, - }: { - record: DisplayedResponseWithoutAnswer[] - created: string - submissionId: string - }): void { + addRecord({ record, created, submissionId }: SubmissionRecord): void { // First pass, create object with { [fieldId]: question } from // decryptedContent to get all the questions. const fieldRecords = record.map((content) => { From c6df4a5f5cf62738151937fd754dc25990f18f2c Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Thu, 17 Jun 2021 10:41:45 +0800 Subject: [PATCH 24/34] refactor: rename ccsv generator in submissions client factory --- .../services/submissions.client.factory.js | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/src/public/modules/forms/services/submissions.client.factory.js b/src/public/modules/forms/services/submissions.client.factory.js index 1b3eb410a4..20e5c4b56b 100644 --- a/src/public/modules/forms/services/submissions.client.factory.js +++ b/src/public/modules/forms/services/submissions.client.factory.js @@ -1,7 +1,7 @@ 'use strict' const { - CsvMergedHeadersGenerator: CsvMHGenerator, + CsvMergedHeadersGenerator, } = require('../helpers/CsvMergedHeadersGenerator') const DecryptionWorker = require('../helpers/decryption.worker.js') const { fixParamsToUrl, triggerFileDownload } = require('../helpers/util') @@ -141,7 +141,7 @@ function SubmissionsFactory($q, $http, $timeout, $window, GTag, FormSgSdk) { } let resUrl = generateDownloadUrl(params, downloadAttachments) - let experimentalCsvGenerator = new CsvMHGenerator( + let csvGenerator = new CsvMergedHeadersGenerator( expectedNumResponses, NUM_OF_METADATA_ROWS, ) @@ -183,7 +183,7 @@ function SubmissionsFactory($q, $http, $timeout, $window, GTag, FormSgSdk) { if (csvRecord.submissionData) { try { // accumulate dataset if it exists, since we may have status columns available - experimentalCsvGenerator.addRecord(csvRecord.submissionData) + csvGenerator.addRecord(csvRecord.submissionData) } catch (error) { errorCount++ console.error('Error in getResponseInstance', error) @@ -280,7 +280,7 @@ function SubmissionsFactory($q, $http, $timeout, $window, GTag, FormSgSdk) { GTag.partialDecryptionFailure( params, numWorkers, - experimentalCsvGenerator.length(), + csvGenerator.length(), errorCount, attachmentErrorCount, timeDifference, @@ -290,7 +290,7 @@ function SubmissionsFactory($q, $http, $timeout, $window, GTag, FormSgSdk) { new Error( JSON.stringify({ expectedCount: expectedNumResponses, - successCount: experimentalCsvGenerator.length(), + successCount: csvGenerator.length(), errorCount, unverifiedCount, }), @@ -298,18 +298,16 @@ function SubmissionsFactory($q, $http, $timeout, $window, GTag, FormSgSdk) { ) } else if ( // All results have been decrypted - experimentalCsvGenerator.length() + - errorCount + - unverifiedCount >= + csvGenerator.length() + errorCount + unverifiedCount >= expectedNumResponses ) { killWorkers(workerPool) // Generate first three rows of meta-data before download - experimentalCsvGenerator.addMetaDataFromSubmission( + csvGenerator.addMetaDataFromSubmission( errorCount, unverifiedCount, ) - experimentalCsvGenerator.downloadCsv( + csvGenerator.downloadCsv( `${params.formTitle}-${params.formId}.csv`, ) @@ -321,18 +319,18 @@ function SubmissionsFactory($q, $http, $timeout, $window, GTag, FormSgSdk) { GTag.downloadResponseSuccess( params, numWorkers, - experimentalCsvGenerator.length(), + csvGenerator.length(), timeDifference, ) resolve({ expectedCount: expectedNumResponses, - successCount: experimentalCsvGenerator.length(), + successCount: csvGenerator.length(), errorCount, unverifiedCount, }) // Kill class instance and reclaim the memory. - experimentalCsvGenerator = null + csvGenerator = null } else { $timeout(checkComplete, 100) } From 85ae863f741e003faa8e9809c1b1f244169ed327 Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Thu, 17 Jun 2021 10:42:24 +0800 Subject: [PATCH 25/34] refactor: make _data field private --- .../csv-response-classes/ArrayAnswerResponse.class.ts | 6 +++--- .../helpers/csv-response-classes/Response.class.ts | 10 +++++----- .../csv-response-classes/SingleAnswerResponse.class.ts | 6 +++--- .../csv-response-classes/TableResponse.class.ts | 10 +++++----- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.ts b/src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.ts index 63e5dad4a6..3dd341b776 100644 --- a/src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.ts +++ b/src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.ts @@ -3,15 +3,15 @@ import { ArrayResponse } from '../../../../../types/response' import { Response } from './Response.class' export class ArrayAnswerResponse extends Response { - _data: ArrayResponse + #data: ArrayResponse constructor(responseData: ArrayResponse) { super(responseData) - this._data = responseData + this.#data = responseData } getAnswer(): string { - return this._data.answerArray.join(';') + return this.#data.answerArray.join(';') } get numCols(): number { diff --git a/src/public/modules/forms/helpers/csv-response-classes/Response.class.ts b/src/public/modules/forms/helpers/csv-response-classes/Response.class.ts index 46726bf66a..dd80b3f5e8 100644 --- a/src/public/modules/forms/helpers/csv-response-classes/Response.class.ts +++ b/src/public/modules/forms/helpers/csv-response-classes/Response.class.ts @@ -1,14 +1,14 @@ import { DisplayedResponseWithoutAnswer } from '../../../../../types/response' export abstract class Response { - _data: DisplayedResponseWithoutAnswer + #data: DisplayedResponseWithoutAnswer constructor(responseData: DisplayedResponseWithoutAnswer) { - this._data = responseData + this.#data = responseData } get id(): string { - return this._data._id + return this.#data._id } /** @@ -16,11 +16,11 @@ export abstract class Response { * @returns {string} */ get question(): string { - return this._data.question + return this.#data.question } get isHeader(): boolean { - return this._data.isHeader ?? false + return this.#data.isHeader ?? false } abstract get numCols(): number diff --git a/src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.ts b/src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.ts index 1a681c6ce1..41e27ee3fd 100644 --- a/src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.ts +++ b/src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.ts @@ -3,15 +3,15 @@ import { SingleResponse } from '../../../../../types/response' import { Response } from './Response.class' export class SingleAnswerResponse extends Response { - _data: SingleResponse + #data: SingleResponse constructor(responseData: SingleResponse) { super(responseData) - this._data = responseData + this.#data = responseData } getAnswer(): string { - return this._data.answer + return this.#data.answer } get numCols(): number { diff --git a/src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.ts b/src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.ts index e94684ab2e..6dee9905eb 100644 --- a/src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.ts +++ b/src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.ts @@ -3,22 +3,22 @@ import { NestedResponse } from '../../../../../types/response' import { Response } from './Response.class' export class TableResponse extends Response { - _data: NestedResponse + #data: NestedResponse constructor(responseData: NestedResponse) { super(responseData) - this._data = responseData + this.#data = responseData } getAnswer(colIndex: number): string { // Leave cell empty if number of rows is fewer than the index - if (colIndex >= this._data.answerArray.length) { + if (colIndex >= this.#data.answerArray.length) { return '' } - return this._data.answerArray[colIndex].join(';') + return this.#data.answerArray[colIndex].join(';') } get numCols(): number { - return this._data.answerArray.length + return this.#data.answerArray.length } } From c0dafebdf09a7634c92994cb30075ba391dc9d54 Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Thu, 17 Jun 2021 10:43:42 +0800 Subject: [PATCH 26/34] test: rrestore deleted test and edit expected unprocessed record --- .../helpers/CsvMergedHeadersGenerator.test.js | 79 ++++++++++++++++++- 1 file changed, 77 insertions(+), 2 deletions(-) diff --git a/tests/unit/frontend/forms/helpers/CsvMergedHeadersGenerator.test.js b/tests/unit/frontend/forms/helpers/CsvMergedHeadersGenerator.test.js index 7b72aba76f..285d143dee 100644 --- a/tests/unit/frontend/forms/helpers/CsvMergedHeadersGenerator.test.js +++ b/tests/unit/frontend/forms/helpers/CsvMergedHeadersGenerator.test.js @@ -1,6 +1,6 @@ import { stringify } from 'csv-string' import moment from 'moment-timezone' - +import { getResponseInstance } from '../../../../../src/public/modules/forms/helpers/response-factory' import { CsvMergedHeadersGenerator } from '../../../../../src/public/modules/forms/helpers/CsvMergedHeadersGenerator' const UTF8_BYTE_ORDER_MARK = '\uFEFF' @@ -27,6 +27,15 @@ const generateRecord = (append, answerArray, fieldType) => { return generated } +const generateHeaderRow = (append) => { + return { + _id: `mock${append}`, + question: `mockQuestion${append}`, + fieldType: `mockFieldType${append}`, + isHeader: true, + answer: '', + } +} /** * Reshapes a mock record to match the expected shape in generator.unprocessed. @@ -38,7 +47,7 @@ const generateRecord = (append, answerArray, fieldType) => { const generateExpectedUnprocessed = (mockRecord) => { const reshapedRecord = {} mockRecord.record.forEach((fieldRecord) => { - reshapedRecord[fieldRecord._id] = { _data: fieldRecord } + reshapedRecord[fieldRecord._id] = getResponseInstance(fieldRecord) }) return { created: mockRecord.created, @@ -206,6 +215,72 @@ describe('CsvMergedHeadersGenerator', () => { expectedQuestionHeader, ) }) + + it('should handle adding of single header record', () => { + // Arrange + const mockDecryptedRecord = [generateHeaderRow(1)] + const mockRecord = { + record: mockDecryptedRecord, + created: mockCreatedEarly, + submissionId: 'mockSubmissionId', + } + expect(generator.unprocessed.length).toEqual(0) + + // Act + generator.addRecord(mockRecord) + + // Assert + // Generate new shape in unprocessed array + const expectedUnprocessed = [generateExpectedUnprocessed(mockRecord)] + + expect(generator.unprocessed.length).toEqual(1) + // Check shape + expect(generator.unprocessed).toEqual(expectedUnprocessed) + // Check headers + expect(generator.fieldIdToQuestion.size).toEqual(0) + }) + + it('should override the question to the latest question', () => { + // Arrange + const expectedQuestionHeader = 'this should override the old question' + const mockDecryptedRecordEarlier = [generateRecord(1), generateRecord(2)] + + // Later record has `mock1` id, same as earlier record. + const mockDecryptedRecordLater = [ + { + _id: 'mock1', + question: expectedQuestionHeader, + answer: 'mockAnswer1', + fieldType: 'mockFieldType1', + }, + ] + const mockRecordEarlier = { + record: mockDecryptedRecordEarlier, + created: mockCreatedEarly, + submissionId: 'mockSubmissionId', + } + const mockRecordLater = { + record: mockDecryptedRecordLater, + created: mockCreatedLater, + submissionId: 'mockSubmissionId', + } + expect(generator.unprocessed.length).toEqual(0) + + // Act + // Add record, order should not matter + generator.addRecord(mockRecordLater) + generator.addRecord(mockRecordEarlier) + + // Assert + expect(generator.unprocessed.length).toEqual(2) + // Should also have 2 headers since `mockDecryptedRecordEarlier` had 2 + // answers. + expect(generator.fieldIdToQuestion.size).toEqual(2) + + expect(generator.fieldIdToQuestion.get('mock1').question).toEqual( + expectedQuestionHeader, + ) + }) }) describe('process()', () => { From ba1179ed9e23ccacf0b76deaa4b939464fdd82ac Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Thu, 17 Jun 2021 10:45:54 +0800 Subject: [PATCH 27/34] test: remove repeated test --- .../helpers/CsvMergedHeadersGenerator.test.js | 42 ------------------- 1 file changed, 42 deletions(-) diff --git a/tests/unit/frontend/forms/helpers/CsvMergedHeadersGenerator.test.js b/tests/unit/frontend/forms/helpers/CsvMergedHeadersGenerator.test.js index 285d143dee..33b825aeae 100644 --- a/tests/unit/frontend/forms/helpers/CsvMergedHeadersGenerator.test.js +++ b/tests/unit/frontend/forms/helpers/CsvMergedHeadersGenerator.test.js @@ -174,48 +174,6 @@ describe('CsvMergedHeadersGenerator', () => { expect(Object.values(generator.fieldIdToNumCols)).toEqual([2]) }) - it('should override the question to the latest question', () => { - // Arrange - const expectedQuestionHeader = 'this should override the old question' - const mockDecryptedRecordEarlier = [generateRecord(1), generateRecord(2)] - - // Later record has `mock1` id, same as earlier record. - const mockDecryptedRecordLater = [ - { - _id: 'mock1', - question: expectedQuestionHeader, - answer: 'mockAnswer1', - fieldType: 'mockFieldType1', - }, - ] - const mockRecordEarlier = { - record: mockDecryptedRecordEarlier, - created: mockCreatedEarly, - submissionId: 'mockSubmissionId', - } - const mockRecordLater = { - record: mockDecryptedRecordLater, - created: mockCreatedLater, - submissionId: 'mockSubmissionId', - } - expect(generator.unprocessed.length).toEqual(0) - - // Act - // Add record, order should not matter - generator.addRecord(mockRecordLater) - generator.addRecord(mockRecordEarlier) - - // Assert - expect(generator.unprocessed.length).toEqual(2) - // Should also have 2 headers since `mockDecryptedRecordEarlier` had 2 - // answers. - expect(generator.fieldIdToQuestion.size).toEqual(2) - - expect(generator.fieldIdToQuestion.get('mock1').question).toEqual( - expectedQuestionHeader, - ) - }) - it('should handle adding of single header record', () => { // Arrange const mockDecryptedRecord = [generateHeaderRow(1)] From b8b1d02057acbc716ab9fcd101b7dd4ff518fe5a Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Thu, 17 Jun 2021 11:44:23 +0800 Subject: [PATCH 28/34] refactor: use private modifier in typescript classes --- .../modules/forms/helpers/CsvMergedHeadersGenerator.ts | 2 +- .../csv-response-classes/ArrayAnswerResponse.class.ts | 6 +++--- .../helpers/csv-response-classes/Response.class.ts | 10 +++++----- .../csv-response-classes/SingleAnswerResponse.class.ts | 6 +++--- .../csv-response-classes/TableResponse.class.ts | 10 +++++----- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts b/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts index 70adfc8c08..b05d22c933 100644 --- a/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts +++ b/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts @@ -138,7 +138,7 @@ export class CsvMergedHeadersGenerator extends CsvGenerator { * @param colIndex * @returns string representation of unprocessed record */ - _extractAnswer( + private _extractAnswer( unprocessedRecord: { [fieldId: string]: Response }, fieldId: string, colIndex: number, diff --git a/src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.ts b/src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.ts index 3dd341b776..f77e486541 100644 --- a/src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.ts +++ b/src/public/modules/forms/helpers/csv-response-classes/ArrayAnswerResponse.class.ts @@ -3,15 +3,15 @@ import { ArrayResponse } from '../../../../../types/response' import { Response } from './Response.class' export class ArrayAnswerResponse extends Response { - #data: ArrayResponse + private response: ArrayResponse constructor(responseData: ArrayResponse) { super(responseData) - this.#data = responseData + this.response = responseData } getAnswer(): string { - return this.#data.answerArray.join(';') + return this.response.answerArray.join(';') } get numCols(): number { diff --git a/src/public/modules/forms/helpers/csv-response-classes/Response.class.ts b/src/public/modules/forms/helpers/csv-response-classes/Response.class.ts index dd80b3f5e8..3f530033f8 100644 --- a/src/public/modules/forms/helpers/csv-response-classes/Response.class.ts +++ b/src/public/modules/forms/helpers/csv-response-classes/Response.class.ts @@ -1,14 +1,14 @@ import { DisplayedResponseWithoutAnswer } from '../../../../../types/response' export abstract class Response { - #data: DisplayedResponseWithoutAnswer + private _data: DisplayedResponseWithoutAnswer constructor(responseData: DisplayedResponseWithoutAnswer) { - this.#data = responseData + this._data = responseData } get id(): string { - return this.#data._id + return this._data._id } /** @@ -16,11 +16,11 @@ export abstract class Response { * @returns {string} */ get question(): string { - return this.#data.question + return this._data.question } get isHeader(): boolean { - return this.#data.isHeader ?? false + return this._data.isHeader ?? false } abstract get numCols(): number diff --git a/src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.ts b/src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.ts index 41e27ee3fd..f495544e3f 100644 --- a/src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.ts +++ b/src/public/modules/forms/helpers/csv-response-classes/SingleAnswerResponse.class.ts @@ -3,15 +3,15 @@ import { SingleResponse } from '../../../../../types/response' import { Response } from './Response.class' export class SingleAnswerResponse extends Response { - #data: SingleResponse + private response: SingleResponse constructor(responseData: SingleResponse) { super(responseData) - this.#data = responseData + this.response = responseData } getAnswer(): string { - return this.#data.answer + return this.response.answer } get numCols(): number { diff --git a/src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.ts b/src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.ts index 6dee9905eb..cd62ec6b24 100644 --- a/src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.ts +++ b/src/public/modules/forms/helpers/csv-response-classes/TableResponse.class.ts @@ -3,22 +3,22 @@ import { NestedResponse } from '../../../../../types/response' import { Response } from './Response.class' export class TableResponse extends Response { - #data: NestedResponse + private response: NestedResponse constructor(responseData: NestedResponse) { super(responseData) - this.#data = responseData + this.response = responseData } getAnswer(colIndex: number): string { // Leave cell empty if number of rows is fewer than the index - if (colIndex >= this.#data.answerArray.length) { + if (colIndex >= this.response.answerArray.length) { return '' } - return this.#data.answerArray[colIndex].join(';') + return this.response.answerArray[colIndex].join(';') } get numCols(): number { - return this.#data.answerArray.length + return this.response.answerArray.length } } From 9b0b79e3c4e0e8fe6a56e815afbf8767aa5fe0bf Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Thu, 17 Jun 2021 11:44:57 +0800 Subject: [PATCH 29/34] test: add tests for error cases --- .../helpers/CsvMergedHeadersGenerator.test.js | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/tests/unit/frontend/forms/helpers/CsvMergedHeadersGenerator.test.js b/tests/unit/frontend/forms/helpers/CsvMergedHeadersGenerator.test.js index 33b825aeae..11c0564a72 100644 --- a/tests/unit/frontend/forms/helpers/CsvMergedHeadersGenerator.test.js +++ b/tests/unit/frontend/forms/helpers/CsvMergedHeadersGenerator.test.js @@ -37,6 +37,16 @@ const generateHeaderRow = (append) => { } } +const generateEmptyRecord = (append) => { + return { + _id: `mock${append}`, + question: `mockQuestion${append}`, + fieldType: `mockFieldType${append}`, + } +} + +const expectedErrorMessage = 'Response did not match any known type' + /** * Reshapes a mock record to match the expected shape in generator.unprocessed. * @param {Object} mockRecord @@ -239,6 +249,79 @@ describe('CsvMergedHeadersGenerator', () => { expectedQuestionHeader, ) }) + + it('should reject response without answer and answerArray', () => { + // Arrange + const mockDecryptedRecord = [generateEmptyRecord(1)] + const mockRecord = { + record: mockDecryptedRecord, + created: mockCreatedEarly, + submissionId: 'mockSubmissionId', + } + expect(generator.unprocessed.length).toEqual(0) + + // Act + const addRecord = () => generator.addRecord(mockRecord) + + // Assert + // Check error + expect(addRecord).toThrow(Error) + expect(addRecord).toThrow(expectedErrorMessage) + // Record should not be added + expect(generator.unprocessed.length).toEqual(0) + // Check headers + expect(generator.fieldIdToQuestion.size).toEqual(0) + }) + + it('should reject response with non-string answer', () => { + // Arrange + const invalidResponse = generateEmptyRecord(1) + invalidResponse.answer = 1 + const mockDecryptedRecord = [invalidResponse] + const mockRecord = { + record: mockDecryptedRecord, + created: mockCreatedEarly, + submissionId: 'mockSubmissionId', + } + expect(generator.unprocessed.length).toEqual(0) + + // Act + const addRecord = () => generator.addRecord(mockRecord) + + // Assert + // Check error + expect(addRecord).toThrow(Error) + expect(addRecord).toThrow(expectedErrorMessage) + // Record should not be added + expect(generator.unprocessed.length).toEqual(0) + // Check headers + expect(generator.fieldIdToQuestion.size).toEqual(0) + }) + + it('should reject response with non-string answerArray', () => { + // Arrange + const invalidResponse = generateEmptyRecord(1) + invalidResponse.answerArray = [1, 2, 3] + const mockDecryptedRecord = [invalidResponse] + const mockRecord = { + record: mockDecryptedRecord, + created: mockCreatedEarly, + submissionId: 'mockSubmissionId', + } + expect(generator.unprocessed.length).toEqual(0) + + // Act + const addRecord = () => generator.addRecord(mockRecord) + + // Assert + // Check error + expect(addRecord).toThrow(Error) + expect(addRecord).toThrow(expectedErrorMessage) + // Record should not be added + expect(generator.unprocessed.length).toEqual(0) + // Check headers + expect(generator.fieldIdToQuestion.size).toEqual(0) + }) }) describe('process()', () => { From f6c001e7e02ec3f1016e6843a19e00ea916132e4 Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Thu, 17 Jun 2021 11:45:59 +0800 Subject: [PATCH 30/34] refactor: add private modifier to date comparator --- src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts b/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts index b05d22c933..d8ad0221f0 100644 --- a/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts +++ b/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts @@ -187,7 +187,7 @@ export class CsvMergedHeadersGenerator extends CsvGenerator { * @param firstDate * @param secondDate */ - _dateComparator(firstDate: string, secondDate: string): number { + private _dateComparator(firstDate: string, secondDate: string): number { // cast to Asia/Singapore to ensure both dates are of the same timezone const first = moment(firstDate).tz('Asia/Singapore') const second = moment(secondDate).tz('Asia/Singapore') From 8140ffaf6b26b97ae70de3d91281d70fd2329869 Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Thu, 17 Jun 2021 11:48:26 +0800 Subject: [PATCH 31/34] refactor: remove underscore from private field --- .../helpers/csv-response-classes/Response.class.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/public/modules/forms/helpers/csv-response-classes/Response.class.ts b/src/public/modules/forms/helpers/csv-response-classes/Response.class.ts index 3f530033f8..9f2c1d62bc 100644 --- a/src/public/modules/forms/helpers/csv-response-classes/Response.class.ts +++ b/src/public/modules/forms/helpers/csv-response-classes/Response.class.ts @@ -1,14 +1,14 @@ import { DisplayedResponseWithoutAnswer } from '../../../../../types/response' export abstract class Response { - private _data: DisplayedResponseWithoutAnswer + private data: DisplayedResponseWithoutAnswer constructor(responseData: DisplayedResponseWithoutAnswer) { - this._data = responseData + this.data = responseData } get id(): string { - return this._data._id + return this.data._id } /** @@ -16,11 +16,11 @@ export abstract class Response { * @returns {string} */ get question(): string { - return this._data.question + return this.data.question } get isHeader(): boolean { - return this._data.isHeader ?? false + return this.data.isHeader ?? false } abstract get numCols(): number From 67db42f340a7d2a993527e362be7e1cb82feaf47 Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Tue, 22 Jun 2021 10:03:55 +0800 Subject: [PATCH 32/34] refactor: extract type in csvmhgenerator --- .../modules/forms/helpers/CsvMergedHeadersGenerator.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts b/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts index d8ad0221f0..9e148e92f2 100644 --- a/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts +++ b/src/public/modules/forms/helpers/CsvMergedHeadersGenerator.ts @@ -7,10 +7,12 @@ import { Response } from './csv-response-classes' import { CsvGenerator } from './CsvGenerator' import { getResponseInstance } from './response-factory' +type KeyedResponse = { [fieldId: string]: Response } + type UnprocessedRecord = { created: string submissionId: string - record: { [fieldId: string]: Response } + record: KeyedResponse } type SubmissionRecord = { @@ -139,7 +141,7 @@ export class CsvMergedHeadersGenerator extends CsvGenerator { * @returns string representation of unprocessed record */ private _extractAnswer( - unprocessedRecord: { [fieldId: string]: Response }, + unprocessedRecord: KeyedResponse, fieldId: string, colIndex: number, ): string { From b90f99c099314da3ff49d105fe0e4bf1620cb0f3 Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Tue, 22 Jun 2021 10:04:32 +0800 Subject: [PATCH 33/34] refactor: replace strings with basic field enums --- src/public/modules/forms/helpers/response-factory.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/public/modules/forms/helpers/response-factory.ts b/src/public/modules/forms/helpers/response-factory.ts index 3a28abb42a..3ba210291b 100644 --- a/src/public/modules/forms/helpers/response-factory.ts +++ b/src/public/modules/forms/helpers/response-factory.ts @@ -1,3 +1,5 @@ +import { BasicField } from 'src/types/field' + import { hasProp } from '../../../../shared/util/has-prop' import { ArrayResponse, @@ -24,12 +26,12 @@ export const getResponseInstance = ( ): Response => { if ( isNestedResponse(fieldRecordData) && - fieldRecordData.fieldType === 'table' + fieldRecordData.fieldType === BasicField.Table ) { return new TableResponse(fieldRecordData) } else if ( isArrayResponse(fieldRecordData) && - fieldRecordData.fieldType === 'checkbox' + fieldRecordData.fieldType === BasicField.Checkbox ) { return new ArrayAnswerResponse(fieldRecordData) } else if (isSingleResponse(fieldRecordData)) { From 61a49a4fb3dd7900c20b9b06cba52f380ae73b68 Mon Sep 17 00:00:00 2001 From: chowyiyin Date: Tue, 22 Jun 2021 10:15:44 +0800 Subject: [PATCH 34/34] fix: fix import statement --- src/public/modules/forms/helpers/response-factory.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/public/modules/forms/helpers/response-factory.ts b/src/public/modules/forms/helpers/response-factory.ts index 3ba210291b..3266101d50 100644 --- a/src/public/modules/forms/helpers/response-factory.ts +++ b/src/public/modules/forms/helpers/response-factory.ts @@ -1,6 +1,5 @@ -import { BasicField } from 'src/types/field' - import { hasProp } from '../../../../shared/util/has-prop' +import { BasicField } from '../../../../types/field' import { ArrayResponse, DisplayedResponseWithoutAnswer,