fix(booking): use the standard visibility rule in the booking pickers

apps/webapp/app/modules/booking/service.server.load-bookings-data.test.ts

@@ -0,0 +1,108 @@
+/**
+ * Visibility regression tests for `loadBookingsData`.
+ *
+ * This is the loader that seeds the "Add to existing booking" pickers. It has
+ * to apply the SAME booking-visibility rule as the search endpoint those
+ * pickers switch to once the user types (`/api/model-filters`), or the list
+ * silently changes the moment someone types into the search box.
+ *
+ * The rule is the standard one: SELF_SERVICE / BASE users see only bookings
+ * they are custodian of, unless the workspace has switched
+ * `selfServiceCanSeeBookings` / `baseUserCanSeeBookings` on. `requirePermission`
+ * resolves that into `canSeeAllBookings`, which is what this loader now takes.
+ * It previously gated on the role alone, so the workspace override never
+ * reached these two dialogs.
+ *
+ * Asserts on the `where` handed to Prisma, because the `where` is the boundary.
+ *
+ * @see {@link file://./service.server.ts} — `loadBookingsData`
+ * @see {@link file://./../../routes/api+/model-filters.ts}
+ */
+import { db } from "~/database/db.server";
+import { loadBookingsData } from "./service.server";
+
+// @vitest-environment node
+
+// why: the subject is the `where` the loader ends up building, not what a
+// database returns. `count` is mocked because `getBookings` issues it alongside
+// `findMany`; `teamMember.findMany` because `resolveCustodianScope` reads it.
+vitest.mock("~/database/db.server", () => ({
+  db: {
+    booking: {
+      findMany: vitest.fn().mockResolvedValue([]),
+      count: vitest.fn().mockResolvedValue(0),
+    },
+    teamMember: {
+      findMany: vitest.fn().mockResolvedValue([{ id: "tm-1" }]),
+    },
+  },
+}));
+
+const findManyMock = db.booking.findMany as unknown as ReturnType<
+  typeof vitest.fn
+>;
+
+const ORGANIZATION_ID = "org-1";
+const USER_ID = "user-1";
+
+/** The restriction `resolveCustodianScope` produces for our fixture user. */
+const CUSTODIAN_RESTRICTION = {
+  OR: [
+    { custodianUserId: USER_ID },
+    { custodianTeamMemberId: { in: ["tm-1"] } },
+  ],
+};
+
+/**
+ * Runs the loader and returns the `where` Prisma was asked for.
+ *
+ * @param canSeeAllBookings - The resolved visibility flag under test.
+ * @returns The Prisma `where` from the resulting booking query.
+ */
+async function whereFor(canSeeAllBookings: boolean) {
+  findManyMock.mockClear();
+
+  await loadBookingsData({
+    request: new Request(
+      "http://localhost/assets/a1/overview/add-to-existing-booking"
+    ),
+    organizationId: ORGANIZATION_ID,
+    userId: USER_ID,
+    canSeeAllBookings,
+    ids: ["a1"],
+  });
+
+  return findManyMock.mock.calls.at(-1)?.[0]?.where;
+}
+
+describe("loadBookingsData booking visibility", () => {
+  it("restricts to the caller's own bookings when they may not see all", async () => {
+    const where = await whereFor(false);
+
+    expect(where.AND).toEqual(
+      expect.arrayContaining([expect.objectContaining(CUSTODIAN_RESTRICTION)])
+    );
+  });
+
+  it("does not restrict when the caller may see all bookings", async () => {
+    const where = await whereFor(true);
+
+    const restrictions = (where.AND ?? []).filter(
+      (clause: Record<string, unknown>) =>
+        JSON.stringify(clause).includes("custodianTeamMemberId") ||
+        JSON.stringify(clause).includes("custodianUserId")
+    );
+
+    expect(restrictions).toEqual([]);
+  });
+
+  it("never drops the draft-visibility clause, whichever way the flag goes", async () => {
+    for (const flag of [true, false]) {
+      const where = await whereFor(flag);
+      const serialized = JSON.stringify(where.AND ?? []);
+
+      expect(serialized).toContain("DRAFT");
+      expect(serialized).toContain("creatorId");
+    }
+  });
+});

apps/webapp/app/modules/booking/service.server.ts

