fix: User updates are not synced between clients (#6490)

* Add Model.changeset method to get minified changes since last update

* fix: Handle arrays

* Add changes column, types

* test

app/components/WebsocketProvider.tsx

@@ -88,6 +88,7 @@ class WebsocketProvider extends React.Component<Props> {
       pins,
       stars,
       memberships,
+      users,
       userMemberships,
       policies,
       comments,
@@ -509,6 +510,10 @@ class WebsocketProvider extends React.Component<Props> {
       }
     );
 
+    this.socket.on("users.update", (event: PartialWithId<User>) => {
+      users.add(event);
+    });
+
     this.socket.on("users.demote", async (event: PartialWithId<User>) => {
       if (event.id === auth.user?.id) {
         documents.all.forEach((document) => policies.remove(document.id));

plugins/webhooks/server/tasks/DeliverWebhookTask.ts

@@ -184,6 +184,8 @@ export default class DeliverWebhookTask extends BaseTask<Props> {
         await this.handleIntegrationEvent(subscription, event);
         return;
       case "teams.create":
+      case "teams.delete":
+      case "teams.destroy":
         // Ignored
         return;
       case "teams.update":

server/migrations/20240204171556-add-event-changeset.js

@@ -0,0 +1,14 @@
+"use strict";
+
+module.exports = {
+  async up(queryInterface, Sequelize) {
+    await queryInterface.addColumn("events", "changes", {
+      type: Sequelize.JSONB,
+      allowNull: true,
+    });
+    
+  },
+  async down(queryInterface) {
+    await queryInterface.removeColumn("events", "changes");
+  },
+};

server/models/Event.ts

@@ -1,4 +1,5 @@
 import type {
+  CreateOptions,
   InferAttributes,
   InferCreationAttributes,
   SaveOptions,
@@ -17,7 +18,7 @@ import {
   Length,
 } from "sequelize-typescript";
 import { globalEventQueue } from "../queues";
-import { Event as TEvent } from "../types";
+import { APIContext, Event as TEvent } from "../types";
 import Collection from "./Collection";
 import Document from "./Document";
 import Team from "./Team";
@@ -35,20 +36,36 @@ class Event extends IdModel<
   @Column(DataType.UUID)
   modelId: string | null;
 
+  /**
+   * The name of the event.
+   */
   @Length({
     max: 255,
     msg: "name must be 255 characters or less",
   })
-  @Column
+  @Column(DataType.STRING)
   name: string;
 
+  /**
+   * The originating IP address of the event.
+   */
   @IsIP
   @Column
   ip: string | null;
 
+  /**
+   * Metadata associated with the event, previously used for storing some changed attributes.
+   */
   @Column(DataType.JSONB)
   data: Record<string, any> | null;
 
+  /**
+   * The changes made to the model – gradually moving to this column away from `data` which can be
+   * used for arbitrary data associated with the event.
+   */
+  @Column(DataType.JSONB)
+  changes?: Record<string, any> | null;
+
   // hooks
 
   @BeforeCreate
@@ -132,6 +149,30 @@ class Event extends IdModel<
     });
   }
 
+  /**
+   * Create and persist new event from request context
+   *
+   * @param ctx The request context to use
+   * @param attributes The event attributes
+   * @returns A promise resolving to the new event
+   */
+  static createFromContext(
+    ctx: APIContext,
+    attributes: Omit<Partial<Event>, "ip" | "teamId" | "actorId"> = {},
+    options?: CreateOptions<InferAttributes<Event>>
+  ) {
+    const { user } = ctx.state.auth;
+    return this.create(
+      {
+        ...attributes,
+        actorId: user.id,
+        teamId: user.teamId,
+        ip: ctx.request.ip,
+      },
+      options
+    );
+  }
+
   static ACTIVITY_EVENTS: TEvent["name"][] = [
     "collections.create",
     "collections.delete",

server/models/Team.ts

@@ -228,8 +228,11 @@ class Team extends ParanoidModel<
     if (!this.preferences) {
       this.preferences = {};
     }
-    this.preferences[preference] = value;
-    this.changed("preferences", true);
+
+    this.preferences = {
+      ...this.preferences,
+      [preference]: value,
+    };
 
     return this.preferences;
   };

server/models/User.ts

@@ -290,8 +290,10 @@ class User extends ParanoidModel<
     type: NotificationEventType,
     value = true
   ) => {
-    this.notificationSettings[type] = value;
-    this.changed("notificationSettings", true);
+    this.notificationSettings = {
+      ...this.notificationSettings,
+      [type]: value,
+    };
   };
 
   /**
@@ -318,8 +320,10 @@ class User extends ParanoidModel<
     }
     const binary = value ? 1 : 0;
     if (this.flags[flag] !== binary) {
-      this.flags[flag] = binary;
-      this.changed("flags", true);
+      this.flags = {
+        ...this.flags,
+        [flag]: binary,
+      };
     }
 
     return this.flags;
@@ -345,9 +349,10 @@ class User extends ParanoidModel<
     if (!this.flags) {
       this.flags = {};
     }
-    this.flags[flag] = (this.flags[flag] ?? 0) + value;
-    this.changed("flags", true);
-
+    this.flags = {
+      ...this.flags,
+      [flag]: (this.flags[flag] ?? 0) + value,
+    };
     return this.flags;
   };
 
@@ -362,9 +367,10 @@ class User extends ParanoidModel<
     if (!this.preferences) {
       this.preferences = {};
     }
-    this.preferences[preference] = value;
-    this.changed("preferences", true);
-
+    this.preferences = {
+      ...this.preferences,
+      [preference]: value,
+    };
     return this.preferences;
   };
 

server/models/base/Model.test.ts

@@ -0,0 +1,52 @@
+import { v4 as uuid } from "uuid";
+import { TeamPreference } from "@shared/types";
+import { buildDocument, buildTeam } from "@server/test/factories";
+
+describe("Model", () => {
+  describe("changeset", () => {
+    it("should return attributes changed since last save", async () => {
+      const team = await buildTeam({
+        name: "Test Team",
+      });
+      team.name = "New Name";
+      expect(Object.keys(team.changeset.attributes).length).toEqual(1);
+      expect(Object.keys(team.changeset.previous).length).toEqual(1);
+      expect(team.changeset.attributes.name).toEqual("New Name");
+      expect(team.changeset.previous.name).toEqual("Test Team");
+
+      await team.save();
+      expect(team.changeset.attributes).toEqual({});
+      expect(team.changeset.previous).toEqual({});
+    });
+
+    it("should return partial of objects", async () => {
+      const team = await buildTeam();
+      team.setPreference(TeamPreference.Commenting, false);
+      expect(team.changeset.attributes.preferences).toEqual({
+        commenting: false,
+      });
+      expect(team.changeset.previous.preferences).toEqual({});
+    });
+
+    it("should return boolean values", async () => {
+      const team = await buildTeam({
+        guestSignin: false,
+      });
+      team.guestSignin = true;
+      expect(team.changeset.attributes.guestSignin).toEqual(true);
+      expect(team.changeset.previous.guestSignin).toEqual(false);
+    });
+
+    it("should return full array if value changed", async () => {
+      const collaboratorId = uuid();
+      const document = await buildDocument();
+      const prev = document.collaboratorIds;
+
+      document.collaboratorIds = [...document.collaboratorIds, collaboratorId];
+      expect(document.changeset.attributes.collaboratorIds).toEqual(
+        document.collaboratorIds
+      );
+      expect(document.changeset.previous.collaboratorIds).toEqual(prev);
+    });
+  });
+});

server/models/base/Model.ts

@@ -1,5 +1,9 @@
 /* eslint-disable @typescript-eslint/ban-types */
-import { FindOptions } from "sequelize";
+import isArray from "lodash/isArray";
+import isEqual from "lodash/isEqual";
+import isObject from "lodash/isObject";
+import pick from "lodash/pick";
+import { FindOptions, NonAttribute } from "sequelize";
 import { Model as SequelizeModel } from "sequelize-typescript";
 
 class Model<
@@ -31,6 +35,60 @@ class Model<
       query.offset += query.limit;
     } while (results.length >= query.limit);
   }
