Skip to content

Conversation

@SandGrainOne
Copy link
Member

@SandGrainOne SandGrainOne commented Nov 27, 2025

Description

Not waiting for renovate to include these next week.

Summary by CodeRabbit

  • Chores
    • Bumped access-token client packages to newer patch/minor versions for compatibility and security.
  • Tests
    • Expanded and reorganized integration tests to cover both bridge-enabled and -disabled paths.
    • Added test helpers, mocks and centralized factory-based client setup for deterministic tests.
    • Added in-memory configuration support and optional mock injection in the test factory.
  • Telemetry
    • Split and restructured telemetry tests into focused suites for clearer verification.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 27, 2025

📝 Walkthrough

Walkthrough

Updated two NuGet package versions and reworked integration tests: refactored UnitContactPointController tests with new classes, helpers and a mock service; added in-memory configuration and UnitContactPointsService mock to the test web factory; reorganized telemetry tests into fixture-backed inner classes.

Changes

Cohort / File(s) Summary
Package dependency updates
src/Altinn.Profile.Integrations/Altinn.Profile.Integrations.csproj, src/Altinn.Profile/Altinn.Profile.csproj
Updated package versions: Altinn.Common.AccessTokenClient 3.0.153.1.0; Altinn.Common.AccessToken 5.0.55.1.0.
UnitContactPoint controller tests & helpers
test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/UnitContactPointControllerTests.cs
Replaced original test class with new multi-class structure (UseSblBridge, DoNotUseSblBridge, MockedUnitContactPointsService), added deterministic helper methods for SBL/register/repository responses, centralized client creation via _factory.CreateClient(), and added SBL-bridge toggle coverage.
Test web application factory
test/Altinn.Profile.Tests/IntegrationTests/ProfileWebApplicationFactory.cs
Added UnitContactPointsServiceMock property and conditional DI registration, introduced InMemoryConfigurationCollection wired into host config via AddInMemoryCollection, and increased ShutdownTimeout from 5s to 10s.
Telemetry tests refactor
test/Altinn.Profile.Tests/IntegrationTests/TelemetryTests.cs
Converted telemetry tests into inner classes (RegistryMetrics, NotificationAddressMetrics) each implementing IClassFixture<ProfileWebApplicationFactory<Program>>, moved tests into those classes, and switched to factory-based client creation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify conditional DI registration of UnitContactPointsServiceMock and its lifetime/override behavior in ProfileWebApplicationFactory.cs.
  • Review new test helpers in UnitContactPointControllerTests.cs for deterministic behavior and coverage of SBL/no-registry/no-repo edge cases.
  • Confirm telemetry tests retained correct meter/provider scoping and that factory-based client usage matches prior test semantics.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title contains a typo ('Bumb' instead of 'Bump') and is partially related to the changeset, which includes not only access token library updates but also significant test restructuring and refactoring. Correct the typo to 'Bump accesstoken libraries' and consider whether the title should be more comprehensive given the substantial test restructuring changes included in this PR.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/patch-accesstoken-libraries-after-update

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/UnitContactPointControllerTests.cs (1)

256-281: Potential test isolation issue: mock setup override persists across tests.

This test overrides the RegisterClientMock.Setup established in the constructor to return null. Since all tests in DoNotUseSblBridge share the same factory instance, this override persists for any tests that run afterward, potentially causing flaky test behavior depending on execution order.

Consider resetting the mock after the test or using a dedicated test class for this scenario:

             // Assert
             Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode);
+
+            // Reset the mock to default behavior for other tests
+            _factory.RegisterClientMock.Setup(s => s.GetPartyUuids(It.IsAny<string[]>(), It.IsAny<CancellationToken>()))
+                .ReturnsAsync((string[] orgNumbers, CancellationToken _) => GetRegisterResponse(orgNumbers));
         }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4d6e3aa and a20b40b.

