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-3334 Judge Approve Orders - approves or changes #1997

Open
wants to merge 67 commits into
base: DFR-3332-Judge-Approve-Orders---starting-page
Choose a base branch
from

Conversation

so99y
Copy link
Contributor

@so99y so99y commented Nov 28, 2024

Jira link

See DFR-3334

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

Ashley Wong and others added 25 commits November 21, 2024 17:48
…-3334-Approves-or-Changesv2

# Conflicts:
#	src/main/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/judgeapproval/ApproveDraftOrdersAboutToStartHandler.java
…-3334-Approves-or-Changesv2

# Conflicts:
#	src/main/java/uk/gov/hmcts/reform/finrem/caseorchestration/model/ccd/draftorders/review/DraftOrderDocumentReview.java
@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.

@so99y so99y marked this pull request as ready for review November 29, 2024 15:17
@so99y so99y changed the base branch from master to DFR-3332-Judge-Approve-Orders---starting-page November 29, 2024 15:26
Copy link
Contributor

@jthmcts jthmcts left a comment

Choose a reason for hiding this comment

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

ApproveOrderService has a lot of protected methods but they really seem to be private. So the protected visibility is only there to enable them to be unit tested. This doesn't seem a good approach. If a class is so complex that private methods need to be tested in isolation then that suggests you should refactor your code. It's ok to create small classes. They can be package private if they are only required in judgeapproval.

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.

3 participants