chore: Separate changelog logic in preparation to table separation [DHIS2-19848] #21366
Conversation
...c/main/java/org/hisp/dhis/tracker/export/singleevent/HibernateSingleEventChangeLogStore.java
Fixed
Show fixed
Hide fixed
...c/main/java/org/hisp/dhis/tracker/export/singleevent/HibernateSingleEventChangeLogStore.java
Fixed
Show fixed
Hide fixed
| @Getter | ||
| @Setter | ||
| @RequiredArgsConstructor | ||
| public class EventChangeLog { |
There was a problem hiding this comment.
Could this now be a record 🤔 or @Value. Maybe before it was not possible because of Hibernate 🤷🏻 @muilpp
There was a problem hiding this comment.
Why do we have this EventChangeLog and the 2 new event changelog classes?
There was a problem hiding this comment.
I can make it a record, I was just lazy to rename all the getters and I didn't want to pollute the PR.
I will do it after the reviews.
There was a problem hiding this comment.
EventChangeLog is the object that we retrieve from the DB and we then map to the view object.
This object does not need to link to an event because we are always retrieving changelogs in the context of one event and in the view object the link to the event is not needed.
On the other side, we need the hibernate objects to link to the event and they will be different events (singleevent and trackerevent) as soon as we split the event table.
...-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventChangeLogService.java
Outdated
Show resolved
Hide resolved
| @@ -152,7 +155,6 @@ public Page<EventChangeLog> getEventChangeLogs( | |||
| results.stream() | |||
| .map( | |||
| row -> { | |||
| Event e = (Event) row[0]; | |||
There was a problem hiding this comment.
why is this not needed anymore?
There was a problem hiding this comment.
We are creating an EventChangeLog that does not need to link to an event.
That event was never needed, as it is not mapped to the view in the web layer.
There was a problem hiding this comment.
In that case, can we remove the event from the select clause?
...c/main/java/org/hisp/dhis/tracker/export/singleevent/HibernateSingleEventChangeLogStore.java
Outdated
Show resolved
Hide resolved
...st/java/org/hisp/dhis/tracker/export/singleevent/OrderAndFilterSingleEventChangeLogTest.java
Outdated
Show resolved
Hide resolved
| <type name="org.hibernate.type.EnumType"> | ||
| <param name="enumClass">org.hisp.dhis.changelog.ChangeLogType</param> | ||
| <param name="useNamed">true</param> | ||
| <param name="type">12</param> |
There was a problem hiding this comment.
I don't think we need to specify the type.
Hibernate infers it because we use the name method of the changeLogType enum, which already returns a String.
There was a problem hiding this comment.
Nope, I cannot remove it, hibernate is complaining that the type is wrong and the application is not starting at all.
| @@ -152,7 +155,6 @@ public Page<EventChangeLog> getEventChangeLogs( | |||
| results.stream() | |||
| .map( | |||
| row -> { | |||
| Event e = (Event) row[0]; | |||
There was a problem hiding this comment.
In that case, can we remove the event from the select clause?
|
The idea of this task is to prepare the code for the separation of
eventtable that will happen later.We will need to create 2 separate tables for
eventchangelogas well, so we are already creating different hibernate mappings, stores and services.Chnages in this PR
EventServiceto be used inEventChangeLogService.EventChangeLogServiceis abstract and the correct event service will be injected, here we only need to check if an event exists (or it is accessible) in order to retrieve or not the changelogs.HibernateEventChangeLogStoreis an abstract class, as all the logic is the same for the 2 types, only the name of the table is different.EventChangeLogServiceis abstract because the changelogs retrieved here are the DTOs, that are the same for the 2 types; the only difference is the hibernate object that we are persisting to the DB and this is injected by the implementation of the service.What we will need to do when separating the event table
Eventtype