From 7c7febd49589e88123e7ee9f30e5cadb63eba630 Mon Sep 17 00:00:00 2001 From: forehalo Date: Thu, 6 Feb 2025 10:52:05 +0000 Subject: [PATCH] refactor(server): remove never used column in page permission (#9985) --- .../migration.sql | 16 +++ packages/backend/server/schema.prisma | 5 +- .../backend/server/src/base/graphql/index.ts | 3 + .../server/src/base/graphql/pagination.ts | 127 ++++++++++++++++++ packages/backend/server/src/base/index.ts | 9 +- .../server/src/core/permission/service.ts | 117 +++++++--------- .../src/core/workspaces/resolvers/page.ts | 112 +++------------ packages/backend/server/src/models/page.ts | 2 - packages/backend/server/src/schema.gql | 50 ++++--- 9 files changed, 251 insertions(+), 190 deletions(-) create mode 100644 packages/backend/server/migrations/20250206082414_break_update_used_page_permission_table/migration.sql create mode 100644 packages/backend/server/src/base/graphql/pagination.ts diff --git a/packages/backend/server/migrations/20250206082414_break_update_used_page_permission_table/migration.sql b/packages/backend/server/migrations/20250206082414_break_update_used_page_permission_table/migration.sql new file mode 100644 index 0000000000..d2cea62464 --- /dev/null +++ b/packages/backend/server/migrations/20250206082414_break_update_used_page_permission_table/migration.sql @@ -0,0 +1,16 @@ +/* + Warnings: + + - The primary key for the `workspace_page_user_permissions` table will be changed. If it partially fails, the table could be left without primary key constraint. + - You are about to drop the column `accepted` on the `workspace_page_user_permissions` table. All the data in the column will be lost. + - You are about to drop the column `id` on the `workspace_page_user_permissions` table. All the data in the column will be lost. + +*/ +-- DropIndex +DROP INDEX "workspace_page_user_permissions_workspace_id_page_id_user_i_key"; + +-- AlterTable +ALTER TABLE "workspace_page_user_permissions" DROP CONSTRAINT "workspace_page_user_permissions_pkey", +DROP COLUMN "accepted", +DROP COLUMN "id", +ADD CONSTRAINT "workspace_page_user_permissions_pkey" PRIMARY KEY ("workspace_id", "page_id", "user_id"); diff --git a/packages/backend/server/schema.prisma b/packages/backend/server/schema.prisma index 0453735a04..843d7c18e6 100644 --- a/packages/backend/server/schema.prisma +++ b/packages/backend/server/schema.prisma @@ -161,20 +161,17 @@ model WorkspaceUserPermission { } model WorkspacePageUserPermission { - id String @id @default(uuid()) @db.VarChar workspaceId String @map("workspace_id") @db.VarChar pageId String @map("page_id") @db.VarChar userId String @map("user_id") @db.VarChar // External/Reader/Editor/Manager/Owner type Int @db.SmallInt - /// Whether the permission invitation is accepted by the user - accepted Boolean @default(false) createdAt DateTime @default(now()) @map("created_at") @db.Timestamptz(3) user User @relation(fields: [userId], references: [id], onDelete: Cascade) workspace Workspace @relation(fields: [workspaceId], references: [id], onDelete: Cascade) - @@unique([workspaceId, pageId, userId]) + @@id([workspaceId, pageId, userId]) @@map("workspace_page_user_permissions") } diff --git a/packages/backend/server/src/base/graphql/index.ts b/packages/backend/server/src/base/graphql/index.ts index c513f8ffdc..7e7b4fad61 100644 --- a/packages/backend/server/src/base/graphql/index.ts +++ b/packages/backend/server/src/base/graphql/index.ts @@ -84,3 +84,6 @@ export type GraphqlContext = { ], }) export class GqlModule {} + +export * from './pagination'; +export { registerObjectType } from './register'; diff --git a/packages/backend/server/src/base/graphql/pagination.ts b/packages/backend/server/src/base/graphql/pagination.ts new file mode 100644 index 0000000000..de28cc3625 --- /dev/null +++ b/packages/backend/server/src/base/graphql/pagination.ts @@ -0,0 +1,127 @@ +import { Type } from '@nestjs/common'; +import { + Field, + FieldMiddleware, + InputType, + Int, + MiddlewareContext, + NextFn, + ObjectType, +} from '@nestjs/graphql'; + +const parseCursorMiddleware: FieldMiddleware = async ( + _ctx: MiddlewareContext, + next: NextFn +) => { + const value = await next(); + return value === undefined || value === null ? null : decode(value); +}; + +@InputType() +export class PaginationInput { + @Field(() => Int, { + nullable: true, + description: 'returns the first n elements from the list.', + defaultValue: 10, + }) + first!: number; + + @Field(() => Int, { + nullable: true, + description: 'ignore the first n elements from the list.', + defaultValue: 0, + }) + offset!: number; + + @Field(() => String, { + nullable: true, + description: + 'returns the elements in the list that come after the specified cursor.', + middleware: [parseCursorMiddleware], + }) + after!: string | null; + + @Field(() => String, { + nullable: true, + description: + 'returns the elements in the list that come before the specified cursor.', + middleware: [parseCursorMiddleware], + }) + before!: string | null; +} + +const encode = (input: string) => Buffer.from(input).toString('base64'); +const decode = (base64String: string) => + Buffer.from(base64String, 'base64').toString('utf-8'); + +export function paginate( + list: T[], + cursorField: keyof T, + paginationInput: PaginationInput, + total: number +): PaginatedType { + const edges = list.map(item => ({ + node: item, + cursor: encode(String(item[cursorField])), + })); + + return { + totalCount: total, + edges, + pageInfo: { + hasNextPage: edges.length >= paginationInput.first, + hasPreviousPage: paginationInput.offset > 0, + endCursor: edges.length ? edges[edges.length - 1].cursor : null, + startCursor: edges.length ? edges[0].cursor : null, + }, + }; +} + +export interface PaginatedType { + totalCount: number; + edges: { + cursor: string; + node: T; + }[]; + pageInfo: PageInfo; +} + +@ObjectType() +export class PageInfo { + @Field(() => String, { nullable: true }) + startCursor?: string | null; + + @Field(() => String, { nullable: true }) + endCursor?: string | null; + + @Field() + hasNextPage!: boolean; + + @Field() + hasPreviousPage!: boolean; +} + +export function Paginated(classRef: Type): any { + @ObjectType(`${classRef.name}Edge`) + abstract class EdgeType { + @Field(() => String) + cursor!: string; + + @Field(() => classRef) + node!: T; + } + + @ObjectType({ isAbstract: true }) + abstract class PaginatedType { + @Field(() => Int) + totalCount!: number; + + @Field(() => [EdgeType]) + edges!: EdgeType[]; + + @Field(() => PageInfo) + pageInfo!: PageInfo; + } + + return PaginatedType; +} diff --git a/packages/backend/server/src/base/index.ts b/packages/backend/server/src/base/index.ts index 588c02447b..e33a27dfe9 100644 --- a/packages/backend/server/src/base/index.ts +++ b/packages/backend/server/src/base/index.ts @@ -15,8 +15,13 @@ export { } from './config'; export * from './error'; export { EventBus, OnEvent } from './event'; -export type { GraphqlContext } from './graphql'; -export { registerObjectType } from './graphql/register'; +export { + type GraphqlContext, + paginate, + Paginated, + PaginationInput, + registerObjectType, +} from './graphql'; export * from './guard'; export { CryptoHelper, URLHelper } from './helpers'; export { AFFiNELogger } from './logger'; diff --git a/packages/backend/server/src/core/permission/service.ts b/packages/backend/server/src/core/permission/service.ts index 430b204454..34915c9d88 100644 --- a/packages/backend/server/src/core/permission/service.ts +++ b/packages/backend/server/src/core/permission/service.ts @@ -35,8 +35,8 @@ export class PermissionService { const { workspaceId, docId, editor } = payload; await this.prisma.$queryRaw` - INSERT INTO "workspace_page_user_permissions" ("workspace_id", "page_id", "user_id", "type", "accepted", "created_at", "updated_at") - VALUES (${workspaceId}, ${docId}, ${editor}, ${DocRole.Owner}, true, now(), now()) + INSERT INTO "workspace_page_user_permissions" ("workspace_id", "page_id", "user_id", "type", "created_at") + VALUES (${workspaceId}, ${docId}, ${editor}, ${DocRole.Owner}, now()) ON CONFLICT ("workspace_id", "page_id", "user_id") DO NOTHING `; @@ -576,7 +576,6 @@ export class PermissionService { workspaceId: ws, pageId: page, userId: user, - accepted: true, type: { gte: role, }, @@ -658,65 +657,48 @@ export class PermissionService { }); } - async grantPage( - ws: string, - page: string, - user: string, - permission: DocRole - ): Promise { - const data = await this.prisma.workspacePageUserPermission.findFirst({ - where: { - workspaceId: ws, - pageId: page, - userId: user, - accepted: true, - }, - }); - - if (data) { - const [p] = await this.prisma.$transaction( - [ - this.prisma.workspacePageUserPermission.update({ - where: { - id: data.id, + async grantPage(ws: string, page: string, user: string, permission: DocRole) { + const [p] = await this.prisma.$transaction( + [ + this.prisma.workspacePageUserPermission.upsert({ + where: { + workspaceId_pageId_userId: { + workspaceId: ws, + pageId: page, + userId: user, }, - data: { - type: permission, - }, - }), + }, + update: { + type: permission, + }, + create: { + workspaceId: ws, + pageId: page, + userId: user, + type: permission, + }, + }), - // If the new permission is owner, we need to revoke old owner - permission === DocRole.Owner - ? this.prisma.workspacePageUserPermission.updateMany({ - where: { - workspaceId: ws, - pageId: page, - type: DocRole.Owner, - userId: { - not: user, - }, + // If the new permission is owner, we need to revoke old owner + permission === DocRole.Owner + ? this.prisma.workspacePageUserPermission.updateMany({ + where: { + workspaceId: ws, + pageId: page, + type: DocRole.Owner, + userId: { + not: user, }, - data: { - type: DocRole.Manager, - }, - }) - : null, - ].filter(Boolean) as Prisma.PrismaPromise[] - ); + }, + data: { + type: DocRole.Manager, + }, + }) + : null, + ].filter(Boolean) as Prisma.PrismaPromise[] + ); - return p.id; - } - - return this.prisma.workspacePageUserPermission - .create({ - data: { - workspaceId: ws, - pageId: page, - userId: user, - type: permission, - }, - }) - .then(p => p.id); + return p; } async revokePage(ws: string, page: string, users: string[]) { @@ -746,14 +728,11 @@ export class PermissionService { if (userIds.length === 0) { return []; } - if (role === DocRole.Owner) { - if (userIds.length > 1) { - throw new SpaceShouldHaveOnlyOneOwner({ spaceId: workspaceId }); - } - return [await this.grantPage(workspaceId, pageId, userIds[0], role)]; + if (role === DocRole.Owner && userIds.length > 1) { + throw new SpaceShouldHaveOnlyOneOwner({ spaceId: workspaceId }); } - const ret = await this.prisma.$transaction(async tx => + return await this.prisma.$transaction(async tx => Promise.all( userIds.map(id => tx.workspacePageUserPermission.upsert({ @@ -777,7 +756,6 @@ export class PermissionService { ) ) ); - return ret.map(p => p.id); } async updatePagePermission( @@ -798,14 +776,17 @@ export class PermissionService { return this.grantPage(workspaceId, pageId, userId, role); } - const { id } = await this.prisma.workspacePageUserPermission.update({ + return await this.prisma.workspacePageUserPermission.update({ where: { - id: permission.id, + workspaceId_pageId_userId: { + workspaceId, + pageId, + userId, + }, }, data: { type: role, }, }); - return id; } } diff --git a/packages/backend/server/src/core/workspaces/resolvers/page.ts b/packages/backend/server/src/core/workspaces/resolvers/page.ts index 25cb330eca..53d2138c58 100644 --- a/packages/backend/server/src/core/workspaces/resolvers/page.ts +++ b/packages/backend/server/src/core/workspaces/resolvers/page.ts @@ -3,7 +3,6 @@ import { Args, Field, InputType, - Int, Mutation, ObjectType, Parent, @@ -21,6 +20,9 @@ import { ExpectToRevokePublicPage, ExpectToUpdateDocUserRole, PageIsNotPublic, + paginate, + Paginated, + PaginationInput, registerObjectType, } from '../../../base'; import { CurrentUser } from '../../auth'; @@ -34,7 +36,6 @@ import { PublicPageMode, WorkspaceRole, } from '../../permission'; -import { UserType } from '../../user'; import { DocID } from '../../utils/doc'; import { WorkspaceType } from '../types'; @@ -73,65 +74,23 @@ class GrantDocUserRolesInput { userIds!: string[]; } -@InputType() -class PageGrantedUsersInput { - @Field(() => Int) - first!: number; - - @Field(() => Int) - offset?: number; - - @Field(() => String, { description: 'Cursor', nullable: true }) - after?: string; - - @Field(() => String, { description: 'Cursor', nullable: true }) - before?: string; -} - @ObjectType() class GrantedDocUserType { - @Field(() => UserType) - user!: UserType; - - @Field(() => DocRole) - role!: DocRole; -} - -@ObjectType() -class PageInfo { - @Field(() => String, { nullable: true }) - startCursor?: string; - - @Field(() => String, { nullable: true }) - endCursor?: string; - - @Field(() => Boolean) - hasNextPage!: boolean; - - @Field(() => Boolean) - hasPreviousPage!: boolean; -} - -@ObjectType() -class GrantedDocUserEdge { - @Field(() => GrantedDocUserType) - user!: GrantedDocUserType; + @Field(() => String) + workspaceId!: string; @Field(() => String) - cursor!: string; + pageId!: string; + + @Field(() => String) + userId!: string; + + @Field(() => DocRole, { name: 'role' }) + type!: DocRole; } @ObjectType() -class GrantedDocUsersConnection { - @Field(() => Int) - totalCount!: number; - - @Field(() => [GrantedDocUserEdge]) - edges!: GrantedDocUserEdge[]; - - @Field(() => PageInfo) - pageInfo!: PageInfo; -} +class PaginatedGrantedDocUserType extends Paginated(GrantedDocUserType) {} const DocPermissions = registerObjectType( Object.fromEntries( @@ -261,16 +220,15 @@ export class PagePermissionResolver { }; } - @ResolveField(() => GrantedDocUsersConnection, { + @ResolveField(() => PaginatedGrantedDocUserType, { description: 'Page granted users list', complexity: 4, }) async pageGrantedUsersList( @Parent() workspace: WorkspaceType, @Args('pageId') pageId: string, - @Args('pageGrantedUsersInput') - pageGrantedUsersInput: PageGrantedUsersInput - ): Promise { + @Args('pagination') pagination: PaginationInput + ): Promise { const docId = new DocID(pageId, workspace.id); const [permissions, totalCount] = await this.prisma.$transaction(tx => { return Promise.all([ @@ -279,19 +237,11 @@ export class PagePermissionResolver { workspaceId: workspace.id, pageId: docId.guid, }, - include: { - user: true, - }, orderBy: { createdAt: 'desc', }, - take: pageGrantedUsersInput.first, - skip: pageGrantedUsersInput.offset, - cursor: pageGrantedUsersInput.after - ? { - id: pageGrantedUsersInput.after, - } - : undefined, + take: pagination.first, + skip: pagination.offset, }), tx.workspacePageUserPermission.count({ where: { @@ -302,31 +252,7 @@ export class PagePermissionResolver { ]); }); - return { - totalCount, - edges: permissions.map(permission => ({ - user: { - user: { - id: permission.user.id, - name: permission.user.name, - email: permission.user.email, - avatarUrl: permission.user.avatarUrl, - emailVerified: permission.user.emailVerifiedAt !== null, - hasPassword: permission.user.password !== null, - }, - role: permission.type, - }, - cursor: permission.id, - })), - pageInfo: { - startCursor: permissions.at(0)?.id, - endCursor: permissions.at(-1)?.id, - hasNextPage: totalCount > pageGrantedUsersInput.first, - hasPreviousPage: - pageGrantedUsersInput.offset !== undefined && - pageGrantedUsersInput.offset > 0, - }, - }; + return paginate(permissions, 'createdAt', pagination, totalCount); } /** diff --git a/packages/backend/server/src/models/page.ts b/packages/backend/server/src/models/page.ts index 83f5140ade..b17dc23eb7 100644 --- a/packages/backend/server/src/models/page.ts +++ b/packages/backend/server/src/models/page.ts @@ -127,8 +127,6 @@ export class PageModel extends BaseModel { pageId, userId, type: permission, - // page permission does not require invitee to accept, the accepted field will be deprecated later. - accepted: true, }, }); } diff --git a/packages/backend/server/src/schema.gql b/packages/backend/server/src/schema.gql index 15baaba95c..2c6dbd2980 100644 --- a/packages/backend/server/src/schema.gql +++ b/packages/backend/server/src/schema.gql @@ -383,20 +383,16 @@ input GrantDocUserRolesInput { workspaceId: String! } -type GrantedDocUserEdge { - cursor: String! - user: GrantedDocUserType! -} - type GrantedDocUserType { + pageId: String! role: DocRole! - user: UserType! + userId: String! + workspaceId: String! } -type GrantedDocUsersConnection { - edges: [GrantedDocUserEdge!]! - pageInfo: PageInfo! - totalCount: Int! +type GrantedDocUserTypeEdge { + cursor: String! + node: GrantedDocUserType! } type InvalidEmailDataType { @@ -685,16 +681,6 @@ enum OAuthProviderType { OIDC } -input PageGrantedUsersInput { - """Cursor""" - after: String - - """Cursor""" - before: String - first: Int! - offset: Int! -} - type PageInfo { endCursor: String hasNextPage: Boolean! @@ -702,6 +688,28 @@ type PageInfo { startCursor: String } +type PaginatedGrantedDocUserType { + edges: [GrantedDocUserTypeEdge!]! + pageInfo: PageInfo! + totalCount: Int! +} + +input PaginationInput { + """returns the elements in the list that come after the specified cursor.""" + after: String + + """ + returns the elements in the list that come before the specified cursor. + """ + before: String + + """returns the first n elements from the list.""" + first: Int = 10 + + """ignore the first n elements from the list.""" + offset: Int = 0 +} + type PasswordLimitsType { maxLength: Int! minLength: Int! @@ -1200,7 +1208,7 @@ type WorkspaceType { owner: UserType! """Page granted users list""" - pageGrantedUsersList(pageGrantedUsersInput: PageGrantedUsersInput!, pageId: String!): GrantedDocUsersConnection! + pageGrantedUsersList(pageId: String!, pagination: PaginationInput!): PaginatedGrantedDocUserType! """Cloud page metadata of workspace""" pageMeta(pageId: String!): WorkspacePageMeta!