HDPI-5493 - Create Document Type list dynamically for England and Wales specific#1850
HDPI-5493 - Create Document Type list dynamically for England and Wales specific#1850guygrewal77 wants to merge 9 commits into
Conversation
… differences between England and Wales.
CCD diff summary👉 Full report: https://github.com/hmcts/pcs-api/actions/runs/26235997915?check_suite_focus=true CaseField.json
AuthorisationCaseField/caseworker-pcs-solicitor.json
ComplexTypes/AdditionalDocuments.json
CaseEventToFields/resumePossessionClaim.json
FixedLists/AdditionalDocumentType.json
|
guygrewal77
requested review from
a team,
AmandaRichards,
benouaer,
gmmagruder and
paddy-hmcts
| LegislativeCountry legislativeCountry = caseData.getLegislativeCountry(); | ||
|
|
||
| additionalDocuments.setDocumentTypeList(createAdditionalDocumentList(legislativeCountry)); | ||
| caseData.setAdditionalDocs(new ArrayList<>()); |
There was a problem hiding this comment.
Could we add a test for selecting NO here? The mid-event currently initialises additionalDocs regardless of the answer, so a test would confirm no blank upload row is created when the user does not want to upload documents.
|
|
||
| public static boolean isEnglandSpecific(AdditionalDocumentType val) { | ||
| return val == TENANCY_AGREEMENT; | ||
| } |
There was a problem hiding this comment.
I think a more elegant way to achieve this is for the enum constructor to take a Set of the legislative countries for which it applies. See ClaimantType for an example.
It also makes it easier to build up the dynamic list to show based on the legislative country.
| label = "Add document", | ||
| hint = "Upload a document to the system" | ||
| ) | ||
| private List<ListValue<AdditionalDocuments>> additionalDocs; |
There was a problem hiding this comment.
This doesn't feel very clean to have another field which has a similar name to the one above. I presume you're only doing it to prevent an issue with the ES index but I think at this stage it would be better to make the change properly and be ready to drop the ES index since we're not in Prod yet.
Also it would be better to re-use the existing AdditionalDocument class, rather than making an AdditionalDocuments class with an 's', even though it still only contains one document 🙂
| if (lastDot >= 0 && lastDot < uploadedFilename.length() - 1) { | ||
| fileExtension = uploadedFilename.substring(lastDot + 1); | ||
| fileName = uploadedFilename.substring(0, lastDot); | ||
| } |
There was a problem hiding this comment.
You can use FilenameUtils.getBaseName and FilenameUtils.getExtension here to save having to implement it yourself.
tvr-hmcts
left a comment
tvr-hmcts
left a comment
There was a problem hiding this comment.
LGTM now. Just a minor and I saw Scotts comment while looking at this which I think is worth the consideration.
| private ComponentLauncher caseFileView; | ||
|
|
||
| @CCD(searchable = false) | ||
|
|
4 participants
Jira link
See HDPI-5493
Change description
The Document List will be different for England and Wales and so will be created dynamically in the Upload Additional documents page.
Testing done
XUI testing to ensure the Document Types are in the right order and country specific, and also the documents are placed in the correct folders in Case File View.
Security Vulnerability Assessment
CVE Suppression: Are there any CVEs present in the codebase (either newly introduced or pre-existing) that are being intentionally suppressed or ignored by this commit?
Checklist