-
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
fix(my-pages): add locale param to oragn donation query #16465
Conversation
WalkthroughThe pull request modifies the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 0
🧹 Outside diff range and nitpick comments (2)
libs/service-portal/health/src/screens/OrganDonation/OrganDonation.tsx (2)
20-22
: LGTM: Locale parameter added to queryThe inclusion of the
locale
variable in the query parameters successfully implements the PR's main objective. This change enhances the query's ability to handle different locales, improving the overall functionality of the organ donation feature.As a minor optimization, consider using object shorthand notation:
variables: { locale },This makes the code slightly more concise while maintaining readability.
32-32
: LGTM: Improved string formatting consistencyThe modification to always append a period to the
cardText
string ensures consistent formatting. This change improves the quality of the displayed text by guaranteeing that all sentences end with a period.For added clarity, consider extracting this string construction logic into a separate function:
const getCardText = (donorStatus) => { if (!donorStatus?.isDonor) { return formatMessage(m.iAmNotOrganDonorText); } if (donorStatus.limitations?.hasLimitations) { return `${formatMessage(m.iAmOrganDonorWithExceptionsText)} ${donorStatus.limitations.limitedOrgansList?.map((organ) => organ.name).join(', ')}.`; } return formatMessage(m.iAmOrganDonorText); }; // Usage const cardText = getCardText(donorStatus);This refactoring would improve readability and make the logic easier to test and maintain.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- libs/service-portal/health/src/screens/OrganDonation/OrganDonation.tsx (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
libs/service-portal/health/src/screens/OrganDonation/OrganDonation.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."
🔇 Additional comments (1)
libs/service-portal/health/src/screens/OrganDonation/OrganDonation.tsx (1)
17-17
: LGTM: Locale extraction aligns with PR objectiveThe addition of
locale
to the destructured values fromuseLocale
is in line with the PR's goal of adding a locale parameter to the organ donation query. This change enhances the component's ability to handle different locales, improving its reusability across the application.
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.
LGTM
Datadog ReportAll test runs ✅ 101 Total Test Services: 0 Failed, 98 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (1)
|
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: 0
🧹 Outside diff range and nitpick comments (2)
libs/service-portal/health/src/screens/OrganDonationRegistration/RegistrationForm.tsx (2)
40-40
: Approved: Addition offetchPolicy
improves data freshness.The addition of
fetchPolicy: 'no-cache'
ensures that the organ donation list is always up-to-date, which is crucial for medical information. This change aligns with best practices for data integrity in sensitive applications.Consider making the
fetchPolicy
configurable:const { data, loading } = useGetOrgansListQuery({ variables: { locale: lang }, fetchPolicy: process.env.NEXT_PUBLIC_ORGAN_DONATION_FETCH_POLICY || 'no-cache', });This approach allows for easy adjustment of the fetch policy based on the environment, enhancing the component's flexibility across different NextJS apps.
Line range hint
1-208
: Approved: Well-structured and reusable component implementation.The
Form2
component is well-implemented, adhering to the coding guidelines forlibs/**/*
files. It demonstrates good practices in terms of reusability, TypeScript usage, and effective tree-shaking. The component structure and logic are clear and maintainable.To further improve code organization, consider extracting the form submission logic into a custom hook:
const useOrganDonationForm = (lang: string) => { const [updateDonorStatus] = useUpdateOrganDonationInfoMutation({ onCompleted: () => { toast.success(formatMessage(messages.registrationComplete)) navigate(HealthPaths.HealthOrganDonation, { replace: true }) }, onError: () => { toast.error(formatMessage(messages.registrationFailed)) }, }); const onSubmit = async (e: React.FormEvent<HTMLFormElement>) => { e.preventDefault() // ... (rest of the submission logic) } return { onSubmit }; };This refactoring would enhance the component's readability and make the form submission logic more reusable across different components if needed.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- libs/service-portal/health/src/screens/OrganDonation/OrganDonation.tsx (2 hunks)
- libs/service-portal/health/src/screens/OrganDonationRegistration/RegistrationForm.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/service-portal/health/src/screens/OrganDonation/OrganDonation.tsx
🧰 Additional context used
📓 Path-based instructions (1)
libs/service-portal/health/src/screens/OrganDonationRegistration/RegistrationForm.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."
My pages - Health - Organ donation
What
Add missing locale parameter to organ donation query
Checklist:
Summary by CodeRabbit