Audit Log Filtering - #2200
Conversation
📝 WalkthroughWalkthroughThis PR introduces a comprehensive audit logging feature with new API integration, React components for displaying and filtering audit logs with server-side pagination, internationalization support, and extensive test coverage via Storybook stories across both v1 and v2 code paths. Changes
Sequence DiagramsequenceDiagram
participant User
participant Component as Audit Log Component
participant Query as React-Query
participant API as API Client
participant Server as Backend API
User->>Component: Interact (filter, paginate)
Component->>Component: Update tableState with filters/pagination
Component->>Query: Call useAuditLogsQuery(params)
Query->>API: Call auditApi.getAuditlogs(params)
API->>Server: GET /api/audit-logs?limit=20&offset=0&filters...
Server->>API: Return AuditLogPagination
API->>Query: response.data
Query->>Component: Provide paginated results + metadata
Component->>Component: Map API entries to table rows
Component->>User: Render filtered audit log table
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 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. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip You can customize the tone of the review comments and chat replies.Configure the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/v2/data/queries/audit.ts (1)
19-21:⚠️ Potential issue | 🔴 CriticalUse the repo's RBAC-client cast workaround for
auditApi.getAuditlogs.Line 20 directly invokes
auditApi.getAuditlogs(params)without the required(as any)cast. Persrc/docs/APIClientPatterns.mdx, the@redhat-cloud-services/rbac-clientlibrary has broken TypeScript types and requires the(api.method as any)pattern to prevent silent runtime failures.🔧 Suggested fix
- const response = await auditApi.getAuditlogs(params); + const response = await (auditApi.getAuditlogs as any)(params);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/v2/data/queries/audit.ts` around lines 19 - 21, The call inside the async queryFn to auditApi.getAuditlogs should use the repo's RBAC-client cast workaround to avoid broken TypeScript types; replace the direct invocation auditApi.getAuditlogs(params) with the casted form (auditApi.getAuditlogs as any)(params) (i.e., cast the method to any before calling) so that queryFn returns response.data safely; update the invocation within the queryFn block where auditApi.getAuditlogs is called.
🧹 Nitpick comments (1)
src/user-journeys/access-management/AuditLog.stories.tsx (1)
66-68: The action filter still is not exercised in this journey.The docs table now advertises requester/resource/action filtering, but the new
playcoverage only hits requester and resource. Please add a third story for the action select so the last new control cannot regress silently.Also applies to: 249-368
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/user-journeys/access-management/AuditLog.stories.tsx` around lines 66 - 68, The docs table claims filtering by action but the existing Storybook stories only exercise requester and resource; add a new story in AuditLog.stories.tsx (e.g., AuditLogActionFilter or AuditLog_Play_Action) that renders the same AuditLog component and in its play function selects the action <select> control and verifies the filter behavior (mirror how the existing AuditLog_Play_Requester and AuditLog_Play_Resource stories operate), using the same query selectors/assertions to choose an action option and assert results update; ensure the new story name is exported so Storybook picks it up and follow the same test utilities and imports used by the other play functions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 6-18: Update the CHANGELOG.md entries that link to the fork by
replacing the username segment "CodyWMitchell" with the upstream
"RedHatInsights" in the commit and issue URLs (e.g., the links in the shown
entries referencing commits like 9442e25, 3c1436a, 0ba659a, etc.); regenerate or
search/replace all other occurrences (also apply the same fix to the entries
noted at lines 196-198) so published release notes point to
RedHatInsights/insights-rbac-ui instead of the fork.
- Line 4: Several release subsection headings are using "###" (for example the
"### Bug Fixes" heading) which causes a jump from "#" to "###"; update these
headings to "##" so they are promoted by one level. Search for subsection
headings that begin with "###" in the changelog (e.g., "### Bug Fixes" and the
other similar subsection headings flagged in the review) and replace the leading
"###" with "##" consistently.
In `@src/data/queries/audit.ts`:
- Around line 19-20: The hook's queryFn is calling the generated RBAC client
method directly (auditApi.getAuditlogs(params)), which must be invoked using the
documented cast pattern to avoid runtime failures; update the call inside
queryFn (in this file's audit query hook) to cast the method before invoking it
(i.e., use the (auditApi.getAuditlogs as any)(...) pattern) so the RBAC client
method is called via the documented (api.method as any) call pattern.
In `@src/features/audit-log/AuditLog.tsx`:
- Around line 41-47: The table is only wired for pagination: update the
useTableState call in AuditLog.tsx (the const tableState = useTableState<typeof
columns, AuditLogRow>...) to include filter support and then pass the filter
props into TableView; specifically ensure you expose filterConfig, filters, and
onFiltersChange from tableState (or provide them via the same state hook) and
add filterConfig={...}, filters={tableState.filters}, and
onFiltersChange={tableState.setFilters} (or the appropriate handlers) to the
TableView component so the new filter UI is actually enabled.
- Line 38: The file currently only has a default export for the page component;
add a named export for the component symbol (AuditLog) so the page/container is
exported both as default and as a named export; update the export lines so that
the React component declaration remains (const AuditLog: React.FC = () => { ...
}) and you add/export the named symbol (export { AuditLog };) while leaving the
default export in place—also apply the same named-export change for the other
page component instance referenced around the second occurrence (the one at the
later export on line ~115).
- Around line 49-55: The queryParams currently sort by id ascending (orderBy:
'id') which yields oldest-first; change the audit feed to sort newest-first
before paginating by updating the queryParams in AuditLog.tsx (the
GetAuditlogsParams object constructed in the useMemo) to request descending id
order (e.g., use an API-supported descending spec such as orderBy: 'id:desc' or
add an orderDirection: 'desc' / order: 'desc' field depending on the backend
contract) so pages return newest entries first and pagination remains stable.
In `@src/features/audit-log/AuditLogTable.stories.tsx`:
- Around line 189-209: The test assertions look for exact strings "Audit action
6" and "Audit action 21" but buildMockEntriesForPagination renders entries like
"Audit action N: ...", so update the expectations in AuditLogTable.stories.tsx
to match the actual mock text (e.g., use canvas.getByText with a regex or
substring that includes the colon and following text such as /Audit action 6:/
and /Audit action 21:/, or assert via contains/startsWith) for the initial page
check, the post-pagination check, and the absence check (canvas.queryByText
should use a matching pattern for "Audit action 6:" to confirm it's not
present).
- Around line 162-168: The story export named "Error" shadows the global Error
constructor; rename the export (e.g., ErrorState or ErrorStory) wherever it's
declared (export const Error: Story) and update any references to that story
export (imports/usages within this file or Storybook configs) so the new
identifier is used instead, ensuring the Story type annotation remains (e.g.,
export const ErrorState: Story = { args: { ... } }).
---
Outside diff comments:
In `@src/v2/data/queries/audit.ts`:
- Around line 19-21: The call inside the async queryFn to auditApi.getAuditlogs
should use the repo's RBAC-client cast workaround to avoid broken TypeScript
types; replace the direct invocation auditApi.getAuditlogs(params) with the
casted form (auditApi.getAuditlogs as any)(params) (i.e., cast the method to any
before calling) so that queryFn returns response.data safely; update the
invocation within the queryFn block where auditApi.getAuditlogs is called.
---
Nitpick comments:
In `@src/user-journeys/access-management/AuditLog.stories.tsx`:
- Around line 66-68: The docs table claims filtering by action but the existing
Storybook stories only exercise requester and resource; add a new story in
AuditLog.stories.tsx (e.g., AuditLogActionFilter or AuditLog_Play_Action) that
renders the same AuditLog component and in its play function selects the action
<select> control and verifies the filter behavior (mirror how the existing
AuditLog_Play_Requester and AuditLog_Play_Resource stories operate), using the
same query selectors/assertions to choose an action option and assert results
update; ensure the new story name is exported so Storybook picks it up and
follow the same test utilities and imports used by the other play functions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8bef961b-73a4-4a7e-8c73-d85090fded90
📒 Files selected for processing (15)
CHANGELOG.mdsrc/Messages.jssrc/data/api/audit.tssrc/data/api/index.tssrc/data/queries/audit.tssrc/features/audit-log/AuditLog.tsxsrc/features/audit-log/AuditLogTable.stories.tsxsrc/features/audit-log/AuditLogTable.tsxsrc/user-journeys/access-management/AuditLog.stories.tsxsrc/v2/data/api/audit.tssrc/v2/data/mocks/audit.fixtures.tssrc/v2/data/mocks/audit.handlers.tssrc/v2/data/queries/audit.tssrc/v2/features/audit-log/AuditLog.tsxsrc/v2/features/audit-log/README.md
| # 1.0.0 (2026-03-12) | ||
|
|
||
|
|
||
| ### Bug Fixes |
There was a problem hiding this comment.
Promote these section headings by one level.
The new release block jumps from # straight to ###, which is why markdownlint is flagging MD001 here. Use ## for these section headings.
Also applies to: 194-194, 321-321, 326-326, 335-335
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 4-4: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CHANGELOG.md` at line 4, Several release subsection headings are using "###"
(for example the "### Bug Fixes" heading) which causes a jump from "#" to "###";
update these headings to "##" so they are promoted by one level. Search for
subsection headings that begin with "###" in the changelog (e.g., "### Bug
Fixes" and the other similar subsection headings flagged in the review) and
replace the leading "###" with "##" consistently.
| * add sast tasks ([#1787](https://github.com/CodyWMitchell/insights-rbac-ui/issues/1787)) ([9442e25](https://github.com/CodyWMitchell/insights-rbac-ui/commit/9442e2568bbd2536885bfbc4e71a7c9cb772a3ab)) | ||
| * **addGroup:** incorporate pf6 to branch ([3c1436a](https://github.com/CodyWMitchell/insights-rbac-ui/commit/3c1436a48f1750170a2a2065c595be5b18e30105)) | ||
| * **AddGroupMembers:** restored filtering capabilities ([0ba659a](https://github.com/CodyWMitchell/insights-rbac-ui/commit/0ba659a6a5c8f7f3120cdc13716340bb62da6987)) | ||
| * **AddGroupWizard:** ensure service accounts are properly added ([6393cd6](https://github.com/CodyWMitchell/insights-rbac-ui/commit/6393cd650b7a03eba4aaa2f34962e5ac224b9ff2)) | ||
| * **AddGroupWizard:** fix lint ([61fd6e3](https://github.com/CodyWMitchell/insights-rbac-ui/commit/61fd6e3b75a19ab82168bbece5e5b436df12c0aa)) | ||
| * address CodeRabbit review feedback ([6c81602](https://github.com/CodyWMitchell/insights-rbac-ui/commit/6c81602a47129f6975ba3fa7090eba9359129c3b)) | ||
| * address console.warns ([c6d342d](https://github.com/CodyWMitchell/insights-rbac-ui/commit/c6d342d3ab2cc9a328294964b68bfcc479853755)) | ||
| * address remaining CodeRabbit review findings ([ad74c06](https://github.com/CodyWMitchell/insights-rbac-ui/commit/ad74c06477f99b136e5dea44be502cbbc13d7469)) | ||
| * address skipped Storybook tests ([8d84829](https://github.com/CodyWMitchell/insights-rbac-ui/commit/8d848294883b95a8fb9324c0524a84496391e4b6)) | ||
| * **admin-group:** when we don't have admin group do not break ([#1765](https://github.com/CodyWMitchell/insights-rbac-ui/issues/1765)) ([881e1da](https://github.com/CodyWMitchell/insights-rbac-ui/commit/881e1dad988d8e624a7eb4f245ffa1e2b1a5cd66)) | ||
| * allow users to see the full list of permissions for their own profile ([#1858](https://github.com/CodyWMitchell/insights-rbac-ui/issues/1858)) ([0d0d4de](https://github.com/CodyWMitchell/insights-rbac-ui/commit/0d0d4de06a05e54e5cca1b6597b176b9ff669bd1)) | ||
| * **api-arg:** js clients has broken arguments so we have to use list of args ([#1844](https://github.com/CodyWMitchell/insights-rbac-ui/issues/1844)) ([e3f2d4a](https://github.com/CodyWMitchell/insights-rbac-ui/commit/e3f2d4a02b78c869bd1dab30048f9f4e4402881d)) | ||
| * attempt at fixing issue with user access roles not seeing what they should in rbac UI ([f1c44d1](https://github.com/CodyWMitchell/insights-rbac-ui/commit/f1c44d1ae82713ba11f4dc485f5de15101f1e540)) |
There was a problem hiding this comment.
Regenerate this entry against the upstream repo.
These new changelog links point at CodyWMitchell/insights-rbac-ui instead of RedHatInsights/insights-rbac-ui, so the published release notes will send readers to fork-specific issue and commit pages.
Also applies to: 196-198
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CHANGELOG.md` around lines 6 - 18, Update the CHANGELOG.md entries that link
to the fork by replacing the username segment "CodyWMitchell" with the upstream
"RedHatInsights" in the commit and issue URLs (e.g., the links in the shown
entries referencing commits like 9442e25, 3c1436a, 0ba659a, etc.); regenerate or
search/replace all other occurrences (also apply the same fix to the entries
noted at lines 196-198) so published release notes point to
RedHatInsights/insights-rbac-ui instead of the fork.
| queryFn: async () => { | ||
| const response = await auditApi.getAuditlogs(params); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd 'APIClientPatterns\.mdx$' -x sed -n '1,200p' {}
rg -nP --type=ts --type=tsx '\.\s*getAuditlogs\s*\(' src
rg -nP --type=ts --type=tsx '\(\s*[^)]*\.getAuditlogs\s+as any\s*\)\s*\(' srcRepository: RedHatInsights/insights-rbac-ui
Length of output: 5036
🏁 Script executed:
# Read the audit.ts file to see the actual code
cat -n src/data/queries/audit.ts | head -30
# Search for auditApi imports and usage more reliably
rg -n "auditApi" src/data/queries/audit.ts
rg -n "import.*auditApi\|from.*audit" src/data/queries/audit.tsRepository: RedHatInsights/insights-rbac-ui
Length of output: 1488
🏁 Script executed:
cat -n src/data/api/audit.ts | head -50Repository: RedHatInsights/insights-rbac-ui
Length of output: 1110
🏁 Script executed:
# Search for similar query patterns with response.data
rg -n "response\.data" src/data/queries/ -A 1 -B 1Repository: RedHatInsights/insights-rbac-ui
Length of output: 262
🏁 Script executed:
# Check the getAuditlogs response type definition
rg -n "AuditLogPagination\|GetAuditlogsResponse" src/ -A 3
# Search for how other API calls return responses
rg -n "queryFn:\s*async" src/data/queries/ -A 3 | head -40Repository: RedHatInsights/insights-rbac-ui
Length of output: 320
🏁 Script executed:
# Search for AuditLogPagination type definition more thoroughly
rg -n "AuditLogPagination" src/data/api/ -B 2 -A 5
# Check what type getAuditlogs returns from rbac-client
rg -n "export type" src/data/api/audit.tsRepository: RedHatInsights/insights-rbac-ui
Length of output: 1263
🏁 Script executed:
# Search for correct (as any) pattern usage with rbac-client calls
rg -n "\(as any\)" src/data/ -B 1 -A 1 | head -30Repository: RedHatInsights/insights-rbac-ui
Length of output: 57
🏁 Script executed:
# Search entire src for (as any) pattern to see if it's used elsewhere
rg -n "\(as any\)" src/ | head -20
# Also search for any rbac-client API calls to see what pattern is standard
rg -n "Api\." src/data/ -A 1 | grep -E "(queryFn|return|const)" | head -30Repository: RedHatInsights/insights-rbac-ui
Length of output: 224
🏁 Script executed:
# Check if there are other direct API calls to rbac-client methods that might need the cast
rg -n "getAuditlogs\|listRoles\|listUsers\|listGroups\|listPolicies" src/data/ | grep -v "import"Repository: RedHatInsights/insights-rbac-ui
Length of output: 57
Use the documented (api.method as any) call pattern here.
Line 20 calls the generated RBAC client directly. In this repo, that pattern is known to break at runtime, so this hook should cast the method before invoking it.
Suggested fix
- const response = await auditApi.getAuditlogs(params);
+ const response = await (auditApi.getAuditlogs as any)(params);As per coding guidelines: "If the PR adds direct calls to @redhat-cloud-services/rbac-client methods WITHOUT the (api.method as any) cast pattern, flag it. The client types are broken and direct usage causes silent runtime failures."
📝 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.
| queryFn: async () => { | |
| const response = await auditApi.getAuditlogs(params); | |
| queryFn: async () => { | |
| const response = await (auditApi.getAuditlogs as any)(params); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/data/queries/audit.ts` around lines 19 - 20, The hook's queryFn is
calling the generated RBAC client method directly
(auditApi.getAuditlogs(params)), which must be invoked using the documented cast
pattern to avoid runtime failures; update the call inside queryFn (in this
file's audit query hook) to cast the method before invoking it (i.e., use the
(auditApi.getAuditlogs as any)(...) pattern) so the RBAC client method is called
via the documented (api.method as any) call pattern.
| }; | ||
| } | ||
|
|
||
| const AuditLog: React.FC = () => { |
There was a problem hiding this comment.
Add a named export for the component itself.
The type re-export on Line 14 does not satisfy the feature export rule. This file can keep the default export, but the page component should also be exported as a named symbol.
Suggested fix
-const AuditLog: React.FC = () => {
+export const AuditLog: React.FC = () => {
const intl = useIntl();
...
};
export default AuditLog;As per coding guidelines, "Features (route/page containers, i.e. files directly under src/features/<island>/) may additionally export a default, but must always have a named export too."
Also applies to: 115-115
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/audit-log/AuditLog.tsx` at line 38, The file currently only has
a default export for the page component; add a named export for the component
symbol (AuditLog) so the page/container is exported both as default and as a
named export; update the export lines so that the React component declaration
remains (const AuditLog: React.FC = () => { ... }) and you add/export the named
symbol (export { AuditLog };) while leaving the default export in place—also
apply the same named-export change for the other page component instance
referenced around the second occurrence (the one at the later export on line
~115).
| const tableState = useTableState<typeof columns, AuditLogRow>({ | ||
| columns, | ||
| getRowId: (row) => row.id, | ||
| initialPerPage: 20, | ||
| perPageOptions: [10, 20, 50], | ||
| syncWithUrl: true, | ||
| }); |
There was a problem hiding this comment.
This page still never exposes the new filters.
useTableState is initialized for pagination only, and TableView never receives filterConfig, filters, or onFiltersChange. So this route remains a plain paginated table even though filtering was added elsewhere in the audit-log UI.
Also applies to: 96-109
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/audit-log/AuditLog.tsx` around lines 41 - 47, The table is only
wired for pagination: update the useTableState call in AuditLog.tsx (the const
tableState = useTableState<typeof columns, AuditLogRow>...) to include filter
support and then pass the filter props into TableView; specifically ensure you
expose filterConfig, filters, and onFiltersChange from tableState (or provide
them via the same state hook) and add filterConfig={...},
filters={tableState.filters}, and onFiltersChange={tableState.setFilters} (or
the appropriate handlers) to the TableView component so the new filter UI is
actually enabled.
| const queryParams: GetAuditlogsParams = useMemo( | ||
| () => ({ | ||
| limit: tableState.perPage, | ||
| offset: (tableState.page - 1) * tableState.perPage, | ||
| orderBy: 'id' as const, | ||
| }), | ||
| [tableState.perPage, tableState.page], |
There was a problem hiding this comment.
Sort the feed newest-first before paginating.
Line 53 uses orderBy: 'id', which sorts this page differently from the v2 audit-log page and from the visible Date column. With server-side pagination, that can put the oldest entries on page 1 and reshuffle later pages when new events arrive.
Suggested fix
- orderBy: 'id' as const,
+ orderBy: '-created' as const,📝 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.
| const queryParams: GetAuditlogsParams = useMemo( | |
| () => ({ | |
| limit: tableState.perPage, | |
| offset: (tableState.page - 1) * tableState.perPage, | |
| orderBy: 'id' as const, | |
| }), | |
| [tableState.perPage, tableState.page], | |
| const queryParams: GetAuditlogsParams = useMemo( | |
| () => ({ | |
| limit: tableState.perPage, | |
| offset: (tableState.page - 1) * tableState.perPage, | |
| orderBy: '-created' as const, | |
| }), | |
| [tableState.perPage, tableState.page], |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/audit-log/AuditLog.tsx` around lines 49 - 55, The queryParams
currently sort by id ascending (orderBy: 'id') which yields oldest-first; change
the audit feed to sort newest-first before paginating by updating the
queryParams in AuditLog.tsx (the GetAuditlogsParams object constructed in the
useMemo) to request descending id order (e.g., use an API-supported descending
spec such as orderBy: 'id:desc' or add an orderDirection: 'desc' / order: 'desc'
field depending on the backend contract) so pages return newest entries first
and pagination remains stable.
| export const Error: Story = { | ||
| args: { | ||
| entries: [], | ||
| totalCount: 0, | ||
| isLoading: false, | ||
| error: 'Failed to load audit log. Please try again.', | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "AuditLogTable.stories.tsx" -type fRepository: RedHatInsights/insights-rbac-ui
Length of output: 245
🏁 Script executed:
cat -n src/features/audit-log/AuditLogTable.stories.tsx | sed -n '155,175p'Repository: RedHatInsights/insights-rbac-ui
Length of output: 820
🏁 Script executed:
# Check if biome/eslint config exists and look for noShadowRestrictedNames rule
find . -maxdepth 2 -name "biome.json*" -o -name ".eslintrc*" -o -name "eslint.config.*" | head -5Repository: RedHatInsights/insights-rbac-ui
Length of output: 106
🏁 Script executed:
cat biome.json | head -100Repository: RedHatInsights/insights-rbac-ui
Length of output: 1849
🏁 Script executed:
# Check for noShadowRestrictedNames rule in biome config
rg "noShadowRestrictedNames" biome.jsonRepository: RedHatInsights/insights-rbac-ui
Length of output: 57
🏁 Script executed:
# Check eslint config for any rules related to shadowing
cat eslint.config.js | grep -A 5 -B 5 "shadow\|Shadow" || echo "No shadow rules found in eslint.config.js"Repository: RedHatInsights/insights-rbac-ui
Length of output: 116
🏁 Script executed:
# Check if there are any linting violations in the specific file
rg "noShadowRestrictedNames" . --type json -A 2 -B 2Repository: RedHatInsights/insights-rbac-ui
Length of output: 57
🏁 Script executed:
# Let's check the Biome documentation reference in the schema to understand the default behavior
# Also check if the Error export is actually flagged anywhere in the codebase
rg "export const Error" src/features/audit-log/AuditLogTable.stories.tsxRepository: RedHatInsights/insights-rbac-ui
Length of output: 104
🏁 Script executed:
# Let's see if there are any recent commits or lint reports mentioning this issue
git log --oneline -20 -- src/features/audit-log/AuditLogTable.stories.tsx 2>/dev/null || echo "Git history not available"Repository: RedHatInsights/insights-rbac-ui
Length of output: 128
🏁 Script executed:
# Check what the actual component file looks like to verify this is indeed a story file
ls -la src/features/audit-log/ | head -20Repository: RedHatInsights/insights-rbac-ui
Length of output: 390
🏁 Script executed:
# Verify the Biome rule by checking official Biome documentation or testing the specific rule
# Let's check if there are any other similar violations in the codebase
rg "export const (Error|Array|Object|Function|String|Boolean|Number|Symbol|BigInt)" --type tsx src/Repository: RedHatInsights/insights-rbac-ui
Length of output: 102
🏁 Script executed:
# Check the biome documentation inline in a comment or any related rules being enforced
# Let's understand the default severity of noShadowRestrictedNames in Biome
python3 << 'EOF'
# Biome v2.4.7 noShadowRestrictedNames rule information
# This rule is part of the 'correctness' ruleset and is included in 'recommended': true
# The rule detects when you shadow names of restricted global variables
# Examples: Error, Array, Object, String, Boolean, Number, Symbol, Map, Set, etc.
# When included in recommended rules without explicit override, the default level is 'warn' or 'error'
# For this codebase with "recommended": true, the rule should be ACTIVE
restricted_names = [
'Error', 'Array', 'Object', 'Function', 'String', 'Boolean',
'Number', 'Symbol', 'BigInt', 'Map', 'Set', 'WeakMap', 'WeakSet'
]
print("Biome noShadowRestrictedNames rule details:")
print(f"- Rule name: noShadowRestrictedNames")
print(f"- Category: correctness")
print(f"- Status: Active (via 'recommended': true in biome.json)")
print(f"- Restricted names include: {', '.join(restricted_names)}")
print(f"- Violation in file: export const Error triggers this rule")
EOFRepository: RedHatInsights/insights-rbac-ui
Length of output: 423
Rename this story export to avoid shadowing the global Error constructor.
export const Error: Story shadows the global Error constructor and violates Biome's noShadowRestrictedNames rule, which is active in this codebase's linter configuration.
🔧 Suggested fix
-export const Error: Story = {
+export const ErrorState: Story = {📝 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.
| export const Error: Story = { | |
| args: { | |
| entries: [], | |
| totalCount: 0, | |
| isLoading: false, | |
| error: 'Failed to load audit log. Please try again.', | |
| }, | |
| export const ErrorState: Story = { | |
| args: { | |
| entries: [], | |
| totalCount: 0, | |
| isLoading: false, | |
| error: 'Failed to load audit log. Please try again.', | |
| }, |
🧰 Tools
🪛 Biome (2.4.7)
[error] 162-162: Do not shadow the global "Error" property.
(lint/suspicious/noShadowRestrictedNames)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/audit-log/AuditLogTable.stories.tsx` around lines 162 - 168, The
story export named "Error" shadows the global Error constructor; rename the
export (e.g., ErrorState or ErrorStory) wherever it's declared (export const
Error: Story) and update any references to that story export (imports/usages
within this file or Storybook configs) so the new identifier is used instead,
ensuring the Story type annotation remains (e.g., export const ErrorState: Story
= { args: { ... } }).
| // Wait for first page: one of the first-page-only entries (id 6 = "Audit action 6") | ||
| await waitFor(() => { | ||
| expect(canvas.getByText('Audit action 6')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| // Pagination should show total (e.g. "1 - 20 of 25" or "25" in total-items) | ||
| const paginationRegion = canvasElement.querySelector('.pf-v6-c-pagination, [class*="pagination"]'); | ||
| expect(paginationRegion).toBeInTheDocument(); | ||
| expect(canvasElement.textContent).toMatch(/25/); | ||
|
|
||
| // Go to next page | ||
| const nextButtons = canvas.getAllByRole('button', { name: /next/i }); | ||
| expect(nextButtons.length).toBeGreaterThan(0); | ||
| await userEvent.click(nextButtons[0]); | ||
|
|
||
| // Page 2 should show entries 21–25 (first on page 2 is "Audit action 21") | ||
| await waitFor(() => { | ||
| expect(canvas.getByText('Audit action 21')).toBeInTheDocument(); | ||
| }); | ||
| // First-page-only entry should not be visible | ||
| expect(canvas.queryByText('Audit action 6')).not.toBeInTheDocument(); |
There was a problem hiding this comment.
Match the pagination assertions to the actual mock text.
Lines 191, 206, and 209 query for Audit action 6 / Audit action 21, but buildMockEntriesForPagination renders Audit action N: .... These exact-string lookups will fail before the page-change behavior is validated.
🔧 Suggested fix
- await waitFor(() => {
- expect(canvas.getByText('Audit action 6')).toBeInTheDocument();
- });
+ await expect(canvas.findByText(/Audit action 6:/i)).resolves.toBeInTheDocument();
@@
- await waitFor(() => {
- expect(canvas.getByText('Audit action 21')).toBeInTheDocument();
- });
+ await expect(canvas.findByText(/Audit action 21:/i)).resolves.toBeInTheDocument();
@@
- expect(canvas.queryByText('Audit action 6')).not.toBeInTheDocument();
+ expect(canvas.queryByText(/Audit action 6:/i)).not.toBeInTheDocument();📝 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.
| // Wait for first page: one of the first-page-only entries (id 6 = "Audit action 6") | |
| await waitFor(() => { | |
| expect(canvas.getByText('Audit action 6')).toBeInTheDocument(); | |
| }); | |
| // Pagination should show total (e.g. "1 - 20 of 25" or "25" in total-items) | |
| const paginationRegion = canvasElement.querySelector('.pf-v6-c-pagination, [class*="pagination"]'); | |
| expect(paginationRegion).toBeInTheDocument(); | |
| expect(canvasElement.textContent).toMatch(/25/); | |
| // Go to next page | |
| const nextButtons = canvas.getAllByRole('button', { name: /next/i }); | |
| expect(nextButtons.length).toBeGreaterThan(0); | |
| await userEvent.click(nextButtons[0]); | |
| // Page 2 should show entries 21–25 (first on page 2 is "Audit action 21") | |
| await waitFor(() => { | |
| expect(canvas.getByText('Audit action 21')).toBeInTheDocument(); | |
| }); | |
| // First-page-only entry should not be visible | |
| expect(canvas.queryByText('Audit action 6')).not.toBeInTheDocument(); | |
| // Wait for first page: one of the first-page-only entries (id 6 = "Audit action 6") | |
| await expect(canvas.findByText(/Audit action 6:/i)).resolves.toBeInTheDocument(); | |
| // Pagination should show total (e.g. "1 - 20 of 25" or "25" in total-items) | |
| const paginationRegion = canvasElement.querySelector('.pf-v6-c-pagination, [class*="pagination"]'); | |
| expect(paginationRegion).toBeInTheDocument(); | |
| expect(canvasElement.textContent).toMatch(/25/); | |
| // Go to next page | |
| const nextButtons = canvas.getAllByRole('button', { name: /next/i }); | |
| expect(nextButtons.length).toBeGreaterThan(0); | |
| await userEvent.click(nextButtons[0]); | |
| // Page 2 should show entries 21–25 (first on page 2 is "Audit action 21") | |
| await expect(canvas.findByText(/Audit action 21:/i)).resolves.toBeInTheDocument(); | |
| // First-page-only entry should not be visible | |
| expect(canvas.queryByText(/Audit action 6:/i)).not.toBeInTheDocument(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/audit-log/AuditLogTable.stories.tsx` around lines 189 - 209, The
test assertions look for exact strings "Audit action 6" and "Audit action 21"
but buildMockEntriesForPagination renders entries like "Audit action N: ...", so
update the expectations in AuditLogTable.stories.tsx to match the actual mock
text (e.g., use canvas.getByText with a regex or substring that includes the
colon and following text such as /Audit action 6:/ and /Audit action 21:/, or
assert via contains/startsWith) for the initial page check, the post-pagination
check, and the absence check (canvas.queryByText should use a matching pattern
for "Audit action 6:" to confirm it's not present).
|
This branch looks like it was started before the v1/v2 split freeze and has pulled in a massive amount of cross-boundary conflicts and auto-generated artifacts (like the bot-managed CHANGELOG.md). > The actual fix for this ticket in the new v2 architecture only requires touching a few lines. I'm going to close this PR so we don't accidentally merge this git history. Let's sync up this morning so I can show you exactly where this goes in the new structure! |
3 participants
What and why
RHCLOUD-28101
Screenshots
Anything non-obvious reviewers should know?
Attention needed
Summary by CodeRabbit
New Features
Documentation