Merge pull request #70 from open-reception/67-add-tenant-should-return-errors-for-shortname-or-email-already-in-use

67 - Refactored backend errors, added additional guards for tenant cr…
This commit is contained in:
Karl Ludwig Weise
2025-09-07 20:18:19 +02:00
committed by GitHub
5 changed files with 80 additions and 23 deletions
@@ -84,11 +84,17 @@ describe("TenantAdminService", () => {
returning: vi.fn().mockResolvedValue([mockCreatedTenant]),
};
const mockSelectBuilder = {
from: vi.fn().mockReturnThis(),
where: vi.fn().mockResolvedValue([]),
};
const mockDeleteBuilder = {
where: vi.fn().mockResolvedValue({ count: 1 }),
};
mockCentralDb.insert.mockReturnValue(mockInsertBuilder);
mockCentralDb.select.mockReturnValue(mockSelectBuilder);
mockCentralDb.delete.mockReturnValue(mockDeleteBuilder);
mockTenantConfig.create.mockResolvedValue(mockConfig);
mockTenantMigrationService.createAndInitializeTenantDatabase.mockResolvedValue();
@@ -126,11 +132,17 @@ describe("TenantAdminService", () => {
returning: vi.fn().mockResolvedValue([mockCreatedTenant]),
};
const mockSelectBuilder = {
from: vi.fn().mockReturnThis(),
where: vi.fn().mockResolvedValue([]),
};
const mockDeleteBuilder = {
where: vi.fn().mockResolvedValue({ count: 1 }),
};
mockCentralDb.insert.mockReturnValue(mockInsertBuilder);
mockCentralDb.select.mockReturnValue(mockSelectBuilder);
mockCentralDb.delete.mockReturnValue(mockDeleteBuilder);
mockTenantMigrationService.createAndInitializeTenantDatabase.mockRejectedValue(
new Error("Database initialization failed"),
@@ -8,7 +8,7 @@ import { env } from "$env/dynamic/private";
import { eq } from "drizzle-orm";
import logger from "$lib/logger";
import z from "zod/v4";
import { ValidationError, NotFoundError } from "../utils/errors";
import { ValidationError, NotFoundError, ConflictError } from "../utils/errors";
import { sendTenantAdminInviteEmail } from "../email/email-service";
if (!env.DATABASE_URL) throw new Error("DATABASE_URL is not set");
@@ -68,6 +68,26 @@ export class TenantAdminService {
shortName: request.shortName,
});
// Check if tenant can be created without any duplications. Do not create tenant if:
// - short name already exists
// - invited tenant admin email already exists
const tenantExists = await centralDb
.select()
.from(centralSchema.tenant)
.where(eq(centralSchema.tenant.shortName, request.shortName));
if (tenantExists.length > 0) {
throw new ConflictError("Tenant with shortname already exists");
}
if (request.inviteAdmin) {
const adminExists = await centralDb
.select()
.from(centralSchema.user)
.where(eq(centralSchema.user.email, request.inviteAdmin));
if (adminExists.length > 0) {
throw new ConflictError("Tenant Admin E-Mail Address already exists");
}
}
const configuration = TenantAdminService.getConfigDefaults();
const urlParts = env.DATABASE_URL?.split("/") ?? [];
+40 -8
View File
@@ -1,19 +1,51 @@
export class AuthenticationError extends Error {
constructor(message: string) {
import { json } from "@sveltejs/kit";
export class BackendError extends Error {
constructor(
message: string,
public code: number,
) {
super(message);
}
public toJson = () => json({ message: this.message }, { status: this.code });
}
export class AuthenticationError extends BackendError {
constructor(
message: string,
public code = 403,
) {
super(message, code);
this.name = "AuthenticationError";
}
}
export class ValidationError extends Error {
constructor(message: string) {
super(message);
export class ValidationError extends BackendError {
constructor(
message: string,
public code = 422,
) {
super(message, code);
this.name = "ValidationError";
}
}
export class NotFoundError extends Error {
constructor(message: string) {
super(message);
export class NotFoundError extends BackendError {
constructor(
message: string,
public code = 404,
) {
super(message, code);
this.name = "ValidationError";
}
}
export class ConflictError extends BackendError {
constructor(
message: string,
public code = 409,
) {
super(message, code);
this.name = "ConflictError";
}
}
+3 -3
View File
@@ -1,6 +1,6 @@
import { json } from "@sveltejs/kit";
import { TenantAdminService } from "$lib/server/services/tenant-admin-service";
import { ValidationError } from "$lib/server/utils/errors";
import { BackendError } from "$lib/server/utils/errors";
import type { RequestHandler } from "@sveltejs/kit";
import { registerOpenAPIRoute } from "$lib/server/openapi";
import { db } from "$lib/server/db";
@@ -178,8 +178,8 @@ export const POST: RequestHandler = async ({ request }) => {
} catch (error) {
log.error("Tenant creation error:", JSON.stringify(error || "?"));
if (error instanceof ValidationError) {
return json({ error: error.message }, { status: 400 });
if (error instanceof BackendError) {
return error.toJson();
}
// Handle unique constraint violation (shortName already exists)
+4 -11
View File
@@ -35,15 +35,8 @@ vi.mock("$lib/logger", () => ({
},
}));
// Mock ValidationError
vi.mock("$lib/server/utils/errors", () => ({
ValidationError: class ValidationError extends Error {
constructor(message: string) {
super(message);
this.name = "ValidationError";
}
},
}));
// Don't mock errors - use real ones
// vi.mock("$lib/server/utils/errors");
describe("/api/tenants", () => {
beforeEach(() => {
@@ -173,9 +166,9 @@ describe("/api/tenants", () => {
const data = await response.json();
expect(data).toEqual({
error: "Invalid tenant creation request",
message: "Invalid tenant creation request",
});
expect(response.status).toBe(400);
expect(response.status).toBe(422);
});
it("should handle unique constraint violations", async () => {