Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug/roe 2018 roe 1507 fix duplication of trust #717

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/controllers/trust.information.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ export const post = async (req: Request, res: Response, next: NextFunction) => {
try {
logger.debugRequest(req, `POST ${config.TRUST_INFO_PAGE}`);

if (req.body.submit) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like moving the submit redirect here means that it will ignore json posted in the json field on teh page and won't add it to the model. This is different behaviour from what was there previously, it might be ok, we'll have to check with the PO tomorrow in the office. It also allows us to move to the check your answers screen with 0 trusts added, so perhaps we should only allow the user to move forward if there is at least one trust in the model/session data. Can discuss in office tomorrow.

return res.redirect(config.CHECK_YOUR_ANSWERS_PAGE);
}

// If only one BO is selected, data is a string.
// If multiple selected, data is an array.
const beneficialOwnerIds = (typeof req.body.beneficialOwners === 'string') ? [req.body.beneficialOwners] : req.body.beneficialOwners;
Expand Down Expand Up @@ -62,12 +66,8 @@ export const post = async (req: Request, res: Response, next: NextFunction) => {

await saveAndContinue(req, session);

if (req.body.add) {
return res.redirect(config.TRUST_INFO_URL);
}
if (req.body.submit) {
return res.redirect(config.CHECK_YOUR_ANSWERS_PAGE);
}
return res.redirect(config.TRUST_INFO_URL);

} catch (error) {
logger.errorRequest(req, error);
next(error);
Expand Down
1 change: 1 addition & 0 deletions src/middleware/validation.middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ export function checkTrustValidations(req: Request, res: Response, next: NextFun
...req.body,
beneficialOwners: getBeneficialOwnerList(appData),
trusts_input: req.body.trusts?.toString(),
trustsAddedSoFar: (appData.trusts) ? appData.trusts.length : 0,
errors
});
}
Expand Down
43 changes: 39 additions & 4 deletions src/validation/custom.validation.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Custom validation utils - For now checking is not empty

import { Session } from '@companieshouse/node-session-handler';

import { VALID_CHARACTERS, VALID_EMAIL_FORMAT } from "./regex/regex.validation";
import { DateTime } from "luxon";
import { ErrorMessages } from "./error.messages";
Expand Down Expand Up @@ -452,22 +454,55 @@ export const checkAtLeastOneFieldHasValue = (errMsg: string, ...fields: any[]) =
throw new Error(errMsg);
};

