feat(native-app): add third party error template and use in health overview #16860
Conversation
WalkthroughThis pull request introduces several enhancements to error handling and user feedback within the application. New error messages related to third-party service connectivity issues are added to the English and Icelandic translation files. 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 ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16860 +/- ##
==========================================
+ Coverage 36.42% 36.48% +0.06%
==========================================
Files 6852 6852
Lines 143504 143624 +120
Branches 40952 41007 +55
==========================================
+ Hits 52266 52405 +139
+ Misses 91238 91219 -19
Flags with carried forward coverage won't be shown. Click here to find out more. see 33 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 6 Total Test Services: 0 Failed, 6 Passed Test Services
🔻 Code Coverage Decreases vs Default Branch (1)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (13)
apps/native/app/src/utils/get-organization-slug-from-error.ts (2)
3-13: Consider enhancing type definitions for better type safetyThe type definitions could be improved in several ways:
- Make properties readonly to prevent accidental mutations
- Document possible values for the
codeproperty- Consider using a discriminated union if there are specific error codes
-type PartialProblem = { +/** Represents a subset of problem information related to organization errors */ +type PartialProblem = Readonly<{ organizationSlug?: string -} +}> -type CustomExtension = { +/** Extension type for Apollo GraphQL errors with organization context */ +type CustomExtension = Readonly<{ - code: string + code: string // TODO: Document possible values or use string literal union problem?: PartialProblem exception?: { problem?: PartialProblem } -} +}>
15-17: Enhance function documentationThe current JSDoc could be more descriptive about parameters, return value, and usage examples.
/** - * Extracts the organization slug from the Apollo error, if it exists. + * Extracts the organization slug from an Apollo GraphQL error. + * @param error - The error object from Apollo Client or unknown source + * @returns The organization slug if found in the error object, undefined otherwise + * @example + * const error = new ApolloError(...) + * const slug = getOrganizationSlugFromError(error) + * if (slug) { + * // Handle organization-specific error + * } */apps/native/app/src/ui/lib/problem/third-party-service-error.tsx (2)
7-10: Consider extracting and enhancing type definitionsThe type definitions could be improved for better reusability and clarity:
- Extract the 'size' type to a shared types file since it's likely used by other components
- Consider making the interface name more descriptive (e.g.,
Props→ThirdPartyServiceErrorProps)+type TemplateSize = 'small' | 'large' + type ThirdPartyServiceErrorProps = { organizationSlug: string - size: 'small' | 'large' + size: TemplateSize }
12-30: Overall implementation looks good with suggested improvementsThe component is well-structured and follows React/TypeScript best practices. The suggested improvements above will enhance error handling, accessibility, and maintainability, but the core implementation is solid.
Consider adding unit tests to verify:
- Error handling for organization name lookup
- Correct rendering of both states (with/without organization name)
- Proper propagation of size prop
apps/native/app/src/ui/lib/problem/problem.tsx (2)
67-67: Remove redundant size default in fallbackProps.The size prop is already defaulted in the component props. The fallback assignment is unnecessary.
- size: size ?? 'large', + size,Also applies to: 80-80
108-119: Refactor error handling logic for better readability.The nested if statements can be simplified using early returns or optional chaining.
- if (error) { - const organizationSlug = getOrganizationSlugFromError(error) - - if (organizationSlug) { - return ( - <ThirdPartyServiceError - organizationSlug={organizationSlug} - size={size} - /> - ) - } - } + const organizationSlug = error && getOrganizationSlugFromError(error) + if (organizationSlug) { + return ( + <ThirdPartyServiceError + organizationSlug={organizationSlug} + size={size} + /> + ) + }apps/native/app/src/stores/organizations-store.ts (1)
76-79: Consider enhancing robustness and performance.While the implementation is functional, consider these improvements:
- Add input validation for the slug parameter
- Consider memoization for frequently accessed organizations
- Consider a more explicit error handling approach instead of an empty string
Here's a suggested implementation:
- getOrganizationNameBySlug(slug: string) { - const org = get().organizations.find((o) => o.slug === slug) - return org?.title ?? '' - }, + getOrganizationNameBySlug(slug: string) { + if (!slug?.trim()) { + return ''; + } + + // Memoize results + if (!this.orgNameCache) { + this.orgNameCache = new Map(); + } + + const cachedName = this.orgNameCache.get(slug); + if (cachedName !== undefined) { + return cachedName; + } + + const org = get().organizations.find((o) => o.slug === slug); + const name = org?.title ?? ''; + this.orgNameCache.set(slug, name); + + return name; + },apps/native/app/src/ui/lib/problem/problem-template.tsx (2)
120-120: Consider performance and DRY improvements.The implementation is good but could be enhanced:
- Move repeated
textAlign="center"to a styled component- Memoize the component to prevent unnecessary re-renders
Apply these improvements:
+const StyledTypography = styled(Typography)` + text-align: center; +`; -export const ProblemTemplate = ({ +export const ProblemTemplate = React.memo(({ variant, title, message, showIcon, tag, withContainer, size = 'large', }: ProblemTemplateProps) => { // ... existing code ... return ( <Host borderColor={borderColor} noContainer={withContainer} size={size}> {/* ... existing code ... */} <Content> - <Typography - variant={size === 'small' ? 'heading5' : 'heading3'} - textAlign="center" - > + <StyledTypography + variant={size === 'small' ? 'heading5' : 'heading3'} + > {title} - </Typography> + </StyledTypography> - <Typography - variant={size === 'small' ? 'body3' : 'body'} - textAlign="center" - > + <StyledTypography + variant={size === 'small' ? 'body3' : 'body'} + > {message} - </Typography> + </StyledTypography> </Content> </Host> ) -} +})Also applies to: 126-126, 136-147
Line range hint
8-24: Add JSDoc documentation for better maintainability.Consider adding JSDoc comments to document the component props and their purpose, especially the new
sizeprop and its impact on the component's appearance.Example documentation:
/** * Base props for the ProblemTemplate component * @property {Variant} variant - The type of problem to display * @property {string} title - The main heading of the problem * @property {string | ReactNode} message - The detailed message or content * @property {boolean} [withContainer] - Whether to show the container * @property {'small' | 'large'} [size='large'] - Controls the visual size of the component */ export type ProblemTemplateBaseProps = { // ... existing props }apps/native/app/src/screens/health/health-overview.tsx (3)
263-302: Consider simplifying the value prop logicWhile the conditional rendering logic is good, the nested ternary operator in the value prop could be simplified for better readability.
-value={ - healthCenterRes.data - ?.rightsPortalHealthCenterRegistrationHistory?.current - ?.healthCenterName ?? - intl.formatMessage({ - id: 'health.overview.noHealthCenterRegistered', - }) -} +value={healthCenterRes.data + ?.rightsPortalHealthCenterRegistrationHistory?.current + ?.healthCenterName || intl.formatMessage({ + id: 'health.overview.noHealthCenterRegistered', + })}
Line range hint
312-347: Simplify complex conditional renderingThe current condition
(healthInsuranceRes.data && healthInsuranceData?.isInsured) || healthInsuranceRes.loadingcould be simplified for better readability.-{(healthInsuranceRes.data && healthInsuranceData?.isInsured) || -healthInsuranceRes.loading ? ( +{(healthInsuranceData?.isInsured || healthInsuranceRes.loading) && (
457-463: Extract date formatting logic to a utility functionConsider extracting the date formatting logic to improve code maintainability and reusability.
+const formatDateRange = (dateFrom: string, dateTo: string, intl: IntlShape) => { + if (!dateFrom || !dateTo) return ''; + return `${intl.formatDate(dateFrom)} - ${intl.formatDate(dateTo)}`; +}; -value={ - medicinePurchaseData?.dateFrom && - medicinePurchaseData?.dateTo - ? `${intl.formatDate( - medicinePurchaseData.dateFrom, - )} - ${intl.formatDate(medicinePurchaseData.dateTo)}` - : '' -} +value={formatDateRange( + medicinePurchaseData?.dateFrom, + medicinePurchaseData?.dateTo, + intl +)}apps/native/app/src/messages/en.ts (1)
597-599: LGTM! Consider making the error message more specific.The new error messages follow the established pattern and maintain consistency with other error messages in the file. However, to improve user experience, consider making the message more specific by mentioning that this is a third-party service issue.
'problem.thirdParty.title': 'No connection', 'problem.thirdParty.message': - 'An error occurred while communicating with the service provider', + 'An error occurred while communicating with the third-party service provider',
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (8)
apps/native/app/src/messages/en.ts(1 hunks)apps/native/app/src/messages/is.ts(1 hunks)apps/native/app/src/screens/health/health-overview.tsx(7 hunks)apps/native/app/src/stores/organizations-store.ts(2 hunks)apps/native/app/src/ui/lib/problem/problem-template.tsx(5 hunks)apps/native/app/src/ui/lib/problem/problem.tsx(7 hunks)apps/native/app/src/ui/lib/problem/third-party-service-error.tsx(1 hunks)apps/native/app/src/utils/get-organization-slug-from-error.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
apps/native/app/src/messages/en.ts (1)
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."
apps/native/app/src/messages/is.ts (1)
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."
apps/native/app/src/screens/health/health-overview.tsx (1)
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."
apps/native/app/src/stores/organizations-store.ts (1)
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."
apps/native/app/src/ui/lib/problem/problem-template.tsx (1)
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."
apps/native/app/src/ui/lib/problem/problem.tsx (1)
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."
apps/native/app/src/ui/lib/problem/third-party-service-error.tsx (1)
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."
apps/native/app/src/utils/get-organization-slug-from-error.ts (1)
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 (13)
apps/native/app/src/utils/get-organization-slug-from-error.ts (1)
18-36: Verify error structure against API responses
Let's ensure the error structure matches the actual GraphQL API responses.
✅ Verification successful
Let me gather more information about the error handling patterns.
Error structure implementation is consistent with codebase patterns
The implementation correctly handles GraphQL error structures as evidenced by:
- Matching error handling patterns in
libs/shared/problem/src/utils/findProblemInApolloError.ts - Consistent implementation across the codebase (identical structure in
libs/react-spa/shared/src/utils/getOrganizationSlugFromError.ts) - Proper interface definition in
libs/clients/smart-solutions-v2/src/lib/types/graphqlFetchResponses.type.ts
The error structure follows the established pattern:
- Primary check for
problem.organizationSlug - Fallback to
exception.problem.organizationSlug
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for GraphQL error handling patterns in the codebase
echo "Searching for GraphQL error handling patterns..."
rg -t typescript -t javascript "graphQLErrors.*extensions" -B 2 -A 2
# Search for organization-related GraphQL queries
echo "Searching for organization-related GraphQL queries..."
fd -e graphql -e gql | xargs rg "organization"
Length of output: 14185
Script:
#!/bin/bash
# Search for GraphQL error handling patterns with ast-grep
ast-grep --pattern 'interface $_ {
$$$
extensions: $_
$$$
}'
# Search for error handling files and their content
fd -e ts -e js error | xargs rg "extensions.*problem" -B 2 -A 2
Length of output: 2836
apps/native/app/src/ui/lib/problem/third-party-service-error.tsx (1)
25-26: Verify translation key existence
Ensure that the translation keys 'problem.thirdParty.title' and 'problem.thirdParty.message' exist in all language files.
✅ Verification successful
Translation keys are properly defined and available
The translation keys 'problem.thirdParty.title' and 'problem.thirdParty.message' are correctly defined in both language files:
- English (en.ts)
- Icelandic (is.ts)
The translations are properly set up and available for use in the component.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for the existence of required translation keys
# Search for translation keys in language files
echo "Checking for translation keys in language files..."
rg -l "problem\.thirdParty\.(title|message)" --type yaml
Length of output: 169
Script:
#!/bin/bash
# Let's try to find translation files first
echo "Looking for translation/locale files..."
fd -e yaml -e json "translations\|locale\|i18n\|lang"
# Then let's check content of these files if found
echo -e "\nChecking content of translation files for the specific keys..."
rg "problem:" -A 5 --type yaml
Length of output: 316
Script:
#!/bin/bash
# Let's try a broader search for any yaml/json files
echo "Looking for all yaml/json files..."
fd -e yaml -e json
# Let's also check the imports in the file to see where translations might be defined
echo -e "\nChecking imports in the file..."
cat "apps/native/app/src/ui/lib/problem/third-party-service-error.tsx"
# Let's also search for any files with 'translations' in their path
echo -e "\nSearching for translation-related files..."
fd . --full-path ".*translation.*"
Length of output: 122887
Script:
#!/bin/bash
# Let's try to find translation files in a different way
echo "Looking for translation files..."
fd -t f -e json -e yaml "translations\|messages\|i18n\|locales"
# Let's also check the content of any found files for these specific keys
echo -e "\nChecking content of any found files for the translation keys..."
rg "problem\.thirdParty\." -A 2 -B 2
Length of output: 1930
apps/native/app/src/ui/lib/problem/problem.tsx (4)
2-7: LGTM! Clean type definitions and well-organized imports.
The type definitions are properly structured using TypeScript's Pick utility type, and the imports are well-organized.
Also applies to: 26-26
Line range hint 27-66: LGTM! Well-structured TypeScript interfaces.
The interfaces use TypeScript's discriminated union pattern effectively, providing good type safety and clear prop definitions.
98-98: LGTM! Consistent size prop propagation.
The size prop is correctly propagated to all ProblemTemplate instances.
Also applies to: 130-130
108-119: Verify organization slug extraction and handling.
The getOrganizationSlugFromError utility is being used to extract organization information from errors, but we should verify its implementation and error handling.
✅ Verification successful
Organization slug extraction implementation is robust and properly handles errors
The implementation of getOrganizationSlugFromError is well-structured and handles errors appropriately:
- Safely type-casts and accesses Apollo errors using optional chaining
- Properly extracts organization slug from both direct problem and exception paths
- Returns undefined when no valid organization slug is found
- Used consistently across both native and web applications
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the implementation of getOrganizationSlugFromError utility
# Check the utility implementation
ast-grep --pattern 'function getOrganizationSlugFromError($_) {
$$$
}'
# Check for error handling in the utility usage
rg -A 5 'getOrganizationSlugFromError'
Length of output: 7307
apps/native/app/src/stores/organizations-store.ts (2)
33-33: LGTM! Well-typed interface method.
The new method signature is properly typed and follows good naming conventions.
76-79: Verify error handling integration.
Since this method is part of the error handling flow for third-party services, let's verify its integration points.
✅ Verification successful
Based on the results, I can now generate the final response:
Integration verified - method is properly used in error handling component
The getOrganizationNameBySlug method is correctly integrated:
- Used in
ThirdPartyServiceErrorcomponent for displaying organization-specific error messages - Safely handles missing organizations by returning an empty string as fallback
- Properly imported and used via the organizations store hook
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usages of getOrganizationNameBySlug to ensure proper error handling
# Find direct usage of the method
echo "Direct usage of getOrganizationNameBySlug:"
rg "getOrganizationNameBySlug" -A 3
# Find related error handling components
echo "\nRelated error handling components:"
rg "ThirdPartyServiceError" -A 5
# Check for potential error state handling
echo "\nError state handling:"
ast-grep --pattern 'catch (error) {
$$$
getOrganizationNameBySlug($$$)
$$$
}'
Length of output: 6214
apps/native/app/src/ui/lib/problem/problem-template.tsx (1)
13-13: LGTM! Type definition follows best practices.
The size prop is properly typed as an optional union of literal types, providing good type safety and clear documentation of allowed values.
apps/native/app/src/screens/health/health-overview.tsx (3)
89-91: LGTM: Well-structured error handling utility
The utility function follows React best practices and promotes code reuse across multiple error states in the component.
303-305: LGTM: Proper error handling implementation
The error handling logic correctly shows the error component only when there's an error and no data available.
433-437: LGTM: Comprehensive error handling for payment data
The error handling correctly considers both payment overview and payment status queries, showing the error component only when both queries fail and no data is available.
apps/native/app/src/messages/is.ts (1)
598-599: LGTM! The translations are well-implemented.
The new third-party error messages follow the established patterns and naming conventions. The translations are appropriate for communicating third-party service connectivity issues to users in Icelandic.
…erview (#16860) * feat: add third party error template and use in health overview * fix: use arrow function instead of function --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
What
Modify Problem component in app to support Third Party Error, so we show the name of the organization that should provide the information that is erroring.
Using that new error component in health-overview section
Why
Error states for inputs are currently loading state which is not good - this is better. Will rewrite errors for inputs soon.
Screenshots / Gifs
Checklist:
Summary by CodeRabbit
Release Notes
New Features
ThirdPartyServiceErrorcomponent for better user feedback on service errors.Improvements
UI Enhancements