-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(native-app): add organ donation to app #16401
Conversation
WalkthroughThe changes in this pull request introduce a new GraphQL query named 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
apps/native/app/src/graphql/queries/health.graphql (1)
39-53
: LGTM! Well-structured query for organ donation status.The new
GetOrganDonorStatus
query is well-designed and aligns perfectly with the PR objectives. It provides comprehensive information about organ donation status, including limitations and specific organs. The use of the$locale
parameter is excellent for supporting multiple languages.For consistency with other queries in this file, consider adding an
Input
type for the locale parameter. This would make it easier to extend the query parameters in the future if needed. For example:query GetOrganDonorStatus($input: OrganDonorStatusInput!) { healthDirectorateOrganDonation(input: $input) { # ... (rest of the query remains the same) } }You would then need to define the
OrganDonorStatusInput
type in your schema.apps/native/app/src/screens/health/health-overview.tsx (1)
70-70
: Consider internationalizing the fallback text.The fallback text for the
linkTextId
prop is hardcoded as 'button.seeAll'. Consider moving this fallback text to the internationalization files to ensure consistency and maintainability of the fallback text across the codebase.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
- apps/native/app/src/graphql/queries/health.graphql (1 hunks)
- apps/native/app/src/messages/en.ts (2 hunks)
- apps/native/app/src/messages/is.ts (2 hunks)
- apps/native/app/src/screens/health/health-overview.tsx (9 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/native/app/src/graphql/queries/health.graphql (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/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."
🔇 Additional comments (10)
apps/native/app/src/messages/en.ts (1)
Line range hint
609-629
: LGTM: Health overview translations added successfully.The new translations for the health overview screen have been added correctly. The keys are well-structured, following the existing naming conventions, and the English translations are clear and comprehensive.
apps/native/app/src/messages/is.ts (2)
Line range hint
609-629
: Health overview translations look good!The new translations for the health overview section are well-structured and comprehensive. They cover all necessary aspects of the health information screen, including health center, physician, insurance status, and payment details. The Icelandic translations are consistent and appropriate for the context.
Line range hint
609-642
: Overall, the new health and organ donation translations are well-implemented.The additions to this file align well with the PR objectives of introducing organ donation features to the application. The new translations provide comprehensive coverage of both the health overview and organ donation information, maintaining consistency with the existing translation style.
A few key points:
- The health overview translations (lines 609-629) are complete and cover all necessary aspects.
- The organ donation translations (lines 630-642) effectively convey the different organ donation statuses and options.
- The translations are consistently in Icelandic and follow the established key structure.
These changes will effectively support the new organ donation feature in the Icelandic version of the application.
apps/native/app/src/screens/health/health-overview.tsx (7)
27-27
: Confirm the feature flag usage adheres to best practices.The
useFeatureFlag
hook is used to conditionally enable the organ donation feature. Ensure that the feature flag is consistently used throughout the component and that the flag name follows the naming conventions for feature flags in the codebase.
100-106
: Verify the feature flag and query skip logic.The
isOrganDonationEnabled
feature flag is used to conditionally skip theuseGetOrganDonorStatusQuery
query. Ensure that this logic is correct and that the query is skipped when the feature flag is disabled.
122-134
: Confirm the organ donation status logic.The
isOrganDonor
andisOrganDonorWithLimitations
variables are derived from theuseGetOrganDonorStatusQuery
query result. Verify that the logic for determining the organ donor status and limitations is correct and handles all possible cases, including when the query result is undefined or the relevant fields are missing.
Line range hint
156-170
: Verify theonRefresh
function update.The
onRefresh
function has been updated to conditionally refetch theuseGetOrganDonorStatusQuery
query based on theisOrganDonationEnabled
feature flag. Ensure that this update is correct and that the query is refetched only when the feature flag is enabled.
407-447
: Confirm the organ donation UI integration.The organ donation information is conditionally rendered based on the
isOrganDonationEnabled
feature flag. Verify that the UI integration is correct and that the appropriate messages are displayed based on the user's organ donor status and limitations.
43-51
: Verify the impact of thelinkTextId
prop addition.The
HeadingSectionProps
interface and theHeadingSection
component have been updated to include an optionallinkTextId
prop. Confirm that this change does not introduce any breaking changes or inconsistencies in the usage of theHeadingSection
component throughout the codebase.Run the following script to verify the
HeadingSection
component usage:#!/bin/bash # Description: Verify the usage of the `HeadingSection` component in the codebase. # Test: Search for the component usage. Expect: No breaking changes or inconsistencies. rg --type typescript $'HeadingSection'
18-18
: Verify the usage of the new query in the codebase.The new
useGetOrganDonorStatusQuery
query is introduced. Ensure that it is being used correctly and consistently throughout the codebase, especially in any other components or screens related to organ donation.Run the following script to verify the query usage:
✅ Verification successful
Usage of
useGetOrganDonorStatusQuery
is correctly confined tohealth-overview.tsx
.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `useGetOrganDonorStatusQuery` in the codebase. # Test: Search for the query usage. Expect: Only occurrences in the `health-overview.tsx` file. rg --type typescript $'useGetOrganDonorStatusQuery'Length of output: 166
Script:
#!/bin/bash # Description: Verify the usage of `useGetOrganDonorStatusQuery` in the codebase. # Test: Search for the query usage in TypeScript and TSX files. Expect: Only occurrences in the `health-overview.tsx` file. rg --type 'ts' --type 'tsx' 'useGetOrganDonorStatusQuery'Length of output: 81
Script:
#!/bin/bash # Description: Verify the usage of `useGetOrganDonorStatusQuery` in the codebase. # Test: Search for the query usage in .ts and .tsx files. Expect: Only occurrences in the `health-overview.tsx` file. rg --glob '*.ts' --glob '*.tsx' 'useGetOrganDonorStatusQuery'Length of output: 259
d654f23
to
eb70b79
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
apps/native/app/src/screens/health/health-overview.tsx (2)
101-110
: LGTM with suggestions: Organ donation query implementation.The implementation of the organ donation query with a feature flag is well done and allows for controlled feature rollout. The use of the
useLocale
hook ensures proper localization.Suggestions for improvement:
- Consider adding error handling for the organ donation query.
- Implement a loading state for the organ donation information to improve user experience.
Example:
const { data, loading, error } = useGetOrganDonorStatusQuery({ variables: { locale: useLocale() }, skip: !isOrganDonationEnabled, }); // Later in the component if (loading) { // Show loading state } if (error) { // Show error state or log error }
411-454
: LGTM with accessibility suggestions: Organ donation UI implementation.The UI implementation for organ donation follows the existing pattern in the component and uses conditional rendering appropriately. Good job on integrating the new feature seamlessly into the existing UI.
Suggestions for improvement:
- Add appropriate ARIA labels to improve accessibility. For example:
<Input label={intl.formatMessage({ id: isOrganDonorWithLimitations ? 'health.organDonation.isDonorWithLimitations' : isOrganDonor ? 'health.organDonation.isDonor' : 'health.organDonation.isNotDonor', })} value={`${intl.formatMessage( // ... existing code ... )}`} // Add this line: aria-label={intl.formatMessage({ id: 'health.organDonation.status' })} // ... rest of the props ... />
- Consider adding a brief explanation or tooltip for users who might not understand what organ donation status means or how to interpret the information displayed.
These changes will enhance the accessibility and user-friendliness of the new feature.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
- apps/native/app/src/graphql/queries/health.graphql (1 hunks)
- apps/native/app/src/messages/en.ts (2 hunks)
- apps/native/app/src/messages/is.ts (2 hunks)
- apps/native/app/src/screens/health/health-overview.tsx (9 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/native/app/src/graphql/queries/health.graphql
- apps/native/app/src/messages/is.ts
🧰 Additional context used
📓 Path-based instructions (2)
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/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."
🔇 Additional comments (4)
apps/native/app/src/screens/health/health-overview.tsx (2)
18-18
: LGTM: New imports for organ donation feature.The new imports for
useGetOrganDonorStatusQuery
,useLocale
, anduseFeatureFlag
are appropriate for implementing the organ donation feature. They align with the PR objectives and follow NextJS best practices for importing necessary hooks and queries.Also applies to: 27-28
44-44
: LGTM: HeadingSection component update.The changes to the
HeadingSection
component, including the addition of the optionallinkTextId
prop, enhance its flexibility. This update allows for customized link text, which is particularly useful for the organ donation feature. The implementation follows TypeScript best practices and maintains type safety.Also applies to: 48-52, 71-71
apps/native/app/src/messages/en.ts (2)
Line range hint
612-632
: LGTM: Health overview translations are comprehensive and well-structured.The added translations for the health overview section are comprehensive, covering various aspects of health information. The keys follow a consistent naming convention, and the content is appropriate for a health overview screen.
633-645
: Translate Icelandic text to English for consistency.The organ donation translations contain Icelandic text instead of English. This is inconsistent with the purpose of the
en.ts
file, which should contain English translations.Please translate the Icelandic text to English as suggested in the previous review comment. This will ensure consistency throughout the English translation file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work 💯
0c168b4
to
0c1ecce
Compare
0fbb433
to
068efda
Compare
c6a6194
to
b32e6ab
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16401 +/- ##
=======================================
Coverage 36.44% 36.44%
=======================================
Files 6853 6853
Lines 143622 143622
Branches 41000 41000
=======================================
Hits 52347 52347
Misses 91275 91275
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 ReportBranch report: ✅ 0 Failed, 4 Passed, 0 Skipped, 2.92s Total Time |
* feat: add organ donation to app * feat: better usage of limitations string * fix: send locale into organ donor query * fix: improve type safety and null checks * fix: use correct res for organ donation * fix: create organDonationData * feat: use error component for organ donation as well * fix: update english translation for third party error --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
What
Add organ donation to app. Hidden behind a feature flag for now.
Screenshots / Gifs
The three states organ donation can be in:
TODO before merge
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes