fix(ids-api): Use name field instead of fullname. #17164
Conversation
WalkthroughThe changes in this pull request primarily focus on the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
apps/services/auth/ids-api/src/app/user-profile/user-profile.service.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
apps/services/auth/ids-api/src/app/user-profile/user-profile.service.ts (2)
Pattern apps/services/**/*: "Confirm that the code adheres to the following:
- NestJS architecture, including modules, services, and controllers.
- Dependency injection patterns and service encapsulation.
- Integration and unit testing coverage and practices."
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (2)
apps/services/auth/ids-api/src/app/user-profile/user-profile.service.ts (2)
Line range hint 1-40: Service structure follows NestJS best practices
The service implementation adheres to NestJS architecture guidelines with proper dependency injection and clear separation of concerns.
133-133: Simplified name field access aligns with PR objective
The change to use individual.nafn instead of fulltNafn?.fulltNafn aligns with the PR objective to use more reliable data.
Let's verify the impact of this change:
✅ Verification successful
Let me gather more context about the test files and the individual interface to better understand the impact.
Let me check the actual implementation to understand how nafn is being used in the codebase.
Change to use nafn is consistent with existing codebase patterns
Based on the codebase analysis, this change is safe and follows established patterns:
individual.nafnis already used as the primary name field across the codebase- The national registry mapper uses
nafnas a fallback in multiple places:fullName: individual.fulltNafn?.fulltNafn ?? individual.nafn - Other services like auth-api-lib and national-registry client also use
individual.nafndirectly
The change aligns with the existing implementation patterns and maintains consistency across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any direct references to fulltNafn in the codebase
rg "fulltNafn" --type ts
# Search for potential test files that might need updates
fd "user-profile.service.spec.ts|user-profile.e2e-spec.ts" -t f
Length of output: 6373
Script:
#!/bin/bash
# Check the Individual interface definition
ast-grep --pattern 'interface Individual {
$$$
}'
# Look for test files content
rg -A 5 "nafn|fulltNafn" apps/services/auth/ids-api/src/app/user-profile/user-profile.service.spec.ts || echo "File not found"
Length of output: 313
Script:
#!/bin/bash
# Search for nafn usage in mapper and related files
rg "nafn:" -A 3 -B 3 libs/api/domains/national-registry/src/lib/v3/mapper.ts
# Check if there are any direct usages of individual.nafn
rg "individual\.nafn" --type ts
Length of output: 1203
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17164 +/- ##
=======================================
Coverage 35.73% 35.73%
=======================================
Files 6926 6926
Lines 147681 147674 -7
Branches 42062 42059 -3
=======================================
- Hits 52776 52775 -1
+ Misses 94905 94899 -6
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 5 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 23 Total Test Services: 0 Failed, 22 Passed Test ServicesThis report shows up to 10 services
|
valurefugl
added
the
deprecated:automerge
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
What
Use
nafninstead offulltNafnfor name in user profile.Why
The value in
fulltNafnapparently has not always been up to date. Usingnafnalso aligns with my pages and other services.Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
New Features
Chores