fix(notifications): polish modal styling, credential display, and trigger filters#3571
Conversation
…gger filters - Show credential display name instead of raw account ID in Slack account selector - Fix label styling to use default Label component (text-primary) for consistency - Fix modal body spacing with proper top padding after tab bar - Replace list-card skeleton with form-field skeleton matching actual layout - Replace custom "Select a Slack account first" box with disabled Combobox (dependsOn pattern) - Use proper Label component in WorkflowSelector with consistent gap spacing - Add overflow badge pattern (slice + +N) to level and trigger filter badges - Use dynamic trigger options from getTriggerOptions() instead of hardcoded CORE_TRIGGER_TYPES - Relax API validation to accept integration trigger types (z.string instead of z.enum) - Deduplicate account rows from credential leftJoin in accounts API - Extract getTriggerOptions() to module-level constants to avoid per-render calls Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PR SummaryMedium Risk Overview Updates trigger filtering to use dynamic options from Enhances OAuth account display by joining Written by Cursor Bugbot for commit 2d4f25e. Configure here. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile SummaryThis PR polishes the notifications modal with a set of focused UI and API improvements: it standardises label colours by removing Key changes:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant UI as NotificationSettings UI
participant AccountsAPI as GET /api/auth/accounts
participant CredDB as credential table
participant NotifAPI as POST/PUT /notifications
participant EventsLib as lib/logs/events.ts
UI->>AccountsAPI: GET ?provider=slack
AccountsAPI->>CredDB: SELECT account LEFT JOIN credential ON credential.accountId = account.id
CredDB-->>AccountsAPI: rows (may include multiple credential rows per account across workspaces)
AccountsAPI-->>AccountsAPI: Deduplicate via Map (keeps first credential seen)
AccountsAPI-->>UI: { accounts: [{ displayName: credentialDisplayName || accountId || providerId }] }
UI->>NotifAPI: POST { triggerFilter: string[] (any value, min(1)) }
NotifAPI-->>NotifAPI: Validate with z.string().min(1) (default: [])
NotifAPI-->>UI: 201 Created subscription
Note over EventsLib: On workflow execution
EventsLib->>EventsLib: triggerMatches = triggerFilter.length === 0 OR triggerFilter.includes(log.trigger)
EventsLib-->>EventsLib: Empty triggerFilter → matches ALL trigger types
Last reviewed commit: 2d4f25e |
...kspace/[workspaceId]/logs/components/logs-toolbar/components/notifications/notifications.tsx
Show resolved
Hide resolved
- Restore accountId in displayName fallback chain (credentialDisplayName || accountId || providerId) - Add .default([]) to triggerFilter in create schema to preserve backward compatibility - Treat empty triggerFilter as "match all" in notification matching logic - Remove unreachable overflow badge for levelFilter (only 2 possible values) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@greptile |
|
@cursor review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Summary
Labelcomponent (text-primary) matching all other modals in the appdependsOnpattern used in subblocks)Labelcomponent inWorkflowSelectorwith consistentgap-[8px]spacing+N) to level and trigger filter badge overlaysgetTriggerOptions()instead of hardcodedCORE_TRIGGER_TYPES, matching the logs toolbarz.string()instead ofz.enum)leftJoinin accounts APIgetTriggerOptions()to module-level constants to avoid per-render callsTest plan
+Nafter 6 visible badges