Created new endpoint GetContactInformationByEmailAddress #647
Conversation
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
…ted integrationtests
Contributor
📝 WalkthroughWalkthroughAdds email-based contact lookup end-to-end (repository → service → controller), adds RegisterClient organization-number lookup by party UUID, relaxes SSN requirement in service filtering, updates DTO nullability and organization-number fields, and adds/updates tests and Bru test scenarios. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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: 4
🧹 Nitpick comments (8)
src/Altinn.Profile.Core/Integrations/IProfessionalNotificationsRepository.cs (1)
35-41: New email-based repository method is well-shaped; consider tightening XML docsThe method name, parameters and return type align with the existing repository API. To make the contract clearer, you could document that the returned list is non-null and may be empty rather than null.
For example, update
<returns>to specify “A non-null list (possibly empty) of contact infos for the given email address.”src/Altinn.Profile.Core/Integrations/IRegisterClient.cs (1)
33-39: New GetOrganizationNumberByPartyUuid API aligns with existing register client patternThe method signature and XML documentation fit well with the other
IRegisterClientmethods and enable the required org-number enrichment for dashboard/email flows.To make callers’ expectations explicit, you might document whether
nullmeans “party not found”, “party is not an organization”, or both, so downstream logic can distinguish cases if needed.src/Altinn.Profile/Models/DashboardUserContactInformationResponse.cs (1)
16-39: Dashboard response model updates correctly reflect relaxed SSN and added org contextMaking
NationalIdentityNumbernullable and addingOrganizationNumberas an optional field matches the new behavior exercised in the integration tests (including the “SSN null but name present” case and the email-address lookups that surface organization numbers).You might optionally extend the class summary to mention that the response now includes
OrganizationNumber, to keep high-level docs in sync.src/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfoWithIdentity.cs (1)
12-33: Identity-enriched contact model matches new dashboard/data requirementsAllowing
NationalIdentityNumberto be nullable and addingOrganizationNumberhere is consistent with the updated dashboard response model and the new service methods that map org numbers fromIRegisterClient. This keeps the internal identity shape aligned with what the API surfaces.Optionally, you could update the class-level XML summary to mention that it now includes organization number as well as identity fields.
src/Altinn.Profile.Integrations/Repositories/ProfessionalNotificationsRepository.cs (1)
64-75: Redundant condition and missingIncludefor resources.Two observations:
The condition
!string.IsNullOrEmpty(x.EmailAddress)is redundant when combined withx.EmailAddress == emailAddress(assumingemailAddressis a non-null/non-empty string).Unlike
GetAllNotificationAddressesForUserAsyncandGetAllNotificationAddressesForPartyAsync, this method doesn't includeUserPartyContactInfoResources. If the caller needs resource information, this could cause incomplete data. If intentional, consider adding a comment explaining why.public async Task<IReadOnlyList<UserPartyContactInfo>> GetAllContactInfoByEmailAddressAsync(string emailAddress, CancellationToken cancellationToken) { using ProfileDbContext databaseContext = await _contextFactory.CreateDbContextAsync(cancellationToken); var contactInfos = await databaseContext.UserPartyContactInfo - .AsNoTracking() - .Where(x => x.EmailAddress == emailAddress && !string.IsNullOrEmpty(x.EmailAddress)) + .AsNoTracking() + .Where(x => x.EmailAddress == emailAddress) .ToListAsync(cancellationToken); return contactInfos; }src/Altinn.Profile.Integrations/Register/RegisterClient.cs (2)
110-126: Minor: Unnecessary null-conditional operator.At line 125,
responseData?[0].OrgNumberuses a null-conditional operator, butresponseDatais guaranteed to be non-null at this point due to the check at line 115. This is a minor inconsistency withGetPartyId(line 107) which doesn't use the null-conditional.- return responseData?[0].OrgNumber; + return responseData[0].OrgNumber;
169-190: LGTM! Good refactoring to extract common logic.The helper method
GetPartyIdentifierscleanly consolidates the HTTP request logic. One minor suggestion: the error log message at line 183 ("Failed to get partyId for party") could be updated to be more generic (e.g., "Failed to get party identifiers") since this helper now serves multiple purposes.if (!response.IsSuccessStatusCode) { - _logger.LogError("Failed to get partyId for party. Status code: {StatusCode}", response.StatusCode); + _logger.LogError("Failed to get party identifiers. Status code: {StatusCode}", response.StatusCode); return null; }src/Altinn.Profile/Controllers/DashboardController.cs (1)
206-213: Consider addingOrganizationNumberto the organization-based endpoint response for consistency.The email-based endpoint (lines 251-259) maps
OrganizationNumberto the response, but this endpoint doesn't. While the caller knows the org number from the request, including it in the response would maintain API consistency and align with the updatedDashboardUserContactInformationResponsemodel.var responses = contactInfos.Select(c => new DashboardUserContactInformationResponse { NationalIdentityNumber = c.NationalIdentityNumber, Name = c.Name, Email = c.EmailAddress, Phone = c.PhoneNumber, + OrganizationNumber = c.OrganizationNumber ?? organizationNumber, LastChanged = c.LastChanged }).ToList();Note: This also requires populating
OrganizationNumberinProfessionalNotificationsService.GetContactInformationByOrganizationNumberAsyncas flagged in the service review.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/Altinn.Profile.Core/Integrations/IProfessionalNotificationsRepository.cs(1 hunks)src/Altinn.Profile.Core/Integrations/IRegisterClient.cs(1 hunks)src/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.cs(1 hunks)src/Altinn.Profile.Core/ProfessionalNotificationAddresses/ProfessionalNotificationsService.cs(2 hunks)src/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfoWithIdentity.cs(2 hunks)src/Altinn.Profile.Integrations/Register/PartyIdentifiersResponse.cs(1 hunks)src/Altinn.Profile.Integrations/Register/RegisterClient.cs(2 hunks)src/Altinn.Profile.Integrations/Repositories/ProfessionalNotificationsRepository.cs(1 hunks)src/Altinn.Profile/Controllers/DashboardController.cs(1 hunks)src/Altinn.Profile/Models/DashboardUserContactInformationResponse.cs(2 hunks)test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/DashboardContactInformationControllerTests.cs(3 hunks)test/Bruno/.env.sample(1 hunks)test/Bruno/Profile/Dashboard/GetContactInformationByEmailAddress.bru(1 hunks)test/Bruno/environments/Local.bru(2 hunks)
🧰 Additional context used
🧠 Learnings (17)
📓 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-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:
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.cssrc/Altinn.Profile.Integrations/Repositories/ProfessionalNotificationsRepository.cssrc/Altinn.Profile.Core/Integrations/IProfessionalNotificationsRepository.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfoWithIdentity.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/ProfessionalNotificationsService.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:
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.cssrc/Altinn.Profile.Integrations/Repositories/ProfessionalNotificationsRepository.cssrc/Altinn.Profile.Core/Integrations/IProfessionalNotificationsRepository.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/ProfessionalNotificationsService.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:
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.cssrc/Altinn.Profile.Integrations/Repositories/ProfessionalNotificationsRepository.cssrc/Altinn.Profile/Controllers/DashboardController.cssrc/Altinn.Profile.Core/Integrations/IProfessionalNotificationsRepository.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfoWithIdentity.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/ProfessionalNotificationsService.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:
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.cssrc/Altinn.Profile.Integrations/Repositories/ProfessionalNotificationsRepository.cssrc/Altinn.Profile.Core/Integrations/IProfessionalNotificationsRepository.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfoWithIdentity.cstest/Altinn.Profile.Tests/IntegrationTests/API/Controllers/DashboardContactInformationControllerTests.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/ProfessionalNotificationsService.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:
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.cssrc/Altinn.Profile.Integrations/Repositories/ProfessionalNotificationsRepository.cssrc/Altinn.Profile.Core/Integrations/IProfessionalNotificationsRepository.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfoWithIdentity.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/ProfessionalNotificationsService.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:
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.cssrc/Altinn.Profile.Integrations/Repositories/ProfessionalNotificationsRepository.cssrc/Altinn.Profile.Core/Integrations/IProfessionalNotificationsRepository.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfoWithIdentity.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/ProfessionalNotificationsService.cs
📚 Learning: 2025-07-01T12:20:26.893Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 440
File: src/Altinn.Profile.Core/ProfessionalNotificationAddresses/ProfessionalNotificationsService.cs:45-62
Timestamp: 2025-07-01T12:20:26.893Z
Learning: In the Altinn.Profile repository, when sending notifications after a database commit (such as in ProfessionalNotificationsService.HandleNotificationAddressChangedAsync), CancellationToken.None is intentionally used instead of propagating the original cancellation token. This ensures notifications are sent even if the original request is cancelled, maintaining consistency since the database changes have already been committed.
Applied to files:
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.cssrc/Altinn.Profile.Integrations/Repositories/ProfessionalNotificationsRepository.cssrc/Altinn.Profile.Core/Integrations/IProfessionalNotificationsRepository.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/ProfessionalNotificationsService.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:
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.cssrc/Altinn.Profile.Integrations/Repositories/ProfessionalNotificationsRepository.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfoWithIdentity.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/ProfessionalNotificationsService.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:
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.cssrc/Altinn.Profile.Core/Integrations/IRegisterClient.cssrc/Altinn.Profile.Integrations/Register/RegisterClient.cssrc/Altinn.Profile.Integrations/Repositories/ProfessionalNotificationsRepository.cstest/Bruno/environments/Local.brusrc/Altinn.Profile.Integrations/Register/PartyIdentifiersResponse.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfoWithIdentity.cssrc/Altinn.Profile/Models/DashboardUserContactInformationResponse.cstest/Altinn.Profile.Tests/IntegrationTests/API/Controllers/DashboardContactInformationControllerTests.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/ProfessionalNotificationsService.cs
📚 Learning: 2025-06-18T09:02:33.802Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 418
File: test/Bruno/Profile/Personal notificationaddress for an org/Get party.bru:0-0
Timestamp: 2025-06-18T09:02:33.802Z
Learning: Bruno request files in the test/Bruno directory can serve as example requests for manual API testing and documentation, not just automated tests. Example requests don't require assert blocks since they're meant for developers to manually run and explore API responses.
Applied to files:
test/Bruno/Profile/Dashboard/GetContactInformationByEmailAddress.bru
📚 Learning: 2025-04-25T06:50:48.433Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 351
File: src/Altinn.Profile.Core/OrganizationNotificationAddresses/OrganizationNotificationAddressesService.cs:29-31
Timestamp: 2025-04-25T06:50:48.433Z
Learning: The IOrganizationNotificationAddressUpdateClient implementation already handles null registry IDs by throwing OrganizationNotificationAddressChangesException rather than returning null, so null checks on the returned registry ID are not needed in calling code.
Applied to files:
src/Altinn.Profile.Integrations/Register/RegisterClient.cs
📚 Learning: 2025-05-26T08:36:42.454Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 377
File: src/Altinn.Profile/Controllers/FavoritesController.cs:51-54
Timestamp: 2025-05-26T08:36:42.454Z
Learning: The PartyGroupService.GetFavorites method in src/Altinn.Profile.Core/PartyGroups/PartyGroupService.cs never returns null. It has a contract that returns Task<Group> (non-nullable) and uses null coalescing operator to create a default Group object when the repository returns null.
Applied to files:
src/Altinn.Profile.Integrations/Register/RegisterClient.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:
src/Altinn.Profile.Integrations/Repositories/ProfessionalNotificationsRepository.cssrc/Altinn.Profile/Controllers/DashboardController.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfoWithIdentity.cssrc/Altinn.Profile/Models/DashboardUserContactInformationResponse.cs
📚 Learning: 2025-04-10T15:38:24.909Z
Learnt from: SandGrainOne
Repo: Altinn/altinn-profile PR: 343
File: src/Altinn.Profile/appsettings.json:40-42
Timestamp: 2025-04-10T15:38:24.909Z
Learning: Environment-specific values in Altinn.Profile are introduced as environment variables following standard naming conventions. These environment variables are defined outside the repository code, not hardcoded in configuration files.
Applied to files:
test/Bruno/environments/Local.bru
📚 Learning: 2025-11-07T11:50:03.810Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 0
File: :0-0
Timestamp: 2025-11-07T11:50:03.810Z
Learning: In the Altinn.Profile repository, the AddressOrigin field in the Organization model is a derived/computed field set by the service layer (OrganizationNotificationAddressesService) during request processing, not stored in the database. Test data should reflect the raw database state and should not include AddressOrigin, as the service layer handles this enrichment when addresses are inherited from parent/main units.
Applied to files:
src/Altinn.Profile.Integrations/Register/PartyIdentifiersResponse.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfoWithIdentity.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/ProfessionalNotificationsService.cs
📚 Learning: 2025-07-28T11:47:37.773Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 470
File: src/Altinn.Profile.Integrations/SblBridge/User.Favorites/UserFavoriteClient.cs:23-31
Timestamp: 2025-07-28T11:47:37.773Z
Learning: In the Altinn Profile repository, explicit null safety validation in constructors (such as ArgumentNullException.ThrowIfNull checks) should be avoided as it's considered code bloat. The team prefers to rely on dependency injection validation rather than adding defensive null checks in constructor parameters.
Applied to files:
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfoWithIdentity.cs
🧬 Code graph analysis (5)
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.cs (1)
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfoWithIdentity.cs (1)
UserPartyContactInfoWithIdentity(7-39)
src/Altinn.Profile.Integrations/Register/RegisterClient.cs (1)
src/Altinn.Profile.Core/Integrations/IRegisterClient.cs (4)
Task(16-16)Task(23-23)Task(31-31)Task(39-39)
src/Altinn.Profile.Integrations/Repositories/ProfessionalNotificationsRepository.cs (2)
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfo.cs (1)
UserPartyContactInfo(12-64)src/Altinn.Profile.Integrations/Persistence/ProfiledbContext.cs (2)
ProfileDbContext(18-256)ProfileDbContext(24-27)
src/Altinn.Profile.Core/Integrations/IProfessionalNotificationsRepository.cs (1)
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfo.cs (1)
UserPartyContactInfo(12-64)
test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/DashboardContactInformationControllerTests.cs (2)
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/ProfessionalNotificationsService.cs (7)
Task(28-43)Task(46-58)Task(61-76)Task(84-101)Task(104-107)Task(110-168)Task(171-218)src/Altinn.Profile/Models/DashboardUserContactInformationResponse.cs (1)
DashboardUserContactInformationResponse(11-47)
⏰ 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 (7)
test/Bruno/.env.sample (1)
9-9: Sample configuration update aligns with new email-based feature.The test email value change is appropriate for the new
GetContactInformationByEmailAddressendpoint testing. Assuming the[email protected]email is validated to exist in the test environment (which aligns with your completed manual testing), this configuration is ready.test/Bruno/environments/Local.bru (1)
2-11: Local Bruno env changes look consistentSwitching
Environmenttoat22and keepingParty_PartyUuidexplicitly empty is fine for a local/manual test profile, assuming that matches how the new dashboard/email tests are exercised in your environment.test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/DashboardContactInformationControllerTests.cs (2)
463-526: Updated SSN-null org-number test matches new nullable SSN semanticsThis test now correctly captures the intended behavior: when only SSN is null but the name is present, the user is included with an empty/nullable
NationalIdentityNumberrather than being skipped. Assertions on the rest of the fields andLastChangedlook consistent with the model and service behavior.
589-816: Email-address integration tests give solid coverage of the new endpointThe four new tests collectively verify:
- Happy path with multiple users across different parties/org numbers.
- No-contact-info case returning
200 OKand an empty list.- Missing scope returning
403 Forbidden.- Skipping users when profile lookup fails while still enriching organization number via
IRegisterClient.Use of
Uri.EscapeDataStringfor the path parameter and the repository/register mocks both align well with the newGetAllContactInfoByEmailAddressAsyncandGetOrganizationNumberByPartyUuidAPIs.src/Altinn.Profile.Integrations/Register/PartyIdentifiersResponse.cs (1)
18-21: OrgNumber addition to PartyIdentifiersResponse is straightforward and safeAdding nullable
OrgNumberwith an init-only setter cleanly extends the register response without affecting existing consumers. XML comments clearly document the organization-only nature of the field.src/Altinn.Profile.Core/ProfessionalNotificationAddresses/ProfessionalNotificationsService.cs (2)
147-159: Inconsistency:OrganizationNumbernot populated in organization-based lookup.The
GetContactInformationByOrganizationNumberAsyncmethod doesn't populateOrganizationNumberin the result, while the newGetContactInformationByEmailAddressAsyncdoes (line 207). This creates an inconsistency in the returnedUserPartyContactInfoWithIdentityobjects depending on which method is called.If intentional (caller already knows the org number from the request), consider adding a comment. Otherwise, populate it for consistency:
results.Add(new UserPartyContactInfoWithIdentity { NationalIdentityNumber = profile.Party.SSN, Name = profile.Party.Name, EmailAddress = contactInfo.EmailAddress, PhoneNumber = contactInfo.PhoneNumber, + OrganizationNumber = organizationNumber, LastChanged = contactInfo.LastChanged });
170-218: Implementation looks correct and follows established patterns.The method correctly:
- Returns an empty list when no contact infos are found (line 178)
- Follows the same sequential execution pattern as
GetContactInformationByOrganizationNumberAsync(acceptable for low-traffic dashboard endpoint)- Enriches results with organization numbers and user identity
- Silently skips users when profile retrieval fails (consistent with existing behavior)
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.cs
Show resolved
Hide resolved
…c inside ProfessionalNotificationsRepository
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/Altinn.Profile.Tests/Profile.Integrations/ProfessionalNotifications/ProfessionalNotificationsRepositoryTests.cs (1)
865-889: Consider extending edge‑case coverage for null/empty email handling
GetAllContactInfoByEmailAddressAsync_ExcludesNullOrEmptyEmailsalready verifies that:
- Records with
EmailAddress == nullare excluded from matches, and- An empty search string returns an empty result.
To fully lock in the method’s contract, consider:
- Adding a record with
EmailAddress = string.Emptyand asserting it is not returned for a non‑empty search.- Adding a separate test that calls
GetAllContactInfoByEmailAddressAsync(null, ...)and asserts an empty result, to mirror thestring.IsNullOrEmpty(emailAddress)guard.For example:
+ [Fact] + public async Task GetAllContactInfoByEmailAddressAsync_WhenEmailIsNull_ReturnsEmptyList() + { + // Act + var result = await _repository.GetAllContactInfoByEmailAddressAsync(null, CancellationToken.None); + + // Assert + Assert.NotNull(result); + Assert.Empty(result); + }This would tighten regression protection around the null/empty input behavior without changing the current tests.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Altinn.Profile.Integrations/Repositories/ProfessionalNotificationsRepository.cs(1 hunks)test/Altinn.Profile.Tests/Profile.Integrations/ProfessionalNotifications/ProfessionalNotificationsRepositoryTests.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Altinn.Profile.Integrations/Repositories/ProfessionalNotificationsRepository.cs
🧰 Additional context used
🧠 Learnings (9)
📓 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.
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.
📚 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/Profile.Integrations/ProfessionalNotifications/ProfessionalNotificationsRepositoryTests.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/Profile.Integrations/ProfessionalNotifications/ProfessionalNotificationsRepositoryTests.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/Profile.Integrations/ProfessionalNotifications/ProfessionalNotificationsRepositoryTests.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/Profile.Integrations/ProfessionalNotifications/ProfessionalNotificationsRepositoryTests.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/Profile.Integrations/ProfessionalNotifications/ProfessionalNotificationsRepositoryTests.cs
📚 Learning: 2025-07-01T12:20:26.893Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 440
File: src/Altinn.Profile.Core/ProfessionalNotificationAddresses/ProfessionalNotificationsService.cs:45-62
Timestamp: 2025-07-01T12:20:26.893Z
Learning: In the Altinn.Profile repository, when sending notifications after a database commit (such as in ProfessionalNotificationsService.HandleNotificationAddressChangedAsync), CancellationToken.None is intentionally used instead of propagating the original cancellation token. This ensures notifications are sent even if the original request is cancelled, maintaining consistency since the database changes have already been committed.
Applied to files:
test/Altinn.Profile.Tests/Profile.Integrations/ProfessionalNotifications/ProfessionalNotificationsRepositoryTests.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/Profile.Integrations/ProfessionalNotifications/ProfessionalNotificationsRepositoryTests.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/Profile.Integrations/ProfessionalNotifications/ProfessionalNotificationsRepositoryTests.cs
🧬 Code graph analysis (1)
test/Altinn.Profile.Tests/Profile.Integrations/ProfessionalNotifications/ProfessionalNotificationsRepositoryTests.cs (2)
src/Altinn.Profile.Integrations/Repositories/ProfessionalNotificationsRepository.cs (8)
Task(22-32)Task(35-47)Task(50-62)Task(65-81)Task(84-118)Task(121-140)Task(144-177)Task(218-236)src/Altinn.Profile.Core/Integrations/IProfessionalNotificationsRepository.cs (6)
Task(17-17)Task(25-25)Task(33-33)Task(41-41)Task(52-52)Task(61-61)
⏰ 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 (1)
test/Altinn.Profile.Tests/Profile.Integrations/ProfessionalNotifications/ProfessionalNotificationsRepositoryTests.cs (1)
828-849: New email-lookup tests align well with repository behaviorThe added tests for multiple users, no contacts, and case-insensitive matching are clear, follow existing naming/arrange‑act‑assert patterns, and correctly exercise
GetAllContactInfoByEmailAddressAsyncwithout relying on ordering. No functional issues spotted here.Also applies to: 851-863, 891-913
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.cs (1)
54-61: Fix copy-pasted documentation.Line 55 incorrectly states "registered for a specific organization" but this method retrieves contact information by email address, not organization number.
Apply this diff:
/// <summary> -/// Retrieves all user contact information registered for a specific organization, enriched with user identity (SSN and Name). +/// Retrieves all user contact information for a specific email address, enriched with user identity (SSN and Name). /// Used by the Support Dashboard to display personal contact details that users have registered. /// </summary>
🧹 Nitpick comments (1)
src/Altinn.Profile.Integrations/Register/RegisterClient.cs (1)
95-108: LGTM! Good refactoring to eliminate duplication.The refactoring of
GetPartyIdto use the newGetPartyIdentifiershelper consolidates the HTTP request logic and maintains the same behavior while reducing code duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/Altinn.Profile.Core/Integrations/IProfessionalNotificationsRepository.cs(1 hunks)src/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.cs(1 hunks)src/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfoWithIdentity.cs(2 hunks)src/Altinn.Profile.Integrations/Register/RegisterClient.cs(2 hunks)src/Altinn.Profile/Controllers/DashboardController.cs(1 hunks)src/Altinn.Profile/Models/DashboardUserContactInformationResponse.cs(2 hunks)test/Bruno/Profile/Dashboard/GetContactInformationByEmailAddress.bru(1 hunks)
🧰 Additional context used
🧠 Learnings (16)
📓 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.
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.
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 351
File: src/Altinn.Profile.Core/OrganizationNotificationAddresses/OrganizationNotificationAddressesService.cs:29-31
Timestamp: 2025-04-25T06:50:48.433Z
Learning: The IOrganizationNotificationAddressUpdateClient implementation already handles null registry IDs by throwing OrganizationNotificationAddressChangesException rather than returning null, so null checks on the returned registry ID are not needed in calling code.
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.
📚 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:
src/Altinn.Profile.Core/Integrations/IProfessionalNotificationsRepository.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.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:
src/Altinn.Profile.Core/Integrations/IProfessionalNotificationsRepository.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfoWithIdentity.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:
src/Altinn.Profile.Core/Integrations/IProfessionalNotificationsRepository.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfoWithIdentity.cs
📚 Learning: 2025-07-01T12:20:26.893Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 440
File: src/Altinn.Profile.Core/ProfessionalNotificationAddresses/ProfessionalNotificationsService.cs:45-62
Timestamp: 2025-07-01T12:20:26.893Z
Learning: In the Altinn.Profile repository, when sending notifications after a database commit (such as in ProfessionalNotificationsService.HandleNotificationAddressChangedAsync), CancellationToken.None is intentionally used instead of propagating the original cancellation token. This ensures notifications are sent even if the original request is cancelled, maintaining consistency since the database changes have already been committed.
Applied to files:
src/Altinn.Profile.Core/Integrations/IProfessionalNotificationsRepository.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.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:
src/Altinn.Profile.Core/Integrations/IProfessionalNotificationsRepository.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.cssrc/Altinn.Profile/Controllers/DashboardController.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfoWithIdentity.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:
src/Altinn.Profile.Core/Integrations/IProfessionalNotificationsRepository.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.cssrc/Altinn.Profile/Controllers/DashboardController.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfoWithIdentity.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:
src/Altinn.Profile.Core/Integrations/IProfessionalNotificationsRepository.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.cssrc/Altinn.Profile/Controllers/DashboardController.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfoWithIdentity.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:
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.cssrc/Altinn.Profile/Controllers/DashboardController.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfoWithIdentity.cs
📚 Learning: 2025-04-25T06:50:48.433Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 351
File: src/Altinn.Profile.Core/OrganizationNotificationAddresses/OrganizationNotificationAddressesService.cs:29-31
Timestamp: 2025-04-25T06:50:48.433Z
Learning: The IOrganizationNotificationAddressUpdateClient implementation already handles null registry IDs by throwing OrganizationNotificationAddressChangesException rather than returning null, so null checks on the returned registry ID are not needed in calling code.
Applied to files:
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.cssrc/Altinn.Profile.Integrations/Register/RegisterClient.cs
📚 Learning: 2025-04-11T11:18:33.803Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 338
File: src/Altinn.Profile.Integrations/OrganizationNotificationAddressRegistry/OrganizationNotificationAddressHttpClient.cs:0-0
Timestamp: 2025-04-11T11:18:33.803Z
Learning: The UpdateEndpoint property from OrganizationNotificationAddressSettings is validated in another location in the codebase, so explicit null checks are not needed in the PostAsync method of OrganizationNotificationAddressHttpClient.
Applied to files:
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.cs
📚 Learning: 2025-06-18T09:02:33.802Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 418
File: test/Bruno/Profile/Personal notificationaddress for an org/Get party.bru:0-0
Timestamp: 2025-06-18T09:02:33.802Z
Learning: Bruno request files in the test/Bruno directory can serve as example requests for manual API testing and documentation, not just automated tests. Example requests don't require assert blocks since they're meant for developers to manually run and explore API responses.
Applied to files:
test/Bruno/Profile/Dashboard/GetContactInformationByEmailAddress.bru
📚 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:
src/Altinn.Profile/Controllers/DashboardController.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfoWithIdentity.cssrc/Altinn.Profile/Models/DashboardUserContactInformationResponse.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:
src/Altinn.Profile.Integrations/Register/RegisterClient.cssrc/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfoWithIdentity.cssrc/Altinn.Profile/Models/DashboardUserContactInformationResponse.cs
📚 Learning: 2025-05-26T08:36:42.454Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 377
File: src/Altinn.Profile/Controllers/FavoritesController.cs:51-54
Timestamp: 2025-05-26T08:36:42.454Z
Learning: The PartyGroupService.GetFavorites method in src/Altinn.Profile.Core/PartyGroups/PartyGroupService.cs never returns null. It has a contract that returns Task<Group> (non-nullable) and uses null coalescing operator to create a default Group object when the repository returns null.
Applied to files:
src/Altinn.Profile.Integrations/Register/RegisterClient.cs
📚 Learning: 2025-11-07T11:50:03.810Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 0
File: :0-0
Timestamp: 2025-11-07T11:50:03.810Z
Learning: In the Altinn.Profile repository, the AddressOrigin field in the Organization model is a derived/computed field set by the service layer (OrganizationNotificationAddressesService) during request processing, not stored in the database. Test data should reflect the raw database state and should not include AddressOrigin, as the service layer handles this enrichment when addresses are inherited from parent/main units.
Applied to files:
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfoWithIdentity.cs
🧬 Code graph analysis (3)
src/Altinn.Profile.Core/Integrations/IProfessionalNotificationsRepository.cs (1)
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfo.cs (1)
UserPartyContactInfo(12-64)
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.cs (1)
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfoWithIdentity.cs (1)
UserPartyContactInfoWithIdentity(8-40)
src/Altinn.Profile.Integrations/Register/RegisterClient.cs (1)
src/Altinn.Profile.Core/Integrations/IRegisterClient.cs (4)
Task(16-16)Task(23-23)Task(31-31)Task(39-39)
⏰ 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 (7)
src/Altinn.Profile.Core/Integrations/IProfessionalNotificationsRepository.cs (1)
35-41: LGTM!The interface method is well-documented and follows existing patterns in the repository interface. The return type correctly indicates a non-nullable list that will be empty when no contacts are found.
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfoWithIdentity.cs (1)
6-6: LGTM!The data model changes appropriately support the new email-based lookup feature:
- Making
NationalIdentityNumbernullable accommodates cases where SSN might not be available- Adding
OrganizationNumberenables displaying which organization the user is acting on behalf ofThese changes align with the corresponding updates in
DashboardUserContactInformationResponse.Also applies to: 13-13, 30-34
src/Altinn.Profile/Models/DashboardUserContactInformationResponse.cs (1)
10-10: LGTM!The response model changes are consistent with the domain model (
UserPartyContactInfoWithIdentity) and properly support the new email-based lookup endpoint. MakingNationalIdentityNumbernullable and addingOrganizationNumberaligns with the feature requirements.Also applies to: 17-17, 36-40
src/Altinn.Profile.Integrations/Register/RegisterClient.cs (2)
110-126: LGTM! Consistent implementation pattern.The new
GetOrganizationNumberByPartyUuidmethod follows the same pattern asGetPartyId, including error handling and logging. Returningnullwhen the organization number is not available is appropriate and aligns with the nullableOrganizationNumberfield in the response models.
169-190: LGTM! Well-structured helper method.The
GetPartyIdentifiershelper consolidates the party identifiers lookup logic effectively. Error handling, logging, and token injection are all properly implemented.src/Altinn.Profile/Controllers/DashboardController.cs (2)
246-249: LGTM! Correct empty-list check.The previous review concern about an unreachable null check has been properly addressed. Checking
Count == 0correctly returns 404 when no contact information is found for the email address.
251-259: LGTM! Complete response mapping.The response mapping correctly includes the new
OrganizationNumberfield, providing complete contact information including which organization the user is acting on behalf of.
test/Bruno/Profile/Dashboard/GetContactInformationByEmailAddress.bru
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
test/Altinn.Profile.Tests/Profile.Integrations/ProfessionalNotifications/ProfessionalNotificationsRepositoryTests.cs (2)
835-836: Fix inconsistent spacing around assignment operators.Lines 835-836 have inconsistent spacing around the
=operator (userId1 =101should beuserId1 = 101).Apply this diff:
- int userId1 =101; - int userId2 =202; + int userId1 = 101; + int userId2 = 202;
871-872: Fix inconsistent spacing around assignment operators.Lines 871-872 have the same formatting issue as lines 835-836.
Apply this diff:
- int userIdMatching =303; - int userIdNull =404; + int userIdMatching = 303; + int userIdNull = 404;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/Altinn.Profile.Tests/Profile.Integrations/ProfessionalNotifications/ProfessionalNotificationsRepositoryTests.cs(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 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.
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.
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 351
File: src/Altinn.Profile.Core/OrganizationNotificationAddresses/OrganizationNotificationAddressesService.cs:29-31
Timestamp: 2025-04-25T06:50:48.433Z
Learning: The IOrganizationNotificationAddressUpdateClient implementation already handles null registry IDs by throwing OrganizationNotificationAddressChangesException rather than returning null, so null checks on the returned registry ID are not needed in calling code.
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.
📚 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/Profile.Integrations/ProfessionalNotifications/ProfessionalNotificationsRepositoryTests.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/Profile.Integrations/ProfessionalNotifications/ProfessionalNotificationsRepositoryTests.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/Profile.Integrations/ProfessionalNotifications/ProfessionalNotificationsRepositoryTests.cs
📚 Learning: 2025-07-01T12:20:26.893Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 440
File: src/Altinn.Profile.Core/ProfessionalNotificationAddresses/ProfessionalNotificationsService.cs:45-62
Timestamp: 2025-07-01T12:20:26.893Z
Learning: In the Altinn.Profile repository, when sending notifications after a database commit (such as in ProfessionalNotificationsService.HandleNotificationAddressChangedAsync), CancellationToken.None is intentionally used instead of propagating the original cancellation token. This ensures notifications are sent even if the original request is cancelled, maintaining consistency since the database changes have already been committed.
Applied to files:
test/Altinn.Profile.Tests/Profile.Integrations/ProfessionalNotifications/ProfessionalNotificationsRepositoryTests.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/Profile.Integrations/ProfessionalNotifications/ProfessionalNotificationsRepositoryTests.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/Profile.Integrations/ProfessionalNotifications/ProfessionalNotificationsRepositoryTests.cs
🧬 Code graph analysis (1)
test/Altinn.Profile.Tests/Profile.Integrations/ProfessionalNotifications/ProfessionalNotificationsRepositoryTests.cs (4)
src/Altinn.Profile.Core/Integrations/IProfessionalNotificationsRepository.cs (6)
Task(17-17)Task(25-25)Task(33-33)Task(41-41)Task(52-52)Task(61-61)src/Altinn.Profile.Integrations/Repositories/ProfessionalNotificationsRepository.cs (8)
Task(22-32)Task(35-47)Task(50-62)Task(65-81)Task(84-118)Task(121-140)Task(144-177)Task(218-236)src/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfo.cs (1)
List(57-63)src/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfoResource.cs (1)
UserPartyContactInfoResource(6-27)
⏰ 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)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
test/Bruno/Profile/Dashboard/GetContactInformationByEmailAddress.bru (1)
113-118: Update documentation to remove 404 status code.Following the correction to the controller implementation (which will remove the
NotFound()return), the documentation should be updated to reflect that this endpoint always returns200 OK(with a possibly-empty array) and never returns404.Unlike the organization-number endpoint, there is no external registry to verify email existence, so the endpoint cannot distinguish between "email doesn't exist" and "email has no registered contact info."
Apply this diff:
## Status Codes - **200 OK**: Successfully retrieved user contact information (may be empty array) - **403 Forbidden**: Missing required scope - - **404 Not Found**: Email address doesn't exist
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Altinn.Profile/Controllers/DashboardController.cs(1 hunks)test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/DashboardContactInformationControllerTests.cs(3 hunks)test/Bruno/Profile/Dashboard/GetContactInformationByEmailAddress.bru(1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 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.
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.
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 351
File: src/Altinn.Profile.Core/OrganizationNotificationAddresses/OrganizationNotificationAddressesService.cs:29-31
Timestamp: 2025-04-25T06:50:48.433Z
Learning: The IOrganizationNotificationAddressUpdateClient implementation already handles null registry IDs by throwing OrganizationNotificationAddressChangesException rather than returning null, so null checks on the returned registry ID are not needed in calling code.
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.
📚 Learning: 2025-06-18T09:02:33.802Z
Learnt from: sigridbra
Repo: Altinn/altinn-profile PR: 418
File: test/Bruno/Profile/Personal notificationaddress for an org/Get party.bru:0-0
Timestamp: 2025-06-18T09:02:33.802Z
Learning: Bruno request files in the test/Bruno directory can serve as example requests for manual API testing and documentation, not just automated tests. Example requests don't require assert blocks since they're meant for developers to manually run and explore API responses.
Applied to files:
test/Bruno/Profile/Dashboard/GetContactInformationByEmailAddress.bru
📚 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/Bruno/Profile/Dashboard/GetContactInformationByEmailAddress.brusrc/Altinn.Profile/Controllers/DashboardController.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:
src/Altinn.Profile/Controllers/DashboardController.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:
src/Altinn.Profile/Controllers/DashboardController.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:
src/Altinn.Profile/Controllers/DashboardController.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:
src/Altinn.Profile/Controllers/DashboardController.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:
src/Altinn.Profile/Controllers/DashboardController.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:
src/Altinn.Profile/Controllers/DashboardController.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/API/Controllers/DashboardContactInformationControllerTests.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/DashboardContactInformationControllerTests.cs
🧬 Code graph analysis (2)
src/Altinn.Profile/Controllers/DashboardController.cs (2)
src/Altinn.Profile/Models/DashboardUserContactInformationResponse.cs (1)
DashboardUserContactInformationResponse(12-48)src/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.cs (6)
Task(15-15)Task(23-23)Task(34-34)Task(43-43)Task(52-52)Task(61-61)
test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/DashboardContactInformationControllerTests.cs (3)
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/IProfessionalNotificationsService.cs (6)
Task(15-15)Task(23-23)Task(34-34)Task(43-43)Task(52-52)Task(61-61)src/Altinn.Profile.Core/ProfessionalNotificationAddresses/ProfessionalNotificationsService.cs (7)
Task(28-43)Task(46-58)Task(61-76)Task(84-101)Task(104-107)Task(110-168)Task(171-218)src/Altinn.Profile/Models/DashboardUserContactInformationResponse.cs (1)
DashboardUserContactInformationResponse(12-48)
⏰ 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 (5)
test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/DashboardContactInformationControllerTests.cs (5)
462-526: LGTM: Test correctly validates nullable SSN behavior.The renamed test method now correctly validates that users with null
NationalIdentityNumberare included in results (not skipped), which aligns with the updatedDashboardUserContactInformationResponsemodel whereNationalIdentityNumberis nullable.
589-687: LGTM: Comprehensive test coverage for multiple users with email-based lookup.This test thoroughly validates the email-based contact information endpoint with multiple users across different organizations, including:
- Identity enrichment via SBL Bridge (SSN, Name)
- Organization number resolution via RegisterClient
- Proper mapping of all response fields
689-714: Test expectation is correct but will fail until controller is fixed.This test correctly expects
200 OKwith an empty array when no contact information exists for an email address. However, it will fail with the current controller implementation which returns404 NotFound(see review comment on DashboardController.cs lines 246-249).Once the controller is updated to remove the
NotFound()return, this test will pass as expected.
716-732: LGTM: Authorization test is correct.This test correctly validates that requests without the required
altinn:profile.support.adminscope receive a403 Forbiddenresponse.
734-817: LGTM: Test correctly validates profile-not-found skipping behavior.This test comprehensively validates that when a user profile cannot be retrieved from SBL Bridge (404 response), that user is silently skipped from the results while other valid users are still returned. This aligns with the error-handling pattern used in the service layer.
| [ProducesResponseType(typeof(List<DashboardUserContactInformationResponse>), StatusCodes.Status200OK)] | ||
| [ProducesResponseType(typeof(ValidationProblemDetails), StatusCodes.Status400BadRequest)] | ||
| [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status403Forbidden)] | ||
| [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] |
Contributor
There was a problem hiding this comment.
Remove 404 ProducesResponseType attribute to match corrected behavior.
Following the removal of the NotFound() return (lines 246-249), this attribute is no longer accurate.
Apply this diff:
[ProducesResponseType(typeof(List<DashboardUserContactInformationResponse>), StatusCodes.Status200OK)]
[ProducesResponseType(typeof(ValidationProblemDetails), StatusCodes.Status400BadRequest)]
[ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status403Forbidden)]
- [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)]🤖 Prompt for AI Agents
In src/Altinn.Profile/Controllers/DashboardController.cs around line 233, the
[ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)]
attribute is stale because the controller no longer returns NotFound(); remove
this 404 ProducesResponseType attribute so method's response metadata matches
current behavior and remaining ProducesResponseType attributes.
Comment on lines
+246
to
+249
| if (contactInfosByEmail.Count == 0) | ||
| { | ||
| return NotFound(); | ||
| } |
Contributor
There was a problem hiding this comment.
Remove NotFound check: inconsistent with tests and documentation.
The controller returns NotFound() when no contact information exists, but:
- The integration test
GetContactInformationByEmailAddress_WhenEmailHasNoContactInfo_ReturnsEmptyList(line 708 in test file) expects200 OKwith an empty array - Line 225 XML documentation states "empty array if no contacts found"
- Unlike the org-number endpoint which can verify if an org exists in an external registry, there's no way to distinguish between "email doesn't exist" and "email has no registered contact info"
The org-number endpoint only returns 404 when the organization isn't found in the register (line 201), but returns 200 OK with an empty array when the org exists but has no contact info (consistent with line 206-213 mapping).
Apply this diff to align with test expectations and documentation:
var contactInfosByEmail = await _professionalNotificationsService
.GetContactInformationByEmailAddressAsync(emailAddress, cancellationToken);
- if (contactInfosByEmail.Count == 0)
- {
- return NotFound();
- }
-
var responses = contactInfosByEmail.Select(c => new DashboardUserContactInformationResponseAlso update the XML documentation to remove the contradictory 404 status code:
/// <response code="200">Successfully retrieved user contact information. Returns an array of contacts (empty array if no contacts found).</response>
/// <response code="400">Invalid request parameters (model validation failed).</response>
/// <response code="403">Caller does not have the required Dashboard Maskinporten scope (altinn:profile.support.admin).</response>
-/// <response code="404">No contact information found for the specified email address.</response>🤖 Prompt for AI Agents
In src/Altinn.Profile/Controllers/DashboardController.cs around lines 246-249,
remove the NotFound() branch and ensure the method always returns
Ok(contactInfosByEmail) (which will be an empty array when no contacts exist) to
match tests and XML docs; additionally update the method's XML documentation
near line 225 to remove the contradictory 404 status code and state that it
returns 200 OK with an empty array when no contacts are found.
Comment on lines
+37
to
+50
| expect(contact).to.have.property('organizationNumber'); | ||
| if(contact.organizationNumber !== null) { | ||
| expect(contact.organizationNumber).to.be.a('string'); | ||
| expect(contact.lastChanged).to.be.a('string'); | ||
| expect(new Date(contact.lastChanged).toString()).to.not.equal('Invalid Date'); | ||
| // email and phone are optional but should be strings if present | ||
| if ('email' in contact && contact.email !== null) { | ||
| expect(contact.email).to.be.a('string'); | ||
| } | ||
| if ('phone' in contact && contact.phone !== null) { | ||
| expect(contact.phone).to.be.a('string'); | ||
| } | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
Fix syntax error: missing closing brace causes incorrect test logic.
Line 38 opens an if block to check if organizationNumber is not null, but the closing brace is missing. This causes the lastChanged assertions (lines 40-41) to be incorrectly nested inside the organizationNumber null check. Since lastChanged is a required field (always present), its assertions should execute unconditionally.
Apply this diff to fix the structure:
expect(contact).to.have.property('organizationNumber');
if(contact.organizationNumber !== null) {
expect(contact.organizationNumber).to.be.a('string');
+ }
expect(contact.lastChanged).to.be.a('string');
expect(new Date(contact.lastChanged).toString()).to.not.equal('Invalid Date');📝 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
| expect(contact).to.have.property('organizationNumber'); | |
| if(contact.organizationNumber !== null) { | |
| expect(contact.organizationNumber).to.be.a('string'); | |
| expect(contact.lastChanged).to.be.a('string'); | |
| expect(new Date(contact.lastChanged).toString()).to.not.equal('Invalid Date'); | |
| // email and phone are optional but should be strings if present | |
| if ('email' in contact && contact.email !== null) { | |
| expect(contact.email).to.be.a('string'); | |
| } | |
| if ('phone' in contact && contact.phone !== null) { | |
| expect(contact.phone).to.be.a('string'); | |
| } | |
| }); | |
| }); | |
| expect(contact).to.have.property('organizationNumber'); | |
| if(contact.organizationNumber !== null) { | |
| expect(contact.organizationNumber).to.be.a('string'); | |
| } | |
| expect(contact.lastChanged).to.be.a('string'); | |
| expect(new Date(contact.lastChanged).toString()).to.not.equal('Invalid Date'); | |
| // email and phone are optional but should be strings if present | |
| if ('email' in contact && contact.email !== null) { | |
| expect(contact.email).to.be.a('string'); | |
| } | |
| if ('phone' in contact && contact.phone !== null) { | |
| expect(contact.phone).to.be.a('string'); | |
| } | |
| }); | |
| }); |
🤖 Prompt for AI Agents
In test/Bruno/Profile/Dashboard/GetContactInformationByEmailAddress.bru around
lines 37 to 50, there's a missing closing brace for the if
(contact.organizationNumber !== null) block which causes the subsequent
lastChanged assertions to be nested incorrectly; add the missing closing brace
immediately after the expect(contact.organizationNumber).to.be.a('string'); line
so that lastChanged assertions run unconditionally, and ensure the following
if-checks for email and phone remain correctly scoped and indented.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/DashboardContactInformationControllerTests.cs (1)
520-525: Consider using a more specific assertion forNationalIdentityNumber.
Assert.True(string.IsNullOrEmpty(...))is ambiguous—it accepts bothnulland"". Given thatNationalIdentityNumberisstring?inDashboardUserContactInformationResponse, consider being explicit about the expected value:- Assert.True(string.IsNullOrEmpty(result[0].NationalIdentityNumber)); + Assert.Null(result[0].NationalIdentityNumber);Or if the service layer explicitly sets it to empty string:
- Assert.True(string.IsNullOrEmpty(result[0].NationalIdentityNumber)); + Assert.Equal(string.Empty, result[0].NationalIdentityNumber);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/DashboardContactInformationControllerTests.cs(3 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 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.
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.
📚 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/DashboardContactInformationControllerTests.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/DashboardContactInformationControllerTests.cs
🧬 Code graph analysis (1)
test/Altinn.Profile.Tests/IntegrationTests/API/Controllers/DashboardContactInformationControllerTests.cs (2)
src/Altinn.Profile.Core/ProfessionalNotificationAddresses/UserPartyContactInfo.cs (1)
UserPartyContactInfo(12-64)src/Altinn.Profile/Models/DashboardUserContactInformationResponse.cs (1)
DashboardUserContactInformationResponse(12-48)
⏰ 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/DashboardContactInformationControllerTests.cs (4)
589-687: Well-structured test for the new email-based endpoint.The test thoroughly validates the identity enrichment flow, including:
- Repository lookup by email address
- Organization number resolution via
GetOrganizationNumberByPartyUuid- User profile enrichment from the SBL bridge
- Proper assertions on all response fields including
OrganizationNumberThe use of
Uri.EscapeDataStringfor email encoding is appropriate.
689-709: LGTM!The test correctly validates that a 404 response is returned when no contact information exists for the given email address.
711-727: LGTM!The test correctly validates that requests without the required
altinn:profile.support.adminscope receive a 403 Forbidden response.
729-812: LGTM - Test correctly validates graceful handling of missing user profiles.The test properly validates that users whose profile cannot be fetched from the SBL bridge are skipped, and that the remaining valid users are still returned in the response.
Consider whether additional edge case tests are needed for the email endpoint to match org-number endpoint coverage:
WhenUserPartyIsNull_SkipsUserWhenPartySsnAndNameAreNull_SkipsUserWhenOnlyNameIsNull_SkipsUserWhenOnlySsnIsNull_ReturnsWithEmptySSNIf the identity enrichment logic is shared between both endpoints (as suggested by the service layer code), the existing org-number tests may sufficiently cover these paths. However, explicit tests for the email endpoint would provide better regression protection.
|
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.
Description
Created new endpoint GetContactInformationByEmailAddress
Related Issue(s)
Verification
Summary by CodeRabbit
New Features
Enhancements
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.