From 7145f7ef5123ab259ca4701d94b76c6a7cc25b5f Mon Sep 17 00:00:00 2001 From: Apoorv Mishra Date: Mon, 25 Sep 2023 10:51:29 +0530 Subject: [PATCH] `UserPermission` and `GroupPermission` models (#5860) * fix: rename to group_permissions * fix: delete null collectionId records before setting non null constraint * fix: use scope with collectionId not null * fix: update model with documentId * fix: rename to GroupPermission * fix: rename collection_users to user_permissions * fix: teamPermanentDeleter test * fix: use scope with collectionId not null * fix: update model with documentId * fix: rename to UserPermission * fix: create views upon table rename for zero downtime * fix: remove comments --- .../server/tasks/DeliverWebhookTask.ts | 12 +- server/commands/userDemoter.test.ts | 4 +- ...20230921071140-rename-collection-groups.js | 196 ++++++++++++++++++ .../20230922105047-rename-collection-users.js | 183 ++++++++++++++++ server/models/Collection.ts | 44 ++-- server/models/Group.ts | 8 +- ...{CollectionGroup.ts => GroupPermission.ts} | 27 ++- server/models/User.test.ts | 4 +- server/models/User.ts | 4 +- .../{CollectionUser.ts => UserPermission.ts} | 27 ++- server/models/index.ts | 4 +- server/policies/collection.test.ts | 16 +- .../presenters/collectionGroupMembership.ts | 6 +- server/presenters/membership.ts | 6 +- .../queues/processors/WebsocketsProcessor.ts | 28 ++- .../api/attachments/attachments.test.ts | 4 +- .../api/collections/collections.test.ts | 36 ++-- server/routes/api/collections/collections.ts | 26 +-- server/routes/api/documents/documents.test.ts | 32 +-- server/routes/api/revisions/revisions.test.ts | 4 +- server/routes/api/shares/shares.test.ts | 4 +- server/routes/api/views/views.test.ts | 6 +- 22 files changed, 558 insertions(+), 123 deletions(-) create mode 100644 server/migrations/20230921071140-rename-collection-groups.js create mode 100644 server/migrations/20230922105047-rename-collection-users.js rename server/models/{CollectionGroup.ts => GroupPermission.ts} (62%) rename server/models/{CollectionUser.ts => UserPermission.ts} (63%) diff --git a/plugins/webhooks/server/tasks/DeliverWebhookTask.ts b/plugins/webhooks/server/tasks/DeliverWebhookTask.ts index 9ca74e37a..230e871cc 100644 --- a/plugins/webhooks/server/tasks/DeliverWebhookTask.ts +++ b/plugins/webhooks/server/tasks/DeliverWebhookTask.ts @@ -18,8 +18,8 @@ import { Revision, View, Share, - CollectionUser, - CollectionGroup, + UserPermission, + GroupPermission, GroupUser, Comment, } from "@server/models"; @@ -426,7 +426,7 @@ export default class DeliverWebhookTask extends BaseTask { subscription: WebhookSubscription, event: CollectionUserEvent ): Promise { - const model = await CollectionUser.scope([ + const model = await UserPermission.scope([ "withUser", "withCollection", ]).findOne({ @@ -443,7 +443,7 @@ export default class DeliverWebhookTask extends BaseTask { payload: { id: `${event.userId}-${event.collectionId}`, model: model && presentMembership(model), - collection: model && presentCollection(model.collection), + collection: model && presentCollection(model.collection!), user: model && presentUser(model.user), }, }); @@ -453,7 +453,7 @@ export default class DeliverWebhookTask extends BaseTask { subscription: WebhookSubscription, event: CollectionGroupEvent ): Promise { - const model = await CollectionGroup.scope([ + const model = await GroupPermission.scope([ "withGroup", "withCollection", ]).findOne({ @@ -470,7 +470,7 @@ export default class DeliverWebhookTask extends BaseTask { payload: { id: `${event.modelId}-${event.collectionId}`, model: model && presentCollectionGroupMembership(model), - collection: model && presentCollection(model.collection), + collection: model && presentCollection(model.collection!), group: model && presentGroup(model.group), }, }); diff --git a/server/commands/userDemoter.test.ts b/server/commands/userDemoter.test.ts index 6c14c8682..cea51a871 100644 --- a/server/commands/userDemoter.test.ts +++ b/server/commands/userDemoter.test.ts @@ -1,5 +1,5 @@ import { CollectionPermission, UserRole } from "@shared/types"; -import { CollectionUser } from "@server/models"; +import { UserPermission } from "@server/models"; import { buildUser, buildAdmin, buildCollection } from "@server/test/factories"; import userDemoter from "./userDemoter"; @@ -11,7 +11,7 @@ describe("userDemoter", () => { const user = await buildUser({ teamId: admin.teamId }); const collection = await buildCollection({ teamId: admin.teamId }); - const membership = await CollectionUser.create({ + const membership = await UserPermission.create({ createdById: admin.id, userId: user.id, collectionId: collection.id, diff --git a/server/migrations/20230921071140-rename-collection-groups.js b/server/migrations/20230921071140-rename-collection-groups.js new file mode 100644 index 000000000..68966e195 --- /dev/null +++ b/server/migrations/20230921071140-rename-collection-groups.js @@ -0,0 +1,196 @@ +"use strict"; + +const { Op } = require("sequelize"); + +module.exports = { + async up(queryInterface, Sequelize) { + await queryInterface.sequelize.transaction(async (transaction) => { + await queryInterface.sequelize.query( + `DROP VIEW IF EXISTS group_permissions`, + { transaction } + ); + await queryInterface.sequelize.query( + `ALTER INDEX "collection_groups_collection_id_group_id" RENAME TO "group_permissions_collection_id_group_id"`, + { transaction } + ); + await queryInterface.sequelize.query( + `ALTER INDEX "collection_groups_deleted_at" RENAME TO "group_permissions_deleted_at"`, + { transaction } + ); + await queryInterface.sequelize.query( + `ALTER INDEX "collection_groups_group_id" RENAME TO "group_permissions_group_id"`, + { transaction } + ); + + await queryInterface.sequelize.query( + `ALTER TABLE ONLY collection_groups RENAME CONSTRAINT "collection_groups_collectionId_fkey" TO "group_permissions_collectionId_fkey"`, + { transaction } + ); + await queryInterface.sequelize.query( + `ALTER TABLE ONLY collection_groups RENAME CONSTRAINT "collection_groups_createdById_fkey" TO "group_permissions_createdById_fkey"`, + { transaction } + ); + await queryInterface.sequelize.query( + `ALTER TABLE ONLY collection_groups RENAME CONSTRAINT "collection_groups_groupId_fkey" TO "group_permissions_groupId_fkey"`, + { transaction } + ); + await queryInterface.addColumn( + "collection_groups", + "documentId", + { + type: Sequelize.UUID, + allowNull: true, + onDelete: "set null", + references: { + model: "documents", + }, + }, + { transaction } + ); + await queryInterface.sequelize.query( + `ALTER TABLE ONLY collection_groups RENAME CONSTRAINT "collection_groups_documentId_fkey" TO "group_permissions_documentId_fkey"`, + { transaction } + ); + await queryInterface.removeConstraint( + "collection_groups", + "group_permissions_collectionId_fkey", + { transaction } + ); + await queryInterface.addConstraint("collection_groups", { + fields: ["collectionId"], + name: "group_permissions_collectionId_fkey", + type: "foreign key", + onDelete: "set null", + references: { + table: "collections", + field: "id", + }, + transaction, + }); + await queryInterface.changeColumn( + "collection_groups", + "collectionId", + { + type: Sequelize.UUID, + allowNull: true, + }, + { transaction } + ); + await queryInterface.removeConstraint( + "collection_groups", + "group_permissions_createdById_fkey", + { transaction } + ); + await queryInterface.addConstraint("collection_groups", { + fields: ["createdById"], + name: "group_permissions_createdById_fkey", + type: "foreign key", + onDelete: "set null", + references: { + table: "users", + field: "id", + }, + transaction, + }); + await queryInterface.renameTable( + "collection_groups", + "group_permissions", + { transaction } + ); + await queryInterface.sequelize.query( + `CREATE VIEW collection_groups AS SELECT * FROM group_permissions`, + { transaction } + ); + }); + }, + + async down(queryInterface, Sequelize) { + await queryInterface.sequelize.transaction(async (transaction) => { + await queryInterface.sequelize.query( + `DROP VIEW IF EXISTS collection_groups`, + { transaction } + ); + await queryInterface.sequelize.query( + `ALTER INDEX "group_permissions_collection_id_group_id" RENAME TO "collection_groups_collection_id_group_id"`, + { transaction } + ); + await queryInterface.sequelize.query( + `ALTER INDEX "group_permissions_deleted_at" RENAME TO "collection_groups_deleted_at"`, + { transaction } + ); + await queryInterface.sequelize.query( + `ALTER INDEX "group_permissions_group_id" RENAME TO "collection_groups_group_id"`, + { transaction } + ); + await queryInterface.sequelize.query( + `ALTER TABLE ONLY group_permissions RENAME CONSTRAINT "group_permissions_collectionId_fkey" TO "collection_groups_collectionId_fkey"`, + { transaction } + ); + await queryInterface.sequelize.query( + `ALTER TABLE ONLY group_permissions RENAME CONSTRAINT "group_permissions_createdById_fkey" TO "collection_groups_createdById_fkey"`, + { transaction } + ); + await queryInterface.sequelize.query( + `ALTER TABLE ONLY group_permissions RENAME CONSTRAINT "group_permissions_groupId_fkey" TO "collection_groups_groupId_fkey"`, + { transaction } + ); + await queryInterface.removeColumn("group_permissions", "documentId", { + transaction, + }); + await queryInterface.removeConstraint( + "group_permissions", + "collection_groups_collectionId_fkey", + { transaction } + ); + await queryInterface.addConstraint("group_permissions", { + fields: ["collectionId"], + name: "collection_groups_collectionId_fkey", + type: "foreign key", + onDelete: "cascade", + references: { + table: "collections", + field: "id", + }, + transaction, + }); + // Delete records where collectionId is null before setting back non null constraint + await queryInterface.sequelize.query( + `DELETE FROM group_permissions WHERE "collectionId" IS NULL`, + { transaction } + ); + await queryInterface.changeColumn( + "group_permissions", + "collectionId", + { + type: Sequelize.UUID, + allowNull: false, + }, + { transaction } + ); + await queryInterface.removeConstraint( + "group_permissions", + "collection_groups_createdById_fkey", + { transaction } + ); + await queryInterface.addConstraint("group_permissions", { + fields: ["createdById"], + name: "collection_groups_createdById_fkey", + type: "foreign key", + references: { + table: "users", + field: "id", + }, + transaction, + }); + await queryInterface.renameTable( + "group_permissions", + "collection_groups", + { transaction } + ); + await queryInterface.sequelize.query( + `CREATE VIEW group_permissions AS SELECT * FROM collection_groups`, + { transaction } + ); + }); + }, +}; diff --git a/server/migrations/20230922105047-rename-collection-users.js b/server/migrations/20230922105047-rename-collection-users.js new file mode 100644 index 000000000..2f423292d --- /dev/null +++ b/server/migrations/20230922105047-rename-collection-users.js @@ -0,0 +1,183 @@ +"use strict"; + +const { Op } = require("sequelize"); + +module.exports = { + async up(queryInterface, Sequelize) { + await queryInterface.sequelize.transaction(async (transaction) => { + await queryInterface.sequelize.query( + `DROP VIEW IF EXISTS user_permissions`, + { transaction } + ); + await queryInterface.sequelize.query( + `ALTER INDEX "collection_users_collection_id_user_id" RENAME TO "user_permissions_collection_id_user_id"`, + { transaction } + ); + await queryInterface.sequelize.query( + `ALTER INDEX "collection_users_user_id" RENAME TO "user_permissions_user_id"`, + { transaction } + ); + await queryInterface.sequelize.query( + `ALTER TABLE ONLY collection_users RENAME CONSTRAINT "collection_users_collectionId_fkey" TO "user_permissions_collectionId_fkey"`, + { transaction } + ); + await queryInterface.sequelize.query( + `ALTER TABLE ONLY collection_users RENAME CONSTRAINT "collection_users_createdById_fkey" TO "user_permissions_createdById_fkey"`, + { transaction } + ); + await queryInterface.sequelize.query( + `ALTER TABLE ONLY collection_users RENAME CONSTRAINT "collection_users_userId_fkey" TO "user_permissions_userId_fkey"`, + { transaction } + ); + await queryInterface.addColumn( + "collection_users", + "documentId", + { + type: Sequelize.UUID, + allowNull: true, + onDelete: "set null", + references: { + model: "documents", + }, + }, + { transaction } + ); + await queryInterface.sequelize.query( + `ALTER TABLE ONLY collection_users RENAME CONSTRAINT "collection_users_documentId_fkey" TO "user_permissions_documentId_fkey"`, + { transaction } + ); + await queryInterface.removeConstraint( + "collection_users", + "user_permissions_collectionId_fkey", + { transaction } + ); + await queryInterface.addConstraint("collection_users", { + fields: ["collectionId"], + name: "user_permissions_collectionId_fkey", + type: "foreign key", + onDelete: "set null", + references: { + table: "collections", + field: "id", + }, + transaction, + }); + await queryInterface.changeColumn( + "collection_users", + "collectionId", + { + type: Sequelize.UUID, + allowNull: true, + }, + { transaction } + ); + await queryInterface.removeConstraint( + "collection_users", + "user_permissions_createdById_fkey", + { transaction } + ); + await queryInterface.addConstraint("collection_users", { + fields: ["createdById"], + name: "user_permissions_createdById_fkey", + type: "foreign key", + onDelete: "set null", + references: { + table: "users", + field: "id", + }, + transaction, + }); + await queryInterface.renameTable("collection_users", "user_permissions", { + transaction, + }); + await queryInterface.sequelize.query( + `CREATE VIEW collection_users AS SELECT * FROM user_permissions`, + { transaction } + ); + }); + }, + + async down(queryInterface, Sequelize) { + await queryInterface.sequelize.transaction(async (transaction) => { + await queryInterface.sequelize.query( + `DROP VIEW IF EXISTS collection_users`, + { transaction } + ); + await queryInterface.sequelize.query( + `ALTER INDEX "user_permissions_collection_id_user_id" RENAME TO "collection_users_collection_id_user_id"`, + { transaction } + ); + await queryInterface.sequelize.query( + `ALTER INDEX "user_permissions_user_id" RENAME TO "collection_users_user_id"`, + { transaction } + ); + await queryInterface.sequelize.query( + `ALTER TABLE ONLY user_permissions RENAME CONSTRAINT "user_permissions_collectionId_fkey" TO "collection_users_collectionId_fkey"`, + { transaction } + ); + await queryInterface.sequelize.query( + `ALTER TABLE ONLY user_permissions RENAME CONSTRAINT "user_permissions_createdById_fkey" TO "collection_users_createdById_fkey"`, + { transaction } + ); + await queryInterface.sequelize.query( + `ALTER TABLE ONLY user_permissions RENAME CONSTRAINT "user_permissions_userId_fkey" TO "collection_users_userId_fkey"`, + { transaction } + ); + await queryInterface.removeColumn("user_permissions", "documentId", { + transaction, + }); + await queryInterface.removeConstraint( + "user_permissions", + "collection_users_collectionId_fkey", + { transaction } + ); + await queryInterface.addConstraint("user_permissions", { + fields: ["collectionId"], + name: "collection_users_collectionId_fkey", + type: "foreign key", + onDelete: "cascade", + references: { + table: "collections", + field: "id", + }, + transaction, + }); + // Delete records where collectionId is null before setting back non null constraint + await queryInterface.sequelize.query( + `DELETE FROM user_permissions WHERE "collectionId" IS NULL`, + { transaction } + ); + await queryInterface.changeColumn( + "user_permissions", + "collectionId", + { + type: Sequelize.UUID, + allowNull: false, + }, + { transaction } + ); + await queryInterface.removeConstraint( + "user_permissions", + "collection_users_createdById_fkey", + { transaction } + ); + await queryInterface.addConstraint("user_permissions", { + fields: ["createdById"], + name: "collection_users_createdById_fkey", + type: "foreign key", + references: { + table: "users", + field: "id", + }, + transaction, + }); + await queryInterface.renameTable("user_permissions", "collection_users", { + transaction, + }); + await queryInterface.sequelize.query( + `CREATE VIEW user_permissions AS SELECT * FROM collection_users`, + { transaction } + ); + }); + }, +}; diff --git a/server/models/Collection.ts b/server/models/Collection.ts index c2977a265..f6ad8cb21 100644 --- a/server/models/Collection.ts +++ b/server/models/Collection.ts @@ -30,14 +30,14 @@ import { sortNavigationNodes } from "@shared/utils/collections"; import slugify from "@shared/utils/slugify"; import { SLUG_URL_REGEX } from "@shared/utils/urlHelpers"; import { CollectionValidation } from "@shared/validations"; -import CollectionGroup from "./CollectionGroup"; -import CollectionUser from "./CollectionUser"; import Document from "./Document"; import FileOperation from "./FileOperation"; import Group from "./Group"; +import GroupPermission from "./GroupPermission"; import GroupUser from "./GroupUser"; import Team from "./Team"; import User from "./User"; +import UserPermission from "./UserPermission"; import ParanoidModel from "./base/ParanoidModel"; import Fix from "./decorators/Fix"; import IsHexColor from "./validators/IsHexColor"; @@ -48,13 +48,23 @@ import NotContainsUrl from "./validators/NotContainsUrl"; withAllMemberships: { include: [ { - model: CollectionUser, + model: UserPermission, as: "memberships", + where: { + collectionId: { + [Op.ne]: null, + }, + }, required: false, }, { - model: CollectionGroup, + model: GroupPermission, as: "collectionGroupMemberships", + where: { + collectionId: { + [Op.ne]: null, + }, + }, required: false, // use of "separate" property: sequelize breaks when there are // nested "includes" with alternating values for "required" @@ -92,16 +102,24 @@ import NotContainsUrl from "./validators/NotContainsUrl"; withMembership: (userId: string) => ({ include: [ { - model: CollectionUser, + model: UserPermission, as: "memberships", where: { userId, + collectionId: { + [Op.ne]: null, + }, }, required: false, }, { - model: CollectionGroup, + model: GroupPermission, as: "collectionGroupMemberships", + where: { + collectionId: { + [Op.ne]: null, + }, + }, required: false, // use of "separate" property: sequelize breaks when there are // nested "includes" with alternating values for "required" @@ -257,7 +275,7 @@ class Collection extends ParanoidModel { model: Collection, options: { transaction: Transaction } ) { - return CollectionUser.findOrCreate({ + return UserPermission.findOrCreate({ where: { collectionId: model.id, userId: model.createdById, @@ -282,16 +300,16 @@ class Collection extends ParanoidModel { @HasMany(() => Document, "collectionId") documents: Document[]; - @HasMany(() => CollectionUser, "collectionId") - memberships: CollectionUser[]; + @HasMany(() => UserPermission, "collectionId") + memberships: UserPermission[]; - @HasMany(() => CollectionGroup, "collectionId") - collectionGroupMemberships: CollectionGroup[]; + @HasMany(() => GroupPermission, "collectionId") + collectionGroupMemberships: GroupPermission[]; - @BelongsToMany(() => User, () => CollectionUser) + @BelongsToMany(() => User, () => UserPermission) users: User[]; - @BelongsToMany(() => Group, () => CollectionGroup) + @BelongsToMany(() => Group, () => GroupPermission) groups: Group[]; @BelongsTo(() => User, "createdById") diff --git a/server/models/Group.ts b/server/models/Group.ts index 80343e81a..91b1df91d 100644 --- a/server/models/Group.ts +++ b/server/models/Group.ts @@ -11,7 +11,7 @@ import { DataType, Scopes, } from "sequelize-typescript"; -import CollectionGroup from "./CollectionGroup"; +import GroupPermission from "./GroupPermission"; import GroupUser from "./GroupUser"; import Team from "./Team"; import User from "./User"; @@ -87,7 +87,7 @@ class Group extends ParanoidModel { groupId: model.id, }, }); - await CollectionGroup.destroy({ + await GroupPermission.destroy({ where: { groupId: model.id, }, @@ -106,8 +106,8 @@ class Group extends ParanoidModel { @HasMany(() => GroupUser, { as: "members", foreignKey: "groupId" }) groupMemberships: GroupUser[]; - @HasMany(() => CollectionGroup, "groupId") - collectionGroupMemberships: CollectionGroup[]; + @HasMany(() => GroupPermission, "groupId") + collectionGroupMemberships: GroupPermission[]; @BelongsTo(() => Team, "teamId") team: Team; diff --git a/server/models/CollectionGroup.ts b/server/models/GroupPermission.ts similarity index 62% rename from server/models/CollectionGroup.ts rename to server/models/GroupPermission.ts index c840da91a..960ff3689 100644 --- a/server/models/CollectionGroup.ts +++ b/server/models/GroupPermission.ts @@ -1,3 +1,4 @@ +import { Op } from "sequelize"; import { BelongsTo, Column, @@ -10,6 +11,7 @@ import { } from "sequelize-typescript"; import { CollectionPermission } from "@shared/types"; import Collection from "./Collection"; +import Document from "./Document"; import Group from "./Group"; import User from "./User"; import Model from "./base/Model"; @@ -26,14 +28,20 @@ import Fix from "./decorators/Fix"; withCollection: { include: [ { - association: "collection", + model: Collection, + as: "collection", + where: { + collectionId: { + [Op.ne]: null, + }, + }, }, ], }, })) -@Table({ tableName: "collection_groups", modelName: "collection_group" }) +@Table({ tableName: "group_permissions", modelName: "group_permission" }) @Fix -class CollectionGroup extends Model { +class GroupPermission extends Model { @Default(CollectionPermission.ReadWrite) @IsIn([Object.values(CollectionPermission)]) @Column(DataType.STRING) @@ -42,11 +50,18 @@ class CollectionGroup extends Model { // associations @BelongsTo(() => Collection, "collectionId") - collection: Collection; + collection?: Collection | null; @ForeignKey(() => Collection) @Column(DataType.UUID) - collectionId: string; + collectionId?: string | null; + + @BelongsTo(() => Document, "documentId") + document?: Document | null; + + @ForeignKey(() => Document) + @Column(DataType.UUID) + documentId?: string | null; @BelongsTo(() => Group, "groupId") group: Group; @@ -59,4 +74,4 @@ class CollectionGroup extends Model { createdBy: User; } -export default CollectionGroup; +export default GroupPermission; diff --git a/server/models/User.test.ts b/server/models/User.test.ts index 997f93c19..fc82c772e 100644 --- a/server/models/User.test.ts +++ b/server/models/User.test.ts @@ -1,8 +1,8 @@ import { faker } from "@faker-js/faker"; import { CollectionPermission } from "@shared/types"; import { buildUser, buildTeam, buildCollection } from "@server/test/factories"; -import CollectionUser from "./CollectionUser"; import UserAuthentication from "./UserAuthentication"; +import UserPermission from "./UserPermission"; beforeAll(() => { jest.useFakeTimers().setSystemTime(new Date("2018-01-02T00:00:00.000Z")); @@ -104,7 +104,7 @@ describe("user model", () => { teamId: team.id, permission: null, }); - await CollectionUser.create({ + await UserPermission.create({ createdById: user.id, collectionId: collection.id, userId: user.id, diff --git a/server/models/User.ts b/server/models/User.ts index d8afe4370..913ecb09b 100644 --- a/server/models/User.ts +++ b/server/models/User.ts @@ -41,10 +41,10 @@ import ApiKey from "./ApiKey"; import Attachment from "./Attachment"; import AuthenticationProvider from "./AuthenticationProvider"; import Collection from "./Collection"; -import CollectionUser from "./CollectionUser"; import Star from "./Star"; import Team from "./Team"; import UserAuthentication from "./UserAuthentication"; +import UserPermission from "./UserPermission"; import ParanoidModel from "./base/ParanoidModel"; import Encrypted, { setEncryptedColumn, @@ -545,7 +545,7 @@ class User extends ParanoidModel { }, options ); - await CollectionUser.update( + await UserPermission.update( { permission: CollectionPermission.Read, }, diff --git a/server/models/CollectionUser.ts b/server/models/UserPermission.ts similarity index 63% rename from server/models/CollectionUser.ts rename to server/models/UserPermission.ts index 8bd8e90cd..5804f5738 100644 --- a/server/models/CollectionUser.ts +++ b/server/models/UserPermission.ts @@ -1,3 +1,4 @@ +import { Op } from "sequelize"; import { Column, ForeignKey, @@ -10,6 +11,7 @@ import { } from "sequelize-typescript"; import { CollectionPermission } from "@shared/types"; import Collection from "./Collection"; +import Document from "./Document"; import User from "./User"; import Model from "./base/Model"; import Fix from "./decorators/Fix"; @@ -25,14 +27,20 @@ import Fix from "./decorators/Fix"; withCollection: { include: [ { - association: "collection", + model: Collection, + as: "collection", + where: { + collectionId: { + [Op.ne]: null, + }, + }, }, ], }, })) -@Table({ tableName: "collection_users", modelName: "collection_user" }) +@Table({ tableName: "user_permissions", modelName: "user_permission" }) @Fix -class CollectionUser extends Model { +class UserPermission extends Model { @Default(CollectionPermission.ReadWrite) @IsIn([Object.values(CollectionPermission)]) @Column(DataType.STRING) @@ -41,11 +49,18 @@ class CollectionUser extends Model { // associations @BelongsTo(() => Collection, "collectionId") - collection: Collection; + collection?: Collection | null; @ForeignKey(() => Collection) @Column(DataType.UUID) - collectionId: string; + collectionId?: string | null; + + @BelongsTo(() => Document, "documentId") + document?: Document | null; + + @ForeignKey(() => Document) + @Column(DataType.UUID) + documentId?: string | null; @BelongsTo(() => User, "userId") user: User; @@ -62,4 +77,4 @@ class CollectionUser extends Model { createdById: string; } -export default CollectionUser; +export default UserPermission; diff --git a/server/models/index.ts b/server/models/index.ts index 1b169e476..02d69a3b9 100644 --- a/server/models/index.ts +++ b/server/models/index.ts @@ -8,9 +8,9 @@ export { default as Backlink } from "./Backlink"; export { default as Collection } from "./Collection"; -export { default as CollectionGroup } from "./CollectionGroup"; +export { default as GroupPermission } from "./GroupPermission"; -export { default as CollectionUser } from "./CollectionUser"; +export { default as UserPermission } from "./UserPermission"; export { default as Comment } from "./Comment"; diff --git a/server/policies/collection.test.ts b/server/policies/collection.test.ts index 989d9ed46..d2474df5e 100644 --- a/server/policies/collection.test.ts +++ b/server/policies/collection.test.ts @@ -1,5 +1,5 @@ import { CollectionPermission } from "@shared/types"; -import { CollectionUser, Collection } from "@server/models"; +import { UserPermission, Collection } from "@server/models"; import { buildUser, buildTeam, @@ -59,7 +59,7 @@ describe("member", () => { teamId: team.id, permission: CollectionPermission.ReadWrite, }); - await CollectionUser.create({ + await UserPermission.create({ createdById: user.id, collectionId: collection.id, userId: user.id, @@ -104,7 +104,7 @@ describe("member", () => { teamId: team.id, permission: CollectionPermission.ReadWrite, }); - await CollectionUser.create({ + await UserPermission.create({ createdById: user.id, collectionId: collection.id, userId: user.id, @@ -147,7 +147,7 @@ describe("member", () => { teamId: team.id, permission: CollectionPermission.Read, }); - await CollectionUser.create({ + await UserPermission.create({ createdById: user.id, collectionId: collection.id, userId: user.id, @@ -192,7 +192,7 @@ describe("member", () => { teamId: team.id, permission: null, }); - await CollectionUser.create({ + await UserPermission.create({ createdById: user.id, collectionId: collection.id, userId: user.id, @@ -242,7 +242,7 @@ describe("viewer", () => { teamId: team.id, permission: CollectionPermission.ReadWrite, }); - await CollectionUser.create({ + await UserPermission.create({ createdById: user.id, collectionId: collection.id, userId: user.id, @@ -271,7 +271,7 @@ describe("viewer", () => { teamId: team.id, permission: CollectionPermission.Read, }); - await CollectionUser.create({ + await UserPermission.create({ createdById: user.id, collectionId: collection.id, userId: user.id, @@ -317,7 +317,7 @@ describe("viewer", () => { teamId: team.id, permission: null, }); - await CollectionUser.create({ + await UserPermission.create({ createdById: user.id, collectionId: collection.id, userId: user.id, diff --git a/server/presenters/collectionGroupMembership.ts b/server/presenters/collectionGroupMembership.ts index f13353e13..64dac840b 100644 --- a/server/presenters/collectionGroupMembership.ts +++ b/server/presenters/collectionGroupMembership.ts @@ -1,15 +1,15 @@ import { CollectionPermission } from "@shared/types"; -import { CollectionGroup } from "@server/models"; +import { GroupPermission } from "@server/models"; type Membership = { id: string; groupId: string; - collectionId: string; + collectionId?: string | null; permission: CollectionPermission; }; export default function presentCollectionGroupMembership( - membership: CollectionGroup + membership: GroupPermission ): Membership { return { id: `${membership.groupId}-${membership.collectionId}`, diff --git a/server/presenters/membership.ts b/server/presenters/membership.ts index 02a6cfa33..81dd80b5a 100644 --- a/server/presenters/membership.ts +++ b/server/presenters/membership.ts @@ -1,15 +1,15 @@ import { CollectionPermission } from "@shared/types"; -import { CollectionUser } from "@server/models"; +import { UserPermission } from "@server/models"; type Membership = { id: string; userId: string; - collectionId: string; + collectionId?: string | null; permission: CollectionPermission; }; export default function presentMembership( - membership: CollectionUser + membership: UserPermission ): Membership { return { id: `${membership.userId}-${membership.collectionId}`, diff --git a/server/queues/processors/WebsocketsProcessor.ts b/server/queues/processors/WebsocketsProcessor.ts index 381cdebe1..e48ac0369 100644 --- a/server/queues/processors/WebsocketsProcessor.ts +++ b/server/queues/processors/WebsocketsProcessor.ts @@ -7,7 +7,7 @@ import { Collection, FileOperation, Group, - CollectionGroup, + GroupPermission, GroupUser, Pin, Star, @@ -435,7 +435,9 @@ export default class WebsocketsProcessor { case "groups.add_user": { // do an add user for every collection that the group is a part of - const collectionGroupMemberships = await CollectionGroup.findAll({ + const collectionGroupMemberships = await GroupPermission.scope( + "withCollection" + ).findAll({ where: { groupId: event.modelId, }, @@ -468,7 +470,9 @@ export default class WebsocketsProcessor { } case "groups.remove_user": { - const collectionGroupMemberships = await CollectionGroup.findAll({ + const collectionGroupMemberships = await GroupPermission.scope( + "withCollection" + ).findAll({ where: { groupId: event.modelId, }, @@ -477,9 +481,11 @@ export default class WebsocketsProcessor { for (const collectionGroup of collectionGroupMemberships) { // if the user has any memberships remaining on the collection // we need to emit add instead of remove - const collection = await Collection.scope({ - method: ["withMembership", event.userId], - }).findByPk(collectionGroup.collectionId); + const collection = collectionGroup.collectionId + ? await Collection.scope({ + method: ["withMembership", event.userId], + }).findByPk(collectionGroup.collectionId) + : null; if (!collection) { continue; @@ -535,7 +541,9 @@ export default class WebsocketsProcessor { }, }, }); - const collectionGroupMemberships = await CollectionGroup.findAll({ + const collectionGroupMemberships = await GroupPermission.scope( + "withCollection" + ).findAll({ paranoid: false, where: { groupId: event.modelId, @@ -546,9 +554,9 @@ export default class WebsocketsProcessor { }); for (const collectionGroup of collectionGroupMemberships) { - const membershipUserIds = await Collection.membershipUserIds( - collectionGroup.collectionId - ); + const membershipUserIds = collectionGroup.collectionId + ? await Collection.membershipUserIds(collectionGroup.collectionId) + : []; for (const groupUser of groupUsers) { if (membershipUserIds.includes(groupUser.userId)) { diff --git a/server/routes/api/attachments/attachments.test.ts b/server/routes/api/attachments/attachments.test.ts index 3d0b16aa4..674ec2d47 100644 --- a/server/routes/api/attachments/attachments.test.ts +++ b/server/routes/api/attachments/attachments.test.ts @@ -1,5 +1,5 @@ import { AttachmentPreset, CollectionPermission } from "@shared/types"; -import { CollectionUser } from "@server/models"; +import { UserPermission } from "@server/models"; import Attachment from "@server/models/Attachment"; import { buildUser, @@ -123,7 +123,7 @@ describe("#attachments.create", () => { collectionId: collection.id, }); - await CollectionUser.create({ + await UserPermission.create({ createdById: user.id, collectionId: collection.id, userId: user.id, diff --git a/server/routes/api/collections/collections.test.ts b/server/routes/api/collections/collections.test.ts index ba93f4b52..2bf832d7d 100644 --- a/server/routes/api/collections/collections.test.ts +++ b/server/routes/api/collections/collections.test.ts @@ -1,6 +1,6 @@ import { CollectionPermission } from "@shared/types"; import { colorPalette } from "@shared/utils/collections"; -import { Document, CollectionUser, CollectionGroup } from "@server/models"; +import { Document, UserPermission, GroupPermission } from "@server/models"; import { buildUser, buildAdmin, @@ -310,7 +310,7 @@ describe("#collections.export", () => { const collection = await buildCollection({ teamId: team.id }); collection.permission = null; await collection.save(); - await CollectionUser.create({ + await UserPermission.create({ createdById: admin.id, collectionId: collection.id, userId: admin.id, @@ -750,13 +750,13 @@ describe("#collections.group_memberships", () => { permission: null, teamId: user.teamId, }); - await CollectionUser.create({ + await UserPermission.create({ createdById: user.id, collectionId: collection.id, userId: user.id, permission: CollectionPermission.ReadWrite, }); - await CollectionGroup.create({ + await GroupPermission.create({ createdById: user.id, collectionId: collection.id, groupId: group.id, @@ -792,19 +792,19 @@ describe("#collections.group_memberships", () => { permission: null, teamId: user.teamId, }); - await CollectionUser.create({ + await UserPermission.create({ createdById: user.id, collectionId: collection.id, userId: user.id, permission: CollectionPermission.ReadWrite, }); - await CollectionGroup.create({ + await GroupPermission.create({ createdById: user.id, collectionId: collection.id, groupId: group.id, permission: CollectionPermission.ReadWrite, }); - await CollectionGroup.create({ + await GroupPermission.create({ createdById: user.id, collectionId: collection.id, groupId: group2.id, @@ -835,19 +835,19 @@ describe("#collections.group_memberships", () => { permission: null, teamId: user.teamId, }); - await CollectionUser.create({ + await UserPermission.create({ createdById: user.id, collectionId: collection.id, userId: user.id, permission: CollectionPermission.ReadWrite, }); - await CollectionGroup.create({ + await GroupPermission.create({ createdById: user.id, collectionId: collection.id, groupId: group.id, permission: CollectionPermission.ReadWrite, }); - await CollectionGroup.create({ + await GroupPermission.create({ createdById: user.id, collectionId: collection.id, groupId: group2.id, @@ -926,7 +926,7 @@ describe("#collections.memberships", () => { const user2 = await buildUser({ name: "Won't find", }); - await CollectionUser.create({ + await UserPermission.create({ createdById: user2.id, collectionId: collection.id, userId: user2.id, @@ -953,13 +953,13 @@ describe("#collections.memberships", () => { teamId: team.id, }); const user2 = await buildUser(); - await CollectionUser.create({ + await UserPermission.create({ createdById: user.id, collectionId: collection.id, userId: user.id, permission: CollectionPermission.ReadWrite, }); - await CollectionUser.create({ + await UserPermission.create({ createdById: user2.id, collectionId: collection.id, userId: user2.id, @@ -1026,7 +1026,7 @@ describe("#collections.info", () => { }); collection.permission = null; await collection.save(); - await CollectionUser.destroy({ + await UserPermission.destroy({ where: { collectionId: collection.id, userId: user.id, @@ -1050,7 +1050,7 @@ describe("#collections.info", () => { }); collection.permission = null; await collection.save(); - await CollectionUser.create({ + await UserPermission.create({ collectionId: collection.id, userId: user.id, createdById: user.id, @@ -1342,7 +1342,7 @@ describe("#collections.update", () => { const collection = await buildCollection({ teamId: team.id }); collection.permission = null; await collection.save(); - await CollectionUser.create({ + await UserPermission.create({ collectionId: collection.id, userId: admin.id, createdById: admin.id, @@ -1371,7 +1371,7 @@ describe("#collections.update", () => { const collection = await buildCollection({ teamId: team.id }); collection.permission = null; await collection.save(); - await CollectionUser.create({ + await UserPermission.create({ collectionId: collection.id, userId: admin.id, createdById: admin.id, @@ -1432,7 +1432,7 @@ describe("#collections.update", () => { }); collection.permission = null; await collection.save(); - await CollectionUser.update( + await UserPermission.update( { createdById: user.id, permission: CollectionPermission.Read, diff --git a/server/routes/api/collections/collections.ts b/server/routes/api/collections/collections.ts index a5ea2a9c0..df0565648 100644 --- a/server/routes/api/collections/collections.ts +++ b/server/routes/api/collections/collections.ts @@ -16,8 +16,8 @@ import { transaction } from "@server/middlewares/transaction"; import validate from "@server/middlewares/validate"; import { Collection, - CollectionUser, - CollectionGroup, + UserPermission, + GroupPermission, Team, Event, User, @@ -220,7 +220,7 @@ router.post( const group = await Group.findByPk(groupId); authorize(user, "read", group); - let membership = await CollectionGroup.findOne({ + let membership = await GroupPermission.findOne({ where: { collectionId: id, groupId, @@ -228,7 +228,7 @@ router.post( }); if (!membership) { - membership = await CollectionGroup.create({ + membership = await GroupPermission.create({ collectionId: id, groupId, permission, @@ -310,7 +310,7 @@ router.post( }).findByPk(id); authorize(user, "read", collection); - let where: WhereOptions = { + let where: WhereOptions = { collectionId: id, }; let groupWhere; @@ -340,8 +340,8 @@ router.post( }; const [total, memberships] = await Promise.all([ - CollectionGroup.count(options), - CollectionGroup.findAll({ + GroupPermission.count(options), + GroupPermission.findAll({ ...options, order: [["createdAt", "DESC"]], offset: ctx.state.pagination.offset, @@ -379,7 +379,7 @@ router.post( const user = await User.findByPk(userId); authorize(actor, "read", user); - let membership = await CollectionUser.findOne({ + let membership = await UserPermission.findOne({ where: { collectionId: id, userId, @@ -389,7 +389,7 @@ router.post( }); if (!membership) { - membership = await CollectionUser.create( + membership = await UserPermission.create( { collectionId: id, userId, @@ -485,7 +485,7 @@ router.post( }).findByPk(id); authorize(user, "read", collection); - let where: WhereOptions = { + let where: WhereOptions = { collectionId: id, }; let userWhere; @@ -515,8 +515,8 @@ router.post( }; const [total, memberships] = await Promise.all([ - CollectionUser.count(options), - CollectionUser.findAll({ + UserPermission.count(options), + UserPermission.findAll({ ...options, order: [["createdAt", "DESC"]], offset: ctx.state.pagination.offset, @@ -627,7 +627,7 @@ router.post( permission !== CollectionPermission.ReadWrite && collection.permission === CollectionPermission.ReadWrite ) { - await CollectionUser.findOrCreate({ + await UserPermission.findOrCreate({ where: { collectionId: collection.id, userId: user.id, diff --git a/server/routes/api/documents/documents.test.ts b/server/routes/api/documents/documents.test.ts index 51dde1a48..e450e6bef 100644 --- a/server/routes/api/documents/documents.test.ts +++ b/server/routes/api/documents/documents.test.ts @@ -5,11 +5,11 @@ import { View, Revision, Backlink, - CollectionUser, + UserPermission, SearchQuery, Event, User, - CollectionGroup, + GroupPermission, } from "@server/models"; import DocumentHelper from "@server/models/helpers/DocumentHelper"; import { @@ -905,7 +905,7 @@ describe("#documents.list", () => { collectionId: collection.id, }); - await CollectionUser.update( + await UserPermission.update( { userId: user.id, permission: CollectionPermission.Read, @@ -1606,7 +1606,7 @@ describe("#documents.search", () => { permission: null, }); - await CollectionUser.create({ + await UserPermission.create({ createdById: user.id, collectionId: collection.id, userId: user.id, @@ -1973,7 +1973,7 @@ describe("#documents.viewed", () => { documentId: document.id, userId: user.id, }); - await CollectionUser.destroy({ + await UserPermission.destroy({ where: { userId: user.id, collectionId: collection.id, @@ -2997,7 +2997,7 @@ describe("#documents.update", () => { userId: user.id, teamId: user.teamId, }); - await CollectionUser.update( + await UserPermission.update( { userId: user.id, permission: CollectionPermission.ReadWrite, @@ -3104,7 +3104,7 @@ describe("#documents.update", () => { teamId: team.id, }); - await CollectionUser.update( + await UserPermission.update( { createdById: user.id, permission: CollectionPermission.ReadWrite, @@ -3142,7 +3142,7 @@ describe("#documents.update", () => { collectionId: collection.id, teamId: team.id, }); - await CollectionUser.update( + await UserPermission.update( { createdById: user.id, permission: CollectionPermission.Read, @@ -3178,7 +3178,7 @@ describe("#documents.update", () => { }); collection.permission = CollectionPermission.Read; await collection.save(); - await CollectionUser.destroy({ + await UserPermission.destroy({ where: { userId: user.id, collectionId: collection.id, @@ -3773,25 +3773,25 @@ describe("#documents.users", () => { // add people and groups to collection await Promise.all([ - CollectionUser.create({ + UserPermission.create({ collectionId: collection.id, userId: alan.id, permission: CollectionPermission.Read, createdById: user.id, }), - CollectionUser.create({ + UserPermission.create({ collectionId: collection.id, userId: bret.id, permission: CollectionPermission.Read, createdById: user.id, }), - CollectionUser.create({ + UserPermission.create({ collectionId: collection.id, userId: ken.id, permission: CollectionPermission.Read, createdById: user.id, }), - CollectionGroup.create({ + GroupPermission.create({ collectionId: collection.id, groupId: group.id, permission: CollectionPermission.ReadWrite, @@ -3863,19 +3863,19 @@ describe("#documents.users", () => { // add people to collection await Promise.all([ - CollectionUser.create({ + UserPermission.create({ collectionId: collection.id, userId: alan.id, permission: CollectionPermission.Read, createdById: user.id, }), - CollectionUser.create({ + UserPermission.create({ collectionId: collection.id, userId: bret.id, permission: CollectionPermission.Read, createdById: user.id, }), - CollectionUser.create({ + UserPermission.create({ collectionId: collection.id, userId: ken.id, permission: CollectionPermission.Read, diff --git a/server/routes/api/revisions/revisions.test.ts b/server/routes/api/revisions/revisions.test.ts index 5aa12f747..850d82352 100644 --- a/server/routes/api/revisions/revisions.test.ts +++ b/server/routes/api/revisions/revisions.test.ts @@ -1,4 +1,4 @@ -import { CollectionUser, Revision } from "@server/models"; +import { UserPermission, Revision } from "@server/models"; import { buildCollection, buildDocument, @@ -175,7 +175,7 @@ describe("#revisions.list", () => { await Revision.createFromDocument(document); collection.permission = null; await collection.save(); - await CollectionUser.destroy({ + await UserPermission.destroy({ where: { userId: user.id, collectionId: collection.id, diff --git a/server/routes/api/shares/shares.test.ts b/server/routes/api/shares/shares.test.ts index 2bb2a13ab..670e25355 100644 --- a/server/routes/api/shares/shares.test.ts +++ b/server/routes/api/shares/shares.test.ts @@ -1,5 +1,5 @@ import { CollectionPermission } from "@shared/types"; -import { CollectionUser, Share } from "@server/models"; +import { UserPermission, Share } from "@server/models"; import { buildUser, buildDocument, @@ -240,7 +240,7 @@ describe("#shares.create", () => { }); collection.permission = null; await collection.save(); - await CollectionUser.update( + await UserPermission.update( { userId: user.id, permission: CollectionPermission.Read, diff --git a/server/routes/api/views/views.test.ts b/server/routes/api/views/views.test.ts index 2c299d928..c165a4e75 100644 --- a/server/routes/api/views/views.test.ts +++ b/server/routes/api/views/views.test.ts @@ -1,5 +1,5 @@ import { CollectionPermission } from "@shared/types"; -import { View, CollectionUser } from "@server/models"; +import { View, UserPermission } from "@server/models"; import { buildAdmin, buildCollection, @@ -71,7 +71,7 @@ describe("#views.list", () => { }); collection.permission = null; await collection.save(); - await CollectionUser.create({ + await UserPermission.create({ createdById: user.id, collectionId: collection.id, userId: user.id, @@ -150,7 +150,7 @@ describe("#views.create", () => { }); collection.permission = null; await collection.save(); - await CollectionUser.create({ + await UserPermission.create({ createdById: user.id, collectionId: collection.id, userId: user.id,