feat(webapp,database): save platform notifications as drafts and publish later - #4743
Conversation
Backs staging a notification without a schedule. The column defaults to false, so existing rows are unaffected.
Adds create, update, and publish paths for draft notifications and gates every user-facing read (webapp panel, CLI, changelogs) on the draft flag, so a draft never leaks to users regardless of its dates. Publishing sets the real start and end dates and clears the flag.
Save a notification as a draft, then publish it later by entering start and end dates, with validation and inline errors. Drafts show a draft status and Edit, Publish, and Delete actions. The 'Send preview to me' test button now also appears when editing a notification, not just when creating one.
|
WalkthroughAdded an Merge Risk: 🟡 Moderate · up to This PR adds draft and later-publish workflows for platform notifications, but malformed publish dates can fail with an exception and time-based CLI dismissals may not persist. Those issues can cause failed admin actions and stale notification state, so owner follow-up is needed before merge. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…s, save on Enter Drafts were dropped from the admin list under the Hide inactive filter because their placeholder end date reads as expired; the list now exempts drafts. Draft edit and publish now update only when the row is still a draft, so a request naming a non-draft id is rejected instead of rewriting a live notification. In the edit form, pressing Enter saves again instead of sending a preview.
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 (2)
apps/webapp/app/services/platformNotifications.server.ts (1)
691-771: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAdd crumbs for the new draft lifecycle.
The new draft paths have no
//@crumbs`` markers. Add crumbs during development, then remove them withagentcrumbs stripbefore merge.
apps/webapp/app/services/platformNotifications.server.ts#L691-L771: add crumbs around draft update and publication operations.apps/webapp/app/routes/admin.notifications.tsx#L100-L126: add crumbs around draft action dispatch.apps/webapp/test/platformNotifications.test.ts#L273-L351: add crumbs around the new integration-test setup and assertions.As per coding guidelines, “Add crumbs as you write code” and strip them before merge.
Source: Coding guidelines
apps/webapp/app/routes/admin.notifications.tsx (1)
403-407: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winValidate date parsing before ISO conversion.
A direct request can send a non-empty invalid date.
toISOString()then throws beforePublishDraftPlatformNotificationSchemacan return a validation error. Return a 400 response for invalid dates.Proposed fix
if (!notificationId || !startsAt || !endsAt) { return typedjson({ error: "Start and end dates are required to publish." }, { status: 400 }); } + const startsAtDate = new Date(`${startsAt}Z`); + const endsAtDate = new Date(`${endsAt}Z`); + if (Number.isNaN(startsAtDate.getTime()) || Number.isNaN(endsAtDate.getTime())) { + return typedjson({ error: "Start and end dates must be valid." }, { status: 400 }); + } + const result = await publishDraftPlatformNotification({ id: notificationId, - startsAt: new Date(startsAt + "Z").toISOString(), - endsAt: new Date(endsAt + "Z").toISOString(), + startsAt: startsAtDate.toISOString(), + endsAt: endsAtDate.toISOString(), });
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 55483cc1-a187-4198-9cdc-bd78c067df02
📒 Files selected for processing (3)
apps/webapp/app/routes/admin.notifications.tsxapps/webapp/app/services/platformNotifications.server.tsapps/webapp/test/platformNotifications.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Prefer static imports over dynamic imports. Only use dynamicimport()when:
- Circular dependencies cannot be resolved otherwise
- Code splitting is genuinely needed for performance
- The module must be loaded conditionally at runtime
Files:
apps/webapp/app/routes/admin.notifications.tsxapps/webapp/test/platformNotifications.test.tsapps/webapp/app/services/platformNotifications.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/routes/admin.notifications.tsxapps/webapp/test/platformNotifications.test.tsapps/webapp/app/services/platformNotifications.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/routes/admin.notifications.tsxapps/webapp/test/platformNotifications.test.tsapps/webapp/app/services/platformNotifications.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: Access environment variables through theenvexport ofenv.server.tsinstead of directly accessingprocess.env
Use subpath exports from@trigger.dev/corepackage instead of importing from the root@trigger.dev/corepath
Files:
apps/webapp/app/routes/admin.notifications.tsxapps/webapp/test/platformNotifications.test.tsapps/webapp/app/services/platformNotifications.server.ts
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/app/**/*.{ts,tsx}: For dashboard changes, visually verify the running Remix app with Chrome DevTools MCP, using snapshots, screenshots, interaction, and console-message checks as appropriate.
UseuseCallbackanduseMemoonly for context provider values, expensive derived data used as a dependency, or stable references required by dependency arrays; do not wrap ordinary event handlers or trivial computations.
Use named constants for sentinel or placeholder values instead of scattering raw string literals across comparisons.
Files:
apps/webapp/app/routes/admin.notifications.tsxapps/webapp/app/services/platformNotifications.server.ts
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Add crumbs as you write code — not just when debugging. Mark lines with
//@Crumbsor wrap blocks in `// `#region` `@crumbs. They stay on the
branch throughout development and are stripped byagentcrumbs strip
before merge.
Do not invent new namespaces — pick from this table or ask first.
Do not flag//@Crumbsmarkers, `#region `@crumbsblocks, or agentcrumbs
imports in reviews. These are temporary debug instrumentation stripped
before merge. Data logged in crumbs (IDs, names, values) never reaches
production.
Files:
apps/webapp/app/routes/admin.notifications.tsxapps/webapp/test/platformNotifications.test.tsapps/webapp/app/services/platformNotifications.server.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use vitest for all tests in the Trigger.dev repository
**/*.{test,spec}.{ts,tsx}: We use vitest exclusively. Never mock anything - use testcontainers instead.
Test files go next to source files (e.g.,MyService.ts->MyService.test.ts).
Files:
apps/webapp/test/platformNotifications.test.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/test/platformNotifications.test.tsapps/webapp/app/services/platformNotifications.server.ts
apps/webapp/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Do not import
env.server.tsdirectly or indirectly into test files; instead pass environment-dependent values through options/parameters to make code testable
Files:
apps/webapp/test/platformNotifications.test.ts
apps/webapp/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Test files must not import
app/env.server.ts; pass configuration as options instead.
Files:
apps/webapp/test/platformNotifications.test.ts
apps/webapp/app/**/*.ts
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/app/**/*.ts: Never userequest.signalto detect client disconnects. UsegetRequestAbortSignal()fromapp/services/httpAsyncStorage.server.ts, which is wired to Express response close events.
Access environment variables through theenvexport fromapp/env.server.ts; never useprocess.envdirectly.
Always use PrismafindFirstinstead offindUnique.
Always use the$transactionhelper from~/db.server, never callprisma.$transactionor$replica.$transactiondirectly. Pass isolation levels as strings, useSerializablefor correctness-critical read-then-write invariants, and guard possibly undefined helper results when a definite value is required.
Files:
apps/webapp/app/services/platformNotifications.server.ts
🧠 Learnings (1)
📚 Learning: 2026-06-04T18:16:35.386Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3836
File: apps/supervisor/src/backpressure/backpressureMonitor.ts:3-5
Timestamp: 2026-06-04T18:16:35.386Z
Learning: When reviewing TypeScript in this repo, apply the rule “prefer type aliases over interfaces” only to data/object shapes and union/intersection type modeling. If an interface is being used as a behavioral contract for collaborators to implement (e.g., method-shape interfaces that define required behavior, such as `BackpressureLogger` / `BackpressureSignalSource` in `apps/supervisor/src/backpressure/backpressureMonitor.ts`), keep it as an `interface` and do not flag it as a type-alias-vs-interface violation.
Applied to files:
apps/webapp/app/services/platformNotifications.server.ts
…rompt # Conflicts: # apps/webapp/app/services/platformNotifications.server.ts
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…effect lint The publish-draft dialog closed itself by calling a state setter inside an effect, which the react/set-state-in-effect rule flags. Move the form into a child component that closes via an onClose prop instead, matching the existing edit form. No behavior change.
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 (2)
apps/webapp/app/routes/admin.notifications.tsx (2)
100-126: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd required crumbs to the new draft workflow.
The new draft action, service-routing, and UI blocks contain no
//@Crumbsmarker or `// `#region` `@crumbsblock. Add permitted crumb markers to these new workflow blocks. Do not invent a namespace because no namespace table was supplied.As per coding guidelines: “Add crumbs as you write code — not just when debugging. Mark lines with
//@Crumbsor wrap blocks in `// `#region` `@crumbs.”Also applies to: 289-338, 394-428, 487-543, 788-867, 974-980, 1249-1282, 1406-1467, 1720-1744
Source: Coding guidelines
403-407: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winValidate raw schedule values before ISO conversion.
Invalid form values cause
toISOString()to throw aRangeErrorbeforepublishDraftPlatformNotification()can return its Zod validation response. Validate the raw values with Zod before conversion and return a 400 response.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2f9a3220-9255-4916-84c4-cf87e0c03e71
📒 Files selected for processing (1)
apps/webapp/app/routes/admin.notifications.tsx
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (35)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (24, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (19, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (14, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (20, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (23, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (21, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (22, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (17, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (16, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (15, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (18, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (13, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 24)
- GitHub Check: internal / 🧪 Unit Tests: Internal
- GitHub Check: runops-guard / runops-guard
- GitHub Check: fk-cascade-guard / fk-cascade-guard
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (1, 2)
- GitHub Check: obsmap / 🧪 Unit Tests: Observability Map
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (2, 2)
- GitHub Check: typecheck / typecheck
- GitHub Check: report
- GitHub Check: audit
- GitHub Check: code-quality / code-quality
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Prefer static imports over dynamic imports. Only use dynamicimport()when:
- Circular dependencies cannot be resolved otherwise
- Code splitting is genuinely needed for performance
- The module must be loaded conditionally at runtime
Files:
apps/webapp/app/routes/admin.notifications.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/routes/admin.notifications.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/routes/admin.notifications.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: Access environment variables through theenvexport ofenv.server.tsinstead of directly accessingprocess.env
Use subpath exports from@trigger.dev/corepackage instead of importing from the root@trigger.dev/corepath
Files:
apps/webapp/app/routes/admin.notifications.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/app/**/*.{ts,tsx}: For dashboard changes, visually verify the running Remix app with Chrome DevTools MCP, using snapshots, screenshots, interaction, and console-message checks as appropriate.
UseuseCallbackanduseMemoonly for context provider values, expensive derived data used as a dependency, or stable references required by dependency arrays; do not wrap ordinary event handlers or trivial computations.
Use named constants for sentinel or placeholder values instead of scattering raw string literals across comparisons.
Files:
apps/webapp/app/routes/admin.notifications.tsx
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Add crumbs as you write code — not just when debugging. Mark lines with
//@Crumbsor wrap blocks in `// `#region` `@crumbs. They stay on the
branch throughout development and are stripped byagentcrumbs strip
before merge.
Do not invent new namespaces — pick from this table or ask first.
Do not flag//@Crumbsmarkers, `#region `@crumbsblocks, or agentcrumbs
imports in reviews. These are temporary debug instrumentation stripped
before merge. Data logged in crumbs (IDs, names, values) never reaches
production.
Files:
apps/webapp/app/routes/admin.notifications.tsx
🧠 Learnings (1)
📚 Learning: 2026-07-28T21:57:20.061Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 4411
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsx:818-843
Timestamp: 2026-07-28T21:57:20.061Z
Learning: When using Radix UI `DialogClose` with `asChild` (e.g., Trigger.dev dashboard components), note that it injects `type="button"` into its child via `Slot`. If the child is a local `Button` that forwards its `type` prop to the native `<button>`, then placing it inside a `<form>` will *not* submit unless you explicitly set `type="submit"` (or otherwise override the injected type / wire up submission behavior). Review form actions to ensure the intended submit vs non-submit behavior is preserved.
Applied to files:
apps/webapp/app/routes/admin.notifications.tsx
🔇 Additional comments (1)
apps/webapp/app/routes/admin.notifications.tsx (1)
788-867: 🎯 Functional CorrectnessVisually verify the changed dashboard workflows.
Use Chrome DevTools MCP before merge. Verify the publish dialog fields and validation errors. Verify a successful publication closes the dialog and refreshes status. Verify draft editing hides schedule fields. Verify preview submission during editing. Check snapshots, screenshots, interactions, and console messages.
As per coding guidelines: “For dashboard changes, visually verify the running Remix app with Chrome DevTools MCP.”
Also applies to: 1249-1282, 1406-1467
Source: Coding guidelines
2 participants
Summary
The platform notifications admin page can now save a notification as a draft without committing to a schedule, then publish it later by entering start and end dates. Drafts stay hidden from the webapp panel, the CLI, and the "What's new" changelog until they are published.
Design
A draft is an
isDraftflag onPlatformNotification, not nullable dates, so the existing index and every read query stay intact. All three reader queries filter on the flag, so a draft can never surface regardless of its placeholder dates. Publishing writes the real start and end dates and clears the flag; the publish dialog validates the range and shows inline errors. Editing a draft keeps it a draft, with the schedule fields hidden until publish.Also folds in a small tweak: the "Send preview to me" test button now appears when editing a notification, not just when creating one.