WEBDIENSTE/appointment-booking-software
1
0
Fork 0
mirror of https://github.com/open-reception/appointment-booking-software.git synced 2026-09-11 19:57:39 +02:00
Code Issues Packages Projects Releases Wiki Activity

fixed error and failing unit tests (#203)

Browse Source
This commit is contained in:
Hendrik
2026-02-26 17:29:49 +01:00
committed by GitHub
parent 513b706877
commit 166373aa85
10 changed files with 191 additions and 109 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/hooks.server.test.ts
+3 -2
View File
@@ -2,6 +2,7 @@
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
import { handle } from "./hooks.server";
import { mockCookies } from "$lib/tests/const";
import { RATE_LIMIT_MAX_REQUESTS } from "./server-hooks/rateLimitHandle";
// Mock the sequence function to avoid the request store issue
vi.mock("@sveltejs/kit/hooks", () => ({
@@ -116,7 +117,7 @@ describe("hooks.server", () => {
const event = createEvent("192.168.1.101"); // Unique IP for this test
// Make multiple requests to exceed rate limit
for (let i = 0; i < 10; i++) {
for (let i = 0; i < RATE_LIMIT_MAX_REQUESTS; i++) {
await handle({ event, resolve: mockResolve });
}
@@ -131,7 +132,7 @@ describe("hooks.server", () => {
const event = createEvent("192.168.1.102"); // Unique IP for this test
// Exceed rate limit
for (let i = 0; i < 11; i++) {
for (let i = 0; i < RATE_LIMIT_MAX_REQUESTS + 1; i++) {
await handle({ event, resolve: mockResolve });
}
src/lib/server/services/__tests__/staff-service.test.ts
+82 -43
View File
@@ -148,20 +148,39 @@ describe("StaffService", () => {
});
describe("deleteStaffMember", () => {
it("should prevent deletion of last staff member", async () => {
it("should propagate validation errors from UserService", async () => {
const { centralDb } = await import("../../db");
const { UserService } = await import("../user-service");
// Mock the non-global admin staff check - only one staff member exists
const mockSelectBuilder = {
from: vi.fn().mockReturnThis(),
where: vi.fn().mockResolvedValue([
{
id: "staff-123", // Only one staff member
},
]),
};
vi.mocked(UserService.deleteUser).mockRejectedValue(
new ValidationError("Cannot delete the last STAFF user for this tenant"),
);
vi.mocked(centralDb.select).mockReturnValue(mockSelectBuilder as any);
const mockTransaction = vi.fn().mockImplementation(async (callback) => {
const tx = {
select: vi.fn().mockReturnValueOnce({
from: vi.fn().mockReturnValue({
where: vi.fn().mockReturnValue({
limit: vi.fn().mockResolvedValue([
{
id: "staff-123",
email: "staff@example.com",
name: "Staff Member",
role: "STAFF",
tenantId: "tenant-123",
},
]),
}),
}),
}),
delete: vi.fn().mockReturnValue({
where: vi.fn().mockResolvedValue({ count: 1 }),
}),
};
return await callback(tx);
});
vi.mocked(centralDb.transaction).mockImplementation(mockTransaction);
await expect(StaffService.deleteStaffMember("tenant-123", "staff-123")).rejects.toThrow(
ValidationError,
@@ -172,21 +191,6 @@ describe("StaffService", () => {
const { centralDb, getTenantDb } = await import("../../db");
const { UserService } = await import("../user-service");
// Mock the non-global admin staff check - multiple staff members exist
const mockSelectBuilder = {
from: vi.fn().mockReturnThis(),
where: vi.fn().mockResolvedValue([
{
id: "staff-123",
},
{
id: "staff-456", // Multiple staff members
},
]),
};
vi.mocked(centralDb.select).mockReturnValue(mockSelectBuilder as any);
// Mock UserService.deleteUser - wichtig: das muss vor der transaction() mock sein
const mockUserDeletionResult = {
success: true,
@@ -253,27 +257,24 @@ describe("StaffService", () => {
it("should throw NotFoundError when staff member not found", async () => {
const { centralDb } = await import("../../db");
// Mock the pre-transaction select queries
const mockSelectBuilder = {
from: vi.fn().mockReturnThis(),
where: vi.fn().mockResolvedValue([
{
id: "staff-456", // Different ID to simulate multiple staff members
},
]),
};
vi.mocked(centralDb.select).mockReturnValue(mockSelectBuilder as any);
const mockTransaction = vi.fn().mockImplementation(async (callback) => {
const tx = {
select: vi.fn().mockReturnValue({
from: vi.fn().mockReturnValue({
where: vi.fn().mockReturnValue({
limit: vi.fn().mockResolvedValue([]), // No user found
select: vi
.fn()
.mockReturnValueOnce({
from: vi.fn().mockReturnValue({
where: vi.fn().mockReturnValue({
limit: vi.fn().mockResolvedValue([]), // No user found
}),
}),
})
.mockReturnValueOnce({
from: vi.fn().mockReturnValue({
where: vi.fn().mockReturnValue({
limit: vi.fn().mockResolvedValue([]), // No invite found
}),
}),
}),
}),
};
return await callback(tx);
});
@@ -284,6 +285,44 @@ describe("StaffService", () => {
NotFoundError,
);
});
it("should delete invited staff member when no user exists", async () => {
const { centralDb } = await import("../../db");
const mockTransaction = vi.fn().mockImplementation(async (callback) => {
const tx = {
select: vi
.fn()
.mockReturnValueOnce({
from: vi.fn().mockReturnValue({
where: vi.fn().mockReturnValue({
limit: vi.fn().mockResolvedValue([]), // No user found
}),
}),
})
.mockReturnValueOnce({
from: vi.fn().mockReturnValue({
where: vi.fn().mockReturnValue({
limit: vi.fn().mockResolvedValue([{ id: "staff-123" }]), // Invite found
}),
}),
}),
delete: vi.fn().mockReturnValue({
where: vi.fn().mockResolvedValue({ count: 1 }),
}),
};
return await callback(tx);
});
vi.mocked(centralDb.transaction).mockImplementation(mockTransaction);
const result = await StaffService.deleteStaffMember("tenant-123", "staff-123");
expect(result.success).toBe(true);
expect(result.deletedUser.id).toBe("staff-123");
expect(result.deletedPasskeysCount).toBe(0);
expect(result.deletedKeySharesCount).toBe(0);
});
});
describe("getStaffPublicKey", () => {
src/lib/server/services/__tests__/user-service.test.ts
+55
View File
@@ -370,6 +370,8 @@ describe("UserService", () => {
name: "Last Admin",
role: "TENANT_ADMIN",
tenantId: "tenant-123",
isActive: true,
confirmationState: "ACCESS_GRANTED",
},
]),
}),
@@ -412,6 +414,8 @@ describe("UserService", () => {
name: "Deleted Admin",
role: "TENANT_ADMIN",
tenantId: "tenant-123",
isActive: true,
confirmationState: "ACCESS_GRANTED",
},
]),
}),
@@ -445,6 +449,57 @@ describe("UserService", () => {
expect(result.deletedPasskeysCount).toBe(1);
expect(result.tenantId).toBe("tenant-123");
});
it("should allow deleting non-ACCESS_GRANTED staff", async () => {
const userId = "018f-a1b2-c3d4-e5f6-789abcdef099";
const mockDeletedUser = {
id: userId,
name: "Pending Staff",
email: "pending@example.com",
role: "STAFF",
};
const mockTransaction = vi.fn().mockImplementation(async (callback) => {
const tx = {
select: vi.fn().mockReturnValueOnce({
from: vi.fn().mockReturnValue({
where: vi.fn().mockReturnValue({
limit: vi.fn().mockResolvedValue([
{
id: userId,
email: "pending@example.com",
name: "Pending Staff",
role: "STAFF",
tenantId: "tenant-123",
isActive: true,
confirmationState: "CONFIRMED",
},
]),
}),
}),
}),
delete: vi
.fn()
.mockReturnValueOnce({
where: vi.fn().mockResolvedValue({ count: 0 }),
})
.mockReturnValueOnce({
where: vi.fn().mockReturnValue({
returning: vi.fn().mockResolvedValue([mockDeletedUser]),
}),
}),
};
return await callback(tx);
});
mockCentralDb.transaction.mockImplementation(mockTransaction);
const result = await UserService.deleteUser(userId);
expect(result.success).toBe(true);
expect(result.deletedUser).toEqual(mockDeletedUser);
expect(result.deletedPasskeysCount).toBe(0);
});
});
describe("addPasskey", () => {
src/lib/server/services/staff-service.ts
+36 -46
View File
@@ -185,7 +185,6 @@ export class StaffService {
tenantId: string,
staffId: string,
currentUserId?: string,
confirmationState?: "INVITED" | "CONFIRMED" | "ACCESS_GRANTED",
): Promise<StaffDeletionResult> {
logger.debug("Deleting staff member", { tenantId, staffId, currentUserId });
@@ -194,58 +193,40 @@ export class StaffService {
throw new ValidationError("You cannot delete your own account");
}
// Check if this is the only non-global admin staff member
const nonGlobalAdminStaff = await centralDb
.select({ id: user.id })
.from(user)
.where(
and(
eq(user.tenantId, tenantId),
eq(user.isActive, true),
or(eq(user.role, "TENANT_ADMIN"), eq(user.role, "STAFF")),
eq(user.confirmationState, "ACCESS_GRANTED"), // prevents us from deleting the last staff member with appointment access
),
);
if (nonGlobalAdminStaff.length === 1 && nonGlobalAdminStaff[0].id === staffId) {
throw new ValidationError("Cannot delete the last active non-global admin staff member");
}
try {
// Use transaction to ensure all related data is deleted consistently
const result = await centralDb.transaction(async (tx) => {
// First, verify the user exists and belongs to this tenant
if (confirmationState !== "INVITED") {
const userToDelete = await tx
.select({
id: user.id,
email: user.email,
name: user.name,
role: user.role,
tenantId: user.tenantId,
})
.from(user)
.where(and(eq(user.id, staffId), eq(user.tenantId, tenantId)))
.limit(1);
const userToDelete = await tx
.select({
id: user.id,
email: user.email,
name: user.name,
role: user.role,
tenantId: user.tenantId,
})
.from(user)
.where(and(eq(user.id, staffId), eq(user.tenantId, tenantId)))
.limit(1);
if (userToDelete.length === 0) {
throw new NotFoundError("Staff member not found in this tenant");
}
// Use UserService to delete central database data (user + passkeys) first
// This ensures all validation logic is applied before deleting tenant data
const userDeletionResult = await UserService.deleteUser(staffId, tx);
// Remove old invites of user if any exist
if (userToDelete.length > 0) {
// Remove invites referencing this user before deleting user to avoid FK violations on createdUserId
const deletedInvites = await tx
.delete(userInvite)
.where(eq(userInvite.email, userToDelete[0].email));
.where(
or(
eq(userInvite.createdUserId, staffId),
eq(userInvite.email, userToDelete[0].email),
),
);
logger.debug("Deleted user invites", {
staffId,
tenantId,
deletedCount: deletedInvites.count || 0,
});
// Use UserService to delete central database data (user + passkeys)
const userDeletionResult = await UserService.deleteUser(staffId, tx);
// Delete tenant-specific data (client tunnel key shares) after user deletion succeeds
const tenantDb = await getTenantDb(tenantId);
const keyShareDeletionResult = await tenantDb
@@ -262,7 +243,6 @@ export class StaffService {
deletedKeySharesCount,
});
// Combine results for staff-specific response format
const staffDeletionResult: StaffDeletionResult = {
success: userDeletionResult.success,
deletedUser: userDeletionResult.deletedUser,
@@ -279,7 +259,15 @@ export class StaffService {
});
return staffDeletionResult;
} else {
}
const inviteToDelete = await tx
.select({ id: userInvite.id })
.from(userInvite)
.where(and(eq(userInvite.id, staffId), eq(userInvite.tenantId, tenantId)))
.limit(1);
if (inviteToDelete.length > 0) {
const deletedInvites = await tx.delete(userInvite).where(eq(userInvite.id, staffId));
logger.debug("Deleted user invites", {
staffId,
@@ -299,17 +287,19 @@ export class StaffService {
deletedKeySharesCount: 0,
};
}
throw new NotFoundError("Staff member not found in this tenant");
});
return result;
} catch (error) {
if (error instanceof ValidationError || error instanceof NotFoundError) {
throw error;
}
logger.error("Failed to delete staff member", {
tenantId,
staffId,
error: String(error),
});
if (error instanceof ValidationError || error instanceof NotFoundError) {
throw error;
}
throw new InternalError("Failed to delete staff member");
}
}
src/lib/server/services/tenant-admin-service.ts
+3 -1
View File
@@ -190,7 +190,9 @@ export class TenantAdminService {
// Do not log logo to not spam logs
data: {
...tenant.#tenant,
databaseUrl: redactDbUrl(tenant.#tenant.databaseUrl),
databaseUrl: tenant.#tenant.databaseUrl
? redactDbUrl(tenant.#tenant.databaseUrl)
: "<missing-db-url>",
logo: tenant.#tenant.logo ? "removed-from-log-but-set" : null,
},
});
src/lib/server/services/user-service.ts
+7 -1
View File
@@ -578,6 +578,8 @@ export class UserService {
name: centralSchema.user.name,
role: centralSchema.user.role,
tenantId: centralSchema.user.tenantId,
isActive: centralSchema.user.isActive,
confirmationState: centralSchema.user.confirmationState,
})
.from(centralSchema.user)
.where(eq(centralSchema.user.id, userId))
@@ -591,7 +593,11 @@ export class UserService {
const user = userToDelete[0];
// We cannot delete the last user with access to the tenant's appointments
if (user.role === "TENANT_ADMIN" || user.role === "STAFF") {
if (
(user.role === "TENANT_ADMIN" || user.role === "STAFF") &&
user.isActive === true &&
user.confirmationState === "ACCESS_GRANTED"
) {
const usersCount = await tx
.select({ count: count() })
.from(centralSchema.user)
src/routes/(pages)/dashboard/staff/+page.server.ts
+1 -1
View File
@@ -162,7 +162,7 @@ export const actions: Actions = {
}
const resp = await event.fetch(
`/api/tenants/${event.locals.user?.tenantId}/staff/${form.data.id}?confirmationState=${form.data.confirmationState}`,
`/api/tenants/${event.locals.user?.tenantId}/staff/${form.data.id}`,
{
method: "DELETE",
headers: {
src/routes/api/tenants/[id]/appointments/[appointmentId]/confirm/__tests__/confirm-api.test.ts
+1 -1
View File
@@ -78,7 +78,7 @@ describe("Appointment Confirm API Route", () => {
expect(mockAppointmentService.confirmAppointment).toHaveBeenCalledWith(
mockAppointmentId,
undefined,
"de",
"en",
);
});
src/routes/api/tenants/[id]/staff/[staffId]/+server.ts
+2 -13
View File
@@ -243,16 +243,10 @@ registerOpenAPIRoute("/tenants/{id}/staff/{staffId}", "PUT", {
},
});
export const DELETE: RequestHandler = async ({ request, params, locals }) => {
export const DELETE: RequestHandler = async ({ params, locals }) => {
const log = logger.setContext("API");
const tenantId = params.id;
const staffId = params.staffId;
const { searchParams } = new URL(request.url);
const confirmationState = searchParams.get("confirmationState") as
| "INVITED"
| "CONFIRMED"
| "ACCESS_GRANTED"
| undefined;
if (!tenantId) {
throw new ValidationError(ERRORS.TENANTS.NO_TENANT_ID);
@@ -266,12 +260,7 @@ export const DELETE: RequestHandler = async ({ request, params, locals }) => {
checkPermission(locals, tenantId, true);
try {
const result = await StaffService.deleteStaffMember(
tenantId,
staffId,
locals.user?.id,
confirmationState,
);
const result = await StaffService.deleteStaffMember(tenantId, staffId, locals.user?.id);
return json(result);
} catch (error) {
src/server-hooks/rateLimitHandle.ts
+1 -1
View File
@@ -6,7 +6,7 @@ const rateLimitStore = new Map<string, { count: number; resetTime: number }>();
/** Rate limiting window duration in milliseconds */
const RATE_LIMIT_WINDOW = 2000; // ms
/** Maximum requests allowed per rate limiting window */
const RATE_LIMIT_MAX_REQUESTS = 20;
export const RATE_LIMIT_MAX_REQUESTS = 20;
/**
* Extracts the client IP address from request headers