diff --git a/app/actions/definitions/collections.tsx b/app/actions/definitions/collections.tsx index d0f4af5890..ee97fd7b33 100644 --- a/app/actions/definitions/collections.tsx +++ b/app/actions/definitions/collections.tsx @@ -8,8 +8,10 @@ import { SearchIcon, ShapesIcon, StarredIcon, + SubscribeIcon, TrashIcon, UnstarredIcon, + UnsubscribeIcon, } from "outline-icons"; import * as React from "react"; import { toast } from "sonner"; @@ -205,6 +207,66 @@ export const unstarCollection = createAction({ }, }); +export const subscribeCollection = createAction({ + name: ({ t }) => t("Subscribe"), + analyticsName: "Subscribe to collection", + section: ActiveCollectionSection, + icon: , + visible: ({ activeCollectionId, stores }) => { + if (!activeCollectionId) { + return false; + } + + const collection = stores.collections.get(activeCollectionId); + + return ( + !collection?.isSubscribed && + stores.policies.abilities(activeCollectionId).subscribe + ); + }, + perform: async ({ activeCollectionId, stores, t }) => { + if (!activeCollectionId) { + return; + } + + const collection = stores.collections.get(activeCollectionId); + + await collection?.subscribe(); + + toast.success(t("Subscribed to document notifications")); + }, +}); + +export const unsubscribeCollection = createAction({ + name: ({ t }) => t("Unsubscribe"), + analyticsName: "Unsubscribe from collection", + section: ActiveCollectionSection, + icon: , + visible: ({ activeCollectionId, stores }) => { + if (!activeCollectionId) { + return false; + } + + const collection = stores.collections.get(activeCollectionId); + + return ( + !!collection?.isSubscribed && + stores.policies.abilities(activeCollectionId).unsubscribe + ); + }, + perform: async ({ activeCollectionId, currentUserId, stores, t }) => { + if (!activeCollectionId || !currentUserId) { + return; + } + + const collection = stores.collections.get(activeCollectionId); + + await collection?.unsubscribe(); + + toast.success(t("Unsubscribed from document notifications")); + }, +}); + export const archiveCollection = createAction({ name: ({ t }) => `${t("Archive")}…`, analyticsName: "Archive collection", @@ -331,5 +393,7 @@ export const rootCollectionActions = [ createCollection, starCollection, unstarCollection, + subscribeCollection, + unsubscribeCollection, deleteCollection, ]; diff --git a/app/actions/definitions/documents.tsx b/app/actions/definitions/documents.tsx index 7f58ff2e67..d7e2950f2a 100644 --- a/app/actions/definitions/documents.tsx +++ b/app/actions/definitions/documents.tsx @@ -333,6 +333,7 @@ export const subscribeDocument = createAction({ const document = stores.documents.get(activeDocumentId); return ( + !document?.collection?.isSubscribed && !document?.isSubscribed && stores.policies.abilities(activeDocumentId).subscribe ); @@ -361,8 +362,9 @@ export const unsubscribeDocument = createAction({ const document = stores.documents.get(activeDocumentId); return ( - !!document?.isSubscribed && - stores.policies.abilities(activeDocumentId).unsubscribe + !!document?.collection?.isSubscribed || + (!!document?.isSubscribed && + stores.policies.abilities(activeDocumentId).unsubscribe) ); }, perform: async ({ activeDocumentId, stores, currentUserId, t }) => { diff --git a/app/components/ContextMenu/Template.tsx b/app/components/ContextMenu/Template.tsx index cb4bf7c191..9dafb79759 100644 --- a/app/components/ContextMenu/Template.tsx +++ b/app/components/ContextMenu/Template.tsx @@ -20,6 +20,7 @@ import { MenuHeading, MenuItem as TMenuItem, } from "~/types"; +import Tooltip from "../Tooltip"; import Header from "./Header"; import MenuItem, { MenuAnchor } from "./MenuItem"; import MouseSafeArea from "./MouseSafeArea"; @@ -167,7 +168,7 @@ function Template({ items, actions, context, showIcons, ...menu }: Props) { } if (item.type === "button") { - return ( + const menuItem = ( ); + + return item.tooltip ? ( + +
{menuItem}
+ + ) : ( + <>{menuItem} + ); } if (item.type === "submenu") { diff --git a/app/menus/CollectionMenu.tsx b/app/menus/CollectionMenu.tsx index e5b8673721..800b638d72 100644 --- a/app/menus/CollectionMenu.tsx +++ b/app/menus/CollectionMenu.tsx @@ -14,6 +14,7 @@ import { useHistory } from "react-router-dom"; import { useMenuState, MenuButton, MenuButtonHTMLProps } from "reakit/Menu"; import { VisuallyHidden } from "reakit/VisuallyHidden"; import { toast } from "sonner"; +import { SubscriptionType } from "@shared/types"; import { getEventFiles } from "@shared/utils/files"; import Collection from "~/models/Collection"; import ContextMenu, { Placement } from "~/components/ContextMenu"; @@ -31,10 +32,13 @@ import { createTemplate, archiveCollection, restoreCollection, + subscribeCollection, + unsubscribeCollection, } from "~/actions/definitions/collections"; import useActionContext from "~/hooks/useActionContext"; import useCurrentTeam from "~/hooks/useCurrentTeam"; import usePolicy from "~/hooks/usePolicy"; +import useRequest from "~/hooks/useRequest"; import useStores from "~/hooks/useStores"; import { MenuItem } from "~/types"; import { newDocumentPath } from "~/utils/routeHelpers"; @@ -63,11 +67,28 @@ function CollectionMenu({ placement, }); const team = useCurrentTeam(); - const { documents, dialogs } = useStores(); + const { documents, dialogs, subscriptions } = useStores(); const { t } = useTranslation(); const history = useHistory(); const file = React.useRef(null); + const { + loading: subscriptionLoading, + loaded: subscriptionLoaded, + request: loadSubscription, + } = useRequest(() => + subscriptions.fetchOne({ + collectionId: collection.id, + event: SubscriptionType.Document, + }) + ); + + const handlePointerEnter = React.useCallback(() => { + if (!subscriptionLoading && !subscriptionLoaded) { + void loadSubscription(); + } + }, [subscriptionLoading, subscriptionLoaded, loadSubscription]); + const handleExport = React.useCallback(() => { dialogs.openModal({ title: t("Export collection"), @@ -157,6 +178,8 @@ function CollectionMenu({ actionToMenuItem(restoreCollection, context), actionToMenuItem(starCollection, context), actionToMenuItem(unstarCollection, context), + actionToMenuItem(subscribeCollection, context), + actionToMenuItem(unsubscribeCollection, context), { type: "separator", }, @@ -272,9 +295,15 @@ function CollectionMenu({ {label ? ( - {label} + + {label} + ) : ( - + )} = ({ label, onTrigger }) => { Promise.all([ subscriptions.fetchOne({ documentId: document.id, - event: "documents.update", + event: SubscriptionType.Document, }), + document.collectionId + ? subscriptions.fetchOne({ + collectionId: document.collectionId, + event: SubscriptionType.Document, + }) + : noop, pins.fetchOne({ documentId: document.id, collectionId: document.collectionId ?? null, @@ -254,8 +261,20 @@ const MenuContent: React.FC = observer(function MenuContent_({ }, actionToMenuItem(starDocument, context), actionToMenuItem(unstarDocument, context), - actionToMenuItem(subscribeDocument, context), - actionToMenuItem(unsubscribeDocument, context), + { + ...actionToMenuItem(subscribeDocument, context), + disabled: collection?.isSubscribed, + tooltip: collection?.isSubscribed + ? t("Subscription inherited from collection") + : undefined, + } as MenuItemButton, + { + ...actionToMenuItem(unsubscribeDocument, context), + disabled: collection?.isSubscribed, + tooltip: collection?.isSubscribed + ? t("Subscription inherited from collection") + : undefined, + } as MenuItemButton, { type: "button", title: `${t("Find and replace")}…`, diff --git a/app/models/Collection.ts b/app/models/Collection.ts index 47d2092e4b..f97bfcac43 100644 --- a/app/models/Collection.ts +++ b/app/models/Collection.ts @@ -129,6 +129,16 @@ export default class Collection extends ParanoidModel { ); } + /** + * Returns whether there is a subscription for this collection in the store. + * + * @returns True if there is a subscription, false otherwise. + */ + @computed + get isSubscribed(): boolean { + return !!this.store.rootStore.subscriptions.getByCollectionId(this.id); + } + @computed get isManualSort(): boolean { return this.sort.field === "index"; @@ -376,6 +386,22 @@ export default class Collection extends ParanoidModel { @action unstar = async () => this.store.unstar(this); + /** + * Subscribes the current user to this collection. + * + * @returns A promise that resolves when the subscription is created. + */ + @action + subscribe = () => this.store.subscribe(this); + + /** + * Unsubscribes the current user from this collection. + * + * @returns A promise that resolves when the subscription is destroyed. + */ + @action + unsubscribe = () => this.store.unsubscribe(this); + archive = () => this.store.archive(this); restore = () => this.store.restore(this); diff --git a/app/models/Subscription.ts b/app/models/Subscription.ts index e9d3bd70c7..341e3f12c3 100644 --- a/app/models/Subscription.ts +++ b/app/models/Subscription.ts @@ -1,4 +1,5 @@ import { observable } from "mobx"; +import Collection from "./Collection"; import Document from "./Document"; import User from "./User"; import Model from "./base/Model"; @@ -25,6 +26,13 @@ class Subscription extends Model { @Relation(() => Document, { onDelete: "cascade" }) document?: Document; + /** The collection ID being subscribed to */ + collectionId: string; + + /** The collection being subscribed to */ + @Relation(() => Collection, { onDelete: "cascade" }) + collection?: Collection; + /** The event being subscribed to */ @Field @observable diff --git a/app/stores/CollectionsStore.ts b/app/stores/CollectionsStore.ts index b0676bc94e..6014bfdebd 100644 --- a/app/stores/CollectionsStore.ts +++ b/app/stores/CollectionsStore.ts @@ -8,6 +8,7 @@ import { CollectionPermission, CollectionStatusFilter, FileOperationFormat, + SubscriptionType, } from "@shared/types"; import Collection from "~/models/Collection"; import { PaginationParams, Properties } from "~/types"; @@ -213,6 +214,20 @@ export default class CollectionsStore extends Store { await star?.delete(); }; + subscribe = (collection: Collection) => + this.rootStore.subscriptions.create({ + collectionId: collection.id, + event: SubscriptionType.Document, + }); + + unsubscribe = (collection: Collection) => { + const subscription = this.rootStore.subscriptions.getByCollectionId( + collection.id + ); + + return subscription?.delete(); + }; + @computed get navigationNodes() { return this.orderedData.map((collection) => collection.asNavigationNode); diff --git a/app/stores/DocumentsStore.ts b/app/stores/DocumentsStore.ts index 01e34bc79a..32ede115a2 100644 --- a/app/stores/DocumentsStore.ts +++ b/app/stores/DocumentsStore.ts @@ -5,11 +5,12 @@ import find from "lodash/find"; import omitBy from "lodash/omitBy"; import orderBy from "lodash/orderBy"; import { observable, action, computed, runInAction } from "mobx"; -import type { - DateFilter, - NavigationNode, - PublicTeam, - StatusFilter, +import { + SubscriptionType, + type DateFilter, + type NavigationNode, + type PublicTeam, + type StatusFilter, } from "@shared/types"; import { subtractDate } from "@shared/utils/date"; import { bytesToHumanReadable } from "@shared/utils/files"; @@ -817,7 +818,7 @@ export default class DocumentsStore extends Store { subscribe = (document: Document) => this.rootStore.subscriptions.create({ documentId: document.id, - event: "documents.update", + event: SubscriptionType.Document, }); unsubscribe = (document: Document) => { diff --git a/app/stores/SubscriptionsStore.ts b/app/stores/SubscriptionsStore.ts index cc2867a938..a7d9d23217 100644 --- a/app/stores/SubscriptionsStore.ts +++ b/app/stores/SubscriptionsStore.ts @@ -1,5 +1,6 @@ import invariant from "invariant"; import { action } from "mobx"; +import { SubscriptionType } from "@shared/types"; import Subscription from "~/models/Subscription"; import { client } from "~/utils/ApiClient"; import { AuthorizationError, NotFoundError } from "~/utils/errors"; @@ -14,8 +15,16 @@ export default class SubscriptionsStore extends Store { } @action - async fetchOne({ documentId, event }: { documentId: string; event: string }) { - const subscription = this.getByDocumentId(documentId); + async fetchOne( + options: { event: SubscriptionType } & ( + | { documentId: string } + | { collectionId: string } + ) + ) { + const subscription = + "collectionId" in options + ? this.getByCollectionId(options.collectionId) + : this.getByDocumentId(options.documentId); if (subscription) { return subscription; @@ -24,10 +33,7 @@ export default class SubscriptionsStore extends Store { this.isFetching = true; try { - const res = await client.post(`/${this.apiEndpoint}.info`, { - documentId, - event, - }); + const res = await client.post(`/${this.apiEndpoint}.info`, options); invariant(res?.data, "Data should be available"); return this.add(res.data); } catch (err) { @@ -42,4 +48,7 @@ export default class SubscriptionsStore extends Store { getByDocumentId = (documentId: string): Subscription | undefined => this.find({ documentId }); + + getByCollectionId = (collectionId: string): Subscription | undefined => + this.find({ collectionId }); } diff --git a/app/types.ts b/app/types.ts index 8431e63ad4..8068fd7361 100644 --- a/app/types.ts +++ b/app/types.ts @@ -27,6 +27,7 @@ export type MenuItemButton = { selected?: boolean; disabled?: boolean; icon?: React.ReactNode; + tooltip?: React.ReactChild; }; export type MenuItemWithChildren = { diff --git a/server/commands/subscriptionCreator.test.ts b/server/commands/subscriptionCreator.test.ts index 520b8de6cb..84a96515b4 100644 --- a/server/commands/subscriptionCreator.test.ts +++ b/server/commands/subscriptionCreator.test.ts @@ -1,14 +1,51 @@ +import { SubscriptionType } from "@shared/types"; import { createContext } from "@server/context"; import { Subscription, Event } from "@server/models"; import { sequelize } from "@server/storage/database"; -import { buildDocument, buildUser } from "@server/test/factories"; +import { + buildCollection, + buildDocument, + buildUser, +} from "@server/test/factories"; import subscriptionCreator from "./subscriptionCreator"; describe("subscriptionCreator", () => { const ip = "127.0.0.1"; - const subscribedEvent = "documents.update"; + const subscribedEvent = SubscriptionType.Document; - it("should create a subscription", async () => { + it("should create a document subscription for the whole collection", async () => { + const user = await buildUser(); + + const collection = await buildCollection({ + userId: user.id, + teamId: user.teamId, + }); + + const subscription = await sequelize.transaction(async (transaction) => + subscriptionCreator({ + ctx: createContext({ user, transaction, ip }), + collectionId: collection.id, + event: SubscriptionType.Document, + }) + ); + + const event = await Event.findOne({ + where: { + teamId: user.teamId, + }, + }); + + expect(subscription.collectionId).toEqual(collection.id); + expect(subscription.documentId).toBeNull(); + expect(subscription.userId).toEqual(user.id); + expect(event?.name).toEqual("subscriptions.create"); + expect(event?.modelId).toEqual(subscription.id); + expect(event?.actorId).toEqual(subscription.userId); + expect(event?.userId).toEqual(subscription.userId); + expect(event?.collectionId).toEqual(subscription.collectionId); + }); + + it("should create a document subscription", async () => { const user = await buildUser(); const document = await buildDocument({ @@ -31,6 +68,7 @@ describe("subscriptionCreator", () => { }); expect(subscription.documentId).toEqual(document.id); + expect(subscription.collectionId).toBeNull(); expect(subscription.userId).toEqual(user.id); expect(event?.name).toEqual("subscriptions.create"); expect(event?.modelId).toEqual(subscription.id); diff --git a/server/commands/subscriptionCreator.ts b/server/commands/subscriptionCreator.ts index 43049116f2..dffddd004a 100644 --- a/server/commands/subscriptionCreator.ts +++ b/server/commands/subscriptionCreator.ts @@ -1,3 +1,5 @@ +import { WhereOptions } from "sequelize"; +import { SubscriptionType } from "@shared/types"; import { createContext } from "@server/context"; import { Subscription, Document } from "@server/models"; import { sequelize } from "@server/storage/database"; @@ -8,8 +10,10 @@ type Props = { ctx: APIContext; /** The document to subscribe to */ documentId?: string; + /** The collection to subscribe to */ + collectionId?: string; /** Event to subscribe to */ - event: string; + event: SubscriptionType; /** Whether the subscription should be restored if it exists in a deleted state */ resubscribe?: boolean; }; @@ -22,16 +26,27 @@ type Props = { export default async function subscriptionCreator({ ctx, documentId, + collectionId, event, resubscribe = true, }: Props): Promise { const { user } = ctx.context.auth; + + const where: WhereOptions = { + userId: user.id, + event, + }; + + if (documentId) { + where.documentId = documentId; + } + + if (collectionId) { + where.collectionId = collectionId; + } + const [subscription] = await Subscription.findOrCreateWithCtx(ctx, { - where: { - userId: user.id, - documentId, - event, - }, + where, paranoid: false, // Previous subscriptions are soft-deleted, we want to know about them here. }); @@ -68,7 +83,7 @@ export const createSubscriptionsForDocument = async ( transaction, }), documentId: document.id, - event: "documents.update", + event: SubscriptionType.Document, resubscribe: false, }); } diff --git a/server/migrations/20250207120103-add-collectionId-to-subscriptions.js b/server/migrations/20250207120103-add-collectionId-to-subscriptions.js new file mode 100644 index 0000000000..9291a1cf45 --- /dev/null +++ b/server/migrations/20250207120103-add-collectionId-to-subscriptions.js @@ -0,0 +1,44 @@ +"use strict"; + +/** @type {import('sequelize-cli').Migration} */ +module.exports = { + async up(queryInterface, Sequelize) { + await queryInterface.sequelize.transaction(async transaction => { + await queryInterface.addColumn( + "subscriptions", + "collectionId", + { + type: Sequelize.UUID, + allowNull: true, + onDelete: "cascade", + references: { + model: "collections", + }, + }, + { transaction } + ); + await queryInterface.addIndex( + "subscriptions", + ["userId", "collectionId", "event"], + { + name: "subscriptions_user_id_collection_id_event", + type: "UNIQUE", + transaction, + } + ); + }); + }, + + async down(queryInterface, Sequelize) { + await queryInterface.sequelize.transaction(async transaction => { + await queryInterface.removeIndex( + "subscriptions", + ["userId", "collectionId", "event"], + { transaction } + ); + await queryInterface.removeColumn("subscriptions", "collectionId", { + transaction, + }); + }); + }, +}; diff --git a/server/models/Subscription.ts b/server/models/Subscription.ts index 3b70a3a6b7..63c25c6997 100644 --- a/server/models/Subscription.ts +++ b/server/models/Subscription.ts @@ -8,6 +8,8 @@ import { IsIn, Scopes, } from "sequelize-typescript"; +import { SubscriptionType } from "@shared/types"; +import Collection from "./Collection"; import Document from "./Document"; import User from "./User"; import ParanoidModel from "./base/ParanoidModel"; @@ -42,7 +44,14 @@ class Subscription extends ParanoidModel< @Column(DataType.UUID) documentId: string | null; - @IsIn([["documents.update"]]) + @BelongsTo(() => Collection, "collectionId") + collection: Collection | null; + + @ForeignKey(() => Document) + @Column(DataType.UUID) + collectionId: string | null; + + @IsIn([Object.values(SubscriptionType)]) @Column(DataType.STRING) event: string; } diff --git a/server/models/helpers/NotificationHelper.test.ts b/server/models/helpers/NotificationHelper.test.ts new file mode 100644 index 0000000000..c887247771 --- /dev/null +++ b/server/models/helpers/NotificationHelper.test.ts @@ -0,0 +1,152 @@ +import { NotificationEventType } from "@shared/types"; +import { + buildDocument, + buildSubscription, + buildUser, +} from "@server/test/factories"; +import NotificationHelper from "./NotificationHelper"; + +describe("NotificationHelper", () => { + describe("getDocumentNotificationRecipients", () => { + it("should return all users who have notification enabled for the event", async () => { + const documentAuthor = await buildUser(); + const document = await buildDocument({ + userId: documentAuthor.id, + teamId: documentAuthor.teamId, + }); + const notificationEnabledUser = await buildUser({ + teamId: document.teamId, + notificationSettings: { [NotificationEventType.UpdateDocument]: true }, + }); + + const recipients = + await NotificationHelper.getDocumentNotificationRecipients({ + document, + notificationType: NotificationEventType.UpdateDocument, + onlySubscribers: false, + actorId: documentAuthor.id, + }); + + expect(recipients.length).toEqual(1); + expect(recipients[0].id).toEqual(notificationEnabledUser.id); + }); + + it("should return users who have subscribed to the document", async () => { + const documentAuthor = await buildUser(); + const document = await buildDocument({ + userId: documentAuthor.id, + teamId: documentAuthor.teamId, + }); + const subscribedUser = await buildUser({ teamId: document.teamId }); + await buildSubscription({ + userId: subscribedUser.id, + documentId: document.id, + }); + + const recipients = + await NotificationHelper.getDocumentNotificationRecipients({ + document, + notificationType: NotificationEventType.UpdateDocument, + onlySubscribers: true, + actorId: documentAuthor.id, + }); + + expect(recipients.length).toEqual(1); + expect(recipients[0].id).toEqual(subscribedUser.id); + }); + + it("should return users who have subscribed to the collection", async () => { + const documentAuthor = await buildUser(); + const document = await buildDocument({ + userId: documentAuthor.id, + teamId: documentAuthor.teamId, + }); + const subscribedUser = await buildUser({ teamId: document.teamId }); + await buildSubscription({ + userId: subscribedUser.id, + collectionId: document.collectionId!, + }); + + const recipients = + await NotificationHelper.getDocumentNotificationRecipients({ + document, + notificationType: NotificationEventType.UpdateDocument, + onlySubscribers: true, + actorId: documentAuthor.id, + }); + + expect(recipients.length).toEqual(1); + expect(recipients[0].id).toEqual(subscribedUser.id); + }); + + it("should return users who have subscribed to either the document or the containing collection", async () => { + const documentAuthor = await buildUser(); + const document = await buildDocument({ + userId: documentAuthor.id, + teamId: documentAuthor.teamId, + }); + const [documentSubscribedUser, collectionSubscribedUser] = + await Promise.all([ + buildUser({ + teamId: document.teamId, + }), + buildUser({ + teamId: document.teamId, + }), + ]); + await Promise.all([ + buildSubscription({ + userId: documentSubscribedUser.id, + documentId: document.id, + }), + buildSubscription({ + userId: collectionSubscribedUser.id, + collectionId: document.collectionId!, + }), + ]); + + const recipients = + await NotificationHelper.getDocumentNotificationRecipients({ + document, + notificationType: NotificationEventType.UpdateDocument, + onlySubscribers: true, + actorId: documentAuthor.id, + }); + + expect(recipients.length).toEqual(2); + + const recipientIds = recipients.map((u) => u.id); + expect(recipientIds).toContain(collectionSubscribedUser.id); + expect(recipientIds).toContain(documentSubscribedUser.id); + }); + + it("should not return suspended users", async () => { + const documentAuthor = await buildUser(); + const document = await buildDocument({ + userId: documentAuthor.id, + teamId: documentAuthor.teamId, + }); + const notificationEnabledUser = await buildUser({ + teamId: document.teamId, + notificationSettings: { [NotificationEventType.UpdateDocument]: true }, + }); + // suspended user + await buildUser({ + suspendedAt: new Date(), + teamId: document.teamId, + notificationSettings: { [NotificationEventType.UpdateDocument]: true }, + }); + + const recipients = + await NotificationHelper.getDocumentNotificationRecipients({ + document, + notificationType: NotificationEventType.UpdateDocument, + onlySubscribers: false, + actorId: documentAuthor.id, + }); + + expect(recipients.length).toEqual(1); + expect(recipients[0].id).toEqual(notificationEnabledUser.id); + }); + }); +}); diff --git a/server/models/helpers/NotificationHelper.ts b/server/models/helpers/NotificationHelper.ts index db9fa8c62f..4d0839f200 100644 --- a/server/models/helpers/NotificationHelper.ts +++ b/server/models/helpers/NotificationHelper.ts @@ -1,6 +1,10 @@ import uniq from "lodash/uniq"; import { Op } from "sequelize"; -import { NotificationEventType, MentionType } from "@shared/types"; +import { + NotificationEventType, + MentionType, + SubscriptionType, +} from "@shared/types"; import Logger from "@server/logging/Logger"; import { User, @@ -56,12 +60,12 @@ export default class NotificationHelper { comment: Comment, actorId: string ): Promise => { - let recipients = await this.getDocumentNotificationRecipients( + let recipients = await this.getDocumentNotificationRecipients({ document, - NotificationEventType.UpdateDocument, + notificationType: NotificationEventType.UpdateDocument, + onlySubscribers: !comment.parentCommentId, actorId, - !comment.parentCommentId - ); + }); recipients = recipients.filter((recipient) => recipient.subscribedToEventType(NotificationEventType.CreateComment) @@ -127,18 +131,22 @@ export default class NotificationHelper { * Get the recipients of a notification for a document event. * * @param document The document to get recipients for. - * @param eventType The event name. + * @param notificationType The notification type for which to find the recipients. + * @param onlySubscribers Whether to consider only the users who have active subscription to the document. * @param actorId The id of the user that performed the action. - * @param onlySubscribers Whether to only return recipients that are actively - * subscribed to the document. * @returns A list of recipients */ - public static getDocumentNotificationRecipients = async ( - document: Document, - eventType: NotificationEventType, - actorId: string, - onlySubscribers: boolean - ): Promise => { + public static getDocumentNotificationRecipients = async ({ + document, + notificationType, + onlySubscribers, + actorId, + }: { + document: Document; + notificationType: NotificationEventType; + onlySubscribers: boolean; + actorId: string; + }): Promise => { // First find all the users that have notifications enabled for this event // type at all and aren't the one that performed the action. let recipients = await User.findAll({ @@ -151,7 +159,7 @@ export default class NotificationHelper { }); recipients = recipients.filter((recipient) => - recipient.subscribedToEventType(eventType) + recipient.subscribedToEventType(notificationType) ); // Filter further to only those that have a subscription to the document… @@ -160,8 +168,11 @@ export default class NotificationHelper { attributes: ["userId"], where: { userId: recipients.map((recipient) => recipient.id), - documentId: document.id, - event: eventType, + event: SubscriptionType.Document, + [Op.or]: [ + { collectionId: document.collectionId }, + { documentId: document.id }, + ], }, }); diff --git a/server/policies/collection.ts b/server/policies/collection.ts index 2ec24f42d2..132bfdd68a 100644 --- a/server/policies/collection.ts +++ b/server/policies/collection.ts @@ -49,7 +49,7 @@ allow(User, "read", Collection, (user, collection) => { allow( User, - ["readDocument", "star", "unstar"], + ["readDocument", "star", "unstar", "subscribe", "unsubscribe"], Collection, (user, collection) => { if (!collection || user.teamId !== collection.teamId) { diff --git a/server/presenters/subscription.ts b/server/presenters/subscription.ts index 084cb51630..d1cced5332 100644 --- a/server/presenters/subscription.ts +++ b/server/presenters/subscription.ts @@ -5,6 +5,7 @@ export default function presentSubscription(subscription: Subscription) { id: subscription.id, userId: subscription.userId, documentId: subscription.documentId, + collectionId: subscription.collectionId, event: subscription.event, createdAt: subscription.createdAt, updatedAt: subscription.updatedAt, diff --git a/server/queues/tasks/CommentCreatedNotificationsTask.ts b/server/queues/tasks/CommentCreatedNotificationsTask.ts index ebb03896a0..1ad00fd080 100644 --- a/server/queues/tasks/CommentCreatedNotificationsTask.ts +++ b/server/queues/tasks/CommentCreatedNotificationsTask.ts @@ -1,4 +1,8 @@ -import { MentionType, NotificationEventType } from "@shared/types"; +import { + MentionType, + NotificationEventType, + SubscriptionType, +} from "@shared/types"; import subscriptionCreator from "@server/commands/subscriptionCreator"; import { createContext } from "@server/context"; import { Comment, Document, Notification, User } from "@server/models"; @@ -34,7 +38,7 @@ export default class CommentCreatedNotificationsTask extends BaseTask !userIdsMentioned.includes(recipient.id)); for (const recipient of recipients) { diff --git a/server/queues/tasks/DocumentSubscriptionTask.ts b/server/queues/tasks/DocumentSubscriptionTask.ts index 91c87cbd22..9dc048850b 100644 --- a/server/queues/tasks/DocumentSubscriptionTask.ts +++ b/server/queues/tasks/DocumentSubscriptionTask.ts @@ -1,4 +1,5 @@ import { Transaction } from "sequelize"; +import { SubscriptionType } from "@shared/types"; import subscriptionCreator from "@server/commands/subscriptionCreator"; import { createContext } from "@server/context"; import { Subscription, User } from "@server/models"; @@ -34,7 +35,7 @@ export default class DocumentSubscriptionTask extends BaseTask !userIdsMentioned.includes(recipient.id)); if (!recipients.length) { return; diff --git a/server/routes/api/subscriptions/schema.ts b/server/routes/api/subscriptions/schema.ts index d1aa7dd607..f76cb3d941 100644 --- a/server/routes/api/subscriptions/schema.ts +++ b/server/routes/api/subscriptions/schema.ts @@ -1,36 +1,38 @@ +import isEmpty from "lodash/isEmpty"; import { z } from "zod"; +import { SubscriptionType } from "@shared/types"; import { ValidateDocumentId } from "@server/validation"; import { BaseSchema } from "../schema"; +const SubscriptionBody = z + .object({ + event: z.literal(SubscriptionType.Document), + collectionId: z.string().uuid().optional(), + documentId: z + .string() + .refine(ValidateDocumentId.isValid, { + message: ValidateDocumentId.message, + }) + .optional(), + }) + .refine((obj) => !(isEmpty(obj.collectionId) && isEmpty(obj.documentId)), { + message: "one of collectionId or documentId is required", + }); + export const SubscriptionsListSchema = BaseSchema.extend({ - body: z.object({ - documentId: z.string().refine(ValidateDocumentId.isValid, { - message: ValidateDocumentId.message, - }), - event: z.literal("documents.update"), - }), + body: SubscriptionBody, }); export type SubscriptionsListReq = z.infer; export const SubscriptionsInfoSchema = BaseSchema.extend({ - body: z.object({ - documentId: z.string().refine(ValidateDocumentId.isValid, { - message: ValidateDocumentId.message, - }), - event: z.literal("documents.update"), - }), + body: SubscriptionBody, }); export type SubscriptionsInfoReq = z.infer; export const SubscriptionsCreateSchema = BaseSchema.extend({ - body: z.object({ - documentId: z.string().refine(ValidateDocumentId.isValid, { - message: ValidateDocumentId.message, - }), - event: z.literal("documents.update"), - }), + body: SubscriptionBody, }); export type SubscriptionsCreateReq = z.infer; diff --git a/server/routes/api/subscriptions/subscriptions.test.ts b/server/routes/api/subscriptions/subscriptions.test.ts index f8caf23e8d..1cdae3d154 100644 --- a/server/routes/api/subscriptions/subscriptions.test.ts +++ b/server/routes/api/subscriptions/subscriptions.test.ts @@ -1,15 +1,41 @@ +import { SubscriptionType } from "@shared/types"; import { Event } from "@server/models"; import { buildUser, buildSubscription, buildDocument, + buildCollection, } from "@server/test/factories"; import { getTestServer } from "@server/test/support"; const server = getTestServer(); describe("#subscriptions.create", () => { - it("should create a subscription", async () => { + it("should create a document subscription for the whole collection", async () => { + const user = await buildUser(); + + const collection = await buildCollection({ + userId: user.id, + teamId: user.teamId, + }); + + const res = await server.post("/api/subscriptions.create", { + body: { + token: user.getJwtToken(), + collectionId: collection.id, + event: SubscriptionType.Document, + }, + }); + + const body = await res.json(); + + expect(res.status).toEqual(200); + expect(body.data.id).toBeDefined(); + expect(body.data.userId).toEqual(user.id); + expect(body.data.collectionId).toEqual(collection.id); + }); + + it("should create a document subscription", async () => { const user = await buildUser(); const document = await buildDocument({ @@ -21,7 +47,7 @@ describe("#subscriptions.create", () => { body: { token: user.getJwtToken(), documentId: document.id, - event: "documents.update", + event: SubscriptionType.Document, }, }); @@ -45,7 +71,7 @@ describe("#subscriptions.create", () => { body: { token: user.getJwtToken(), documentId: document.id, - event: "documents.update", + event: SubscriptionType.Document, }, }); @@ -79,7 +105,7 @@ describe("#subscriptions.create", () => { body: { token: user.getJwtToken(), documentId: document.id, - event: "documents.update", + event: SubscriptionType.Document, }, }); @@ -88,7 +114,7 @@ describe("#subscriptions.create", () => { body: { token: user.getJwtToken(), documentId: document.id, - event: "documents.update", + event: SubscriptionType.Document, }, }); @@ -97,17 +123,16 @@ describe("#subscriptions.create", () => { body: { token: user.getJwtToken(), documentId: document.id, - event: "documents.update", + event: SubscriptionType.Document, }, }); - // List subscriptions associated with - // `document.id` + // List subscriptions associated with `document.id` const res = await server.post("/api/subscriptions.list", { body: { token: user.getJwtToken(), documentId: document.id, - event: "documents.update", + event: SubscriptionType.Document, }, }); @@ -132,8 +157,7 @@ describe("#subscriptions.create", () => { body: { token: user.getJwtToken(), documentId: document.id, - // Subscription on event - // that cannot be subscribed to. + // Subscription on event that cannot be subscribed to. event: "documents.publish", }, }); @@ -147,10 +171,62 @@ describe("#subscriptions.create", () => { `event: Invalid literal value, expected "documents.update"` ); }); + + it("should throw 400 when neither documentId nor collectionId is provided", async () => { + const user = await buildUser(); + + const res = await server.post("/api/subscriptions.create", { + body: { + token: user.getJwtToken(), + event: SubscriptionType.Document, + }, + }); + const body = await res.json(); + + expect(res.status).toEqual(400); + expect(body.ok).toEqual(false); + expect(body.error).toEqual("validation_error"); + expect(body.message).toEqual( + "body: one of collectionId or documentId is required" + ); + }); }); describe("#subscriptions.info", () => { - it("should provide info about a subscription", async () => { + it("should provide info about a document subscription for the collection", async () => { + const user = await buildUser(); + + const subscriber = await buildUser({ teamId: user.teamId }); + + const collection = await buildCollection({ + userId: user.id, + teamId: user.teamId, + }); + + await server.post("/api/subscriptions.create", { + body: { + token: subscriber.getJwtToken(), + collectionId: collection.id, + event: SubscriptionType.Document, + }, + }); + + const res = await server.post("/api/subscriptions.info", { + body: { + token: subscriber.getJwtToken(), + collectionId: collection.id, + event: SubscriptionType.Document, + }, + }); + const subscription = await res.json(); + + expect(res.status).toEqual(200); + expect(subscription.data.id).toBeDefined(); + expect(subscription.data.userId).toEqual(subscriber.id); + expect(subscription.data.collectionId).toEqual(collection.id); + }); + + it("should provide info about a document subscription", async () => { const creator = await buildUser(); const subscriber = await buildUser({ teamId: creator.teamId }); @@ -171,7 +247,7 @@ describe("#subscriptions.info", () => { body: { token: subscriber.getJwtToken(), documentId: document0.id, - event: "documents.update", + event: SubscriptionType.Document, }, }); @@ -180,7 +256,7 @@ describe("#subscriptions.info", () => { body: { token: subscriber.getJwtToken(), documentId: document1.id, - event: "documents.update", + event: SubscriptionType.Document, }, }); @@ -190,7 +266,7 @@ describe("#subscriptions.info", () => { body: { token: subscriber.getJwtToken(), documentId: document0.id, - event: "documents.update", + event: SubscriptionType.Document, }, }); @@ -202,6 +278,25 @@ describe("#subscriptions.info", () => { expect(response0.data.documentId).toEqual(document0.id); }); + it("should throw 400 when neither documentId nor collectionId is provided", async () => { + const user = await buildUser(); + + const res = await server.post("/api/subscriptions.info", { + body: { + token: user.getJwtToken(), + event: SubscriptionType.Document, + }, + }); + const body = await res.json(); + + expect(res.status).toEqual(400); + expect(body.ok).toEqual(false); + expect(body.error).toEqual("validation_error"); + expect(body.message).toEqual( + "body: one of collectionId or documentId is required" + ); + }); + it("should throw 404 if no subscription found", async () => { const author = await buildUser(); const subscriber = await buildUser({ teamId: author.teamId }); @@ -214,7 +309,7 @@ describe("#subscriptions.info", () => { body: { token: subscriber.getJwtToken(), documentId: document.id, - event: "documents.update", + event: SubscriptionType.Document, }, }); @@ -243,7 +338,7 @@ describe("#subscriptions.info", () => { body: { token: subscriber.getJwtToken(), documentId: document0.id, - event: "documents.update", + event: SubscriptionType.Document, }, }); @@ -252,17 +347,16 @@ describe("#subscriptions.info", () => { body: { token: subscriber.getJwtToken(), documentId: document1.id, - event: "documents.update", + event: SubscriptionType.Document, }, }); - // `viewer` wants info about `subscriber`'s - // subscription on `document0`. + // `viewer` wants info about `subscriber`'s subscription on `document0`. const subscription0 = await server.post("/api/subscriptions.info", { body: { token: viewer.getJwtToken(), documentId: document0.id, - event: "documents.update", + event: SubscriptionType.Document, }, }); @@ -274,13 +368,12 @@ describe("#subscriptions.info", () => { expect(response0.error).toEqual("authorization_error"); expect(response0.message).toEqual("Authorization error"); - // `viewer` wants info about `subscriber`'s - // subscription on `document0`. + // `viewer` wants info about `subscriber`'s subscription on `document0`. const subscription1 = await server.post("/api/subscriptions.info", { body: { token: viewer.getJwtToken(), documentId: document1.id, - event: "documents.update", + event: SubscriptionType.Document, }, }); @@ -316,7 +409,7 @@ describe("#subscriptions.info", () => { body: { token: subscriber.getJwtToken(), documentId: document0.id, - event: "documents.update", + event: SubscriptionType.Document, }, }); @@ -325,13 +418,11 @@ describe("#subscriptions.info", () => { body: { token: subscriber.getJwtToken(), documentId: document1.id, - event: "documents.update", + event: SubscriptionType.Document, }, }); - // `viewer` wants info about `subscriber`'s - // subscription on `document0`. - // They have requested an invalid event. + // `viewer` wants info about `subscriber`'s subscription on `document0` - they have requested an invalid event. const subscription0 = await server.post("/api/subscriptions.info", { body: { token: viewer.getJwtToken(), @@ -372,7 +463,7 @@ describe("#subscriptions.info", () => { }); describe("#subscriptions.list", () => { - it("should list user subscriptions", async () => { + it("should list user subscriptions for the document", async () => { const user = await buildUser(); const document = await buildDocument({ @@ -380,19 +471,16 @@ describe("#subscriptions.list", () => { teamId: user.teamId, }); - await buildSubscription(); - const subscription = await buildSubscription({ userId: user.id, documentId: document.id, - event: "documents.update", }); const res = await server.post("/api/subscriptions.list", { body: { token: user.getJwtToken(), documentId: document.id, - event: "documents.update", + event: SubscriptionType.Document, }, }); @@ -594,6 +682,25 @@ describe("#subscriptions.list", () => { expect(body.error).toEqual("authorization_error"); expect(body.message).toEqual("Authorization error"); }); + + it("should throw 400 when neither documentId nor collectionId is provided", async () => { + const user = await buildUser(); + + const res = await server.post("/api/subscriptions.list", { + body: { + token: user.getJwtToken(), + event: SubscriptionType.Document, + }, + }); + const body = await res.json(); + + expect(res.status).toEqual(400); + expect(body.ok).toEqual(false); + expect(body.error).toEqual("validation_error"); + expect(body.message).toEqual( + "body: one of collectionId or documentId is required" + ); + }); }); describe("#subscriptions.delete", () => { @@ -608,7 +715,6 @@ describe("#subscriptions.delete", () => { const subscription = await buildSubscription({ userId: user.id, documentId: document.id, - event: "documents.update", }); const res = await server.post("/api/subscriptions.delete", { @@ -637,7 +743,6 @@ describe("#subscriptions.delete", () => { const subscription = await buildSubscription({ userId: user.id, documentId: document.id, - event: "documents.update", }); const res = await server.post("/api/subscriptions.delete", { diff --git a/server/routes/api/subscriptions/subscriptions.ts b/server/routes/api/subscriptions/subscriptions.ts index c4dbad08d4..caa96b5703 100644 --- a/server/routes/api/subscriptions/subscriptions.ts +++ b/server/routes/api/subscriptions/subscriptions.ts @@ -1,5 +1,5 @@ import Router from "koa-router"; -import { Transaction } from "sequelize"; +import { Transaction, WhereOptions } from "sequelize"; import { QueryNotices } from "@shared/types"; import subscriptionCreator from "@server/commands/subscriptionCreator"; import { createContext } from "@server/context"; @@ -8,7 +8,7 @@ import auth from "@server/middlewares/authentication"; import { rateLimiter } from "@server/middlewares/rateLimiter"; import { transaction } from "@server/middlewares/transaction"; import validate from "@server/middlewares/validate"; -import { Subscription, Document, User } from "@server/models"; +import { Subscription, Document, User, Collection } from "@server/models"; import SubscriptionHelper from "@server/models/helpers/SubscriptionHelper"; import { authorize } from "@server/policies"; import { presentSubscription } from "@server/presenters"; @@ -26,18 +26,32 @@ router.post( validate(T.SubscriptionsListSchema), async (ctx: APIContext) => { const { user } = ctx.state.auth; - const { documentId, event } = ctx.input.body; + const { event, collectionId, documentId } = ctx.input.body; - const document = await Document.findByPk(documentId, { userId: user.id }); + const where: WhereOptions = { + userId: user.id, + event, + }; - authorize(user, "read", document); + if (collectionId) { + const collection = await Collection.scope({ + method: ["withMembership", user.id], + }).findByPk(collectionId); + authorize(user, "read", collection); + + where.collectionId = collectionId; + } else { + // documentId will be available here + const document = await Document.findByPk(documentId!, { + userId: user.id, + }); + authorize(user, "read", document); + + where.documentId = documentId; + } const subscriptions = await Subscription.findAll({ - where: { - documentId: document.id, - userId: user.id, - event, - }, + where, order: [["createdAt", "DESC"]], offset: ctx.state.pagination.offset, limit: ctx.state.pagination.limit, @@ -56,19 +70,33 @@ router.post( validate(T.SubscriptionsInfoSchema), async (ctx: APIContext) => { const { user } = ctx.state.auth; - const { documentId, event } = ctx.input.body; + const { event, collectionId, documentId } = ctx.input.body; - const document = await Document.findByPk(documentId, { userId: user.id }); + const where: WhereOptions = { + userId: user.id, + event, + }; - authorize(user, "read", document); + if (collectionId) { + const collection = await Collection.scope({ + method: ["withMembership", user.id], + }).findByPk(collectionId); + authorize(user, "read", collection); + + where.collectionId = collectionId; + } else { + // documentId will be available here + const document = await Document.findByPk(documentId!, { + userId: user.id, + }); + authorize(user, "read", document); + + where.documentId = documentId; + } // There can be only one subscription with these props. const subscription = await Subscription.findOne({ - where: { - userId: user.id, - documentId: document.id, - event, - }, + where, rejectOnEmpty: true, }); @@ -84,20 +112,28 @@ router.post( validate(T.SubscriptionsCreateSchema), transaction(), async (ctx: APIContext) => { - const { transaction } = ctx.state; const { user } = ctx.state.auth; - const { documentId, event } = ctx.input.body; + const { event, collectionId, documentId } = ctx.input.body; - const document = await Document.findByPk(documentId, { - userId: user.id, - transaction, - }); + if (collectionId) { + const collection = await Collection.scope({ + method: ["withMembership", user.id], + }).findByPk(collectionId); - authorize(user, "subscribe", document); + authorize(user, "subscribe", collection); + } else { + // documentId will be available here + const document = await Document.findByPk(documentId!, { + userId: user.id, + }); + + authorize(user, "subscribe", document); + } const subscription = await subscriptionCreator({ ctx, - documentId: document.id, + documentId, + collectionId, event, }); diff --git a/server/test/factories.ts b/server/test/factories.ts index 6308be0c11..3a69bc81ad 100644 --- a/server/test/factories.ts +++ b/server/test/factories.ts @@ -15,6 +15,7 @@ import { NotificationEventType, ProsemirrorData, ReactionSummary, + SubscriptionType, UserRole, } from "@shared/types"; import { parser, schema } from "@server/editor"; @@ -120,7 +121,7 @@ export async function buildSubscription(overrides: Partial = {}) { overrides.userId = user.id; } - if (!overrides.documentId) { + if (!overrides.documentId && !overrides.collectionId) { const document = await buildDocument({ createdById: overrides.userId, teamId: user.teamId, @@ -129,7 +130,7 @@ export async function buildSubscription(overrides: Partial = {}) { } return Subscription.create({ - event: "documents.update", + event: SubscriptionType.Document, ...overrides, }); } diff --git a/server/types.ts b/server/types.ts index e6d0868aa0..ae74bdb61d 100644 --- a/server/types.ts +++ b/server/types.ts @@ -427,6 +427,7 @@ export type SubscriptionEvent = BaseEvent & { modelId: string; userId: string; documentId: string | null; + collectionId: string | null; }; export type ViewEvent = BaseEvent & { diff --git a/shared/i18n/locales/en_US/translation.json b/shared/i18n/locales/en_US/translation.json index 935fa0bc73..ce151f4b1a 100644 --- a/shared/i18n/locales/en_US/translation.json +++ b/shared/i18n/locales/en_US/translation.json @@ -11,6 +11,10 @@ "Search in collection": "Search in collection", "Star": "Star", "Unstar": "Unstar", + "Subscribe": "Subscribe", + "Subscribed to document notifications": "Subscribed to document notifications", + "Unsubscribe": "Unsubscribe", + "Unsubscribed from document notifications": "Unsubscribed from document notifications", "Archive": "Archive", "Archive collection": "Archive collection", "Collection archived": "Collection archived", @@ -44,10 +48,6 @@ "Publish document": "Publish document", "Unpublish": "Unpublish", "Unpublished {{ documentName }}": "Unpublished {{ documentName }}", - "Subscribe": "Subscribe", - "Subscribed to document notifications": "Subscribed to document notifications", - "Unsubscribe": "Unsubscribe", - "Unsubscribed from document notifications": "Unsubscribed from document notifications", "Share this document": "Share this document", "HTML": "HTML", "PDF": "PDF", @@ -533,6 +533,7 @@ "{{ documentName }} restored": "{{ documentName }} restored", "Document options": "Document options", "Choose a collection": "Choose a collection", + "Subscription inherited from collection": "Subscription inherited from collection", "Enable embeds": "Enable embeds", "Export options": "Export options", "Group members": "Group members", diff --git a/shared/types.ts b/shared/types.ts index 2133fdfaaa..32904533cf 100644 --- a/shared/types.ts +++ b/shared/types.ts @@ -270,6 +270,10 @@ export type CollectionSort = { direction: "asc" | "desc"; }; +export enum SubscriptionType { + Document = "documents.update", +} + export enum NotificationEventType { PublishDocument = "documents.publish", UpdateDocument = "documents.update",