feat(org): org-scoped Messages and Requests, and enforce the app/ layering boundary - #1033
Merged
Conversation
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
✅ Deploy Preview for familiarise ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (32)
📝 WalkthroughWalkthroughThe pull request centralizes scheduling utilities and allocation logic, adds organization-scoped messages and requests routes, separates Stream DM channels by funding context, validates Stream search responses, and updates imports and lint rules to enforce shared module boundaries. ChangesScheduling allocation and validation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ADR 19 splits dashboards by the org-ness of the underlying work, but the participant surfaces were never split — they existed only in the personal trees. So an org member's conversations and their slot-allocation requests either vanished or leaked into their B2C dashboard depending on which default the scope hook happened to resolve. This builds the org half. DM channels are now keyed per context --------------------------------------- `getDmChannelId` keyed on the pair alone, which made a channel the RELATIONSHIP rather than the booking: the same two people had one thread however many sessions they booked and whoever funded them, carrying the org tag of whichever booking created it. That thread could not land in the right dashboard, because it belonged in two. personal `dm-<a>-<b>` unchanged, byte for byte org `dmo-<org8>-<pair16>` 29 chars Personal ids are untouched so no existing conversation moves — and there was no room to extend them anyway. Stream caps ids at 64; a seeded cuid pair already produces 54 and the longest live channel measured 61, so appending a 25-char org id was never possible. The org form hashes both halves for that reason. Verified against the live Stream app that zero org-tagged channels existed, so nothing needed migrating. Also adds the length guard that was missing entirely: `channelIdSchema` asserted only `min(1)`, and two users on 36-char uuid ids — 17 accounts have them — would have produced a 76-char id that Stream rejects silently. Messages ---------- `/dashboard/organization/[orgId]/messages`. Nothing in it filters by org: `useOrgScope` is route-pinned under the org tree, so living at this path scopes the query to that org. A member of several orgs gets one clean inbox per org with no picker to set and nothing to get wrong — which was the merged inbox's real failure, since a conversation carries no visible org label. The personal inbox now pins to B2C explicitly (`defaultForOrgMember: "personal"`, a new option) instead of inheriting `first-org`, which hid a member's B2C threads behind whichever org happened to be first and hid a second org's entirely. Operators cannot read any of this. There is no org-wide chat query behind the page and Stream only returns channels the viewer belongs to; ADR 20 stands. Requests ---------- `/dashboard/organization/[orgId]/requests`, and this one had a cost. Slot allocation existed ONLY in the consultant tree — there is no allocation surface anywhere under `app/dashboard/organization/` — and it fetches the bookings endpoints without `orgScope`, which makes them drop org-funded rows. An org-sponsored subscription was therefore paid for and never scheduled: the request existed and no surface in the product would show it. Gated on the same pair as Compensation (`myArrangement.read` + `canHost`), with the page re-checking the membership's own consultantProfileId, so a mismatch degrades to a redirect rather than a visible tab that refuses to load. Dependency direction ---------------------- Reusing the allocation UI meant `components/` importing from `app/dashboard/consultant/[consultantId]/(features)/shared/` — through a dynamic route segment, and backwards: `app/` is the routing layer and should depend on `lib`, `components` and `hooks`, never the reverse. That folder was already shared in intent and merely parked in the wrong place, so it moves to where it belongs — `components/scheduling/`, `hooks/scheduling/`, `lib/scheduling/` — along with the two type modules `lib/dashboard-queries.ts` was reaching into route folders for. Every inversion this branch would have introduced is gone, and two that predated it as well. Four pre-existing ones remain and are untouched: two chat components importing API response types, and `lib/data/explore-programs.ts` importing a helper from `app/explore`. The last is a real inversion worth its own change. Also bumps `target` from es5 to es2017, the Next.js default. Next transpiles to its own browserslist regardless, so es5 only ever constrained the type checker — it is deprecated in TS 7.0 and was what forced iterator spread to be rewritten as `Array.from`. `baseUrl` stays: ~40 files import via bare specifiers and resolve only through it, so removing it is a separate mechanical sweep. Part of #1021 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reusing the slot-allocation UI in the org tree meant `components/` importing from `app/dashboard/consultant/[consultantId]/(features)/shared/` — backwards, and through a dynamic route segment. Fixing that one import surfaced that the pattern was already established, so this closes the class and adds the rule that stops it coming back. Moved out of app/ ------------------- `lib/data/explore-programs.ts` imported live FUNCTIONS from `app/explore/programs/utils` — the only remaining runtime dependency pointing the wrong way. Nothing in that module is routing: pagination constants, program shapes, an image-URL builder. It is now `lib/explore/programs.ts`. Repointed the 19 real importers by RESOLVED PATH rather than by matching `../utils`, after a first pass caught `app/explore/experts/`, which has its own utils module and was briefly rewritten to import the wrong one. Response shapes derive from Zod --------------------------------- `AppointmentSearchResult` and `ConsulteeSearchResult` were `export type` declarations inside route handlers, imported by `components/chat/*`. They move to `schemas/stream-search.ts` as Zod schemas, and both handlers now PARSE their results against them on the way out. That is the part worth having: the components derive their types from the same definitions the handlers validate against, so the two agree by construction rather than by one asserting a shape the other hopes is true. A field renamed in a handler fails at the boundary instead of arriving as `undefined` in a dropdown — the same drift that put ₹NaN on two money tables in #1029. No re-export shims. The first pass left `export type { X }` behind in the old locations so existing importers kept working; those are deleted and the importers point at the real definition. It turned out nothing imported `RecordingData` from the recording card at all, so that shim was pure weight. Enforced, not remembered -------------------------- `no-restricted-imports` bans `@/app/*` from lib, components, hooks, types and schemas, with a message that says what to do instead. Verified it fires on a deliberate violation and that the tree is clean under it, so it lands green. This regressed silently more than once before it was enforced, which is fair evidence that review alone was not catching it. Also fixes a `"use client"` directive that an earlier edit in this branch pushed off line 1 — it has to be the first statement or the client boundary is silently lost. Part of #1021 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A `git add -A` on this branch swept 22 files that were never part of it: `screens/` (generated HTML inventory), `prompts/`, and — worst — four `.claude/worktrees/` entries, which are other agents' live checkouts. Those were removed from the two commits rather than left in and reverted, so the PR diff shows only the work. Ignoring them so the mistake is not available to make again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
teetangh
force-pushed
the
feat/org-participant-surfaces
branch
from
July 27, 2026 18:40
93e1e95 to
a698712
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/stream/backfill-channel-org.ts (1)
126-160: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winBackfill target resolution has the same plan-vs-appointment org precedence gap.
Both the
CONSULTATIONandSUBSCRIPTIONcases pass onlyappointment.organizationIdintogetDmChannelId, without consultingconsultationPlan.organizationId/subscriptionPlan.organizationIdthe waycreateConsultationChannel/createSubscriptionChanneldo. For org-hosted-plan appointments that aren't themselves appointment-org-tagged, this backfill will resolve/target the wrong (personal-scheme) channel id instead of the actual org-scoped one.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/stream/backfill-channel-org.ts` around lines 126 - 160, Update the backfill target-resolution logic for both the CONSULTATION and SUBSCRIPTION cases to load each appointment’s plan organizationId and pass it to getDmChannelId with the same plan-over-appointment precedence used by createConsultationChannel and createSubscriptionChannel. Preserve appointment.organizationId as the fallback when no plan organization is available, and apply the resolved organization consistently when constructing each channelId.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@actions/stream/chat/event-channel.action.ts`:
- Around line 710-724: Update bookingOrgId to prefer the relevant plan’s
organizationId, falling back to the appointment organizationId and then null,
matching createConsultationChannel and createSubscriptionChannel. Extend the
consultant and subscription booking queries used by getDmPairsForUser to
select/include consultationPlan or subscriptionPlan so bookingOrgId receives the
plan context and computes the same organization-scoped channel identity.
In `@app/api/stream/channels/search-appointments/route.ts`:
- Around line 131-138: Update the channelId construction in the consultation and
subscription search-result branches to use the plan-level organizationId first,
falling back to the appointment-level organizationId only when absent. Apply
this precedence in the getDmChannelId calls using
consultation.consultationPlan.organizationId and
subscription.subscriptionPlan.organizationId respectively, without changing the
existing query.
In
`@app/dashboard/consultant/`[consultantId]/(features)/recordings/components/RecordingCard.tsx:
- Line 3: Update the RecordingCard module’s type import so RecordingData remains
re-exported from this module, preserving existing route-local imports while
using the shared definition from types/recording.ts.
In `@app/dashboard/organization/`[orgId]/layout.tsx:
- Around line 242-270: Update the PAGE_LABELS mapping to include entries for the
messages and requests paths, using the corresponding tab names “Messages” and
“Requests” so the breadcrumb builder resolves these routes to title-case labels.
In `@components/dashboard/shared/requests/components/PaymentRequiredBadge.tsx`:
- Around line 29-69: Update both TooltipTrigger elements in PaymentRequiredBadge
so their wrapped triggers are keyboard-focusable, replacing the non-focusable
icon container and ensuring the badge trigger supports keyboard focus. Add an
accessible name to the icon variant, such as “Payment required,” while
preserving the existing visual content and tooltip behavior.
In `@components/dashboard/shared/requests/RequestSlotAllocationTab.tsx`:
- Around line 181-184: Update the dependency array for fetchData in
RequestSlotAllocationTab to include orgScope alongside consultantId, type, and
error. Preserve the existing consultation and subscription URL construction
while ensuring scope changes recreate the callback and use the current
organization scope.
In `@components/dashboard/shared/requests/RequestSlotAllocationTabMini.tsx`:
- Around line 50-88: Update the request-loading flow around the parallel
consultations and subscriptions fetches to detect any non-OK response and
surface an error before processing or calling setRequests. Preserve the existing
mapping behavior when both responses succeed, and do not replace a partial API
failure with an empty or incomplete request list.
In `@eslint.config.mjs`:
- Around line 164-168: Extend the import-restriction rule containing the
`patterns` entry to also reject relative paths targeting the app routing layer,
covering both direct and nested forms such as `../app/*` and `../app/**`. Add or
update ESLint configuration tests to verify that lower-layer files are blocked
from importing `@/app/...` and equivalent `../app/...` paths while preserving
allowed imports.
In `@hooks/useOrgScope.ts`:
- Around line 69-77: Update the documentation for defaultForOrgMember and the
related ADMIN/STAFF handling comment in useOrgScope so it states that an
explicit "personal" value takes precedence for privileged users. Preserve the
existing override behavior and clarify that ADMIN/STAFF default to "all" only
when "personal" is not explicitly selected.
In `@lib/scheduling/allocationAlgorithms.ts`:
- Line 696: Remove the unused hoursPerCall computation from the session
allocation flow and remove the corresponding parameter from selectCallsFromWeek
and its callers, including the code around the referenced lines. Keep block
sizing driven solely by slotsPerCall.
- Around line 560-612: Update the allocation logic around sortedSlots and
consecutive1HourSlots to validate a contiguous run of requiredSlots 30-minute
entries, rather than only Math.ceil(durationHours) entries. Confirm every
emitted slot is present and available before returning the run, and derive the
returned thirtyMinSlots from that verified range without extending beyond it.
Sort a copied availableSlots array so the caller’s input is not mutated.
In `@lib/scheduling/allocationMessages.ts`:
- Around line 129-136: Update notEnoughConsecutive so the duration label uses
singular “hour” when (requiredSlots * 30) / 60 equals 1 and plural “hours”
otherwise, while preserving the existing duration and slot values in the
description.
In `@lib/scheduling/allocationService.ts`:
- Around line 155-234: Extract the shared POST, JSON parsing, response
validation, and error handling from validateConsultationSlots,
validateSubscriptionSlots, validateClassSlots, and validateWebinarSlots into a
private static postValidate helper. Have each public validator preserve its
existing signature while delegating with its URL segment and fallback error
message, and retain the existing callers and validation response behavior.
- Around line 118-137: Update the PATCH response handling in the allocation
request flow and all four validate* methods to preserve response.status even
when the body is empty or non-JSON. Check response.ok before parsing, parse the
body defensively, and return the parsed error when available while retaining the
HTTP status for failed responses such as 409; keep successful data handling
unchanged.
In `@lib/scheduling/calendarUtils.ts`:
- Around line 369-427: Update validateSelectedSlots to accept a
schedulingTimezone parameter and thread it through the consultation validation
path. Pass that timezone to SlotCalculationService.dayKey and
validateDayBasedConsecutiveSlots so same-day and consecutiveness checks use the
event’s scheduling timezone rather than the default.
- Around line 639-683: Replace the global slots.length division and modulo in
the progress calculation with the existing per-day completion logic from
countCompleteCallsInMap in slotSelectionValidation.ts. Derive completedCalls and
incomplete-call warning state from each day’s grouped slots so scattered partial
days do not count as complete calls, while preserving the existing progress text
and warning formatting.
In `@lib/scheduling/slotSelectionValidation.ts`:
- Around line 184-203: Annotate the SlotLimits contract and the subscription
branch around the returned maxSlots value to document that subscriptions use
maxSlots as a call count, while minSlots remains a slot count. Make the unit
distinction explicit for consumers without changing the existing calculation or
behavior.
- Around line 798-826: Update warnOnWeeklyDistribution so its subscription
branch cannot restore weeklyDistributionValid to true after
validateSubscriptionSelection has marked it false; only downgrade an
already-valid result when validateWeeklyDistribution fails, while preserving
existing warnings. Apply the same no-recompute principle to the class branch by
avoiding redundant validation when validateClassSelection has already evaluated
the identical distribution check, and retain the existing invalid warning
behavior.
In `@lib/stream-utils.ts`:
- Around line 65-70: Update the channel ID construction in the surrounding
function to increase the organization digest from 8 to a collision-resistant
length, and lengthen the pair digest within the available identifier limit.
Preserve the existing dmo- prefix and assertFits validation while avoiding
nested template literals when composing the pair input.
- Around line 54-63: Update getDmChannelId so the personal-DM branch does not
call assertFits with an oversized dm-${a}-${b} identifier; detect when the
generated ID exceeds STREAM_CHANNEL_ID_MAX and fall back to the same
fixed-length hashing approach used by the organization branch. Preserve the
existing direct identifier for IDs that fit and keep the organization branch
unchanged.
---
Outside diff comments:
In `@scripts/stream/backfill-channel-org.ts`:
- Around line 126-160: Update the backfill target-resolution logic for both the
CONSULTATION and SUBSCRIPTION cases to load each appointment’s plan
organizationId and pass it to getDmChannelId with the same plan-over-appointment
precedence used by createConsultationChannel and createSubscriptionChannel.
Preserve appointment.organizationId as the fallback when no plan organization is
available, and apply the resolved organization consistently when constructing
each channelId.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 17e33159-a4c9-4794-a388-46fc4316d4d2
📒 Files selected for processing (87)
.gitignore__tests__/booking-algorithm/allocationAlgorithms.test.ts__tests__/booking-algorithm/calendarUtils.test.ts__tests__/booking-algorithm/idempotency-key.test.ts__tests__/booking-algorithm/mode-parity.test.ts__tests__/booking-algorithm/slot-boundary-bucketing.test.ts__tests__/booking-algorithm/toast-queue.test.ts__tests__/dashboard/nav-targets-resolve.test.tsactions/stream/chat/channel.action.tsactions/stream/chat/event-channel.action.tsapp/api/stream/channels/search-appointments/route.tsapp/api/stream/search-consultees/route.tsapp/dashboard/consultant/[consultantId]/(features)/appointments/ConsultantAppointmentsAdapter.tsxapp/dashboard/consultant/[consultantId]/(features)/appointments/components/EventTimingsCalendar.tsxapp/dashboard/consultant/[consultantId]/(features)/appointments/participants/[eventType]/[eventId]/page.tsxapp/dashboard/consultant/[consultantId]/(features)/home/HomeTab.tsxapp/dashboard/consultant/[consultantId]/(features)/planner/components/EventCard.tsxapp/dashboard/consultant/[consultantId]/(features)/planner/components/EventCarousel.tsxapp/dashboard/consultant/[consultantId]/(features)/planner/components/EventManagementDashboard.tsxapp/dashboard/consultant/[consultantId]/(features)/planner/components/EventPlanner.tsxapp/dashboard/consultant/[consultantId]/(features)/planner/components/EventPlannerForClass.tsxapp/dashboard/consultant/[consultantId]/(features)/planner/components/EventPlannerForConsultation.tsxapp/dashboard/consultant/[consultantId]/(features)/planner/components/EventPlannerForSubscription.tsxapp/dashboard/consultant/[consultantId]/(features)/planner/components/EventPlannerForWebinar.tsxapp/dashboard/consultant/[consultantId]/(features)/planner/services/events/class-service.tsapp/dashboard/consultant/[consultantId]/(features)/planner/services/events/webinar-service.tsapp/dashboard/consultant/[consultantId]/(features)/planner/services/planner.tsapp/dashboard/consultant/[consultantId]/(features)/planner/services/plans/consultation-service.tsapp/dashboard/consultant/[consultantId]/(features)/planner/services/plans/subscription-service.tsapp/dashboard/consultant/[consultantId]/(features)/recordings/components/RecordingCard.tsxapp/dashboard/consultant/[consultantId]/(features)/requests/page.tsxapp/dashboard/consultant/[consultantId]/(features)/trials/TrialsTab.tsxapp/dashboard/organization/[orgId]/appointments/MyAppointmentsClient.tsxapp/dashboard/organization/[orgId]/layout.tsxapp/dashboard/organization/[orgId]/messages/MessagesClient.tsxapp/dashboard/organization/[orgId]/messages/page.tsxapp/dashboard/organization/[orgId]/requests/RequestsClient.tsxapp/dashboard/organization/[orgId]/requests/page.tsxapp/explore/programs/ProgramsInteractiveContent.tsxapp/explore/programs/components/AdvancedFilters.tsxapp/explore/programs/components/CategoryGrid.tsxapp/explore/programs/components/FeaturedCarousel.tsxapp/explore/programs/components/ProgramCard.tsxapp/explore/programs/components/ProgramResults.tsxapp/explore/programs/components/ProgramRow.tsxapp/explore/programs/components/ProgramTabs.tsxapp/explore/programs/components/StaticTopRows.tsxapp/explore/programs/hooks/_helpers.tsapp/explore/programs/hooks/useCuratedPrograms.tsapp/explore/programs/hooks/useProgramFilterChips.tsapp/explore/programs/hooks/usePrograms.tsapp/explore/programs/hooks/useProgramsFilters.tsapp/explore/programs/hooks/useTopicsWithCount.tsapp/explore/programs/plans/classes/[classPlanId]/components/ClassDetails.tsxapp/explore/programs/plans/classes/[classPlanId]/components/ClientClassRegistration.tsxapp/explore/programs/plans/classes/[classPlanId]/page.tsxapp/explore/programs/plans/webinars/[webinarPlanId]/components/WebinarDetails.tsxcomponents/chat/AddMembersDialog.tsxcomponents/chat/ChannelSearch.tsxcomponents/chat/ChatSidebar.tsxcomponents/dashboard/shared/requests/RequestSlotAllocationTab.tsxcomponents/dashboard/shared/requests/RequestSlotAllocationTabMini.tsxcomponents/dashboard/shared/requests/components/PaymentRequiredBadge.tsxcomponents/dashboard/shared/requests/components/RequestedSlotsDialog.tsxcomponents/dashboard/shared/requests/types.tscomponents/scheduling/CalendarErrorBoundary.tsxcomponents/scheduling/SafeUnifiedCalendar.tsxcomponents/scheduling/UnifiedCalendar.tsxeslint.config.mjshooks/scheduling/useCalendarData.tshooks/scheduling/useLazyJoinMeeting.tshooks/scheduling/useSlotAllocation.tshooks/useOrgScope.tslib/dashboard-queries.tslib/data/explore-programs.tslib/explore/programs.tslib/scheduling/allocationAlgorithms.tslib/scheduling/allocationMessages.tslib/scheduling/allocationService.tslib/scheduling/calendarUtils.tslib/scheduling/slotSelectionValidation.tslib/stream-utils.tsschemas/stream-search.tsscripts/stream/backfill-channel-org.tstsconfig.jsontypes/planner-events.tstypes/recording.ts
The org appointment card offered exactly one action — Join, and only inside the join window. It named your counterpart and gave you no way to reach them, reschedule, cancel, or hand over a document, for a session the organization paid for. Everything except joining meant switching to the personal dashboard. ADR 19 puts org-funded work in the org dashboard; the actions belong there too. Adds `/dashboard/organization/[orgId]/appointments/[appointmentId]`, reachable from a Details button on each card. Reused, not rebuilt --------------------- The page mounts the same `AppointmentDetailClient` and consultee adapter the B2C tree uses, so reschedule, cancel, report and document upload behave identically in both places rather than being a second implementation that drifts. Only `detailHref` is overridden, so navigating inside the detail view keeps the member in the org context instead of bouncing them to `/dashboard/consultee/...`. `role="consultee"` because this is the ATTENDING side. An EXPERT delivering org sessions works from Requests; the two roles want different actions on the same row, and conflating them behind one page is how the appointments surfaces drifted in the first place. Both ids are bound -------------------- `orgId` and `appointmentId` both come from the URL and neither constrains the other, so membership alone would let any member of any org read any org-funded appointment by pairing their own org id with a foreign appointment id. The page asks three separate questions: is the caller in this org, does the appointment belong to THIS org, and is the caller a party to it. `notFound()` on each — a redirect would confirm the appointment exists to someone who should not know that. Participation rather than `operations.read`, because the page renders documents and offers reschedule and cancel. An operator's view stays the metadata-only list; an OWNER who is not on the session gets a 404 here, per ADR 20. More code out of route folders -------------------------------- The consultee adapter and its four leaf dialogs move to `components/appointments/consultee/`, and `document-utils` to `lib/documents/`. Both were already shared in practice — the consultant adapter was importing the reschedule and cancel dialogs across dashboard trees via a `@/app/dashboard/ consultee/...` path, and three consultant surfaces import document-utils. The lint rule added earlier in this branch is what caught the last one: moving `DocumentUpload` into `components/` made its import of `app/dashboard/shared/utils/document-utils` an error rather than something to notice later. That is the rule doing its job on its author. No cross-tree `app/ → app/` imports remain between the consultant and consultee dashboards either. 6 tests pin the ownership checks. Part of #1021 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/stream/backfill-channel-org.ts (1)
126-160: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winBackfill target resolution has the same plan-vs-appointment org precedence gap.
Both the
CONSULTATIONandSUBSCRIPTIONcases pass onlyappointment.organizationIdintogetDmChannelId, without consultingconsultationPlan.organizationId/subscriptionPlan.organizationIdthe waycreateConsultationChannel/createSubscriptionChanneldo. For org-hosted-plan appointments that aren't themselves appointment-org-tagged, this backfill will resolve/target the wrong (personal-scheme) channel id instead of the actual org-scoped one.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/stream/backfill-channel-org.ts` around lines 126 - 160, Update the backfill target-resolution logic for both the CONSULTATION and SUBSCRIPTION cases to load each appointment’s plan organizationId and pass it to getDmChannelId with the same plan-over-appointment precedence used by createConsultationChannel and createSubscriptionChannel. Preserve appointment.organizationId as the fallback when no plan organization is available, and apply the resolved organization consistently when constructing each channelId.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@actions/stream/chat/event-channel.action.ts`:
- Around line 710-724: Update bookingOrgId to prefer the relevant plan’s
organizationId, falling back to the appointment organizationId and then null,
matching createConsultationChannel and createSubscriptionChannel. Extend the
consultant and subscription booking queries used by getDmPairsForUser to
select/include consultationPlan or subscriptionPlan so bookingOrgId receives the
plan context and computes the same organization-scoped channel identity.
In `@app/api/stream/channels/search-appointments/route.ts`:
- Around line 131-138: Update the channelId construction in the consultation and
subscription search-result branches to use the plan-level organizationId first,
falling back to the appointment-level organizationId only when absent. Apply
this precedence in the getDmChannelId calls using
consultation.consultationPlan.organizationId and
subscription.subscriptionPlan.organizationId respectively, without changing the
existing query.
In
`@app/dashboard/consultant/`[consultantId]/(features)/recordings/components/RecordingCard.tsx:
- Line 3: Update the RecordingCard module’s type import so RecordingData remains
re-exported from this module, preserving existing route-local imports while
using the shared definition from types/recording.ts.
In `@app/dashboard/organization/`[orgId]/layout.tsx:
- Around line 242-270: Update the PAGE_LABELS mapping to include entries for the
messages and requests paths, using the corresponding tab names “Messages” and
“Requests” so the breadcrumb builder resolves these routes to title-case labels.
In `@components/dashboard/shared/requests/components/PaymentRequiredBadge.tsx`:
- Around line 29-69: Update both TooltipTrigger elements in PaymentRequiredBadge
so their wrapped triggers are keyboard-focusable, replacing the non-focusable
icon container and ensuring the badge trigger supports keyboard focus. Add an
accessible name to the icon variant, such as “Payment required,” while
preserving the existing visual content and tooltip behavior.
In `@components/dashboard/shared/requests/RequestSlotAllocationTab.tsx`:
- Around line 181-184: Update the dependency array for fetchData in
RequestSlotAllocationTab to include orgScope alongside consultantId, type, and
error. Preserve the existing consultation and subscription URL construction
while ensuring scope changes recreate the callback and use the current
organization scope.
In `@components/dashboard/shared/requests/RequestSlotAllocationTabMini.tsx`:
- Around line 50-88: Update the request-loading flow around the parallel
consultations and subscriptions fetches to detect any non-OK response and
surface an error before processing or calling setRequests. Preserve the existing
mapping behavior when both responses succeed, and do not replace a partial API
failure with an empty or incomplete request list.
In `@eslint.config.mjs`:
- Around line 164-168: Extend the import-restriction rule containing the
`patterns` entry to also reject relative paths targeting the app routing layer,
covering both direct and nested forms such as `../app/*` and `../app/**`. Add or
update ESLint configuration tests to verify that lower-layer files are blocked
from importing `@/app/...` and equivalent `../app/...` paths while preserving
allowed imports.
In `@hooks/useOrgScope.ts`:
- Around line 69-77: Update the documentation for defaultForOrgMember and the
related ADMIN/STAFF handling comment in useOrgScope so it states that an
explicit "personal" value takes precedence for privileged users. Preserve the
existing override behavior and clarify that ADMIN/STAFF default to "all" only
when "personal" is not explicitly selected.
In `@lib/scheduling/allocationAlgorithms.ts`:
- Line 696: Remove the unused hoursPerCall computation from the session
allocation flow and remove the corresponding parameter from selectCallsFromWeek
and its callers, including the code around the referenced lines. Keep block
sizing driven solely by slotsPerCall.
- Around line 560-612: Update the allocation logic around sortedSlots and
consecutive1HourSlots to validate a contiguous run of requiredSlots 30-minute
entries, rather than only Math.ceil(durationHours) entries. Confirm every
emitted slot is present and available before returning the run, and derive the
returned thirtyMinSlots from that verified range without extending beyond it.
Sort a copied availableSlots array so the caller’s input is not mutated.
In `@lib/scheduling/allocationMessages.ts`:
- Around line 129-136: Update notEnoughConsecutive so the duration label uses
singular “hour” when (requiredSlots * 30) / 60 equals 1 and plural “hours”
otherwise, while preserving the existing duration and slot values in the
description.
In `@lib/scheduling/allocationService.ts`:
- Around line 155-234: Extract the shared POST, JSON parsing, response
validation, and error handling from validateConsultationSlots,
validateSubscriptionSlots, validateClassSlots, and validateWebinarSlots into a
private static postValidate helper. Have each public validator preserve its
existing signature while delegating with its URL segment and fallback error
message, and retain the existing callers and validation response behavior.
- Around line 118-137: Update the PATCH response handling in the allocation
request flow and all four validate* methods to preserve response.status even
when the body is empty or non-JSON. Check response.ok before parsing, parse the
body defensively, and return the parsed error when available while retaining the
HTTP status for failed responses such as 409; keep successful data handling
unchanged.
In `@lib/scheduling/calendarUtils.ts`:
- Around line 369-427: Update validateSelectedSlots to accept a
schedulingTimezone parameter and thread it through the consultation validation
path. Pass that timezone to SlotCalculationService.dayKey and
validateDayBasedConsecutiveSlots so same-day and consecutiveness checks use the
event’s scheduling timezone rather than the default.
- Around line 639-683: Replace the global slots.length division and modulo in
the progress calculation with the existing per-day completion logic from
countCompleteCallsInMap in slotSelectionValidation.ts. Derive completedCalls and
incomplete-call warning state from each day’s grouped slots so scattered partial
days do not count as complete calls, while preserving the existing progress text
and warning formatting.
In `@lib/scheduling/slotSelectionValidation.ts`:
- Around line 184-203: Annotate the SlotLimits contract and the subscription
branch around the returned maxSlots value to document that subscriptions use
maxSlots as a call count, while minSlots remains a slot count. Make the unit
distinction explicit for consumers without changing the existing calculation or
behavior.
- Around line 798-826: Update warnOnWeeklyDistribution so its subscription
branch cannot restore weeklyDistributionValid to true after
validateSubscriptionSelection has marked it false; only downgrade an
already-valid result when validateWeeklyDistribution fails, while preserving
existing warnings. Apply the same no-recompute principle to the class branch by
avoiding redundant validation when validateClassSelection has already evaluated
the identical distribution check, and retain the existing invalid warning
behavior.
In `@lib/stream-utils.ts`:
- Around line 65-70: Update the channel ID construction in the surrounding
function to increase the organization digest from 8 to a collision-resistant
length, and lengthen the pair digest within the available identifier limit.
Preserve the existing dmo- prefix and assertFits validation while avoiding
nested template literals when composing the pair input.
- Around line 54-63: Update getDmChannelId so the personal-DM branch does not
call assertFits with an oversized dm-${a}-${b} identifier; detect when the
generated ID exceeds STREAM_CHANNEL_ID_MAX and fall back to the same
fixed-length hashing approach used by the organization branch. Preserve the
existing direct identifier for IDs that fit and keep the organization branch
unchanged.
---
Outside diff comments:
In `@scripts/stream/backfill-channel-org.ts`:
- Around line 126-160: Update the backfill target-resolution logic for both the
CONSULTATION and SUBSCRIPTION cases to load each appointment’s plan
organizationId and pass it to getDmChannelId with the same plan-over-appointment
precedence used by createConsultationChannel and createSubscriptionChannel.
Preserve appointment.organizationId as the fallback when no plan organization is
available, and apply the resolved organization consistently when constructing
each channelId.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 17e33159-a4c9-4794-a388-46fc4316d4d2
📒 Files selected for processing (87)
.gitignore__tests__/booking-algorithm/allocationAlgorithms.test.ts__tests__/booking-algorithm/calendarUtils.test.ts__tests__/booking-algorithm/idempotency-key.test.ts__tests__/booking-algorithm/mode-parity.test.ts__tests__/booking-algorithm/slot-boundary-bucketing.test.ts__tests__/booking-algorithm/toast-queue.test.ts__tests__/dashboard/nav-targets-resolve.test.tsactions/stream/chat/channel.action.tsactions/stream/chat/event-channel.action.tsapp/api/stream/channels/search-appointments/route.tsapp/api/stream/search-consultees/route.tsapp/dashboard/consultant/[consultantId]/(features)/appointments/ConsultantAppointmentsAdapter.tsxapp/dashboard/consultant/[consultantId]/(features)/appointments/components/EventTimingsCalendar.tsxapp/dashboard/consultant/[consultantId]/(features)/appointments/participants/[eventType]/[eventId]/page.tsxapp/dashboard/consultant/[consultantId]/(features)/home/HomeTab.tsxapp/dashboard/consultant/[consultantId]/(features)/planner/components/EventCard.tsxapp/dashboard/consultant/[consultantId]/(features)/planner/components/EventCarousel.tsxapp/dashboard/consultant/[consultantId]/(features)/planner/components/EventManagementDashboard.tsxapp/dashboard/consultant/[consultantId]/(features)/planner/components/EventPlanner.tsxapp/dashboard/consultant/[consultantId]/(features)/planner/components/EventPlannerForClass.tsxapp/dashboard/consultant/[consultantId]/(features)/planner/components/EventPlannerForConsultation.tsxapp/dashboard/consultant/[consultantId]/(features)/planner/components/EventPlannerForSubscription.tsxapp/dashboard/consultant/[consultantId]/(features)/planner/components/EventPlannerForWebinar.tsxapp/dashboard/consultant/[consultantId]/(features)/planner/services/events/class-service.tsapp/dashboard/consultant/[consultantId]/(features)/planner/services/events/webinar-service.tsapp/dashboard/consultant/[consultantId]/(features)/planner/services/planner.tsapp/dashboard/consultant/[consultantId]/(features)/planner/services/plans/consultation-service.tsapp/dashboard/consultant/[consultantId]/(features)/planner/services/plans/subscription-service.tsapp/dashboard/consultant/[consultantId]/(features)/recordings/components/RecordingCard.tsxapp/dashboard/consultant/[consultantId]/(features)/requests/page.tsxapp/dashboard/consultant/[consultantId]/(features)/trials/TrialsTab.tsxapp/dashboard/organization/[orgId]/appointments/MyAppointmentsClient.tsxapp/dashboard/organization/[orgId]/layout.tsxapp/dashboard/organization/[orgId]/messages/MessagesClient.tsxapp/dashboard/organization/[orgId]/messages/page.tsxapp/dashboard/organization/[orgId]/requests/RequestsClient.tsxapp/dashboard/organization/[orgId]/requests/page.tsxapp/explore/programs/ProgramsInteractiveContent.tsxapp/explore/programs/components/AdvancedFilters.tsxapp/explore/programs/components/CategoryGrid.tsxapp/explore/programs/components/FeaturedCarousel.tsxapp/explore/programs/components/ProgramCard.tsxapp/explore/programs/components/ProgramResults.tsxapp/explore/programs/components/ProgramRow.tsxapp/explore/programs/components/ProgramTabs.tsxapp/explore/programs/components/StaticTopRows.tsxapp/explore/programs/hooks/_helpers.tsapp/explore/programs/hooks/useCuratedPrograms.tsapp/explore/programs/hooks/useProgramFilterChips.tsapp/explore/programs/hooks/usePrograms.tsapp/explore/programs/hooks/useProgramsFilters.tsapp/explore/programs/hooks/useTopicsWithCount.tsapp/explore/programs/plans/classes/[classPlanId]/components/ClassDetails.tsxapp/explore/programs/plans/classes/[classPlanId]/components/ClientClassRegistration.tsxapp/explore/programs/plans/classes/[classPlanId]/page.tsxapp/explore/programs/plans/webinars/[webinarPlanId]/components/WebinarDetails.tsxcomponents/chat/AddMembersDialog.tsxcomponents/chat/ChannelSearch.tsxcomponents/chat/ChatSidebar.tsxcomponents/dashboard/shared/requests/RequestSlotAllocationTab.tsxcomponents/dashboard/shared/requests/RequestSlotAllocationTabMini.tsxcomponents/dashboard/shared/requests/components/PaymentRequiredBadge.tsxcomponents/dashboard/shared/requests/components/RequestedSlotsDialog.tsxcomponents/dashboard/shared/requests/types.tscomponents/scheduling/CalendarErrorBoundary.tsxcomponents/scheduling/SafeUnifiedCalendar.tsxcomponents/scheduling/UnifiedCalendar.tsxeslint.config.mjshooks/scheduling/useCalendarData.tshooks/scheduling/useLazyJoinMeeting.tshooks/scheduling/useSlotAllocation.tshooks/useOrgScope.tslib/dashboard-queries.tslib/data/explore-programs.tslib/explore/programs.tslib/scheduling/allocationAlgorithms.tslib/scheduling/allocationMessages.tslib/scheduling/allocationService.tslib/scheduling/calendarUtils.tslib/scheduling/slotSelectionValidation.tslib/stream-utils.tsschemas/stream-search.tsscripts/stream/backfill-channel-org.tstsconfig.jsontypes/planner-events.tstypes/recording.ts
🛑 Comments failed to post (11)
components/dashboard/shared/requests/components/PaymentRequiredBadge.tsx (1)
29-69: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the tooltip triggers keyboard-accessible.
TooltipTrigger asChildwraps non-focusable elements, and the icon-only state has no accessible name. Use a focusable trigger and give the icon variant anaria-labelsuch as “Payment required”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/dashboard/shared/requests/components/PaymentRequiredBadge.tsx` around lines 29 - 69, Update both TooltipTrigger elements in PaymentRequiredBadge so their wrapped triggers are keyboard-focusable, replacing the non-focusable icon container and ensuring the badge trigger supports keyboard focus. Add an accessible name to the icon variant, such as “Payment required,” while preserving the existing visual content and tooltip behavior.components/dashboard/shared/requests/RequestSlotAllocationTabMini.tsx (1)
50-88: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not render partial API failures as an empty request list.
Non-OK responses are skipped, so a failed subscriptions or consultations request can make pending work disappear without an error. Throw or surface an error when either response is non-OK before calling
setRequests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/dashboard/shared/requests/RequestSlotAllocationTabMini.tsx` around lines 50 - 88, Update the request-loading flow around the parallel consultations and subscriptions fetches to detect any non-OK response and surface an error before processing or calling setRequests. Preserve the existing mapping behavior when both responses succeed, and do not replace a partial API failure with an empty or incomplete request list.lib/scheduling/allocationAlgorithms.ts (2)
560-612: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Consecutiveness is verified over
Math.ceil(durationHours)slots butrequiredSlotsslots are emitted.Everywhere else in this file (
allocateWebinarSlotsLine 635,selectCallsFromWeekLine 788) the input array is treated as 30-minute slots. HereoneHourBlocksNeeded = Math.ceil(durationHours)verifies only that many consecutive entries, then Line 601 fabricatesrequiredSlots(=durationHours / 0.5) slots fromblockStart. For a 1.5h consultation that checks 2 consecutive 30-min slots (1h of availability) and emits 3; for 2h it checks 2 and emits 4. The trailing slots are never confirmed available/unbooked, so the batch can extend into booked time and get rejected server-side.🐛 Verify the full required run
- const oneHourBlocksNeeded = Math.ceil(durationHours); - for (let i = 0; i <= sortedSlots.length - oneHourBlocksNeeded; i++) { - const consecutive1HourSlots: TimeSlot[] = []; + for (let i = 0; i <= sortedSlots.length - requiredSlots; i++) { + const consecutiveSlots: TimeSlot[] = []; let isConsecutive = true; - for (let j = 0; j < oneHourBlocksNeeded; j++) { + for (let j = 0; j < requiredSlots; j++) { const currentSlot = sortedSlots[i + j]; if (!currentSlot) { isConsecutive = false; break; } if (j > 0) { - const prevSlot = consecutive1HourSlots[j - 1]; + const prevSlot = consecutiveSlots[j - 1]; if (currentSlot.startTime.getTime() !== prevSlot.endTime.getTime()) { isConsecutive = false; break; } } - consecutive1HourSlots.push(currentSlot); + consecutiveSlots.push(currentSlot); } - if ( - !isConsecutive || - consecutive1HourSlots.length < oneHourBlocksNeeded - ) { + if (!isConsecutive || consecutiveSlots.length < requiredSlots) { continue; } - // Generate exactly `requiredSlots` 30-minute slots from the contiguous block - const blockStart = consecutive1HourSlots[0].startTime; - const thirtyMinSlots: TimeSlot[] = []; - for (let k = 0; k < requiredSlots; k++) { - const slotStart = new Date(blockStart.getTime() + k * 30 * 60000); - const slotEnd = new Date(slotStart.getTime() + 30 * 60000); - thirtyMinSlots.push({ - ...consecutive1HourSlots[0], - startTime: slotStart, - endTime: slotEnd, - }); - } - - return thirtyMinSlots; + return consecutiveSlots; }Note: Line 560 also sorts
availableSlotsin place, mutating the caller's array.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const sortedSlots = availableSlots.sort( (a, b) => a.startTime.getTime() - b.startTime.getTime(), ); for (let i = 0; i <= sortedSlots.length - requiredSlots; i++) { const consecutiveSlots: TimeSlot[] = []; let isConsecutive = true; for (let j = 0; j < requiredSlots; j++) { const currentSlot = sortedSlots[i + j]; if (!currentSlot) { isConsecutive = false; break; } if (j > 0) { const prevSlot = consecutiveSlots[j - 1]; if (currentSlot.startTime.getTime() !== prevSlot.endTime.getTime()) { isConsecutive = false; break; } } consecutiveSlots.push(currentSlot); } if (!isConsecutive || consecutiveSlots.length < requiredSlots) { continue; } return consecutiveSlots; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/scheduling/allocationAlgorithms.ts` around lines 560 - 612, Update the allocation logic around sortedSlots and consecutive1HourSlots to validate a contiguous run of requiredSlots 30-minute entries, rather than only Math.ceil(durationHours) entries. Confirm every emitted slot is present and available before returning the run, and derive the returned thirtyMinSlots from that verified range without extending beyond it. Sort a copied availableSlots array so the caller’s input is not mutated.
696-696: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
hoursPerCallis threaded through but never used.
selectCallsFromWeekdeclareshoursPerCall(Line 771) and never reads it — the block length comes entirely fromslotsPerCall. Drop the parameter and the Line 696 computation to avoid implying a second sizing input.Also applies to: 745-753, 768-776
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/scheduling/allocationAlgorithms.ts` at line 696, Remove the unused hoursPerCall computation from the session allocation flow and remove the corresponding parameter from selectCallsFromWeek and its callers, including the code around the referenced lines. Keep block sizing driven solely by slotsPerCall.lib/scheduling/allocationMessages.ts (1)
129-136: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
"1 hours" on the most common session length.
(requiredSlots * 30) / 60yields1for a two-slot (1-hour) session, and the string hardcodes "hours".🔧 Pluralize the hour label
export const notEnoughConsecutive = ( requiredSlots: number, availableHere: number, -): AllocationToast => ({ - variant: "destructive", - title: "Not enough consecutive slots", - description: `Each session requires ${(requiredSlots * 30) / 60} hours (${requiredSlots} consecutive slots). Only ${availableHere} available here.`, -}); +): AllocationToast => { + const hours = (requiredSlots * 30) / 60; + return { + variant: "destructive", + title: "Not enough consecutive slots", + description: `Each session requires ${hours} hour${hours === 1 ? "" : "s"} (${requiredSlots} consecutive slot${plural(requiredSlots)}). Only ${availableHere} available here.`, + }; +};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/scheduling/allocationMessages.ts` around lines 129 - 136, Update notEnoughConsecutive so the duration label uses singular “hour” when (requiredSlots * 30) / 60 equals 1 and plural “hours” otherwise, while preserving the existing duration and slot values in the description.lib/scheduling/allocationService.ts (2)
118-137: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
A non-JSON error body drops
httpStatus.
response.json()(Line 124) runs before theresponse.okcheck, so a gateway/edge error with an HTML or empty body throws into the catch and returns{ success: false, error: "…" }with nohttpStatus. Callers keying on 409 ("allocated elsewhere", seeAllocationResult.httpStatus) then fall back to the generic network message. The fourvalidate*methods repeat the same shape.🛡️ Parse defensively, preserve the status
const response = await fetch(url, { method: "PATCH", headers, body: JSON.stringify(request), }); - const data = await response.json(); + const data = await response.json().catch(() => ({}) as { error?: string; data?: AllocatedAppointmentDto[] }); if (!response.ok) { return { success: false, error: data.error || fallbackError, httpStatus: response.status, }; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const response = await fetch(url, { method: "PATCH", headers, body: JSON.stringify(request), }); const data = await response.json().catch(() => ({}) as { error?: string; data?: AllocatedAppointmentDto[] }); if (!response.ok) { return { success: false, error: data.error || fallbackError, httpStatus: response.status, }; } return { success: true, data: data.data, };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/scheduling/allocationService.ts` around lines 118 - 137, Update the PATCH response handling in the allocation request flow and all four validate* methods to preserve response.status even when the body is empty or non-JSON. Check response.ok before parsing, parse the body defensively, and return the parsed error when available while retaining the HTTP status for failed responses such as 409; keep successful data handling unchanged.
155-234: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Four byte-identical validators — extract a
postValidatehelper.
validateConsultationSlots,validateSubscriptionSlots,validateClassSlotsandvalidateWebinarSlotsdiffer only in URL segment and fallback message. This is exactly whatpatchAllocationalready does for the allocate side; mirroring it keeps the JSON-parse hardening above in one place instead of four.♻️ Suggested consolidation
private static async postValidate( url: string, slots: string[], fallbackError: string, ): Promise<ValidationResponse> { try { const response = await fetch(url, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ slots }), }); const data = await response.json().catch(() => ({}) as { error?: string; data?: SlotConflictResult }); if (!response.ok) { return { success: false, error: data.error || fallbackError }; } return { success: true, data: data.data }; } catch (error) { console.error(`${fallbackError}:`, error); return { success: false, error: error instanceof Error ? error.message : "Network error occurred", }; } }Each public validator then becomes a one-line delegation. Note the four methods are exported API surface used by
useSlotAllocationandRequestedSlotsDialog, so keep their signatures.Also applies to: 282-358
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/scheduling/allocationService.ts` around lines 155 - 234, Extract the shared POST, JSON parsing, response validation, and error handling from validateConsultationSlots, validateSubscriptionSlots, validateClassSlots, and validateWebinarSlots into a private static postValidate helper. Have each public validator preserve its existing signature while delegating with its URL segment and fallback error message, and retain the existing callers and validation response behavior.lib/scheduling/calendarUtils.ts (2)
369-427: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
validateSelectedSlotscannot bucket by the event's scheduling timezone.Every sibling in this module (
validateSlotDistribution,validateDayBasedConsecutiveSlots,groupSlotsByWeek,calculateCallProgress) takesschedulingTimezone, but this one doesn't — so the consultation same-day check at Line 401 and the consecutive check at Line 420 always bucket inSlotCalculationService's default timezone. For an event scheduled in a different timezone, the client verdict can diverge from the server near day boundaries, which is exactly the divergence the module header inlib/scheduling/slotSelectionValidation.ts(lines 10-13) says this bucketing was centralized to prevent.🔧 Thread the timezone through
export function validateSelectedSlots( selectedSlots: TimeSlot[], eventType: "consultation" | "subscription" | "webinar" | "class", requiredSlots?: number, sessionDurationInHours?: number, + schedulingTimezone?: string, ): { isValid: boolean; errorMessage?: string } { @@ if (selectedSlots.length > 1) { const firstSlotDay = SlotCalculationService.dayKey( selectedSlots[0].startTime, + schedulingTimezone, ); const allSameDay = selectedSlots.every( (slot) => - SlotCalculationService.dayKey(slot.startTime) === firstSlotDay, + SlotCalculationService.dayKey(slot.startTime, schedulingTimezone) === + firstSlotDay, ); @@ if ( selectedSlots.length > 1 && - !validateDayBasedConsecutiveSlots(selectedSlots) + !validateDayBasedConsecutiveSlots(selectedSlots, schedulingTimezone) ) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export function validateSelectedSlots( selectedSlots: TimeSlot[], eventType: "consultation" | "subscription" | "webinar" | "class", requiredSlots?: number, sessionDurationInHours?: number, schedulingTimezone?: string, ): { isValid: boolean; errorMessage?: string } { if (selectedSlots.length === 0) { return { isValid: false, errorMessage: "Please select at least one slot" }; } switch (eventType) { case "consultation": { // FIXED: Validate consultation slots based on required duration const consultationRequiredSlots = requiredSlots || calculateRequiredSlots( eventType, undefined, undefined, sessionDurationInHours, ); if (selectedSlots.length !== consultationRequiredSlots) { return { isValid: false, errorMessage: `This Consultation requires exactly ${consultationRequiredSlots} slot${consultationRequiredSlots !== 1 ? "s" : ""} (${sessionDurationInHours || 1} hour${(sessionDurationInHours || 1) > 1 ? "s" : ""})`, }; } // Same-UTC-day requirement first (before consecutive check) — matches // the server's same-day rule. if (selectedSlots.length > 1) { const firstSlotDay = SlotCalculationService.dayKey( selectedSlots[0].startTime, schedulingTimezone, ); const allSameDay = selectedSlots.every( (slot) => SlotCalculationService.dayKey(slot.startTime, schedulingTimezone) === firstSlotDay, ); if (!allSameDay) { return { isValid: false, errorMessage: "Consultation is a one-day event - all slots must be on the same day", }; } } // FIXED: Only check consecutiveness if all slots are on the same day if ( selectedSlots.length > 1 && !validateDayBasedConsecutiveSlots(selectedSlots, schedulingTimezone) ) { return { isValid: false, errorMessage: "Consultation slots must be consecutive within the same day", }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/scheduling/calendarUtils.ts` around lines 369 - 427, Update validateSelectedSlots to accept a schedulingTimezone parameter and thread it through the consultation validation path. Pass that timezone to SlotCalculationService.dayKey and validateDayBasedConsecutiveSlots so same-day and consecutiveness checks use the event’s scheduling timezone rather than the default.
639-683: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Progress count is derived from a global modulo, so scattered partial days report phantom complete calls.
completedCallsat Line 640 divides the total slot count byslotsPerCallregardless of day grouping. WithslotsPerCall = 2and one slot selected on each of two different days, this reports1call scheduled, and becauseincompleteCallSlotsis0the "add more slots" warning at Line 673 is suppressed too — the footer claims a complete call that neitherisCompleteCallnor the server would accept.countCompleteCallsInMapinlib/scheduling/slotSelectionValidation.tsalready implements the per-day rule.🔧 Count complete calls per day
const slotsPerCall = Math.ceil((sessionDurationInHours || 1) / 0.5); - const completedCalls = Math.floor(slots.length / slotsPerCall); - const incompleteCallSlots = slots.length % slotsPerCall; // Group by scheduling-timezone day const slotsByDay = new Map<string, TimeSlot[]>(); @@ const incompleteDays = Array.from(slotsByDay.entries()).filter( ([_, daySlots]) => daySlots.length > 0 && daySlots.length < slotsPerCall, ); + + const completedCalls = Array.from(slotsByDay.values()).filter( + (daySlots) => daySlots.length >= slotsPerCall, + ).length; @@ - if (incompleteCallSlots > 0 && incompleteDays.length > 0) { + if (incompleteDays.length > 0) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/scheduling/calendarUtils.ts` around lines 639 - 683, Replace the global slots.length division and modulo in the progress calculation with the existing per-day completion logic from countCompleteCallsInMap in slotSelectionValidation.ts. Derive completedCalls and incomplete-call warning state from each day’s grouped slots so scattered partial days do not count as complete calls, while preserving the existing progress text and warning formatting.lib/scheduling/slotSelectionValidation.ts (2)
184-203: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Document that
maxSlotschanges unit for subscriptions.For webinar/class/consultation
minSlots/maxSlotsare both slot counts, but hereminSlotsstays in slots whilemaxSlotsbecomes a call count (consumed as such at Line 504 and Line 603). All current call sites agree, but nothing on theSlotLimitsinterface signals the switch, so the next consumer that comparesslots.length > maxSlotsfor a subscription silently gets it wrong.📝 Annotate the contract
export interface SlotLimits { + /** Minimum selection, in 30-min slots. */ minSlots: number; + /** Maximum selection. Slots for webinar/class/consultation; **complete + * calls** for subscription (see getSlotLimits). */ maxSlots: number; slotsPerSession: number; totalSessions: number; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/scheduling/slotSelectionValidation.ts` around lines 184 - 203, Annotate the SlotLimits contract and the subscription branch around the returned maxSlots value to document that subscriptions use maxSlots as a call count, while minSlots remains a slot count. Make the unit distinction explicit for consumers without changing the existing calculation or behavior.
798-826: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
warnOnWeeklyDistributioncan overwrite a failedweeklyDistributionValidback totrue.For subscriptions,
validateSubscriptionSelectionalready setresult.weeklyDistributionValidfrom the strict complete-call cap and pushed an error. Line 801 then unconditionally reassigns it from the looservalidateWeeklyDistribution, so a selection that failed the strict check can end up withweeklyDistributionValid: truewhileerrorsstill contains "Weekly call limit exceeded" — a contradictory result for consumers that read the flag rather thanerrors(e.g.ValidationResultincomponents/dashboard/shared/requests/components/RequestedSlotsDialog.tsx). The class branch also recomputesvalidateWeeklySessionsDistributionwith the exact argumentsvalidateClassSelectionjust used.🔧 Only downgrade, never restore
if (eventType === "subscription") { - result.weeklyDistributionValid = validateWeeklyDistribution( + const spreadOk = validateWeeklyDistribution( slots, options.callsPerWeek, limits.slotsPerSession, timeZone, ); - if (!result.weeklyDistributionValid) { + if (!spreadOk) { + result.weeklyDistributionValid = false; result.warnings.push( `Consider distributing calls more evenly across weeks`, ); } - } else if (eventType === "class") { + } else if (eventType === "class" && result.weeklyDistributionValid !== false) { const ok = validateWeeklySessionsDistribution(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if (!options.callsPerWeek || slots.length === 0) return; if (eventType === "subscription") { const spreadOk = validateWeeklyDistribution( slots, options.callsPerWeek, limits.slotsPerSession, timeZone, ); if (!spreadOk) { result.weeklyDistributionValid = false; result.warnings.push( `Consider distributing calls more evenly across weeks`, ); } } else if (eventType === "class" && result.weeklyDistributionValid !== false) { const ok = validateWeeklySessionsDistribution( slots, options.callsPerWeek, limits.slotsPerSession, timeZone, ); if (!ok) { result.weeklyDistributionValid = false; result.warnings.push( `Consider distributing classes more evenly across weeks`, ); } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/scheduling/slotSelectionValidation.ts` around lines 798 - 826, Update warnOnWeeklyDistribution so its subscription branch cannot restore weeklyDistributionValid to true after validateSubscriptionSelection has marked it false; only downgrade an already-valid result when validateWeeklyDistribution fails, while preserving existing warnings. Apply the same no-recompute principle to the class branch by avoiding redundant validation when validateClassSelection has already evaluated the identical distribution check, and retain the existing invalid warning behavior.
…es they host
A webinar or class is ONE Appointment shared by every registrant, and checkout
tags it with the HOST's org — `plan.organizationId`, not the first registrant's.
That is deliberate and the comment says why: otherwise whoever booked first
would decide which organization the event belonged to. Per-registrant funding
lives on `Payment.organizationId` instead.
Two columns answering two different questions — and the org appointments view
only ever asked one of them. Filtering on `Appointment.organizationId` alone
meant a sponsor that paid to put five employees into someone else's public
webinar saw NOTHING. The money showed on its invoice and the seats came off its
program, but the session itself was invisible. 1:1 kinds were never affected:
there the appointment's org already IS the funding org.
The org arm now matches hosted OR funded. No schema change — both columns
already exist and are already correct.
The narrow half matters more than the wide half
-------------------------------------------------
A shared appointment can carry registrants from several sponsors plus the
public, so widening is only safe if a sponsor still sees just its own seats.
The payer include is filtered server-side to the viewing org and selects
identity only:
payment: {
where: { organizationId: params.scope.orgId },
select: { id: true, user: { select: { id, name, email } } },
}
Acme sees the five people it paid for. Not the twenty it did not, not the other
sponsors', and no amounts — money belongs on Billing and Reimbursements, which
gate on finance permissions rather than `operations.read`.
Attached on the `org` scope only. `orgMember` is already narrowed to the
viewer's own rows and `personal` has no org at all.
The Member column previously rendered "—" for every group event, so a sponsor
could see it had paid for a webinar without being told whom it had paid for. It
now names the org's own funded attendees, "+N" beyond the first.
The existing scope test asserted `organizationId` at the top level with no OR.
Rewritten for the union while keeping the property it was actually protecting —
the org arm carries NO user filter, which is why it requires `operations.read`
and why a non-operator is downgraded to `orgMember`. Widening must not smuggle
one in, and the test now asserts that directly.
Part of #1021
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d throwing Review triage on #1033. Nine comments, all legitimate; two were bugs this branch introduced. 1. Consumers disagreed with the creators about org precedence --------------------------------------------------------------- `createConsultationChannel` / `createSubscriptionChannel` resolve context as `plan.organizationId ?? appointment.organizationId ?? null`. Two genuinely distinct cases sit behind that `??`: a plan can be org-HOSTED while the booking is self-funded, and a personal plan can be booked through an org-funded membership. Three consumers read only the appointment, so every org-hosted-plan booking computed as personal. The reconcile set then looked for `dm-<a>-<b>` while the real channel was `dmo-…` — never re-joined at best, and at worst treated as stale, so `syncUserEventChannels` would remove the user from their own conversation. The search route had the same gap, which meant clicking a result opened a channel that had never been created. Fixed in the reconcile pair-map, the appointment search, AND the backfill script, which had it too and was not flagged. Each query now loads the plan's `organizationId` — a precedence reading a field the query never selected is silently `undefined` and falls straight back into the bug. 2. The length guard turned one bad channel into a broken run -------------------------------------------------------------- `dm-<36>-<36>` is 76 characters, over Stream's 64 cap, and 17 accounts still carry uuid ids. `assertFits` threw there — inside `getDmPairsForUser` and the `expectedChannelIds` map, neither of which is wrapped per-item the way the add-pass is. One legacy account would have failed channel sync for everyone paired with it. That is strictly worse than before the guard existed, when the same call produced a long id and failed later at the Stream API, costing one channel rather than the whole reconciliation. It now degrades to a deterministic `dmh-` hash instead. No id already in use changes: every existing channel is under the cap, and the longest measured 61. 3. The org digest was 32 bits ------------------------------- The org segment is the ONLY thing separating two organizations' otherwise identical pair digest, so a collision would merge two orgs' DM threads for the same pair. 8 hex chars made that plausible in the tens of thousands of orgs. Now 16 and 24 (64 and 96 bits), still using only 45 of the 64 available characters. 4. The layering rule was bypassable ------------------------------------- It blocked `@/app/*` but not `../app/...`, which resolves to the same module. Both spellings now, verified with a probe for each. Smaller: `messages` and `requests` were missing from `PAGE_LABELS`, so those breadcrumbs rendered lowercase; `orgScope` was missing from `fetchData`'s dependency array, so a scope change without a remount kept refetching the previous org; and two docstrings described behaviour the code no longer has — `useOrgScope` still claimed ADMIN/STAFF always win, and `types/recording.ts` still claimed the card re-exports the type. Not done as suggested: restoring the `RecordingData` re-export. Nothing imports it through the card — verified — and re-export shims were removed deliberately. The comment's real substance was the stale docstring, which is corrected. 12 tests, verified failing 6/12 against the pre-fix code. Part of #1021 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@__tests__/security/dm-channel-org-precedence.test.ts`:
- Around line 35-40: Update the source assertion in “the creators put plan org
first” to avoid matching exact indentation and line breaks. Normalize whitespace
or use a whitespace-agnostic pattern while still asserting that
consultation.consultationPlan.organizationId precedes
consultation.appointment?.organizationId in channel.action.ts.
In `@__tests__/security/org-sponsor-event-visibility.test.ts`:
- Around line 58-94: Replace source-text assertions in
__tests__/security/org-sponsor-event-visibility.test.ts:58-94 with direct
assertions on an exported buildInclude(params) helper from list-appointments.ts,
including payment.where and payment.select. In
__tests__/security/org-appointment-detail-ownership.test.ts:1-70, replace
whole-file string matching with tests invoking an extracted ownership function
using mocked detail/profile/orgId inputs, covering cross-org and non-participant
cases; preserve the existing buildWhere behavior.
In `@lib/documents/document-utils.ts`:
- Around line 28-88: Update getStatusColor, getStatusIcon, and getStatusLabel to
accept the exact document review-status enum or union type used by the project,
confirming its declared name and importing it from the existing definition such
as `@prisma/client`. Replace the string parameter types consistently so invalid or
newly added statuses are caught at compile time while preserving the existing
switch mappings and fallback behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ca7e1c85-a9d9-4b45-bf49-8107f19666fd
📒 Files selected for processing (32)
__tests__/documents/revision-threading.test.ts__tests__/enterprise/list-appointments-scope.test.ts__tests__/security/dm-channel-org-precedence.test.ts__tests__/security/org-appointment-detail-ownership.test.ts__tests__/security/org-sponsor-event-visibility.test.tsactions/stream/chat/event-channel.action.tsapp/api/stream/channels/search-appointments/route.tsapp/dashboard/consultant/[consultantId]/(features)/appointments/ConsultantAppointmentsAdapter.tsxapp/dashboard/consultant/[consultantId]/(features)/documents/ConsultantResponseUpload.tsxapp/dashboard/consultant/[consultantId]/(features)/documents/DocumentsTab.tsxapp/dashboard/consultant/[consultantId]/(features)/planner/components/PlanMaterialsUpload.tsxapp/dashboard/consultee/[consulteeId]/(features)/appointments/AppointmentsPageClient.tsxapp/dashboard/consultee/[consulteeId]/(features)/appointments/[appointmentId]/DetailPageClient.tsxapp/dashboard/organization/[orgId]/appointments/AppointmentsPageClient.tsxapp/dashboard/organization/[orgId]/appointments/MyAppointmentsClient.tsxapp/dashboard/organization/[orgId]/appointments/[appointmentId]/DetailPageClient.tsxapp/dashboard/organization/[orgId]/appointments/[appointmentId]/page.tsxapp/dashboard/organization/[orgId]/layout.tsxcomponents/appointments/DocumentUpload.tsxcomponents/appointments/consultee/CancelConfirmationDialog.tsxcomponents/appointments/consultee/ConsulteeAppointmentsAdapter.tsxcomponents/appointments/consultee/ReportIssueDialog.tsxcomponents/appointments/consultee/RescheduleSessionsModal.tsxcomponents/appointments/consultee/useEventActions.tscomponents/dashboard/shared/requests/RequestSlotAllocationTab.tsxeslint.config.mjshooks/useOrgScope.tslib/api/scope/list-appointments.tslib/documents/document-utils.tslib/stream-utils.tsscripts/stream/backfill-channel-org.tstypes/recording.ts
Comment on lines
+35
to
+40
| it("the creators put plan org first", () => { | ||
| const src = read("actions/stream/chat/channel.action.ts"); | ||
| expect(src).toContain( | ||
| "consultation.consultationPlan.organizationId ??\n consultation.appointment?.organizationId ??", | ||
| ); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Exact-string precedence assertion is brittle against reformatting.
Matching a literal multi-line substring (with exact indentation/line breaks) means an unrelated Prettier/formatting pass on channel.action.ts could fail this test with no functional regression. Consider normalizing whitespace before comparing, or matching on a formatting-agnostic pattern (e.g. a regex ignoring whitespace/newlines), while keeping the source-level intent the docstring describes.
♻️ Example: whitespace-agnostic match
- expect(src).toContain(
- "consultation.consultationPlan.organizationId ??\n consultation.appointment?.organizationId ??",
- );
+ const normalized = src.replace(/\s+/g, " ");
+ expect(normalized).toContain(
+ "consultation.consultationPlan.organizationId ?? consultation.appointment?.organizationId ??",
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it("the creators put plan org first", () => { | |
| const src = read("actions/stream/chat/channel.action.ts"); | |
| expect(src).toContain( | |
| "consultation.consultationPlan.organizationId ??\n consultation.appointment?.organizationId ??", | |
| ); | |
| }); | |
| it("the creators put plan org first", () => { | |
| const src = read("actions/stream/chat/channel.action.ts"); | |
| const normalized = src.replace(/\s+/g, " "); | |
| expect(normalized).toContain( | |
| "consultation.consultationPlan.organizationId ?? consultation.appointment?.organizationId ??", | |
| ); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@__tests__/security/dm-channel-org-precedence.test.ts` around lines 35 - 40,
Update the source assertion in “the creators put plan org first” to avoid
matching exact indentation and line breaks. Normalize whitespace or use a
whitespace-agnostic pattern while still asserting that
consultation.consultationPlan.organizationId precedes
consultation.appointment?.organizationId in channel.action.ts.
Comment on lines
+58
to
+94
| describe("but only the sponsor's OWN seats are returned", () => { | ||
| it("the payer include is filtered to the viewing org", () => { | ||
| // An unfiltered `payment: true` would hand a sponsor every registrant's | ||
| // identity on a shared webinar — including other sponsors' employees and | ||
| // members of the public. The `where` is the whole guard. | ||
| expect(SRC).toContain("where: { organizationId: params.scope.orgId }"); | ||
|
|
||
| const start = SRC.indexOf("payment: {\n where:"); | ||
| expect(start).toBeGreaterThan(-1); | ||
| }); | ||
|
|
||
| it("the payer include is attached ONLY on the org scope", () => { | ||
| // `orgMember` is already narrowed to the viewer's own rows and `personal` | ||
| // has no org at all; attaching it there would be meaningless at best. | ||
| expect(SRC).toContain('...(params.scope.kind === "org"'); | ||
| }); | ||
|
|
||
| it("selects the payer's identity and nothing else from Payment", () => { | ||
| const start = SRC.indexOf("payment: {\n where:"); | ||
| const block = SRC.slice(start, start + 400); | ||
|
|
||
| expect(block).toContain("user: { select: { id: true, name: true, email: true } }"); | ||
| // No money on this surface: amounts belong on Billing and Reimbursements, | ||
| // both of which gate on finance permissions rather than operations.read. | ||
| for (const field of ["amount", "amountPaise", "paymentIntent", "status"]) { | ||
| expect(block).not.toContain(`${field}: true`); | ||
| } | ||
| }); | ||
|
|
||
| it("still applies no user filter — the org arm is role-gated, not self-scoped", () => { | ||
| const w = buildWhere({ | ||
| scope: { kind: "org", orgId: "acme" }, | ||
| userId: "u1", | ||
| }); | ||
| expect(JSON.stringify(w)).not.toContain("u1"); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy lift
Security-critical tests assert on literal source text instead of executed behavior.
Both new security specs read the implementation file with readFileSync and assert via toContain/indexOf against exact literal strings (including specific indentation). This is fragile in both directions: a harmless reformat/rename breaks the suite with zero real regression, while a refactor that keeps the matched substrings but changes actual control flow (e.g., relocates a check inside a branch that never runs) would still pass. The OR-clause test in the same sponsor-visibility file already shows the stronger pattern — invoking buildWhere and asserting on the real return value.
__tests__/security/org-sponsor-event-visibility.test.ts#L58-L94: replace theSRC.indexOf("payment: {\n where:")/ substring checks with an assertion against the actual Prismaincludeobject (e.g., export a smallbuildInclude(params)helper fromlib/api/scope/list-appointments.tsalongsidebuildWhere, and assert itspayment.where/payment.selectshape directly).__tests__/security/org-appointment-detail-ownership.test.ts#L1-L70: replace the whole-file string-matching approach with a test that exercises the ownership logic directly — e.g., extract theownscomputation and the org/participation guards into a testable function invoked with mockeddetail/profile/orgIdinputs, then assert on its return value for cross-org and non-participant cases.
📍 Affects 2 files
__tests__/security/org-sponsor-event-visibility.test.ts#L58-L94(this comment)__tests__/security/org-appointment-detail-ownership.test.ts#L1-L70
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@__tests__/security/org-sponsor-event-visibility.test.ts` around lines 58 -
94, Replace source-text assertions in
__tests__/security/org-sponsor-event-visibility.test.ts:58-94 with direct
assertions on an exported buildInclude(params) helper from list-appointments.ts,
including payment.where and payment.select. In
__tests__/security/org-appointment-detail-ownership.test.ts:1-70, replace
whole-file string matching with tests invoking an extracted ownership function
using mocked detail/profile/orgId inputs, covering cross-org and non-participant
cases; preserve the existing buildWhere behavior.
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@__tests__/security/dm-channel-org-precedence.test.ts`:
- Around line 35-40: Update the source assertion in “the creators put plan org
first” to avoid matching exact indentation and line breaks. Normalize whitespace
or use a whitespace-agnostic pattern while still asserting that
consultation.consultationPlan.organizationId precedes
consultation.appointment?.organizationId in channel.action.ts.
In `@__tests__/security/org-sponsor-event-visibility.test.ts`:
- Around line 58-94: Replace source-text assertions in
__tests__/security/org-sponsor-event-visibility.test.ts:58-94 with direct
assertions on an exported buildInclude(params) helper from list-appointments.ts,
including payment.where and payment.select. In
__tests__/security/org-appointment-detail-ownership.test.ts:1-70, replace
whole-file string matching with tests invoking an extracted ownership function
using mocked detail/profile/orgId inputs, covering cross-org and non-participant
cases; preserve the existing buildWhere behavior.
In `@lib/documents/document-utils.ts`:
- Around line 28-88: Update getStatusColor, getStatusIcon, and getStatusLabel to
accept the exact document review-status enum or union type used by the project,
confirming its declared name and importing it from the existing definition such
as `@prisma/client`. Replace the string parameter types consistently so invalid or
newly added statuses are caught at compile time while preserving the existing
switch mappings and fallback behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ca7e1c85-a9d9-4b45-bf49-8107f19666fd
📒 Files selected for processing (32)
__tests__/documents/revision-threading.test.ts__tests__/enterprise/list-appointments-scope.test.ts__tests__/security/dm-channel-org-precedence.test.ts__tests__/security/org-appointment-detail-ownership.test.ts__tests__/security/org-sponsor-event-visibility.test.tsactions/stream/chat/event-channel.action.tsapp/api/stream/channels/search-appointments/route.tsapp/dashboard/consultant/[consultantId]/(features)/appointments/ConsultantAppointmentsAdapter.tsxapp/dashboard/consultant/[consultantId]/(features)/documents/ConsultantResponseUpload.tsxapp/dashboard/consultant/[consultantId]/(features)/documents/DocumentsTab.tsxapp/dashboard/consultant/[consultantId]/(features)/planner/components/PlanMaterialsUpload.tsxapp/dashboard/consultee/[consulteeId]/(features)/appointments/AppointmentsPageClient.tsxapp/dashboard/consultee/[consulteeId]/(features)/appointments/[appointmentId]/DetailPageClient.tsxapp/dashboard/organization/[orgId]/appointments/AppointmentsPageClient.tsxapp/dashboard/organization/[orgId]/appointments/MyAppointmentsClient.tsxapp/dashboard/organization/[orgId]/appointments/[appointmentId]/DetailPageClient.tsxapp/dashboard/organization/[orgId]/appointments/[appointmentId]/page.tsxapp/dashboard/organization/[orgId]/layout.tsxcomponents/appointments/DocumentUpload.tsxcomponents/appointments/consultee/CancelConfirmationDialog.tsxcomponents/appointments/consultee/ConsulteeAppointmentsAdapter.tsxcomponents/appointments/consultee/ReportIssueDialog.tsxcomponents/appointments/consultee/RescheduleSessionsModal.tsxcomponents/appointments/consultee/useEventActions.tscomponents/dashboard/shared/requests/RequestSlotAllocationTab.tsxeslint.config.mjshooks/useOrgScope.tslib/api/scope/list-appointments.tslib/documents/document-utils.tslib/stream-utils.tsscripts/stream/backfill-channel-org.tstypes/recording.ts
🛑 Comments failed to post (1)
lib/documents/document-utils.ts (1)
28-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Consider typing
statusagainst the actual review-status enum.Now that this is the single shared implementation (previously duplicated across 4 files), typing
statusasstringmeans a typo or newly-added status value silently falls through to thedefaultbranch instead of failing at compile time.♻️ Suggested tightening
-export function getStatusColor(status: string): string { +export function getStatusColor(status: DocumentReviewStatus): string {(apply similarly to
getStatusIconandgetStatusLabel, importing the enum/union type from@prisma/clientor wherever the document review status is declared)Please confirm the exact enum name backing document review status before applying.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/documents/document-utils.ts` around lines 28 - 88, Update getStatusColor, getStatusIcon, and getStatusLabel to accept the exact document review-status enum or union type used by the project, confirming its declared name and importing it from the existing definition such as `@prisma/client`. Replace the string parameter types consistently so invalid or newly added statuses are caught at compile time while preserving the existing switch mappings and fallback behavior.
teetangh
added a commit
that referenced
this pull request
Jul 28, 2026
Two conflicts, both from #1033 landing on dev while this branch was open. `ClientClassRegistration.tsx` — #1033 moved `ClassPlanProgram` out of `app/explore/programs/utils` into `lib/explore/programs` as part of its app/ layering boundary, while this branch dropped `countUniqueParticipants` in favour of `getClassCapacity`. Kept both: the new import path and the narrowed import list. The `ClassInstance.maxParticipants` field this branch added travelled with the file into its new home. `organization/[orgId]/layout.tsx` — #1033 added the `messages` and `requests` nav labels; this branch had only reformatted the map. Took #1033's entries. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
teetangh
added a commit
that referenced
this pull request
Jul 28, 2026
…tion (#1039) Three changes to the personal dashboards, all about a nav entry meaning one thing. RESOURCES BECOMES DOCUMENTS AND RECORDINGS The consultee side had a group labelled "Activity" containing a single item called "Resources", and that page was tabbed by EVENT TYPE with each card listing its materials and recordings together. "Where is the recording" and "where is the handout" are different errands; one page tabbed by event answered neither directly. The group is now Resources and the two artifacts are the entries; event-type tabs stay as grouping. No API change — the endpoint already returns `materials[]` and `recordings[]` per event, so this is a re-slice of one fetch, and both pages share a query key so opening one warms the other. The consultant side's equivalent group was called "Content"; renamed so both dashboards use one vocabulary. `/resources` redirects rather than 404s. Review caught that the first cut only hid content INSIDE each card while totals, tab counts, the default tab and the empty state still counted every event — so Documents rendered an empty card per recording-only session and the badge disagreed with the screen. The set is narrowed by artifact first now. SUPPORT BECOMES A SECTION Requests, Feedback and Help were UrlTabs on one page. They are distinct destinations — a ticket list, a form and an FAQ share no state — so they are sidebar entries now, with Settings as the fourth. Settings is removed from the bottom user-chip dropdown on both shells: two links to one href is the duplicate-destination problem ADR 19 exists to stop. THE CHAT SKELETON COVERED A FRACTION OF THE SCREEN `ChatSkeleton` had no height at all — `flex-1 flex m-4` only fills the viewport inside a flex parent that has one, and `loading.tsx` renders it as the whole route, outside the `h-[calc(100dvh-…)]` wrapper the live MessagesTab applies. It now mirrors that wrapper including the mobile tab-bar subtraction, and uses theme tokens rather than hardcoded indigo. The consultant tree had a second drifted copy, deleted in favour of the shared one. LAYERING `EventResourceCard` and `ResourcesTab` move to components/, since two routes render them. That surfaced `status-guards`, a leaf util in the consultee appointments route folder used by both the card and the consultee home tab, which moves to lib/appointments/. The lint rule from #1033 made this non-optional rather than something to notice later. VERIFICATION tsc clean · 1828 tests / 160 suites · lint clean · Sonar pass. No schema change, so no DB push. Part of #1021 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Jul 31, 2026
12 tasks
This was referenced Aug 1, 2026
This was referenced Aug 12, 2026
fix(stream): make 1:1 recording possible, rate-limit Stream, lock down debug, give trials chat
#1138
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ADR 19 splits dashboards by the org-ness of the underlying work. The participant
surfaces were never split — they existed only in the personal trees — so a
member's org conversations and their slot-allocation requests either vanished or
leaked into their B2C dashboard depending on which default the scope hook
resolved. This builds the org half.
Replaces #1032, which fixed the symptom the wrong way round by merging org
conversations into the personal inbox.
DM channels are keyed per context
getDmChannelIdkeyed on the pair alone, which made a channel the RELATIONSHIPrather than the booking: the same two people had one thread however many
sessions they booked and whoever funded them, carrying the org tag of whichever
booking happened to create it. That thread belonged in two dashboards, so it
could not land in the right one.
Personal ids are untouched, so no existing conversation moves — and there was no
room to extend them anyway. Stream caps ids at 64; a seeded cuid pair already
produces 54 and the longest live channel measured 61, so appending a 25-char org
id was never possible. Hence hashing both halves for the org form. Verified
against the live Stream app that zero org-tagged channels existed, so nothing
needed migrating.
Also adds the length guard that was missing entirely —
channelIdSchemaasserted only
min(1), and two users on 36-char uuid ids (17 accounts havethem) would produce a 76-char id that Stream rejects silently.
Messages
/dashboard/organization/[orgId]/messages. Nothing in it filters by org:useOrgScopeis route-pinned under the org tree, so living at this path scopesthe query. A member of several orgs gets one clean inbox per org, no picker —
which is what the merged inbox could not do, since a conversation carries no
visible org label.
The personal inbox now pins to B2C explicitly instead of inheriting
first-org,which hid a member's B2C threads behind whichever org happened to be first and
hid a second org's entirely.
Operators cannot read any of it. No org-wide chat query exists behind the
page and Stream only returns channels the viewer belongs to. ADR 20 stands.
Requests
/dashboard/organization/[orgId]/requests, and this one had a live cost. Slotallocation existed ONLY in the consultant tree — there is no allocation surface
anywhere under
app/dashboard/organization/— and it fetches the bookingsendpoints without
orgScope, which makes them drop org-funded rows.An org-sponsored subscription was therefore paid for and never scheduled:
the request existed and no surface in the product would show it.
Layering
Reusing the allocation UI meant
components/importing from a[consultantId]route folder. That folder was already shared in intent and merely parked in the
wrong place, so it moved to
components/scheduling/,hooks/scheduling/andlib/scheduling/.Closing that surfaced the rest of the class: two type modules
lib/reachedinto route folders for, two API response types living in
route.tsfiles, andlib/data/explore-programs.tsimporting live functions fromapp/explore. Theresponse shapes are now Zod schemas in
schemas/that the handlers parseagainst on the way out, so route and consumer agree by construction rather
than by assertion.
no-restricted-importsnow bans@/app/*from lib, components, hooks, typesand schemas. Verified it fires on a deliberate violation and that the tree is
clean under it.
Also bumps
targetes5 → es2017 (the Next.js default; es5 only ever constrainedthe type checker and is deprecated in TS 7.0).
baseUrlstays — ~40 filesimport via bare specifiers and resolve only through it.
Verification
tscclean · 1798 tests / 156 suites · lint clean on the diff. No schemachange, so no DB push.
Still to come on this branch
Org Documents, reschedule/cancel on own sessions, and the org planner. Pushing
the PR now so the foundation and the two live-bug fixes are reviewable rather
than held behind the rest.
Part of #1021
Summary by CodeRabbit