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.

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

Dfr 2689 auto date pension sharing annex #1944

Open
wants to merge 64 commits into
base: master
Choose a base branch
from

Conversation

dawudgovuk
Copy link
Contributor

Jira link

https://tools.hmcts.net/jira/browse/DFR-2689

Change description

Auto date document - pension sharing annex

@hmcts-jenkins-d-to-i
Copy link
Contributor

Plan Result (aat)

No changes. Your infrastructure matches the configuration.

@hmcts-jenkins-d-to-i
Copy link
Contributor

Plan Result (prod)

No changes. Your infrastructure matches the configuration.

String authToken,
LocalDate approvalDate,
String caseId) throws Exception {
log.info("Adding date stamp to Pension Sharing Annex : {}", document);
Copy link
Contributor

@ptrelease ptrelease Nov 21, 2024

Choose a reason for hiding this comment

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

Consider if CaseDocument isn't log safe. For instance, if document_filename contains anything that could identify an individual. You could refer to case Id too as you've passed in.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you decide on something log safe, consider including it on your DocumentStorageException below. Consider including the passed in case id too.

throw new StampDocumentException("Pension Order document PDF is flattened / not editable.");
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like textBox.getDefaultAppearance() returns a string. So you could try isBlank rather than the null and empty check.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd pass case id in, so I could include in the exception. Just slightly easier for support :-D

@@ -141,7 +141,7 @@ void given_case_when_NotPaperApplication_then_shouldNotTriggerConsentOrderApprov
}

@Test
void givenCase_whenNoPendsion_thenShouldUpdateStateToConsentOrderMadeAndBulkPrint() {
void givenCase_whenNoPension_thenShouldUpdateStateToConsentOrderMadeAndBulkPrint() {
CallbackRequest callbackRequest =
doValidCaseDataSetUp(NO_PENSION_VALID_JSON);
whenServiceGeneratesDocument().thenReturn(caseDocument());
Copy link
Contributor

Choose a reason for hiding this comment

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

New function getApprovalDate has been added to this handler. That can either take caseData with a LocalDate or String and responds with a null or OK string. Given that, suggest a test.

PDField field = acroForm.get().getField(FORM_P1_DATE_OF_ORDER_TEXTBOX_NAME);
PDTextField textBox = (PDTextField) field;
if (textBox.getDefaultAppearance() == null && textBox.getDefaultAppearance().isEmpty()) {
textBox.setDefaultAppearance(DEFAULT_PDTYPE_PREFIX + DEFAULT_PDTYPE_FONT_HELV + DEFAULT_PDTYPE_POSTFIX);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just use a single constant for the default appearance (it can be private too)

Copy link

github-actions bot commented Dec 4, 2024

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants