Add inclusive logic for isNodeActive

closes #8825

app/editor/extensions/PasteHandler.tsx

@@ -88,7 +88,7 @@ export default class PasteHandler extends Extension {
 
             // If the users selection is currently in a code block then paste
             // as plain text, ignore all formatting and HTML content.
-            if (isInCode(state)) {
+            if (isInCode(state, { inclusive: true })) {
               event.preventDefault();
               view.dispatch(state.tr.insertText(text));
               return true;

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/components/EventBoundary.tsx

@@ -10,6 +10,10 @@ type Props = {
   captureEvents?: "all" | "pointer" | "click";
 };
 
+/**
+ * EventBoundary is a component that prevents events from propagating to parent elements.
+ * This is useful for preventing clicks or other interactions from bubbling up the DOM tree.
+ */
 const EventBoundary: React.FC<Props> = ({
   children,
   className,

shared/components/Flex.tsx

@@ -5,14 +5,26 @@ type JustifyValues = CSSProperties["justifyContent"];
 
 type AlignValues = CSSProperties["alignItems"];
 
+/**
+ * Flex is a styled component that provides a flexible box layout with convenient props.
+ * It simplifies the use of flexbox CSS properties with a clean, declarative API.
+ */
 const Flex = styled.div<{
+  /** Makes the component grow to fill available space */
   auto?: boolean;
+  /** Changes flex direction to column */
   column?: boolean;
+  /** Sets the align-items CSS property */
   align?: AlignValues;
+  /** Sets the justify-content CSS property */
   justify?: JustifyValues;
+  /** Enables flex-wrap */
   wrap?: boolean;
+  /** Controls flex-shrink behavior */
   shrink?: boolean;
+  /** Reverses the direction (row-reverse or column-reverse) */
   reverse?: boolean;
+  /** Sets gap between flex items in pixels */
   gap?: number;
 }>`
   display: flex;

shared/components/Squircle.tsx

@@ -11,6 +11,11 @@ type Props = {
   className?: string;
 };
 
+/**
+ * Squircle is a component that renders a square with rounded corners (squircle shape).
+ * It's commonly used for app icons, avatars, and other UI elements where a softer
+ * square shape is desired.
+ */
 const Squircle: React.FC<Props> = ({
   color,
   size = 28,

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,
     };
   }
 

shared/editor/queries/isInCode.ts

@@ -7,6 +7,8 @@ type Options = {
   onlyBlock?: boolean;
   /** Only check if the selection is inside a code mark. */
   onlyMark?: boolean;
+  /** If true then code must contain entire selection */
+  inclusive?: boolean;
 };
 
 /**
@@ -20,17 +22,29 @@ export function isInCode(state: EditorState, options?: Options): boolean {
   const { nodes, marks } = state.schema;
 
   if (!options?.onlyMark) {
-    if (nodes.code_block && isNodeActive(nodes.code_block)(state)) {
+    if (
+      nodes.code_block &&
+      isNodeActive(nodes.code_block, undefined, {
+        inclusive: options?.inclusive,
+      })(state)
+    ) {
       return true;
     }
-    if (nodes.code_fence && isNodeActive(nodes.code_fence)(state)) {
+    if (
+      nodes.code_fence &&
+      isNodeActive(nodes.code_fence, undefined, {
+        inclusive: options?.inclusive,
+      })(state)
+    ) {
       return true;
     }
   }
 
   if (!options?.onlyBlock) {
     if (marks.code_inline) {
-      return isMarkActive(marks.code_inline)(state);
+      return isMarkActive(marks.code_inline, undefined, {
+        inclusive: options?.inclusive,
+      })(state);
     }
   }
 

shared/editor/queries/isMarkActive.ts

@@ -6,6 +6,8 @@ import { getMarksBetween } from "./getMarksBetween";
 type Options = {
   /** Only return match if the range and attrs is exact */
   exact?: boolean;
+  /** If true then mark must contain entire selection */
+  inclusive?: boolean;
 };
 
 /**
@@ -40,7 +42,8 @@ export const isMarkActive =
             Object.keys(attrs).every(
               (key) => mark.attrs[key] === attrs[key]
             )) &&
-          (!options?.exact || (start === from && end === to))
+          (!options?.exact || (start === from && end === to)) &&
+          (!options?.inclusive || (start <= from && end >= to))
       );
     }
 

shared/editor/queries/isNodeActive.ts

@@ -3,31 +3,55 @@ import { EditorState } from "prosemirror-state";
 import { Primitive } from "utility-types";
 import { findParentNode } from "./findParentNode";
 
+type Options = {
+  /** Only return match if the range and attrs is exact */
+  exact?: boolean;
+  /** If true then node must contain entire selection */
+  inclusive?: boolean;
+};
+
 /**
  * Checks if a node is active in the current selection or not.
  *
  * @param type The node type to check.
  * @param attrs The attributes to check.
+ * @param options The options to use.
  * @returns A function that checks if a node is active in the current selection or not.
  */
 export const isNodeActive =
-  (type: NodeType, attrs: Record<string, Primitive> = {}) =>
-  (state: EditorState) => {
+  (type: NodeType, attrs?: Record<string, Primitive>, options?: Options) =>
+  (state: EditorState): boolean => {
     if (!type) {
       return false;
     }
 
-    const nodeAfter = state.selection.$from.nodeAfter;
-    let node = nodeAfter?.type === type ? nodeAfter : undefined;
+    const { from, to } = state.selection;
+    const nodeWithPos = findParentNode(
+      (node) =>
+        node.type === type &&
+        (!attrs ||
+          Object.keys(attrs).every((key) => node.attrs[key] === attrs[key]))
+    )(state.selection);
 
-    if (!node) {
-      const parent = findParentNode((n) => n.type === type)(state.selection);
-      node = parent?.node;
+    if (!nodeWithPos) {
+      return false;
     }
 
-    if (!Object.keys(attrs).length || !node) {
-      return !!node;
+    if (options?.inclusive) {
+      // Check if the node's position contains the entire selection
+      return (
+        nodeWithPos.pos <= from &&
+        nodeWithPos.pos + nodeWithPos.node.nodeSize >= to
+      );
     }
 
-    return node.hasMarkup(type, { ...node.attrs, ...attrs });
+    if (options?.exact) {
+      // Check if node's range exactly matches selection
+      return (
+        nodeWithPos.pos === from &&
+        nodeWithPos.pos + nodeWithPos.node.nodeSize === to
+      );
+    }
+
+    return true;
   };