Replace partyId with partyUuid for systemuser #1405
Conversation
📝 WalkthroughWalkthroughThe changes update many API interfaces, services, clients, controllers, and UI components by replacing the integer-based party identifiers with GUIDs. Redundant parameters such as Changes
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code Graph Analysis (1)tests/setupTests.ts (1)
🪛 GitHub Check: CodeQLtests/setupTests.ts[warning] 42-42: Clear text transmission of sensitive cookie ⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
the
kind/feature-request
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Tests/Controllers/SystemUserControllerTest.cs (1)
264-269: 💡 Verification agent🧩 Analysis chain
Consider if partyUuid query parameter is still needed
Now that partyId is a Guid with the same value as partyUuid, it seems redundant to pass the partyUuid as a query parameter. Is this parameter still required by the controller? If not, consider removing it to simplify the API calls.
Also applies to: 284-289
🏁 Script executed:
#!/bin/bash # Check if the controller still requires partyUuid as a query parameter rg -A 5 "DeleteAgentSystemUser" backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/Controllers/Length of output: 1340
Remove Redundant
partyUuidQuery ParameterThe controller’s
DeleteAgentSystemUsermethod is defined to accept onlypartyIdandsystemUserGuidas route parameters—there’s no indication that apartyUuidquery parameter is needed. SincepartyUuidduplicates the value already inpartyId, please remove this parameter from the test calls (e.g., lines 264-269 and 284-289 in SystemUserControllerTest.cs) to simplify the API usage.
🧹 Nitpick comments (3)
backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Tests/Controllers/SystemUserRequestControllerTest.cs (1)
123-123: Method name appears inconsistent with its purposeThe test method name
RejectSystemUserRequest_ApproveOkshould probably beRejectSystemUserRequest_RejectOksince it's testing the rejection functionality, not approval.-public async Task RejectSystemUserRequest_ApproveOk() +public async Task RejectSystemUserRequest_RejectOk()backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Tests/Controllers/SystemUserAgentRequestControllerTest.cs (2)
84-84: Indentation needs adjustmentThe line has an extra space at the beginning compared to other similar declarations in the file.
- Guid partyId = Guid.Parse("cd35779b-b174-4ecc-bbef-ece13611be7f"); + Guid partyId = Guid.Parse("cd35779b-b174-4ecc-bbef-ece13611be7f");
123-137: Method naming inconsistencyThe method name
RejectSystemUserAgentRequest_RejectOkdoesn't follow the same pattern asApproveSystemUserAgentRequest_ApproveOk. Consider renaming for consistency.- public async Task RejectSystemUserAgentRequest_RejectOk() + public async Task RejectSystemUserAgentRequest_ApproveOk()
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Tests/Controllers/SystemUserAgentDelegationControllerTest.cs(14 hunks)backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Tests/Controllers/SystemUserAgentRequestControllerTest.cs(6 hunks)backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Tests/Controllers/SystemUserChangeRequestControllerTest.cs(6 hunks)backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Tests/Controllers/SystemUserControllerTest.cs(12 hunks)backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Tests/Controllers/SystemUserRequestControllerTest.cs(6 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Tests/Controllers/SystemUserAgentDelegationControllerTest.cs (3)
backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Core/Services/SystemUserAgentDelegationService.cs (1)
List(124-135)backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Core/Models/SystemUser/Frontend/CustomerPartyFE.cs (1)
CustomerPartyFE(6-22)backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Core/Models/SystemUser/Frontend/AgentDelegationFE.cs (1)
AgentDelegationFE(6-22)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Continous Integration / Analyze
- GitHub Check: Analyze (csharp)
🔇 Additional comments (9)
backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Tests/Controllers/SystemUserControllerTest.cs (1)
44-44: Consistent type conversion from int to Guid for partyIdAll instances of partyId have been properly converted from int to Guid, using the same consistent UUID value. This change aligns with the PR objective of replacing integer identifiers with UUIDs.
Also applies to: 65-65, 87-87, 106-106, 125-125, 144-144, 172-172, 200-200, 221-221, 243-243, 262-262, 282-282
backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Tests/Controllers/SystemUserAgentDelegationControllerTest.cs (1)
46-52: Improved API design by consolidating partyId and partyUuidThe changes successfully:
- Convert partyId from int to Guid
- Remove redundant partyUuid query parameters from URLs
This simplifies the API by using a single identifier (partyId as Guid) instead of duplicating the same value in different parameters. The endpoint URLs are now cleaner and more maintainable.
Also applies to: 68-74, 90-96, 112-118, 134-140, 156-162, 178-197, 212-231, 246-260, 274-281, 297-304
backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Tests/Controllers/SystemUserRequestControllerTest.cs (1)
41-41: Consistent type conversion from int to Guid for partyIdAll instances of partyId have been properly converted from int to Guid across all test methods. The same UUID value is used consistently throughout the file.
Also applies to: 63-63, 84-84, 105-105, 126-126, 147-147
backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Tests/Controllers/SystemUserChangeRequestControllerTest.cs (1)
41-41: Consistent type conversion from int to Guid for partyIdAll partyId variables have been properly converted from int to Guid, using the same UUID value consistently throughout all test methods.
Also applies to: 63-63, 84-84, 105-105, 126-126, 147-147
backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Tests/Controllers/SystemUserAgentRequestControllerTest.cs (5)
41-41: Type change from int to Guid looks goodThe change from
inttoGuidforpartyIdaligns with the PR objective to replace integer identifiers with UUIDs for party identification.
63-63: Type change consistently appliedThe type change is correctly applied in this error test case, ensuring consistent parameter types across all test scenarios.
105-105: Type change consistently appliedThe type change is correctly applied in this error test case.
126-126: Type change consistently appliedThe type change is correctly applied in the reject test case.
147-147: Type change consistently appliedThe type change is correctly applied in this error test case.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
… of sensitive cookie Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/features/amUI/systemUser/SystemUsersOverviewPage/SystemUserOverviewPage.test.tsx(3 hunks)tests/consts.ts(1 hunks)tests/setupTests.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/features/amUI/systemUser/SystemUsersOverviewPage/SystemUserOverviewPage.test.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/setupTests.ts (1)
tests/consts.ts (2)
TEST_PARTY_ID(2-2)TEST_PARTY_UUID(3-3)
🪛 GitHub Check: CodeQL
tests/setupTests.ts
[warning] 42-42: Clear text transmission of sensitive cookie
Sensitive cookie sent without enforcing SSL encryption.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Continous Integration / Analyze
🔇 Additional comments (2)
tests/consts.ts (1)
3-3: Addition of TEST_PARTY_UUID constant looks good.This new constant aligns with the PR objective to replace party IDs with UUIDs throughout the system. The UUID format is valid and will provide better uniqueness guarantees than the numeric ID.
tests/setupTests.ts (1)
7-7: Import of TEST_PARTY_UUID is appropriate.The addition of TEST_PARTY_UUID to the import statement correctly brings in the new constant needed for the test setup.
|
Description
Related Issue(s)
Verification
Documentation
Summary by CodeRabbit
Refactor
AltinnPartyUuidinstead ofAltinnPartyId.Tests
Chore
AltinnPartyIdandAltinnPartyUuidcookies for consistency.These improvements enhance type safety, streamline API interactions, and simplify system user management.