Delete A2 roles from A3 UI - #2212
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughAdds a feature-flagged role deletion flow: backend RemoveRole contracts, client/service/controller and tests; RTK Query removeRole mutation and type changes; frontend deletion UI (confirmation dialog, delete button), feature-flag helper, styles, localization, and config entries. ChangesRole Deletion Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
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 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds end-user functionality to revoke (delete) legacy Altinn 2 role assignments from the Access Management (A3) UI, guarded by a new enableRoleDeletion feature flag. This includes both frontend UI/UX additions (delete controls + confirmation dialog + updated inherited-status messaging) and backend support (DELETE endpoint + client/service plumbing + env flag config).
Changes:
- Frontend: Introduces role deletion UI (list item control + role info modal control) with a shared confirmation dialog and snackbar feedback.
- Frontend: Extends role/permission DTOs and inheritance status logic to support new “via ER” messaging and revocability metadata.
- Backend: Adds
DELETE /accessmanagement/api/v1/role/rolesto remove A2 role assignments and wires it through service/client layers, plus feature flag configuration and tests/mocks.
Reviewed changes
Copilot reviewed 35 out of 36 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/rtk/features/roleApi.ts | Adds removeRole RTK mutation; extends role/permission DTOs (revocability + reason). |
| src/resources/utils/featureFlagUtils.tsx | Adds enableRoleDeletion() feature flag helper. |
| src/localizations/no_nn.json | Adds new strings for delete confirmation/success + via_er messaging. |
| src/localizations/no_nb.json | Adds new strings for delete confirmation/success + via_er messaging. |
| src/localizations/en.json | Adds new strings for delete confirmation/success + via_er messaging. |
| src/global.d.ts | Extends window.featureFlags typing with enableRoleDeletion. |
| src/features/amUI/common/useInheritedStatus.ts | Updates inheritance resolution to use role-permission “reason” and adds ViaER type. |
| src/features/amUI/common/RoleList/roleSection.module.css | Aligns spacing with design system variables. |
| src/features/amUI/common/RoleList/RoleListItem.tsx | Adds optional controls slot for a delete button. |
| src/features/amUI/common/RoleList/RoleList.tsx | Renders delete button in role list when flag enabled + role is revocable. |
| src/features/amUI/common/RoleList/RoleDeleteButton.tsx | New reusable delete button component that calls removeRole and shows snackbar success. |
| src/features/amUI/common/DeleteUserModal/deletionModalUtils.ts | Adjusts “non-deletable reasons” when role deletion flag is enabled. |
| src/features/amUI/common/DeletePoaConfirmation/DeletePoaConfirmation.tsx | New confirmation dialog component for POA deletion actions. |
| src/features/amUI/common/DeletePoaConfirmation/DeletePoaConfirmation.module.css | Styling for the new delete confirmation dialog. |
| src/features/amUI/common/DelegationModal/Role/RoleStatusMessages.tsx | Adds translation key mapping for ViaER role status. |
| src/features/amUI/common/DelegationModal/Role/RoleInfo.tsx | Adds delete button + error surface in role info modal; fetches role permissions for revocability. |
| src/features/amUI/common/DelegationModal/Role/RoleInfo.module.css | Spacing updates and container for delete button section. |
| src/features/amUI/common/DelegationModal/EditModal.tsx | Wraps modal content with SnackbarProvider to support snackbars in modal flows. |
| backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/Controllers/RoleController.cs | Adds DELETE endpoint to remove legacy role assignments. |
| backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/appsettings.YT01.json | Adds EnableRoleDeletion feature flag config. |
| backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/appsettings.TT02.json | Adds EnableRoleDeletion feature flag config. |
| backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/appsettings.Prod.json | Adds EnableRoleDeletion feature flag config. |
| backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/appsettings.json | Adds EnableRoleDeletion feature flag config; formatting tweaks. |
| backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/appsettings.Development.json | Enables EnableRoleDeletion for development. |
| backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/appsettings.AT24.json | Adds EnableRoleDeletion feature flag config. |
| backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/appsettings.AT23.json | Adds EnableRoleDeletion feature flag config. |
| backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/appsettings.AT22.json | Enables EnableRoleDeletion for AT22. |
| backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Tests/Controllers/RoleControllerTest.cs | Adds integration tests for new delete endpoint (success/bad request/downstream error). |
| backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Mocks/Mocks/RoleClientMock.cs | Adds mock implementation for role removal. |
| backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Integration/Clients/RoleClient.cs | Implements downstream DELETE call to remove role assignment. |
| backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Integration/Altinn.AccessManagement.UI.Integration.csproj | Adds System.Security.Cryptography.Xml package reference. |
| backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Core/Services/RoleService.cs | Adds service method for role removal. |
| backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Core/Services/Interfaces/IRoleService.cs | Adds RemoveRole to the role service interface. |
| backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Core/Models/Common/Role.cs | Adds IsRevocable property to role model. |
| backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Core/Configuration/FeatureFlags.cs | Adds EnableRoleDeletion feature flag definition. |
| backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Core/ClientInterfaces/IRoleClient.cs | Adds RemoveRole to role client interface. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
![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: 4
🤖 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
`@backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/Controllers/RoleController.cs`:
- Around line 141-169: The DeleteRole endpoint currently ignores the
EnableRoleDeletion feature flag; update the RoleController to inject the
FeatureFlags configuration (add a FeatureFlags parameter stored as a private
readonly field, e.g., _featureFlags) and, inside DeleteRole (before calling
_roleService.RemoveRole), check _featureFlags.EnableRoleDeletion and return
Forbid() or NotFound() (match existing app behavior for disabled features) when
the flag is false; keep existing validation and exception handling intact.
In `@src/features/amUI/common/DelegationModal/Role/RoleInfo.tsx`:
- Around line 38-47: The component derives hasRole from permissions that can be
undefined during loading, causing the "cannot assign role" fallback to render
prematurely; update the logic around useGetRolePermissionsQuery to wait for
permissions resolution (use the hook's isLoading/isFetching/isError flags or
explicitly check permissions !== undefined) and only compute hasRole (and render
the fallback at lines ~141-145) after permissions are settled; modify the
hasRole computation inside RoleInfo (and any places using permissions) to guard
with permissions !== undefined (or !isLoading && !isFetching && !isError) so the
UI shows a loading state instead of a false-negative fallback until the query
completes.
In `@src/features/amUI/common/RoleList/RoleDeleteButton.tsx`:
- Around line 46-48: The catch handler in RoleDeleteButton (.catch((error:
FetchBaseQueryError | SerializedError) => { onError?.(error); })) currently
silences failures when onError is not provided; update this catch block to call
onError if present and otherwise invoke the app's default error notification
(e.g., enqueueSnackbar or the project's toast helper) with a user-facing message
and the error details so users always get feedback on failed deletions. Ensure
you reference the same catch location in the RoleDeleteButton component and
include the error object in the fallback notification for debugging.
In `@src/localizations/no_nn.json`:
- Line 380: The localization string for the key "deleting_role_loading" uses
Bokmål ("slettes"); update its value to the Nynorsk verb form by replacing
"Venter på at rolla skal slettes..." with "Venter på at rolla skal slettast..."
in the no_nn.json localization file so the locale remains consistent.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9dc0c2df-de70-48b5-b982-a67be4ec09f5
📒 Files selected for processing (36)
backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Core/ClientInterfaces/IRoleClient.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Core/Configuration/FeatureFlags.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Core/Models/Common/Role.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Core/Services/Interfaces/IRoleService.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Core/Services/RoleService.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Integration/Altinn.AccessManagement.UI.Integration.csprojbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Integration/Clients/RoleClient.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Mocks/Mocks/RoleClientMock.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Tests/Controllers/RoleControllerTest.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/Controllers/RoleController.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/appsettings.AT22.jsonbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/appsettings.AT23.jsonbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/appsettings.AT24.jsonbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/appsettings.Development.jsonbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/appsettings.Prod.jsonbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/appsettings.TT02.jsonbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/appsettings.YT01.jsonbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/appsettings.jsonsrc/features/amUI/common/DelegationModal/EditModal.tsxsrc/features/amUI/common/DelegationModal/Role/RoleInfo.module.csssrc/features/amUI/common/DelegationModal/Role/RoleInfo.tsxsrc/features/amUI/common/DelegationModal/Role/RoleStatusMessages.tsxsrc/features/amUI/common/DeletePoaConfirmation/DeletePoaConfirmation.module.csssrc/features/amUI/common/DeletePoaConfirmation/DeletePoaConfirmation.tsxsrc/features/amUI/common/DeleteUserModal/deletionModalUtils.tssrc/features/amUI/common/RoleList/RoleDeleteButton.tsxsrc/features/amUI/common/RoleList/RoleList.tsxsrc/features/amUI/common/RoleList/RoleListItem.tsxsrc/features/amUI/common/RoleList/roleSection.module.csssrc/features/amUI/common/useInheritedStatus.tssrc/global.d.tssrc/localizations/en.jsonsrc/localizations/no_nb.jsonsrc/localizations/no_nn.jsonsrc/resources/utils/featureFlagUtils.tsxsrc/rtk/features/roleApi.ts
added 2 commits
May 20, 2026 15:35
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/features/amUI/common/DelegationModal/Role/RoleInfo.tsx (1)
58-73:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlways show a generic delete error alert even when technical details are unavailable.
If
createErrorDetailsreturnsnull, the alert content is empty, so users get no error message despite a failed delete action.Suggested fix
const deleteErrorAlert = () => { if (deleteError || actionError) { const error = deleteError || actionError; const details = createErrorDetails(error as Parameters<typeof createErrorDetails>[0]); return ( - <> - {!!details && ( - <DsAlert data-color='danger'> - <DsParagraph>{t('role.delete_role_error')}</DsParagraph> - <TechnicalErrorParagraphs - status={details.status} - time={details.time} - traceId={details.traceId} - /> - </DsAlert> - )} - </> + <DsAlert data-color='danger'> + <DsParagraph>{t('role.delete_role_error')}</DsParagraph> + {!!details && ( + <TechnicalErrorParagraphs + status={details.status} + time={details.time} + traceId={details.traceId} + /> + )} + </DsAlert> ); } };🤖 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 `@src/features/amUI/common/DelegationModal/Role/RoleInfo.tsx` around lines 58 - 73, The deleteErrorAlert function currently returns no visible alert when createErrorDetails(...) yields null; update deleteErrorAlert so that when deleteError or actionError is truthy it always renders a DsAlert containing the generic DsParagraph with t('role.delete_role_error'), and only additionally renders TechnicalErrorParagraphs with details.status/time/traceId if details is non-null (where details comes from createErrorDetails(error)). Keep references to deleteErrorAlert, createErrorDetails, DsAlert, DsParagraph, and TechnicalErrorParagraphs when locating and changing the JSX.
♻️ Duplicate comments (1)
src/features/amUI/common/DelegationModal/Role/RoleInfo.tsx (1)
41-56:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGate
hasRolefallback on resolved permissions to avoid false negatives during loading.This still risks showing
role.cannot_assign_role_poabefore permissions are loaded, becausepermissionsstarts asundefinedandhasRolebecomesfalse.Also applies to: 143-146
🤖 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 `@src/features/amUI/common/DelegationModal/Role/RoleInfo.tsx` around lines 41 - 56, The current hasRole logic can return false while permissions are still loading (permissions === undefined), causing UI like role.cannot_assign_role_poa to show prematurely; update the computed flags so they only resolve after permissions are loaded — e.g., compute rolePermissions from useGetRolePermissionsQuery as now, but change hasRole to require permissions !== undefined && rolePermissions !== undefined (or use an explicit isPermissionsLoaded check from useGetRolePermissionsQuery) and similarly guard roleIsRevocable behind permissions !== undefined; apply the same pattern to the other occurrence referenced (lines around the role checks at 143-146) so no negative/falsey fallback is used while permissions are unresolved.
🤖 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 `@src/features/amUI/userRightsPage/RoleSection/RoleSection.tsx`:
- Around line 20-24: The modal open/close flow leaks a stale delete error
because deleteError is only set on error; update the handlers that open and
close the modal (the onSelect callback that calls setModalItem and the modal
close handler referenced around lines 33-35) to clear the error by calling
setDeleteError(null or undefined) whenever the modal is opened or closed so any
previous error banner is reset; ensure you call setDeleteError(...) both when
setting a new modal item in onSelect and in the modal's onClose/close handler.
---
Outside diff comments:
In `@src/features/amUI/common/DelegationModal/Role/RoleInfo.tsx`:
- Around line 58-73: The deleteErrorAlert function currently returns no visible
alert when createErrorDetails(...) yields null; update deleteErrorAlert so that
when deleteError or actionError is truthy it always renders a DsAlert containing
the generic DsParagraph with t('role.delete_role_error'), and only additionally
renders TechnicalErrorParagraphs with details.status/time/traceId if details is
non-null (where details comes from createErrorDetails(error)). Keep references
to deleteErrorAlert, createErrorDetails, DsAlert, DsParagraph, and
TechnicalErrorParagraphs when locating and changing the JSX.
---
Duplicate comments:
In `@src/features/amUI/common/DelegationModal/Role/RoleInfo.tsx`:
- Around line 41-56: The current hasRole logic can return false while
permissions are still loading (permissions === undefined), causing UI like
role.cannot_assign_role_poa to show prematurely; update the computed flags so
they only resolve after permissions are loaded — e.g., compute rolePermissions
from useGetRolePermissionsQuery as now, but change hasRole to require
permissions !== undefined && rolePermissions !== undefined (or use an explicit
isPermissionsLoaded check from useGetRolePermissionsQuery) and similarly guard
roleIsRevocable behind permissions !== undefined; apply the same pattern to the
other occurrence referenced (lines around the role checks at 143-146) so no
negative/falsey fallback is used while permissions are unresolved.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 00482e2f-7892-47e6-be47-ddb586f158d0
📒 Files selected for processing (8)
src/features/amUI/common/DelegationModal/Role/RoleInfo.tsxsrc/features/amUI/common/RoleList/RoleList.tsxsrc/features/amUI/common/TechnicalErrorParagraphs/TechnicalErrorParagraphs.tsxsrc/features/amUI/common/useInheritedStatus.tssrc/features/amUI/userRightsPage/RoleSection/RoleSection.tsxsrc/localizations/en.jsonsrc/localizations/no_nb.jsonsrc/localizations/no_nn.json
✅ Files skipped from review due to trivial changes (1)
- src/localizations/en.json
added 2 commits
May 21, 2026 14:37
|
sonwit
approved these changes
May 22, 2026
sonwit
left a comment
sonwit
left a comment
Contributor
There was a problem hiding this comment.
Testet og det ser ut som det funker veldig bra.
Koden ser bra ut ✨
✅
5 tasks
5 tasks
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
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.
Description
This PR adds functionality that allows the user to delete A2 roles from the A3 UI.
The feature is only active when the new feature flag
enableRoleDeletionis true, and will then:Since A2 roles are not delegable in A3 (and this feature will not be enabled before A2 is turned off for good), we add a warning to this delete action.
Also: Note that new backend features have been added to allow for
reasonto be populated (for knowing inheritance) and a newisRevocableflag that determines if the assignment can be deleted.State after deleting role
Deletion from list
Related Issue(s)
Verification
Documentation
Summary by CodeRabbit
New Features
Chores
Tests