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

SSCSCI-1588 #4323

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

SSCSCI-1588 #4323

wants to merge 6 commits into from

Conversation

nilay913
Copy link
Contributor

Jira link

See SSCSCI-1588

Change description

Pointing to new templates to replace hardcoded "NI No" with <> placeholder for draft and final decision notice

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

@nilay913 nilay913 requested a review from a team as a code owner January 22, 2025 15:01
Copy link

CCD diff report

No change

Copy link

CCD diff report

No change

Copy link

github-actions bot commented Jan 22, 2025

Integration Tests results

   80 files  ±0     80 suites  ±0   12m 46s ⏱️ -45s
1 423 tests ±0  1 415 ✅ ±0  8 💤 ±0  0 ❌ ±0 
1 424 runs  ±0  1 416 ✅ ±0  8 💤 ±0  0 ❌ ±0 

Results for commit 21e8822. ± Comparison against base commit 2a38b5e.

This pull request removes 8 and adds 8 tests. Note that renamed tests count towards both.
uk.gov.hmcts.reform.sscs.config.DocumentConfigurationTest ‑ testDocumnetConfig(ENGLISH, CORRECTION_GRANTED, Optional.empty, TB-SCS-GNO-ENG-corrected-decision-notice.docx) [0]
uk.gov.hmcts.reform.sscs.config.DocumentConfigurationTest ‑ testDocumnetConfig(ENGLISH, DECISION_ISSUED, Optional.empty, TB-SCS-GNO-ENG-draft-decision-notice.docx) [1]
uk.gov.hmcts.reform.sscs.config.DocumentConfigurationTest ‑ testDocumnetConfig(ENGLISH, DIRECTION_ISSUED, Optional.empty, TB-SCS-GNO-ENG-directions-notice.docx) [2]
uk.gov.hmcts.reform.sscs.config.DocumentConfigurationTest ‑ testDocumnetConfig(ENGLISH, ISSUE_FINAL_DECISION, Optional.empty, TB-SCS-GNO-ENG-final-decision-notice.docx) [3]
uk.gov.hmcts.reform.sscs.config.DocumentConfigurationTest ‑ testDocumnetConfig(WELSH, CORRECTION_GRANTED, Optional.empty, TB-SCS-GNO-ENG-corrected-decision-notice.docx) [4]
uk.gov.hmcts.reform.sscs.config.DocumentConfigurationTest ‑ testDocumnetConfig(WELSH, DECISION_ISSUED, Optional.empty, TB-SCS-GNO-WEL-00473.docx) [5]
uk.gov.hmcts.reform.sscs.config.DocumentConfigurationTest ‑ testDocumnetConfig(WELSH, DIRECTION_ISSUED, Optional.empty, TB-SCS-GNO-WEL-00473.docx) [6]
uk.gov.hmcts.reform.sscs.config.DocumentConfigurationTest ‑ testDocumnetConfig(WELSH, ISSUE_FINAL_DECISION, Optional.empty, TB-SCS-GNO-ENG-final-decision-notice.docx) [7]
uk.gov.hmcts.reform.sscs.config.DocumentConfigurationTest ‑ testDocumentConfig(ENGLISH, CORRECTION_GRANTED, Optional.empty, TB-SCS-GNO-ENG-corrected-decision-notice.docx) [0]
uk.gov.hmcts.reform.sscs.config.DocumentConfigurationTest ‑ testDocumentConfig(ENGLISH, DECISION_ISSUED, Optional.empty, TB-SCS-GNO-ENG-draft-decision-notice-v2.docx) [1]
uk.gov.hmcts.reform.sscs.config.DocumentConfigurationTest ‑ testDocumentConfig(ENGLISH, DIRECTION_ISSUED, Optional.empty, TB-SCS-GNO-ENG-directions-notice.docx) [2]
uk.gov.hmcts.reform.sscs.config.DocumentConfigurationTest ‑ testDocumentConfig(ENGLISH, ISSUE_FINAL_DECISION, Optional.empty, TB-SCS-GNO-ENG-final-decision-notice-v2.docx) [3]
uk.gov.hmcts.reform.sscs.config.DocumentConfigurationTest ‑ testDocumentConfig(WELSH, CORRECTION_GRANTED, Optional.empty, TB-SCS-GNO-ENG-corrected-decision-notice.docx) [4]
uk.gov.hmcts.reform.sscs.config.DocumentConfigurationTest ‑ testDocumentConfig(WELSH, DECISION_ISSUED, Optional.empty, TB-SCS-GNO-ENG-draft-decision-notice-v2.docx) [5]
uk.gov.hmcts.reform.sscs.config.DocumentConfigurationTest ‑ testDocumentConfig(WELSH, DIRECTION_ISSUED, Optional.empty, TB-SCS-GNO-ENG-directions-notice.docx) [6]
uk.gov.hmcts.reform.sscs.config.DocumentConfigurationTest ‑ testDocumentConfig(WELSH, ISSUE_FINAL_DECISION, Optional.empty, TB-SCS-GNO-ENG-final-decision-notice-v2.docx) [7]

♻️ This comment has been updated with latest results.

Copy link

CCD diff report

No change

@@ -39,13 +39,13 @@ public class DocumentConfigurationTest {
public static Object[][] documentParameters() {
return new Object[][] {
{LanguagePreference.ENGLISH, EventType.CORRECTION_GRANTED, Optional.empty(), "TB-SCS-GNO-ENG-corrected-decision-notice.docx"},
Copy link
Contributor

Choose a reason for hiding this comment

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

should we get these from the yaml as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe a task as part of clean up too?

@@ -71,13 +71,13 @@ docmosis.uri=${DOCMOSIS_SERVICE_BASE_URL:https://docmosis.aat.platform.hmcts.net
documents.english.CORRECTION_GRANTED=TB-SCS-GNO-ENG-corrected-decision-notice.docx
Copy link
Contributor

Choose a reason for hiding this comment

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

is this file still being used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

unsure, updated it just in case

@benouaer
Copy link
Contributor

general question not necessarily about your changes:

  • do we really need all those yaml files? surely we can use the main one, and only create a separate one when we need to override some properties
  • do we need to specify the name of the template in all those places? should we not just get it from the yaml?

@nilay913
Copy link
Contributor Author

general question not necessarily about your changes:

  • do we really need all those yaml files? surely we can use the main one, and only create a separate one when we need to override some properties
  • do we need to specify the name of the template in all those places? should we not just get it from the yaml?

I don't think they're really needed, maybe as part of a clean up ticket we can remove some seemingly redundant ones? i.e. application.properties.old

Copy link

CCD diff report

No change

Copy link

CCD diff report

No change

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.

2 participants