feat: enhance feedback table functionality and UI #2240
+251
−179
Conversation
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
- Added sorting capabilities to the feedback table. - Refactored feedback table to improve state management and pagination. - Introduced separate components for feedback deletion and AI processing actions. - Updated date range handling and channel selection logic. - Improved loading states and error handling in feedback management. - Enhanced table pagination to reset page index on page size change. - Updated dependencies in pnpm-lock.yaml for better compatibility.
WalkthroughRefactors feedback UI to be channel-aware, integrates client-side sorting into data fetches, modularizes delete/AI actions, changes query caching defaults (60s), adjusts pagination behavior to reset on page-size changes, and simplifies Next.js config and route type import. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant FeedbackPage as feedback.tsx
participant ChannelService
participant FeedbackContainer
participant FeedbackTable
participant QueryConverter
participant API as DataFetch
User->>FeedbackPage: open feedback page
FeedbackPage->>ChannelService: fetch channels for project
ChannelService-->>FeedbackPage: channels
FeedbackPage->>FeedbackPage: set currentChannelId (auto-select first)
FeedbackPage->>FeedbackContainer: mount with currentChannelId
FeedbackContainer->>API: fetch channelData for channelId
API-->>FeedbackContainer: channelData
FeedbackContainer->>QueryConverter: compute filterFields from channelData
User->>FeedbackTable: change sort / filters / date / channel
FeedbackTable->>FeedbackTable: reset pageIndex & clear selection
FeedbackTable->>QueryConverter: build queries (includes sort + filtered fields)
QueryConverter->>API: fetch feedback list (staleTime=60s)
API-->>FeedbackTable: results
FeedbackTable->>User: render updated table
User->>FeedbackTable: click delete or Run AI
FeedbackTable->>FeedbackDeleteButton: open delete flow / confirm
FeedbackTable->>RunAIButton: trigger AI mutation
FeedbackDeleteButton->>API: delete mutation
RunAIButton->>API: run-ai mutation
API-->>FeedbackTable: mutation results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (1)
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/entities/feedback/lib/use-feedback-query-converter.ts (1)
164-185: Fix stale dependencies inonChangeTableFilters.
onChangeTableFilterscloses overprojectIdanddefaultDateRange, but the dependency list only containsdateRange. WhenfeedbackSearchMaxDaysor the active channel changes, the callback keeps using the previous default range and project ID, so clearing filters reinstates the wrong date window and issues get created against the wrong project. Add all reactive values the function reads so the memoized callback stays in sync.(react.dev)- const onChangeTableFilters = useCallback( + const onChangeTableFilters = useCallback( async (tableFilters: TableFilter[], operator: TableFilterOperator) => { const result = await Promise.all( tableFilters.map(async ({ condition, format, key, value }) => ({ key, value: await toQuery(projectId)[format](value), condition, })), ); if (result.length === 0 && !dateRange) { await setDefaultQueries(defaultDateRange ? [defaultDateRange] : []); await setQueries([]); await setOperator('AND'); return; } await setOperator(operator); await setQueries(result); }, - [dateRange], + [dateRange, defaultDateRange, projectId, setDefaultQueries, setOperator, setQueries], );
🧹 Nitpick comments (1)
apps/web/package.json (1)
40-40: Use the stable React Compiler package.
[email protected]predates the 1.0.0 stable release and has already surfaced install regressions in downstream tooling. Pinning the stable 1.0.0 build keeps us aligned with the React team’s guidance and avoids the RC-only breakages we’ve seen reported in the ecosystem.(ko.react.dev)- "babel-plugin-react-compiler": "19.1.0-rc.3", + "babel-plugin-react-compiler": "1.0.0",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
apps/web/next-env.d.ts(1 hunks)apps/web/next.config.js(1 hunks)apps/web/package.json(1 hunks)apps/web/src/entities/feedback/lib/use-feedback-query-converter.ts(2 hunks)apps/web/src/entities/feedback/ui/feedback-table.ui.tsx(8 hunks)apps/web/src/pages/_app.tsx(1 hunks)apps/web/src/pages/main/project/[projectId]/feedback.tsx(5 hunks)apps/web/src/shared/ui/tables/table-pagination.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/entities/feedback/ui/feedback-table.ui.tsx (1)
apps/web/src/assets/index.tsx (1)
AISparklingIcon(99-109)
Comment on lines
+379
to
+405
| const { mutateAsync: processAI, isPending: isPendingAIProcess } = | ||
| useOAIMutation({ | ||
| method: 'post', | ||
| path: '/api/admin/projects/{projectId}/ai/process', | ||
| pathParams: { projectId }, | ||
| queryOptions: { | ||
| onSuccess() { | ||
| toast.success(t('v2.toast.success')); | ||
| }, | ||
| onSettled() { | ||
| afterProcess(); | ||
| setLoadingFeedbackIds([]); | ||
| }, | ||
| onError(error) { | ||
| toast.error(error.message); | ||
| }, | ||
| }, | ||
| }); | ||
| return ( | ||
| <Button | ||
| variant="outline" | ||
| onClick={() => { | ||
| setLoadingFeedbackIds(selectedRowIds); | ||
| toast.promise(processAI({ feedbackIds: selectedRowIds }), { | ||
| loading: 'Loading', | ||
| success: () => 'Success', | ||
| }); |
There was a problem hiding this comment.
Avoid double toasts when using toast.promise
toast.promise already handles the loading/success/error lifecycle, but the mutation still emits separate toast.success/toast.error, so the user gets two notifications for each outcome. Let toast.promise own the messaging and keep the mutation focused on state cleanup.
const { mutateAsync: processAI, isPending: isPendingAIProcess } =
useOAIMutation({
method: 'post',
path: '/api/admin/projects/{projectId}/ai/process',
pathParams: { projectId },
queryOptions: {
- onSuccess() {
- toast.success(t('v2.toast.success'));
- },
onSettled() {
afterProcess();
setLoadingFeedbackIds([]);
},
- onError(error) {
- toast.error(error.message);
- },
},
});
@@
toast.promise(processAI({ feedbackIds: selectedRowIds }), {
- loading: 'Loading',
- success: () => 'Success',
+ loading: 'Loading',
+ success: () => t('v2.toast.success'),
+ error: (error: Error) => error.message,
});
}}
disabled={isPendingAIProcess || !perms.includes('feedback_update')}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const { mutateAsync: processAI, isPending: isPendingAIProcess } = | |
| useOAIMutation({ | |
| method: 'post', | |
| path: '/api/admin/projects/{projectId}/ai/process', | |
| pathParams: { projectId }, | |
| queryOptions: { | |
| onSuccess() { | |
| toast.success(t('v2.toast.success')); | |
| }, | |
| onSettled() { | |
| afterProcess(); | |
| setLoadingFeedbackIds([]); | |
| }, | |
| onError(error) { | |
| toast.error(error.message); | |
| }, | |
| }, | |
| }); | |
| return ( | |
| <Button | |
| variant="outline" | |
| onClick={() => { | |
| setLoadingFeedbackIds(selectedRowIds); | |
| toast.promise(processAI({ feedbackIds: selectedRowIds }), { | |
| loading: 'Loading', | |
| success: () => 'Success', | |
| }); | |
| const { mutateAsync: processAI, isPending: isPendingAIProcess } = | |
| useOAIMutation({ | |
| method: 'post', | |
| path: '/api/admin/projects/{projectId}/ai/process', | |
| pathParams: { projectId }, | |
| queryOptions: { | |
| onSettled() { | |
| afterProcess(); | |
| setLoadingFeedbackIds([]); | |
| }, | |
| }, | |
| }); | |
| return ( | |
| <Button | |
| variant="outline" | |
| onClick={() => { | |
| setLoadingFeedbackIds(selectedRowIds); | |
| toast.promise(processAI({ feedbackIds: selectedRowIds }), { | |
| loading: 'Loading', | |
| success: () => t('v2.toast.success'), | |
| error: (error: Error) => error.message, | |
| }); |
🤖 Prompt for AI Agents
In apps/web/src/entities/feedback/ui/feedback-table.ui.tsx around lines 379 to
405, the mutation uses both toast.promise and also emits
toast.success/toast.error in its queryOptions which causes duplicate
notifications; remove the toast.success and toast.error calls from the
mutation's onSuccess/onError handlers (leave onSettled to call afterProcess()
and setLoadingFeedbackIds([]) for cleanup) and let the toast.promise call supply
the loading/success/error messages instead (ensure toast.promise options include
appropriate success/error text).
Comment on lines
+51
to
+54
| if (!channels) return; | ||
| void setCurrentChannelId(channels.items[0]?.id ?? null); | ||
| }, [channels]); | ||
|
|
There was a problem hiding this comment.
Don’t overwrite the selected channel on refetch.
Every time useAllChannels refetches, this effect writes the first channel ID back into query state. That means the user loses their selection as soon as the list refreshes (e.g., background stale-time refresh or mutation). Gate the assignment so we only seed the default when no valid selection exists.
- useEffect(() => {
- if (!channels) return;
- void setCurrentChannelId(channels.items[0]?.id ?? null);
- }, [channels]);
+ useEffect(() => {
+ if (!channels) return;
+
+ const firstChannelId = channels.items[0]?.id ?? null;
+ const currentExists = channels.items.some(
+ (channel) => channel.id === currentChannelId,
+ );
+
+ if (currentChannelId !== -1 && currentExists) return;
+
+ void setCurrentChannelId(firstChannelId);
+ }, [channels, currentChannelId, setCurrentChannelId]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (!channels) return; | |
| void setCurrentChannelId(channels.items[0]?.id ?? null); | |
| }, [channels]); | |
| useEffect(() => { | |
| if (!channels) return; | |
| const firstChannelId = channels.items[0]?.id ?? null; | |
| const currentExists = channels.items.some( | |
| (channel) => channel.id === currentChannelId, | |
| ); | |
| if (currentChannelId !== -1 && currentExists) return; | |
| void setCurrentChannelId(firstChannelId); | |
| }, [channels, currentChannelId, setCurrentChannelId]); |
🤖 Prompt for AI Agents
In apps/web/src/pages/main/project/[projectId]/feedback.tsx around lines 51 to
54, the effect unconditionally seeds the current channel with
channels.items[0]?.id on every refetch which clobbers a user’s existing
selection; change the logic to only set the current channel when there is no
valid selection (e.g., currentChannelId is null/undefined or the id is not
present in channels.items), and skip assignment when channels is empty or the
existing selected id is still valid so background refetches won’t overwrite the
user’s choice.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary by CodeRabbit
New Features
Bug Fixes
Chores