feat(notifications): scope notifications by org-ness (ADR 23) - #1051
Merged
Conversation
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>
✅ Deploy Preview for familiarise ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📄 Knowledge reviewDosu skipped reviewing this PR because your organization has used its
|
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughWalkthroughIntroduces organization-aware Novu notification scope and dashboard links, extends subscriber routing and organization preferences, adds Inbox filtering, synchronizes subscribers from dashboard shells, updates notification triggers, and adds ADR/security validation. ChangesNotification scope and routing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant NotificationTrigger
participant notificationScope
participant notificationHref
participant Novu
participant NotificationInbox
NotificationTrigger->>notificationScope: organizationId
notificationScope-->>NotificationTrigger: scope fields
NotificationTrigger->>notificationHref: notification surface
notificationHref-->>NotificationTrigger: dashboard URL
NotificationTrigger->>Novu: scoped notification payload
Novu-->>NotificationInbox: notification feed
NotificationInbox->>Novu: scope-filtered Inbox view
Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/novu/workflows.ts (1)
172-180: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFix the remaining org completion deep links.
The new scope contract is paired with a raw
dashboardUrl, and the supplied completion triggers still combinenotificationScope(organizationId)with a bare/dashboard. For org appointments, that sends recipients back to their personal tree. Update both consultation and subscription completion notifications inscripts/appointments/auto-complete-appointments.tsto usenotificationHref(organizationId, "appointments").🤖 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/novu/workflows.ts` around lines 172 - 180, Update the consultation and subscription completion notification builders in auto-complete-appointments.ts to replace the organization-scoped bare dashboard links with notificationHref(organizationId, "appointments"). Preserve the new raw dashboardUrl payload contract while ensuring both org appointment completion flows link to the appointments destination.
🤖 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/novu-payload-allowlist.test.ts`:
- Around line 67-75: Strengthen the recording notification test around
notifyRecordingAvailable so it verifies recipient IDs actually derive from
getEventAttendeeIds, rather than only checking helper-name presence. Capture the
notifier’s recipient argument in a behavioral test, or structurally assert the
attendee resolver’s result is passed through, while retaining the protection
against roster-based recipients.
In `@app/api/slots/request-for-approval/route.ts`:
- Around line 243-259: Propagate the consultation plan’s organization ID into
the nested appointment creation, then update the notification block around
requestOrgId to read that persisted appointment organizationId. Keep
notificationScope and scopedHref based on the persisted value so
organization-hosted requests retain organization routing and links.
In `@app/dashboard/organization/`[orgId]/layout.tsx:
- Around line 442-447: Update the mobile navigation configuration, specifically
MOBILE_TABS, to remove the settings.manage requirement for the Settings entry so
ordinary LEARNER and EXPERT members can reach Settings and its Notifications tab
on mobile. Preserve the existing per-tab gating behavior.
In `@components/notifications/NotificationInbox.tsx`:
- Around line 20-29: Update the memberships useMemo mapping to validate
organizationId and organizationName as non-empty strings before constructing
OrgMembershipLite entries. Discard memberships with missing or non-string fields
instead of coercing values with String, while preserving valid membership IDs
and names.
In `@lib/novu/subscriber.ts`:
- Around line 49-59: Configure the Novu workflow conditions in
lib/novu/subscriber.ts at lines 49-59 so channel delivery honors the
subscriber’s routingBell and routingEmail fields. Also update each ORG_*
workflow condition at lines 117-120 to use its corresponding categoryOrg* flag,
ensuring these preferences affect delivery.
In `@lib/payments/webhooks/handlers.ts`:
- Around line 809-810: Handle rejected detached Novu notification promises
consistently: in lib/payments/webhooks/handlers.ts at lines 809-810 and 826, add
explicit rejection handling to notifyPaymentSuccess and the booking
notification, preferably with post-transaction error reporting; in
lib/moderation/cancel-user-engagements.ts at lines 417-424, record or retry
cancellation-notification failures instead of treating cancellation as fully
successful.
- Around line 798-806: Verify the deployed Novu PAYMENT_SUCCESS and
APPOINTMENT_BOOKED workflows consume the organization scope and routing metadata
emitted by notificationScope and notificationHref. Update their conditions and
subscriber routing settings to use the new contract, and confirm both workflows
route organization-hosted and B2C notifications correctly before release.
In `@prisma/schema.prisma`:
- Around line 500-507: Add and commit a Prisma migration for the new
orgBillingAlerts, orgMembershipAlerts, and orgProgramAlerts fields on the
affected preferences model, matching their Boolean types and true defaults.
Ensure the migration is included alongside the schema change so
/api/novu/preferences can read and upsert these columns after deployment.
In `@scripts/appointments/auto-complete-appointments.ts`:
- Line 29: Replace the completion notification dashboard URLs with
notificationHref using the appointment’s organizationId and the "appointments"
destination, including the notification paths around lines 297-304 and 428-435.
Update the relevant notification payloads while retaining notificationScope and
all other existing behavior.
In `@scripts/appointments/detect-consultant-no-shows.ts`:
- Around line 251-257: Extend the RefundPayload type to include
NotificationScope fields, then update the no-show refund payload in
notifyAppointmentCancelled to spread notificationScope(noShowOrgId). Preserve
the existing dashboardUrl and refund data while ensuring notifyRefundProcessed
receives the organization scope for routing and inbox filtering.
---
Outside diff comments:
In `@lib/novu/workflows.ts`:
- Around line 172-180: Update the consultation and subscription completion
notification builders in auto-complete-appointments.ts to replace the
organization-scoped bare dashboard links with notificationHref(organizationId,
"appointments"). Preserve the new raw dashboardUrl payload contract while
ensuring both org appointment completion flows link to the appointments
destination.
🪄 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: 49419ef3-19c7-4582-bf7b-86ade0120ee0
📒 Files selected for processing (27)
__tests__/security/novu-payload-allowlist.test.tsactions/maintenance/freeze-appointments.tsapp/api/appointments/[appointmentId]/cancel/route.tsapp/api/appointments/[appointmentId]/reschedule/route.tsapp/api/novu/preferences/route.tsapp/api/novu/subscriber/route.tsapp/api/slots/request-for-approval/route.tsapp/dashboard/org-workspace/[orgWorkspaceId]/OrgWorkspaceShell.tsxapp/dashboard/org-workspace/[orgWorkspaceId]/settings/components/NotificationRoutingSection.tsxapp/dashboard/organization/[orgId]/layout.tsxapp/dashboard/organization/[orgId]/settings/SettingsTabs.tsxcomponents/notifications/NotificationInbox.tsxcomponents/notifications/NotificationPreferencesPanel.tsxdocs/enterprise/70-design-decisions/00-README.mddocs/enterprise/70-design-decisions/23-notification-scope.mdlib/moderation/cancel-user-engagements.tslib/novu/index.tslib/novu/resolve-href.tslib/novu/subscriber.tslib/novu/workflows.tslib/payments/webhooks/handlers.tslib/stream/recording-handlers.tsprisma/schema.prismaschemas/user.tsscripts/appointments/auto-complete-appointments.tsscripts/appointments/detect-consultant-no-shows.tsscripts/appointments/send-appointment-reminders.ts
Comment on lines
+49
to
+59
| data: { | ||
| routingMode: data.routingMode ?? "BELL_AND_EMAIL", | ||
| routingBell: | ||
| data.routingMode === "BELL_AND_EMAIL" || | ||
| data.routingMode === "BELL_ONLY" || | ||
| data.routingMode === undefined, | ||
| routingEmail: | ||
| data.routingMode === "BELL_AND_EMAIL" || | ||
| data.routingMode === "EMAIL_ONLY" || | ||
| data.routingMode === undefined, | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Configure Novu workflow conditions before rollout.
The PR objectives state these conditions are still absent, so these subscriber fields are currently inert and routing/category preferences will not affect delivery.
lib/novu/subscriber.ts#L49-L59: configure relevant workflow channel conditions to honorroutingBellandroutingEmail.lib/novu/subscriber.ts#L117-L120: configure eachORG_*workflow to honor its matchingcategoryOrg*flag.
📍 Affects 1 file
lib/novu/subscriber.ts#L49-L59(this comment)lib/novu/subscriber.ts#L117-L120
🤖 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/novu/subscriber.ts` around lines 49 - 59, Configure the Novu workflow
conditions in lib/novu/subscriber.ts at lines 49-59 so channel delivery honors
the subscriber’s routingBell and routingEmail fields. Also update each ORG_*
workflow condition at lines 117-120 to use its corresponding categoryOrg* flag,
ensuring these preferences affect delivery.
Comment on lines
+798
to
+806
| const orgId = appointmentForNotif?.organizationId ?? null; | ||
| const scope = notificationScope( | ||
| orgId, | ||
| appointmentForNotif?.organization?.name, | ||
| ); | ||
| // Org-hosted → the org route, which is right for every recipient of the | ||
| // batched trigger below. B2C → the bare /dashboard router bounce, because | ||
| // consultant and consultee land in different personal trees. | ||
| const dashboardUrl = notificationHref(orgId, "appointments"); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔵 Trivial
Verify the deployed Novu workflows consume the new routing contract.
This code now emits organization scope and routing metadata, but the PR notes that workflow conditions and subscriber routing settings still require configuration. Confirm the PAYMENT_SUCCESS and APPOINTMENT_BOOKED workflows consume these fields before release.
🤖 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/payments/webhooks/handlers.ts` around lines 798 - 806, Verify the
deployed Novu PAYMENT_SUCCESS and APPOINTMENT_BOOKED workflows consume the
organization scope and routing metadata emitted by notificationScope and
notificationHref. Update their conditions and subscriber routing settings to use
the new contract, and confirm both workflows route organization-hosted and B2C
notifications correctly before release.
Comment on lines
+500
to
+507
| // Org category preferences (ADR 23). The seven categories above are all | ||
| // B2C-shaped, so the entire ORG_* workflow family was unmutable — an org | ||
| // OWNER could not turn off invoice dunning. Split three ways rather than one | ||
| // "org" switch because the audiences differ: an operator wants billing but | ||
| // not every roster change, an EXPERT wants delivery but no invoices at all. | ||
| orgBillingAlerts Boolean @default(true) // invoices, wallet, payouts, overages | ||
| orgMembershipAlerts Boolean @default(true) // invites, roster + role changes | ||
| orgProgramAlerts Boolean @default(true) // caps, exhaustion, renewals |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Add and commit the Prisma migration before merge.
The PR objectives confirm this migration is unapplied. /api/novu/preferences reads and upserts this model, so deployed code will query/write columns that do not exist yet.
🤖 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 `@prisma/schema.prisma` around lines 500 - 507, Add and commit a Prisma
migration for the new orgBillingAlerts, orgMembershipAlerts, and
orgProgramAlerts fields on the affected preferences model, matching their
Boolean types and true defaults. Ensure the migration is included alongside the
schema change so /api/novu/preferences can read and upsert these columns after
deployment.
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>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
__tests__/security/novu-payload-allowlist.test.ts (1)
101-106: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winInclude
RefundPayloadin the scoped-payload contract test.
RefundPayloadnow extendsNotificationScopeinlib/novu/workflows.ts, but this list omits it, leaving the refund routing/filtering contract unguarded.Proposed fix
const SCOPED_PAYLOADS = [ "AppointmentPayload", "PaymentSuccessPayload", + "RefundPayload", "BookingRequestPayload", "RecordingPayload", ];🤖 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/novu-payload-allowlist.test.ts` around lines 101 - 106, Update the SCOPED_PAYLOADS fixture in the novu payload allowlist contract test to include “RefundPayload”, ensuring the scoped-payload coverage also validates refund routing and filtering.
🤖 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.
Outside diff comments:
In `@__tests__/security/novu-payload-allowlist.test.ts`:
- Around line 101-106: Update the SCOPED_PAYLOADS fixture in the novu payload
allowlist contract test to include “RefundPayload”, ensuring the scoped-payload
coverage also validates refund routing and filtering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ff4e13ea-d686-4bf8-be71-b69ec92e597c
📒 Files selected for processing (10)
__tests__/security/novu-payload-allowlist.test.tsapp/api/webhooks/utils.tsapp/dashboard/organization/[orgId]/layout.tsxcomponents/notifications/NotificationInbox.tsxlib/novu/workflows.tslib/payments/webhooks/handlers.tsprisma/schema.prismascripts/appointments/auto-complete-appointments.tsscripts/appointments/detect-consultant-no-shows.tsscripts/refunds/reconcile-pending-refunds.ts
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>
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/dashboard/organization/[orgId]/layout.tsx (1)
450-468: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the stale Settings permission comment.
Lines 451-457 still say Settings remains MAINTAINER+-only, while lines 463-468 intentionally make the entry reachable by every active member. Replace the old comment so the navigation contract is unambiguous and future changes do not reintroduce the removed gate.
🤖 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 `@app/dashboard/organization/`[orgId]/layout.tsx around lines 450 - 468, Update the comment above the Settings entry in configurationItems to remove the stale MAINTAINER+-only claim and clearly state that active members may reach Settings, with access controlled by each tab’s individual gate. Keep the navigation and permission behavior unchanged.
🤖 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 `@docs/enterprise/50-operations/09-novu-console-conditions.md`:
- Around line 11-15: Update the runbook to make Novu console condition
configuration an explicit release prerequisite. Require verification of routing
conditions, organization category mappings, and end-to-end preference checks
before deployment, while preserving the documented behavior that switches remain
permissive until configured.
---
Outside diff comments:
In `@app/dashboard/organization/`[orgId]/layout.tsx:
- Around line 450-468: Update the comment above the Settings entry in
configurationItems to remove the stale MAINTAINER+-only claim and clearly state
that active members may reach Settings, with access controlled by each tab’s
individual gate. Keep the navigation and permission behavior unchanged.
🪄 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: cabac9c2-022a-47e2-baa8-4af4f1e16857
📒 Files selected for processing (4)
app/dashboard/organization/[orgId]/layout.tsxdocs/enterprise/50-operations/09-novu-console-conditions.mddocs/enterprise/70-design-decisions/23-notification-scope.mdprisma/schema.prisma
Comment on lines
+11
to
+15
| [ADR 23](../70-design-decisions/23-notification-scope.md) made notifications carry their organization scope and made the organization preference categories writable. The application half of that is complete: every field below is written to the Novu subscriber record by `POST /api/novu/subscriber` and `PUT /api/novu/preferences`. | ||
|
|
||
| The other half lives in the Novu console and cannot be done from the repository. Until the conditions in this document exist, **the preference switches save, display and read back correctly but do not gate delivery** — a member who turns off billing alerts still receives them. Nothing regresses in the meantime, because the default for every flag is permissive; the switches are simply inert. | ||
|
|
||
| This document exists so that work is mechanical rather than reverse-engineered from the code. Work through it once and the feature is complete. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Treat Novu console conditions as a release prerequisite.
This runbook states the preference and routing switches remain inert until the Novu workflow conditions are configured, so verify the routing conditions, organization category mappings, and end-to-end checks before deployment; otherwise users can disable notifications but still receive them.
🧰 Tools
🪛 LanguageTool
[style] ~13-~13: ‘in the meantime’ might be wordy. Consider a shorter alternative.
Context: ... still receives them. Nothing regresses in the meantime, because the default for every flag is ...
(EN_WORDINESS_PREMIUM_IN_THE_MEANTIME)
🤖 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 `@docs/enterprise/50-operations/09-novu-console-conditions.md` around lines 11
- 15, Update the runbook to make Novu console condition configuration an
explicit release prerequisite. Require verification of routing conditions,
organization category mappings, and end-to-end preference checks before
deployment, while preserving the documented behavior that switches remain
permissive until configured.
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.
Context
ADR 19 split the dashboards by the org-ness of the underlying session, plan or payment. Every read path learned the rule — scoped list helpers, the chat channel query, appointment feeds, money views. The notification layer learned none of it.
An audit found the split invisible end to end:
User.id— never per profile or per org. No topics, no tags.organizationId. The single occurrence anywhere underlib/novu/was a Prismawhereclause.ORG_*payloads carried a display-onlyorgName; the payloads that fire in both contexts carried no discriminator at all.NotificationInboxrendered with notabsand nofilter.So a consultant who also delivers for an org received one merged feed, rendered identically on every dashboard, in which an org-hosted booking was byte-for-byte indistinguishable from a B2C one.
Full reasoning in the new ADR 23.
Scope on the payload
Dual-context payloads now compose a
NotificationScope(organizationId, a derivedscope, optionalorgName). The fields are required rather than optional, so a trigger site that forgets to attribute its notification fails the build instead of quietly emitting another unattributable one — adding the type flushed out 13 call sites, which is a fair measure of how far the drift had spread.scopeis derivable fromorganizationIdand stored anyway: Novu filters Inbox 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 how a notification is filed and where it points. It does not widen who receives it; recipient lists are untouched.
Deep links resolve to the owning tree
One constraint shapes the answer: several workflows trigger once for many recipients with a single payload, so one href must be right for all of them.
/dashboard, deliberately. Consultant and consultee have different personal dashboards, and the capability router already resolves the right one 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:
deriveTransactionIdhashes 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 blanket switch, because the audiences genuinely differ: an operator wants invoices but not every roster change, an EXPERT wants delivery notices and no invoices at all. The seven existing categories were all B2C-shaped, which left the entire
ORG_*family unmutable (an org OWNER could not turn off invoice dunning).Surfaced on a Notifications tab in org Settings, which exposed another gate/page disagreement: the Settings 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. The entry is now ungated to match the page, and
UrlTabsshows each role only the tabs it holds.notificationRoutingModeis honoured rather than deleted. It is pushed onto the subscriber asdata.routingModeplus channel booleans, the same mechanism the category flags already use. Its component docstring claimedlib/novu/org-workflows.tsread the column; nothing ever did — an operator choosing EMAIL_ONLY still got bell notifications and was told the setting saved.Both org trees now sync the subscriber. A user onboarded straight into an org by invite was never POSTed to
/api/novu/subscriber, so their record stayed bare and templates interpolatingsubscriber.firstNamedegraded.ADR 20 gains a test it never had
RecordingPayload.recordingUrlputs a live media URL in a notification body. Nothing leaked, but only because the recipient list came fromgetEventAttendeeIdsrather than a roster — which is precisely the "accident of implementation" ADR 20 exists to stop. A future change widening that list torosterForOrg(orgId, VISIBILITY_ROLES)would have leaked it with no test failing.__tests__/security/novu-payload-allowlist.test.tsnow pins both halves: content-bearing payloads reach only participant-derived recipients, and org-roster dispatchers carry no content field.A bug this PR introduced and caught
Importing the pure helpers through the
@/lib/novubarrel turned 100 booking-algorithm tests from 200s into 500s. Suites mock that barrel to keep notifications off the wire, so a helper pulled through it resolves toundefinedand throws at the call site. I baselined those tests on cleandevto confirm they passed beforehand, then fixed it by importing from./workflowsand./resolve-hrefdirectly. There is a note in the barrel so nobody moves them back.Verification
tscclean;eslintcleandevNotificationPreference. The shared dev DB is shared with prod and still carries thePaymentGatewaydrift, so this needs a surgical additive-only migration, never a blanketdb push.categoryOrgBilling,categoryOrgMembership,categoryOrgProgram,routingBell,routingEmail) are written correctly, but only take effect once the Novu workflow conditions read them. Until then the org toggles and routing mode save and display but do not gate delivery. Same mechanism the existing category flags use.Merge order
Overlaps #1050 in exactly one file (
app/dashboard/organization/[orgId]/layout.tsx) in distant hunks. Verified withgit merge-tree: no conflict in either order, and the merged file carries both branches' changes.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests