feat(app-sys): national-id-with-name fetch form more endpoints #17101
Conversation
WalkthroughThe changes in this pull request primarily enhance the functionality of 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
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17101 +/- ##
==========================================
- Coverage 35.69% 35.68% -0.01%
==========================================
Files 6917 6917
Lines 147412 147445 +33
Branches 41952 41976 +24
==========================================
+ Hits 52617 52618 +1
- Misses 94795 94827 +32 Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 21 Total Test Services: 0 Failed, 20 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (3) |
….com:island-is/island.is into feat/more-fetching-for-national-id-with-name
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
libs/application/ui-components/src/components/NationalIdWithName/NationalIdWithName.tsx (4)
133-134: Simplify function call using optional chainingYou can simplify the conditional invocation of
onNameChangeby using optional chaining.Suggested change:
- onNameChange && - onNameChange(companyData.companyRegistryCompany?.name ?? '') + onNameChange?.(companyData.companyRegistryCompany?.name ?? '')🧰 Tools
🪛 Biome (1.9.4)
[error] 133-134: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
143-152: Remove unnecessary block statementsThe curly braces around this code block are unnecessary and can be removed to simplify the code.
Suggested change:
- { - searchPersons && - getIdentity({ - variables: { - input: { - nationalId: nationalIdInput, - }, - }, - }) - } + searchPersons && + getIdentity({ + variables: { + input: { + nationalId: nationalIdInput, + }, + }, + })🧰 Tools
🪛 Biome (1.9.4)
[error] 143-152: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.(lint/complexity/noUselessLoneBlockStatements)
154-163: Remove unnecessary block statementsSimilarly, the curly braces around this block are unnecessary and can be removed.
Suggested change:
- { - searchCompanies && - getCompanyIdentity({ - variables: { - input: { - nationalId: nationalIdInput, - }, - }, - }) - } + searchCompanies && + getCompanyIdentity({ + variables: { + input: { + nationalId: nationalIdInput, + }, + }, + })🧰 Tools
🪛 Biome (1.9.4)
[error] 154-163: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.(lint/complexity/noUselessLoneBlockStatements)
Line range hint
201-205: Handle loading state when both searches are activeWhen both
searchPersonsandsearchCompaniesare true, theloadingprop may not accurately reflect both query loading states.Consider updating the
loadingprop to account for both conditions:- loading={searchPersons ? queryLoading : companyQueryLoading} + loading={ + (searchPersons && queryLoading) || + (searchCompanies && companyQueryLoading) + }🧰 Tools
🪛 Biome (1.9.4)
[error] 198-199: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (5)
libs/application/core/src/lib/fieldBuilders.ts(2 hunks)libs/application/types/src/lib/Fields.ts(1 hunks)libs/application/ui-components/src/components/NationalIdWithName/NationalIdWithName.tsx(8 hunks)libs/application/ui-components/src/components/NationalIdWithName/graphql/queries.ts(1 hunks)libs/application/ui-fields/src/lib/NationalIdWithNameFormField/NationalIdWithNameFormField.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
libs/application/ui-components/src/components/NationalIdWithName/graphql/queries.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/types/src/lib/Fields.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/ui-fields/src/lib/NationalIdWithNameFormField/NationalIdWithNameFormField.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/core/src/lib/fieldBuilders.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/ui-components/src/components/NationalIdWithName/NationalIdWithName.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🪛 Biome (1.9.4)
libs/application/ui-components/src/components/NationalIdWithName/NationalIdWithName.tsx
[error] 133-134: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 143-152: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
[error] 154-163: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
🪛 eslint
libs/application/ui-components/src/components/NationalIdWithName/NationalIdWithName.tsx
[error] 180-180: React Hook useEffect has a missing dependency: 'application.answers'. Either include it or remove the dependency array.
(react-hooks/exhaustive-deps)
🔇 Additional comments (5)
libs/application/ui-components/src/components/NationalIdWithName/graphql/queries.ts (1)
10-16: LGTM
The COMPANY_IDENTITY_QUERY is correctly defined and extends the functionality to fetch company names.
libs/application/ui-fields/src/lib/NationalIdWithNameFormField/NationalIdWithNameFormField.tsx (2)
1-14: Enhance component with localization and layout
The added imports and use of Box, Text, FieldDescription, and useLocale effectively improve the component's layout and localization support.
26-63: LGTM
The integration of new props and conditional rendering for title and description enhances the component's flexibility.
libs/application/core/src/lib/fieldBuilders.ts (1)
Line range hint 751-778: Integrate new properties into field builder
The additional properties searchPersons, searchCompanies, titleVariant, description, marginTop, and marginBottom are correctly integrated into the buildNationalIdWithNameField function.
libs/application/types/src/lib/Fields.ts (1)
573-577: Extend NationalIdWithNameField interface with new options
The new optional properties enhance the NationalIdWithNameField interface, providing greater configurability.
libs/application/ui-components/src/components/NationalIdWithName/NationalIdWithName.tsx
Outdated
Show resolved
Hide resolved
libs/application/ui-components/src/components/NationalIdWithName/NationalIdWithName.tsx
Show resolved
Hide resolved
jonnigs
added
the
deprecated:automerge
jonnigs
added
deprecated:automerge
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
libs/application/ui-components/src/components/NationalIdWithName/NationalIdWithName.tsx (2)
119-138: Simplify query execution blocksThe query execution logic can be simplified by removing unnecessary block statements and using optional chaining.
Consider this cleaner implementation:
- { - searchPersons && - getIdentity({ - variables: { - input: { - nationalId: nationalIdInput, - }, - }, - }) - } - { - searchCompanies && - getCompanyIdentity({ - variables: { - input: { - nationalId: nationalIdInput, - }, - }, - }) - } + searchPersons && getIdentity({ + variables: { + input: { + nationalId: nationalIdInput, + }, + }, + }); + + searchCompanies && getCompanyIdentity({ + variables: { + input: { + nationalId: nationalIdInput, + }, + }, + });Also, simplify the callback:
- onNameChange && - onNameChange(companyData.companyRegistryCompany?.name ?? '') + onNameChange?.(companyData.companyRegistryCompany?.name ?? '')Also applies to: 143-163
🧰 Tools
🪛 Biome (1.9.4)
[error] 133-134: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
173-180: Optimize name synchronization logicThe name synchronization effect could be simplified to reduce complexity.
Consider this more concise implementation:
useEffect(() => { const nameInAnswers = getValueViaPath(application.answers, nameField) - if (personName && nameInAnswers !== personName) { - setValue(nameField, personName) - } else if (companyName && nameInAnswers !== companyName) { - setValue(nameField, companyName) - } + const newName = personName || companyName + if (newName && nameInAnswers !== newName) { + setValue(nameField, newName) + } }, [personName, companyName, setValue, nameField, application.answers])
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
libs/application/ui-components/src/components/NationalIdWithName/NationalIdWithName.tsx(8 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
libs/application/ui-components/src/components/NationalIdWithName/NationalIdWithName.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🪛 Biome (1.9.4)
libs/application/ui-components/src/components/NationalIdWithName/NationalIdWithName.tsx
[error] 133-134: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 143-152: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
[error] 154-163: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
🔇 Additional comments (4)
libs/application/ui-components/src/components/NationalIdWithName/NationalIdWithName.tsx (4)
11-15: LGTM: Type-safe interface extension
The new imports and interface properties properly support the addition of company lookup functionality while maintaining type safety.
Also applies to: 36-37
56-57: LGTM: Clean state separation
Good practice using separate state variables for person and company names, with sensible defaults that maintain backward compatibility.
Also applies to: 69-70
217-233: Refactor error handling for combined searches
The error handling logic remains complex and could be simplified for better maintainability.
Line range hint 1-243: LGTM: Component meets library guidelines
The component successfully:
- Maintains reusability across NextJS apps
- Uses TypeScript appropriately
- Follows proper bundling practices
🧰 Tools
🪛 Biome (1.9.4)
[error] 113-113: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 133-134: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 143-152: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
[error] 154-163: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
What
Add in the options to fetch name by id for companies and vanished persons
Why
Expand shared components useability
Screenshot
Checklist:
Summary by CodeRabbit
New Features
NationalIdWithNameFieldto support searching for both persons and companies.Improvements
Documentation