refactor: add invitation api tests (#5324)

* refactor: add invitation api tests

* refactor: update alteration

* refactor: fix config guard

packages/core/src/libraries/organization-invitation.ts

@@ -72,7 +72,8 @@ export class OrganizationInvitationLibrary {
         await this.sendEmail(invitee, magicLink.token);
       }
 
-      return invitation;
+      // Additional query to get the full invitation data
+      return organizationQueries.invitations.findById(invitation.id);
     });
   }
 

packages/core/src/queries/organization/index.ts

@@ -134,7 +134,7 @@ class OrganizationInvitationsQueries extends SchemaQueries<
         ${buildJsonObjectSql(magicLinks.fields)} as "magicLink"
       from ${table}
       left join ${roleRelations.table}
-        on ${roleRelations.fields.invitationId} = ${fields.id}
+        on ${roleRelations.fields.organizationInvitationId} = ${fields.id}
       left join ${roles.table}
         on ${roles.fields.id} = ${roleRelations.fields.organizationRoleId}
       left join ${magicLinks.table}

packages/core/src/routes/organization/invitations.ts

@@ -1,6 +1,8 @@
-import { OrganizationInvitations } from '@logto/schemas';
+import { OrganizationInvitations, organizationInvitationEntityGuard } from '@logto/schemas';
+import { yes } from '@silverhand/essentials';
 import { z } from 'zod';
 
+import RequestError from '#src/errors/RequestError/index.js';
 import koaGuard from '#src/middleware/koa-guard.js';
 import SchemaRouter from '#src/utils/SchemaRouter.js';
 import assertThat from '#src/utils/assert-that.js';
@@ -26,13 +28,14 @@ export default function organizationInvitationRoutes<T extends AuthedRouter>(
       post: true,
       patchById: true,
     },
