feat(aurora): security group data grid, permissions#952
Draft
KirylSAP wants to merge 5 commits into
Draft
Conversation
Contributor
📝 WalkthroughWalkthroughIntroduces a ChangesSecurity Group Permissions Rollout
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Route as SecurityGroupId Route
participant Hook as useSecurityGroupPermissions
participant DetailsView as SecurityGroupDetailsView
participant RulesTable as SecurityGroupRulesTable
participant RBACPolicies as SecurityGroupRBACPolicies
Route->>Hook: useSecurityGroupPermissions(projectId)
Hook-->>Route: permissions / isLoading / isError
Route->>DetailsView: permissions (or safePermissions)
DetailsView->>RulesTable: canCreateRule, canDeleteRule
DetailsView->>RBACPolicies: canManageAccess
RulesTable-->>DetailsView: gated Add rule / delete actions
RBACPolicies-->>DetailsView: gated Share / remove actions
sequenceDiagram
participant User
participant SecurityGroupsList
participant urlHelpers
participant Router
User->>SecurityGroupsList: change filter/sort/search
SecurityGroupsList->>urlHelpers: applyFilterSelection/buildUrlSearchParams
urlHelpers-->>SecurityGroupsList: updated params
SecurityGroupsList->>Router: navigate({ search, replace: true })
Router-->>SecurityGroupsList: updated searchParams
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupRBACPolicies.test.tsx (1)
1-632: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winNo test coverage for
canManageAccess={false}.Every render call in this file passes
canManageAccess={true}. This leaves the permission-gating logic — hiding the "Share Security Group" button and disabling delete onRBACPolicyRowviacanDelete={canManageAccess}— completely untested for the restricted case. Note also thatRBACPolicyRowis mocked here (lines 40-48) and always renders itsDeletebutton regardless ofcanDelete, so even acanManageAccess={false}test using the current mock wouldn't verify the button is hidden — the mock would need to respectcanDeletetoo for such a test to be meaningful.Consider adding a test with
canManageAccess={false}asserting the Share button is absent, plus updating theRBACPolicyRowmock to honorcanDelete.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/aurora/src/client/routes/_auth/projects/`$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupRBACPolicies.test.tsx around lines 1 - 632, Add coverage for the restricted permission path in SecurityGroupRBACPolicies by introducing a test that renders with canManageAccess=false and asserts the Share Security Group action is hidden. Update the RBACPolicyRow mock to respect the canDelete prop so it only renders the Delete button when deletion is allowed, otherwise the test will not verify the real gating behavior. Use the existing SecurityGroupRBACPolicies render setup and RBACPolicyRow mock symbol to keep the new case aligned with the current test structure.packages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupRulesTable.test.tsx (1)
95-618: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winNo test coverage for
canCreateRule={false}/canDeleteRule={false}.Every render call across this file passes
canCreateRule={true}andcanDeleteRule={true}. Since gating the Add rule button, Actions column, delete menu, delete dialog, and add-rule modal on these exact flags is the entire purpose of this change, there is currently no test that would catch a regression such as an inverted condition (e.g.!canDeleteRule) or an accidentally-always-true gate.Consider adding at least two tests:
canCreateRule={false}→ "Add rule" button is not rendered even whenonCreateRule/securityGroupIdare provided.canDeleteRule={false}→ "Actions" header, per-row delete menu, andDeleteRuleDialogare not rendered.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/aurora/src/client/routes/_auth/projects/`$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupRulesTable.test.tsx around lines 95 - 618, The SecurityGroupRulesTable test suite is missing coverage for the permission gates, so add assertions for the canCreateRule and canDeleteRule branches in SecurityGroupRulesTable. Introduce a test that renders with canCreateRule set to false while still providing onCreateRule and securityGroupId, and verify the Add rule button is hidden. Also add a test that renders with canDeleteRule set to false and verify the Actions header and delete-related UI (row delete menu / DeleteRuleDialog trigger) are not shown, so regressions in the gating logic are caught.
🧹 Nitpick comments (1)
packages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/-components/-modals/EditSecurityGroupModal.tsx (1)
148-149: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNo user-visible explanation for the missing Stateful field.
The prior informational message about the
statefulrestriction was removed and replaced only with a JSX comment ({/* Note: ... */}), which is never rendered. Users editing the security group now see no indication of why the Stateful checkbox disappeared, unlike before. This is also inconsistent with the summary's description of "an inline note about elevated permission" — there is no visible note in the actual UI.Consider rendering a brief
Message/help text in the form instead of a code-only comment.💡 Example: surface a visible note
</FormRow> + <FormRow className="mb-0"> + <Message variant="info" dismissible={false}> + <Trans>Stateful cannot be changed here. Updating it requires elevated permissions.</Trans> + </Message> + </FormRow> - {/* Note: Stateful checkbox is not shown here because it requires 'update_security_group:stateful' permission (cloud admin only) */}Also applies to: 187-187
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/aurora/src/client/routes/_auth/projects/`$projectId/network/securitygroups/-components/-modals/EditSecurityGroupModal.tsx around lines 148 - 149, The Stateful restriction note is currently only present as a JSX comment in EditSecurityGroupModal and never reaches the UI, so users get no explanation for the missing field. Update EditSecurityGroupModal to render a visible inline help/message element near the stateful checkbox area (and keep it consistent with the elevated-permission summary) instead of relying on the comment-only placeholder, so the restriction is clearly shown when editing a security group.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/aurora/src/client/routes/_auth/projects/`$projectId/network/securitygroups/-components/SecurityGroupsList.tsx:
- Around line 306-312: The CreateSecurityGroupModal usage in SecurityGroupsList
is not wired to the mutation state and leaves stale errors behind. Update the
create flow so the modal’s loading prop uses the createSecurityGroupMutation
pending state instead of a hardcoded false, and make the close handler clear
createError before or when setting createModalOpen to false, matching the
behavior used for the Edit modal in SecurityGroupListContainer.
- Around line 40-72: The toolbar state in SecurityGroupsList is only initialized
from searchParams once, so sortSettings, filterSettings, and searchTerm can get
out of sync when the URL changes. Update SecurityGroupsList to derive these
controls directly from searchParams or add an effect that resyncs the local
state whenever searchParams changes, using the existing parseFiltersFromUrl,
sortSettings, filterSettings, and searchTerm state as the key places to adjust.
In
`@packages/aurora/src/client/routes/_auth/projects/`$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupRulesTable.tsx:
- Around line 66-75: `localSearchTerm` in `SecurityGroupRulesTable` is only
initialized from the `searchTerm` prop once, so it can drift from later parent
updates. Add an effect to keep `localSearchTerm` in sync whenever `searchTerm`
changes, and make sure the search input state reflects external resets like
clear-filters or URL-driven updates. Use the existing `useState`, `useEffect`,
and `debounceTimer` setup in `SecurityGroupRulesTable` to locate the change.
---
Outside diff comments:
In
`@packages/aurora/src/client/routes/_auth/projects/`$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupRBACPolicies.test.tsx:
- Around line 1-632: Add coverage for the restricted permission path in
SecurityGroupRBACPolicies by introducing a test that renders with
canManageAccess=false and asserts the Share Security Group action is hidden.
Update the RBACPolicyRow mock to respect the canDelete prop so it only renders
the Delete button when deletion is allowed, otherwise the test will not verify
the real gating behavior. Use the existing SecurityGroupRBACPolicies render
setup and RBACPolicyRow mock symbol to keep the new case aligned with the
current test structure.
In
`@packages/aurora/src/client/routes/_auth/projects/`$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupRulesTable.test.tsx:
- Around line 95-618: The SecurityGroupRulesTable test suite is missing coverage
for the permission gates, so add assertions for the canCreateRule and
canDeleteRule branches in SecurityGroupRulesTable. Introduce a test that renders
with canCreateRule set to false while still providing onCreateRule and
securityGroupId, and verify the Add rule button is hidden. Also add a test that
renders with canDeleteRule set to false and verify the Actions header and
delete-related UI (row delete menu / DeleteRuleDialog trigger) are not shown, so
regressions in the gating logic are caught.
---
Nitpick comments:
In
`@packages/aurora/src/client/routes/_auth/projects/`$projectId/network/securitygroups/-components/-modals/EditSecurityGroupModal.tsx:
- Around line 148-149: The Stateful restriction note is currently only present
as a JSX comment in EditSecurityGroupModal and never reaches the UI, so users
get no explanation for the missing field. Update EditSecurityGroupModal to
render a visible inline help/message element near the stateful checkbox area
(and keep it consistent with the elevated-permission summary) instead of relying
on the comment-only placeholder, so the restriction is clearly shown when
editing a security group.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5e491a1a-6e78-4ab6-be0b-938d9672d9a5
📒 Files selected for processing (24)
packages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/RBACPolicyRow.tsxpackages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupBasicInfo.tsxpackages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupRBACPolicies.test.tsxpackages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupRBACPolicies.tsxpackages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupRulesTable.test.tsxpackages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupRulesTable.tsxpackages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/$securityGroupId/-components/SecurityGroupDetailsView.test.tsxpackages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/$securityGroupId/-components/SecurityGroupDetailsView.tsxpackages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/$securityGroupId/index.tsxpackages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/-components/-modals/CreateSecurityGroupModal.tsxpackages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/-components/-modals/EditSecurityGroupModal.tsxpackages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/-components/SecurityGroupListContainer.test.tsxpackages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/-components/SecurityGroupListContainer.tsxpackages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/-components/SecurityGroupTableRow.test.tsxpackages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/-components/SecurityGroupTableRow.tsxpackages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/-components/SecurityGroupsList.test.tsxpackages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/-components/SecurityGroupsList.tsxpackages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/-hooks/useSecurityGroupPermissions.tspackages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/index.tsxpackages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/urlHelpers.tspackages/aurora/src/locales/de/messages.popackages/aurora/src/locales/de/messages.tspackages/aurora/src/locales/en/messages.popackages/aurora/src/locales/en/messages.ts
💤 Files with no reviewable changes (1)
- packages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/-components/SecurityGroupsList.test.tsx
Comment on lines
+40
to
+72
| const [sortSettings, setSortSettings] = useState<RequiredSortSettings>({ | ||
| options: [ | ||
| { label: t`Name`, value: "name" }, | ||
| { label: t`Project id`, value: "project_id" }, | ||
| ], | ||
| sortBy: searchParams.sortBy || "name", | ||
| sortDirection: searchParams.sortDirection || "asc", | ||
| }) | ||
|
|
||
| const [filterSettings, setFilterSettings] = useState<FilterSettings>({ | ||
| filters: [ | ||
| { | ||
| displayName: t`Shared`, | ||
| filterName: "shared", | ||
| values: ["true", "false"], | ||
| supportsMultiValue: false, | ||
| }, | ||
| ], | ||
| selectedFilters: parseFiltersFromUrl(searchParams), | ||
| }) | ||
|
|
||
| const [searchTerm, setSearchTerm] = useState(searchParams.search || "") | ||
| const [createModalOpen, setCreateModalOpen] = useState(false) | ||
| const [deleteError, setDeleteError] = useState<string | null>(null) | ||
| const [createError, setCreateError] = useState<string | null>(null) | ||
| const [updateError, setUpdateError] = useState<string | null>(null) | ||
|
|
||
| const { searchTerm, sortSettings, filterSettings, handleSearchChange, handleSortChange, handleFilterChange } = | ||
| useListWithFiltering<SecurityGroupSortKey>({ | ||
| defaultSortKey: "name", | ||
| defaultSortDir: "asc", | ||
| sortOptions: [ | ||
| { label: t`Name`, value: "name" }, | ||
| { label: t`Project id`, value: "project_id" }, | ||
| ], | ||
| filterSettings: { | ||
| filters: [ | ||
| { | ||
| displayName: t`Shared`, | ||
| filterName: "shared", | ||
| values: Object.values(SECURITY_GROUP_SHARED), | ||
| supportsMultiValue: false, | ||
| }, | ||
| ], | ||
| }, | ||
| }) | ||
|
|
||
| const utils = trpcReact.useUtils() | ||
|
|
||
| // TODO: replace with trpc.network.canUser when security group permissions are available | ||
| const permissions = { | ||
| canCreate: true, | ||
| canUpdate: true, | ||
| canDelete: true, | ||
| canManageAccess: true, | ||
| } | ||
| const urlFilters = parseFiltersFromUrl(searchParams) | ||
| const urlSortBy = (searchParams.sortBy || "name") as SecurityGroupSortKey | ||
| const urlSortDirection = searchParams.sortDirection || "asc" | ||
| const urlSearchTerm = searchParams.search || "" |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the component and related route definitions.
git ls-files 'packages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/**' \
'packages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/**/*' \
'packages/aurora/src/client/routes/_auth/projects/$projectId/network/**' \
| sed 's#^`#FILE` #'
echo
echo "=== outline: SecurityGroupsList.tsx ==="
ast-grep outline 'packages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/-components/SecurityGroupsList.tsx' --view expanded
echo
echo "=== outline: route files nearby ==="
fd -a 'route.tsx|route.ts|_auth.*securitygroups|securitygroups' packages/aurora/src/client/routes | sed -n '1,120p'Repository: cobaltcore-dev/aurora-dashboard
Length of output: 12207
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the relevant component slices and nearby route integration.
FILE='packages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/-components/SecurityGroupsList.tsx'
echo "=== file line count ==="
wc -l "$FILE"
echo
echo "=== SecurityGroupsList.tsx (selected lines) ==="
sed -n '1,360p' "$FILE" | cat -n | sed -n '1,360p'
echo
echo "=== search for searchParams sync / useEffect / router changes ==="
rg -n 'useEffect|searchParams|setSortSettings|setFilterSettings|setSearchTerm|navigate\(|router|replace\(' "$FILE"
echo
echo "=== sibling route/layout files ==="
fd -a 'route.tsx|route.ts' 'packages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups' | sed -n '1,120p'Repository: cobaltcore-dev/aurora-dashboard
Length of output: 14863
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for FILE in \
'packages/aurora/src/client/routes/_auth/projects/$projectId/network/securitygroups/index.tsx' \
'packages/aurora/src/client/routes/_auth/projects/$projectId/network/index.tsx'
do
echo "=== $FILE ==="
wc -l "$FILE"
sed -n '1,220p' "$FILE" | cat -n
echo
doneRepository: cobaltcore-dev/aurora-dashboard
Length of output: 2239
Keep the toolbar state in sync with the URL. sortSettings, filterSettings, and searchTerm are initialized from searchParams once, but the list query reads fresh searchParams on every render. When the URL changes via back/forward navigation or another in-place update, the data changes while the sort/filter/search controls stay stale. Derive the controls from searchParams or resync the local state in an effect.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/aurora/src/client/routes/_auth/projects/`$projectId/network/securitygroups/-components/SecurityGroupsList.tsx
around lines 40 - 72, The toolbar state in SecurityGroupsList is only
initialized from searchParams once, so sortSettings, filterSettings, and
searchTerm can get out of sync when the URL changes. Update SecurityGroupsList
to derive these controls directly from searchParams or add an effect that
resyncs the local state whenever searchParams changes, using the existing
parseFiltersFromUrl, sortSettings, filterSettings, and searchTerm state as the
key places to adjust.
Comment on lines
306
to
312
| <CreateSecurityGroupModal | ||
| isOpen={createModalOpen} | ||
| onClose={() => setCreateModalOpen(false)} | ||
| onCreate={handleCreateSecurityGroup} | ||
| isLoading={createSecurityGroupMutation.isPending} | ||
| isLoading={false} | ||
| error={createError} | ||
| /> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Create modal loading indicator hardcoded and error not cleared on close.
isLoading={false} ignores createSecurityGroupMutation.isPending, so no spinner shows while creating. Also onClose only closes the modal without clearing createError, so reopening after a failed create re-shows the stale error — the same class of bug flagged for the Edit modal in SecurityGroupListContainer.tsx.
🐛 Proposed fix
<CreateSecurityGroupModal
isOpen={createModalOpen}
- onClose={() => setCreateModalOpen(false)}
+ onClose={() => {
+ setCreateModalOpen(false)
+ setCreateError(null)
+ }}
onCreate={handleCreateSecurityGroup}
- isLoading={false}
+ isLoading={createSecurityGroupMutation.isPending}
error={createError}
/>📝 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
| <CreateSecurityGroupModal | |
| isOpen={createModalOpen} | |
| onClose={() => setCreateModalOpen(false)} | |
| onCreate={handleCreateSecurityGroup} | |
| isLoading={createSecurityGroupMutation.isPending} | |
| isLoading={false} | |
| error={createError} | |
| /> | |
| <CreateSecurityGroupModal | |
| isOpen={createModalOpen} | |
| onClose={() => { | |
| setCreateModalOpen(false) | |
| setCreateError(null) | |
| }} | |
| onCreate={handleCreateSecurityGroup} | |
| isLoading={createSecurityGroupMutation.isPending} | |
| error={createError} | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/aurora/src/client/routes/_auth/projects/`$projectId/network/securitygroups/-components/SecurityGroupsList.tsx
around lines 306 - 312, The CreateSecurityGroupModal usage in SecurityGroupsList
is not wired to the mutation state and leaves stale errors behind. Update the
create flow so the modal’s loading prop uses the createSecurityGroupMutation
pending state instead of a hardcoded false, and make the close handler clear
createError before or when setting createModalOpen to false, matching the
behavior used for the Edit modal in SecurityGroupListContainer.
Comment on lines
+66
to
+75
| canCreateRule, | ||
| canDeleteRule, | ||
| }: SecurityGroupRulesTableProps) { | ||
| const { t } = useLingui() | ||
| const [ruleToDelete, setRuleToDelete] = useState<SecurityGroupRule | null>(null) | ||
| const [isAddRuleModalOpen, toggleAddRuleModal] = useModal() | ||
| const [localSearchTerm, setLocalSearchTerm] = useState(searchTerm) | ||
| const debounceTimer = useRef<number | undefined>(undefined) | ||
|
|
||
| useEffect(() => () => clearTimeout(debounceTimer.current), []) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
localSearchTerm never re-syncs with an externally-changed searchTerm prop.
localSearchTerm is initialized from searchTerm only once via useState(searchTerm). If the parent resets/changes searchTerm later (e.g. a "clear filters" action, tab switch, or URL-driven state reset per the PR's URL-persistence feature), the input will keep showing the stale local value while the actual applied search term silently differs.
🔧 Proposed fix
const [localSearchTerm, setLocalSearchTerm] = useState(searchTerm)
const debounceTimer = useRef<number | undefined>(undefined)
useEffect(() => () => clearTimeout(debounceTimer.current), [])
+
+ useEffect(() => {
+ setLocalSearchTerm(searchTerm)
+ }, [searchTerm])📝 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
| canCreateRule, | |
| canDeleteRule, | |
| }: SecurityGroupRulesTableProps) { | |
| const { t } = useLingui() | |
| const [ruleToDelete, setRuleToDelete] = useState<SecurityGroupRule | null>(null) | |
| const [isAddRuleModalOpen, toggleAddRuleModal] = useModal() | |
| const [localSearchTerm, setLocalSearchTerm] = useState(searchTerm) | |
| const debounceTimer = useRef<number | undefined>(undefined) | |
| useEffect(() => () => clearTimeout(debounceTimer.current), []) | |
| canCreateRule, | |
| canDeleteRule, | |
| }: SecurityGroupRulesTableProps) { | |
| const { t } = useLingui() | |
| const [ruleToDelete, setRuleToDelete] = useState<SecurityGroupRule | null>(null) | |
| const [isAddRuleModalOpen, toggleAddRuleModal] = useModal() | |
| const [localSearchTerm, setLocalSearchTerm] = useState(searchTerm) | |
| const debounceTimer = useRef<number | undefined>(undefined) | |
| useEffect(() => () => clearTimeout(debounceTimer.current), []) | |
| useEffect(() => { | |
| setLocalSearchTerm(searchTerm) | |
| }, [searchTerm]) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/aurora/src/client/routes/_auth/projects/`$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupRulesTable.tsx
around lines 66 - 75, `localSearchTerm` in `SecurityGroupRulesTable` is only
initialized from the `searchTerm` prop once, so it can drift from later parent
updates. Add an effect to keep `localSearchTerm` in sync whenever `searchTerm`
changes, and make sure the search input state reflects external resets like
clear-filters or URL-driven updates. Use the existing `useState`, `useEffect`,
and `debounceTimer` setup in `SecurityGroupRulesTable` to locate the change.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
Fixed permission-related bugs where users without appropriate permissions could see and attempt actions, and where error messages persisted when closing and reopening modals.
Bugs Fixed
Bug 1: Edit on the DataGrid and details of Security group page
Bug 2: Error message persist on the Create security group Modal
Task: Implement DataGrid Header for Security Groups
Changes Made
Core Changes
useSecurityGroupPermissionsthat fetches all 9 security group permissions with proper React Query configuration (infinite cache)canCreateis falseclientprop)DataGrid Refactor
urlHelpers.tsPermission-Based UI Updates
All UI elements now properly respect user permissions:
canCreateis truecanCreateRule/canDeleteRuleare truecanManageAccessis truecanDeleteis truecanUpdateis trueModal State Management
handleClose()to prevent error persistencehandleClose()to prevent error persistenceTest Updates
SecurityGroupListContainer.test.tsxSecurityGroupTableRow.test.tsxSecurityGroupRulesTable.test.tsxSecurityGroupRBACPolicies.test.tsxSecurityGroupDetailsView.test.tsxRelated Issues
Fixes permission visibility bugs where users could see and attempt actions they didn't have permissions for, and where error messages persisted across modal reopens.
Key Technical Details
Permission Structure
All 9 permissions are now consistently checked:
network:security_groups:read→canViewnetwork:security_groups:create→canCreatenetwork:security_groups:update→canUpdatenetwork:security_groups:delete→canDeletenetwork:security_group_rules:create→canCreateRulenetwork:security_group_rules:delete→canDeleteRulenetwork:rbac_policies:create+network:rbac_policies:delete→canManageAccessnetwork:rbac_policies:read→canViewRBACReact Query Configuration
The hook uses:
staleTime: Infinity- permissions don't change during sessiongcTime: Infinity- keep in cache foreverenabled: Boolean(projectId)- only fetch if projectId existsWhy Remove Stateful Field from Edit?
OpenStack Neutron distinguishes between:
create_security_group- allows setting stateful during creationupdate_security_group:stateful- requires cloud admin role to modify after creationRegular users can set stateful during creation but cannot change it later, so the field was removed from the Edit modal.
Testing Instructions
pnpm ipnpm run typecheck- verify no TypeScript errorspnpm run test- verify all tests passChecklist
Summary by CodeRabbit
New Features
Bug Fixes