Skip to content

fix(appointments): gate Manage Timings on commitment, and give group events a real Unschedule - #1083

Merged
teetangh merged 9 commits into
devfrom
fix/manage-timings-counterparty-gate
Aug 1, 2026
Merged

fix(appointments): gate Manage Timings on commitment, and give group events a real Unschedule#1083
teetangh merged 9 commits into
devfrom
fix/manage-timings-counterparty-gate

Conversation

@teetangh

@teetangh teetangh commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

A consultee books a consultation, pays for it, and puts the time in their diary. The consultant then opens Manage Timings on that booking, picks a different slot, and saves. There is no notice period, no request, and nothing for the consultee to accept — the session simply moves, and they find out when their calendar changes. The same is true of a subscription session part-way through a programme.

The cause is one line in the wrong place, not a wrong policy. The MANAGE_TIMINGS picker sets minLeadHours: 0 and its own comment explains why: the surface has no counterparty to give notice to. That is entirely true of what it was built for, which is a consultant deciding when their own webinar or class instance runs. The consultant appointments menu simply never made the distinction, and offered the action for every appointment in a non-cancelled, non-past bucket.

This PR fixes that, and in doing so gives the consultant's menu three named actions where it previously had two, one of which was wearing the wrong label.

The model — three actions, three predicates

The deciding question for the first two is whether anyone has already committed to a time, not who owns the calendar.

Manage Timings stays wherever nobody has committed. A webinar or class instance keeps it unconditionally, including once its slots are confirmed: the organiser publishes a schedule and attendees buy into it, so there is no counterparty to negotiate with and asking thirty attendees to each accept a new time is not a coherent flow. An offering that has never been scheduled keeps it, since it has no Appointment row at all and travels as an unscheduled-class-<id> / unscheduled-webinar-<id> synthetic id. A booking whose slots are still tentative keeps it too, because a tentative slot means the request is awaiting allocation and nothing has been placed yet.

Reschedule replaces it wherever a counterparty holds a confirmed time. That means a consultation or a subscription session with allocated, non-tentative slots. Moving one of those is a two-party negotiation, so it goes through the proposal the consultee has to answer. It is 1:1 only.

Unschedule is new, and it is orthogonal to that pair rather than a third branch of it. It pulls a placed group event off the calendar and back into the allocate queue without cancelling it, so a confirmed webinar offers Manage Timings and Unschedule, while a 1:1 offers neither of those and gets Reschedule instead. The "never both, never neither" property from the original change still holds, unchanged, within the Timings/Reschedule pair.

Trials follow the same rule and land on the same side as a consultation. A trial is strictly one consultant and one consultee, and although the consultant is the one who allocates the time, the slot is created with isTentative: false and the consultee is notified of it immediately — so it is a commitment by exactly the test above. This changes nothing in practice: the menu already withheld Timings from trials because a trial VM carries no raw.appointment, and readManageTimingsTarget already returned null for a TRIAL appointment type. The predicate now says so explicitly rather than leaving it to those two accidents.

Unschedule is not Cancel, and a reviewer should not read it as one

This is the distinction the whole second half of the PR exists to draw. The two actions sit one menu row apart and undo wildly different amounts.

Unschedule Cancel booking
the booking still exists, still sold over
money untouched, no refund refunded per policy tier
attendees still enrolled refunded and released
earnings / ledger untouched reversed
slots tentative, back in the allocate queue CANCELLED, terminal
reversible yes, by setting new times no

Unschedule means "this is still happening, I just do not know when yet." For a webinar with thirty paid attendees, that is the difference between moving the date and refunding ₹150,000. Nothing on this path touches money, earnings, the ledger or utilization, and the confirm dialog says so in as many words before naming Cancel booking as the action for the other outcome. It is deliberately not styled destructive, because a red button would read as "refund everyone".

Why the capability already existed

The first commit's removal of Reschedule from webinars and classes was correct, but it removed a real capability along with a wrong label. For a group event that route never opened a proposal — its own docstring says "For WEBINAR/CLASS: Marks all slots as tentative", and the auto-confirm block notes that "Only the two 1:1 kinds carry proposals". Marking every slot tentative and re-stamping the event SCHEDULED is an unschedule. It was an Unschedule wearing the Reschedule label.

