feat(my-pages): add back "other" for organ donation registration #17228
Conversation
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
warning [email protected]: This version is no longer supported. Please see https://eslint.org/version-support for other options. WalkthroughThis pull request introduces comprehensive changes to the organ donation feature across multiple files. The modifications focus on enhancing the donor status handling by adding new properties like 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 #17228 +/- ##
=======================================
Coverage 35.68% 35.68%
=======================================
Files 6922 6922
Lines 148524 148524
Branches 42419 42419
=======================================
Hits 53008 53008
Misses 95516 95516
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 ✅ 5 Total Test Services: 0 Failed, 5 Passed Test Services
🔻 Code Coverage Decreases vs Default Branch (1)
|
disaerna
added
the
deprecated:automerge
![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
🧹 Nitpick comments (12)
libs/portals/my-pages/health/src/screens/OrganDonation/components/NoAccess.tsx (1)
3-3: Define Props interface for 'NoAccess' componentTo improve readability and reusability, consider defining a Props interface for the
NoAccesscomponent.Apply this change:
+interface NoAccessProps { + text: string +} + -export const NoAccess = ({ text }: { text: string }) => { +export const NoAccess = ({ text }: NoAccessProps) => {libs/portals/my-pages/health/src/screens/OrganDonation/helpers/textMapper.ts (1)
4-13: Define a TypeScript interface for 'texts' parameterTo enhance code maintainability and clarity, consider defining a TypeScript interface for the
textsparameter.Apply this change:
+interface OrganTexts { + iAmOrganDonorWithExceptionsText: string + iAmOrganDonorText: string + iAmNotOrganDonorText: string + iAmOrganDonorWithExceptions: string + iAmOrganDonor: string + iAmNotOrganDonor: string +} + -export const getOrganText = ( - isDonor: boolean, - hasLimitations: boolean, - texts: { - iAmOrganDonorWithExceptionsText: string - iAmOrganDonorText: string - iAmNotOrganDonorText: string - iAmOrganDonorWithExceptions: string - iAmOrganDonor: string - iAmNotOrganDonor: string - }, - limitations: string[], -) => { +export const getOrganText = ( + isDonor: boolean, + hasLimitations: boolean, + texts: OrganTexts, + limitations: string[], +) => {libs/portals/my-pages/health/src/screens/OrganDonation/components/Limitations.tsx (3)
30-31: Consider adding validation for comment stateThe initial state setup looks good, but consider adding validation for the comment length and content.
const [checked, setChecked] = useState<Array<string>>(selected ?? []) -const [comment, setComment] = useState<string>(exceptionComment ?? '') +const [comment, setComment] = useState<string>(exceptionComment ?? '') +const [commentError, setCommentError] = useState<string | null>(null) + +const validateComment = (value: string) => { + if (value.length > 220) { + setCommentError('Comment exceeds maximum length') + return false + } + setCommentError(null) + return true +}
67-78: Enhance accessibility for the 'other' checkboxWhile the implementation is functional, consider adding aria-labels and description for better accessibility.
<Checkbox id={'other'} name={`selected-limitations-other`} label={formatMessage(messages.otherPascalCase)} value={comment} + aria-label={formatMessage(messages.otherPascalCase)} + aria-describedby="other-limitation-description" onChange={(e) => { setComment(e.target.value) handleCheckboxChange('other', e.target.checked) }} checked={checked.includes('other')} />
92-103: Consider adding error handling for the comment inputThe textarea implementation could benefit from error state display and aria-invalid attribute.
<Input id="organ-donation-limitation" name="otherLimitatons" textarea label={formatMessage(messages.organRegistrationOtherLabel)} placeholder={formatMessage( messages.organRegistrationOtherText, )} maxLength={220} + error={commentError} + aria-invalid={!!commentError} onChange={(e) => setComment(e.target.value)} value={comment} />libs/portals/my-pages/health/src/screens/OrganDonation/OrganDonation.tsx (2)
32-37: Consider extracting limitations logic to a helper functionThe limitations array construction logic could be moved to a separate helper function for better maintainability.
+const getLimitationsArray = ( + limitations?: { limitedOrgansList?: { name: string }[] | null, comment?: string | null } +): string[] => { + const result = limitations?.limitedOrgansList?.map((item) => item.name) ?? [] + if (limitations?.comment?.length) { + result.push(limitations.comment) + } + return result +} + -const allLimitations: Array<string> = - donorStatus?.limitations?.limitedOrgansList?.map((item) => item.name) ?? [] - -if (comment !== undefined && comment !== null && comment.length > 0) { - allLimitations.push(comment) -} +const allLimitations = getLimitationsArray(donorStatus?.limitations)
75-96: Consider extracting the ActionCard rendering to a separate componentThe conditional rendering block for ActionCard is quite large and could be moved to a separate component for better maintainability.
+const DonorActionCard = ({ texts, loading }: { texts: ReturnType<typeof getOrganText>, loading: boolean }) => ( + <Box> + <Text variant="eyebrow" color="purple400" marginBottom={1}> + {formatMessage(m.takeOnOrganDonation)} + </Text> + <ActionCard + heading={texts.heading} + text={texts.cardText} + cta={{ + url: HealthPaths.HealthOrganDonationRegistration, + label: formatMessage(m.changeTake), + centered: true, + variant: 'text', + }} + loading={loading} + /> + </Box> +)libs/api/domains/health-directorate/src/lib/health-directorate.service.ts (3)
31-34: Improve variable naming for clarity.The boolean variables could be more descriptive to better convey their purpose.
-const hasExceptionComment: boolean = - data.exceptionComment !== undefined && data.exceptionComment.length > 0 -const hasExceptions: boolean = - data.exceptions !== undefined && data.exceptions.length > 0 +const hasValidExceptionComment: boolean = + data.exceptionComment !== undefined && data.exceptionComment.length > 0 +const hasValidExceptions: boolean = + data.exceptions !== undefined && data.exceptions.length > 0
39-41: Simplify boolean expression.The nullish coalescing operator is unnecessary since the expression already evaluates to a boolean.
-hasLimitations: - ((hasExceptionComment || hasExceptions) && data.isDonor) ?? false, +hasLimitations: (hasExceptionComment || hasExceptions) && data.isDonor,
71-72: Consider adding type safety for the filter operation.The optional chaining and nullish coalescing are good, but type safety could be improved.
-const filteredList = - input.organLimitations?.filter((item) => item !== 'other') ?? [] +const filteredList = + input.organLimitations?.filter((item: string) => item !== 'other') ?? []libs/clients/health-directorate/src/lib/clients/organ-donation/clientConfig.json (1)
71-82: Consider marking IP parameter as deprecated.The IP parameter might be redundant as it can be obtained from the request context.
{ "name": "ip", "required": false, "in": "query", + "deprecated": true, "description": "The IP address of the user", "schema": { "type": "string" } }libs/portals/my-pages/health/src/lib/messages.ts (1)
1039-1042: Consider using a consistent naming pattern.The message ID for
otherPascalCasecould follow the same pattern as other message IDs.- id: 'sp.health:other', + id: 'sp.health:other-pascal-case',
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
libs/api/domains/health-directorate/src/lib/health-directorate.service.ts(2 hunks)libs/api/domains/health-directorate/src/lib/models/organ-donation.model.ts(2 hunks)libs/clients/health-directorate/src/lib/clients/organ-donation/clientConfig.json(7 hunks)libs/portals/my-pages/health/src/lib/messages.ts(1 hunks)libs/portals/my-pages/health/src/module.tsx(1 hunks)libs/portals/my-pages/health/src/screens/OrganDonation/OrganDonation.css.ts(1 hunks)libs/portals/my-pages/health/src/screens/OrganDonation/OrganDonation.graphql(1 hunks)libs/portals/my-pages/health/src/screens/OrganDonation/OrganDonation.tsx(2 hunks)libs/portals/my-pages/health/src/screens/OrganDonation/components/Limitations.tsx(1 hunks)libs/portals/my-pages/health/src/screens/OrganDonation/components/Loader.tsx(1 hunks)libs/portals/my-pages/health/src/screens/OrganDonation/components/NoAccess.tsx(1 hunks)libs/portals/my-pages/health/src/screens/OrganDonation/components/RegistrationForm.tsx(5 hunks)libs/portals/my-pages/health/src/screens/OrganDonation/helpers/textMapper.ts(1 hunks)libs/portals/my-pages/health/src/screens/OrganDonationRegistration/Limitations.tsx(0 hunks)libs/portals/my-pages/health/src/screens/OrganDonationRegistration/OrganDonationRegistration.css.ts(0 hunks)
💤 Files with no reviewable changes (2)
- libs/portals/my-pages/health/src/screens/OrganDonationRegistration/Limitations.tsx
- libs/portals/my-pages/health/src/screens/OrganDonationRegistration/OrganDonationRegistration.css.ts
✅ Files skipped from review due to trivial changes (3)
- libs/portals/my-pages/health/src/screens/OrganDonation/components/Loader.tsx
- libs/portals/my-pages/health/src/module.tsx
- libs/portals/my-pages/health/src/screens/OrganDonation/OrganDonation.css.ts
🧰 Additional context used
📓 Path-based instructions (10)
libs/portals/my-pages/health/src/screens/OrganDonation/OrganDonation.graphql (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/portals/my-pages/health/src/screens/OrganDonation/components/NoAccess.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/portals/my-pages/health/src/lib/messages.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/api/domains/health-directorate/src/lib/health-directorate.service.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/api/domains/health-directorate/src/lib/models/organ-donation.model.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/portals/my-pages/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."
libs/portals/my-pages/health/src/screens/OrganDonation/helpers/textMapper.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/portals/my-pages/health/src/screens/OrganDonation/components/Limitations.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/clients/health-directorate/src/lib/clients/organ-donation/clientConfig.json (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/portals/my-pages/health/src/screens/OrganDonation/components/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."
🔇 Additional comments (11)
libs/portals/my-pages/health/src/screens/OrganDonation/components/RegistrationForm.tsx (1)
32-32: Component renamed to 'OrganRegistrationForm' enhances clarity
Renaming the component to OrganRegistrationForm improves readability and clarity of the codebase.
Also applies to: 214-214
libs/portals/my-pages/health/src/screens/OrganDonation/OrganDonation.graphql (1)
13-14: GraphQL query updated with new fields 'isMinor' and 'isTemporaryResident'
The addition of isMinor and isTemporaryResident fields to the donor object aligns the frontend with the updated API schema.
libs/api/domains/health-directorate/src/lib/models/organ-donation.model.ts (2)
37-41: LGTM: New donor eligibility flags properly defined
The addition of isMinor and isTemporaryResident flags with appropriate GraphQL Field decorators and default values enhances the donor validation model.
64-65: LGTM: Comment field addition supports 'other' option
The nullable comment field in DonorInput aligns with the PR objective of adding back the 'other' option for organ donation registration.
libs/portals/my-pages/health/src/screens/OrganDonation/components/Limitations.tsx (1)
17-21: LGTM: Props interface well defined
The interface clearly defines the component's contract with proper typing.
libs/portals/my-pages/health/src/screens/OrganDonation/OrganDonation.tsx (1)
27-28: LGTM: Clear status flags extraction
The extraction of isMinor and isTemporaryResident flags is clean and follows TypeScript best practices.
libs/api/domains/health-directorate/src/lib/health-directorate.service.ts (2)
43-44: LGTM! Proper null handling.
The nullish coalescing operator is correctly used to provide default values for the new properties.
78-79: LGTM! Clean and clear assignment.
The property assignments are straightforward and maintain consistency with the API contract.
libs/clients/health-directorate/src/lib/clients/organ-donation/clientConfig.json (2)
Line range hint 4-63: LGTM! Well-structured endpoint configuration.
The new endpoint /v1/donation-exceptions is properly configured with appropriate responses and error handling.
252-266: LGTM! Proper schema definition for new properties.
The OrganDonorDto schema is well-defined with appropriate types and required fields.
libs/portals/my-pages/health/src/lib/messages.ts (1)
1027-1034: LGTM! Clear and informative messages.
The new messages for minor and temporary national ID restrictions are well-written and informative.
libs/portals/my-pages/health/src/screens/OrganDonation/components/RegistrationForm.tsx
Outdated
Show resolved
Hide resolved
libs/portals/my-pages/health/src/screens/OrganDonation/components/RegistrationForm.tsx
Outdated
Show resolved
Hide resolved
disaerna
removed
the
deprecated:automerge
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
libs/portals/my-pages/health/src/screens/OrganDonation/components/RegistrationForm.tsx (1)
Line range hint
89-101: Add form data validationThe current form data handling lacks validation:
- Direct string conversion without type checking
- No validation of form values
- No error handling for invalid form data
Consider adding validation:
const formData = new FormData(e.currentTarget) const data = Object.fromEntries(formData.entries()) const idKey = 'selected-limitations-' - const otherLimitations = data['otherLimitatons'].toString() + const otherLimitations = data['otherLimitatons'] + if (typeof otherLimitations !== 'string') { + toast.error(formatMessage(messages.invalidFormData)) + return + } const limitations = Object.keys(data) .filter((key) => key.includes(idKey)) .map((key) => key.replace(idKey, '').toLowerCase())
🧹 Nitpick comments (1)
libs/portals/my-pages/health/src/screens/OrganDonation/components/RegistrationForm.tsx (1)
Line range hint
125-209: Enhance form accessibilityThe form controls could benefit from improved accessibility:
- Radio buttons lack aria-labels
- Form sections could use fieldset and legend
- Missing form validation feedback
Consider these improvements:
- <form onSubmit={onSubmit}> + <form onSubmit={onSubmit} aria-label={formatMessage(messages.organDonation)}> + <fieldset> + <legend className="visually-hidden"> + {formatMessage(messages.organDonationOptions)} + </legend> <Stack space={2}>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/portals/my-pages/health/src/screens/OrganDonation/components/RegistrationForm.tsx(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
libs/portals/my-pages/health/src/screens/OrganDonation/components/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."
🔇 Additional comments (2)
libs/portals/my-pages/health/src/screens/OrganDonation/components/RegistrationForm.tsx (2)
90-90: Fix typo 'otherLimitatons' in form data retrieval
The typo in otherLimitatons needs to be corrected to otherLimitations to match the type definition.
1-26: LGTM! Component name change improves clarity.
The renaming from Form2 to OrganRegistrationForm better reflects the component's purpose and follows TypeScript naming conventions.
Let's verify the component's reusability across NextJS apps as per the libs/** guidelines:
✅ Verification successful
Component renaming and imports are appropriate for its scope
The component is correctly scoped within the health portal's organ donation feature and is not intended for cross-application reuse. The renaming from Form2 to OrganRegistrationForm improves code clarity while maintaining proper encapsulation within its specific domain.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if this component is imported in other NextJS apps
rg -l "OrganRegistrationForm" --type ts --type tsx
Length of output: 78
Script:
#!/bin/bash
# Let's try a different approach to check component usage
rg -t typescript -t javascriptreact -t typescriptreact "OrganRegistrationForm"
# Also check the directory structure to understand the component's scope
fd -t f "RegistrationForm.tsx"
Length of output: 231
Script:
#!/bin/bash
# Let's try one more time with ripgrep without specific file type flags
rg "OrganRegistrationForm"
# Also check the imports structure to understand component usage
rg -l "from.*OrganDonation/components"
Length of output: 323
libs/portals/my-pages/health/src/screens/OrganDonation/components/RegistrationForm.tsx
Outdated
Show resolved
Hide resolved
disaerna
added
the
deprecated:automerge
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
libs/portals/my-pages/health/src/screens/OrganDonation/components/RegistrationForm.tsx (1)
90-90:⚠️ Potential issueFix the 'otherLimitatons' typo
The typo in the form field name persists and needs to be corrected to match the type definition.
Apply this diff:
- const otherLimitations = data['otherLimitatons'].toString() + const otherLimitations = data['otherLimitations'].toString()Also applies to: 100-100
🧹 Nitpick comments (2)
libs/portals/my-pages/health/src/screens/OrganDonation/components/RegistrationForm.tsx (2)
116-124: Consider extracting the access restriction conditionThe conditional logic for access restriction could be more readable by extracting it into a constant.
Consider this improvement:
+ const isAccessRestricted = isMinor || isTemporaryResident; + const accessRestrictionMessage = isMinor + ? formatMessage(messages.organMinor) + : formatMessage(messages.organTemporaryNationalId); - {!loading && (isMinor || isTemporaryResident) && ( + {!loading && isAccessRestricted && ( <NoAccess - text={ - isMinor - ? formatMessage(messages.organMinor) - : formatMessage(messages.organTemporaryNationalId) - } + text={accessRestrictionMessage} /> )}
166-167: Consider using more explicit TypeScript typesWhile the null coalescing operators provide runtime safety, consider adding explicit type annotations for better type checking during development.
- selected={updatedLimitations ?? []} - exceptionComment={exceptionComment ?? undefined} + selected={updatedLimitations as string[] ?? []} + exceptionComment={exceptionComment as string | undefined}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/portals/my-pages/health/src/screens/OrganDonation/components/RegistrationForm.tsx(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
libs/portals/my-pages/health/src/screens/OrganDonation/components/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."
🔇 Additional comments (2)
libs/portals/my-pages/health/src/screens/OrganDonation/components/RegistrationForm.tsx (2)
32-32: LGTM! Improved component naming
The rename from Form2 to OrganRegistrationForm better reflects the component's purpose and follows React naming conventions.
57-59: LGTM! Safe array manipulation implemented
The implementation uses immutable array operations with spread operator, addressing the previous review comment about type safety and array mutation.
…#17298) * fix: merge conflicts * fix(my-pages-assets): chart fixes (#17289) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(my-pages): add back "other" for organ donation registration (#17228) * feat: add back "other" for organ donation registration * refactor: update api client for organ donations * refactor: texts * fix: as * fix: type safety --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> Co-authored-by: Ásdís Erna Guðmundsdóttir <[email protected]>
My pages - Health - Organ donation
What
Adding back in "other" section for compatibility with current Heilsuvera
Adding logic to display a message when user is a minor and is a "kerfiskennitala"
Checklist:
Summary by CodeRabbit
New Features
isMinorandisTemporaryResident.NoAccesscomponent for users who do not meet donor criteria.Limitationscomponent for managing organ donation limitations.Bug Fixes
Chores