+    entityGuard: organizationInvitationEntityGuard,
   });
 
   router.post(
     '/',
     koaGuard({
       query: z.object({
-        skipEmail: z.boolean().optional(),
+        skipEmail: z.string().optional(),
       }),
       body: OrganizationInvitations.createGuard
         .pick({
@@ -45,7 +48,7 @@ export default function organizationInvitationRoutes<T extends AuthedRouter>(
           invitee: z.string().email(),
           organizationRoleIds: z.string().array().optional(),
         }),
-      response: OrganizationInvitations.guard,
+      response: organizationInvitationEntityGuard,
       status: [201],
     }),
     async (ctx) => {
@@ -53,12 +56,14 @@ export default function organizationInvitationRoutes<T extends AuthedRouter>(
 
       assertThat(
         body.expiresAt > Date.now(),
-        // TODO: Throw `RequestError` instead.
-        new Error('The value of `expiresAt` must be in the future.')
+        new RequestError({
+          code: 'request.invalid_input',
+          details: 'The value of `expiresAt` must be in the future.',
+        })
       );
 
-      ctx.body = await organizationInvitations.insert(body, query.skipEmail);
-      ctx.body = 201;
+      ctx.body = await organizationInvitations.insert(body, yes(query.skipEmail));
+      ctx.status = 201;
     }
   );
 

packages/core/src/utils/SchemaRouter.ts

@@ -2,7 +2,6 @@ import { type SchemaLike, type GeneratedSchema } from '@logto/schemas';
 import { generateStandardId } from '@logto/shared';
 import { type DeepPartial } from '@silverhand/essentials';
 import camelcase from 'camelcase';
-import deepmerge from 'deepmerge';
 import { type MiddlewareType } from 'koa';
 import Router, { type IRouterParamContext } from 'koa-router';
 import { z } from 'zod';
@@ -15,6 +14,17 @@ import { type TwoRelationsQueries } from './RelationQueries.js';
 import type SchemaQueries from './SchemaQueries.js';
 import { parseSearchOptions } from './search.js';
 
+const defaultConfig = Object.freeze({
+  disabled: {
+    get: false,
+    post: false,
+    getById: false,
+    patchById: false,
+    deleteById: false,
+  },
+  searchFields: [],
+});
+
 /**
  * Generate the pathname for from a table name.
  *
@@ -63,6 +73,16 @@ type SchemaRouterConfig<Key extends string> = {
    * @see {@link generateStandardId} for the default length.
    */
   idLength?: number;
+  /**
+   * The guard for the entity returned by the following routes:
+   *
+   * - `GET /:id`
+   * - `POST /`
+   * - `PATCH /:id`
+   *
+   * If not provided, the `schema.guard` will be used.
+   */
+  entityGuard?: z.ZodTypeAny;
 };
 
 type RelationRoutesConfig = {
@@ -102,19 +122,16 @@ export default class SchemaRouter<
   ) {
     super({ prefix: '/' + tableToPathname(schema.table) });
 
-    this.config = deepmerge<typeof this.config, DeepPartial<typeof this.config>>(
-      {
-        disabled: {
-          get: false,
-          post: false,
-          getById: false,
-          patchById: false,
-          deleteById: false,
-        },
-        searchFields: [],
+    const { disabled, ...rest } = config;
+
+    this.config = {
+      ...defaultConfig,
+      disabled: {
+        ...defaultConfig.disabled,
+        ...disabled,
       },
-      config
-    );
+      ...rest,
+    };
 
     if (this.config.middlewares?.length) {
       this.use(...this.config.middlewares);
@@ -131,94 +148,7 @@ export default class SchemaRouter<
       });
     }
 
-    const { disabled, searchFields, idLength } = this.config;
-
-    if (!disabled.get) {
-      this.get(
-        '/',
-        koaPagination(),
-        koaGuard({
-          query: z.object({ q: z.string().optional() }),
-          response: schema.guard.array(),
-          status: [200],
-        }),
-        async (ctx, next) => {
-          const search = parseSearchOptions(searchFields, ctx.guard.query);
-          const { limit, offset } = ctx.pagination;
-          const [count, entities] = await queries.findAll(limit, offset, search);
-
-          ctx.pagination.totalCount = count;
-          ctx.body = entities;
-          return next();
-        }
-      );
-    }
-
-    if (!disabled.post) {
-      this.post(
-        '/',
-        koaGuard({
-          body: schema.createGuard.omit({ id: true }),
-          response: schema.guard,
-          status: [201], // TODO: 409/422 for conflict?
-        }),
-        async (ctx, next) => {
-          // eslint-disable-next-line no-restricted-syntax -- `.omit()` doesn't play well with generics
-          ctx.body = await queries.insert({
-            id: generateStandardId(idLength),
-            ...ctx.guard.body,
-          } as CreateSchema);
-          ctx.status = 201;
-          return next();
-        }
-      );
-    }
-
-    if (!disabled.getById) {
-      this.get(
-        '/:id',
-        koaGuard({
-          params: z.object({ id: z.string().min(1) }),
-          response: schema.guard,
-          status: [200, 404],
-        }),
-        async (ctx, next) => {
-          ctx.body = await queries.findById(ctx.guard.params.id);
-          return next();
-        }
-      );
-    }
-
-    if (!disabled.patchById) {
-      this.patch(
-        '/:id',
-        koaGuard({
-          params: z.object({ id: z.string().min(1) }),
-          body: schema.updateGuard,
-          response: schema.guard,
-          status: [200, 404], // TODO: 409/422 for conflict?
-        }),
-        async (ctx, next) => {
-          ctx.body = await queries.updateById(ctx.guard.params.id, ctx.guard.body);
-          return next();
-        }
-      );
-    }
-
-    if (!disabled.deleteById) {
-      this.delete(
-        '/:id',
-        koaGuard({
-          params: z.object({ id: z.string().min(1) }),
-          status: [204, 404],
-        }),
-        async (ctx, next) => {
-          await queries.deleteById(ctx.guard.params.id);
-          ctx.status = 204;
-          return next();
-        }
-      );
-    }
+    this.#addRoutes();
   }
 
   /**
@@ -362,4 +292,96 @@ export default class SchemaRouter<
       }
     );
   }
+
+  #addRoutes() {
+    const { queries, schema, config } = this;
+    const { disabled, searchFields, idLength, entityGuard } = config;
+
+    if (!disabled.get) {
+      this.get(
+        '/',
+        koaPagination(),
+        koaGuard({
+          query: z.object({ q: z.string().optional() }),
+          response: (entityGuard ?? schema.guard).array(),
+          status: [200],
+        }),
+        async (ctx, next) => {
+          const search = parseSearchOptions(searchFields, ctx.guard.query);
+          const { limit, offset } = ctx.pagination;
+          const [count, entities] = await queries.findAll(limit, offset, search);
+
+          ctx.pagination.totalCount = count;
+          ctx.body = entities;
+          return next();
+        }
+      );
+    }
+
+    if (!disabled.post) {
+      this.post(
+        '/',
+        koaGuard({
+          body: schema.createGuard.omit({ id: true }),
+          response: entityGuard ?? schema.guard,
+          status: [201], // TODO: 409/422 for conflict?
+        }),
+        async (ctx, next) => {
+          // eslint-disable-next-line no-restricted-syntax -- `.omit()` doesn't play well with generics
+          ctx.body = await queries.insert({
+            id: generateStandardId(idLength),
+            ...ctx.guard.body,
+          } as CreateSchema);
+          ctx.status = 201;
+          return next();
+        }
+      );
+    }
+
+    if (!disabled.getById) {
+      this.get(
+        '/:id',
+        koaGuard({
+          params: z.object({ id: z.string().min(1) }),
+          response: entityGuard ?? schema.guard,
+          status: [200, 404],
+        }),
+        async (ctx, next) => {
+          ctx.body = await queries.findById(ctx.guard.params.id);
+          return next();
+        }
+      );
+    }
+
+    if (!disabled.patchById) {
+      this.patch(
+        '/:id',
+        koaGuard({
+          params: z.object({ id: z.string().min(1) }),
+          body: schema.updateGuard,
+          response: entityGuard ?? schema.guard,
+          status: [200, 404], // TODO: 409/422 for conflict?
+        }),
+        async (ctx, next) => {
+          ctx.body = await queries.updateById(ctx.guard.params.id, ctx.guard.body);
+          return next();
+        }
+      );
+    }
+
+    if (!disabled.deleteById) {
+      this.delete(
+        '/:id',
+        koaGuard({
+          params: z.object({ id: z.string().min(1) }),
+          status: [204, 404],
+        }),
+        async (ctx, next) => {
+          await queries.deleteById(ctx.guard.params.id);
+          ctx.status = 204;
+          return next();
+        }
+      );
+    }
+  }
 }

packages/integration-tests/src/api/organization-invitation.ts

@@ -0,0 +1,32 @@
+import { type OrganizationInvitationEntity } from '@logto/schemas';
+
+import { authedAdminApi } from './api.js';
+import { ApiFactory } from './factory.js';
+
+export type PostOrganizationInvitationData = {
+  inviterId?: string;
+  invitee: string;
+  organizationId: string;
+  expiresAt: number;
+  organizationRoleIds?: string[];
+};
+
+export class OrganizationInvitationApi extends ApiFactory<
+  OrganizationInvitationEntity,
+  PostOrganizationInvitationData
+> {
+  constructor() {
+    super('organization-invitations');
+  }
+
+  override async create(data: PostOrganizationInvitationData, skipEmail = false) {
+    return authedAdminApi
+      .post(this.path, {
+        searchParams: {
+          skipEmail: skipEmail.toString(),
+        },
+        json: data,
+      })
+      .json<OrganizationInvitationEntity>();
+  }
+}

packages/integration-tests/src/helpers/organization.ts

@@ -3,9 +3,14 @@ import {
   type OrganizationRole,
   type Organization,
   type OrganizationRoleWithScopes,
+  type OrganizationInvitationEntity,
 } from '@logto/schemas';
 import { trySafe } from '@silverhand/essentials';
 
+import {
+  OrganizationInvitationApi,
+  type PostOrganizationInvitationData,
+} from '#src/api/organization-invitation.js';
 import {
   type CreateOrganizationRolePostData,
   OrganizationRoleApi,
@@ -14,6 +19,35 @@ import { OrganizationScopeApi } from '#src/api/organization-scope.js';
 import { OrganizationApi } from '#src/api/organization.js';
 
 /* eslint-disable @silverhand/fp/no-mutating-methods */
