chore: Missing react key in SuggestionMenu

* Manage collection subscriptions when user (or) group is removed from a collection

* rename collection task

* rename document task

* remove unnecessary actor filter

app/editor/components/SuggestionsMenu.tsx

@@ -645,12 +645,11 @@ function SuggestionsMenu<T extends MenuItem>(props: Props<T>) {
                     "section" in item ? item.section?.({ t }) : undefined;
 
                   const response = (
-                    <>
+                    <React.Fragment key={`${index}-${item.name}`}>
                       {currentHeading !== previousHeading && (
                         <Header key={currentHeading}>{currentHeading}</Header>
                       )}
                       <ListItem
-                        key={index}
                         onPointerMove={handlePointerMove}
                         onPointerDown={handlePointerDown}
                       >
@@ -659,7 +658,7 @@ function SuggestionsMenu<T extends MenuItem>(props: Props<T>) {
                           onClick: () => handleClickItem(item),
                         })}
                       </ListItem>
-                    </>
+                    </React.Fragment>
                   );
 
                   previousHeading = currentHeading;

app/models/Import.ts

@@ -15,6 +15,9 @@ class Import extends Model {
   /** The name of the import. */
   name: string;
 
+  /** Descriptive error message when the import errors out. */
+  error: string | null;
+
   /** The current state of the import. */
   @Field
   @observable

app/scenes/Settings/components/ImportListItem.tsx

@@ -15,6 +15,7 @@ import Time from "~/components/Time";
 import useCurrentUser from "~/hooks/useCurrentUser";
 import useStores from "~/hooks/useStores";
 import { ImportMenu } from "~/menus/ImportMenu";
+import isCloudHosted from "~/utils/isCloudHosted";
 
 type Props = {
   /** Import that's displayed as list item. */
@@ -29,6 +30,10 @@ export const ImportListItem = observer(({ importModel }: Props) => {
   const showProgress =
     importModel.state !== ImportState.Canceled &&
     importModel.state !== ImportState.Errored;
+  const showErrorInfo =
+    !isCloudHosted &&
+    importModel.state === ImportState.Errored &&
+    !!importModel.error;
 
   const stateMap = React.useMemo(
     () => ({
@@ -114,6 +119,12 @@ export const ImportListItem = observer(({ importModel }: Props) => {
       subtitle={
         <>
           {stateMap[importModel.state]}&nbsp;•&nbsp;
+          {showErrorInfo && (
+            <>
+              {importModel.error}
+              {`. ${t("Check server logs for more details.")}`}&nbsp;•&nbsp;
+            </>
+          )}
           {t(`{{userName}} requested`, {
             userName:
               user.id === importModel.createdBy.id

server/migrations/20250327110739-add-error-to-imports.js

@@ -0,0 +1,37 @@
+"use strict";
+
+/** @type {import('sequelize-cli').Migration} */
+module.exports = {
+  async up(queryInterface, Sequelize) {
+    await queryInterface.sequelize.transaction(async transaction => {
+      await queryInterface.addColumn(
+        "imports",
+        "error",
+        {
+          type: Sequelize.STRING,
+          allowNull: true,
+        },
+        { transaction }
+      );
+
+      await queryInterface.addColumn(
+        "import_tasks",
+        "error",
+        {
+          type: Sequelize.STRING,
+          allowNull: true,
+        },
+        { transaction }
+      );
+    });
+  },
+
+  async down(queryInterface, Sequelize) {
+    await queryInterface.sequelize.transaction(async transaction => {
+      await queryInterface.removeColumn("imports", "error", { transaction });
+      await queryInterface.removeColumn("import_tasks", "error", {
+        transaction,
+      });
+    });
+  },
+};

server/models/Import.ts

@@ -60,6 +60,9 @@ class Import<T extends ImportableIntegrationService> extends ParanoidModel<
   @Column(DataType.INTEGER)
   documentCount: number;
 
+  @Column
+  error: string | null;
+
   // associations
 
   @BelongsTo(() => Integration, "integrationId")

server/models/ImportTask.ts

@@ -45,6 +45,9 @@ class ImportTask<T extends ImportableIntegrationService> extends IdModel<
   @Column(DataType.JSONB)
   output: ImportTaskOutput | null;
 
+  @Column
+  error: string | null;
+
   // associations
 
   @BelongsTo(() => Import, "importId")

server/models/helpers/NotificationHelper.test.ts

@@ -222,6 +222,13 @@ describe("NotificationHelper", () => {
         documentId: document.id,
       });
 
+      const deletedUser = await buildUser({ teamId: document.teamId });
+      await buildSubscription({
+        userId: deletedUser.id,
+        documentId: document.id,
+      });
+      await deletedUser.destroy();
+
       const recipients =
         await NotificationHelper.getDocumentNotificationRecipients({
           document,

server/models/helpers/NotificationHelper.ts

@@ -201,6 +201,7 @@ export default class NotificationHelper {
         include: [
           {
             association: "user",
+            required: true,
           },
         ],
       });

server/presenters/import.ts

@@ -11,6 +11,7 @@ export default function presentImport(
     service: importModel.service,
     state: importModel.state,
     documentCount: importModel.documentCount,
+    error: importModel.error,
     createdBy: presentUser(importModel.createdBy),
     createdById: importModel.createdById,
     createdAt: importModel.createdAt,

server/queues/processors/DocumentSubscriptionProcessor.ts

@@ -1,44 +1,85 @@
-import { Op } from "sequelize";
 import { GroupUser } from "@server/models";
-import { DocumentGroupEvent, DocumentUserEvent, Event } from "@server/types";
-import DocumentSubscriptionTask from "../tasks/DocumentSubscriptionTask";
+import {
+  CollectionGroupEvent,
+  CollectionUserEvent,
+  DocumentGroupEvent,
+  DocumentUserEvent,
+  Event,
+} from "@server/types";
+import CollectionSubscriptionRemoveUserTask from "../tasks/CollectionSubscriptionRemoveUserTask";
+import DocumentSubscriptionRemoveUserTask from "../tasks/DocumentSubscriptionRemoveUserTask";
 import BaseProcessor from "./BaseProcessor";
 
+type ReceivedEvent =
+  | CollectionUserEvent
+  | CollectionGroupEvent
+  | DocumentUserEvent
+  | DocumentGroupEvent;
+
 export default class DocumentSubscriptionProcessor extends BaseProcessor {
   static applicableEvents: Event["name"][] = [
+    "collections.remove_user",
+    "collections.remove_group",
     "documents.remove_user",
     "documents.remove_group",
   ];
 
-  async perform(event: DocumentUserEvent | DocumentGroupEvent) {
+  async perform(event: ReceivedEvent) {
     switch (event.name) {
+      case "collections.remove_user": {
+        await CollectionSubscriptionRemoveUserTask.schedule(event);
+        return;
+      }
+
+      case "collections.remove_group":
+        return this.handleRemoveGroupFromCollection(event);
+
       case "documents.remove_user": {
-        await DocumentSubscriptionTask.schedule(event);
+        await DocumentSubscriptionRemoveUserTask.schedule(event);
         return;
       }
 
       case "documents.remove_group":
-        return this.handleGroup(event);
+        return this.handleRemoveGroupFromDocument(event);
 
       default:
     }
   }
 
-  private async handleGroup(event: DocumentGroupEvent) {
+  private async handleRemoveGroupFromCollection(event: CollectionGroupEvent) {
     await GroupUser.findAllInBatches<GroupUser>(
       {
         where: {
           groupId: event.modelId,
-          userId: {
-            [Op.ne]: event.actorId,
-          },
         },
         batchLimit: 10,
       },
       async (groupUsers) => {
         await Promise.all(
           groupUsers.map((groupUser) =>
-            DocumentSubscriptionTask.schedule({
+            CollectionSubscriptionRemoveUserTask.schedule({
+              ...event,
+              name: "collections.remove_user",
+              userId: groupUser.userId,
+            })
+          )
+        );
+      }
+    );
+  }
+
+  private async handleRemoveGroupFromDocument(event: DocumentGroupEvent) {
+    await GroupUser.findAllInBatches<GroupUser>(
+      {
+        where: {
+          groupId: event.modelId,
+        },
+        batchLimit: 10,
+      },
+      async (groupUsers) => {
+        await Promise.all(
+          groupUsers.map((groupUser) =>
+            DocumentSubscriptionRemoveUserTask.schedule({
               ...event,
               name: "documents.remove_user",
               userId: groupUser.userId,

server/queues/processors/ImportsProcessor.ts

@@ -49,33 +49,46 @@ export default abstract class ImportsProcessor<
    * @param event The import event
    */
   public async perform(event: ImportEvent) {
-    await sequelize.transaction(async (transaction) => {
-      const importModel = await Import.findByPk<Import<T>>(event.modelId, {
-        rejectOnEmpty: true,
-        paranoid: false,
-        transaction,
-        lock: transaction.LOCK.UPDATE,
+    try {
+      await sequelize.transaction(async (transaction) => {
+        const importModel = await Import.findByPk<Import<T>>(event.modelId, {
+          rejectOnEmpty: true,
+          paranoid: false,
+          transaction,
+          lock: transaction.LOCK.UPDATE,
+        });
+
+        if (
+          !this.canProcess(importModel) ||
+          importModel.state === ImportState.Errored ||
+          importModel.state === ImportState.Canceled
+        ) {
+          return;
+        }
+
+        switch (event.name) {
+          case "imports.create":
+            return this.onCreation(importModel, transaction);
+
+          case "imports.processed":
+            return this.onProcessed(importModel, transaction);
+
+          case "imports.delete":
+            return this.onDeletion(importModel, event, transaction);
+        }
       });
-
-      if (
-        !this.canProcess(importModel) ||
-        importModel.state === ImportState.Errored ||
-        importModel.state === ImportState.Canceled
-      ) {
-        return;
+    } catch (err) {
+      if (event.name !== "imports.delete" && err instanceof Error) {
+        const importModel = await Import.findByPk<Import<T>>(event.modelId, {
+          rejectOnEmpty: true,
+          paranoid: false,
+        });
+        importModel.error = truncate(err.message, { length: 255 });
+        await importModel.save();
       }
 
-      switch (event.name) {
-        case "imports.create":
-          return this.onCreation(importModel, transaction);
-
-        case "imports.processed":
-          return this.onProcessed(importModel, transaction);
-
-        case "imports.delete":
-          return this.onDeletion(importModel, event, transaction);
-      }
-    });
+      throw err; // throw error for retry.
+    }
   }
 
   public async onFailed(event: ImportEvent) {
@@ -173,6 +186,7 @@ export default abstract class ImportsProcessor<
     }
 
     importModel.state = ImportState.Completed;
+    importModel.error = null; // unset any error from previous attempts.
     await importModel.saveWithCtx(
       createContext({
         user: importModel.createdBy,

server/queues/tasks/APIImportTask.ts

@@ -1,5 +1,6 @@
 import { JobOptions } from "bull";
 import chunk from "lodash/chunk";
+import truncate from "lodash/truncate";
 import uniqBy from "lodash/uniqBy";
 import { Fragment, Node } from "prosemirror-model";
 import { Transaction, WhereOptions } from "sequelize";
@@ -63,20 +64,29 @@ export default abstract class APIImportTask<
       return;
     }
 
-    switch (importTask.state) {
-      case ImportTaskState.Created: {
-        importTask.state = ImportTaskState.InProgress;
-        importTask = await importTask.save();
-        return await this.onProcess(importTask);
+    try {
+      switch (importTask.state) {
+        case ImportTaskState.Created: {
+          importTask.state = ImportTaskState.InProgress;
+          importTask = await importTask.save();
+          return await this.onProcess(importTask);
+        }
+
+        case ImportTaskState.InProgress:
+          return await this.onProcess(importTask);
+
+        case ImportTaskState.Completed:
+          return await this.onCompletion(importTask);
+
+        default:
+      }
+    } catch (err) {
+      if (err instanceof Error) {
+        importTask.error = truncate(err.message, { length: 255 });
+        await importTask.save();
       }
 
-      case ImportTaskState.InProgress:
-        return await this.onProcess(importTask);
-
-      case ImportTaskState.Completed:
-        return await this.onCompletion(importTask);
-
-      default:
+      throw err; // throw error for retry.
     }
   }
 
@@ -108,6 +118,7 @@ export default abstract class APIImportTask<
       await importTask.save({ transaction });
 
       const associatedImport = importTask.import;
+      associatedImport.error = importTask.error; // copy error from ImportTask that caused the failure.
       associatedImport.state = ImportState.Errored;
       await associatedImport.saveWithCtx(
         createContext({
@@ -155,6 +166,7 @@ export default abstract class APIImportTask<
 
       importTask.output = taskOutputWithReplacements;
       importTask.state = ImportTaskState.Completed;
+      importTask.error = null; // unset any error from previous attempts.
       await importTask.save({ transaction });
 
       const associatedImport = importTask.import;

server/queues/tasks/CollectionSubscriptionRemoveUserTask.ts

@@ -0,0 +1,52 @@
+import { Transaction } from "sequelize";
+import { SubscriptionType } from "@shared/types";
+import { createContext } from "@server/context";
+import Logger from "@server/logging/Logger";
+import { Collection, Subscription, User } from "@server/models";
+import { can } from "@server/policies";
+import { sequelize } from "@server/storage/database";
+import { CollectionUserEvent } from "@server/types";
+import BaseTask from "./BaseTask";
+
+export default class CollectionSubscriptionRemoveUserTask extends BaseTask<CollectionUserEvent> {
+  public async perform(event: CollectionUserEvent) {
+    const user = await User.findByPk(event.userId);
+
+    if (!user) {
+      return;
+    }
+
+    const collection = await Collection.scope({
+      method: ["withMembership", user.id],
+    }).findByPk(event.collectionId);
+
+    if (can(user, "read", collection)) {
+      Logger.debug(
+        "task",
+        `Skip unsubscribing user ${user.id} as they have permission to the collection ${event.collectionId} through other means`
+      );
+      return;
+    }
+
+    await sequelize.transaction(async (transaction) => {
+      const subscription = await Subscription.findOne({
+        where: {
+          userId: user.id,
+          collectionId: event.collectionId,
+          event: SubscriptionType.Document,
+        },
+        transaction,
+        lock: Transaction.LOCK.UPDATE,
+      });
+
+      await subscription?.destroyWithCtx(
+        createContext({
+          user,
+          authType: event.authType,
+          ip: event.ip,
+          transaction,
+        })
+      );
+    });
+  }
+}

server/queues/tasks/DocumentSubscriptionRemoveUserTask.ts

@@ -8,11 +8,11 @@ import { sequelize } from "@server/storage/database";
 import { DocumentUserEvent } from "@server/types";
 import BaseTask from "./BaseTask";
 
-export default class DocumentSubscriptionTask extends BaseTask<DocumentUserEvent> {
+export default class DocumentSubscriptionRemoveUserTask extends BaseTask<DocumentUserEvent> {
   public async perform(event: DocumentUserEvent) {
     const user = await User.findByPk(event.userId);
 
-    if (!user || event.name !== "documents.remove_user") {
+    if (!user) {
       return;
     }
 

server/queues/tasks/ErrorTimedOutImportsTask.ts

@@ -56,11 +56,13 @@ export default class ErrorTimedOutImportsTask extends BaseTask<Props> {
 
             await sequelize.transaction(async (transaction) => {
               importTask.state = ImportTaskState.Errored;
+              importTask.error = "Timed out";
               await importTask.save({ transaction });
 
               // this import could have been seen before in another import_task.
               if (!importsErrored[associatedImport.id]) {
                 associatedImport.state = ImportState.Errored;
+                associatedImport.error = "Timed out";
                 await associatedImport.save({ transaction });
                 importsErrored[associatedImport.id] = true;
               }

shared/editor/nodes/Mention.tsx

@@ -32,6 +32,11 @@ export default class Mention extends Node {
   }
 
   get schema(): NodeSpec {
+    const toPlainText = (node: ProsemirrorNode) =>
+      node.attrs.type === MentionType.User
+        ? `@${node.attrs.label}`
+        : node.attrs.label;
+
     return {
       attrs: {
         type: {
@@ -88,12 +93,9 @@ export default class Mention extends Node {
           "data-actorid": node.attrs.actorId,
           "data-url": `mention://${node.attrs.id}/${node.attrs.type}/${node.attrs.modelId}`,
         },
-        String(node.attrs.label),
+        toPlainText(node),
       ],
-      toPlainText: (node) =>
-        node.attrs.type === MentionType.User
-          ? `@${node.attrs.label}`
-          : node.attrs.label,
+      toPlainText,
     };
   }