Conversation
✅ Deploy Preview for dashboard-v2-novu-staging ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
github-actions
bot
changed the title
|
✅ PR title automatically updated! I found the Linear ticket ID Updated title: |
WalkthroughThis pull request migrates context key handling from single string values to arrays throughout the codebase. The backend GetWorkflowRuns filtering logic is refactored to explicitly handle empty arrays and use whereHasAll for matching. API endpoints updated to serialize contextKeys as multiple query parameters. A new ContextFilter component provides UI for multi-select context filtering. Feature flags gate context-based filtering in subscriber preferences and subscriptions. FacetedFormFilter enhanced with controlled search and loading states. Multiple hooks and API functions are updated to accept and propagate contextKeys arrays. Utility functions added to convert context keys to backend payload format. 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 1
🤖 Fix all issues with AI agents
In `@apps/dashboard/src/components/topics/topic-drawer.tsx`:
- Around line 190-204: The contextKeys state is initialized to [''] which
applies the “no-context” filter immediately; change the initialization of
contextKeys in topic-drawer (the useState for contextKeys) from [''] to an empty
array [] and ensure the code that calls useTopicSubscriptions (where contextKeys
is passed when isContextPreferencesEnabled is true) only applies a no-context
value when the user explicitly selects it (i.e., treat an empty array as “no
filter” rather than as the no-context sentinel), updating any UI handlers that
set contextKeys to use the explicit no-context token only on user action.
🧹 Nitpick comments (3)
apps/dashboard/src/components/primitives/form/faceted-filter/components/multi-filter-content.tsx (1)
59-66: Consider using a spinner component for loading state.The loading indicator currently shows plain text. For visual consistency with other loading states in the dashboard, consider using a spinner or loading component.
♻️ Suggested improvement
+import { Loader2 } from 'lucide-react'; ... {isLoading ? ( <div className="flex items-center justify-center p-4"> - <span className="text-xs text-neutral-400">Loading...</span> + <Loader2 className="h-4 w-4 animate-spin text-neutral-400" /> </div> ) : options.length === 0 && searchQuery ? (apps/dashboard/src/components/subscribers/preferences/preferences.tsx (1)
68-80: Consider the fallback value semantics.Line 73 uses
contextKeys || ['']as a fallback. Given that an empty string in the array has semantic meaning (filter for records with no context), this may unintentionally apply a "no context" filter whencontextKeysisundefined.Consider using
contextKeys ?? []if the intent is to show all contexts when no filter is specified, or document this behavior if intentional.💡 Suggested change if "all contexts" is the intended default
<ContextFilter - contextKeys={contextKeys || ['']} + contextKeys={contextKeys ?? []} onContextKeysChange={(keys) => onContextChange?.(keys)} defaultOnClear={true} />apps/dashboard/src/components/subscribers/subscriptions/subscriber-subscriptions.tsx (1)
74-120: Avoid nested ternary in the render branch.
Extracting acontentblock improves readability and aligns with the no-nested-ternary guideline.As per coding guidelines, avoid nested ternaries.
♻️ Example refactor
const subscriptions = data?.data || []; + let content: JSX.Element; + + if (isPending) { + content = ( + <div className="flex h-full w-full flex-col p-4"> + <div className="flex flex-col gap-2"> + {Array.from({ length: 3 }).map((_, index) => ( + <Skeleton key={index} className="h-[62px] w-full rounded-lg" /> + ))} + </div> + </div> + ); + } else if (subscriptions.length === 0) { + content = <SubscriptionsEmptyState />; + } else { + content = ( + <motion.div className="flex flex-col" initial="hidden" animate="visible" variants={listVariants}> + {subscriptions.map((subscription: TopicSubscription) => ( + <motion.div key={subscription._id} variants={itemVariants}> + <SubscriptionItem + subscription={subscription} + onDeleteSubscription={handleDeleteSubscription} + onViewTopic={handleViewTopic} + onViewSubscriptionPreferences={handleViewSubscriptionPreferences} + /> + </motion.div> + ))} + </motion.div> + ); + } + return ( <> <motion.div key="subscription-list" className="flex h-full w-full flex-col border-t border-t-neutral-200" initial={{ opacity: 0 }} animate={{ opacity: 1 }} exit={{ opacity: 0 }} transition={{ duration: 0.15, ease: [0.4, 0, 0.2, 1], }} > {isContextPreferencesEnabled && ( <SidebarContent size="md" className="min-h-max overflow-x-auto border-b border-neutral-200 py-2 px-2"> <div className="flex items-center gap-2"> <ContextFilter contextKeys={contextKeys} onContextKeysChange={setContextKeys} defaultOnClear={true} /> </div> </SidebarContent> )} - - {isPending ? ( - <div className="flex h-full w-full flex-col p-4"> - <div className="flex flex-col gap-2"> - {Array.from({ length: 3 }).map((_, index) => ( - <Skeleton key={index} className="h-[62px] w-full rounded-lg" /> - ))} - </div> - </div> - ) : subscriptions.length === 0 ? ( - <SubscriptionsEmptyState /> - ) : ( - <motion.div className="flex flex-col" initial="hidden" animate="visible" variants={listVariants}> - {subscriptions.map((subscription: TopicSubscription) => ( - <motion.div key={subscription._id} variants={itemVariants}> - <SubscriptionItem - subscription={subscription} - onDeleteSubscription={handleDeleteSubscription} - onViewTopic={handleViewTopic} - onViewSubscriptionPreferences={handleViewSubscriptionPreferences} - /> - </motion.div> - ))} - </motion.div> - )} + {content} </motion.div>
ChmaraX
deleted the
nv-7067-dashboard-update-preferences-ui-with-context
branch
1 participant
What changed? Why was the change needed?
This pull request introduces significant improvements to how context filtering is handled throughout the dashboard and API. The main change is the migration of
contextKeysfrom a comma-separated string to an array of strings, which simplifies the logic for filtering and improves consistency across the codebase. Additionally, a new reusableContextFiltercomponent has been added to enhance the user experience when selecting contexts, including support for searching and loading states.Context Filtering Improvements
Refactored all instances of
contextKeysin filters and API calls from a single string (often comma-separated) to an array of strings, ensuring consistent handling and simplifying the logic for filtering activities, workflow runs, and subscriptions. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13]Updated the backend query logic to distinguish between three cases: no filter, filtering for records with no context, and filtering for records containing all specified contexts.
UI Enhancements
Introduced a new
ContextFiltercomponent, allowing users to select multiple contexts with search and loading support. This replaces the previous string input for context keys in activity filters and subscriber preferences. [1] [2] [3] [4]Updated the activity filters and related components to use the new
ContextFilter, including support for clearing and default context selection. [1] [2] [3]Component and API Propagation
contextKeysas an array through various API methods and components, such as subscriber preferences, topic subscriptions, and workflow runs. [1] [2] [3] [4] [5] [6] [7]Faceted Filter Improvements
FacetedFormFilterand its multi-filter content to support loading states and controlled search input, improving the UX for filters with dynamic options like contexts. [1] [2] [3] [4] [5] [6] [7]These changes collectively provide a more robust, user-friendly, and maintainable approach to context-based filtering throughout the application.
context.filtering.mov