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.

Sign up for GitHub

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

Jump to bottom

New Timeline #4936

Merged
merged 17 commits into from
Apr 19, 2024
Merged

New Timeline #4936

merged 17 commits into from
Apr 19, 2024

Conversation

FelixMalfait
Copy link
Member

@FelixMalfait FelixMalfait commented Apr 12, 2024 •
edited

Refactored the code to introduce two different concepts:

  • AuditLogs (immutable, raw data)
  • TimelineActivities (user-friendly, transformed data)

Still some work needed:

  • Add message, files, calendar events to timeline (~2 hours if done naively)
  • Refactor repository to try to abstract concept when we can (tbd, wait for Twenty ORM)
  • Introduce ability to display child timelines on parent timeline with filtering (~2 days)
  • Improve UI: add links to open note/task, improve diff display, etc (half a day)
  • Decide the path forward for Task vs Notes: either introduce a new field type "Record Type" and start going into that direction ; or split in two objects?
  • Trigger updates when a field is changed (will be solved by real-time / websockets: 2 weeks)
  • Integrate behavioral events (1 day for POC, 1 week for clean/documented)
Screenshot 2024-04-12 at 09 24 49

@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 12, 2024 •
edited

TODOs/FIXMEs:

  • // Todo: remove: packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/constants/standard-object-ids.ts
  • // TODO: need to support objects others than "person", "company", "opportunity": packages/twenty-server/src/modules/timeline/jobs/create-audit-log-from-internal-event.ts

Generated by 🚫 dangerJS against 55d2fd1

FelixMalfait
FelixMalfait commented Apr 12, 2024
View reviewed changes
packages/twenty-front/src/modules/activities/events/components/EventList.tsx
import { ScrollWrapper } from '@/ui/utilities/scroll/components/ScrollWrapper';

type EventListProps = {
targetableObject: ActivityTargetableObject;
title: string;
events: Event[];
events: TimelineActivity[];
Copy link
Member Author

Choose a reason for hiding this comment

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

Tbd if we introduce a proper sub-type concept for Activity (note/task) or if we transform activity into two objects. If we do introduce a proper subtype then TimelineActvity probably isn't the right naming.

FelixMalfait
FelixMalfait commented Apr 12, 2024
View reviewed changes
packages/twenty-front/src/modules/activities/events/components/EventRow.tsx
objectNameType: 'id',
}),
);

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we'll need to think about the right mental model for the different types of timeline items. I kept the logic which was to build it dynamically (but moved it on the TS part / away from the JSX part) but it feels like it might become hard to read if we add more and more conditions

FelixMalfait
FelixMalfait commented Apr 12, 2024
View reviewed changes
...es/twenty-server/src/engine/integrations/event-emitter/utils/object-record-changed-values.ts Outdated
(field) =>
field.type === FieldMetadataType.RELATION && field.name === key,
) ||
(objectMetadata.nameSingular === 'activity' && key === 'body')
Copy link
Member Author

Choose a reason for hiding this comment

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

Todo need to hand diff for activity.body (thought I had done it but somehow got lost)

FelixMalfait
FelixMalfait commented Apr 12, 2024
View reviewed changes
packages/twenty-server/src/modules/event/standard-objects/behavioral-event.object-metadata.ts Outdated
import { ObjectMetadata } from 'src/engine/workspace-manager/workspace-sync-metadata/decorators/object-metadata.decorator';
import { BaseObjectMetadata } from 'src/engine/workspace-manager/workspace-sync-metadata/standard-objects/base.object-metadata';

@ObjectMetadata({
Copy link
Member Author

Choose a reason for hiding this comment

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

Behavioral events = event coming from Segment

@FelixMalfait FelixMalfait mentioned this pull request Apr 12, 2024
Open
charlesBochet
charlesBochet reviewed Apr 15, 2024
View reviewed changes
packages/twenty-front/src/modules/activities/events/components/EventList.tsx Outdated Show resolved Hide resolved
packages/twenty-front/src/modules/activities/events/components/EventList.tsx Outdated Show resolved Hide resolved
packages/twenty-front/src/modules/activities/events/components/EventRow.tsx
return (
event.name === type + '.' + targetableObject.targetObjectNameSingular
);
if (event.name.includes('.')) {
Copy link
Member

Choose a reason for hiding this comment

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

isEventType could be extracted to an util and tested with unit tests

packages/twenty-front/src/modules/activities/events/components/EventRow.tsx Outdated
};

const { getIcon } = useIcons();

const linkedObjectMetadata = useRecoilValue(
Copy link
Member

Choose a reason for hiding this comment

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

useObjectMetadataItem

packages/twenty-front/src/modules/object-metadata/states/objectMetadataItemFamilySelector.ts Outdated Show resolved Hide resolved
packages/twenty-server/src/modules/event/standard-objects/timeline-activity.object-metadata.ts Outdated Show resolved Hide resolved
packages/twenty-server/src/modules/event/standard-objects/timeline-activity.object-metadata.ts Outdated Show resolved Hide resolved
packages/twenty-server/src/modules/event/standard-objects/timeline-activity.object-metadata.ts Outdated Show resolved Hide resolved
packages/twenty-server/src/modules/event/standard-objects/timeline-activity.object-metadata.ts Outdated Show resolved Hide resolved
packages/twenty-server/src/modules/event/standard-objects/timeline-activity.object-metadata.ts Outdated Show resolved Hide resolved
@FelixMalfait FelixMalfait force-pushed the refacto-events branch 3 times, most recently from 6d33847 to a6a7700 Compare April 17, 2024 09:13
@charlesBochet charlesBochet mentioned this pull request Apr 17, 2024
Merged
Weiko pushed a commit that referenced this pull request Apr 17, 2024
@charlesBochet
Disable audit log on system objects (#5018)
977927a 
## Context

We have recently added an event listener to create audit logs on objects
update. However, we have only created the structure (relations on event
standard objects) for Company, Person, Opportunity and custom objects.
There is a larger effort in #4936 to refactor this.
For now, we are disabling log auditing on all other objects

## How
Add @IsNotAuditLogged() annotation on all standard objects except
Company, Person, Opportunity
@FelixMalfait FelixMalfait merged commit d145684 into main Apr 19, 2024
9 of 10 checks passed
@FelixMalfait FelixMalfait deleted the refacto-events branch April 19, 2024 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@charlesBochet charlesBochet charlesBochet left review comments

Assignees

@FelixMalfait FelixMalfait

Labels
PR: awaiting author
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@FelixMalfait @charlesBochet