Improve typing on Database Events #9819
Conversation
charlesBochet
changed the title
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR enhances type safety across the event handling system by introducing generic type parameters and modifying event inheritance structures.
- Changed
ObjectRecordRestoreEventto inherit fromObjectRecordBaseEventinstead ofObjectRecordCreateEventin/packages/twenty-server/src/engine/core-modules/event-emitter/types/object-record-restore.event.ts - Added generic type parameter
<T = object>toObjectRecordNonDestructiveEventin/packages/twenty-server/src/engine/core-modules/event-emitter/types/object-record-non-destructive-event.ts - Introduced
TimelineActivityWorkspaceEntitytype safety in/packages/twenty-server/src/modules/timeline/jobs/upsert-timeline-activity-from-internal-event.job.ts - Inconsistent type usage in
TimelineActivityServicemethods needs addressing -transformEventToTimelineActivitiesandgetLinkedTimelineActivitiesstill use untypedObjectRecordBaseEvent
4 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
| @@ -7,7 +7,10 @@ import { WorkspaceDataSourceService } from 'src/engine/workspace-datasource/work | |||
| import { TimelineActivityRepository } from 'src/modules/timeline/repositiories/timeline-activity.repository'; | |||
| import { TimelineActivityWorkspaceEntity } from 'src/modules/timeline/standard-objects/timeline-activity.workspace-entity'; | |||
|
|
|||
| type TimelineActivity = Omit<ObjectRecordNonDestructiveEvent, 'properties'> & { | |||
| type TimelineActivity = Omit< | |||
| ObjectRecordNonDestructiveEvent<TimelineActivityWorkspaceEntity>, | |||
There was a problem hiding this comment.
The event itself is not about a timeline activity, there's a confusion
| @@ -18,7 +19,9 @@ export class UpsertTimelineActivityFromInternalEvent { | |||
|
|
|||
| @Process(UpsertTimelineActivityFromInternalEvent.name) | |||
| async handle( | |||
| workspaceEventBatch: WorkspaceEventBatch<ObjectRecordNonDestructiveEvent>, | |||
| workspaceEventBatch: WorkspaceEventBatch< | |||
| ObjectRecordNonDestructiveEvent<TimelineActivityWorkspaceEntity> | |||
| | ObjectRecordUpdateEvent | ||
| | ObjectRecordDeleteEvent | ||
| | ObjectRecordRestoreEvent; | ||
| export type ObjectRecordNonDestructiveEvent<T = object> = |
There was a problem hiding this comment.
(I didn't add it because it wasn't needed)
|
|
||
| export class ObjectRecordRestoreEvent< | ||
| T = object, | ||
| > extends ObjectRecordCreateEvent<T> { | ||
| > extends ObjectRecordBaseEvent<T> { |
There was a problem hiding this comment.
This was enforced on purpose because from an API perspective I think the RestoreEvent should always have the properties that the create event has, because people don't want to implemented two different hooks with different logics. I think adding it creates a protection and if there's a need to remove it then it will raise the point
|
Got all the comments! closing the PR :) |
Follow up on #9719