Fix update events reporting untouched columns as changed (#22797)

## Before https://github.com/user-attachments/assets/b02dbd50-87c9-4699-a6a9-254a4c8b9182 The image uploaded during the onboarding is deleted and doesn't appear in the animation ## After https://github.com/user-attachments/assets/9443b837-044e-47c6-b2df-c392c238e935 The Image appears in the animation ## Description TwentyORM's `.save()` emits UPDATE events whose `after` record carries default (empty) values for columns that weren't written: TypeORM null-injects untouched nullable columns on the entity in place, and `formatResult` turns those into empty strings. So a partial update (e.g. renaming a workspace member) reports untouched fields like `avatarUrl` and `userEmail` as changed, which is what made the avatar-file-deletion listener delete the picture uploaded during onboarding. The same stale data also reaches webhooks, workflow/logic-function triggers and record subscriptions. Fix: `save()` was the only write path building its event `after` from the in-memory payload instead of re-reading the row. `update()`, `upsert()` and `softDelete()` all re-SELECT after the write, so `save()` now does the same. `withDeleted` is on both the before and the after find, since `save({ id, deletedAt })` and `save({ id, deletedAt: null })` are valid soft-delete and restore, and an asymmetric find would leave a restored row with no matching before-record. Worth noting: a genuinely no-op save now emits no update event, where it previously emitted one with a bogus diff.
2026-07-21 17:44:18 +02:00
parent 00e5917d4d
commit 578d69e4fb
3 changed files with 177 additions and 7 deletions
@@ -19,6 +19,7 @@ import {
  withWorkspaceContext,
  type ORMWorkspaceContext,
 } from 'src/engine/twenty-orm/storage/orm-workspace-context.storage';
+import { formatResult } from 'src/engine/twenty-orm/utils/format-result.util';
 import { getObjectMetadataFromEntityTarget } from 'src/engine/twenty-orm/utils/get-object-metadata-from-entity-target.util';

 import { WorkspaceEntityManager } from './workspace-entity-manager';
@@ -475,6 +476,70 @@ describe('WorkspaceEntityManager', () => {
        updatedColumns: [],
      });
    });
