-
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
CPR-458 Add Recluster event log #729
base: main
Are you sure you want to change the base?
Conversation
|
||
withType<JavaCompile>().configureEach { | ||
options.isFork = true | ||
} |
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.
❓
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.
@@ -1,5 +1,7 @@ | |||
package uk.gov.justice.digital.hmpps.personrecord.jpa.entity | |||
|
|||
import com.fasterxml.jackson.annotation.JsonBackReference | |||
import com.fasterxml.jackson.annotation.JsonIgnore |
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.
❓
@@ -7,5 +7,8 @@ import uk.gov.justice.digital.hmpps.personrecord.jpa.entity.EventLoggingEntity | |||
@Repository | |||
interface EventLoggingRepository : JpaRepository<EventLoggingEntity, Long> { | |||
fun findFirstBySourceSystemIdOrderByEventTimestampDesc(sourceSystemId: String): EventLoggingEntity? | |||
|
|||
fun findFirstByUuidOrderByEventTimestampDesc(uuid: String): EventLoggingEntity? | |||
|
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.
❓
@@ -35,24 +34,13 @@ data class Person( | |||
val contacts: List<Contact> = emptyList(), | |||
val addresses: List<Address> = emptyList(), | |||
val references: List<Reference> = emptyList(), | |||
var selfMatchScore: Double? = null, |
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.
❓
@@ -187,31 +178,6 @@ data class Person( | |||
currentlyManaged = prisoner.currentlyManaged, | |||
) | |||
} | |||
|
|||
fun from(existingPersonEntity: PersonEntity): Person { |
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.
❓
val sourceSystemType: SourceSystemType, | ||
val sentences: List<SentenceInfo> = emptyList(), | ||
val currentlyManaged: Boolean? = null, | ||
) { | ||
|
||
companion object { | ||
|
||
fun Person?.extractSourceSystemId(): String? { |
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.
❓
@@ -16,25 +20,50 @@ class EventLoggingService( | |||
private val objectMapper: ObjectMapper, | |||
) { | |||
|
|||
fun snapshotPersonEntity(personEntity: PersonEntity) = snapshotEntity<PersonEntity>(personEntity) | |||
|
|||
fun snapshotPersonKeyEntity(personKeyEntity: PersonKeyEntity) = snapshotEntity<PersonKeyEntity>(personKeyEntity) |
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.
interesting, possibly could be used more widely
processedPerson: Person?, | ||
uuid: String? = null, | ||
beforePersonEntity: PersonEntity?, | ||
afterPersonEntity: PersonEntity?, |
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 tried this and found that they were often one and the same - maybe things have moved on since then
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.
This naming is definitely better
|
||
fun recordEventLog( |
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.
❓
return personEntity | ||
} | ||
|
||
private fun handlePersonUpdate(person: Person, existingPersonEntity: PersonEntity, event: String?): PersonEntity { | ||
val beforePersonEntity = eventLoggingService.snapshotPersonEntity(existingPersonEntity) |
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.
As far as I can see, this method call is compulsory now before calling recordEventLog? Otherwise you run the (considerable) risk of just sending in the entity, which is probably managed and therefore the before and after will be the same.
No description provided.