Merge pull request #51 from DEFRA/DSFAAP-577

DSFAAP-577: filter payload in answer model

src/server/common/model/answer/address.js

@@ -97,6 +97,22 @@ export class Address extends AnswerModel {
     return validateAnswerAgainstSchema(addressPayloadSchema, this._data ?? {})
   }
 
+  _extractFields({
+    addressLine1,
+    addressLine2,
+    addressTown,
+    addressCounty,
+    addressPostcode
+  }) {
+    return {
+      addressLine1,
+      ...(addressLine2 !== undefined && { addressLine2 }),
+      addressTown,
+      ...(addressCounty !== undefined && { addressCounty }),
+      addressPostcode
+    }
+  }
+
   /**
    * @param {AddressData | undefined} state
    * @returns {Address}

src/server/common/model/answer/address.test.js

@@ -8,6 +8,26 @@ const validAddress = {
   addressPostcode: 'RG24 8RR'
 }
 
+describe('Address.new', () => {
+  it('should strip away any irrelevant values', () => {
+    const payload = { ...validAddress, nextPage: '/other/page' }
+    const address = new Address(payload)
+
+    expect(address._data).toEqual(validAddress)
+  })
+
+  it('should graceully handle optional values', () => {
+    const validAddressWithOptionalsMissing = {
+      addressLine1: validAddress.addressLine1,
+      addressTown: validAddress.addressTown,
+      addressPostcode: validAddress.addressPostcode
+    }
+    const address = new Address(validAddressWithOptionalsMissing)
+
+    expect(address._data).toEqual(validAddressWithOptionalsMissing)
+  })
+})
+
 describe('Address.validate', () => {
   it('should return true for valid address', () => {
     const address = new Address(validAddress)

src/server/common/model/answer/answer-model.js

@@ -20,7 +20,7 @@ export class AnswerModel {
    * @param {Payload | undefined } [data]
    */
   constructor(data) {
-    this._data = data
+    this._data = data === undefined ? undefined : this._extractFields(data)
     Object.seal(this)
   }
 
@@ -35,6 +35,15 @@ export class AnswerModel {
     throw new NotImplementedError()
   }
 
+  /**
+   * @param {Payload} _data
+   * @returns {Payload}
+   */
+  // eslint-disable-next-line @typescript-eslint/no-unused-vars
+  _extractFields(_data) {
+    throw new NotImplementedError()
+  }
+
   /**
    * @returns {AnswerValidationResult}
    */
@@ -47,17 +56,14 @@ export class AnswerModel {
     throw new NotImplementedError()
   }
 
-  /* eslint-disable @typescript-eslint/no-unused-vars */
-
   /**
    * @param {unknown} _data
    * @returns {unknown}
    */
+  // eslint-disable-next-line @typescript-eslint/no-unused-vars
   static fromState(_data) {
     throw new NotImplementedError()
   }
-
-  /* eslint-enable @typescript-eslint/no-unused-vars */
 }
 
 /**

src/server/common/model/answer/answer-model.test.js

@@ -29,7 +29,12 @@ describe('AnswerModel', () => {
   })
 
   it('should seal the object to prevent property additions or deletions', () => {
-    answer = new AnswerModel({ key: 'value' })
+    class AnswerModelBasic extends AnswerModel {
+      _extractFields(data) {
+        return data
+      }
+    }
+    answer = new AnswerModelBasic({ key: 'value' })
 
     expect(() => {
       answer.something = 'should fail'

src/server/common/model/answer/confirmation/confirmation.js

@@ -48,6 +48,10 @@ export class Confirmation extends AnswerModel {
     })
   }
 
+  _extractFields({ confirmation }) {
+    return { confirmation }
+  }
+
   /**
    * @param {ConfirmationData | undefined} state
    * @returns {Confirmation}

src/server/common/model/answer/confirmation/confirmation.test.js

@@ -1,6 +1,17 @@
 import { Confirmation } from './confirmation.js'
 
 describe('#ConfirmationModel', () => {
+  describe('Confirmation.new', () => {
+    it('should strip away any irrelevant values', () => {
+      const validConfirmation = { confirmation: ['confirm', 'other'] }
+
+      const payload = { ...validConfirmation, nextPage: '/other/page' }
+      const confirmation = new Confirmation(payload)
+
+      expect(confirmation._data).toEqual(validConfirmation)
+    })
+  })
+
   describe('validate', () => {
     it('should return valid when only confirmed', () => {
       const confirmation = new Confirmation({ confirmation: ['confirm'] })

src/server/common/model/answer/cph-number.js

@@ -48,6 +48,10 @@ export class CphNumber extends AnswerModel {
     return validateAnswerAgainstSchema(cphNumberPayloadSchema, this._data ?? {})
   }
 
+  _extractFields({ cphNumber }) {
+    return { cphNumber }
+  }
+
   /**
    * @param {CphNumberData | undefined} state
    * @returns {CphNumber}

src/server/common/model/answer/cph-number.test.js

@@ -4,6 +4,15 @@ const validCphNumberPayload = {
   cphNumber: '12/456/7899'
 }
 
+describe('CphNumber.new', () => {
+  it('should strip away any irrelevant values', () => {
+    const payload = { ...validCphNumberPayload, nextPage: '/other/page' }
+    const cphNumber = new CphNumber(payload)
+
+    expect(cphNumber._data).toEqual(validCphNumberPayload)
+  })
+})
+
 describe('#CphNumber.validate', () => {
   it('should return true for valid cphNumber', () => {
     const cphNumber = new CphNumber(validCphNumberPayload)

src/server/common/model/answer/email-address.js

@@ -51,6 +51,10 @@ export class EmailAddress extends AnswerModel {
     return validateAnswerAgainstSchema(emailAddressPayloadSchema, this._data)
   }
 
+  _extractFields({ emailAddress }) {
+    return { emailAddress }
+  }
+
   /**
    * @param {EmailAddressData | undefined} state
    * @returns {EmailAddress}

src/server/common/model/answer/email-address.test.js

@@ -31,6 +31,15 @@ const invalidEmailAddresses = [
 ]
 
 describe('EmailAddress', () => {
+  describe('new', () => {
+    it('should strip away any irrelevant values', () => {
+      const payload = { ...validEmailAddressPayload, nextPage: '/other/page' }
+      const emailAddress = new EmailAddress(payload)
+
+      expect(emailAddress._data).toEqual(validEmailAddressPayload)
+    })
+  })
+
   describe('validate', () => {
     validEmailAddresses.forEach((email) => {
       it(`should return true for valid email address ${email}`, () => {

src/server/common/model/answer/on-off-farm.js

@@ -56,4 +56,8 @@ export class OnOffFarm extends AnswerModel {
   validate() {
     return validateAnswerAgainstSchema(onOffFarmPayloadSchema, this._data ?? {})
   }
+
+  _extractFields({ onOffFarm }) {
+    return { onOffFarm }
+  }
 }

src/server/common/model/answer/on-off-farm.test.js

@@ -6,6 +6,15 @@ const validOnOffRadio = {
   onOffFarm: 'on'
 }
 
+describe('OnOffFarm.new', () => {
+  it('should strip away any irrelevant values', () => {
+    const payload = { ...validOnOffRadio, nextPage: '/other/page' }
+    const onOffFarm = new OnOffFarm(payload)
+
+    expect(onOffFarm._data).toEqual(validOnOffRadio)
+  })
+})
+
 describe('#OnOffFarm.validate', () => {
   test('should return true for on', () => {
     const { isValid, errors } = new OnOffFarm({

src/server/common/model/section/validation.test.js

@@ -1,21 +1,29 @@
 import { validateSection } from './validation.js'
+import { AnswerModel } from '../answer/answer-model.js'
 
-const baseMockAnswer = {
-  toState: jest.fn(),
-  value: 'value',
-  html: '',
-  _data: {}
+class MockAnswer extends AnswerModel {
+  // eslint-disable-next-line @typescript-eslint/no-unused-vars
+  _extractFields(data) {
+    return data
+  }
 }
 
-const validAnswer = {
-  ...baseMockAnswer,
-  validate: jest.fn().mockReturnValue({ isValid: true })
+class ValidAnswer extends MockAnswer {
+  // eslint-disable-next-line @typescript-eslint/no-unused-vars
+  validate(_) {
+    return { isValid: true, errors: {} }
+  }
 }
 
-const invalidAnswer = {
-  ...baseMockAnswer,
-  validate: jest.fn().mockReturnValue({ isValid: false })
+class InvalidAnswer extends MockAnswer {
+  // eslint-disable-next-line @typescript-eslint/no-unused-vars
+  validate(_) {
+    return { isValid: false, errors: { field1: { text: 'Invalid' } } }
+  }
 }
+const validAnswer = new ValidAnswer()
+
+const invalidAnswer = new InvalidAnswer()
 
 describe('validateSection', () => {
   it('should return isValid as true when all answers are valid', () => {
@@ -46,16 +54,8 @@ describe('validateSection', () => {
 
   it('should return the correct validation result for each answer', () => {
     const data = {
-      answer1: {
-        ...baseMockAnswer,
-        validate: jest.fn().mockReturnValue({ isValid: true, errors: {} })
-      },
-      answer2: {
-        ...baseMockAnswer,
-        validate: jest
-          .fn()
-          .mockReturnValue({ isValid: false, errors: { field1: 'Invalid' } })
-      }
+      answer1: validAnswer,
+      answer2: invalidAnswer
     }
 
     const { isValid, result } = validateSection(data)
@@ -64,7 +64,7 @@ describe('validateSection', () => {
     expect(result.answer1).toEqual({ isValid: true, errors: {} })
     expect(result.answer2).toEqual({
       isValid: false,
-      errors: { field1: 'Invalid' }
+      errors: { field1: { text: 'Invalid' } }
     })
   })
 })

user-journey-tests/helpers/testHelpers/checkAnswers.js

@@ -1,3 +1,4 @@
+import checkAnswersPage from '../../page-objects/origin/checkAnswersPage.js'
 import newAddressPage from '../../page-objects/origin/newAddressPage.js'
 import parishHoldingNumberPage from '../../page-objects/origin/parishHoldingNumberPage.js'
 import toFromFarmPage from '../../page-objects/origin/toFromFarmPage.js'
@@ -59,11 +60,16 @@ export const validateAndAdjustAddress = async (
     postcode
   })
 
-  await validateElementVisibleAndText(valueElement, lineOne)
-  await validateElementVisibleAndText(valueElement, lineTwo)
-  await validateElementVisibleAndText(valueElement, townOrCity)
-  await validateElementVisibleAndText(valueElement, county)
-  await validateElementVisibleAndText(valueElement, postcode)
+  // Expected text must be kept to the left of the page to ensure accuracy in the check
+  const elementText = await checkAnswersPage.addressValue
+    .getHTML(false)
+    .then((text) => text.replace(/<br\s*\/?>/g, '\n').trim())
+  const expectedText = `${lineOne}
+${lineTwo}
+${townOrCity}
+${county}
+${postcode}`
+  expect(elementText).toBe(expectedText)
 }
 
 export const validateAndAdjustEmail = async (