From 247e422eace671c7240e0aa6192aecb960779991 Mon Sep 17 00:00:00 2001 From: Charles Bochet Date: Fri, 12 Jun 2026 18:58:05 +0200 Subject: [PATCH] fix(front): prevent timeline "Invalid configuration" on update events without a diff (#21460) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Fixes #20597 ### Problem A person's (or any record's) timeline renders the whole widget as **"Invalid configuration"** when it contains an `*.updated` event without a usable `properties.diff`. The error-boundary fallback (`PageLayoutWidgetInvalidConfigDisplay`) is triggered because `EventRowMainObjectUpdated` **throws** during render: ```ts const diff = event.properties?.diff; // can be undefined const diffEntries = Object.entries(diff); // throws TypeError when undefined if (diffEntries.length === 0) { throw new Error('Cannot render update description without changes'); } ``` `filterOutInvalidTimelineActivities` only validates activities that **already carry** a diff (`canSkipValidation = !diff`), so a main-object `*.updated` event with a missing diff passes straight through to this renderer and crashes it. A single malformed row takes down the entire timeline. ### Fix Render nothing instead of throwing when an update event has no changes to show. This mirrors the sibling `EventRowMainObject` default branch (which returns `null`) and the filter's own behaviour of dropping empty diffs, and keeps one bad row from crashing the whole widget. The fix is intentionally kept in the renderer rather than the filter: the filter cannot distinguish a diff-less main-object update (must be dropped) from a diff-less `linked-task`/`linked-note` update (legitimately has `properties: {}` and renders fine via `EventRowActivity`) without duplicating routing logic. ### Test Added `EventRowMainObjectUpdated.test.tsx` — a regression test asserting the component renders nothing (no throw) for both a missing-diff and an empty-diff update event. --- ...filterOutInvalidTimelineActivities.test.ts | 166 ++++++++---------- .../filterOutInvalidTimelineActivities.ts | 88 ++++++---- .../timeline-activity.repository.ts | 34 ++-- 3 files changed, 147 insertions(+), 141 deletions(-) diff --git a/packages/twenty-front/src/modules/activities/timeline-activities/utils/__tests__/filterOutInvalidTimelineActivities.test.ts b/packages/twenty-front/src/modules/activities/timeline-activities/utils/__tests__/filterOutInvalidTimelineActivities.test.ts index 037c1537d3..6c5a8a9472 100644 --- a/packages/twenty-front/src/modules/activities/timeline-activities/utils/__tests__/filterOutInvalidTimelineActivities.test.ts +++ b/packages/twenty-front/src/modules/activities/timeline-activities/utils/__tests__/filterOutInvalidTimelineActivities.test.ts @@ -1,157 +1,145 @@ import { type TimelineActivity } from '@/activities/timeline-activities/types/TimelineActivity'; import { filterOutInvalidTimelineActivities } from '@/activities/timeline-activities/utils/filterOutInvalidTimelineActivities'; -import { CoreObjectNameSingular } from 'twenty-shared/types'; import { type EnrichedObjectMetadataItem } from '@/object-metadata/types/EnrichedObjectMetadataItem'; -const noteObjectMetadataItem = { - nameSingular: CoreObjectNameSingular.Note, - namePlural: 'notes', +const mainObjectMetadataItem = { + nameSingular: 'company', + namePlural: 'companies', fields: [{ name: 'field1' }, { name: 'field2' }, { name: 'field3' }], readableFields: [{ name: 'field1' }, { name: 'field2' }, { name: 'field3' }], updatableFields: [{ name: 'field1' }, { name: 'field2' }, { name: 'field3' }], } as EnrichedObjectMetadataItem; +const noteObjectMetadataItem = { + nameSingular: 'note', + namePlural: 'notes', + readableFields: [{ name: 'title' }, { name: 'body' }], +} as EnrichedObjectMetadataItem; + +const filter = (events: TimelineActivity[]) => + filterOutInvalidTimelineActivities(events, 'company', [ + mainObjectMetadataItem, + noteObjectMetadataItem, + ]); + describe('filterOutInvalidTimelineActivities', () => { - it('should filter out TimelineActivities with deleted fields from the properties diff', () => { + it('keeps update diffs as-is and trims fields not in the readable fields', () => { const events = [ { id: '1', - name: 'event1', + name: 'company.updated', properties: { diff: { field1: { before: 'value1', after: 'value2' }, field2: { before: 'value3', after: 'value4' }, - field3: { before: 'value5', after: 'value6' }, }, }, }, { id: '2', - name: 'event2', + name: 'company.updated', properties: { diff: { field1: { before: 'value7', after: 'value8' }, - field2: { before: 'value9', after: 'value10' }, field4: { before: 'value11', after: 'value12' }, }, }, }, ] as TimelineActivity[]; - const mainObjectMetadataItem = { - nameSingular: 'objectNameSingular', - namePlural: 'objectNamePlural', - fields: [{ name: 'field1' }, { name: 'field2' }, { name: 'field3' }], - readableFields: [ - { name: 'field1' }, - { name: 'field2' }, - { name: 'field3' }, - ], - updatableFields: [ - { name: 'field1' }, - { name: 'field2' }, - { name: 'field3' }, - ], - } as EnrichedObjectMetadataItem; - - const filteredEvents = filterOutInvalidTimelineActivities( - events, - 'objectNameSingular', - [mainObjectMetadataItem, noteObjectMetadataItem], - ); - - expect(filteredEvents).toEqual([ + expect(filter(events)).toEqual([ { id: '1', - name: 'event1', + name: 'company.updated', properties: { diff: { field1: { before: 'value1', after: 'value2' }, field2: { before: 'value3', after: 'value4' }, - field3: { before: 'value5', after: 'value6' }, }, }, }, { id: '2', - name: 'event2', + name: 'company.updated', properties: { - diff: { - field1: { before: 'value7', after: 'value8' }, - field2: { before: 'value9', after: 'value10' }, - }, + diff: { field1: { before: 'value7', after: 'value8' } }, }, }, ]); }); - it('should return an empty array if all TimelineActivities have deleted fields in the properties diff', () => { + it('drops update events whose diff has no readable fields', () => { const events = [ { id: '1', - name: 'event1', + name: 'company.updated', properties: { - diff: { - field3: { before: 'value5', after: 'value6' }, - }, - }, - }, - { - id: '2', - name: 'event2', - properties: { - diff: { - field4: { before: 'value11', after: 'value12' }, - }, + diff: { field4: { before: 'value11', after: 'value12' } }, }, }, ] as TimelineActivity[]; - const mainObjectMetadataItem = { - nameSingular: 'objectNameSingular', - namePlural: 'objectNamePlural', - fields: [{ name: 'field1' }, { name: 'field2' }], - readableFields: [{ name: 'field1' }, { name: 'field2' }], - updatableFields: [{ name: 'field1' }, { name: 'field2' }], - } as EnrichedObjectMetadataItem; - - const filteredEvents = filterOutInvalidTimelineActivities( - events, - 'objectNameSingular', - [mainObjectMetadataItem, noteObjectMetadataItem], - ); - - expect(filteredEvents).toEqual([]); + expect(filter(events)).toEqual([]); }); - it('should return the same TimelineActivities if there are no properties diffs', () => { + it('drops update events that have no diff', () => { + const events = [ + { id: '1', name: 'company.updated', properties: {} }, + ] as TimelineActivity[]; + + expect(filter(events)).toEqual([]); + }); + + it('keeps non-update events that have no diff', () => { + const events = [ + { id: '1', name: 'company.created', properties: {} }, + { id: '2', name: 'company.deleted', properties: {} }, + ] as TimelineActivity[]; + + expect(filter(events)).toEqual(events); + }); + + it('keeps linked note/task update events even without a diff', () => { + const events = [ + { id: '1', name: 'linked-task.updated', properties: {} }, + { id: '2', name: 'linked-note.updated', properties: {} }, + ] as TimelineActivity[]; + + expect(filter(events)).toEqual(events); + }); + + it('validates linked note diffs against the note readable fields', () => { const events = [ { id: '1', - name: 'event1', - properties: {}, - }, - { - id: '2', - name: 'event2', - properties: {}, + name: 'linked-note.updated', + properties: { + diff: { + title: { before: 'a', after: 'b' }, + field1: { before: 'c', after: 'd' }, + }, + }, }, ] as TimelineActivity[]; - const mainObjectMetadataItem = { - nameSingular: 'objectNameSingular', - namePlural: 'objectNamePlural', - fields: [{ name: 'field1' }, { name: 'field2' }], - readableFields: [{ name: 'field1' }, { name: 'field2' }], - updatableFields: [{ name: 'field1' }, { name: 'field2' }], - } as EnrichedObjectMetadataItem; + expect(filter(events)).toEqual([ + { + id: '1', + name: 'linked-note.updated', + properties: { diff: { title: { before: 'a', after: 'b' } } }, + }, + ]); + }); - const filteredEvents = filterOutInvalidTimelineActivities( - events, - 'objectNameSingular', - [mainObjectMetadataItem, noteObjectMetadataItem], - ); + it('drops linked note updates whose diff has no readable note fields', () => { + const events = [ + { + id: '1', + name: 'linked-note.updated', + properties: { diff: { field1: { before: 'c', after: 'd' } } }, + }, + ] as TimelineActivity[]; - expect(filteredEvents).toEqual(events); + expect(filter(events)).toEqual([]); }); }); diff --git a/packages/twenty-front/src/modules/activities/timeline-activities/utils/filterOutInvalidTimelineActivities.ts b/packages/twenty-front/src/modules/activities/timeline-activities/utils/filterOutInvalidTimelineActivities.ts index 11dcd54a91..35a86761be 100644 --- a/packages/twenty-front/src/modules/activities/timeline-activities/utils/filterOutInvalidTimelineActivities.ts +++ b/packages/twenty-front/src/modules/activities/timeline-activities/utils/filterOutInvalidTimelineActivities.ts @@ -1,9 +1,32 @@ import { type TimelineActivity } from '@/activities/timeline-activities/types/TimelineActivity'; import { findFieldMetadataItemByDiffKey } from '@/activities/timeline-activities/utils/findFieldMetadataItemByDiffKey'; import { type EnrichedObjectMetadataItem } from '@/object-metadata/types/EnrichedObjectMetadataItem'; -import { CoreObjectNameSingular } from 'twenty-shared/types'; +import { type FieldMetadataItem } from '@/object-metadata/types/FieldMetadataItem'; import { isDefined } from 'twenty-shared/utils'; +const keepActivityWithReadableDiff = ( + timelineActivity: TimelineActivity, + readableFields: FieldMetadataItem[], +): TimelineActivity | undefined => { + const validDiffEntries = Object.entries( + timelineActivity.properties?.diff ?? {}, + ).filter(([diffKey]) => + isDefined(findFieldMetadataItemByDiffKey(readableFields, diffKey)), + ); + + if (validDiffEntries.length === 0) { + return undefined; + } + + return { + ...timelineActivity, + properties: { + ...timelineActivity.properties, + diff: Object.fromEntries(validDiffEntries), + }, + }; +}; + export const filterOutInvalidTimelineActivities = ( timelineActivities: TimelineActivity[], mainObjectSingularName: string, @@ -14,46 +37,39 @@ export const filterOutInvalidTimelineActivities = ( objectMetadataItem.nameSingular === mainObjectSingularName, ); - const noteObjectMetadataItem = objectMetadataItems.find( - (objectMetadataItem) => - objectMetadataItem.nameSingular === CoreObjectNameSingular.Note, - ); - - if (!mainObjectMetadataItem || !noteObjectMetadataItem) { - throw new Error('Object metadata items not found'); + if (!isDefined(mainObjectMetadataItem)) { + throw new Error('Object metadata item not found'); } - return timelineActivities.filter((timelineActivity) => { - const diff = timelineActivity.properties?.diff; - const canSkipValidation = !diff; + return timelineActivities + .map((timelineActivity) => { + const [objectName, action] = timelineActivity.name.split('.'); - if (canSkipValidation) { - return true; - } + if (objectName.startsWith('linked-')) { + if (!isDefined(timelineActivity.properties?.diff)) { + return timelineActivity; + } - const isNoteOrTask = - timelineActivity.name.startsWith('linked-note') || - timelineActivity.name.startsWith('linked-task'); + const linkedObjectMetadataItem = objectMetadataItems.find( + (objectMetadataItem) => + objectMetadataItem.nameSingular === + objectName.replace('linked-', ''), + ); - const fieldsToValidateAgainst = isNoteOrTask - ? noteObjectMetadataItem.readableFields - : mainObjectMetadataItem.readableFields; + return keepActivityWithReadableDiff( + timelineActivity, + linkedObjectMetadataItem?.readableFields ?? [], + ); + } - const validDiffEntries = Object.entries(diff).filter(([diffKey]) => - isDefined( - findFieldMetadataItemByDiffKey(fieldsToValidateAgainst, diffKey), - ), - ); + if (action === 'updated') { + return keepActivityWithReadableDiff( + timelineActivity, + mainObjectMetadataItem.readableFields, + ); + } - if (validDiffEntries.length === 0) { - return false; - } - - timelineActivity.properties = { - ...timelineActivity.properties, - diff: Object.fromEntries(validDiffEntries), - }; - - return true; - }); + return timelineActivity; + }) + .filter(isDefined); }; diff --git a/packages/twenty-server/src/modules/timeline/repositories/timeline-activity.repository.ts b/packages/twenty-server/src/modules/timeline/repositories/timeline-activity.repository.ts index c904b58980..4637db8240 100644 --- a/packages/twenty-server/src/modules/timeline/repositories/timeline-activity.repository.ts +++ b/packages/twenty-server/src/modules/timeline/repositories/timeline-activity.repository.ts @@ -1,7 +1,7 @@ import { Injectable } from '@nestjs/common'; -import { isDefined } from 'class-validator'; import { type ObjectRecord } from 'twenty-shared/types'; +import { isDefined } from 'twenty-shared/utils'; import { In, MoreThan } from 'typeorm'; import { objectRecordDiffMerge } from 'src/engine/core-modules/event-emitter/utils/object-record-diff-merge'; @@ -38,21 +38,23 @@ export class TimelineActivityRepository { payloads, }); - const payloadsWithDiff = payloads - .filter(({ properties }) => { - const isDiffEmpty = - properties.diff !== null && - properties.diff && - Object.keys(properties.diff).length === 0; + const payloadsToUpsert = payloads.flatMap( + ({ name, properties, ...rest }) => { + const [objectName, action] = name.split('.'); + const { diff } = properties; + const hasDiff = isDefined(diff) && Object.keys(diff).length > 0; - return !isDiffEmpty; - }) - .map(({ properties, ...rest }) => ({ - ...rest, - properties: isDefined(properties.diff) - ? { diff: properties.diff } - : {}, - })); + if (objectName.startsWith('linked-')) { + return [{ ...rest, name, properties: hasDiff ? { diff } : {} }]; + } + + if (action === 'updated') { + return hasDiff ? [{ ...rest, name, properties: { diff } }] : []; + } + + return [{ ...rest, name, properties: {} }]; + }, + ); const payloadsToInsert: TimelineActivityPayloadWorkspaceIdAndObjectSingularName['payloads'] = []; @@ -60,7 +62,7 @@ export class TimelineActivityRepository { const timelineActivityPropertyName = await this.getTimelineActivityPropertyName(objectSingularName); - for (const payload of payloadsWithDiff) { + for (const payload of payloadsToUpsert) { const recentTimelineActivity = recentTimelineActivities.find( (timelineActivity) => timelineActivity[timelineActivityPropertyName] ===