release: dev → prod — org catalog, plan archiving, notification scoping - #1056
Merged
Conversation
…xt roll-up (#1050) * feat(dashboards): org-owned catalog, consultant nav trim, cross-context roll-up Applies ADR 19's rules to the two places the July pass left alone, and gives `canHost` organizations a way to own the offerings they sell. Org Catalog. #778 collapsed the standalone OrganizationPlan table into the per-type plans, which left "an org's catalog" defined as its WebinarPlan / ClassPlan rows carrying an organizationId. Nothing ever wrote that column: the consultant planner hardcoded it to null, there was no /catalog route, and CATALOG_PLAN_CREATED / CATALOG_PLAN_DEACTIVATED sat in the audit taxonomy with no emitter. A canHost org could pay experts but could not own anything they delivered. Adds the surface, gated on canHost plus a new catalog.manage key on the operator tier. Only Webinar and Class appear — ConsultationPlan and SubscriptionPlan declare a required consultantProfileId, so an org can never solely own one. Authoring is an operator act because an org-owned plan commits the org's revenue and payout obligation; the EXPERT is named as deliverer and re-checked server-side against an ACTIVE membership rather than trusted from the form. The two planner forms moved to components/planner/ so both surfaces author from one implementation instead of two that drift, and take organizationId as a prop. usePlanner followed (single consumer) and IPlanMaterial moved to types/planner-events.ts, whose own header explains why components/ must not reach into app/. Consultant nav, 16 entries to 14. Trials is a tab on Appointments for the reason the org sidebar already folded it there — a trial IS an appointment. Analytics is a tab on Earnings: it read the same /api/consultant/earnings endpoint and differed only by ?includeMonthly=1. The shared AppointmentsShell grew an opt-in extraTabs prop rather than a sixth hardcoded bucket, because the consultee shares it and has no trials concept. Earnings became a server component so the Analytics panel keeps the SSR prefetch it had as its own page, and with it the ownership guard. Needs-you card. ADR 19 accepts multi-dashboard cost and fixes the only sanctioned remedy: a derived read, never a re-merge. Returns counts and hrefs only, copies its scope predicates verbatim from the endpoints behind the Requests page, and hides itself when the only context is personal. Two defects found in passing. The org Requests nav gate tested exact-role EXPERT while the page admits anyone holding a consultantProfileId, so an OWNER who also delivers had a reachable page with no way to reach it. And the personal Messages badge read Stream's global total_unread_count while its inbox filters to channels without an organization_id, so an org message lit a badge that led nowhere; both now test what the destination tests. Test-only edits repoint three assertions at moved paths and add the new nav path to the org list — each caught a real break rather than being loosened. No schema change: everything the catalog needs already exists on the plan models. Note DELETE is a hard delete refusing at 409 once sessions exist, because neither model has an isActive column; soft-archiving needs a field and belongs in the pre-launch freeze. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf(catalog): render one tab body, not both Switching Webinars <-> Classes was visibly slow. UrlTabs writes the active tab with router.replace, and Next 15's App Router re-renders the tree via useSearchParams reactivity even with { scroll: false } — a lesson this repo already learned and documented at app/explore/hooks/useUrlSyncedFilters.ts:34, where the explore filters abandoned router.replace for window.history. replaceState. UrlTabs never got that migration. That router-level cause is filed separately (it is shared with Members, Settings and ScopeToggle, and its ?page= reset semantics depend on the re-render it would remove, so it needs its own change). This removes the amplifier that made the catalog the worst-hit consumer. CatalogClient passed inline renderList(...) calls as tab content, so every re-render rebuilt BOTH kinds' full row sets, columns array and cell closures — unpaginated, from one combined fetch. MembersTabs and SettingsTabs pass component references instead, so the same re-render only reconciles a cheap element descriptor and Radix mounts one panel. Catalog now does the same: CatalogPanel owns the table, columns are memoized, and the per-kind remove callbacks are stable so that memo is not invalidated on every parent render. Shared row/response types move to ./types so the panel and the client agree without one importing the other's internals. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(dashboards): org-owned catalog, consultant nav trim, cross-context roll-up Applies ADR 19's rules to the two places the July pass left alone, and gives `canHost` organizations a way to own the offerings they sell. Org Catalog. #778 collapsed the standalone OrganizationPlan table into the per-type plans, which left "an org's catalog" defined as its WebinarPlan / ClassPlan rows carrying an organizationId. Nothing ever wrote that column: the consultant planner hardcoded it to null, there was no /catalog route, and CATALOG_PLAN_CREATED / CATALOG_PLAN_DEACTIVATED sat in the audit taxonomy with no emitter. A canHost org could pay experts but could not own anything they delivered. Adds the surface, gated on canHost plus a new catalog.manage key on the operator tier. Only Webinar and Class appear — ConsultationPlan and SubscriptionPlan declare a required consultantProfileId, so an org can never solely own one. Authoring is an operator act because an org-owned plan commits the org's revenue and payout obligation; the EXPERT is named as deliverer and re-checked server-side against an ACTIVE membership rather than trusted from the form. The two planner forms moved to components/planner/ so both surfaces author from one implementation instead of two that drift, and take organizationId as a prop. usePlanner followed (single consumer) and IPlanMaterial moved to types/planner-events.ts, whose own header explains why components/ must not reach into app/. Consultant nav, 16 entries to 14. Trials is a tab on Appointments for the reason the org sidebar already folded it there — a trial IS an appointment. Analytics is a tab on Earnings: it read the same /api/consultant/earnings endpoint and differed only by ?includeMonthly=1. The shared AppointmentsShell grew an opt-in extraTabs prop rather than a sixth hardcoded bucket, because the consultee shares it and has no trials concept. Earnings became a server component so the Analytics panel keeps the SSR prefetch it had as its own page, and with it the ownership guard. Needs-you card. ADR 19 accepts multi-dashboard cost and fixes the only sanctioned remedy: a derived read, never a re-merge. Returns counts and hrefs only, copies its scope predicates verbatim from the endpoints behind the Requests page, and hides itself when the only context is personal. Two defects found in passing. The org Requests nav gate tested exact-role EXPERT while the page admits anyone holding a consultantProfileId, so an OWNER who also delivers had a reachable page with no way to reach it. And the personal Messages badge read Stream's global total_unread_count while its inbox filters to channels without an organization_id, so an org message lit a badge that led nowhere; both now test what the destination tests. Test-only edits repoint three assertions at moved paths and add the new nav path to the org list — each caught a real break rather than being loosened. No schema change: everything the catalog needs already exists on the plan models. Note DELETE is a hard delete refusing at 409 once sessions exist, because neither model has an isActive column; soft-archiving needs a field and belongs in the pre-launch freeze. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(catalog): archive org plans instead of deleting them The catalog shipped with a hard delete that refused with 409 the moment any session existed, which meant a plan that had ever been booked could never leave the catalog. The 409 was load-bearing rather than cautious: the plan foreign keys are onDelete: Cascade at every hop — WebinarPlan -> Webinar -> Appointment -> Payment so deleting one catalog row would physically destroy the sessions booked from it and their payment records. The row is also the TERMS of every sale made against it; past appointments, invoices and earnings resolve title, price and duration by reading it. Deleting it does not just remove a listing, it removes what was sold. So archiving is the primitive. `archivedAt` on WebinarPlan and ClassPlan withdraws a plan from booking and discovery, keeps every reference resolvable, and reverses. The endpoint archives and restores; the UI grows an Archive / Restore action and a separate Archived view that stays hidden until something has been withdrawn. There is deliberately NO hard-delete path left, not even for a plan nothing has referenced. One code path cannot trip the cascade; two invite a later edit that misses a reference — materials, collaborators — and turns a catalog button into a history shredder. An unused plan sitting archived costs nothing. Discovery needed a second helper rather than an extension of the existing one. `marketplaceVisibilityWhere` is also spread into ConsultationPlan and SubscriptionPlan queries, and those models have no archivedAt, so attaching the filter there would make Prisma reject them at runtime. The two gates now sit side by side in visibility.ts for the auditability reason that file's header already gives, and `buildPlanWhereClause` — which serves only the webinar and class routes — carries the filter directly. The test pins the premise as well as the conclusion: that the endpoint holds no delete call, that discovery filters archived rows, that the two models WITHOUT the column keep the plain filter, and that the cascade being guarded against is still declared. If someone relaxes those relations to SetNull the archive-only rule is worth revisiting, and the test will say so. Stacked on #1050 — the catalog surface it modifies lands there. Schema adds one nullable column per model; the migration is NOT applied. Part of #1050 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(catalog): pin the ORG_ONLY and archive discovery guards #726 shipped with no test at all. That mattered less while nothing could actually produce an ORG_ONLY row — the org catalog is the first surface that can, so the guard is now load-bearing and gets pinned. Behavioural rather than source-level: these call the real buildPlanWhereClause and the two visibility helpers and assert the where object actually produced. A grep would pass if the filter were built and then dropped before the query. Also covers the case where caller-supplied query params might dislodge the gate, and asserts the two guards are independent — a plan can be perfectly PUBLIC and still withdrawn, so visibility does not subsume archived. Verified by injecting both regressions (dropping archivedAt from the builder, admitting ORG_ONLY to the marketplace constant): 5 of 7 fail, clean on restore. Deliberately not end-to-end. Proving 'an ORG_ONLY plan stays off the marketplace' against the live database means creating one there first, and if the guard is broken — the case the test exists to catch — that plan is briefly live on the public marketplace. The failure mode of the experiment is the incident it is meant to prevent. Part of #1050 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf(catalog): render one tab body, not both Switching Webinars <-> Classes was visibly slow. UrlTabs writes the active tab with router.replace, and Next 15's App Router re-renders the tree via useSearchParams reactivity even with { scroll: false } — a lesson this repo already learned and documented at app/explore/hooks/useUrlSyncedFilters.ts:34, where the explore filters abandoned router.replace for window.history. replaceState. UrlTabs never got that migration. That router-level cause is filed separately (it is shared with Members, Settings and ScopeToggle, and its ?page= reset semantics depend on the re-render it would remove, so it needs its own change). This removes the amplifier that made the catalog the worst-hit consumer. CatalogClient passed inline renderList(...) calls as tab content, so every re-render rebuilt BOTH kinds' full row sets, columns array and cell closures — unpaginated, from one combined fetch. MembersTabs and SettingsTabs pass component references instead, so the same re-render only reconciles a cheap element descriptor and Radix mounts one panel. Catalog now does the same: CatalogPanel owns the table, columns are memoized, and the per-kind remove callbacks are stable so that memo is not invalidated on every parent render. Shared row/response types move to ./types so the panel and the client agree without one importing the other's internals. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(notifications): scope notifications by org-ness (ADR 23) ADR 19 split the dashboards by the org-ness of the underlying session, plan or payment. Every read path learned the rule; the notification layer learned none of it. One Novu subscriber per user, no organizationId on any payload — the single occurrence under lib/novu/ was a Prisma where clause — and an Inbox rendered with no tabs and no filter. A consultant who also delivers for an org got one merged feed in which an org booking was byte-identical to a B2C one. Scope. Payloads for work that happens in both contexts now compose a NotificationScope carrying organizationId, a derived scope, and an optional orgName. Required rather than optional so an unattributed trigger fails the build; adding the type flushed out 13 call sites, which measures how far the drift had spread. `scope` is derivable and stored anyway because Novu filters tabs by payload equality and "this field is null" is not expressible that way; one helper produces both so they cannot disagree. Attribution is not delivery — the tag changes filing and routing, never who receives. Deep links. Several workflows trigger once for many recipients with one payload, so a single href must suit all of them. Org work resolves to the org route, correct for every participant. B2C keeps the bare /dashboard deliberately: consultant and consultee have different personal trees and the capability router already picks per viewer. That bounce was never wrong in itself — it was wrong because org work used it too. Single-recipient triggers with a known side get a precise route instead. This also fixes transactionId collisions as a side effect: deriveTransactionId hashes the canonical payload, so two same-shaped events in different scopes previously collided and Novu dropped the second silently. Preferences. Three org categories (billing, membership, programs) rather than one switch, because an operator wants invoices but not roster churn while an EXPERT wants the reverse. The seven existing categories were all B2C-shaped, leaving the whole ORG_* family unmutable. Surfaced on a Notifications tab in org Settings, which exposed another gate/page disagreement: the page has always floored at active membership while the sidebar entry demanded an operator grant, so a LEARNER could reach Settings only by typing the URL. notificationRoutingMode is honoured rather than deleted — pushed onto the subscriber as data.routingMode plus channel booleans, the same mechanism the category flags use. Its component docstring claimed org-workflows.ts read the column; nothing ever did, and an operator choosing EMAIL_ONLY still got bell notifications and was told it saved. Both org trees now sync the Novu subscriber. A user onboarded straight into an org by invite was never POSTed to /api/novu/subscriber, so templates interpolating subscriber.firstName degraded. ADR 20 gains a test it never had. RecordingPayload.recordingUrl is a live URL in a notification body and nothing leaked only because the recipient list came from getEventAttendeeIds rather than a roster — the exact accident ADR 20 exists to stop. The new suite pins both halves. Note the pure helpers are imported from ./workflows and ./resolve-href directly, never through the lib/novu barrel: suites mock that barrel to keep notifications off the wire, and a helper pulled through a barrel mock is undefined at the call site. Caught by booking-algorithm turning 200s into 500s. Schema adds three additive boolean columns; the migration is NOT applied. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(notifications): address CodeRabbit review on #1051 Five of the ten inline findings held up against the current code. Mobile Settings was still gated. Ungating only the desktop sidebar left MOBILE_TABS carrying surface: "settings.manage", so a LEARNER or EXPERT on mobile had no route to the Notifications tab the previous commit added for them. Same fix, same reasoning, the half I missed. Validate membership fields instead of coercing them. String(someObject) yields "[object Object]", which would have become a tab filter matching nothing and a label rendering that literal. Malformed entries now drop out rather than producing a broken tab. This was also the SonarCloud gate failure. auto-complete-appointments kept ${getAppUrl()}/dashboard after gaining notificationScope, so its org-scoped completions still pointed at the personal dashboard — exactly the defect this branch exists to fix, left in one of the call sites that only got half the treatment. RefundPayload now carries scope. A refund inherits the org-ness of the payment it reverses (Payment.organizationId is the org tag). dashboardUrl stays a router bounce on all three sites deliberately: these go to the PAYER, and an org billing page is not readable by a LEARNER whose booking was sponsored. The ADR 20 guard test asserted that getEventAttendeeIds merely APPEARS, which would still have passed with the notifier handed a roster while the resolver sat unused elsewhere. It now binds the two — it takes the identifier actually passed as the recipient argument and requires that identifier to be the one assigned from the attendee resolver. Verified by injecting the exact bypass: the test fails with the resolver still present and used. Five findings did not survive verification and are left as-is: - "Persist organizationId before deriving scope" in request-for-approval: Appointment.organizationId is set at checkout from the sponsoring program's contract org (#674), not from the plan. Stamping it at request time conflates host-ownership with sponsorship and changes booking scope semantics platform-wide. Null at request time is accurate, and the personal Requests page filters on exactly that, so the link lands where the item actually is. - "Handle detached Novu failures": triggerWorkflow try/catches, reports to Sentry and returns a result object. It never rejects, so `void` cannot orphan a rejection. - "Add and commit the Prisma migration": there is no prisma/migrations directory — this repo applies schema changes by push plus surgical additive migrations. The real concern is deploy ordering, already stated in the PR. - Two findings restate the PR's own disclosure that Novu workflow conditions are console config still to be applied. No code change is possible. Part of #1051 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(notifications): Novu console conditions runbook The application half of ADR 23 is complete — every routing and category flag is written to the subscriber record. The console half cannot be done from the repository, and until it exists the preference switches save and read back correctly but do not gate delivery. This is the exact mapping: which subscriber.data key, which workflow slug, which step, and two end-to-end checks that prove the wiring. Written so the console work is mechanical rather than reverse-engineered from lib/novu. Records the two judgement calls in the mapping — the overage workflows file under billing because they address the member who owes money rather than the operator watching a cap, and the SSO workflows file under membership because they concern how people get into the org. Part of #1051 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
📄 Knowledge reviewDosu skipped reviewing this PR because your organization has used its
|
❌ Deploy Preview for familiarise failed. Why did it fail? →
|
|
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.
Release
dev→prod. Three PRs, all merged to dev and verified there.What ships
#1050 — org-owned catalog, consultant nav trim, cross-context roll-up
A
canHostorg could pay experts but could not own anything they delivered:WebinarPlan/ClassPlanhadorganizationIdandOrgPlanVisibilityin the schema, andCATALOG_PLAN_CREATEDin the audit taxonomy, but nothing ever wrote them. Adds the Catalog surface (gated oncanHost+ a newcatalog.manageoperator key), applies ADR 19's "a nav entry is a destination" rule to the consultant sidebar (Trials → Appointments tab, Analytics → Earnings tab, 16 entries to 14), and adds the "Needs you" cross-context roll-up. Also fixes two gate/page disagreements and the personal Messages badge, which counted org unreads its own inbox filters out.#1053 — archive org plans instead of deleting them
The catalog's delete refused with 409 once any session existed, so a plan that had ever been booked could never leave. That guard was load-bearing: the FK chain is
onDelete: CascadefromWebinarPlanthroughWebinarandAppointmenttoPayment, so a delete would destroy settled money records. Archiving withdraws a plan from booking and discovery while keeping every reference resolvable, and reverses. Also fixes earnings attribution — host earnings resolved the settling org from the expert's oldest membership, so Org B could sell a plan and Org A get paid.#1051 — notification scoping (ADR 23)
ADR 19 split the dashboards by org-ness; the notification layer never learned it. One subscriber per user, no
organizationIdon any payload, an Inbox with no filter — so a consultant who also delivers for an org got one merged, undifferentiated feed. Adds a requiredNotificationScopeto dual-context payloads (which flushed out 13 unattributed trigger sites), routes deep links to the owning dashboard, adds Inbox scope tabs, and makes the previously unmutableORG_*family configurable.Migration — already applied ✅
Five additive columns were applied to the shared database before these merged, via a surgical
apply_migration(never a blanketdb push):notification_preferences.orgBillingAlerts/orgMembershipAlerts/orgProgramAlerts—BOOLEAN NOT NULL DEFAULT trueWebinarPlan.archivedAt,ClassPlan.archivedAt—TIMESTAMP(3)nullableNo drops, no type changes, no backfill.
PaymentGatewayverified untouched (and is already narrowed to the four live values, so that old drift blocker is resolved).Verification
LintandTypeScript, Tests & BuildSUCCESS on all three/catalogby URL is redirected, and a sponsor-only org has no Catalog but keeps Programs/explore/programs,/api/plans/webinars,/api/plans/classesall 200 with dataKnown, tracked, not blocking
docs/enterprise/50-operations/09-novu-console-conditions.md.UrlTabsstill usesrouter.replace, forcing a tree re-render on tab switch across several surfaces. The catalog-specific amplifier was fixed in feat(dashboards): org-owned catalog, consultant nav trim, cross-context roll-up #1050; the shared root cause needs its own change because the?page=reset depends on the re-render it would remove.🤖 Generated with Claude Code