Support For BULK beds creation in location #10815
Conversation
WalkthroughThis pull request introduces new internationalized strings for bed management in the locale file and extends the facility location form to allow bulk bed creation with customizable bed names. It also downgrades two Rollup dependency versions in the package file and adjusts the styling of a sheet component for improved responsiveness and overflow handling. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant F as LocationForm
participant S as Server
participant N as Notification
U->>F: Toggle "Create Multiple Beds"
U->>F: Enter number of beds & customize names
U->>F: Submit form
F->>S: Send BatchRequestBody for bulk bed creation
S-->>F: Return success with bed count
F->>N: Display notification with created bed count
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🪧 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. |
![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 (2)
src/pages/Facility/settings/locations/LocationForm.tsx (2)
136-149: Implemented batch submission mutation.The batch submission mutation is added to handle creating multiple beds at once. However, there is a commented-out error handler that should be either implemented or removed.
Consider either implementing the error handling by uncommenting and updating the error toast:
- // onError: () => { - // toast.error(t("submission_failed")); - // }, + onError: () => { + toast.error(t("bed_creation_failed")); + },Or remove the commented-out code if it's not needed.
152-177: Implemented bulk bed creation functionality.This implementation allows creating multiple beds at once when beds_count > 1.
However, there's an inconsistency in the mode parameter between single and batch submissions.
In line 180, the single submission uses
mode: "kind", but in the batch submission body, it usesmode: "instance". Consider using the same mode value for consistency:const data: LocationWrite = { ...values, - mode: "instance", + mode: "kind", description: values.description || "", organizations: values.organizations, parent: values.parent || undefined, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
public/locale/en.json(2 hunks)src/pages/Facility/settings/locations/LocationForm.tsx(9 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Redirect rules - care-ohc
- GitHub Check: Header rules - care-ohc
- GitHub Check: Pages changed - care-ohc
- GitHub Check: Test
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (10)
public/locale/en.json (2)
471-471: Localization key added for bed count validation.This localization key provides an error message for validation when the number of beds is not greater than zero, supporting the new bulk bed creation feature.
2297-2297: Localization key added for bed count field label.This localization key provides the label text for the total number of beds field, supporting the new bulk bed creation feature.
src/pages/Facility/settings/locations/LocationForm.tsx (8)
3-3: Added direct i18next import for translations.Good addition of the direct import of the translation function, which is used for schema validation messages.
39-42: Added batch operation type imports.These imports support the new bulk beds creation functionality by providing the types needed for batch requests.
45-45: Updated validation message to use translation function.Good improvement for internationalization by using the translation function for the field validation error message.
51-54: Added beds_count field to form schema.The beds_count field is properly defined with appropriate validation using the translation function for the error message.
73-73: Added default value for beds_count.Setting a default value of 1 for beds_count is appropriate and ensures the form has a valid initial state.
95-96: Added isEditMode flag for better code readability.Good addition of this variable which makes the code more readable and avoids repeating the condition checking throughout the component.
231-244: Added real-time preview of generated bed names.Great UX enhancement! This preview shows users exactly what bed names will be created based on their input, helping them understand the batch creation process.
361-364: Updated button text with translations.Good improvement for internationalization by using the translation function for the action button, with different text based on create/update mode.
Screen.Recording.2025-02-26.at.12.19.23.AM.mov |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
public/locale/en.json(2 hunks)src/pages/Facility/settings/locations/LocationForm.tsx(9 hunks)
🔇 Additional comments (8)
public/locale/en.json (3)
471-471: LGTM: Added validation error message for bed count.The new error message is clear and descriptive, ensuring users understand they must enter a positive number for bed count.
474-474: LGTM: Added error message for bed creation failure.This provides appropriate feedback when bulk bed creation fails.
2298-2298: LGTM: Added label for total number of beds field.This label clearly describes the purpose of the beds_count field in the form.
src/pages/Facility/settings/locations/LocationForm.tsx (5)
152-177: LGTM: Added bulk bed creation functionality.The implementation of bulk bed creation correctly:
- Checks if the form type is for beds (
"bd")- Verifies it's not in edit mode
- Ensures beds_count is greater than 1
- Builds a batch request with multiple locations
This will significantly improve workflow efficiency for users who need to add multiple beds at once.
289-306: LGTM: Added beds_count field to the form.The implementation correctly:
- Shows the field only when creating beds (form type is "bd")
- Hides it in edit mode
- Uses proper form control and validation
- Sets appropriate minimum value
95-96: LGTM: Added clear isEditMode flag.Using a dedicated variable for checking edit mode improves code readability and maintainability throughout the component.
136-149: LGTM: Added batch submission mutation.The implementation properly handles:
- Success case with appropriate toast message
- Error case with user-friendly error message
- Query invalidation to refresh the data
180-180:✅ Verification successful
Verify change in location mode from "instance" to "kind".
You've changed the mode from "instance" (in the bulk creation) to "kind" for the regular submission. Please verify this is intentional and won't affect functionality.
🏁 Script executed:
#!/bin/bash # Search for other usages of mode to understand the expected values echo "Searching for mode usage in location-related files..." rg -A 2 -B 2 "mode.*['\"]kind['\"]|mode.*['\"]instance['\"]" --type tsLength of output: 2502
Confirmed: The change to use mode "kind" for regular submission is intentional.
- The code in
src/pages/Facility/settings/locations/LocationForm.tsxnow uses mode"instance"for bulk creation and mode"kind"for regular submissions.- The check in
src/pages/Facility/settings/locations/LocationView.tsxverifies that locations display correctly whenlocation.mode === "kind".- Components like
<LocationSearch>already support both"kind"and"instance"as valid values.
Screen.Recording.2025-02-26.at.2.40.39.PM.mov |
|
LGTM |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/pages/Facility/settings/locations/LocationForm.tsx (2)
3-3: 🛠️ Refactor suggestionRemove direct import of i18next translation function.
The
tfunction is imported directly from i18next, but the component already uses theuseTranslation()hook. This creates two separate instances of the translation function and can lead to inconsistencies.-import { t } from "i18next";
53-75: 🛠️ Refactor suggestionMove schema definition inside the component to avoid i18n initialization issues.
Defining the schema outside the component with direct calls to
t()can cause issues because:
- It executes during module initialization, before the i18n system is fully initialized
- It won't respond to language changes
- It creates inconsistency with the hook-based translation used elsewhere in the component
Move the schema definition inside the component and use the translation function from the hook:
-const formSchema = z.object({ - name: z - .string() - .trim() - .min(1, { message: t("field_required") }), - description: z.string().optional(), - status: z.enum(["active", "inactive", "unknown"] as const), - operational_status: z.enum(["C", "H", "O", "U", "K", "I"] as const), - form: z.enum(LocationFormOptions), - parent: z.string().optional().nullable(), - enableBulkCreation: z.boolean().default(false), - numberOfBeds: z.string().optional(), - customizeNames: z.boolean().default(false), - organizations: z.array(z.string()).default([]), - availability_status: z.enum(["available", "unavailable"] as const), - bedNames: z - .array( - z.object({ - name: z.string().min(1, { message: t("field_required") }), - }), - ) - .default([]), -}); function LocationForm({ facilityId, onSuccess, locationId, parentId, }: Props) { const { t } = useTranslation(); + + const formSchema = z.object({ + name: z + .string() + .trim() + .min(1, { message: t("field_required") }), + description: z.string().optional(), + status: z.enum(["active", "inactive", "unknown"] as const), + operational_status: z.enum(["C", "H", "O", "U", "K", "I"] as const), + form: z.enum(LocationFormOptions), + parent: z.string().optional().nullable(), + enableBulkCreation: z.boolean().default(false), + numberOfBeds: z.string().optional(), + customizeNames: z.boolean().default(false), + organizations: z.array(z.string()).default([]), + availability_status: z.enum(["available", "unavailable"] as const), + bedNames: z + .array( + z.object({ + name: z.string().min(1, { message: t("field_required") }), + }), + ) + .default([]), + });
🧹 Nitpick comments (6)
src/pages/Facility/settings/locations/LocationForm.tsx (6)
144-180: Simplify and optimize the useEffect for managing bed names.The current implementation has several issues:
- It relies on multiple form.watch() calls, leading to potential performance issues
- It has complex nesting conditions that are hard to follow
- The dependency array uses form.watch() calls, which is not recommended
Consider refactoring this effect to be more efficient:
useEffect(() => { - const formType = form.watch("form"); - const bulkCreationEnabled = form.watch("enableBulkCreation"); - const numberOfBeds = form.watch("numberOfBeds"); - const locationName = form.watch("name"); - const customizeNames = form.watch("customizeNames"); + const { form: formType, enableBulkCreation, numberOfBeds, name, customizeNames } = form.getValues(); + + // Only proceed if we're in the correct state for bulk bed creation + if (formType !== "bd" || !enableBulkCreation || !numberOfBeds || !name) { + return; + } - if ( - formType === "bd" && - bulkCreationEnabled && - numberOfBeds && - locationName - ) { - if (!customizeNames || bedFields.length === 0 || locationName) { - resetToDefaultNames(); - } else { - const newCount = Number.parseInt(numberOfBeds ?? "0", 10); - const currentFields = form.getValues("bedNames") ?? []; - const updatedFields = [...currentFields]; - - while (updatedFields.length < newCount) { - updatedFields.push({ - name: `${locationName} ${updatedFields.length + 1}`, - }); - } - - replaceBedFields(updatedFields.slice(0, newCount)); - } - } + // If not customizing or initial setup, use default names + if (!customizeNames || bedFields.length === 0) { + resetToDefaultNames(); + return; + } + + // Handle changes in bed count while maintaining custom names where possible + const newCount = Number.parseInt(numberOfBeds ?? "0", 10); + const currentFields = form.getValues("bedNames") ?? []; + const updatedFields = [...currentFields]; + + // Add any needed additional fields + while (updatedFields.length < newCount) { + updatedFields.push({ + name: `${name} ${updatedFields.length + 1}`, + }); + } + + // Trim to the correct size + replaceBedFields(updatedFields.slice(0, newCount)); }, [ - form.watch("form"), - form.watch("enableBulkCreation"), - form.watch("numberOfBeds"), - form.watch("name"), - form.watch("customizeNames"), + form.watch(["form", "enableBulkCreation", "numberOfBeds", "name", "customizeNames"]), bedFields.length, + resetToDefaultNames, + replaceBedFields, + form ]);
236-244: Improve reference_id in batch requests.The current reference_id isn't very descriptive, which could make it difficult to trace batch operations or debug issues.
requests: values.bedNames.map((bed) => ({ url: `/api/v1/facility/${facilityId}/location/`, method: "POST", - reference_id: parentId ? `Location ${parentId}` : "Location", + reference_id: `location-${bed.name}-${parentId || "root"}`, body: { ...data, name: bed.name, }, })),
234-248: Implement safeguards for large bulk operations.The code allows creation of up to 15 beds in bulk, but there's no confirmation dialog or warning for larger numbers, which could lead to accidental large-scale operations.
Consider adding a confirmation step for larger batch operations:
if (values.form === "bd" && !isEditMode && values.enableBulkCreation) { + // If creating a large number of beds, you might want to add a confirmation step + const bedCount = Number(values.numberOfBeds); + if (bedCount > 5 && !window.confirm(t("confirm_large_bulk_operation", { count: bedCount }))) { + return; + } + const batchRequest: BatchRequestBody = { requests: values.bedNames.map((bed) => ({ url: `/api/v1/facility/${facilityId}/location/`, method: "POST", reference_id: parentId ? `Location ${parentId}` : "Location", body: { ...data, name: bed.name, }, })), }; submitBatch(batchRequest); return; }
362-366: Add capping mechanism for numberOfBeds selection.While the UI limits selection to 2-15 beds, it would be good to add a type safety check to ensure the number stays within appropriate limits.
<SelectContent> - {Array.from({ length: 14 }, (_, i) => i + 2).map((num) => ( + {Array.from({ length: 14 }, (_, i) => i + 2) + .filter(num => num >= 2 && num <= 15) + .map((num) => ( <SelectItem key={num} value={num.toString()}> {num} {t("beds")} </SelectItem> - ))} + ))} </SelectContent>
481-491: Consider pagination or scrollable container for large number of beds preview.When creating many beds, the preview section could become quite large and potentially unwieldy for users.
<h4 className="font-medium mb-2">{t("preview_bed_names")}</h4> - <div className="text-sm text-muted-foreground flex flex-wrap gap-2"> + <div className="text-sm text-muted-foreground flex flex-wrap gap-2 max-h-60 overflow-y-auto p-2"> {bedFields.map((field) => ( <div key={field.id} className="px-3 py-1 bg-gray-100 rounded-md shadow-sm" > {field.name} </div> ))} </div>
503-503: Add translated placeholder for description field.The description field has a hardcoded English placeholder, which should be localized.
- <Textarea {...field} placeholder="Description" /> + <Textarea {...field} placeholder={t("description_placeholder")} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pages/Facility/settings/locations/LocationForm.tsx(9 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (4)
src/pages/Facility/settings/locations/LocationForm.tsx (4)
389-494: Well-implemented UI for bed customization.The implementation of the bed customization UI is well-structured and provides good user experience with:
- Clear information alerts
- Toggle for custom naming
- Accordion component for organizing the UI
- Visual preview of bed names
- Reset functionality
This approach effectively balances complexity with usability.
118-118: Well-defined edit mode boolean.Creating a clear
isEditModevariable makes the code more readable and helps avoid repetitive checks forlocation?.idthroughout the component.
279-279: Good approach for conditionally showing bed options.The
showBedOptionsvariable clarifies when to display bed-related UI elements, making the code more maintainable and easier to understand.
132-142: Well-implemented reset function for bed names.The
resetToDefaultNamesfunction is a good implementation that ensures bed names follow a consistent pattern when the user requests a reset.
|
merge checklist not followed |
|
Conflicts have been detected against the base branch. Please merge the base branch into your branch.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
public/locale/en.json (1)
893-893: Trailing Whitespace in "edit_bed_names"
The key "edit_bed_names" has a trailing space at the end of its value ("Edit {{count}} bed names "). This extra space might lead to display inconsistencies.Suggested Fix:
-"edit_bed_names": "Edit {{count}} bed names ", +"edit_bed_names": "Edit {{count}} bed names",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
public/locale/en.json(9 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Redirect rules - care-ohc
- GitHub Check: Header rules - care-ohc
- GitHub Check: Pages changed - care-ohc
- GitHub Check: Test
- GitHub Check: cypress-run (1)
- GitHub Check: CodeQL-Build
- GitHub Check: lint
- GitHub Check: OSSAR-Scan
🔇 Additional comments (9)
public/locale/en.json (9)
482-482: Validation Message Addition: "bed_count_validation_error"
The new key clearly indicates that the number of beds must be greater than 0. The text is concise and informative.
486-486: Placeholder for Bed Name: "bed_name_placeholder"
The placeholder uses the templated variable{{number}}which is clear and consistent with dynamic content needs.
488-488: Dynamic Label for Bed Number: "bed_number"
The label "Bed Number {{number}}" is straightforward and aligns well with the dynamic numbering required in the UI.
496-496: Error Message for Bed Creation: "beds_creation_failed"
The message "Failed to create beds" succinctly communicates the error. This key should integrate smoothly with the batch creation flow.
716-717: Bulk Bed Creation Keys: "create_multiple_beds" & "create_multiple_beds_description"
These new keys support the bulk bed creation feature introduced in this PR. The wording is clear and aligns with the feature’s objective. Please ensure the phrasing is consistent with the overall UI text guidelines.
754-755: Individual Bed Name Customization: "customize_bed_names" & "customize_bed_names_description"
These keys clearly instruct users on how to edit individual bed names. They are well-worded and enhance the personalization aspect of the bulk creation feature.
1900-1900: Preview Bed Names Key: "preview_bed_names"
The new key "preview_bed_names" with the value "Preview Bed Names" is clear and consistent with the feature that allows users to preview the bed names before finalizing creation.
2035-2035: Reset to Default Label: "reset_to_default"
The value "Reset to Default" is intuitive and matches expected UI behavior for restoring default values.
2386-2386: Total Beds Label: "total_number_of_beds"
The new key "total_number_of_beds" with the value "Total number of beds" clearly communicates the information and complements the bulk creation functionality.
|
@Jacobjeevan, the changes are done. Regarding the accordion, I’ve specifically added the ShadCN accordion for this form. If users don’t want to customize the names, the form stays clean and uncluttered. I personally feel it’s a great way to keep things organized—users can expand it only if they need to customize, otherwise, they can move to the next input field. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/pages/Facility/settings/locations/LocationForm.tsx (3)
135-135: Specify radix parameter in Number.parseInt calls.When using Number.parseInt, it's best practice to explicitly specify the radix parameter to avoid unexpected parsing behavior.
- { length: Number.parseInt(form.watch("numberOfBeds") ?? "0") }, + { length: Number.parseInt(form.watch("numberOfBeds") ?? "0", 10) }, - const newCount = Number.parseInt(numberOfBeds ?? "0", 10); + const newCount = Number.parseInt(numberOfBeds ?? "0", 10);Also applies to: 160-160
151-172: Simplify complex conditional logic.The nested conditions in this useEffect could be simplified for better readability and maintenance.
- if ( - formType === "bd" && - bulkCreationEnabled && - numberOfBeds && - locationName - ) { - if (!customizeNames || bedFields.length === 0 || locationName) { - resetToDefaultNames(); - } else { - const newCount = Number.parseInt(numberOfBeds ?? "0", 10); - const currentFields = form.getValues("bedNames") ?? []; - const updatedFields = [...currentFields]; - - while (updatedFields.length < newCount) { - updatedFields.push({ - name: `${locationName} ${updatedFields.length + 1}`, - }); - } - - replaceBedFields(updatedFields.slice(0, newCount)); - } - } + // Skip if conditions aren't met + if (formType !== "bd" || !bulkCreationEnabled || !numberOfBeds || !locationName) { + return; + } + + // Use default naming for beds if not customizing or fields are empty + if (!customizeNames || bedFields.length === 0) { + resetToDefaultNames(); + return; + } + + // Update existing custom bed names preserving user edits + const newCount = Number.parseInt(numberOfBeds ?? "0", 10); + const currentFields = form.getValues("bedNames") ?? []; + const updatedFields = [...currentFields]; + + while (updatedFields.length < newCount) { + updatedFields.push({ + name: `${locationName} ${updatedFields.length + 1}`, + }); + } + + replaceBedFields(updatedFields.slice(0, newCount));
174-180: Optimize useEffect dependencies.Instead of watching multiple form fields separately, you can use a single form.watch() with an array of fields.
- }, [ - form.watch("form"), - form.watch("enableBulkCreation"), - form.watch("numberOfBeds"), - form.watch("name"), - form.watch("customizeNames"), - bedFields.length, - ]); + }, [ + form.watch(["form", "enableBulkCreation", "numberOfBeds", "name", "customizeNames"]), + bedFields.length, + form, + replaceBedFields, + ]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pages/Facility/settings/locations/LocationForm.tsx(9 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (4)
src/pages/Facility/settings/locations/LocationForm.tsx (4)
3-3: Direct import of i18next translation function.The
tfunction is imported directly from i18next, while the component also usesuseTranslation()hook later.
53-75: Schema definition uses direct translation calls.Using the translation function directly in the schema definition is problematic as it executes during module initialization before the i18n system is fully initialized.
234-248: Well-implemented batch submission.The batch request implementation for bulk bed creation is well-structured and efficiently handles the creation of multiple beds in a single request.
387-492: Excellent UI for bed customization.The UI implementation for customizing bed names is well-designed with:
- Clear information alert
- Toggle for custom vs. default names
- Accordion for managing many bed names
- Visual preview of resulting bed names
- Reset button for reverting to defaults
This provides a great user experience for the bulk creation feature.
I see where you are going with that, but let's not do that. It's redundant because of the checkbox. If a user checks "Customize individual bed names" they should immediately see the editable fields. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
public/locale/en.json (3)
483-497: Bed Management Keys Initialization:The new keys
"bed_count_validation_error","bed_name_placeholder","bed_number", and"beds_creation_failed"are well-named and provide clear messaging for validations and error handling during bed creation. Make sure that the error message for bed count (i.e. "Number of beds must be greater than 0") exactly aligns with the underlying business logic.
519-519: Bulk Bed Creation Information:The
"bulk_bed_creation_info"key is informative and explains the bulk bed creation process well. Consider a minor wording change for clarity—for example, change “based on the beds names you provided” to “based on the bed names you provided” to improve grammatical consistency.
890-890: Edit Bed Names Prompt:The
"edit_bed_names"key is properly templated with{{count}}to indicate the number of bed names to edit. There is a minor issue: an extra trailing whitespace at the end of the value. Consider removing it to maintain consistency across locale entries.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
package.json(1 hunks)public/locale/en.json(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Redirect rules - care-ohc
- GitHub Check: Header rules - care-ohc
- GitHub Check: Pages changed - care-ohc
- GitHub Check: Test
- GitHub Check: OSSAR-Scan
- GitHub Check: CodeQL-Build
- GitHub Check: cypress-run (1)
- GitHub Check: lint
🔇 Additional comments (4)
public/locale/en.json (4)
717-718: Multiple Beds Creation Labels:The keys
"create_multiple_beds"and"create_multiple_beds_description"clearly introduce the option for bulk bed creation. The texts are concise and user-friendly. No issues found here.
755-756: Individual Bed Names Customization:The new keys
"customize_bed_names"and"customize_bed_names_description"effectively communicate that users can edit each bed’s name individually. They are clear and consistent with the overall tone.
1903-1903: Preview Bed Names Label:The
"preview_bed_names"key is concise and clear for indicating the preview functionality. It fits well with similar UI text elements.
2394-2394: Total Beds Display:The
"total_number_of_beds"key is appropriately named and clearly communicates its purpose. Just ensure its usage in the UI remains consistent with the other bed-related labels.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/pages/Facility/settings/locations/LocationForm.tsx (3)
138-174: Consider simplifying the complex useEffect for bed name management.The useEffect hook is handling several scenarios with nested conditions, making it difficult to maintain. Consider extracting smaller, more focused functions for each scenario.
- useEffect(() => { - const formType = form.watch("form"); - const bulkCreationEnabled = form.watch("enableBulkCreation"); - const numberOfBeds = form.watch("numberOfBeds"); - const locationName = form.watch("name"); - const customizeNames = form.watch("customizeNames"); - - if ( - formType === "bd" && - bulkCreationEnabled && - numberOfBeds && - locationName - ) { - if (!customizeNames || bedFields.length === 0 || locationName) { - resetToDefaultNames(); - } else { - const newCount = Number.parseInt(numberOfBeds ?? "0", 10); - const currentFields = form.getValues("bedNames") ?? []; - const updatedFields = [...currentFields]; - - while (updatedFields.length < newCount) { - updatedFields.push({ - name: `${locationName} ${updatedFields.length + 1}`, - }); - } - - replaceBedFields(updatedFields.slice(0, newCount)); - } - } - }, [ - form.watch("form"), - form.watch("enableBulkCreation"), - form.watch("numberOfBeds"), - form.watch("name"), - form.watch("customizeNames"), - bedFields.length, - ]); + useEffect(() => { + const formType = form.watch("form"); + const bulkCreationEnabled = form.watch("enableBulkCreation"); + const numberOfBeds = form.watch("numberOfBeds"); + const locationName = form.watch("name"); + const customizeNames = form.watch("customizeNames"); + + // Only proceed if we're in bed creation mode with valid inputs + if (formType !== "bd" || !bulkCreationEnabled || !numberOfBeds || !locationName) { + return; + } + + // If not customizing names or fields need initialization, use default naming + if (!customizeNames || bedFields.length === 0) { + resetToDefaultNames(); + return; + } + + // Update existing bed fields with new count + const newCount = Number.parseInt(numberOfBeds, 10); + const currentFields = form.getValues("bedNames") ?? []; + const updatedFields = [...currentFields]; + + // Add additional fields if needed + while (updatedFields.length < newCount) { + updatedFields.push({ + name: `${locationName} ${updatedFields.length + 1}`, + }); + } + + replaceBedFields(updatedFields.slice(0, newCount)); + }, [ + form.watch("form"), + form.watch("enableBulkCreation"), + form.watch("numberOfBeds"), + form.watch("name"), + form.watch("customizeNames"), + bedFields.length, + resetToDefaultNames, + replaceBedFields, + form, + ]);
233-233: Improve the reference_id for better traceability in batch requests.The current reference_id doesn't provide enough context for debugging or auditing purposes.
- reference_id: parentId ? `Location ${parentId}` : "Location", + reference_id: `bed-creation-${parentId || 'root'}-${bed.name}`,
434-461: Consider performance optimization for large number of beds.When dealing with a large number of beds (e.g., 15), rendering individual input fields might impact performance. Consider implementing virtualization for smoother UX.
You could use a library like
react-windowto virtualize the list of input fields for better performance. Here's an example implementation:import { FixedSizeList as List } from 'react-window'; // Inside your component where you render the bed fields const BedNameInputs = () => { const bedCount = Number(form.watch("numberOfBeds")); const renderBedItem = ({ index, style }) => ( <div style={style}> <FormField key={bedFields[index].id} control={form.control} name={`bedNames.${index}.name`} render={({ field }) => ( <FormItem> <div className="flex items-center gap-2"> <FormLabel className="text-sm font-medium min-w-[60px]"> {t("bed_number", { number: index + 1 })}: </FormLabel> <FormControl> <Input {...field} placeholder={t("bed_name_placeholder", { number: index + 1, })} className="flex-1" /> </FormControl> </div> <FormMessage /> </FormItem> )} /> </div> ); return ( <List height={400} itemCount={bedCount} itemSize={60} width="100%" > {renderBedItem} </List> ); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
package.json(1 hunks)src/pages/Facility/settings/locations/LocationForm.tsx(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🔇 Additional comments (8)
src/pages/Facility/settings/locations/LocationForm.tsx (8)
3-3: Remove direct i18next import in favor of useTranslation hook.The
tfunction is imported directly from i18next, but the component also usesuseTranslation()hook later. This creates two separate instances of the translation function.
47-69: Move schema definition inside the component to use the hook-provided translation function.Using the translation function directly in the schema definition is problematic because it executes during module initialization, before the i18n system is fully initialized.
126-136: LGTM! Excellent implementation of resetToDefaultNames function.The function correctly generates default bed names based on the location name and selected number of beds.
208-217: LGTM! Batch submission is set up correctly.The batch submission mutation is configured to handle successful bulk bed creation appropriately.
228-242: LGTM! Batch request submission logic is implemented correctly.The solution correctly handles bulk bed creation by generating a batch request with individual bed entries.
317-338: LGTM! Well-implemented checkbox for enabling bulk creation.The UI for enabling bulk creation is clear and provides a good description.
340-365: LGTM! Good implementation of the beds count selector.The dropdown for selecting the number of beds to create is well-implemented with clear options.
381-479: Well-structured UI for bed name customization.The UI for customizing bed names is comprehensive, with a reset option and clear preview.
|
@Jacobjeevan Done with the changes
|
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
demo video
https://drive.google.com/file/d/1VznD5d2_8CrokxlTDqYAYliyT5N3BAdH/view?usp=sharing
Summary by CodeRabbit
New Features
Style
Chores