HDPI-4054 Adding caseManagementLocation as top level field#1570
HDPI-4054 Adding caseManagementLocation as top level field#1570SachinPNaikHMCTS wants to merge 9 commits intomasterfrom
Conversation
CCD diff summary👉 Full report: https://github.com/hmcts/pcs-api/actions/runs/23590769287?check_suite_focus=true CaseField.json
AuthorisationCaseField/caseworker-pcs.json
AuthorisationCaseField/caseworker-pcs-solicitor.json
|
tvr-hmcts
left a comment
There was a problem hiding this comment.
Some thoughts.
Please check the formatting on the classes also.
src/main/java/uk/gov/hmcts/reform/pcs/ccd/view/globalsearch/CaseManagementLocationView.java
Outdated
Show resolved
Hide resolved
src/main/java/uk/gov/hmcts/reform/pcs/ccd/view/globalsearch/CaseManagementLocationView.java
Outdated
Show resolved
Hide resolved
|
|
||
| @Component | ||
| @AllArgsConstructor | ||
| @Slf4j |
src/main/java/uk/gov/hmcts/reform/pcs/ccd/view/globalsearch/CaseManagementLocationView.java
Outdated
Show resolved
Hide resolved
src/main/java/uk/gov/hmcts/reform/pcs/ccd/view/globalsearch/CaseManagementLocationView.java
Outdated
Show resolved
Hide resolved
| claimant.getValue().getLastName()) | ||
| .orElse(null)); | ||
| .findFirst() | ||
| .map(ListValue<Party>::getValue) |
There was a problem hiding this comment.
can be interred. Ok to keep if it helps others though I guess.
There was a problem hiding this comment.
It was done as part of review. #1496 (comment)
There was a problem hiding this comment.
Sorry ... typo ... I meant 'infered' ... Kinesis split keyboard pain.
| private CaseNameHmctsFormatter caseNameHmctsFormatter; | ||
| private CaseNameHmctsView caseNameHmctsView; | ||
| @Mock | ||
| CaseManagementLocationView caseManagementLocationView; |
| } | ||
|
|
||
|
|
||
| private String getFormattedValue(final Integer region, final Integer epimsId) { |
|
|
||
|
|
||
| private String getFormattedValue(final Integer region, final Integer epimsId) { | ||
| return String.format("{region:%s,baseLocation:%s}", region, epimsId); |
There was a problem hiding this comment.
Not sure on this approach. Suspect it maybe fragile.
There was a problem hiding this comment.
Sorry I didn't catch that?
There was a problem hiding this comment.
return "{region:%s,baseLocation:%s}".formatted(region, epimsId);
| @Test | ||
| void shouldSetCaseManagementLocationField() { | ||
| // When | ||
| doNothing().when(caseManagementLocationView).setCaseManagementLocationField(any(PCSCase.class)); |
There was a problem hiding this comment.
Minor - why is this needed? Isn't that the default behaviour?
There was a problem hiding this comment.
caseManagementLocationView is a Mock. So when setCaseManagementLocationField is called we wanted to inform mock to do nothing since its a void method.
| } | ||
|
|
||
|
|
||
| private String getFormattedValue(final Integer region, final Integer epimsId) { |
There was a problem hiding this comment.
You know at this stage that the parameters are not null so you might as well make them primitive types.
Plan Result (aat) |
| * | ||
| * @param pcsCase The current case data | ||
| */ | ||
| public void setCaseManagementLocationField(final PCSCase pcsCase) { |
There was a problem hiding this comment.
No direct tests on this method. However I see it is only called internally so it should - private or tested
src/test/java/uk/gov/hmcts/reform/pcs/ccd/view/globalsearch/CaseFieldsViewTest.java
Outdated
Show resolved
Hide resolved
src/test/java/uk/gov/hmcts/reform/pcs/ccd/view/globalsearch/CaseFieldsViewTest.java
Outdated
Show resolved
Hide resolved
src/test/java/uk/gov/hmcts/reform/pcs/ccd/view/globalsearch/CaseFieldsViewTest.java
Outdated
Show resolved
Hide resolved
src/test/java/uk/gov/hmcts/reform/pcs/ccd/view/globalsearch/CaseFieldsViewTest.java
Outdated
Show resolved
Hide resolved
src/main/java/uk/gov/hmcts/reform/pcs/ccd/view/globalsearch/CaseFieldsView.java
Show resolved
Hide resolved
src/main/java/uk/gov/hmcts/reform/pcs/ccd/view/globalsearch/CaseFieldsView.java
Outdated
Show resolved
Hide resolved
src/main/java/uk/gov/hmcts/reform/pcs/ccd/view/globalsearch/CaseFieldsView.java
Outdated
Show resolved
Hide resolved
tvr-hmcts
left a comment
There was a problem hiding this comment.
The refactoring helped :) Though there are still some elements that can be changed up a bit.
tvr-hmcts
left a comment
There was a problem hiding this comment.
Just these two. One a minor naming and the other a clarification as to what I meant in my earlier comment.
Jira link
See PROJ-XXXXXX
Change description
Testing done
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