export const checkTrustFields = (trustsJson: string) => {
const isSubmitTrustSelected = (session: Session, action: string = ""): boolean => {
const appData: ApplicationData = getApplicationData(session);
return action === "submit" && (appData.trusts || [] ).length > 0;
};

export const checkTrustBOs = (req) => {
if (isSubmitTrustSelected(req.session, req.body.submit)) {
return true;
}
return checkAtLeastOneFieldHasValue(ErrorMessages.TRUST_BO_CHECKBOX, req.body.beneficialOwners);
};

export const checkTrustFields = (req) => {

if (isSubmitTrustSelected(req.session, req.body.submit)) {
return true;
}

const trustsJson = (req.body.trusts || "").trim();

if (!trustsJson) {
throw new Error(ErrorMessages.TRUST_DATA_EMPTY);
}

if (!isValidJson(trustsJson)) {
throw new Error(ErrorMessages.TRUST_DATA_INVALID_FORMAT);
}

const trusts: trustType.Trust[] = JSON.parse(trustsJson);
const addressMaxLength = 50;

for (const trust of trusts) {
checkTrustCreationDate(trust);

checkTrustName(trust);

checkIndividualsAddress(trust, addressMaxLength);

checkCorporatesAddress(trust, addressMaxLength);
}
return true;
};

const isValidJson = (str) => {
try {
JSON.parse(str);
} catch (e) {
return false;
}
return true;
};

export const checkBeneficialOwnerType = (beneficialOwnersStatement: string, value) => {
if (!value) {
let errMsg = ErrorMessages.SELECT_THE_TYPE_OF_BENEFICIAL_OWNER_OR_MANAGING_OFFICER_YOU_WANT_TO_ADD;
Expand Down
1 change: 1 addition & 0 deletions src/validation/error.messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export enum ErrorMessages {

// Trusts
TRUST_DATA_EMPTY = "Paste the trust information from the Excel document into the box",
TRUST_DATA_INVALID_FORMAT = "Format of trust information is invalid",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we'll need to check the content of this message with UX or an analyst.

TRUST_NAME = "Enter the trust name",
TRUST_NAME_2 = "Enter the name of the trust",
TRUST_CREATION_DATE = "Enter the trust creation date",
Expand Down
13 changes: 3 additions & 10 deletions src/validation/trust.information.validation.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,7 @@
import { body } from "express-validator";
import { checkAtLeastOneFieldHasValue, checkTrustFields } from "./custom.validation";

import { ErrorMessages } from "./error.messages";
import { checkTrustBOs, checkTrustFields } from "./custom.validation";

export const trustInformation = [
body("beneficialOwners").custom((value, { req }) =>
checkAtLeastOneFieldHasValue(ErrorMessages.TRUST_BO_CHECKBOX, req.body.beneficialOwners)),

body("trusts")
.not().isEmpty({ ignore_whitespace: true }).withMessage(ErrorMessages.TRUST_DATA_EMPTY)
.custom((value, { req }) =>
checkTrustFields(req.body.trusts))
body("beneficialOwners").custom((_, { req }) => checkTrustBOs(req)),
body("trusts").custom((_, { req }) => checkTrustFields(req))
];
34 changes: 24 additions & 10 deletions test/__mocks__/session.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -785,6 +785,8 @@ export const TRUST_DATA: string = `[{
"CORPORATES": []
}]`;

export const TRUST_DATA_INVALID: string = "abc";

export const TRUST_DATA_NO_NAME: string = `[{
"creation_date_day": "31",
"creation_date_month": "12",
Expand Down Expand Up @@ -854,26 +856,38 @@ export const TRUSTS_SUBMIT = {
[trustType.TrustKey]: TRUST_DATA
};

export const TRUSTS_SUBMIT_MULTIPLE_BENEFICIAL_OWNERS = {
submit: "submit",
export const TRUSTS_ADD = {
add: "add",
beneficialOwners: "123",
[trustType.TrustKey]: TRUST_DATA
};

export const TRUSTS_ADD_MULTIPLE_BENEFICIAL_OWNERS = {
add: "add",
beneficialOwners: ["123", "456"],
[trustType.TrustKey]: TRUST_DATA
};

export const TRUSTS_SUBMIT_NO_NAME = {
submit: "submit",
export const TRUSTS_ADD_INVALID = {
add: "add",
beneficialOwners: "123",
[trustType.TrustKey]: TRUST_DATA_INVALID
};

export const TRUSTS_ADD_NO_NAME = {
add: "add",
beneficialOwners: "123",
[trustType.TrustKey]: TRUST_DATA_NO_NAME
};

export const TRUSTS_SUBMIT_NO_CREATION_DATE = {
submit: "submit",
export const TRUSTS_ADD_NO_CREATION_DATE = {
add: "add",
beneficialOwners: "123",
[trustType.TrustKey]: TRUST_DATA_NO_CREATION_DATE
};

export const TRUSTS_SUBMIT_PARTIAL_CREATION_DATE = {
submit: "submit",
export const TRUSTS_ADD_PARTIAL_CREATION_DATE = {
add: "add",
beneficialOwners: "123",
[trustType.TrustKey]: TRUST_DATA_PARTIAL_CREATION_DATE
};
Expand All @@ -894,8 +908,8 @@ export const TRUSTS_EMPTY_CHECKBOX = {
[trustType.TrustKey]: TRUST_DATA
};

export const TRUSTS_SUBMIT_LEADING_AND_TRAILING_WHITESPACE = {
submit: "submit",
export const TRUSTS_ADD_LEADING_AND_TRAILING_WHITESPACE = {
add: "add",
beneficialOwners: "123",
[trustType.TrustKey]: TRUST_DATA_LEADING_AND_TRAILING_SPACES
};
Expand Down
1 change: 1 addition & 0 deletions test/__mocks__/text.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export const MANAGING_OFFICER_CORPORATE_PAGE_TITLE = "Tell us about the corporat
export const SIGN_OUT_PAGE_TITLE = "Are you sure you want to sign out?";
export const SIGN_OUT_HINT_TEXT = "We will save your application.";
export const TRUST_INFO_PAGE_TITLE = "Add trust information for the beneficial owners";
export const TRUST_INFO_NO_MORE_TO_ADD_BUTTON = "No more";
export const CHECK_YOUR_ANSWERS_PAGE_TITLE = "Check your answers before sending your application";
export const VERIFICATION_CHECKS = "Verification checks";
export const VERIFICATION_CHECKS_DATE = "Date the verification checks were completed";
Expand Down
20 changes: 10 additions & 10 deletions test/__mocks__/validation.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -337,8 +337,8 @@ export const MANAGING_OFFICER_CORPORATE_WITH_INVALID_CHARS_SERVICE_ADDRESS_MOCK
...SERVICE_ADDRESS_WITH_INVALID_CHAR_FIELDS_MOCK
};

export const TRUSTS_SUBMIT_CORPORATE_SA_ADDRESS_PREMISES_TOO_LONG = {
submit: "submit",
export const TRUSTS_ADD_CORPORATE_SA_ADDRESS_PREMISES_TOO_LONG = {
add: "add",
beneficialOwners: "123",
[trustType.TrustKey]: `[{
"trust_name": "name of trust",
Expand Down Expand Up @@ -368,8 +368,8 @@ export const TRUSTS_SUBMIT_CORPORATE_SA_ADDRESS_PREMISES_TOO_LONG = {
}]`
};

export const TRUSTS_SUBMIT_CORPORATE_RO_ADDRESS_PREMISES_TOO_LONG = {
submit: "submit",
export const TRUSTS_ADD_CORPORATE_RO_ADDRESS_PREMISES_TOO_LONG = {
add: "add",
beneficialOwners: "123",
[trustType.TrustKey]: `[{
"trust_name": "name of trust",
Expand Down Expand Up @@ -399,8 +399,8 @@ export const TRUSTS_SUBMIT_CORPORATE_RO_ADDRESS_PREMISES_TOO_LONG = {
}]`
};

export const TRUSTS_SUBMIT_INDIVIDUAL_URA_ADDRESS_PREMISES_TOO_LONG = {
submit: "submit",
export const TRUSTS_ADD_INDIVIDUAL_URA_ADDRESS_PREMISES_TOO_LONG = {
add: "add",
beneficialOwners: "123",
[trustType.TrustKey]: `[{
"trust_name": "my trust",
Expand Down Expand Up @@ -431,8 +431,8 @@ export const TRUSTS_SUBMIT_INDIVIDUAL_URA_ADDRESS_PREMISES_TOO_LONG = {
}]`
};

export const TRUSTS_SUBMIT_INDIVIDUAL_SA_ADDRESS_PREMISES_TOO_LONG = {
submit: "submit",
export const TRUSTS_ADD_INDIVIDUAL_SA_ADDRESS_PREMISES_TOO_LONG = {
add: "add",
beneficialOwners: "123",
[trustType.TrustKey]: `[{
"trust_name": "my trust",
Expand Down Expand Up @@ -463,8 +463,8 @@ export const TRUSTS_SUBMIT_INDIVIDUAL_SA_ADDRESS_PREMISES_TOO_LONG = {
}]`
};

export const TRUSTS_SUBMIT_INDIVIDUAL_AND_CORPORATE_NO_ADDRESS_PREMISES = {
submit: "submit",
export const TRUSTS_ADD_INDIVIDUAL_AND_CORPORATE_NO_ADDRESS_PREMISES = {
add: "add",
beneficialOwners: "123",
[trustType.TrustKey]: `[{
"trust_name": "my trust",
Expand Down
Loading