So handleUnschedule posts to the same POST /api/appointments/[appointmentId]/reschedule with no body, exactly as the group-event Reschedule menu item used to. There is no parallel implementation, no new route and no new transaction. What is new is the name, the predicate that decides when to offer it, and copy that tells the truth about what it does. The type query param is deliberately omitted: the route derives the type from the DB and only compares when one is supplied, so omitting it cannot mismatch.

Attendee notification — already correct, and confirmed

A sibling audit found that group-event cancellation notifies nobody (being fixed separately in #1081), so the same gap was expected here. It is not present. The reschedule route's post-transaction fan-out re-reads the appointment with slotsOfAppointment: { select: { user: … } } and pushes every connected user into the recipient list before calling notifyAppointmentRescheduled — the "FIX #624" block. Attendees are excluded only if they are the initiator.

That is sufficient for classes as well as webinars, even though the notification is scoped to a single appointment while the release covers every session of the class. Group checkout never mints a per-buyer slot: handleClassCheckout connects the buyer to every slot of every session appointment, and handleWebinarCheckout to every slot of the shared appointment. One appointment's slots therefore already carry the whole roster. No second recipient assembly was written.

Why no new mechanism was needed for the 1:1 side

A reviewer will reasonably ask whether this should have shipped a reschedule-request feature. It should not, because the two-party protocol already exists and is already asymmetric in the right direction. A consultee proposing a time inside the consultant's published availability auto-confirms, since publishing availability is standing consent to be booked within it. A consultant proposing never auto-confirms, since a consultee merely being free at a time is not consent to be moved to it. The initiator may withdraw, the recipient may decline, and both decline and expiry drop the booking into the consultant's allocate queue, so nothing dead-ends. A consultant with a genuine emergency opens a proposal and the consultee accepts; if they never answer, expiry hands it back to the allocate queue; and cancellation with a refund remains available throughout. There is therefore no case that requires moving a committed session without agreement.

What changed

lib/appointments/slots.ts gains allowsManageTimings(kind, slots) and allowsUnschedule(kind, slots), alongside the slotsAllowReschedule the first is the complement of. allowsUnschedule returns true only for a WEBINAR or CLASS with at least one placed (non-tentative) upcoming slot, which makes the action idempotent by construction rather than by a second guard — the release leaves every slot tentative, so an already-unscheduled event stops offering it and offers Manage Timings instead. The module also gains upcomingSlots, the "still ahead of now, chronological" filter that the consultant adapter had inline, so the menu and the page decide on the same list: a finished session is not what "has someone committed to a time" is asking about, and without the filter a subscription with past sessions and unallocated future ones would read as committed.

ConsultantAppointmentsAdapter.tsx gates the Timings item on the new predicate and adds its negation to the Reschedule item's condition, so those two are now mutually exclusive by construction rather than by coincidence. The one deliberate exception is a booking with a reschedule already in flight, where the released slot awaiting a new time is the open proposal — Reschedule would earn a 409 and Timings would write straight over the proposal the consultee is still answering, so neither is offered. There is a test naming that case. The adapter also gains the Unschedule item, gated on plan ownership (the same canManageBookingLifecycle check Cancel and Reschedule use, so a collaborator who is not the HOST does not get it), a non-terminal status, isConfirmedStatus — which matches the route's own from-state of SCHEDULED/IN_PROGRESS for a group release — and the new predicate.

components/appointments/UnscheduleConfirmationDialog.tsx is new: the confirm step described above.

useConsultantEventActions.ts gains handleUnschedule, the thin call onto the existing route, with a success toast that repeats the two things a consultant needs to be sure of — the event is back in their queue, and attendees stay enrolled and have been told.

timings/page.tsx enforces the Manage Timings rule with notFound() after the ownership check. The menu is not a control on its own: the URL is linkable and survives a refresh. generateMetadata already gated the offering title on planOwnerIds; that stays, and the counterparty gate joins it so a route that 404s never gets a titled tab either. Only the two 1:1 kinds need the extra slot read and only they pay for it, and it goes through React.cache so metadata and body share one query. The slots are fetched in the page rather than widened into ManageTimingsTarget deliberately, to stay clear of #1075, which is editing that file concurrently.

Not in scope

Whether an attendee may refund out of a booking that has been unscheduled is an open question. If a consultant withdraws the date of a webinar someone paid for, it is arguable that the attendee should be able to take their money back rather than wait for a date that may not suit them. That is a consumer-protection decision rather than an engineering one, it has not been made, and nothing here builds toward either answer: there is no refund affordance on the unschedule path and no policy hook waiting for one. It needs a decision before this ships to consultants at scale.

The route's existing 24-hour floor applies to Unschedule unchanged, since it is the same route: an event starting within 24 hours cannot be unscheduled and the API returns a 400. The menu does not pre-empt that, exactly as it does not for Reschedule today, so the consultant meets it as an error rather than as a hidden action. Duplicating MINIMUM_HOURS_BEFORE_RESCHEDULE into the client to hide the item would put the policy in two places, which is worse.

The proposal protocol, the auto-confirm asymmetry and the single round are all untouched, as is the MANAGE_TIMINGS policy itself — minLeadHours: 0 and "no counterparty" remain correct for the surface's real use. There is no schema change and no migration. The consultant's menu still offers a trial neither Timings nor Reschedule, because the reschedule API has no TRIAL branch and returns 403 for one; that gap predates this change. Group-event cancellation still notifies nobody; that is #1081, not this.

Also in this PR — two bugs on the notification surface

Neither belongs to the Manage Timings story. Both were found while looking at it, this branch already owns the reschedule route that causes the first, and there are more open branches than is useful right now, so they ride here as two separate commits rather than as a third pull request.

A reschedule with no new time rendered "from  to"

A reschedule notification reached the consultant's inbox with both of its times missing, reading "rescheduled … from to". AppointmentRescheduledPayload declared oldDateTime? and newDateTime?, the reschedule route passed neither, and the template interpolated both anyway.

Passing two values would not have been the fix, because the sentence itself is wrong for the common case. A plain release has no destination — that is the entire point of "Any time works", where the released slots go back to the consultant's queue and no new time exists yet. Only an auto-confirmed proposal has actually moved anything. Between those two sits a third case the route already distinguishes in its own response message, which is a proposal that has been made and is still waiting for the other party to answer.

The payload now carries an outcome of MOVED, PROPOSED or RELEASED, and its arms are a union rather than a bag of optional fields: MOVED and PROPOSED cannot be constructed without both times, so the blank-blank payload that produced the screenshot is a compile error rather than a rendering accident. One workflow id with a variant flag in the payload is the idiom the enterprise workflows already use, where reminderStage drives the dunning copy and kind separates a rejected payout from a reversed one, so no second workflow id was minted.

rescheduleNotificationVariant in lib/booking/reschedule-proposals.ts derives the variant, which keeps the route a caller and makes the rule unit-testable without a database. The released time is captured inside the transaction rather than read back afterwards, because an auto-confirm deletes those slot rows and writes new ones — by the time the notification is assembled, the time being given up exists nowhere else.

What still has to change in Novu

The appointment-rescheduled template lives in the Novu dashboard and not in this repository, so this PR cannot fix the rendered sentence on its own. What it does is make the payload carry enough for the template to tell the three cases apart. The dashboard template has to branch on the discriminator:

{% if payload.outcome == "MOVED" %}
  Your {{payload.appointmentType}} moved from {{payload.oldDateTime}} to {{payload.newDateTime}}.
{% elsif payload.outcome == "PROPOSED" %}
  {{payload.consulteeName}} asked to move your {{payload.appointmentType}} from {{payload.oldDateTime}} to {{payload.newDateTime}}.
{% else %}
  Your {{payload.appointmentType}} was released and is awaiting a new time.
{% endif %}

The RELEASED branch must not reference newDateTime at all: the payload does not carry the key, and it is absent rather than undefined so that nothing serializes into the interpolation. oldDateTime is present on RELEASED too in every ordinary case and may be used as colour, but the branch must still read correctly without it, since a booking with no placed slots supplies none.

Until that template change lands, the auto-confirmed case renders real times where it rendered two blanks, and the released case still renders the wrong sentence. Merging this alone does not close the bug.

The same shape elsewhere in lib/novu

A payload field that is declared optional and then passed by no caller is a pattern rather than a one-off, so every optional field on every payload in lib/novu/workflows.ts and lib/novu/org-workflows.ts was checked against its call sites. Four fields are never passed by anyone.

AppointmentRescheduledPayload.oldDateTime and newDateTime are the bug above. SupportTicketPayload.respondedBy is the same shape one subsystem over — a template naming the staff member who replied had nothing to name — and it is fixed here, since the responder is already loaded on the row the route just created. AppointmentPayload.dateTime is missing from notifyAppointmentBooked in the payment webhook handler and from both notifyAppointmentCompleted calls in the auto-complete sweep; neither is fixed here. The booked one needs a slot read added to a select that #734 deliberately trimmed, in a payments file another branch is currently editing, and the completed ones need the sweep's slot bookkeeping changed to keep the row rather than only its end time. The reminder path, which is the one where a missing date would be most obviously wrong, does pass it.

Every other optional field is passed by at least one caller.

The Inbox panel sat on top of the calendar

The notification panel covered the Friday and Saturday columns of the slot grid entirely at a normal laptop width. Two things put it there. It was Novu's own bundled popover, fixed at 400px on a bespoke z-index of 9999 with no collision handling, and its open state belonged to Novu, so clicking a notification routed the user to the page that notification was about and then stayed open on top of it. The panel a consultant found over their calendar was usually the one that had just sent them there.

Given children, Novu's Inbox drops to being a provider and exposes Bell and InboxContent as composition parts, which makes the panel ours to place. It is now the repo's Radix popover from components/ui/popover, so it gets collision-aware placement, a size capped against the viewport on both axes, Escape and outside-click dismissal, and the shared z-[1200] layer instead of a number picked to beat everything else on the page. Because the open state is ours, the panel closes before routing. It is also narrower, at 22rem against the old 25rem, which leaves more of the grid legible while it is open.

This is presentational and nothing about the notification data flow moved. The subscriber, the ADR 23 scope tabs, the appearance variables and the click-to-route behaviour are all unchanged. The two appearance keys that styled Novu's popover are dropped because that popover no longer renders, and bellContainer goes with them, because a custom renderBell has always bypassed the container it styled.

Verification

tsc --noEmit is clean from a cold build (cache cleared, 8 GB heap). eslint is clean on all nine changed files. Prettier is clean on every changed file but three; the three the notification commits touch — the reschedule route, the staff support-response route and the Inbox component — were already Prettier-dirty on dev before this branch existed, and running --write over them would reformat unrelated lines, so the added code is hand-formatted to match what Prettier would emit and the pre-existing drift is left where it was. Jest goes from 192 suites / 2166 tests on the untouched branch, to 193 / 2178 after the first commit, to 193 / 2185 after the second, and to 194 suites / 2199 tests here — all passing, no regressions. The extra suite is not one of ours: it arrived with the dev merge that brought #1079 onto this branch. The nineteen new tests cover each case in the rule, the mutual exclusivity of Timings and Reschedule across all of them, the in-flight exception, the upcoming-slots filter, and for Unschedule: that a confirmed webinar and a confirmed class each offer Timings and Unschedule but not Reschedule, that a never-scheduled offering and an already-unscheduled event offer Timings but not Unschedule, that a part-released class still offers it while any session is placed, that a confirmed 1:1 offers Reschedule and neither other action, and that Unschedule never appears for any 1:1 kind whatever its slots look like.

Four further tests cover the reschedule payload: an auto-confirmed proposal carries both times as MOVED, an unanswered one carries the same pair as PROPOSED, a plain release carries RELEASED with the released time and no newDateTime key at all, and a reschedule with no released time to report degrades to RELEASED rather than half-filling the other sentence.

One caveat on the Inbox change. The instruction for this work was no dev server, so the panel has been verified by type-checking, linting and reading Novu's composition API — Inbox with children mounts the provider only, Bell with a custom renderBell renders no button of its own so nothing nests inside the trigger, and InboxContent carries the tabs through context — but not by opening it in a browser. It wants one look before merge.

Closes #1082

Summary by CodeRabbit

  • New Features
    • Added Unschedule for eligible webinar and class bookings, with confirmation messaging that distinguishes it from cancellation.
    • Improved appointment actions by showing Manage Timings or Reschedule based on booking status and upcoming time slots.
    • Added safeguards preventing Manage Timings access when committed upcoming sessions exist.
    • Enhanced rescheduling notifications with clearer moved, proposed, and released outcomes and relevant times.
    • Staff response notifications now include the responder’s name.
    • Improved notification inbox behavior, placement, and unread-count presentation.

Manage Timings writes new times with no notice requirement and no
acceptance, which is honest only while nobody else holds the time. The
consultant menu offered it for any non-cancelled, non-past booking, so a
consultation or subscription session a consultee had paid for could be
moved out from under them.

Adds `allowsManageTimings` next to `slotsAllowReschedule`: group events
and anything unallocated keep the surface, a 1:1 with a confirmed slot
loses it and gets the negotiated Reschedule instead. The two are now
complements, so a booking never shows both. The page enforces the same
rule after its ownership check, because the URL is linkable.

Closes #1082
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@netlify

netlify Bot commented Aug 1, 2026

Copy link
Copy Markdown

Deploy Preview for familiarise ready!

Name Link
🔨 Latest commit 35987d7
🔍 Latest deploy log https://app.netlify.com/projects/familiarise/deploys/6a6dd9107bc81e00085a2f95
😎 Deploy Preview https://deploy-preview-1083--familiarise.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 61 (🟢 up 3 from production)
Accessibility: 99 (🟢 up 3 from production)
Best Practices: 92 (🟢 up 9 from production)
SEO: 99 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 3e3feecc-ac25-4403-b6cc-226cc6d22d80

📥 Commits

Reviewing files that changed from the base of the PR and between 90acda5 and 35987d7.

📒 Files selected for processing (2)
  • __tests__/booking-algorithm/manage-timings-affordance.test.ts
  • lib/appointments/slots.ts

📝 Walkthrough

Walkthrough

Changes

Appointment timing actions

Layer / File(s) Summary
Slot eligibility rules and validation
lib/appointments/slots.ts, __tests__/booking-algorithm/manage-timings-affordance.test.ts
Shared helpers determine Manage Timings, Unschedule, and upcoming slots. Tests cover appointment kinds, slot states, proposals, and completed sessions.
Consultant timing actions and route gate
app/dashboard/consultant/..., components/appointments/UnscheduleConfirmationDialog.tsx
Consultant actions use shared eligibility rules. Unschedule includes a confirmation dialog and request handler. Manage-timings routes enforce counterparty eligibility.

Reschedule notification outcomes

Layer / File(s) Summary
Reschedule outcome contract and mapping
lib/novu/workflows.ts, lib/booking/reschedule-proposals.ts, __tests__/booking-algorithm/reschedule-proposals.test.ts
Reschedule notifications use MOVED, PROPOSED, and RELEASED variants with validated timestamp fields.
Reschedule route notification integration
app/api/appointments/[appointmentId]/reschedule/route.ts
The route preserves released and proposed times and uses them to build notification payloads.

Notification UI and response metadata

Layer / File(s) Summary
Notification presentation and support metadata
components/notifications/NotificationInbox.tsx, app/api/staff/support-tickets/[ticketId]/responses/route.ts
The notification inbox uses a controlled popover. Support response notifications include the responder name or "Support".

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Consultant
  participant ConsultantAppointmentsAdapter
  participant useConsultantEventActions
  participant RescheduleRoute
  Consultant->>ConsultantAppointmentsAdapter: select Unschedule
  ConsultantAppointmentsAdapter->>useConsultantEventActions: call handleUnschedule
  useConsultantEventActions->>RescheduleRoute: post unschedule request
  RescheduleRoute-->>useConsultantEventActions: return action result
Loading
sequenceDiagram
  participant RescheduleRoute
  participant rescheduleNotificationVariant
  participant NotificationService
  RescheduleRoute->>RescheduleRoute: capture releasedAt and proposedAt
  RescheduleRoute->>rescheduleNotificationVariant: classify outcome
  rescheduleNotificationVariant-->>RescheduleRoute: return outcome fields
  RescheduleRoute->>NotificationService: send rescheduled payload
Loading

Possibly related issues

Possibly related PRs

Poem

A rabbit sorts the future slots,
And guards the times that bookings lock.
Unschedule waits for clear consent,
While MOVED notes show where sessions went.
A popover closes, neat and bright—
New dates hop into view tonight.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning Issue #1082 is addressed, but issue #624 is not: the changes do not add webinar/class attendee expansion or participant notification tests. Load booked webinar/class participants, notify them and the organizer, and add route tests that verify the full recipient list for reschedules.
Out of Scope Changes check ⚠️ Warning The PR includes unrelated changes to staff response notification names and the Novu notification popover, which are not required by issues #624 or #1082. Move the staff response and NotificationInbox changes to a separate pull request, or link them to explicit requirements.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the primary Manage Timings gating and group-event Unschedule changes.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/manage-timings-counterparty-gate

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@dosubot

dosubot Bot commented Aug 1, 2026

Copy link
Copy Markdown

📄 Knowledge review

Dosu skipped reviewing this PR because your organization has used its 200 included credits for the month. Your usage will reset on 2026-09-01. To have Dosu review this PR before then, ask your organization admin to upgrade to a pro account.


Leave Feedback Ask Dosu about familiarise_web Add Dosu to your team

Gating Manage Timings on whether anyone has committed to a time also took
Reschedule off webinars and classes. That was right — for a group event
that route never opened a proposal, it only marked every slot tentative
and handed the instance back to the allocate queue — but it removed a
real capability along with the wrong label.

Adds `allowsUnschedule` beside the other two predicates. It is orthogonal
to them, not a third branch: a confirmed webinar offers Timings AND
Unschedule, a 1:1 offers Reschedule and never Unschedule, and the "never
both, never neither" property still holds within the Timings/Reschedule
pair. The action routes at the existing reschedule endpoint rather than a
parallel implementation, so the behaviour is unchanged and only its name,
its gate and its confirm copy are new.

Unschedule is emphatically not Cancel. The booking stays sold, attendees
stay enrolled, no refund is issued and no earnings, ledger row or
utilisation figure moves; the only thing withdrawn is the date. The
confirm dialog says all of that plainly and points at Cancel booking for
the other outcome, because for a webinar with thirty paid attendees the
difference is a new date versus refunding all of them.

Attendee notification was already correct on this path: the route's
post-transaction fan-out adds every user connected to the event's slots
(#624), and a group buyer is connected to every slot of every session, so
enrolled attendees are told the date has been withdrawn.

Part of #1082
@teetangh teetangh changed the title fix(appointments): gate Manage Timings on whether anyone has committed to a time fix(appointments): gate Manage Timings on commitment, and give group events a real Unschedule Aug 1, 2026
…m to"

The consultant's inbox rendered "rescheduled ... from&nbsp;&nbsp;to". The
payload type declared `oldDateTime?` and `newDateTime?`, the reschedule route
passed neither, and the template interpolated both anyway.

Passing two values would not have fixed it, because the sentence is wrong for
the common case. A plain release has no destination — that is the entire point
of "Any time works": the slots go back to the consultant's queue and no new
time exists yet. Only an auto-confirmed proposal actually moved anything, and
between those two sits a third case the route already distinguishes in its own
response message, a proposal that has been made and is waiting to be answered.

So the payload now carries `outcome`, one of MOVED, PROPOSED or RELEASED, and
the two arms are a union rather than optional fields: MOVED and PROPOSED cannot
be constructed without both times, which makes the blank-blank payload a
compile error rather than a rendering accident. This is the idiom the org
workflows already use — `reminderStage` on the dunning notice and `kind` on the
payout failure both drive their copy from one workflow id.

`rescheduleNotificationVariant` in the policy module derives the variant, so the
route stays a caller and the rule is unit-testable. The released time is
captured inside the transaction because an auto-confirm deletes those slot rows
and writes new ones — by the time the notification is assembled the time being
given up exists nowhere else.

The template itself lives in Novu's dashboard and cannot be changed from here.
Until it branches on `outcome`, the moved case renders real times where it
rendered blanks, and the released case still renders the wrong sentence. The
Novu-side change is spelled out in the PR body.

An audit of every optional field on every payload in lib/novu found the same
shape once more: `SupportTicketPayload.respondedBy` is declared and never
passed, so a template naming the responder rendered an empty attribution. It is
passed now. Three more are declared and never passed but need a query or a loop
change to supply, and are listed in the PR body rather than fixed here.
The notification panel covered the Friday and Saturday columns of the calendar
at a normal laptop width. Two things put it there. It was Novu's own bundled
popover, fixed at 400px on a bespoke z-index of 9999 with no collision
handling; and its open state belonged to Novu, so clicking a notification
routed the user to the page the notification was about and then stayed open on
top of it. The panel a consultant saw over their calendar was usually the one
that had just sent them there.

Given children, `Inbox` drops to being a provider — `Bell` and `InboxContent`
are the composition parts — so the panel becomes ours to place. It is now the
repo's Radix popover from components/ui, which brings collision-aware
placement, a viewport-capped size, Escape and outside-click dismissal, and the
shared z-layer instead of a number picked to beat everything else on the page.
Because the open state is ours, the panel closes before routing.

It is also narrower: 22rem against the old 25rem, capped at the viewport on
both axes, which leaves more of the grid legible while it is open.

Presentational only. The subscriber, the ADR 23 scope tabs, the appearance
variables and the click-to-route behaviour are unchanged; the two appearance
keys that styled Novu's popover are dropped because that popover no longer
renders, and `bellContainer` with them, since a custom `renderBell` has always
bypassed the container it styled.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@lib/appointments/slots.ts`:
- Around line 62-90: Update allowsManageTimings so CONSULTATION and SUBSCRIPTION
return false when any slot is confirmed, rather than checking only
slots[0]?.isTentative; preserve the existing empty-slot and WEBINAR/CLASS
behavior. Add coverage for slots ordered with a tentative or RESCHEDULED slot
before a later confirmed slot.
🪄 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: 4a9c9082-9f15-4a0a-b3fb-0047174c5027

📥 Commits

Reviewing files that changed from the base of the PR and between 9749b68 and 90acda5.

📒 Files selected for processing (12)
  • __tests__/booking-algorithm/manage-timings-affordance.test.ts
  • __tests__/booking-algorithm/reschedule-proposals.test.ts
  • app/api/appointments/[appointmentId]/reschedule/route.ts
  • app/api/staff/support-tickets/[ticketId]/responses/route.ts
  • app/dashboard/consultant/[consultantId]/(features)/appointments/ConsultantAppointmentsAdapter.tsx
  • app/dashboard/consultant/[consultantId]/(features)/appointments/[appointmentId]/timings/page.tsx
  • app/dashboard/consultant/[consultantId]/(features)/appointments/components/useConsultantEventActions.ts
  • components/appointments/UnscheduleConfirmationDialog.tsx
  • components/notifications/NotificationInbox.tsx
  • lib/appointments/slots.ts
  • lib/booking/reschedule-proposals.ts
  • lib/novu/workflows.ts

Comment thread lib/appointments/slots.ts
teetangh and others added 4 commits August 1, 2026 14:18
A partial reschedule releases one session of a multi-session booking and
leaves the rest confirmed. The released slot can sort first, so reading
only `slots[0].isTentative` saw "tentative", concluded nobody had
committed, and handed the consultant Manage Timings — the unilateral
surface — for a booking whose later sessions the consultee still holds.

The existing in-flight test happened to put the released slot second,
which is why it passed. The new case puts it first, and fails against
the old predicate.

Part of #1082
#1067 rewrote slotTimes to take the looser SessionSlotLike so the join
surfaces can pass planner rows and MeetingSlot; that signature wins. The
three affordance helpers this branch adds are additive alongside it.

SlotLike is no longer imported: its only use here was the slotTimes
signature #1067 replaced, and an unused import fails Sonar.

Co-authored-by: Cursor <cursoragent@cursor.com>
@sonarqubecloud

sonarqubecloud Bot commented Aug 1, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant