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.

Sign up for GitHub

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

Feature/uar 1635 save resume deselect nocs #1631

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Feature/uar 1635 save resume deselect nocs #1631

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e9e410a
uar-1635 : beneficial.owner.type - add noc validator and remove inval…
lduranteau Oct 11, 2024
35c4638
uar-1635 : update.beneficial.owner.type - add noc validator and remov…
lduranteau Oct 11, 2024
d354ed2
uar-1635 : add error message and add error tests
lduranteau Oct 14, 2024
e639769
uar-1635 : remove redundant use of `await` on a return value (SonarQube)
lduranteau Oct 14, 2024
5ee40f4
uar-1635 : update functions name
lduranteau Oct 15, 2024
74c6dab
uar-1635 : checkIfBeneficialOwnerHasNOC indentation
lduranteau Oct 15, 2024
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
84 changes: 60 additions & 24 deletions src/controllers/beneficial.owner.type.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,35 +12,19 @@ import {
} from "../model/beneficial.owner.type.model";
import { isActiveFeature } from '../utils/feature.flag';
import { getUrlWithParamsToPath } from "../utils/url";
import { beneficialOwnersTypeEmptyNOCList } from "../validation/async";
mwejuli-ch marked this conversation as resolved.
Show resolved Hide resolved
import { FormattedValidationErrors, formatValidationError, } from "../middleware/validation.middleware";
import { ValidationError } from "express-validator";
import { BeneficialOwnerIndividualKey } from "../model/beneficial.owner.individual.model";
import { BeneficialOwnerOtherKey } from "../model/beneficial.owner.other.model";
import { BeneficialOwnerGovKey } from "../model/beneficial.owner.gov.model";
import { NonLegalFirmNoc } from "../model/data.types.model";

export const get = async (req: Request, res: Response, next: NextFunction) => {
try {
logger.debugRequest(req, `${req.method} ${req.route.path}`);

const appData: ApplicationData = await getApplicationData(req.session);
const requiresTrusts: boolean = checkEntityRequiresTrusts(appData);

logger.infoRequest(req, `${config.BENEFICIAL_OWNER_TYPE_PAGE} requiresTrusts=${requiresTrusts}`);

if (isActiveFeature(config.FEATURE_FLAG_ENABLE_REDIS_REMOVAL)) {
return res.render(config.BENEFICIAL_OWNER_TYPE_PAGE, {
// Even though the value of this feature flag is available in the template via the OE_CONFIGS variable, passing it in
// like this enables unit tests to assert different outcomes, based on whether it is set or not
FEATURE_FLAG_ENABLE_REDIS_REMOVAL: true,
activeSubmissionBasePath: getUrlWithParamsToPath(config.ACTIVE_SUBMISSION_BASE_PATH, req),
backLinkUrl: getUrlWithParamsToPath(config.BENEFICIAL_OWNER_STATEMENTS_WITH_PARAMS_URL, req),
templateName: config.BENEFICIAL_OWNER_TYPE_PAGE,
requiresTrusts,
...appData,
});
}

return res.render(config.BENEFICIAL_OWNER_TYPE_PAGE, {
backLinkUrl: config.BENEFICIAL_OWNER_STATEMENTS_URL,
templateName: config.BENEFICIAL_OWNER_TYPE_PAGE,
requiresTrusts,
...appData,
});
return await getPageRender(req, res);
} catch (error) {
logger.errorRequest(req, error);
next(error);
Expand All @@ -57,14 +41,30 @@ export const postSubmit = async (req: Request, res: Response) => {
const appData: ApplicationData = await getApplicationData(req.session);
const requiresTrusts: boolean = checkEntityRequiresTrusts(appData);
let nextPageUrl = config.CHECK_YOUR_ANSWERS_URL;

const errors = await getValidationErrors(appData, req);

if (errors.length) {
return await getPageRender(req, res, formatValidationError(errors));
}

if (isActiveFeature(config.FEATURE_FLAG_ENABLE_REDIS_REMOVAL)) {
nextPageUrl = getUrlWithParamsToPath(config.CHECK_YOUR_ANSWERS_WITH_PARAMS_URL, req);
}

if (requiresTrusts) {
nextPageUrl = isActiveFeature(config.FEATURE_FLAG_ENABLE_TRUSTS_WEB)
? getTrustLandingUrl(appData, req)
: config.TRUST_INFO_URL;
}

if (isActiveFeature(config.FEATURE_FLAG_ENABLE_PROPERTY_OR_LAND_OWNER_NOC)) {
console.debug("Removing old NOCs");
markpit marked this conversation as resolved.
Show resolved Hide resolved
appData?.[BeneficialOwnerIndividualKey]?.forEach(boi => { delete boi[NonLegalFirmNoc]; });
appData?.[BeneficialOwnerOtherKey]?.forEach(boo => { delete boo[NonLegalFirmNoc]; });
appData?.[BeneficialOwnerGovKey]?.forEach(bog => { delete bog[NonLegalFirmNoc]; });
}

return res.redirect(nextPageUrl);
};

Expand Down Expand Up @@ -96,3 +96,39 @@ const getNextPage = (req: Request): string => {
return config.MANAGING_OFFICER_URL;
}
};

// Get validation errors that depend on an asynchronous request
const getValidationErrors = async (appData: ApplicationData, req: Request): Promise<ValidationError[]> => {
const beneficialOwnersTypeEmptyNOCListErrors = await beneficialOwnersTypeEmptyNOCList(req, appData);

return [...beneficialOwnersTypeEmptyNOCListErrors];
};

const getPageRender = async (req: Request, res: Response, errors?: FormattedValidationErrors) => {
mwestacott marked this conversation as resolved.
Show resolved Hide resolved
const appData: ApplicationData = await getApplicationData(req.session);
const requiresTrusts: boolean = checkEntityRequiresTrusts(appData);

logger.infoRequest(req, `${config.BENEFICIAL_OWNER_TYPE_PAGE} requiresTrusts=${requiresTrusts}`);

if (isActiveFeature(config.FEATURE_FLAG_ENABLE_REDIS_REMOVAL)) {
return res.render(config.BENEFICIAL_OWNER_TYPE_PAGE, {
// Even though the value of this feature flag is available in the template via the OE_CONFIGS variable, passing it in
// like this enables unit tests to assert different outcomes, based on whether it is set or not
FEATURE_FLAG_ENABLE_REDIS_REMOVAL: true,
activeSubmissionBasePath: getUrlWithParamsToPath(config.ACTIVE_SUBMISSION_BASE_PATH, req),
backLinkUrl: getUrlWithParamsToPath(config.BENEFICIAL_OWNER_STATEMENTS_WITH_PARAMS_URL, req),
templateName: config.BENEFICIAL_OWNER_TYPE_PAGE,
requiresTrusts,
...appData,
errors,
});
}

return res.render(config.BENEFICIAL_OWNER_TYPE_PAGE, {
backLinkUrl: config.BENEFICIAL_OWNER_STATEMENTS_URL,
templateName: config.BENEFICIAL_OWNER_TYPE_PAGE,
requiresTrusts,
...appData,
errors,
});
};
8 changes: 0 additions & 8 deletions src/controllers/shared/common.resume.submission.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { getOverseasEntity } from "../../service/overseas.entities.service";
import {
HasSoldLandKey,
ID,
NonLegalFirmNoc,
IsSecureRegisterKey,
OverseasEntityKey,
Transactionkey
Expand Down Expand Up @@ -111,12 +110,5 @@ const setWebApplicationData = (session: Session, appData: ApplicationData, trans
mapTrustApiReturnModelToWebModel(appData);
}

if (isActiveFeature(config.FEATURE_FLAG_ENABLE_PROPERTY_OR_LAND_OWNER_NOC) && (!appData.entity_number)) {
console.debug("Removing old NOCs");
appData[BeneficialOwnerIndividualKey].forEach(boi => { delete boi[NonLegalFirmNoc]; });
appData[BeneficialOwnerOtherKey].forEach(boo => { delete boo[NonLegalFirmNoc]; });
appData[BeneficialOwnerGovKey].forEach(bog => { delete bog[NonLegalFirmNoc]; });
}

setExtraData(session, appData);
};
57 changes: 43 additions & 14 deletions src/controllers/update/beneficial.owner.type.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ import { saveAndContinue } from "../../utils/save.and.continue";
import { Session } from "@companieshouse/node-session-handler";
import { validationResult } from "express-validator/src/validation-result";
import { FormattedValidationErrors, formatValidationError } from "../../middleware/validation.middleware";
import { isActiveFeature } from "../../utils/feature.flag";
import { NonLegalFirmNoc } from "../../model/data.types.model";
import { beneficialOwnersTypeEmptyNOCList } from "../../validation/async";
import { ValidationError } from "express-validator";

type BeneficialOwnerTypePageProperties = {
backLinkUrl: string;
Expand Down Expand Up @@ -62,21 +66,8 @@ const getPageProperties = async (
export const get = async (req: Request, res: Response, next: NextFunction) => {
try {
logger.debugRequest(req, `${req.method} ${req.route.path}`);
const appData: ApplicationData = await getApplicationData(req.session);

const checkIsRedirect = checkAndReviewBeneficialOwner(appData);
if (checkIsRedirect && checkIsRedirect !== "") {
return res.redirect(checkIsRedirect);
}

const checkMoRedirect = checkAndReviewManagingOfficers(appData);
if (checkMoRedirect){
return res.redirect(checkMoRedirect);
}

const pageProps = await getPageProperties(req);

return res.render(pageProps.templateName, pageProps);
return await getPageRender(req, res);
} catch (error) {
logger.errorRequest(req, error);
next(error);
Expand All @@ -103,6 +94,12 @@ export const postSubmit = async (req: Request, res: Response, next: NextFunction

const appData: ApplicationData = await getApplicationData(req.session);

const errors = await getValidationErrors(appData, req);

if (errors.length) {
return await getPageRender(req, res, formatValidationError(errors));
}

if (!appData.update?.trust_data_fetched) {
const session = req.session as Session;
await retrieveTrustData(req, appData);
Expand All @@ -116,6 +113,13 @@ export const postSubmit = async (req: Request, res: Response, next: NextFunction
moveReviewableTrustsIntoReview(appData);
resetReviewStatusOnAllTrustsToBeReviewed(appData);

if (isActiveFeature(config.FEATURE_FLAG_ENABLE_PROPERTY_OR_LAND_OWNER_NOC)) {
console.debug("Removing old NOCs");
markpit marked this conversation as resolved.
Show resolved Hide resolved
appData?.[BeneficialOwnerIndividualKey]?.forEach(boi => { delete boi[NonLegalFirmNoc]; });
appData?.[BeneficialOwnerOtherKey]?.forEach(boo => { delete boo[NonLegalFirmNoc]; });
appData?.[BeneficialOwnerGovKey]?.forEach(bog => { delete bog[NonLegalFirmNoc]; });
}

if (hasTrustsToReview(appData)) {
return res.redirect(config.UPDATE_MANAGE_TRUSTS_INTERRUPT_URL);
}
Expand Down Expand Up @@ -152,3 +156,28 @@ const getNextPage = (beneficialOwnerTypeChoices: BeneficialOwnerTypeChoice | Man
return config.UPDATE_BENEFICIAL_OWNER_INDIVIDUAL_URL;
}
};

// Get validation errors that depend on an asynchronous request
const getValidationErrors = async (appData: ApplicationData, req: Request): Promise<ValidationError[]> => {
const beneficialOwnersTypeEmptyNOCListErrors = await beneficialOwnersTypeEmptyNOCList(req, appData);

return [...beneficialOwnersTypeEmptyNOCListErrors];
};

const getPageRender = async (req: Request, res: Response, errors?: FormattedValidationErrors) => {
mwestacott marked this conversation as resolved.
Show resolved Hide resolved
const appData: ApplicationData = await getApplicationData(req.session);

const checkIsRedirect = checkAndReviewBeneficialOwner(appData);
if (checkIsRedirect && checkIsRedirect !== "") {
return res.redirect(checkIsRedirect);
}

const checkMoRedirect = checkAndReviewManagingOfficers(appData);
if (checkMoRedirect){
return res.redirect(checkMoRedirect);
}

const pageProps = await getPageProperties(req, errors);

return res.render(pageProps.templateName, pageProps);
};
55 changes: 55 additions & 0 deletions src/validation/async/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ import { checkCeasedDateOnOrAfterDateOfBirth, checkCeasedDateOnOrAfterTrustCreat
import { dateContext } from '../../validation/fields/helper/date.validation.helper';
import { checkDatePreviousToFilingDate } from '../../validation/custom.validation';
import isAllowedUrls from './isAllowedUrls';
import { BeneficialOwnerIndividualKey } from '../../model/beneficial.owner.individual.model';
import { BeneficialOwnerOtherKey } from '../../model/beneficial.owner.other.model';
import { BeneficialOwnerGovKey } from '../../model/beneficial.owner.gov.model';

export const checkTrustIndividualCeasedDate = async (appData: ApplicationData, req: Request): Promise<ValidationError[]> => {
const allowedUrls = [
Expand Down Expand Up @@ -152,3 +155,55 @@ export const filingPeriodTrustStartDateValidations = async (req: Request) => is_

export const filingPeriodTrustCeaseDateValidations = async (req: Request) => is_date_within_filing_period_trusts(req, historicalBOEndDateContext, ErrorMessages.CEASED_DATE_BEFORE_FILING_DATE);

export const beneficialOwnersTypeEmptyNOCList = async (req: Request, appData: ApplicationData): Promise<ValidationError[]> => {
const allowedUrls = [
[config.REGISTER_AN_OVERSEAS_ENTITY_URL, config.BENEFICIAL_OWNER_TYPE_PAGE],
[config.UPDATE_AN_OVERSEAS_ENTITY_URL, config.UPDATE_BENEFICIAL_OWNER_TYPE_PAGE]
];

const allowed: boolean = isAllowedUrls(allowedUrls, req);

const errors: ValidationError[] = [];

if (!allowed) {
return errors;
}

try {
const boiList = checkIfBeneficialOwnerHasNOC(appData?.[BeneficialOwnerIndividualKey] ?? []);
const booList = checkIfBeneficialOwnerHasNOC(appData?.[BeneficialOwnerOtherKey] ?? []);
const bogList = checkIfBeneficialOwnerHasNOC(appData?.[BeneficialOwnerGovKey] ?? []);

const boList: string[] = [...boiList, ...booList, ...bogList];

if (boList.length) {
throw new Error(`
markpit marked this conversation as resolved.
Show resolved Hide resolved
${boList.join(", ")}\n
may have no control types assigned. Please click the 'change' link for each owner to specify their control types.
`);
}

return errors;
} catch (error) {
errors.push({
value: '',
msg: error.message,
param: 'beneficial_owner_list',
location: 'body',
});

return errors;
}
};

const checkIfBeneficialOwnerHasNOC = (beneficialOwner): string[] => {
return beneficialOwner?.filter(bo =>
!bo?.beneficial_owner_nature_of_control_types?.length &&
mwestacott marked this conversation as resolved.
Show resolved Hide resolved
!bo?.trustees_nature_of_control_types?.length &&
!bo?.non_legal_firm_control_nature_of_control_types?.length &&
!bo?.trust_control_nature_of_control_types?.length &&
!bo?.owner_of_land_person_nature_of_control_jurisdictions?.length &&
!bo?.owner_of_land_other_entity_nature_of_control_jurisdictions?.length
).map(bo => bo?.first_name ?? bo?.name);
};

53 changes: 53 additions & 0 deletions test/controllers/beneficial.owner.type.controller.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
} from '../__mocks__/text.mock';
import {
APPLICATION_DATA_MOCK,
BENEFICIAL_OWNER_INDIVIDUAL_OBJECT_MOCK,
ERROR
} from '../__mocks__/session.mock';
import { ErrorMessages } from '../../src/validation/error.messages';
Expand All @@ -53,6 +54,8 @@ import { checkEntityRequiresTrusts, getTrustLandingUrl } from "../../src/utils/t
import { serviceAvailabilityMiddleware } from "../../src/middleware/service.availability.middleware";
import { isActiveFeature } from "../../src/utils/feature.flag";
import { getUrlWithParamsToPath } from "../../src/utils/url";
import { beneficialOwnerIndividualType } from "../../src/model";
import { NatureOfControlType } from "../../src/model/data.types.model";

mockCsrfProtectionMiddleware.mockClear();
const mockIsActiveFeature = isActiveFeature as jest.Mock;
Expand Down Expand Up @@ -524,6 +527,56 @@ describe("BENEFICIAL OWNER TYPE controller", () => {
expect(mockGetUrlWithParamsToPath.mock.calls[0][0]).toEqual(config.CHECK_YOUR_ANSWERS_WITH_PARAMS_URL);
});

test(`redirects to the ${config.CHECK_YOUR_ANSWERS_PAGE} page and remove old NOCs after submission and FEATURE_FLAG_ENABLE_PROPERTY_OR_LAND_OWNER_NOC is ON`, async () => {
mockIsActiveFeature.mockReturnValueOnce(true); // For FEATURE_FLAG_ENABLE_REDIS_REMOVAL
mockIsActiveFeature.mockReturnValueOnce(true); // For FEATURE_FLAG_ENABLE_REDIS_REMOVAL
const mockAppData = {
...APPLICATION_DATA_MOCK,
[beneficialOwnerIndividualType.BeneficialOwnerIndividualKey]: [{
...BENEFICIAL_OWNER_INDIVIDUAL_OBJECT_MOCK,
non_legal_firm_members_nature_of_control_types: [NatureOfControlType.APPOINT_OR_REMOVE_MAJORITY_BOARD_DIRECTORS, NatureOfControlType.OVER_25_PERCENT_OF_SHARES]
}, BENEFICIAL_OWNER_INDIVIDUAL_OBJECT_MOCK ]
};
mockGetApplicationData.mockReturnValue(mockAppData);
mockcheckEntityRequiresTrusts.mockReturnValueOnce(false);
const resp = await request(app)
.post(config.BENEFICIAL_OWNER_TYPE_SUBMIT_URL);

expect(resp.status).toEqual(302);
expect(resp.text).toContain(MOCKED_URL);
expect(resp.header.location).toEqual(MOCKED_URL);
expect(mockGetUrlWithParamsToPath).toHaveBeenCalledTimes(1);
expect(mockGetUrlWithParamsToPath.mock.calls[0][0]).toEqual(config.CHECK_YOUR_ANSWERS_WITH_PARAMS_URL);

expect(mockAppData.beneficial_owners_individual?.[0].non_legal_firm_members_nature_of_control_types).toEqual(undefined);
expect(mockAppData.beneficial_owners_individual?.[1].non_legal_firm_members_nature_of_control_types).toEqual(undefined);
});

test(`redirects to the ${config.CHECK_YOUR_ANSWERS_PAGE} page and not remove old NOCs after submission and FEATURE_FLAG_ENABLE_PROPERTY_OR_LAND_OWNER_NOC is OFF`, async () => {
mockIsActiveFeature.mockReturnValueOnce(true); // For FEATURE_FLAG_ENABLE_REDIS_REMOVAL
mockIsActiveFeature.mockReturnValueOnce(false); // For FEATURE_FLAG_ENABLE_REDIS_REMOVAL
const mockAppData = {
...APPLICATION_DATA_MOCK,
[beneficialOwnerIndividualType.BeneficialOwnerIndividualKey]: [{
...BENEFICIAL_OWNER_INDIVIDUAL_OBJECT_MOCK,
non_legal_firm_members_nature_of_control_types: [NatureOfControlType.APPOINT_OR_REMOVE_MAJORITY_BOARD_DIRECTORS, NatureOfControlType.OVER_25_PERCENT_OF_SHARES]
}, BENEFICIAL_OWNER_INDIVIDUAL_OBJECT_MOCK ]
};
mockGetApplicationData.mockReturnValue(mockAppData);
mockcheckEntityRequiresTrusts.mockReturnValueOnce(false);
const resp = await request(app)
.post(config.BENEFICIAL_OWNER_TYPE_SUBMIT_URL);

expect(resp.status).toEqual(302);
expect(resp.text).toContain(MOCKED_URL);
expect(resp.header.location).toEqual(MOCKED_URL);
expect(mockGetUrlWithParamsToPath).toHaveBeenCalledTimes(1);
expect(mockGetUrlWithParamsToPath.mock.calls[0][0]).toEqual(config.CHECK_YOUR_ANSWERS_WITH_PARAMS_URL);

expect(mockAppData.beneficial_owners_individual?.[0].non_legal_firm_members_nature_of_control_types).toEqual(
[NatureOfControlType.APPOINT_OR_REMOVE_MAJORITY_BOARD_DIRECTORS, NatureOfControlType.OVER_25_PERCENT_OF_SHARES]);
});

test(`renders the current page with error message ${BeneficialOwnersStatementType.SOME_IDENTIFIED_ALL_DETAILS} has both beneficial owner and managing officer with trusts`, async () => {
mockIsActiveFeature.mockReturnValueOnce(true); // For FEATURE_FLAG_ENABLE_REDIS_REMOVAL
mockGetApplicationData.mockReturnValueOnce(APPLICATION_DATA_MOCK);
Expand Down
Loading