Edit feature for facilities in organization | pincode, geo_organization info auto populates #9662
Conversation
WalkthroughThis pull request introduces comprehensive changes to the facility creation and management workflow. The modifications primarily focus on enhancing the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Finishing Touches
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
CodeRabbit Configuration File (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
On facility edit the organisation is also not getting prefilled, I think we should solve that also |
You mean |
|
I don't think jeevan is working on it, I will assign it to you. lets have one PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/pages/Organization/components/OrganizationSelector.tsx (1)
35-55: Enhance defensive checks around JSON parsing.
Currently, atry/catchblock is in place to catchJSON.parseerrors, logging them to the console. Consider adding a fallback path or early return to ensure the component’s state remains stable in edge cases (e.g., an empty string or invalid structure) to avoid undesired behavior.useEffect(() => { if (value) { try { const parsedValue = typeof value === "string" ? JSON.parse(value) : value; if (parsedValue) { // ... } } catch (e) { + setSelectedLevels([]); console.error("Invalid value for geo_organization:", e); } } }, [value]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
public/locale/en.json(1 hunks)src/components/Facility/FacilityCreate.tsx(4 hunks)src/pages/Organization/components/OrganizationSelector.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Facility/FacilityCreate.tsx
🔇 Additional comments (4)
src/pages/Organization/components/OrganizationSelector.tsx (3)
2-2: Import statement is correctly updated.
No issues found with the import of React hooks.
31-31: Prop destructuring for newvalueis fine.
Thevalueprop is properly introduced here, and the necessary props are being destructured clearly.
171-171: No issues with the Autocomplete value assignment.
Using the last selected organization’sidor an empty string is a sensible approach.public/locale/en.json (1)
1584-1584: New i18n key added.
The"select_location_from": "Select location from"entry aligns with the updated UI text references. Ensure that its usage is consistent across the app, and consider whether this prompt needs additional context or placeholders.
Mahendar0701
changed the title
|
the issue still presist, can you attach a video after fixing the issue |
Mahendar0701
changed the title
|
edit may work fine, but is it auto-filling when pincode is entered? |
Nope , i will fix that too 👍 |
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
src/components/Facility/FacilityCreate.tsx (1)
Line range hint
523-534: Internationalize visibility settings strings.The visibility settings section contains hardcoded English strings. These should be internationalized for consistency with the rest of the application.
<FormLabel className="text-base"> - Make this facility public + {t("make_facility_public")} </FormLabel> <p className="text-sm text-muted-foreground"> - When enabled, this facility will be visible to the - public and can be discovered by anyone using the - platform + {t("facility_public_description")} </p>
🧹 Nitpick comments (5)
src/components/Facility/FacilityForm.tsx (2)
58-58: Improve the validation error message for 'geo_organization'The error message for the
geo_organizationfield is currently set to "required", which might not be informative for users. Consider providing a more descriptive message to enhance user understanding.Apply this diff to improve the error message:
- geo_organization: z.string().min(1, { message: "required" }), + geo_organization: z.string().min(1, { message: "Geo organization is required" }),
396-405: Synchronize 'geo_organization' value with form stateThe
valueprop ofOrganizationSelectoris set tofacilityData?.geo_organization, which may not update if the user changes the field. To keep the component in sync with the form state, consider usingform.watch("geo_organization").Apply this diff to synchronize the value:
- value={facilityData?.geo_organization} + value={form.watch("geo_organization")}src/pages/Organization/components/AddFacilitySheet.tsx (1)
16-16: Align component naming with file name for consistencyThe component is named
CreateFacilityFormbut is imported from"FacilityForm". For clarity and consistency, consider renaming the component or updating the import statement to match the file name.If the component is intended to be
FacilityForm, update the export and import:In
FacilityForm.tsx:- export default function CreateFacilityForm(props: FacilityProps) { + export default function FacilityForm(props: FacilityProps) {In
AddFacilitySheet.tsx:- import CreateFacilityForm from "@/components/Facility/FacilityForm"; + import FacilityForm from "@/components/Facility/FacilityForm";Alternatively, if the component name is correct, consider renaming the file to
CreateFacilityForm.tsx.src/components/Facility/FacilityCreate.tsx (2)
Line range hint
91-106: Simplify phone number validation logic.The current validation combines multiple checks in a way that might be confusing. Consider extracting this into a separate validator function for better maintainability.
+const validatePhoneNumber = (value: string) => { + return PhoneNumberValidator(["mobile", "landline"])(value) !== undefined && phonePreg(value); +}; + phone_number: z .string() .min(1, { message: t("required") }) - .refine( - (val: string) => { - if ( - !PhoneNumberValidator(["mobile", "landline"])(val) === undefined || - !phonePreg(val) - ) { - return false; - } - return true; - }, - { - message: t("invalid_phone_number"), - }, - ), + .refine(validatePhoneNumber, { message: t("invalid_phone_number") }),
224-232: Enhance error handling for pincode queries.The current error handling only shows a generic "Invalid pincode" message. Consider providing more specific error information to help users understand and resolve the issue.
if (isPincodeError) { - toast.error("Invalid pincode"); + toast.error(t("invalid_pincode_details"), { + description: t("please_check_pincode_and_try_again"), + }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
public/locale/en.json(2 hunks)src/components/Facility/FacilityCreate.tsx(8 hunks)src/components/Facility/FacilityForm.tsx(8 hunks)src/pages/Organization/OrganizationFacilities.tsx(1 hunks)src/pages/Organization/components/AddFacilitySheet.tsx(1 hunks)src/pages/Organization/components/OrganizationSelector.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- public/locale/en.json
- src/pages/Organization/components/OrganizationSelector.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/pages/Organization/OrganizationFacilities.tsx
[error] 117-117: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (2)
src/components/Facility/FacilityCreate.tsx (2)
Line range hint
63-77: Consider implementing debouncing for pincode changes.To prevent potential race conditions when users type quickly, consider implementing debouncing for pincode changes. This will help manage API requests more efficiently and prevent stale data updates.
+import { useDebounce } from "@/hooks/useDebounce"; + const [pincode, setPincode] = useState(""); +const debouncedPincode = useDebounce(pincode, 300);
Line range hint
1-566: Implementation successfully addresses the PR objectives.The changes effectively implement the facility editing feature with pincode-based autofill and organization selection. The code is well-structured and includes proper form validation, error handling, and internationalization support.
|
I added edit feature and support for organization selector for facility form in organization And i have a doubt |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/Utils/utils.ts (3)
193-196: Use i18n for error messagesThe error message is hardcoded. Since the file already imports
tfromi18next, use it for internationalization.- toast.error("Invalid pincode"); + toast.error(t("invalid_pincode"));
193-196: Add TypeScript interface for the API responseDefine an interface for the API response to improve type safety and code maintainability.
interface PincodeApiResponse { records: Array<{ pincode: string; officename: string; statename: string; districtname: string; // add other fields as needed }>; }
193-196: Consider implementing caching for pincode dataSince pincode data rarely changes, consider implementing a caching mechanism to:
- Reduce API calls
- Improve performance
- Handle rate limiting
src/components/Facility/FacilityForm.tsx (2)
213-213: Consider reducing geolocation timeout.The current 10-second timeout for geolocation might lead to a poor user experience. Consider reducing it to 5 seconds and providing a retry option.
- { timeout: 10000 }, + { timeout: 5000 },
220-224: Optimize pincode validation and data fetching.The current implementation might trigger unnecessary API calls as it validates the pincode on every change.
Consider debouncing the pincode validation:
const { data: pincodeData } = useQuery({ queryKey: ["pincodeDetails", pincode], queryFn: () => getPincodeDetails(pincode, careConfig.govDataApiKey), - enabled: validatePincode(pincode) && pincode != facilityData?.pincode, + enabled: validatePincode(pincode) && pincode !== facilityData?.pincode, + staleTime: 5 * 60 * 1000, // Cache pincode data for 5 minutes });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
public/locale/en.json(10 hunks)src/Utils/utils.ts(1 hunks)src/components/Facility/FacilityForm.tsx(12 hunks)src/pages/Organization/OrganizationFacilities.tsx(1 hunks)src/pages/Organization/components/AddFacilitySheet.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- public/locale/en.json
🔇 Additional comments (4)
src/Utils/utils.ts (1)
193-196: Review API key handlingThe API key is passed as a parameter which could expose it in client-side code. Consider:
- Moving the API call to a backend service
- Implementing proper API key rotation
- Adding rate limiting
src/pages/Organization/components/AddFacilitySheet.tsx (1)
24-27: LGTM! Well-structured component with clear separation of create/edit modes.The component effectively handles both creation and editing scenarios with proper conditional rendering and internationalization support.
Also applies to: 34-48
src/components/Facility/FacilityForm.tsx (2)
380-385: LGTM! Well-implemented pincode change handler.The implementation correctly handles pincode changes and resets the selected levels appropriately.
134-139: Review form reset behavior after update.The current implementation resets the form immediately after a successful update, which might clear user input if they're still making changes.
Consider showing a confirmation dialog before resetting the form or removing the reset altogether since the sheet will likely close:
onSuccess: (_data: FacilityModel) => { toast.success(t("facility_updated_successfully")); queryClient.invalidateQueries({ queryKey: ["organizationFacilities"] }); - form.reset(); onSubmitSuccess?.(); },
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/components/Facility/FacilityForm.tsx (3)
79-83: Initialize pincode state with form valueThe
pincodestate should be initialized with the form's pincode value to ensure consistency when editing a facility.- const [pincode, setPincode] = useState(""); + const [pincode, setPincode] = useState(form.getValues("pincode") || "");Also applies to: 89-91
130-150: Enhance error handling for API failuresThe error handling could be improved in the following ways:
- Add specific error messages for different API failure scenarios
- Handle network timeouts
- Add retry logic for pincode API calls
const { data: pincodeData } = useQuery({ queryKey: ["pincodeDetails", pincode], queryFn: () => getPincodeDetails(pincode, careConfig.govDataApiKey), enabled: validatePincode(pincode) && pincode != facilityData?.pincode, + retry: 3, + retryDelay: 1000, + onError: (error) => { + toast.error(t("pincode_fetch_error")); + } });Also applies to: 152-158, 220-224
392-401: Enhance accessibility for form controlsConsider adding the following accessibility improvements:
- ARIA labels for the organization selector
- Loading state announcements for screen readers
<OrganizationSelector required={true} value={facilityData?.geo_organization} parentSelectedLevels={selectedLevels} + aria-label={t("select_organization")} onChange={(value) => { form.setValue("geo_organization", value); }} />Also applies to: 546-570
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/Facility/FacilityForm.tsx(12 hunks)src/pages/Organization/OrganizationFacilities.tsx(1 hunks)src/pages/Organization/components/AddFacilitySheet.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/pages/Organization/OrganizationFacilities.tsx
- src/pages/Organization/components/AddFacilitySheet.tsx
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Redirect rules - care-ohc
- GitHub Check: OSSAR-Scan
- GitHub Check: cypress-run (1)
🔇 Additional comments (2)
src/components/Facility/FacilityForm.tsx (2)
1-1: LGTM! Import and schema changes align with requirements.The additions of
careConfig,useQuery, and organization-related imports, along with the schema update forgeo_organization, properly support the new auto-population features.Also applies to: 3-3, 43-44, 47-49, 58-58
Line range hint
1-574: Implementation successfully meets PR objectivesThe changes effectively implement the facility edit feature with auto-population of pincode and geo_organization information. The code is well-structured, includes proper error handling, and uses TypeScript effectively.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/components/Facility/FacilityForm.tsx (2)
130-150: Consider enhancing error handling with more specific error messages.The error handling is good but could be more informative for users.
onError: (error: Error) => { const errorData = error.cause as { errors: { msg: string[] } }; if (errorData?.errors?.msg) { errorData.errors.msg.forEach((msg) => { toast.error(msg); }); } else { - toast.error(t("facility_update_error")); + toast.error(t("facility_update_error"), { + description: error.message || t("please_try_again_later") + }); } }
180-197: Add cleanup to prevent memory leaks.The form reset effect should handle component unmounting.
useEffect(() => { + let mounted = true; if (facilityData) { + if (!mounted) return; form.reset({ facility_type: facilityData.facility_type, // ... other fields }); } + return () => { + mounted = false; + }; }, [facilityData, form]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Facility/FacilityCreate.tsx(9 hunks)src/components/Facility/FacilityForm.tsx(12 hunks)
🔇 Additional comments (4)
src/components/Facility/FacilityForm.tsx (3)
1-1: LGTM! Imports and schema updates are well-structured.The new imports and schema changes appropriately support the pincode and organization auto-fill functionality.
Also applies to: 3-3, 47-47, 48-48, 58-58
79-83: LGTM! Interface changes enhance component reusability.The renamed interface and added
facilityIdprop properly support both create and edit scenarios.
238-244: 🛠️ Refactor suggestionHandle potential race conditions in organization data fetching.
The sequential organization queries could lead to race conditions if responses arrive out of order.
const { data: stateOrg } = useFetchOrganizationByName(stateName); const { data: districtOrg } = useFetchOrganizationByName( districtName, stateOrg?.id, + { + enabled: !!stateOrg?.id && !!districtName, + } );Likely invalid or redundant comment.
src/components/Facility/FacilityCreate.tsx (1)
259-259: LGTM! Internationalization is well implemented.The translation keys are consistently used and properly structured.
Also applies to: 513-515
|
👋 Hi, @Mahendar0701, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/components/Facility/FacilityCreate.tsx (1)
224-232:⚠️ Potential issueMove error handling to useEffect.
The current error handling can trigger multiple toasts during renders. Move it to useEffect for better user experience.
-if (isPincodeError) { - toast.error("Invalid pincode"); -} +useEffect(() => { + if (isPincodeError) { + toast.error(t("invalid_pincode")); + } + return () => { + toast.dismiss(); + }; +}, [isPincodeError, t]);
🧹 Nitpick comments (1)
src/components/Facility/FacilityCreate.tsx (1)
234-250: Add loading states for organization selection.The organization selection logic should handle loading states to provide better feedback to users.
+const [isLoadingOrg, setIsLoadingOrg] = useState(false); + useEffect(() => { + setIsLoadingOrg(true); if (stateOrg && districtOrg) { setSelectedLevels([stateOrg, districtOrg]); } else { setSelectedLevels([]); } + setIsLoadingOrg(false); }, [stateOrg, districtOrg]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
public/locale/en.json(10 hunks)src/components/Facility/FacilityCreate.tsx(9 hunks)src/pages/Organization/components/OrganizationSelector.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/pages/Organization/components/OrganizationSelector.tsx
- public/locale/en.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (4)
src/components/Facility/FacilityCreate.tsx (4)
1-1: LGTM! Well-structured imports and state management.The new imports and state variables are properly organized and typed, supporting the pincode auto-fill and organization selection features.
Also applies to: 63-66, 75-77
158-158: LGTM! Proper form reset handling.The form reset logic correctly handles the geo_organization field, ensuring it's properly populated during facility editing.
513-515: LGTM! Well-structured visibility settings section.The visibility settings section is properly internationalized and follows the UI pattern consistently.
375-379: Verify organization prefill functionality.Based on the PR comments, please verify that the organization prefill works correctly when editing an existing facility.
✅ Verification successful
Organization prefill implementation is correct
The code properly handles organization prefill in edit mode through the useEffect hook that initializes the form with facility data, including the geo_organization field. The watch and setValue implementations in the reviewed code correctly maintain the field's state.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if organization field is properly handled in edit mode ast-grep --pattern 'useEffect(() => { if (facilityData) { $$$ geo_organization: $$$ $$$ } }, $$$)'Length of output: 116
Script:
#!/bin/bash # Search for form initialization with facilityData rg "facilityData" -A 5 -B 5 "src/components/Facility/FacilityCreate.tsx" # Also check for form default values ast-grep --pattern 'defaultValues: { $$$ }'Length of output: 1731
There was a problem hiding this comment.
This file could be deleted now, no? 👍
There was a problem hiding this comment.
@Jacobjeevan this file is being used for updating facility in facility detials page
There was a problem hiding this comment.
Add an EditFacilitySheet (instead of modifying AddFacilitySheet) and call that component here and for the edit link in Organization Facilities page.
| description: facilityData.description || "", | ||
| features: facilityData.features || [], | ||
| pincode: facilityData.pincode?.toString() || "", | ||
| geo_organization: facilityData.geo_organization || "", |
There was a problem hiding this comment.
geo_organization here would be an Organization object rather than a string.
Update return type for getPermittedFacility to be FacilityData rather than FacilityModel (make any additional changes as needed when updating this route).
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
src/pages/Organization/components/EditFacilitySheet.tsx (2)
27-27: Remove debug console.log statement.Debug logging should be removed before merging to production.
- console.log(open);
19-22: Remove unused organizationId prop.The
organizationIdprop is defined in the Props interface but not used in the component.interface Props { - organizationId?: string; facilityId: string; }src/pages/Appointments/components/AppointmentTokenCard.tsx (1)
Line range hint
71-72: Address TODO comment about token number.The comment indicates that the token number should come from the backend instead of using
getFakeTokenNumber.Would you like me to help create a GitHub issue to track the implementation of backend token number support?
src/components/Facility/FacilityForm.tsx (2)
Line range hint
1-58: Consider enhancing schema validation for geo_organization.The current validation only checks for non-empty strings. Consider adding more specific validation rules to ensure the value is a valid organization reference.
- geo_organization: z.string().min(1, { message: "required" }), + geo_organization: z.string().min(1, { message: "required" }).refine( + (val) => { + try { + const parsed = JSON.parse(val); + return parsed && typeof parsed === 'object'; + } catch { + return false; + } + }, + { message: "Invalid organization format" } + ),
372-381: Add error handling for organization selection.Consider adding error handling for cases where the organization selection fails or becomes invalid.
<OrganizationSelector required={true} value={form.watch("geo_organization")} parentSelectedLevels={selectedLevels} onChange={(value) => { + try { form.setValue("geo_organization", value); + } catch (error) { + console.error("Failed to update organization:", error); + toast.error(t("organization_update_failed")); + } }} />src/components/Facility/FacilityCreate.tsx (1)
375-379: Add validation before setting organization value.Consider validating the organization value before setting it in the form.
value={form.watch("geo_organization")} parentSelectedLevels={selectedLevels} onChange={(value) => { + if (!value) { + toast.error(t("invalid_organization_selection")); + return; + } form.setValue("geo_organization", value); }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
public/locale/en.json(10 hunks)src/Utils/featureFlags.tsx(2 hunks)src/Utils/request/api.tsx(1 hunks)src/Utils/utils.ts(2 hunks)src/components/Facility/FacilityCreate.tsx(9 hunks)src/components/Facility/FacilityForm.tsx(12 hunks)src/components/Facility/FacilityHome.tsx(3 hunks)src/pages/Appointments/AppointmentDetail.tsx(2 hunks)src/pages/Appointments/components/AppointmentTokenCard.tsx(1 hunks)src/pages/Appointments/utils.ts(2 hunks)src/pages/Organization/OrganizationFacilities.tsx(2 hunks)src/pages/Organization/components/AddFacilitySheet.tsx(3 hunks)src/pages/Organization/components/EditFacilitySheet.tsx(1 hunks)src/pages/Organization/components/OrganizationSelector.tsx(4 hunks)src/types/facility/facility.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/Utils/utils.ts
- src/pages/Organization/components/AddFacilitySheet.tsx
- src/pages/Organization/components/OrganizationSelector.tsx
- public/locale/en.json
🔇 Additional comments (14)
src/types/facility/facility.ts (1)
31-35: LGTM! Well-structured type definitions.The new optional properties
locationandfacility_flagsare properly typed and maintain backward compatibility.src/pages/Organization/components/EditFacilitySheet.tsx (1)
47-53: LGTM! Proper query invalidation.Good practice to invalidate the currentUser query after successful form submission to ensure UI reflects the latest data.
src/Utils/featureFlags.tsx (1)
50-53: LGTM! Consistent type updates.The type changes from FacilityModel to FacilityData are consistent with the type definitions and maintain type safety throughout the feature flags system.
src/pages/Appointments/components/AppointmentTokenCard.tsx (2)
10-10: LGTM! Consistent type updates.The type change from FacilityModel to FacilityData is consistent with the type system updates.
Also applies to: 16-16
Line range hint
47-53: Verify auto-population functionality.Based on PR comments, there are concerns about:
- Organization info not being prefilled
- Auto-fill not working when pincode is entered
Let's verify the implementation:
src/pages/Appointments/utils.ts (1)
144-144: LGTM! Type update aligns with codebase refactoring.The change from
FacilityModeltoFacilityDatais consistent with the broader type refactoring across the codebase.src/Utils/request/api.tsx (1)
199-199: LGTM! API response type update is consistent.The update of the
getPermittedFacilityroute's response type toFacilityDataaligns with the codebase's type system refactoring.src/pages/Appointments/AppointmentDetail.tsx (1)
210-210: LGTM! Props type update is consistent.The change from
FacilityModeltoFacilityDatain theAppointmentDetailscomponent props aligns with the type system refactoring.src/pages/Organization/OrganizationFacilities.tsx (4)
20-20: LGTM!The import statement follows the project's import order convention.
118-120: LGTM! Key prop added as requested.The change addresses the previous review comment by adding a unique
keyprop to theCardcomponent.
122-148: LGTM! Well-structured facility card implementation.The code follows best practices with:
- Proper conditional rendering of facility images
- Fallback to avatar when no image is available
- Clear organization of facility information
149-172: LGTM! Accessible card footer implementation.The implementation:
- Properly separates edit and view actions
- Uses semantic HTML structure
- Follows accessibility best practices with proper button and link usage
src/components/Facility/FacilityCreate.tsx (2)
224-232:⚠️ Potential issueMove error handling to useEffect to prevent render issues.
The current implementation shows toast errors during render, which can cause multiple notifications and side effects.
- if (isPincodeError) { - toast.error("Invalid pincode"); - } + useEffect(() => { + if (isPincodeError) { + toast.error(t("invalid_pincode")); + } + return () => { + toast.dismiss(); + }; + }, [isPincodeError, t]);Likely invalid or redundant comment.
234-250: 🛠️ Refactor suggestionEnhance error handling for missing organization data.
Add proper error handling when state or district organizations are not found.
useEffect(() => { if (stateOrg && districtOrg) { setSelectedLevels([stateOrg, districtOrg]); } else { setSelectedLevels([]); + if (stateName && !stateOrg) { + toast.error(t("state_not_found"), { + description: t("state_not_found_in_system", { state: stateName }) + }); + } else if (stateOrg && districtName && !districtOrg) { + toast.error(t("district_not_found"), { + description: t("district_not_found_in_system", { district: districtName }) + }); + } } -}, [stateOrg, districtOrg]); +}, [stateOrg, districtOrg, stateName, districtName, t]);Likely invalid or redundant comment.
| > | ||
| <Settings className="mr-2 h-4 w-4" /> | ||
| {t("update_facility")} | ||
| <EditFacilitySheet facilityId={facilityId} /> |
There was a problem hiding this comment.
Remove duplicate EditFacilitySheet component.
The EditFacilitySheet component is rendered twice:
- Inside the dropdown menu (line 250)
- Outside the dropdown menu (line 264)
This duplication could lead to UI/UX issues and potential conflicts in functionality.
Keep only one instance, preferably the one in the dropdown menu, and remove the other.
<DropdownMenuItem
onClick={(e) => {
e.stopPropagation();
}}
>
<Settings className="mr-2 h-4 w-4" />
<EditFacilitySheet facilityId={facilityId} />
</DropdownMenuItem>
</DropdownMenuContent>
</DropdownMenu>
- <EditFacilitySheet facilityId={facilityId} />Also applies to: 264-264
|
👋 Hi, @Mahendar0701, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
Proposed Changes
Fixes Switch to using CreateFacilityForm #9849
Pincode autofill and geo_organization info auto populates
Added edit feature for facility in orgainization
Added support for organization selector in facility form
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Release Notes
New Features
Improvements
Localization
The release introduces more flexible facility management with enhanced location and organization selection capabilities.