@@ -2272,7 +2272,7 @@
   bookingFound,
   userId,
   effectiveStatus,
   effectiveBooking,
   effectiveTo,
   hints,
   organizationId,
@@ -2303,7 +2303,7 @@
 
   /** Calculate the time difference between the booking.to and the current time */
   const { hours } = calcTimeDifference(effectiveTo!, new Date());
   const lessThanOneHourToCheckin = hours < 1;
 
   /** We cancel just in case there is something pending */
   await cancelScheduler(bookingFound);
@@ -12622,13 +12622,19 @@
   request,
   organizationId,
   userId,
-  isSelfServiceOrBase,
+  canSeeAllBookings,
   ids,
 }: {
   request: Request;
   organizationId: string;
   userId: string;
-  isSelfServiceOrBase: boolean;
+  /**
+   * Standard booking visibility, from `requirePermission`. Gating on the role
+   * alone ignored the workspace's `selfServiceCanSeeBookings` /
+   * `baseUserCanSeeBookings` overrides, so these pickers stayed restricted even
+   * when the workspace had switched the setting on.
+   */
+  canSeeAllBookings: boolean;
   ids?: string[];
 }): Promise<BookingLoaderResponse> {
   // Get search parameters and pagination settings
@@ -12639,10 +12645,10 @@
   // Fetch bookings with filters. Includes ONGOING/OVERDUE so assets/kits can be
   // added to active bookings (they stay AVAILABLE — progressive checkout), not
   // just to not-yet-started DRAFT/RESERVED ones.
-  // Self-service / base users may only work with their own bookings — resolve
-  // the full scope (user link + every team-member link) so legacy rows aren't
-  // hidden here while showing on the index.
-  const custodianScope = isSelfServiceOrBase
+  // Restricted users may only work with their own bookings — resolve the full
+  // scope (user link + every team-member link) so legacy rows aren't hidden
+  // here while showing on the index.
+  const custodianScope = !canSeeAllBookings
     ? await resolveCustodianScope({ userId, organizationId })
     : undefined;
 

apps/webapp/app/routes/_layout+/assets.$assetId.overview.add-to-existing-booking.tsx

@@ -62,7 +62,7 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) {
   });
 
   try {
-    const { organizationId, isSelfServiceOrBase } = await requirePermission({
+    const { organizationId, canSeeAllBookings } = await requirePermission({
       userId: authSession?.userId,
       request,
       entity: PermissionEntity.booking,
@@ -77,7 +77,7 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) {
         request,
         organizationId,
         userId: authSession?.userId,
-        isSelfServiceOrBase,
+        canSeeAllBookings,
         ids: assetId ? [assetId] : undefined,
       }),
       db.asset.findFirst({
@@ -286,10 +286,6 @@ export default function ExistingBooking() {
               // `loadBookingsData` seeds the list with — otherwise searching
               // returns bookings this dialog then refuses to render.
               status: ADDABLE_BOOKING_STATUSES.join(","),
-              // Keep the typed list inside the same custodian scope
-              // `loadBookingsData` seeds it with, so SELF_SERVICE / BASE users
-              // are not offered bookings that submit would then reject.
-              scopeToCustodian: true,
             }}
             fieldName="bookingId"
             contentLabel="Existing Bookings"

apps/webapp/app/routes/_layout+/kits.$kitId.assets.add-to-existing-booking.tsx

@@ -68,7 +68,7 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) {
   });
 
   try {
-    const { organizationId, isSelfServiceOrBase } = await requirePermission({
+    const { organizationId, canSeeAllBookings } = await requirePermission({
       userId: authSession?.userId,
       request,
       entity: PermissionEntity.booking,
@@ -79,7 +79,7 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) {
       request,
       organizationId,
       userId: authSession?.userId,
-      isSelfServiceOrBase,
+      canSeeAllBookings,
       ids: kitId ? [kitId] : undefined,
     });
 
@@ -303,10 +303,6 @@ export default function ExistingBooking() {
               // `loadBookingsData` seeds the list with — otherwise searching
               // returns bookings this dialog then refuses to render.
               status: ADDABLE_BOOKING_STATUSES.join(","),
-              // Keep the typed list inside the same custodian scope
-              // `loadBookingsData` seeds it with, so SELF_SERVICE / BASE users
-              // are not offered bookings that submit would then reject.
-              scopeToCustodian: true,
             }}
             fieldName="bookingId"
             contentLabel=" Existing Bookings"

apps/webapp/app/routes/api+/model-filters.test.server.ts

@@ -133,11 +133,20 @@ async function readFilters(response: Response) {
 /** The clause every booking read path AND-s in — drafts are creator-only. */
 const DRAFT_VISIBILITY = clause.buildDraftVisibility("user-1");
 
-/** Points the session at a workspace where the caller holds `role`. */
-function setRole(role: OrganizationRoles) {
+/**
+ * Points the session at a workspace where the caller holds `role`.
+ *
+ * @param role - Caller's role in the workspace.
+ * @param canSeeAllOverride - Workspace override settings, both off by default.
+ */
+function setRole(role: OrganizationRoles, canSeeAllOverride = false) {
   orgMocks.getSelectedOrganization.mockResolvedValue({
     organizationId: ORG_ID,
     userOrganizations: [{ organization: { id: ORG_ID }, roles: [role] }],
+    currentOrganization: {
+      selfServiceCanSeeBookings: canSeeAllOverride,
+      baseUserCanSeeBookings: canSeeAllOverride,
+    },
   });
 }
 
@@ -267,54 +276,45 @@ describe("GET /api/model-filters", () => {
     });
   });
 
-  describe("scopeToCustodian", () => {
+  describe("booking visibility (standard rule)", () => {
     beforeEach(() => {
       dbMocks.dynamicFindMany.mockResolvedValue([]);
-      bookingMocks.resolveCustodianScope.mockResolvedValue({
-        userId: "user-1",
-        teamMemberIds: ["tm-1"],
-      });
     });
 
     it.each([OrganizationRoles.SELF_SERVICE, OrganizationRoles.BASE])(
-      "restricts %s callers that opt in to bookings they are custodian of",
+      "restricts %s users to their own bookings when the setting is off",
       async (role) => {
-        setRole(role);
+        setRole(role, false);
 
-        await loader(
-          buildArgs("name=booking&queryKey=name&scopeToCustodian=true")
-        );
+        await loader(buildArgs("name=booking&queryKey=name&queryValue=x"));
 
         expect(lastWhere().AND).toEqual([
           DRAFT_VISIBILITY,
-          {
-            OR: [
-              { custodianUserId: "user-1" },
-              { custodianTeamMemberId: { in: ["tm-1"] } },
-            ],
-          },
+          { custodianUserId: "user-1" },
         ]);
       }
     );
 
-    it("is a no-op for admins and owners", async () => {
-      setRole(OrganizationRoles.ADMIN);
+    it.each([OrganizationRoles.SELF_SERVICE, OrganizationRoles.BASE])(
+      "lets %s users see all bookings when the workspace enables it",
+      async (role) => {
+        setRole(role, true);
 
-      await loader(
-        buildArgs("name=booking&queryKey=name&scopeToCustodian=true")
-      );
+        await loader(buildArgs("name=booking&queryKey=name&queryValue=x"));
 
-      expect(lastWhere().AND).toEqual([DRAFT_VISIBILITY]);
-      expect(bookingMocks.resolveCustodianScope).not.toHaveBeenCalled();
-    });
+        expect(lastWhere().AND).toEqual([DRAFT_VISIBILITY]);
+      }
+    );
 
-    it("leaves callers that do not opt in unscoped (advanced filter)", async () => {
-      setRole(OrganizationRoles.SELF_SERVICE);
+    it.each([OrganizationRoles.ADMIN, OrganizationRoles.OWNER])(
+      "never restricts %s",
+      async (role) => {
+        setRole(role, false);
 
-      await loader(buildArgs("name=booking&queryKey=name&queryValue=x"));
+        await loader(buildArgs("name=booking&queryKey=name&queryValue=x"));
 
-      expect(lastWhere().AND).toEqual([DRAFT_VISIBILITY]);
-      expect(bookingMocks.resolveCustodianScope).not.toHaveBeenCalled();
-    });
+        expect(lastWhere().AND).toEqual([DRAFT_VISIBILITY]);
+      }
+    );
   });
 });