+
+    it('should emit the update event with the record re-selected after the write', async () => {
+      const recordBefore = {
+        id: 'record-id',
+        fieldName: 'Old Name',
+        avatarUrl: 'http://localhost:3000/file/core-picture/abc',
+      };
+      const recordAfter = {
+        id: 'record-id',
+        fieldName: 'New Name',
+        avatarUrl: 'http://localhost:3000/file/core-picture/abc',
+      };
+      const persistedPayloadWithUntouchedColumnsNulled = {
+        id: 'record-id',
+        fieldName: 'New Name',
+        avatarUrl: '',
+      };
+
+      (formatResult as jest.Mock).mockReturnValue([
+        persistedPayloadWithUntouchedColumnsNulled,
+      ]);
+
+      const findSpy = jest
+        .spyOn(entityManager, 'find')
+        .mockResolvedValueOnce([recordBefore])
+        .mockResolvedValueOnce([recordAfter]);
+
+      await withWorkspaceContext(mockWorkspaceContext, () =>
+        entityManager.save(
+          'test-entity',
+          { id: 'record-id', fieldName: 'New Name' },
+          { reload: false },
+          mockPermissionOptions,
+        ),
+      );
+
+      expect(findSpy).toHaveBeenCalledTimes(2);
+
+      const emitDatabaseBatchEvent = mockInternalContext.eventEmitterService
+        .emitDatabaseBatchEvent as jest.Mock;
+      const updatedBatchEvent = emitDatabaseBatchEvent.mock.calls[0][0];
+
+      expect(updatedBatchEvent.events[0].properties.after).toBe(recordAfter);
+      expect(updatedBatchEvent.events[0].properties.updatedFields).toEqual([
+        'fieldName',
+      ]);
+    });
+
+    it('should not re-select when the save only creates records', async () => {
+      (formatResult as jest.Mock).mockReturnValue([{ id: 'created-id' }]);
+
+      const findSpy = jest.spyOn(entityManager, 'find').mockResolvedValue([]);
+
+      await withWorkspaceContext(mockWorkspaceContext, () =>
+        entityManager.save(
+          'test-entity',
+          { fieldName: 'New Name' },
+          { reload: false },
+          mockPermissionOptions,
+        ),
+      );
+
+      expect(findSpy).toHaveBeenCalledTimes(1);
+    });
  });

  describe('Update Methods', () => {
@@ -3,7 +3,7 @@ import {
  type ObjectsPermissions,
  type ObjectsPermissionsByRoleId,
 } from 'twenty-shared/types';
-import { isDefined } from 'twenty-shared/utils';
+import { isDefined, isNonEmptyArray } from 'twenty-shared/utils';
 import {
  type DeleteResult,
  EntityManager,
@@ -1210,6 +1210,7 @@ export class WorkspaceEntityManager extends EntityManager {
        entityTarget,
        {
          where: { id: In(entityIds) },
+          withDeleted: true,
        },
        { shouldBypassPermissionChecks: true }, // Bypass as this is for event emission
      );
@@ -1301,13 +1302,25 @@ export class WorkspaceEntityManager extends EntityManager {
        this.internalContext.flatFieldMetadataMaps,
      );

-      const updatedEntities = formattedResult.filter(
-        (entity) => beforeUpdateMapById[entity.id],
-      );
      const createdEntities = formattedResult.filter(
        (entity) => !beforeUpdateMapById[entity.id],
      );

+      const updatedEntityIds = formattedResult
+        .map((entity) => entity.id)
+        .filter((entityId) => isDefined(beforeUpdateMapById[entityId]));
+
+      const updatedEntities = isNonEmptyArray(updatedEntityIds)
+        ? await this.find(
+            entityTarget,
+            {
+              where: { id: In(updatedEntityIds) },
+              withDeleted: true,
+            },
+            { shouldBypassPermissionChecks: true }, // Bypass as this is for event emission
+          )
+        : [];
+
      this.internalContext.eventEmitterService.emitDatabaseBatchEvent(
        formatTwentyOrmEventToDatabaseBatchEvent({
          action: DatabaseEventAction.UPDATED,
@@ -1315,9 +1328,7 @@ export class WorkspaceEntityManager extends EntityManager {
          flatFieldMetadataMaps: this.internalContext.flatFieldMetadataMaps,
          workspaceId: this.internalContext.workspaceId,
          recordsAfter: updatedEntities,
-          recordsBefore: updatedEntities.map(
-            (entity) => beforeUpdateMapById[entity.id],
-          ),
+          recordsBefore: beforeUpdate,
        }),
      );

@@ -0,0 +1,94 @@
+import { type ObjectRecordUpdateEvent } from 'twenty-shared/database-events';
+
+import { type FileCorePictureService } from 'src/engine/core-modules/file/file-core-picture/services/file-core-picture.service';
+import { type WorkspaceEventBatch } from 'src/engine/workspace-event-emitter/types/workspace-event-batch.type';
+import { WorkspaceMemberAvatarFileDeletionListener } from 'src/modules/workspace-member/listeners/workspace-member-avatar-file-deletion.listener';
+import { type WorkspaceMemberWorkspaceEntity } from 'src/modules/workspace-member/standard-objects/workspace-member.workspace-entity';
+
+const WORKSPACE_ID = 'ec6d123f-0d1c-4b3a-9c1f-2b1a9c8d7e6f';
+const OLD_FILE_ID = '11111111-1111-4111-8111-111111111111';
+const NEW_FILE_ID = '22222222-2222-4222-8222-222222222222';
+const OLD_URL = `http://localhost:3000/file/core-picture/${OLD_FILE_ID}`;
+const NEW_URL = `http://localhost:3000/file/core-picture/${NEW_FILE_ID}`;
+
+const buildUpdateBatch = ({
+  before,
+  after,
+  updatedFields,
+}: {
+  before: Partial<WorkspaceMemberWorkspaceEntity>;
+  after: Partial<WorkspaceMemberWorkspaceEntity>;
+  updatedFields: string[];
+}): WorkspaceEventBatch<
+  ObjectRecordUpdateEvent<WorkspaceMemberWorkspaceEntity>
+> =>
+  ({
+    workspaceId: WORKSPACE_ID,
+    events: [
+      {
+        properties: { before, after, updatedFields, diff: {} },
+      },
+    ],
+  }) as unknown as WorkspaceEventBatch<
+    ObjectRecordUpdateEvent<WorkspaceMemberWorkspaceEntity>
+  >;
+
+describe('WorkspaceMemberAvatarFileDeletionListener', () => {
+  let listener: WorkspaceMemberAvatarFileDeletionListener;
+  let deleteCorePicture: jest.Mock;
+
+  beforeEach(() => {
+    deleteCorePicture = jest.fn().mockResolvedValue(undefined);
+
+    listener = new WorkspaceMemberAvatarFileDeletionListener({
+      deleteCorePicture,
+    } as unknown as FileCorePictureService);
+  });
+
+  it('does not delete the avatar file when avatarUrl is unchanged', async () => {
+    await listener.handleUpdate(
+      buildUpdateBatch({
+        before: { avatarUrl: OLD_URL },
+        after: {
+          name: { firstName: 'Tim', lastName: 'Apple' },
+          avatarUrl: OLD_URL,
+        },
+        updatedFields: ['name'],
+      }),
+    );
+
+    expect(deleteCorePicture).not.toHaveBeenCalled();
+  });
+
+  it('deletes the previous avatar file when the avatar is replaced', async () => {
+    await listener.handleUpdate(
+      buildUpdateBatch({
+        before: { avatarUrl: OLD_URL },
+        after: { avatarUrl: NEW_URL },
+        updatedFields: ['avatarUrl'],
+      }),
+    );
+
+    expect(deleteCorePicture).toHaveBeenCalledTimes(1);
+    expect(deleteCorePicture).toHaveBeenCalledWith({
+      workspaceId: WORKSPACE_ID,
+      fileId: OLD_FILE_ID,
+    });
+  });
+
+  it('deletes the previous avatar file when the avatar is removed', async () => {
+    await listener.handleUpdate(
+      buildUpdateBatch({
+        before: { avatarUrl: OLD_URL },
+        after: { avatarUrl: null },
+        updatedFields: ['avatarUrl'],
+      }),
+    );
+
+    expect(deleteCorePicture).toHaveBeenCalledTimes(1);
+    expect(deleteCorePicture).toHaveBeenCalledWith({
+      workspaceId: WORKSPACE_ID,
+      fileId: OLD_FILE_ID,
+    });
+  });
+});