-
Notifications
You must be signed in to change notification settings - Fork 2
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
DMP-3534: Retention Confidence #2382
base: master
Are you sure you want to change the base?
DMP-3534: Retention Confidence #2382
Conversation
0b674db
to
a78340f
Compare
a78340f
to
7eaab33
Compare
|
||
// when | ||
closeOldCasesProcessor.closeCases(2); | ||
|
||
// then | ||
assertTrue(courtCase.getClosed()); | ||
assertEquals(CURRENT_DATE_TIME, courtCase.getRetConfUpdatedTs()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Author note: The class under test no longer has responsibility for setting this attribute, therefore the assertion is removed here. Dedicated unit tests exist in RetentionServiceImplTest
to verify this attribute is set correctly.
verify(caseRepository).save(courtCaseEntityArgumentCaptor.capture()); | ||
CourtCaseEntity savedCourtCase = courtCaseEntityArgumentCaptor.getValue(); | ||
assertEquals(RetentionConfidenceReasonEnum.MANUAL_OVERRIDE, savedCourtCase.getRetConfReason()); | ||
assertEquals(RetentionConfidenceScoreEnum.CASE_PERFECTLY_CLOSED, savedCourtCase.getRetConfScore()); | ||
assertEquals(CURRENT_DATE_TIME, savedCourtCase.getRetConfUpdatedTs()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Author note: The class under test no longer has responsibility for setting these attributes, therefore the assertions are removed here. Dedicated unit tests exist in RetentionServiceImplTest
to verify these attributes are set correctly.
Similar comment applies to the other tests in this test class.
src/main/java/uk/gov/hmcts/darts/retention/enums/RetentionConfidenceScoreEnum.java
Show resolved
Hide resolved
src/main/java/uk/gov/hmcts/darts/common/entity/RetentionConfidenceCategoryMapperEntity.java
Show resolved
Hide resolved
…g value for confidenceReason; add associated test
|
||
OffsetDateTime closeDate = OffsetDateTime.now().minusYears(7); | ||
|
||
EventEntity eventEntity1 = dartsDatabase.getEventStub().createEvent(hearing, 8);//Re-examination |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does comment on this line mean? Is it the event name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will review the comments in this test method, I have copy/pasted an existing test setup to test a slightly different scenario here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The existing comments were relating to the name of the event handler, but since the primary keys were hard-coded they have since went out of sync. Regardless, the choice of event handler doesn't matter, we just need some event.
I've reworked this test to remove the hard-coded primary keys and have simplified it somewhat.
DMP-3534
Two fundamental changes in this PR:
src/main/java/uk/gov/hmcts/darts/retention/enums/RetentionConfidenceScoreEnum.java
retention_confidence_category_mapper
to obtain these values. Seesrc/main/java/uk/gov/hmcts/darts/retention/service/impl/RetentionServiceImpl.java
Some additional work has been done to facilitate these changes:
RetentionConfidenceCategoryMapperEntity
has been updated to utilise enums rather than ints, improving code readability and aligning with the approach taken forCaseRetentionEntity
andCourtCaseEntity
.RetentionServiceImpl
, and calling code has been adjusted to share this common code.RetentionConfidenceCategoryMapperEntity
objects for both unit and integration tests going forward.Does this PR introduce a breaking change? (check one with "x")