+export class OrganizationInvitationApiTest extends OrganizationInvitationApi {
+  #invitations: OrganizationInvitationEntity[] = [];
+
+  get invitations(): OrganizationInvitationEntity[] {
+    return this.#invitations;
+  }
+
+  override async create(
+    data: PostOrganizationInvitationData,
+    skipEmail = false
+  ): Promise<OrganizationInvitationEntity> {
+    const created = await super.create(data, skipEmail);
+    this.invitations.push(created);
+    return created;
+  }
+
+  /**
+   * Delete all created invitations. This method will ignore errors when deleting invitations to
+   * avoid error when they are deleted by other tests.
+   */
+  async cleanUp(): Promise<void> {
+    // Use `trySafe` to avoid error when invitation is deleted by other tests.
+    await Promise.all(
+      this.invitations.map(async (invitation) => trySafe(this.delete(invitation.id)))
+    );
+    this.#invitations = [];
+  }
+}
+
 /**
  * A help class that records the created organization roles, and provides a `cleanUp` method to
  * delete them.

packages/integration-tests/src/tests/api/organization/organization-invitation.test.ts

@@ -0,0 +1,151 @@
+import assert from 'node:assert';
+
+import { generateStandardId } from '@logto/shared';
+import { HTTPError } from 'got';
+
+import { OrganizationApiTest, OrganizationInvitationApiTest } from '#src/helpers/organization.js';
+
+const randomId = () => generateStandardId(4);
+
+const expectErrorResponse = (error: unknown, status: number, code: string) => {
+  assert(error instanceof HTTPError);
+  const { statusCode, body: raw } = error.response;
+  const body: unknown = JSON.parse(String(raw));
+  expect(statusCode).toBe(status);
+  expect(body).toMatchObject({ code });
+};
+
+describe('organization invitations', () => {
+  const invitationApi = new OrganizationInvitationApiTest();
+  const organizationApi = new OrganizationApiTest();
+
+  afterEach(async () => {
+    await Promise.all([
+      organizationApi.cleanUp(),
+      organizationApi.roleApi.cleanUp(),
+      invitationApi.cleanUp(),
+    ]);
+  });
+
+  it('should be able to create an invitation without sending email', async () => {
+    const organization = await organizationApi.create({ name: 'test' });
+    await invitationApi.create(
+      {
+        organizationId: organization.id,
+        invitee: `${randomId()}@example.com`,
+        expiresAt: Date.now() + 1_000_000,
+      },
+      true
+    );
+  });
+
+  it('should not be able to create invitations with the same email', async () => {
+    const organization = await organizationApi.create({ name: 'test' });
+    const email = `${randomId()}@example.com`;
+    await invitationApi.create(
+      {
+        organizationId: organization.id,
+        invitee: email,
+        expiresAt: Date.now() + 1_000_000,
+      },
+      true
+    );
+    const error = await invitationApi
+      .create({
+        organizationId: organization.id,
+        invitee: email,
+        expiresAt: Date.now() + 1_000_000,
+      })
+      .catch((error: unknown) => error);
+
+    expectErrorResponse(error, 422, 'entity.unique_integrity_violation');
+  });
+
+  it('should be able to create invitations with the same email for different organizations', async () => {
+    const [organization1, organization2] = await Promise.all([
+      organizationApi.create({ name: 'test1' }),
+      organizationApi.create({ name: 'test2' }),
+    ]);
+    const email = `${randomId()}@example.com`;
+    await Promise.all([
+      invitationApi.create(
+        {
+          organizationId: organization1.id,
+          invitee: email,
+          expiresAt: Date.now() + 1_000_000,
+        },
+        true
+      ),
+      invitationApi.create(
+        {
+          organizationId: organization2.id,
+          invitee: email,
+          expiresAt: Date.now() + 1_000_000,
+        },
+        true
+      ),
+    ]);
+  });
+
+  it('should not be able to create invitations with a past expiration date', async () => {
+    const organization = await organizationApi.create({ name: 'test' });
+    const error = await invitationApi
+      .create({
+        organizationId: organization.id,
+        invitee: `${randomId()}@example.com`,
+        expiresAt: Date.now() - 1_000_000,
+      })
+      .catch((error: unknown) => error);
+
+    expectErrorResponse(error, 400, 'request.invalid_input');
+  });
+
+  it('should not be able to create invitations with an invalid email', async () => {
+    const organization = await organizationApi.create({ name: 'test' });
+    const error = await invitationApi
+      .create({
+        organizationId: organization.id,
+        invitee: 'invalid',
+        expiresAt: Date.now() + 1_000_000,
+      })
+      .catch((error: unknown) => error);
+
+    expectErrorResponse(error, 400, 'guard.invalid_input');
+  });
+
+  it('should not be able to create invitations with an invalid organization id', async () => {
+    const error = await invitationApi
+      .create({
+        organizationId: 'invalid',
+        invitee: `${randomId()}@example.com`,
+        expiresAt: Date.now() + 1_000_000,
+      })
+      .catch((error: unknown) => error);
+
+    expectErrorResponse(error, 422, 'entity.relation_foreign_key_not_found');
+  });
+
+  it('should be able to create invitations with organization role ids', async () => {
+    const organization = await organizationApi.create({ name: 'test' });
+    const role = await organizationApi.roleApi.create({ name: 'test' });
+    const invitation = await invitationApi.create(
+      {
+        organizationId: organization.id,
+        invitee: `${randomId()}@example.com`,
+        expiresAt: Date.now() + 1_000_000,
+        organizationRoleIds: [role.id],
+      },
+      true
+    );
+    expect(invitation.organizationRoles.map((role) => role.id)).toEqual([role.id]);
+    expect(invitation.magicLink).toBeDefined();
+
+    // Test if get invitation by id works
+    const invitationById = await invitationApi.get(invitation.id);
+    expect(invitationById).toEqual(invitation);
+
+    // Test if get invitations works
+    const invitations = await invitationApi.getList();
+    expect(invitations).toEqual([invitation]);
+  });
+});

packages/schemas/alterations/next-1706449174-update-organization-invitation-column.ts

@@ -0,0 +1,24 @@
+import { sql } from 'slonik';
+
+import type { AlterationScript } from '../lib/types/alteration.js';
+
+const alteration: AlterationScript = {
+  up: async (pool) => {
+    await pool.query(sql`
+      alter table organization_invitation_role_relations
+        rename column invitation_id to organization_invitation_id;
+      alter table organization_invitation_role_relations
+        rename constraint organization_invitation_role_relations_invitation_id_fkey to organization_invitation_role_re_organization_invitation_id_fkey;
+    `);
+  },
+  down: async (pool) => {
+    await pool.query(sql`
+      alter table organization_invitation_role_relations
+        rename column organization_invitation_id to invitation_id;
+        alter table organization_invitation_role_relations
+        rename constraint organization_invitation_role_re_organization_invitation_id_fkey to organization_invitation_role_relations_invitation_id_fkey;
+    `);
+  },
+};
+
+export default alteration;

packages/schemas/tables/organization_invitation_role_relations.sql

@@ -5,10 +5,10 @@ create table organization_invitation_role_relations (
   tenant_id varchar(21) not null
     references tenants (id) on update cascade on delete cascade,
   /** The ID of the invitation. */
-  invitation_id varchar(21) not null
+  organization_invitation_id varchar(21) not null
     references organization_invitations (id) on update cascade on delete cascade,
   /** The ID of the organization role. */
   organization_role_id varchar(21) not null
     references organization_roles (id) on update cascade on delete cascade,
-  primary key (tenant_id, invitation_id, organization_role_id)
+  primary key (tenant_id, organization_invitation_id, organization_role_id)
 );