Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a full audit-logs feature to the settings UI: new AuditLogsCard component, supporting hooks for querying, metadata, and export (CSV/JSON), integrated into the user settings route; also a small TwoFactorSettingsCard header/layout adjustment. LGTM. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (3)📚 Learning: 2025-08-19T10:22:55.323ZApplied to files:
📚 Learning: 2025-10-24T08:08:23.128ZApplied to files:
📚 Learning: 2025-08-19T10:22:55.323ZApplied to files:
🧬 Code graph analysis (1)echo/frontend/src/components/settings/hooks/useAuditLogsQuery.ts (2)
⏰ 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). (2)
🔇 Additional comments (10)
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 |
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on December 5
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
|
|
||
| return { | ||
| items, | ||
| total: normalizedTotal > 0 ? normalizedTotal : page * pageSize + items.length, |
There was a problem hiding this comment.
Bug: Pagination Error Hides Available Content
The fallback total calculation page * pageSize + items.length produces incorrect results for pagination. When on page 1 (second page) with 25 items per page and 25 items returned, it calculates 50 total items, suggesting only 2 pages exist. If additional pages exist beyond this, users cannot navigate to them because the pagination component receives an incorrect total page count.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
echo/frontend/src/components/settings/AuditLogsCard.tsx(1 hunks)echo/frontend/src/components/settings/TwoFactorSettingsCard.tsx(2 hunks)echo/frontend/src/components/settings/hooks/index.ts(1 hunks)echo/frontend/src/components/settings/hooks/useAuditLogsQuery.ts(1 hunks)echo/frontend/src/routes/settings/UserSettingsRoute.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-08T10:39:31.114Z
Learnt from: ussaama
Repo: Dembrane/echo PR: 259
File: echo/frontend/src/components/layout/ParticipantLayout.tsx:33-33
Timestamp: 2025-08-08T10:39:31.114Z
Learning: In echo/frontend/src/components/layout/ParticipantLayout.tsx, prefer using simple pathname.includes("start") and pathname.includes("finish") to control the settings button visibility. No need to switch to segment-based matching or add a useEffect to auto-close the modal for these routes, per ussaama’s preference in PR #259.
Applied to files:
echo/frontend/src/routes/settings/UserSettingsRoute.tsxecho/frontend/src/components/settings/TwoFactorSettingsCard.tsx
🧬 Code graph analysis (3)
echo/frontend/src/routes/settings/UserSettingsRoute.tsx (1)
echo/frontend/src/components/settings/AuditLogsCard.tsx (1)
AuditLogsCard(75-469)
echo/frontend/src/components/settings/AuditLogsCard.tsx (2)
echo/frontend/src/components/settings/hooks/useAuditLogsQuery.ts (7)
AuditLogEntry(12-21)AuditLogOption(39-43)AuditLogFilters(23-26)useAuditLogsQuery(309-320)useAuditLogMetadata(322-333)useAuditLogsExport(335-354)AuditLogExportFormat(50-50)echo/frontend/src/components/common/Toaster.tsx (1)
toast(34-34)
echo/frontend/src/components/settings/hooks/useAuditLogsQuery.ts (1)
echo/frontend/src/lib/directus.ts (1)
directus(6-14)
⏰ 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). (2)
- GitHub Check: Cursor Bugbot
- GitHub Check: ci-check-server
🔇 Additional comments (2)
echo/frontend/src/components/settings/TwoFactorSettingsCard.tsx (2)
284-298: LGTM - clean layout refactor.Nesting the icon/title in a Group within a Stack and pulling the description text out as a sibling is solid structural improvement. Better spacing control, cleaner component hierarchy. Ships.
315-337: LGTM - smart UX decision.Hiding the recommended apps list when 2FA is already enabled is the right call. Users don't need authenticator app suggestions once they're already set up. Clean conditional logic, proper fragment usage. This is the way.
| return { | ||
| items, | ||
| total: normalizedTotal > 0 ? normalizedTotal : page * pageSize + items.length, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
Keep pagination working without filter_count.
If Directus runs with QUERY_TRACK_TOTAL=false or the role can’t read counts, response.meta.filter_count comes back undefined. Our fallback collapses total to the current page slice, so Mantine renders a single page and you can never reach the next batch even though the API still has data. Harden the fallback so a completely filled page still advertises the next page.
@@
- const normalizedTotal = normalizeCount(metaTotal);
-
- return {
- items,
- total:
- normalizedTotal > 0 ? normalizedTotal : page * pageSize + items.length,
- };
+ const normalizedTotal = normalizeCount(metaTotal);
+ const fallbackTotal =
+ items.length < pageSize
+ ? page * pageSize + items.length
+ : (page + 1) * pageSize + 1;
+
+ return {
+ items,
+ total: normalizedTotal > 0 ? normalizedTotal : fallbackTotal,
+ };Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In echo/frontend/src/components/settings/hooks/useAuditLogsQuery.ts around lines
155 to 159, the fallback for total when response.meta.filter_count is missing
collapses pagination; change the fallback so that if filter_count is undefined
and the returned items length equals pageSize (a full page) you add 1 to the
computed total to indicate there is at least one more page, otherwise keep total
as page * pageSize + items.length; implement this conditional fallback so
Mantine can render the next page when counts are unavailable.
| aggregate: { | ||
| count: "*", | ||
| }, | ||
| groupBy: ["action"], | ||
| sort: ["action"], | ||
| }, | ||
| ), | ||
| requestActivities< | ||
| Array<{ | ||
| collection: string | null; | ||
| count: number; | ||
| }> | ||
| >( | ||
| { | ||
| aggregate: { | ||
| count: "*", | ||
| }, | ||
| groupBy: ["collection"], | ||
| sort: ["collection"], | ||
| }, | ||
| ), | ||
| ]); |
There was a problem hiding this comment.
Stop truncating filter metadata.
Directus caps grouped queries at 100 rows unless we override limit. Tenants with more than 100 collections/actions will silently lose everything past the cutoff, so the dropdown can’t target those logs. Force limit: -1 on both aggregates so we always fetch the full universe.
@@
aggregate: {
count: "*",
},
groupBy: ["action"],
sort: ["action"],
+ limit: -1,
},
@@
aggregate: {
count: "*",
},
groupBy: ["collection"],
sort: ["collection"],
+ limit: -1,
},📝 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.
| aggregate: { | |
| count: "*", | |
| }, | |
| groupBy: ["action"], | |
| sort: ["action"], | |
| }, | |
| ), | |
| requestActivities< | |
| Array<{ | |
| collection: string | null; | |
| count: number; | |
| }> | |
| >( | |
| { | |
| aggregate: { | |
| count: "*", | |
| }, | |
| groupBy: ["collection"], | |
| sort: ["collection"], | |
| }, | |
| ), | |
| ]); | |
| aggregate: { | |
| count: "*", | |
| }, | |
| groupBy: ["action"], | |
| sort: ["action"], | |
| limit: -1, | |
| }, | |
| ), | |
| requestActivities< | |
| Array<{ | |
| collection: string | null; | |
| count: number; | |
| }> | |
| >( | |
| { | |
| aggregate: { | |
| count: "*", | |
| }, | |
| groupBy: ["collection"], | |
| sort: ["collection"], | |
| limit: -1, | |
| }, | |
| ), |
🤖 Prompt for AI Agents
In echo/frontend/src/components/settings/hooks/useAuditLogsQuery.ts around lines
170 to 191, Directus grouped aggregate queries are being truncated at the
default 100-row cap; add a limit: -1 to both aggregate request objects (the one
grouping by "action" and the one grouping by "collection") so the queries return
the full result set instead of only the first 100 rows. Ensure the limit: -1
property is placed at the top level of each request payload alongside
aggregate/groupBy/sort.
<!-- CURSOR_SUMMARY --> > [!NOTE] > Adds a Settings audit logs viewer with filterable, paginated results and CSV/JSON export, and streamlines the Two‑Factor settings layout. > > - **Settings UI** > - **Audit Logs**: New `AuditLogsCard` in `routes/settings/UserSettingsRoute.tsx` displaying Directus activities with filtering (`action`, `collection`), pagination, refresh, and export to `CSV`/`JSON`. > - **Two‑Factor Auth**: Simplifies header/layout and shows "Recommended apps" only when 2FA is disabled in `TwoFactorSettingsCard.tsx`. > - **Data Layer** > - **Hooks**: Adds `useAuditLogsQuery.ts` with types, paginated fetch via `readActivities`, metadata aggregations, and export utilities; re-exported in `hooks/index.ts`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit ce4a992. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a comprehensive Audit Logs viewer in Settings with filters (action, collection), pagination, refresh, and CSV/JSON export. * Audit entries show action, timestamp, collection, target, actor, IP, and user agent. * **UI/UX Improvements** * Two‑Factor Settings header/layout refined; "Recommended apps" shown only when 2FA is disabled. * Audit Logs integrated into the user settings page. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Note
Adds a Settings audit logs viewer with filterable, paginated results and CSV/JSON export, and streamlines the Two‑Factor settings layout.
AuditLogsCardinroutes/settings/UserSettingsRoute.tsxdisplaying Directus activities with filtering (action,collection), pagination, refresh, and export toCSV/JSON.TwoFactorSettingsCard.tsx.useAuditLogsQuery.tswith types, paginated fetch viareadActivities, metadata aggregations, and export utilities; re-exported inhooks/index.ts.Written by Cursor Bugbot for commit ce4a992. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
UI/UX Improvements