+
+  /**
+   * Returns the attributes that have changed since the last save and their previous values.
+   *
+   * @returns An object with `attributes` and `previousAttributes` keys.
+   */
+  public get changeset(): NonAttribute<{
+    attributes: Partial<TModelAttributes>;
+    previous: Partial<TModelAttributes>;
+  }> {
+    const changes = this.changed() as Array<keyof TModelAttributes> | false;
+    const attributes: Partial<TModelAttributes> = {};
+    const previousAttributes: Partial<TModelAttributes> = {};
+
+    if (!changes) {
+      return {
+        attributes,
+        previous: previousAttributes,
+      };
+    }
+
+    for (const change of changes) {
+      const previous = this.previous(change);
+      const current = this.getDataValue(change);
+
+      if (
+        isObject(previous) &&
+        isObject(current) &&
+        !isArray(previous) &&
+        !isArray(current)
+      ) {
+        const difference = Object.keys(previous)
+          .concat(Object.keys(current))
+          .filter((key) => !isEqual(previous[key], current[key]));
+
+        previousAttributes[change] = pick(
+          previous,
+          difference
+        ) as TModelAttributes[keyof TModelAttributes];
+        attributes[change] = pick(
+          current,
+          difference
+        ) as TModelAttributes[keyof TModelAttributes];
+      } else {
+        previousAttributes[change] = previous;
+        attributes[change] = current;
+      }
+    }
+
+    return {
+      attributes,
+      previous: previousAttributes,
+    };
+  }
 }
 
 export default Model;

