Skip to content
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.

Sign up for GitHub

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

Jump to bottom

SSCSCI-1137 Address logic changes #4157

Merged
merged 20 commits into from
Dec 13, 2024
Merged

SSCSCI-1137 Address logic changes #4157

merged 20 commits into from
Dec 13, 2024

Conversation

linusnorton
Copy link
Contributor

Jira link

See PROJ-XXXXXX

Change description

Testing done

Checklist

  • commit messages are meaningful and follow good commit message guidelines
  • README and other documentation has been updated / added (if needed)
  • tests have been updated / new tests has been added (if needed)
  • Does this PR introduce a breaking change

@linusnorton linusnorton requested a review from a team as a code owner November 19, 2024 13:14
@github-actions GitHub Actions
Copy link

CCD diff report

No change

@hmcts-jenkins-j-to-z hmcts-jenkins-j-to-z bot requested a deployment to preview November 19, 2024 13:22 Abandoned
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 19, 2024 •
edited
Loading

Integration Tests results

   82 files  ±0     82 suites  ±0   9m 19s ⏱️ -25s
1 424 tests ±0  1 416 ✅ ±0  8 💤 ±0  0 ❌ ±0 
1 425 runs  ±0  1 417 ✅ ±0  8 💤 ±0  0 ❌ ±0 

Results for commit bbda289. ± Comparison against base commit 1191ec5.

This pull request removes 9 and adds 9 tests. Note that renamed tests count towards both. 
uk.gov.hmcts.reform.sscs.tyanotifications.config.BundledNotificationsConfigTestIt ‑ given_bundledLetters_should_notHaveTemplate(DIRECTION_ISSUED) [3]
uk.gov.hmcts.reform.sscs.tyanotifications.config.BundledNotificationsConfigTestIt ‑ given_bundledLetters_should_notHaveTemplate(DIRECTION_ISSUED_WELSH) [4]
uk.gov.hmcts.reform.sscs.tyanotifications.config.BundledNotificationsConfigTestIt ‑ given_bundledLetters_should_notHaveTemplate(LIBERTY_TO_APPLY_GRANTED) [9]
uk.gov.hmcts.reform.sscs.tyanotifications.config.BundledNotificationsConfigTestIt ‑ given_bundledLetters_should_notHaveTemplate(LIBERTY_TO_APPLY_REFUSED) [7]
uk.gov.hmcts.reform.sscs.tyanotifications.config.BundledNotificationsConfigTestIt ‑ given_bundledLetters_should_notHaveTemplate(PROCESS_AUDIO_VIDEO) [6]
uk.gov.hmcts.reform.sscs.tyanotifications.config.BundledNotificationsConfigTestIt ‑ given_bundledLetters_should_notHaveTemplate(PROCESS_AUDIO_VIDEO_WELSH) [0]
uk.gov.hmcts.reform.sscs.tyanotifications.config.BundledNotificationsConfigTestIt ‑ given_bundledLetters_should_notHaveTemplate(SET_ASIDE_GRANTED) [5]
uk.gov.hmcts.reform.sscs.tyanotifications.config.BundledNotificationsConfigTestIt ‑ given_bundledLetters_should_notHaveTemplate(SOR_EXTEND_TIME) [8]
uk.gov.hmcts.reform.sscs.tyanotifications.config.BundledNotificationsConfigTestIt ‑ given_bundledLetters_should_notHaveTemplate(SOR_REFUSED) [1]

uk.gov.hmcts.reform.sscs.tyanotifications.config.BundledNotificationsConfigTestIt ‑ given_bundledLetters_should_notHaveTemplate(DIRECTION_ISSUED) [1]
uk.gov.hmcts.reform.sscs.tyanotifications.config.BundledNotificationsConfigTestIt ‑ given_bundledLetters_should_notHaveTemplate(DIRECTION_ISSUED_WELSH) [8]
uk.gov.hmcts.reform.sscs.tyanotifications.config.BundledNotificationsConfigTestIt ‑ given_bundledLetters_should_notHaveTemplate(LIBERTY_TO_APPLY_GRANTED) [5]
uk.gov.hmcts.reform.sscs.tyanotifications.config.BundledNotificationsConfigTestIt ‑ given_bundledLetters_should_notHaveTemplate(LIBERTY_TO_APPLY_REFUSED) [9]
uk.gov.hmcts.reform.sscs.tyanotifications.config.BundledNotificationsConfigTestIt ‑ given_bundledLetters_should_notHaveTemplate(PROCESS_AUDIO_VIDEO) [3]
uk.gov.hmcts.reform.sscs.tyanotifications.config.BundledNotificationsConfigTestIt ‑ given_bundledLetters_should_notHaveTemplate(PROCESS_AUDIO_VIDEO_WELSH) [7]
uk.gov.hmcts.reform.sscs.tyanotifications.config.BundledNotificationsConfigTestIt ‑ given_bundledLetters_should_notHaveTemplate(SET_ASIDE_GRANTED) [6]
uk.gov.hmcts.reform.sscs.tyanotifications.config.BundledNotificationsConfigTestIt ‑ given_bundledLetters_should_notHaveTemplate(SOR_EXTEND_TIME) [0]
uk.gov.hmcts.reform.sscs.tyanotifications.config.BundledNotificationsConfigTestIt ‑ given_bundledLetters_should_notHaveTemplate(SOR_REFUSED) [4]

♻️ This comment has been updated with latest results.

@hmcts-jenkins-j-to-z hmcts-jenkins-j-to-z bot requested a deployment to preview November 19, 2024 13:50 Abandoned
@hmcts-jenkins-j-to-z hmcts-jenkins-j-to-z bot requested a deployment to preview November 19, 2024 14:18 Abandoned
benouaer
benouaer reviewed Dec 13, 2024
View reviewed changes
...esubmit/furtherevidence/actionfurtherevidence/ActionFurtherEvidenceAboutToSubmitHandler.java
return true;
}

var isMainlandUkAndMissingPostcode = !YesNo.NO.equals(address.getInMainlandUk()) && isBlank(address.getPostcode());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is inMainlandUK question optional? because if it is. we're not always going to get the No if they don't answer it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, it's crafted carefully, so that NULL or Yes give the desired behaviour

benouaer
benouaer reviewed Dec 13, 2024
View reviewed changes
src/main/java/uk/gov/hmcts/reform/sscs/service/coversheet/CoversheetService.java
@@ -47,15 +48,20 @@ public Optional<byte[]> createCoverSheet(String identifier) {
String derivedTemplate = documentConfiguration.getEvidence()
.get(sscsCaseData.getLanguagePreference()).get(TEMPLATE);
Address address = sscsCaseData.getAppeal().getAppellant().getAddress();
var lines = LetterUtils.lines(address);
for (int i = lines.size(); i < 5; i++) {
lines.add("");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure I understand what this line is doing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

padding it to ensure there are 5 lines even if they are blank

benouaer
benouaer reviewed Dec 13, 2024
View reviewed changes
...k/gov/hmcts/reform/sscs/transform/deserialize/SubmitYourAppealToCcdCaseDataDeserializer.java
Address.AddressBuilder addressBuilder = Address.builder()
.line1(contactDetails.getAddressLine1())
.line2(contactDetails.getAddressLine2())
.town(contactDetails.getTownCity());
if (isIbc && contactDetails.getInMainlandUk() != null) {
.town(contactDetails.getTownCity())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to include addressLine3 or is it not relevant in this context
see bug ticket https://tools.hmcts.net/jira/browse/SSCSCI-1474

@linusnorton linusnorton merged commit 13b7456 into master Dec 13, 2024
9 checks passed
@linusnorton linusnorton deleted the address-logic-changes branch December 13, 2024 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benouaer benouaer benouaer left review comments

@nilay913 nilay913 nilay913 approved these changes

Assignees
No one assigned
Labels
enable_keep_helm ns:sscs prd:sscs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@linusnorton @benouaer @nilay913 @Ziziou91