📒 Files selected for processing (3)
  • test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/UnitContactPointControllerTests.cs (14 hunks)
  • test/Altinn.Profile.Tests/IntegrationTests/ProfileWebApplicationFactory.cs (6 hunks)
  • test/Altinn.Profile.Tests/IntegrationTests/TelemetryTests.cs (1 hunks)
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 385
File: src/Altinn.Profile.Integrations/Persistence/ProfiledbContext.cs:124-124
Timestamp: 2025-05-26T07:52:15.096Z
Learning: The "Feature/change from party id to party UUID" (PR #385) in the Altinn Profile repository is specifically scoped to the PartyGroupAssociation entity only. Other PartyId references in the codebase (such as in UnitContactPoints, telemetry, SblBridge components, and tests) are separate from this feature and should not be updated as part of this change.
📚 Learning: 2025-09-17T10:51:24.562Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 531
File: test/Altinn.Profile.Tests/Profile.Integrations/ProfessionalNotifications/ProfessionalNotificationsRepositoryTests.cs:66-87
Timestamp: 2025-09-17T10:51:24.562Z
Learning: In the Altinn.Profile repository testing, when mocking IDbContextOutbox for professional notification events, the SaveChangesAsync call should use CancellationToken.None instead of forwarding the original cancellation token, ensuring events are processed even if the original request is cancelled (consistent with the production behavior).

Applied to files:

  • test/Altinn.Profile.Tests/IntegrationTests/ProfileWebApplicationFactory.cs
  • test/Altinn.Profile.Tests/IntegrationTests/TelemetryTests.cs
  • test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/UnitContactPointControllerTests.cs
📚 Learning: 2025-05-26T07:52:15.096Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 385
File: src/Altinn.Profile.Integrations/Persistence/ProfiledbContext.cs:124-124
Timestamp: 2025-05-26T07:52:15.096Z
Learning: The "Feature/change from party id to party UUID" (PR #385) in the Altinn Profile repository is specifically scoped to the PartyGroupAssociation entity only. Other PartyId references in the codebase (such as in UnitContactPoints, telemetry, SblBridge components, and tests) are separate from this feature and should not be updated as part of this change.

Applied to files:

  • test/Altinn.Profile.Tests/IntegrationTests/ProfileWebApplicationFactory.cs
  • test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/UnitContactPointControllerTests.cs
📚 Learning: 2025-11-05T10:51:23.134Z
Learnt from: suhasini3091
Repo: Altinn/altinn-profile PR: 617
File: src/Altinn.Profile/Program.cs:196-196
Timestamp: 2025-11-05T10:51:23.134Z
Learning: In the Altinn.Profile repository, the `ScopeAccessHandler` authorization handler is provided by the external NuGet package `Altinn.Common.PEP` (version 4.2.1) and is used to validate scope-based authorization requirements. This handler is registered in Program.cs and works with `ScopeAccessRequirement` to validate JWT token claims containing the required scope.

Applied to files:

  • test/Altinn.Profile.Tests/IntegrationTests/ProfileWebApplicationFactory.cs
📚 Learning: 2025-07-01T12:23:00.322Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 440
File: src/Altinn.Profile.Integrations/Notifications/NotificationsClient.cs:69-90
Timestamp: 2025-07-01T12:23:00.322Z
Learning: In the Altinn.Profile repository NotificationsClient, input validation for parameters like emailAddress and languageCode is intentionally omitted because validation is handled at the service layer, not in the integration layer.

Applied to files:

  • test/Altinn.Profile.Tests/IntegrationTests/ProfileWebApplicationFactory.cs
📚 Learning: 2025-04-30T12:42:11.245Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 356
File: src/Altinn.Profile.Integrations/Repositories/OrganizationNotificationAddressRepository.cs:189-203
Timestamp: 2025-04-30T12:42:11.245Z
Learning: In the Altinn.Profile repository, existence checks for notification addresses are performed in the service layer (OrganizationNotificationAddressesService) before calling repository methods, making additional existence checks in the repository layer redundant.

Applied to files:

  • test/Altinn.Profile.Tests/IntegrationTests/ProfileWebApplicationFactory.cs
📚 Learning: 2025-04-22T12:57:56.096Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 351
File: src/Altinn.Profile/Mappers/OrganizationRequestMapper.cs:7-39
Timestamp: 2025-04-22T12:57:56.096Z
Learning: In the Altinn.Profile repository, email validation for NotificationAddressModel is handled at the request model level using data annotations, making additional validation in mapper classes redundant.

Applied to files:

  • test/Altinn.Profile.Tests/IntegrationTests/ProfileWebApplicationFactory.cs
📚 Learning: 2025-08-26T09:04:52.003Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 509
File: src/Altinn.Profile.Core/Telemetry/Telemetry.ImportJob.cs:20-24
Timestamp: 2025-08-26T09:04:52.003Z
Learning: In Altinn.Profile.Core telemetry files, using static directives can import ActivityPrefix constants from other nested telemetry classes, making them available unqualified in the current file. Check for using static imports before assuming unqualified references are compilation errors.

Applied to files:

  • test/Altinn.Profile.Tests/IntegrationTests/ProfileWebApplicationFactory.cs
  • test/Altinn.Profile.Tests/IntegrationTests/TelemetryTests.cs
📚 Learning: 2025-07-01T12:23:00.322Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 440
File: src/Altinn.Profile.Integrations/Notifications/NotificationsClient.cs:69-90
Timestamp: 2025-07-01T12:23:00.322Z
Learning: In the Altinn.Profile repository NotificationsClient, GetTmpEmailBody is used intentionally instead of GetEmailBody because reportee name information is not available at the integration layer level.

Applied to files:

  • test/Altinn.Profile.Tests/IntegrationTests/ProfileWebApplicationFactory.cs
📚 Learning: 2025-10-22T06:26:08.775Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 596
File: src/Altinn.Profile.Core/Unit.ContactPoints/UnitContactPointService.cs:78-93
Timestamp: 2025-10-22T06:26:08.775Z
Learning: In the Altinn.Profile repository, the `resourceId` parameter in `UnitContactPointService.IsValidEndpoint` is validated by the `[Required]` attribute on the `UnitContactPointLookup.ResourceId` property, ensuring it cannot be null or empty when called through the controller. No additional null/empty guards are needed in this method.

Applied to files:

  • test/Altinn.Profile.Tests/IntegrationTests/ProfileWebApplicationFactory.cs
  • test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/UnitContactPointControllerTests.cs
📚 Learning: 2025-04-22T12:57:56.096Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 351
File: src/Altinn.Profile/Mappers/OrganizationRequestMapper.cs:7-39
Timestamp: 2025-04-22T12:57:56.096Z
Learning: In the Altinn.Profile repository, email validation for NotificationAddressModel is handled at the request model level using the CustomRegexForNotificationAddresses attribute, making additional validation in mapper classes redundant.

Applied to files:

  • test/Altinn.Profile.Tests/IntegrationTests/ProfileWebApplicationFactory.cs
📚 Learning: 2025-08-26T09:04:52.003Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 509
File: src/Altinn.Profile.Core/Telemetry/Telemetry.ImportJob.cs:20-24
Timestamp: 2025-08-26T09:04:52.003Z
Learning: In Altinn.Profile.Core telemetry files, using static directives can import ActivityPrefix constants from other nested telemetry classes, making them available unqualified. Always check which ActivityPrefix value is actually being resolved when multiple telemetry classes define their own ActivityPrefix constants.

Applied to files:

  • test/Altinn.Profile.Tests/IntegrationTests/ProfileWebApplicationFactory.cs
  • test/Altinn.Profile.Tests/IntegrationTests/TelemetryTests.cs
📚 Learning: 2025-04-22T12:57:56.096Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 351
File: src/Altinn.Profile/Mappers/OrganizationRequestMapper.cs:7-39
Timestamp: 2025-04-22T12:57:56.096Z
Learning: In the Altinn.Profile repository, input validation for NotificationAddressModel (including email format) is handled using CustomRegexForNotificationAddressesAttribute at the model level, and the controller checks ModelState.IsValid before processing. This makes additional validation in mapper classes redundant.

Applied to files:

  • test/Altinn.Profile.Tests/IntegrationTests/ProfileWebApplicationFactory.cs
🧬 Code graph analysis (1)
test/Altinn.Profile.Tests/IntegrationTests/TelemetryTests.cs (1)
src/Altinn.Profile.Integrations/Repositories/OrganizationNotificationAddressRepository.cs (13)
  • Task (20-37)
  • Task (45-59)
  • Task (67-82)
  • Task (90-110)
  • Task (118-125)
  • Task (127-144)
  • Task (147-152)
  • Task (155-170)
  • Task (173-188)
  • Task (191-213)
  • Task (216-233)
  • Task (236-249)
  • Task (252-266)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Analyze (csharp, autobuild)
  • GitHub Check: Build, test & analyze
🔇 Additional comments (4)
test/Altinn.Profile.Tests/IntegrationTests/ProfileWebApplicationFactory.cs (2)

51-52: Clean extension of the test factory for flexible test configuration.

The nullable UnitContactPointsServiceMock and InMemoryConfigurationCollection properties follow the existing patterns in the factory and provide flexible ways to configure tests without modifying the base factory behavior. The nullable mock ensures most tests aren't affected while allowing specific tests to inject custom behavior.

Also applies to: 82-83


131-136: Conditional service registration follows existing pattern.

The null check before registering UnitContactPointsServiceMock mirrors the pattern used for PdpMock (lines 120-127), maintaining consistency in how optional mocks are handled.

test/Altinn.Profile.Tests/IntegrationTests/TelemetryTests.cs (1)

15-18: Good use of nested test classes for logical grouping.

The restructuring using nested test classes with primary constructors is a clean approach. Each inner class gets its own IClassFixture<ProfileWebApplicationFactory<Program>> instance, ensuring proper test isolation between RegistryMetrics and NotificationAddressMetrics test groups.

Also applies to: 56-59

test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/UnitContactPointControllerTests.cs (1)

22-47: Well-structured nested test classes with clear configuration intent.

The separation into UseSblBridge, DoNotUseSblBridge, and MockedUnitContactPointsService clearly communicates the test scenarios. Each class configures its own InMemoryConfigurationCollection and mock setups in the constructor, providing good test organization.

Also applies to: 101-122, 321-336

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/UnitContactPointControllerTests.cs (2)

336-336: Consider explicit mock initialization for test isolation.

The null-coalescing assignment ??= means the mock is only created if it's null. If UnitContactPointsServiceMock already exists from a previous test, it won't be reset, potentially carrying over setups or invocations. For better test isolation, consider explicitly creating a new mock or resetting the existing one.

Apply this diff to ensure a clean mock state:

-            _factory.UnitContactPointsServiceMock ??= new Mock<IUnitContactPointsService>();
+            _factory.UnitContactPointsServiceMock = new Mock<IUnitContactPointsService>();

Alternatively, if the mock should be reused, explicitly reset it:

-            _factory.UnitContactPointsServiceMock ??= new Mock<IUnitContactPointsService>();
+            _factory.UnitContactPointsServiceMock ??= new Mock<IUnitContactPointsService>();
+            _factory.UnitContactPointsServiceMock.Reset();

393-393: Use deterministic GUID for test consistency.

Generating a new GUID with Guid.NewGuid() on each call makes test behavior non-deterministic. While the tests may not currently assert on this value, using a hardcoded GUID (like in the GetPartyUuidForOrgNo method) would make tests more predictable and easier to debug.

Apply this diff to use a deterministic GUID:

-                        PartyId = Guid.NewGuid()
+                        PartyId = Guid.Parse("8baab949-07f9-4ac5-b8cb-af6208b59092")
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a20b40b and fc467b7.

📒 Files selected for processing (1)
  • test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/UnitContactPointControllerTests.cs (14 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 385
File: src/Altinn.Profile.Integrations/Persistence/ProfiledbContext.cs:124-124
Timestamp: 2025-05-26T07:52:15.096Z
Learning: The "Feature/change from party id to party UUID" (PR #385) in the Altinn Profile repository is specifically scoped to the PartyGroupAssociation entity only. Other PartyId references in the codebase (such as in UnitContactPoints, telemetry, SblBridge components, and tests) are separate from this feature and should not be updated as part of this change.
📚 Learning: 2025-05-26T07:52:15.096Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 385
File: src/Altinn.Profile.Integrations/Persistence/ProfiledbContext.cs:124-124
Timestamp: 2025-05-26T07:52:15.096Z
Learning: The "Feature/change from party id to party UUID" (PR #385) in the Altinn Profile repository is specifically scoped to the PartyGroupAssociation entity only. Other PartyId references in the codebase (such as in UnitContactPoints, telemetry, SblBridge components, and tests) are separate from this feature and should not be updated as part of this change.

Applied to files:

  • test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/UnitContactPointControllerTests.cs
📚 Learning: 2025-09-17T10:51:24.562Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 531
File: test/Altinn.Profile.Tests/Profile.Integrations/ProfessionalNotifications/ProfessionalNotificationsRepositoryTests.cs:66-87
Timestamp: 2025-09-17T10:51:24.562Z
Learning: In the Altinn.Profile repository testing, when mocking IDbContextOutbox for professional notification events, the SaveChangesAsync call should use CancellationToken.None instead of forwarding the original cancellation token, ensuring events are processed even if the original request is cancelled (consistent with the production behavior).

Applied to files:

  • test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/UnitContactPointControllerTests.cs
📚 Learning: 2025-10-22T06:26:08.775Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 596
File: src/Altinn.Profile.Core/Unit.ContactPoints/UnitContactPointService.cs:78-93
Timestamp: 2025-10-22T06:26:08.775Z
Learning: In the Altinn.Profile repository, the `resourceId` parameter in `UnitContactPointService.IsValidEndpoint` is validated by the `[Required]` attribute on the `UnitContactPointLookup.ResourceId` property, ensuring it cannot be null or empty when called through the controller. No additional null/empty guards are needed in this method.

Applied to files:

  • test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/UnitContactPointControllerTests.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Build, test & analyze
  • GitHub Check: Analyze (csharp, autobuild)
🔇 Additional comments (4)
test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/UnitContactPointControllerTests.cs (4)

22-99: Well-structured test class for SBL bridge enabled scenario.

The UseSblBridge class is properly organized with fixture-based setup and covers both success and error paths for the SBL bridge feature flag enabled case.


125-282: Comprehensive test coverage for non-SBL bridge scenario.

The test methods provide excellent coverage including input validation, success cases, resource ID filtering, and error handling scenarios. The approach of returning OK with empty lists for errors (when SBL bridge is disabled) versus returning InternalServerError (when enabled) is well tested.


376-436: Helper methods provide good test data abstraction.

The static helper methods (GetSBlResponseFromSBL, GetRegisterResponse, GetPartyUuidForOrgNo) appropriately abstract test data generation and can be reused across the nested test classes. The organization is clear and maintainable.


117-117: I'll verify the review comment about thread safety of Mock.Reset() in the test file. Let me investigate how the factory is shared, xUnit's parallel execution configuration, and the scope of the mock reset.
<function_calls>

#!/bin/bash

Search for ProfileWebApplicationFactory usage and definition

rg -n "ProfileWebApplicationFactory" --type cs -A 2 -B 2 | head -100

echo "---"

Check for xUnit parallel execution configuration

fd -e json "xunit.runner.json" --exec cat {}

echo "---"

Check for parallel execution attributes/disabling

rg -n "DisableParallelization|CollectionDefinition|Collection(" --type cs -A 1 | head -80


</function_calls>

@sonarqubecloud
Copy link

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/UnitContactPointControllerTests.cs (1)

284-319: Consider extracting hard-coded test data as constants.

The UUIDs, party IDs, and organization numbers are duplicated across GetRepositoryResponse (lines 284-319) and GetPartyUuidForOrgNo (lines 419-437). Extracting these as shared constants would improve maintainability.

Example:

+    private static class TestData
+    {
+        public const string OrgNumber1 = "123456789";
+        public const string OrgNumber2 = "111111111";
+        public const int PartyId1 = 50001;
+        public const int PartyId2 = 50002;
+        public static readonly Guid PartyUuid1 = Guid.Parse("8baab949-07f9-4ac5-b8cb-af6208b59092");
+        public static readonly Guid PartyUuid2 = Guid.Parse("f81d7b22-4acb-4d59-b544-ef028f183ebc");
+    }

Then reference these constants in both helper methods.

Also applies to: 419-437

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fc467b7 and 9a8e72c.

📒 Files selected for processing (1)
  • test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/UnitContactPointControllerTests.cs (14 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 385
File: src/Altinn.Profile.Integrations/Persistence/ProfiledbContext.cs:124-124
Timestamp: 2025-05-26T07:52:15.096Z
Learning: The "Feature/change from party id to party UUID" (PR #385) in the Altinn Profile repository is specifically scoped to the PartyGroupAssociation entity only. Other PartyId references in the codebase (such as in UnitContactPoints, telemetry, SblBridge components, and tests) are separate from this feature and should not be updated as part of this change.
📚 Learning: 2025-05-26T07:52:15.096Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 385
File: src/Altinn.Profile.Integrations/Persistence/ProfiledbContext.cs:124-124
Timestamp: 2025-05-26T07:52:15.096Z
Learning: The "Feature/change from party id to party UUID" (PR #385) in the Altinn Profile repository is specifically scoped to the PartyGroupAssociation entity only. Other PartyId references in the codebase (such as in UnitContactPoints, telemetry, SblBridge components, and tests) are separate from this feature and should not be updated as part of this change.

Applied to files:

  • test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/UnitContactPointControllerTests.cs
📚 Learning: 2025-09-17T10:51:24.562Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 531
File: test/Altinn.Profile.Tests/Profile.Integrations/ProfessionalNotifications/ProfessionalNotificationsRepositoryTests.cs:66-87
Timestamp: 2025-09-17T10:51:24.562Z
Learning: In the Altinn.Profile repository testing, when mocking IDbContextOutbox for professional notification events, the SaveChangesAsync call should use CancellationToken.None instead of forwarding the original cancellation token, ensuring events are processed even if the original request is cancelled (consistent with the production behavior).

Applied to files:

  • test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/UnitContactPointControllerTests.cs
📚 Learning: 2025-10-22T06:26:08.775Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 596
File: src/Altinn.Profile.Core/Unit.ContactPoints/UnitContactPointService.cs:78-93
Timestamp: 2025-10-22T06:26:08.775Z
Learning: In the Altinn.Profile repository, the `resourceId` parameter in `UnitContactPointService.IsValidEndpoint` is validated by the `[Required]` attribute on the `UnitContactPointLookup.ResourceId` property, ensuring it cannot be null or empty when called through the controller. No additional null/empty guards are needed in this method.

Applied to files:

  • test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/UnitContactPointControllerTests.cs
🧬 Code graph analysis (1)
test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/UnitContactPointControllerTests.cs (1)
src/Altinn.Profile.Integrations/SblBridge/Unit.Profile/PartyNotificationContactPoints.cs (2)
  • UnitContactPointsList (33-48)
  • PartyNotificationContactPoints (8-49)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Build, test & analyze
  • GitHub Check: Analyze (csharp, autobuild)
🔇 Additional comments (4)
test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/UnitContactPointControllerTests.cs (4)

18-21: LGTM! Well-organized test structure.

The file-scoped namespace and nested test class organization provides clear separation of concerns for different test scenarios (SBL bridge enabled/disabled, mocked service).


22-99: LGTM! SBL bridge enabled tests are well-structured.

The test class properly configures the feature flag, sets up necessary mocks, and covers both success and error scenarios when the SBL bridge is enabled.


101-283: LGTM! Comprehensive test coverage with SBL bridge disabled.

The test class properly configures the feature flag to disable SBL bridge and includes comprehensive test coverage for validation, filtering, and error scenarios.


322-375: LGTM! Resource ID sanitization test is correctly implemented.

The test class properly mocks the service and verifies that the urn:altinn:resource: prefix is stripped from the resource ID before being passed to the service.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants