feat(webapp): Platform notifications admin imporovements#3324
Conversation
|
WalkthroughThis pull request adds organization and project context scoping to help/feedback and changelog features, and significantly refactors the admin notifications system. The component props for Estimated code review effort🎯 3 (Moderate) | ⏱️ ~40 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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.
0ski
force-pushed
the
oskar/feat-platform-notifications-admin
branch
from
360060a to
e83679c
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/webapp/app/services/platformNotifications.server.ts (2)
292-319: Consider distinguishing "no membership" from "membership but invalid project".The function silently returns
{}when membership verification fails, making it impossible for callers to distinguish between "user provided no organizationId" vs "user is not a member of the organization". This could mask authorization failures.If explicit failure signaling is not needed for the current use case, consider at least adding a brief comment explaining that silent fallback is intentional.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/services/platformNotifications.server.ts` around lines 292 - 319, The verifyOrgMembership function currently returns {} for multiple different failure cases which hides whether the caller omitted organizationId, the user is not a member, or the project is invalid; update verifyOrgMembership to return an explicit discriminated result (e.g., { status: 'ok', organizationId, projectId } | { status: 'no_org_id' } | { status: 'not_member' } | { status: 'invalid_project' }) or throw specific errors so callers can distinguish cases; change the function signature and replace the three blank-object returns with the appropriate discriminated responses (refer to verifyOrgMembership, prisma.orgMember and prisma.project usage) and update call sites to handle the new statuses, or if you intend the current silent behavior, add a clear comment above verifyOrgMembership explaining the intentional fallback.
781-797: Inconsistent error handling in mutation functions.
deletePlatformNotification,publishNowPlatformNotification, andarchivePlatformNotificationwill throw Prisma errors if the record doesn't exist, but the handlers (handleArchiveAction,handleDeleteAction,handlePublishNowAction) don't wrap these calls in try-catch. This is inconsistent withcreatePlatformNotificationandupdatePlatformNotification, which use the neverthrow Result type and are properly handled with error checks. Consider wrapping the three mutation functions with error handling to match the established pattern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/services/platformNotifications.server.ts` around lines 781 - 797, Change the three mutation helpers (deletePlatformNotification, publishNowPlatformNotification, archivePlatformNotification) to mirror the create/update pattern by returning a neverthrow Result instead of letting Prisma throw: wrap the Prisma call in try-catch, return ok(void 0) on success and err(error) on failure (import Result/ok/err from neverthrow), and update call sites/handlers if needed to handle the Result; this ensures consistency with createPlatformNotification and updatePlatformNotification and prevents unhandled Prisma exceptions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/webapp/app/services/platformNotifications.server.ts`:
- Around line 292-319: The verifyOrgMembership function currently returns {} for
multiple different failure cases which hides whether the caller omitted
organizationId, the user is not a member, or the project is invalid; update
verifyOrgMembership to return an explicit discriminated result (e.g., { status:
'ok', organizationId, projectId } | { status: 'no_org_id' } | { status:
'not_member' } | { status: 'invalid_project' }) or throw specific errors so
callers can distinguish cases; change the function signature and replace the
three blank-object returns with the appropriate discriminated responses (refer
to verifyOrgMembership, prisma.orgMember and prisma.project usage) and update
call sites to handle the new statuses, or if you intend the current silent
behavior, add a clear comment above verifyOrgMembership explaining the
intentional fallback.
- Around line 781-797: Change the three mutation helpers
(deletePlatformNotification, publishNowPlatformNotification,
archivePlatformNotification) to mirror the create/update pattern by returning a
neverthrow Result instead of letting Prisma throw: wrap the Prisma call in
try-catch, return ok(void 0) on success and err(error) on failure (import
Result/ok/err from neverthrow), and update call sites/handlers if needed to
handle the Result; this ensures consistency with createPlatformNotification and
updatePlatformNotification and prevents unhandled Prisma exceptions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 79d98d09-4f20-4ba3-8efd-58e1be185ba1
📒 Files selected for processing (7)
apps/webapp/app/components/navigation/HelpAndFeedbackPopover.tsxapps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/routes/admin.notifications.tsxapps/webapp/app/routes/resources.platform-changelogs.tsxapps/webapp/app/routes/resources.platform-notifications.tsxapps/webapp/app/services/platformNotifications.server.ts
✅ Files skipped from review due to trivial changes (2)
- apps/webapp/app/routes/resources.platform-notifications.tsx
- apps/webapp/app/routes/admin.notifications.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/webapp/app/routes/resources.platform-changelogs.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (27)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: typecheck / typecheck
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{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
Files:
apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/components/navigation/HelpAndFeedbackPopover.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/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/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/components/navigation/HelpAndFeedbackPopover.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/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/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/components/navigation/HelpAndFeedbackPopover.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/services/platformNotifications.server.ts
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/components/navigation/HelpAndFeedbackPopover.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/services/platformNotifications.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/components/navigation/HelpAndFeedbackPopover.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/services/platformNotifications.server.ts
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/components/navigation/HelpAndFeedbackPopover.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/services/platformNotifications.server.ts
apps/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
When modifying only server components (
apps/webapp/,apps/supervisor/, etc.) with no package changes, add a.server-changes/file instead of a changeset
Files:
apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/components/navigation/HelpAndFeedbackPopover.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/services/platformNotifications.server.ts
apps/webapp/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Only use
useCallback/useMemofor context provider values, expensive derived data with external dependencies, or stable refs required by dependency arrays; don't wrap ordinary event handlers or trivial computations
Files:
apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/components/navigation/HelpAndFeedbackPopover.tsxapps/webapp/app/components/navigation/SideMenu.tsx
apps/webapp/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Use named constants for sentinel/placeholder values instead of raw string literals scattered across comparisons
Files:
apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/components/navigation/HelpAndFeedbackPopover.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/services/platformNotifications.server.ts
apps/webapp/app/services/**/*.server.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Separate testable services from configuration files; follow the pattern of
realtimeClient.server.ts(testable service) andrealtimeClientGlobal.server.ts(configuration) in the webapp
Files:
apps/webapp/app/services/platformNotifications.server.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
**/*.ts: Usetypecheckto verify changes in apps and internal packages (apps/*,internal-packages/*), notbuild- building proves almost nothing about correctness
When writing Trigger.dev tasks, always import from@trigger.dev/sdk. Never use@trigger.dev/sdk/v3or deprecatedclient.defineJob
Add crumbs as you write code - mark lines with//@Crumbsor wrap blocks in `// `#region` `@crumbsfor agentcrumbs debug tracing, then strip before merge
Files:
apps/webapp/app/services/platformNotifications.server.ts
apps/webapp/**/*.server.{ts,tsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/**/*.server.{ts,tsx}: Environment variables must be accessed via theenvexport fromapp/env.server.tsand never useprocess.envdirectly
Always usefindFirstinstead offindUniquein Prisma queries due to implicit DataLoader batching issues and performance concerns
Files:
apps/webapp/app/services/platformNotifications.server.ts
🧠 Learnings (12)
📚 Learning: 2026-04-02T19:18:34.807Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3319
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens/route.tsx:249-258
Timestamp: 2026-04-02T19:18:34.807Z
Learning: In `apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens/route.tsx`, the `onCollapseChange={() => {}}` no-op on the `ResizablePanel` for the waitpoint inspector is intentional. The panel collapse is controlled externally via `collapsed={!isShowingWaitpoint}` (driven by URL params), so the callback is deliberately left as a no-op. Do not flag this as a missing implementation.
Applied to files:
apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/components/navigation/SideMenu.tsx
📚 Learning: 2026-04-03T11:54:21.609Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3319
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.regions/route.tsx:265-267
Timestamp: 2026-04-03T11:54:21.609Z
Learning: In `apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.regions/route.tsx`, the `TableCellMenu` for the "Set as default" region action intentionally uses `hiddenButtons` (hover-only visibility) rather than `visibleButtons`. This is a deliberate UX design choice by the team. Do not flag the hover-only visibility as an accessibility concern for this specific instance.
Applied to files:
apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsx
📚 Learning: 2026-02-11T16:37:32.429Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/components/primitives/charts/Card.tsx:26-30
Timestamp: 2026-02-11T16:37:32.429Z
Learning: In projects using react-grid-layout, avoid relying on drag-handle class to imply draggability. Ensure drag-handle elements only affect dragging when the parent grid item is configured draggable in the layout; conditionally apply cursor styles based on the draggable prop. This improves correctness and accessibility.
Applied to files:
apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/components/navigation/HelpAndFeedbackPopover.tsxapps/webapp/app/components/navigation/SideMenu.tsx
📚 Learning: 2026-03-22T13:26:12.060Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).
Applied to files:
apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/components/navigation/HelpAndFeedbackPopover.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/services/platformNotifications.server.ts
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.
Applied to files:
apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/components/navigation/HelpAndFeedbackPopover.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/services/platformNotifications.server.ts
📚 Learning: 2026-02-11T16:50:14.167Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.$dashboardId.widgets.tsx:126-131
Timestamp: 2026-02-11T16:50:14.167Z
Learning: In apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.$dashboardId.widgets.tsx, MetricsDashboard entities are intentionally scoped to the organization level, not the project level. The dashboard lookup should filter by organizationId only (not projectId), allowing dashboards to be accessed across projects within the same organization. The optional projectId field on MetricsDashboard serves other purposes and should not be used as an authorization constraint.
Applied to files:
apps/webapp/app/components/navigation/HelpAndFeedbackPopover.tsx
📚 Learning: 2026-04-01T13:27:35.831Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3308
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.models._index/route.tsx:579-584
Timestamp: 2026-04-01T13:27:35.831Z
Learning: In `apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.models._index/route.tsx`, the `useEffect` inside `CompareDialog` intentionally uses `[open]` as its only dependency. This is a deliberate "fetch-on-open" pattern: the fetcher.load call should fire only when the dialog opens, not on every reference change of `models`, `organization`, `project`, or `environment`. Do not flag this as a missing-dependency lint issue.
Applied to files:
apps/webapp/app/components/navigation/HelpAndFeedbackPopover.tsx
📚 Learning: 2026-03-26T10:00:07.558Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 3254
File: packages/cli-v3/src/apiClient.ts:61-86
Timestamp: 2026-03-26T10:00:07.558Z
Learning: In triggerdotdev/trigger.dev, the `api.v1.platform-notifications` loader filters `PlatformNotification` records by `surface: "CLI"` before returning them. As a result, `card` and `changelog` notification types (which are `WEBAPP`-only) never reach the CLI endpoint, and the `CliPlatformNotificationResponseSchema` in `packages/cli-v3/src/apiClient.ts` does not need to include those types in its `data.type` enum. Do not flag missing `card`/`changelog` enum values in this schema as a bug.
Applied to files:
apps/webapp/app/services/platformNotifications.server.ts
📚 Learning: 2026-03-26T10:02:25.354Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 3254
File: apps/webapp/app/services/platformNotifications.server.ts:363-385
Timestamp: 2026-03-26T10:02:25.354Z
Learning: In `triggerdotdev/trigger.dev`, the `getNextCliNotification` fallback in `apps/webapp/app/services/platformNotifications.server.ts` intentionally uses `prisma.orgMember.findFirst` (single org) when no `projectRef` is provided. This is acceptable for v1 because the CLI (`dev` and `login` commands) always passes `projectRef` in normal usage, making the fallback a rare edge case. Do not flag the single-org fallback as a multi-org correctness bug in this file.
Applied to files:
apps/webapp/app/services/platformNotifications.server.ts
📚 Learning: 2026-03-02T12:43:37.906Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: packages/core/CLAUDE.md:0-0
Timestamp: 2026-03-02T12:43:37.906Z
Learning: Exercise caution with changes to trigger.dev/core as they affect both the customer-facing SDK and server-side webapp - breaking changes can impact deployed user tasks and the platform simultaneously
Applied to files:
apps/webapp/app/services/platformNotifications.server.ts
📚 Learning: 2026-03-25T15:29:25.889Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2026-03-25T15:29:25.889Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `schemaTask()` from `trigger.dev/sdk` with a Zod schema for payload validation
Applied to files:
apps/webapp/app/services/platformNotifications.server.ts
📚 Learning: 2026-03-26T09:02:07.973Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 3274
File: apps/webapp/app/services/runsReplicationService.server.ts:922-924
Timestamp: 2026-03-26T09:02:07.973Z
Learning: When parsing Trigger.dev task run annotations in server-side services, keep `TaskRun.annotations` strictly conforming to the `RunAnnotations` schema from `trigger.dev/core/v3`. If the code already uses `RunAnnotations.safeParse` (e.g., in a `#parseAnnotations` helper), treat that as intentional/necessary for atomic, schema-accurate annotation handling. Do not recommend relaxing the annotation payload schema or using a permissive “passthrough” parse path, since the annotations are expected to be written atomically in one operation and should not contain partial/legacy payloads that would require a looser parser.
Applied to files:
apps/webapp/app/services/platformNotifications.server.ts
🔇 Additional comments (10)
apps/webapp/app/services/platformNotifications.server.ts (6)
62-70: LGTM!The rename from
hideArchivedtohideInactivewith the updated filter logic (archivedAt: nullANDendsAt > now) correctly captures both archived and expired notifications for hiding.
103-118: LGTM!Good additions exposing scope foreign keys and parsed payload fields for the admin list. The fallback defaults (
falsefordismissOnAction,nullfordiscovery) when parsing fails are sensible.
323-362: LGTM!The updated
getRecentChangelogscorrectly:
- Requires
userIdand filters bystartsAt <= nowto hide future changelogs- Applies scope filtering to prevent user-scoped changelogs leaking to others
- Conditionally adds
ORGANIZATION/PROJECTscope filters only when IDs are providedThis addresses the PR objective to show changelogs to the target audience.
557-573: LGTM!Extracting
NotificationBaseFieldsas a reusable const object is a clean DRY refactor, and using a const object instead of an interface aligns with the coding guidelines.
685-700: LGTM!The
UpdatePlatformNotificationSchemacorrectly reusesNotificationBaseFields, makesstartsAtrequired (unlike create), and appropriately skipsvalidateStartsAtsince existing notifications may have past start dates.
743-779: LGTM!The
updatePlatformNotificationfunction correctly:
- Nullifies scope FKs when scope changes (lines 762-764)
- Nullifies CLI-only fields when surface is not CLI (lines 768-770)
- Reuses the
CreateErrortype for consistent error handling via neverthrowapps/webapp/app/components/navigation/HelpAndFeedbackPopover.tsx (1)
33-46: LGTM!The component correctly accepts optional
organizationIdandprojectIdprops and passes them touseRecentChangelogsfor context-aware changelog filtering. The props are appropriately optional to maintain backward compatibility.apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsx (1)
207-209: LGTM!Correctly passes
organizationIdtoHelpAndFeedbackin the organization settings context. OmittingprojectIdis appropriate here since this menu is at the organization level, not scoped to a specific project.apps/webapp/app/components/navigation/SideMenu.tsx (2)
716-716: LGTM!Correctly passes both
organizationIdandprojectIdtoHelpAndAIin the project-scoped side menu context, enabling full context-aware changelog filtering.
1166-1181: LGTM!The
HelpAndAIcomponent correctly:
- Accepts
organizationIdandprojectIdas required props (appropriate since it's always rendered with both inSideMenu)- Forwards them to
HelpAndFeedback
2 participants
Uh oh!
There was an error while loading. Please reload this page.