Skip to content

Commit

Permalink
Merge pull request #122 from companieshouse/feature/idva6-1702-bug-fi…
Browse files Browse the repository at this point in the history
…x-add-user-page-errors-clear

IDVA6-1702 - Bug Fix - Language change removes 'add a user' page errors
  • Loading branch information
bwallace-ch authored Oct 18, 2024
2 parents d6e8f44 + 9e8925e commit 734187e
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 11 deletions.
6 changes: 2 additions & 4 deletions src/lib/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ export const INTERNAL_API_URL = getEnvironmentValue("INTERNAL_API_URL");
export const ACCOUNTS_USER_INTERNAL_API_KEY = getEnvironmentValue("ACCOUNTS_USER_INTERNAL_API_KEY");
export const ACCOUNT_URL = getEnvironmentValue("ACCOUNT_URL");
export const COOKIE_SECURE_ONLY = getEnvironmentValue("COOKIE_SECURE_ONLY");
export const MATOMO_ADD_USER_GOAL_ID = ():string => getEnvironmentValue("MATOMO_ADD_USER_GOAL_ID");
export const MATOMO_REMOVE_USER_GOAL_ID = ():string => getEnvironmentValue("MATOMO_REMOVE_USER_GOAL_ID");
export const MATOMO_ADD_USER_GOAL_ID = (): string => getEnvironmentValue("MATOMO_ADD_USER_GOAL_ID");
export const MATOMO_REMOVE_USER_GOAL_ID = (): string => getEnvironmentValue("MATOMO_REMOVE_USER_GOAL_ID");

export const SERVICE_NAME = "ACSP Manage Users Web";

Expand All @@ -96,8 +96,6 @@ export const MEMBER_ALREADY_REMOVED_ERROR = "Member already removed";
export const ERRORS_ENTER_AN_EMAIL_ADDRESS_IN_THE_CORRECT_FORMAT = "errors_enter_an_email_address_in_the_correct_format";

// query params
export const CLEAR_FORM = "cf";
export const CLEAR_FORM_TRUE = "?cf=true";
export const OWNER_PAGE_QUERY_PARAM = "ownerPage";
export const ADMIN_PAGE_QUERY_PARAM = "adminPage";
export const STANDARD_PAGE_QUERY_PARAM = "standardPage";
Expand Down
4 changes: 2 additions & 2 deletions src/lib/validation/clear.form.validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ export function validateClearForm (clearForm: string): boolean {
}
}

export const clearFormSessionValues = (req: Request, sessionKey: string): void => {
if (validateClearForm(req.query.cf as string)) {
export const clearFormSessionValues = (req: Request, sessionKey: string, referrer: string | undefined, hrefA: string): void => {
if (referrer?.includes(hrefA) || referrer === undefined) {
deleteExtraData(req.session, sessionKey);
}
};
6 changes: 5 additions & 1 deletion src/routers/controllers/addUserController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import { AcspMembership } from "private-api-sdk-node/dist/services/acsp-manage-u
export const addUserControllerGet = async (req: Request, res: Response): Promise<void> => {
const loggedInUserMembership: AcspMembership = getExtraData(req.session, constants.LOGGED_USER_ACSP_MEMBERSHIP);
const loggedInUserRole = loggedInUserMembership.userRole;
const referrer: string | undefined = req.get("Referrer");
const hrefA = constants.MANAGE_USERS_FULL_URL;

const viewData: ViewData = {
lang: getTranslationsForView(req.lang, constants.ADD_USER_PAGE),
Expand All @@ -23,7 +25,9 @@ export const addUserControllerGet = async (req: Request, res: Response): Promise
loggedInUserRole,
templateName: constants.ADD_USER_PAGE
};
clearFormSessionValues(req, constants.DETAILS_OF_USER_TO_ADD);

clearFormSessionValues(req, constants.DETAILS_OF_USER_TO_ADD, referrer, hrefA);

const savedNewUserDetails = getExtraData(
req.session,
constants.DETAILS_OF_USER_TO_ADD
Expand Down
2 changes: 1 addition & 1 deletion src/routers/controllers/manageUsersController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export const getViewData = async (req: Request): Promise<AnyRecord> => {
const viewData: AnyRecord = {
lang: translations,
backLinkUrl: constants.DASHBOARD_FULL_URL,
addUserUrl: constants.ADD_USER_FULL_URL + constants.CLEAR_FORM_TRUE,
addUserUrl: constants.ADD_USER_FULL_URL,
companyName: acspName,
companyNumber: acspNumber,
loggedInUserRole: userRole,
Expand Down
60 changes: 58 additions & 2 deletions test/src/routers/controllers/addUserController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,28 +78,84 @@ describe(`GET ${url}`, () => {
session.setExtraData(constants.DETAILS_OF_USER_TO_ADD, {
email: invalidEmail
});
mocks.mockSessionMiddleware.mockImplementationOnce((req: Request, res: Response, next: NextFunction) => {
req.headers = { referrer: "/authorised-agent/add-user" };
req.session = session;
next();
});
// When
const response = await router.get(`${url}`);
// Then
expect(response.text).toContain("Enter an email address in the correct format");
expect(response.text).toContain(invalidEmail);
});

it("should not display saved session values when url has cf query param - /authorised-agent/add-user?cf=true", async () => {
it("should not display saved session values when the referrer is the manage-users url", async () => {
// Given
sessionUtilsSpy.mockReturnValue("demo@ch.gov.uk");
const emailStoredInSession = "bob@bob.com";
session.setExtraData(constants.DETAILS_OF_USER_TO_ADD, {
email: emailStoredInSession
});

mocks.mockSessionMiddleware.mockImplementationOnce((req: Request, res: Response, next: NextFunction) => {
req.headers = { referrer: "/authorised-agent/manage-users" };
req.session = session;
next();
});

// When
const response = await router.get(`${url}?cf=true`);
const response = await router.get(`${url}`);

// Then
expect(response.text).not.toContain(emailStoredInSession);
expect(response.text).not.toContain("Enter an email address in the correct format");
expect(response.text).toContain(en.page_header);
expect(response.text).toContain(en.option_1);
});

it("should not display saved session values when the referrer is undefined", async () => {
// Given
sessionUtilsSpy.mockReturnValue("demo@ch.gov.uk");
const emailStoredInSession = "bob@bob.com";
session.setExtraData(constants.DETAILS_OF_USER_TO_ADD, {
email: emailStoredInSession
});

mocks.mockSessionMiddleware.mockImplementationOnce((req: Request, res: Response, next: NextFunction) => {
req.headers = { referrer: undefined };
req.session = session;
next();
});

// When
const response = await router.get(`${url}`);

// Then
expect(response.text).not.toContain(emailStoredInSession);
expect(response.text).not.toContain("Enter an email address in the correct format");
expect(response.text).toContain(en.page_header);
expect(response.text).toContain(en.option_1);
});

it("should display page with error message instead of clearing session data if referrer url contains hrefB (user has switched languages)", async () => {
// Given
const invalidEmail = "bad email";
session.setExtraData(constants.DETAILS_OF_USER_TO_ADD, {
email: invalidEmail
});
const newUrl = "/authorised-agent/add-user?lang=en";
mocks.mockSessionMiddleware.mockImplementationOnce((req: Request, res: Response, next: NextFunction) => {
req.headers = { referrer: "/authorised-agent/add-user?lang=cy" };
req.session = session;
next();
});
// When
const response = await router.get(`${newUrl}`);
// Then
expect(response.text).toContain("Enter an email address in the correct format");
expect(response.text).toContain(invalidEmail);
});
});

describe(`POST ${url}`, () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ describe("manageUsersController - getViewData", () => {
}

]],
addUserUrl: "/authorised-agent/add-user?cf=true",
addUserUrl: "/authorised-agent/add-user",
backLinkUrl: "/authorised-agent/",
companyName: "Acme ltd",
companyNumber: "123456",
Expand Down

0 comments on commit 734187e

Please sign in to comment.