server/queues/processors/WebsocketsProcessor.ts

@@ -15,6 +15,7 @@ import {
   Subscription,
   Notification,
   UserMembership,
+  User,
 } from "@server/models";
 import {
   presentComment,
@@ -27,6 +28,7 @@ import {
   presentSubscription,
   presentTeam,
   presentMembership,
+  presentUser,
 } from "@server/presenters";
 import presentNotification from "@server/presenters/notification";
 import { Event } from "../../types";
@@ -667,6 +669,19 @@ export default class WebsocketsProcessor {
           .emit(event.name, presentTeam(team));
       }
 
+      case "users.update": {
+        const user = await User.findByPk(event.userId);
+        if (!user) {
+          return;
+        }
+        socketio
+          .to(`user-${event.userId}`)
+          .emit(event.name, presentUser(user, { includeDetails: true }));
+
+        socketio.to(`team-${user.teamId}`).emit(event.name, presentUser(user));
+        return;
+      }
+
       case "users.demote": {
         return socketio
           .to(`user-${event.userId}`)

server/routes/api/users/users.ts

@@ -226,17 +226,17 @@ router.post(
         user.setPreference(key, preferences[key] as boolean);
       }
     }
-    await user.save({ transaction });
-    await Event.create(
+
+    await Event.createFromContext(
+      ctx,
       {
         name: "users.update",
-        actorId: user.id,
         userId: user.id,
-        teamId: user.teamId,
-        ip: ctx.request.ip,
+        changes: user.changeset,
       },
       { transaction }
     );
+    await user.save({ transaction });
 
     ctx.body = {
       data: presentUser(user, {
@@ -547,9 +547,18 @@ router.post(
   async (ctx: APIContext<T.UsersNotificationsSubscribeReq>) => {
     const { eventType } = ctx.input.body;
     const { transaction } = ctx.state;
-
     const { user } = ctx.state.auth;
     user.setNotificationEventType(eventType, true);
+
+    await Event.createFromContext(
+      ctx,
+      {
+        name: "users.update",
+        userId: user.id,
+        changes: user.changeset,
+      },
+      { transaction }
+    );
     await user.save({ transaction });
 
     ctx.body = {
@@ -566,9 +575,18 @@ router.post(
   async (ctx: APIContext<T.UsersNotificationsUnsubscribeReq>) => {
     const { eventType } = ctx.input.body;
     const { transaction } = ctx.state;
-
     const { user } = ctx.state.auth;
     user.setNotificationEventType(eventType, false);
+
+    await Event.createFromContext(
+      ctx,
+      {
+        name: "users.update",
+        userId: user.id,
+        changes: user.changeset,
+      },
+      { transaction }
+    );
     await user.save({ transaction });
 
     ctx.body = {

server/types.ts

@@ -1,6 +1,6 @@
 import { ParameterizedContext, DefaultContext } from "koa";
 import { IRouterParamContext } from "koa-router";
-import { Transaction } from "sequelize";
+import { InferAttributes, Model, Transaction } from "sequelize";
 import { z } from "zod";
 import {
   CollectionSort,
@@ -11,7 +11,29 @@ import {
 } from "@shared/types";
 import { BaseSchema } from "@server/routes/api/schema";
 import { AccountProvisionerResult } from "./commands/accountProvisioner";
-import { FileOperation, Team, User } from "./models";
+import type {
+  ApiKey,
+  Attachment,
+  AuthenticationProvider,
+  FileOperation,
+  Revision,
+  Team,
+  User,
+  UserMembership,
+  WebhookSubscription,
+  Pin,
+  Star,
+  Document,
+  Collection,
+  Group,
+  Integration,
+  Comment,
+  Subscription,
+  View,
+  Notification,
+  Share,
+  GroupPermission,
+} from "./models";
 
 export enum AuthenticationType {
   API = "api",
@@ -56,13 +78,17 @@ export interface APIContext<ReqT = BaseReq, ResT = BaseRes>
   input: ReqT;
 }
 
-type BaseEvent = {
+type BaseEvent<T extends Model> = {
   teamId: string;
   actorId: string;
   ip: string;
+  changes?: {
+    attributes: Partial<InferAttributes<T>>;
+    previous: Partial<InferAttributes<T>>;
+  };
 };
 
-export type ApiKeyEvent = BaseEvent & {
+export type ApiKeyEvent = BaseEvent<ApiKey> & {
   name: "api_keys.create" | "api_keys.delete";
   modelId: string;
   data: {
@@ -70,7 +96,7 @@ export type ApiKeyEvent = BaseEvent & {
   };
 };
 
-export type AttachmentEvent = BaseEvent &
+export type AttachmentEvent = BaseEvent<Attachment> &
   (
     | {
         name: "attachments.create";
@@ -89,7 +115,7 @@ export type AttachmentEvent = BaseEvent &
       }
   );
 
-export type AuthenticationProviderEvent = BaseEvent & {
+export type AuthenticationProviderEvent = BaseEvent<AuthenticationProvider> & {
   name: "authenticationProviders.update";
   modelId: string;
   data: {
@@ -97,7 +123,7 @@ export type AuthenticationProviderEvent = BaseEvent & {
   };
 };
 
-export type UserEvent = BaseEvent &
+export type UserEvent = BaseEvent<User> &
   (
     | {
         name:
@@ -126,7 +152,7 @@ export type UserEvent = BaseEvent &
       }
   );
 
-export type UserMembershipEvent = BaseEvent & {
+export type UserMembershipEvent = BaseEvent<UserMembership> & {
   name: "userMemberships.update";
   modelId: string;
   userId: string;
@@ -136,9 +162,8 @@ export type UserMembershipEvent = BaseEvent & {
   };
 };
 
-export type DocumentEvent = BaseEvent &
+export type DocumentEvent = BaseEvent<Document> &
   (
-    | DocumentUserEvent
     | {
         name:
           | "documents.create"
@@ -191,14 +216,14 @@ export type DocumentEvent = BaseEvent &
       }
   );
 
-export type RevisionEvent = BaseEvent & {
+export type RevisionEvent = BaseEvent<Revision> & {
   name: "revisions.create";
   documentId: string;
   collectionId: string;
   modelId: string;
 };
 
-export type FileOperationEvent = BaseEvent & {
+export type FileOperationEvent = BaseEvent<FileOperation> & {
   name:
     | "fileOperations.create"
     | "fileOperations.update"
@@ -207,7 +232,7 @@ export type FileOperationEvent = BaseEvent & {
   data: Partial<FileOperation>;
 };
 
-export type CollectionUserEvent = BaseEvent & {
+export type CollectionUserEvent = BaseEvent<UserMembership> & {
   name: "collections.add_user" | "collections.remove_user";
   userId: string;
   modelId: string;
@@ -218,14 +243,14 @@ export type CollectionUserEvent = BaseEvent & {
   };
 };
 
-export type CollectionGroupEvent = BaseEvent & {
+export type CollectionGroupEvent = BaseEvent<GroupPermission> & {
   name: "collections.add_group" | "collections.remove_group";
   collectionId: string;
   modelId: string;
   data: { name: string };
 };
 
-export type DocumentUserEvent = BaseEvent & {
+export type DocumentUserEvent = BaseEvent<UserMembership> & {
   name: "documents.add_user" | "documents.remove_user";
   userId: string;
   modelId: string;
@@ -237,10 +262,8 @@ export type DocumentUserEvent = BaseEvent & {
   };
 };
 
-export type CollectionEvent = BaseEvent &
+export type CollectionEvent = BaseEvent<Collection> &
   (
-    | CollectionUserEvent
-    | CollectionGroupEvent
     | {
         name: "collections.create";
         collectionId: string;
@@ -273,7 +296,7 @@ export type CollectionEvent = BaseEvent &
       }
   );
 
-export type GroupUserEvent = BaseEvent & {
+export type GroupUserEvent = BaseEvent<UserMembership> & {
   name: "groups.add_user" | "groups.remove_user";
   userId: string;
   modelId: string;
@@ -282,7 +305,7 @@ export type GroupUserEvent = BaseEvent & {
   };
 };
 
-export type GroupEvent = BaseEvent &
+export type GroupEvent = BaseEvent<Group> &
   (
     | GroupUserEvent
     | {
@@ -294,24 +317,24 @@ export type GroupEvent = BaseEvent &
       }
   );
 
-export type IntegrationEvent = BaseEvent & {
+export type IntegrationEvent = BaseEvent<Integration> & {
   name: "integrations.create" | "integrations.update" | "integrations.delete";
   modelId: string;
 };
 
-export type TeamEvent = BaseEvent & {
-  name: "teams.create" | "teams.update";
+export type TeamEvent = BaseEvent<Team> & {
+  name: "teams.create" | "teams.update" | "teams.delete" | "teams.destroy";
   data: Partial<Team>;
 };
 
-export type PinEvent = BaseEvent & {
+export type PinEvent = BaseEvent<Pin> & {
   name: "pins.create" | "pins.update" | "pins.delete";
   modelId: string;
   documentId: string;
   collectionId?: string;
 };
 
-export type CommentUpdateEvent = BaseEvent & {
+export type CommentUpdateEvent = BaseEvent<Comment> & {
   name: "comments.update";
   modelId: string;
   documentId: string;
@@ -322,14 +345,14 @@ export type CommentUpdateEvent = BaseEvent & {
 };
 
 export type CommentEvent =
-  | (BaseEvent & {
+  | (BaseEvent<Comment> & {
       name: "comments.create";
       modelId: string;
       documentId: string;
       actorId: string;
     })
   | CommentUpdateEvent
-  | (BaseEvent & {
+  | (BaseEvent<Comment> & {
       name: "comments.delete";
       modelId: string;
       documentId: string;
@@ -337,14 +360,14 @@ export type CommentEvent =
       collectionId: string;
     });
 
-export type StarEvent = BaseEvent & {
+export type StarEvent = BaseEvent<Star> & {
   name: "stars.create" | "stars.update" | "stars.delete";
   modelId: string;
   documentId: string;
   userId: string;
 };
 
-export type ShareEvent = BaseEvent & {
+export type ShareEvent = BaseEvent<Share> & {
   name: "shares.create" | "shares.update" | "shares.revoke";
   modelId: string;
   documentId: string;
@@ -354,14 +377,14 @@ export type ShareEvent = BaseEvent & {
   };
 };
 
-export type SubscriptionEvent = BaseEvent & {
+export type SubscriptionEvent = BaseEvent<Subscription> & {
   name: "subscriptions.create" | "subscriptions.delete";
   modelId: string;
   userId: string;
   documentId: string | null;
 };
 
-export type ViewEvent = BaseEvent & {
+export type ViewEvent = BaseEvent<View> & {
   name: "views.create";
   documentId: string;
   collectionId: string;
@@ -373,7 +396,7 @@ export type ViewEvent = BaseEvent & {
 
 export type WebhookDeliveryStatus = "pending" | "success" | "failed";
 
-export type WebhookSubscriptionEvent = BaseEvent & {
+export type WebhookSubscriptionEvent = BaseEvent<WebhookSubscription> & {
   name:
     | "webhookSubscriptions.create"
     | "webhookSubscriptions.delete"
@@ -386,7 +409,7 @@ export type WebhookSubscriptionEvent = BaseEvent & {
   };
 };
 
-export type NotificationEvent = BaseEvent & {
+export type NotificationEvent = BaseEvent<Notification> & {
   name: "notifications.create" | "notifications.update";
   modelId: string;
   teamId: string;
@@ -402,10 +425,13 @@ export type Event =
   | AttachmentEvent
   | AuthenticationProviderEvent
   | DocumentEvent
+  | DocumentUserEvent
   | PinEvent
   | CommentEvent
   | StarEvent
   | CollectionEvent
+  | CollectionUserEvent
+  | CollectionGroupEvent
   | FileOperationEvent
   | IntegrationEvent
   | GroupEvent