-
Notifications
You must be signed in to change notification settings - Fork 3
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
UAR-1502 Make 'property name or number' field optional #1376
base: main
Are you sure you want to change the base?
Conversation
* data entry in this field is now optional for the Update and Remove journeys * a value must still be entered if on the Registration journey Change is for a spike - no unit-tests added
}; | ||
|
||
export const checkPresenceOfPropertyNameOrNumber = (propertyNameOrNumber: string, req) => { | ||
if (propertyNameOrNumber === undefined || propertyNameOrNumber === "") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I copied the approach taken for the 'email' field here, but if we do ever implement this, we may wish to trim
the value too before testing it.
looks ok to me |
export const overseas_entity_address_validations = (errors: ErrorMessagesOptional = defaultOptionalErrorMessages) => { | ||
errors = { ...defaultOptionalErrorMessages, ...errors }; | ||
return [ | ||
body("principal_address_property_name_number") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seeing as this returns an a list of validators, some might be shared with the other lists, could we have a common list and use ... to import the common validations into the list mixed with the specific validations for that field/page, or wold that mean refactoring all the address validations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't know, but a sensible suggestion. We should investigate if this becomes a 'real' changes.
JIRA link
https://companieshouse.atlassian.net/browse/UAR-1502
Change description
Change is for a spike - no unit-tests added
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.