feat(admin): implement advanced filtering for audit logs(#248)#263
Hidden character warning
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 18 minutes and 51 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe changes add filtering functionality to the activity log admin interface and extend the activity log response DTO with additional metadata fields. The frontend component now includes a filter panel for search, action type, user role, clinic, and date filtering, while the backend DTO is enriched with performer role, clinic name, and pet name fields. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
frontend/src/components/admin/AuditLogView.jsx (1)
38-46: Minor: inconsistent casing on recordId search.All other fields search against
searchLower, butlog.recordId?.includes(searchTerm)uses the raw term. UUIDs are lowercase but users may paste mixed case. Also, whensearchTermis empty,includes("")is triviallytrue— currently fine because other branches also pass on empty, but still worth normalizing for clarity.- log.recordId?.includes(searchTerm); + log.recordId?.toLowerCase().includes(searchLower);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/admin/AuditLogView.jsx` around lines 38 - 46, In the useMemo that builds filteredLogs, normalize the recordId comparison the same way as the other fields: compute searchLower and compare using log.recordId?.toLowerCase().includes(searchLower) (keeping the existing optional chaining), so mixed-case UUIDs match and the behavior is consistent/clear when searchTerm is empty; update the recordId check in the filteredLogs predicate accordingly.src/main/java/org/example/vet1177/dto/response/activitylog/ActivityLogResponse.java (1)
33-33: Avoid hardcoded localized strings in DTO layer.Embedding the Swedish fallback
"Okänt djur"inside the DTO mixes i18n concerns into the data layer. Prefer returningnull(or an empty string) here and letting the frontend render the localized fallback (the UI atAuditLogView.jsxalready uses a similar pattern forclinicName || 'System'). This keeps the API locale-agnostic for future clients/translations.♻️ Proposed change
- log.getMedicalRecord().getPet() != null ? log.getMedicalRecord().getPet().getName() : "Okänt djur", + log.getMedicalRecord().getPet() != null ? log.getMedicalRecord().getPet().getName() : null,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/vet1177/dto/response/activitylog/ActivityLogResponse.java` at line 33, The DTO currently embeds a Swedish fallback string ("Okänt djur") when building ActivityLogResponse, which mixes i18n into the data layer; modify the construction in ActivityLogResponse (the expression using log.getMedicalRecord().getPet().getName()) to return null (or an empty string) instead of the hardcoded "Okänt djur" so the API remains locale-agnostic and the frontend (AuditLogView.jsx) can render a localized fallback like it already does for clinicName.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/admin/AuditLogView.jsx`:
- Around line 163-169: The span rendering in AuditLogView.jsx incorrectly treats
ROLE_OWNER as the default slate style and can throw when performedByRole is
null; update the conditional on log.performedByRole to add an explicit branch
for 'ROLE_OWNER' (e.g., use a green owner chip class like bg-green-50
text-green-600 border-green-100) and change the text expression to use optional
chaining and a safe fallback (e.g., log.performedByRole?.replace('ROLE_', '') ??
'UNKNOWN') so the UI won't crash on null values.
- Around line 99-103: The role filter options in AuditLogView.jsx are using
short values ("VET","OWNER","ADMIN") that never match backend values like
"ROLE_VET"/"ROLE_ADMIN" (seen on log.performedByRole), so update the <option>
value attributes to the backend form (e.g.,
"ROLE_VET","ROLE_OWNER","ROLE_ADMIN") and keep the "ALL" value as-is;
alternatively, change the filtering predicate (where roleFilter is used, e.g.,
log.performedByRole === roleFilter) to normalize/compare by suffix or accept
both forms (strip the "ROLE_" prefix from log.performedByRole or prepend "ROLE_"
to roleFilter) so the comparison can succeed.
- Line 51: The date filter check in AuditLogView.jsx uses
log.createdAt.startsWith(dateFilter) which can throw if createdAt is
null/undefined; update the matchesDate assignment (the const matchesDate line)
to guard with optional chaining or a null check (e.g., use
log.createdAt?.startsWith(dateFilter) or (log.createdAt &&
log.createdAt.startsWith(dateFilter))) so the filter safely handles missing
createdAt values.
---
Nitpick comments:
In `@frontend/src/components/admin/AuditLogView.jsx`:
- Around line 38-46: In the useMemo that builds filteredLogs, normalize the
recordId comparison the same way as the other fields: compute searchLower and
compare using log.recordId?.toLowerCase().includes(searchLower) (keeping the
existing optional chaining), so mixed-case UUIDs match and the behavior is
consistent/clear when searchTerm is empty; update the recordId check in the
filteredLogs predicate accordingly.
In
`@src/main/java/org/example/vet1177/dto/response/activitylog/ActivityLogResponse.java`:
- Line 33: The DTO currently embeds a Swedish fallback string ("Okänt djur")
when building ActivityLogResponse, which mixes i18n into the data layer; modify
the construction in ActivityLogResponse (the expression using
log.getMedicalRecord().getPet().getName()) to return null (or an empty string)
instead of the hardcoded "Okänt djur" so the API remains locale-agnostic and the
frontend (AuditLogView.jsx) can render a localized fallback like it already does
for clinicName.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2a884611-07a2-40f5-b8b1-2b3b474fa0be
📒 Files selected for processing (2)
frontend/src/components/admin/AuditLogView.jsxsrc/main/java/org/example/vet1177/dto/response/activitylog/ActivityLogResponse.java
closes #248
Summary by CodeRabbit
New Features