Uar 1285 need changes entity before removed #1233
Conversation
…now middleware has been updated
|
SonarQube Quality Gate
|
| @@ -146,6 +146,7 @@ export const UPDATE_STATEMENT_VALIDATION_ERRORS_PAGE = "update-statement-validat | |||
| export const REMOVE_SOLD_ALL_LAND_FILTER_PAGE = "remove-sold-all-land-filter"; | |||
| export const REMOVE_IS_ENTITY_REGISTERED_OWNER_PAGE = "remove-is-entity-registered-owner"; | |||
| export const REMOVE_CANNOT_USE_PAGE = "remove-cannot-use"; | |||
| export const REMOVE_NEED_MAKE_CHANGES_PAGE = "remove-need-to-make-changes-to-entity"; | |||
There was a problem hiding this comment.
It's not a great choice of words for a constant name but totally understand that you've tried to match the defined HTML page name. How about if you drop the _MAKE_ so we're left with just REMOVE_NEED_CHANGES_PAGE?
(REMOVE_CHANGES_REQUIRED_PAGE would be my first choice, but then we diverge from the HTML page name)
| logger.debugRequest(req, `POST ${config.REMOVE_NEED_MAKE_CHANGES_PAGE}`); | ||
|
|
||
| if (req.body["make_changes"] === 'yes') { | ||
| return res.redirect(`${config.WHO_IS_MAKING_UPDATE_URL}${config.JOURNEY_REMOVE_QUERY_PARAM}`); |
There was a problem hiding this comment.
We don't need to pass the JOURNEY_REMOVE_QUERY_PARAM in now for future pages as it is the update journey and we are avoiding changing all those controllers (for now).
| return res.redirect(`${config.WHO_IS_MAKING_UPDATE_URL}${config.JOURNEY_REMOVE_QUERY_PARAM}`); | ||
| } | ||
| // TODO redirect to new component - Has the overseas entity identified any registrable beneficial owners? | ||
| return res.redirect(`${config.UPDATE_LANDING_URL}${config.JOURNEY_REMOVE_QUERY_PARAM}`); |
There was a problem hiding this comment.
From Mural board, looks like the 'No' option takes you to an existing update screen, so in theory this could be done now (no need for TODO).
| logger.debugRequest(req, `GET ${config.REMOVE_NEED_MAKE_CHANGES_PAGE}`); | ||
| return res.render(config.REMOVE_NEED_MAKE_CHANGES_PAGE, { | ||
| journey: config.JourneyType.remove, | ||
| backLinkUrl: `${config.OVERSEAS_ENTITY_PRESENTER_URL}${config.JOURNEY_REMOVE_QUERY_PARAM}`, |
There was a problem hiding this comment.
In theory, back link here doesn't need to set the JOURNEY_REMOVE_QUERY_PARAM because by this point the 'is_remove=true' flag is set in the session and Mongo JSON, so that screen can pick it up from there (changes will be required for this though...). A discussion required, perhaps.
| @@ -0,0 +1,30 @@ | |||
| import { NextFunction, Request, Response } from "express"; | |||
There was a problem hiding this comment.
Given my comments below and the fact that this is basically just an update screen, I wonder if the new controller for remove (and HTML template page) is required.
| @@ -199,6 +199,7 @@ export enum ErrorMessages { | |||
| SELECT_DO_YOU_WANT_TO_MAKE_CHANGES_UPDATE_STATEMENT = "Select yes if all the information in this update statement is correct", | |||
| SELECT_UPDATE_STATEMENT_VALIDATION_RESOLUTION = "Select if you want to change your statements or change the information provided in this update", | |||
| SELECT_IF_REMOVE_SOLD_ALL_LAND_FILTER = "Select yes if the entity has disposed of all its property or land in the UK", | |||
| SELECT_REMOVE_NEED_TO_MAKE_CHANGES = "Select yes if you need to make changes to this overseas entity before it is removed", | |||
There was a problem hiding this comment.
If the validation error message is the only thing changing here for the remove journey, I think we can probably use the isRemove() check to vary this.
| @@ -250,6 +250,7 @@ export const UPDATE_MANAGE_TRUSTS_REVIEWED_HEADING = "What you have reviewed"; | |||
| export const REMOVE_SOLD_ALL_LAND_FILTER_PAGE_TITLE = "Has the overseas entity disposed of all its property or land in the UK?"; | |||
| export const REMOVE_IS_ENTITY_REGISTERED_OWNER_TITLE = "Is the overseas entity currently listed on any land registry records as the registered owner of property or land in the UK?"; | |||
| export const REMOVE_INTERRUPT_CARD_TEXT = "Once an overseas entity is removed"; | |||
| export const REMOVE_NEED_MAKE_CHANGES_TITLE = "Do you need to make any changes to this overseas entity before it is removed?"; | |||
There was a problem hiding this comment.
Again, we should be able to vary the title displayed in the existing HTML template without the need for a new controller and template for Remove.
| @@ -0,0 +1,76 @@ | |||
| {% extends "update-layout.html" %} | |||
There was a problem hiding this comment.
Potentially, no new template required here - see earlier comments and take a look at update-interrupt-card.html where an existing Update screen content is varied depending on the journey.
|
After a discussion with @mwestacott, it appears as though the update and remove screens are identical in terms of routing and navigation and almost identical in terms of the template, apart from the title and some hints. Therefore, it will be better to modify the existing update page with conditionals similar to the the presenter page and those before it rather than adding a new template and new controllers. |
JIRA link
https://companieshouse.atlassian.net/browse/UAR-1285
Change description
Template and underlying functionality for 'Do you need to make any changes to this overseas entity before it is removed?'
Work checklist
Merge instructions
We are committed to keeping commit history clean, consistent and linear. To achieve this, this commit should be structured as follows:
and contain the following structural elements:
BREAKING CHANGE:introduces a breaking API change (correlating with MAJOR in semantic versioning). A BREAKING CHANGE can be part of commits of any type,fix:andfeat:are allowed, for examplebuild:,chore:,ci:,docs:,style:,refactor:,perf:,test:, and others,BREAKING CHANGE: <description>may be provided.