flw_829 Changes of cataract surgery done#334
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis pull request introduces an eye surgery form management feature with a bottom-sheet UI for visit tracking, updates the beneficiary age threshold from 30 to 40 years across the application, adds a database migration for the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AllBenFragment
participant EyeSurgeryBottomSheetFragment
participant EyeSurgeryListViewModel
participant EyeSurgeryFormRepository
participant EyeSurgeryFormFragment
participant EyeSurgeryFormViewModel
participant InAppDb
User->>AllBenFragment: Click eye surgery button (non-IFA)
AllBenFragment->>EyeSurgeryBottomSheetFragment: showEyeSurgeryBottomSheet(benId, hhId, name, gender, age)
activate EyeSurgeryBottomSheetFragment
EyeSurgeryBottomSheetFragment->>EyeSurgeryListViewModel: getSavedVisits(benId)
activate EyeSurgeryListViewModel
EyeSurgeryListViewModel->>EyeSurgeryFormRepository: getAllVisitsByBenId(benId)
EyeSurgeryFormRepository->>InAppDb: Query ALL_EYE_SURGERY_VISIT_HISTORY by benId
InAppDb-->>EyeSurgeryFormRepository: Return list of EyeSurgeryFormResponseJsonEntity
EyeSurgeryFormRepository-->>EyeSurgeryListViewModel: Visit list
deactivate EyeSurgeryListViewModel
EyeSurgeryBottomSheetFragment->>EyeSurgeryBottomSheetFragment: Display visits + "Add New Visit" in RecyclerView
deactivate EyeSurgeryBottomSheetFragment
User->>EyeSurgeryBottomSheetFragment: Click visit item
activate EyeSurgeryBottomSheetFragment
EyeSurgeryBottomSheetFragment->>EyeSurgeryFormFragment: setNavigationCallback → navigateToEyeSurgeryForm(eyeSide, formDataJson, recordId, isViewMode)
deactivate EyeSurgeryBottomSheetFragment
activate EyeSurgeryFormFragment
EyeSurgeryFormFragment->>EyeSurgeryFormViewModel: loadFormSchemaFromJson(formDataJson) OR loadFormSchema(eyeSide)
activate EyeSurgeryFormViewModel
EyeSurgeryFormViewModel->>InAppDb: Fetch schema and populate fields
InAppDb-->>EyeSurgeryFormViewModel: Form fields with values
EyeSurgeryFormViewModel-->>EyeSurgeryFormFragment: Schema loaded
deactivate EyeSurgeryFormViewModel
EyeSurgeryFormFragment->>EyeSurgeryFormFragment: Render form with field values
User->>EyeSurgeryFormFragment: Fill form and submit
EyeSurgeryFormFragment->>EyeSurgeryFormViewModel: saveFormResponses(benId, hhId, eyeSide, recordId)
activate EyeSurgeryFormViewModel
EyeSurgeryFormViewModel->>EyeSurgeryFormRepository: upsertByEye(EyeSurgeryFormResponseJsonEntity)
activate EyeSurgeryFormRepository
EyeSurgeryFormRepository->>InAppDb: Persist with eyeSide (insert or update by benId+formId+eyeSide)
InAppDb-->>EyeSurgeryFormRepository: Persisted entity id
EyeSurgeryFormRepository-->>EyeSurgeryFormViewModel: Upsert complete
alt referral required
EyeSurgeryFormViewModel->>EyeSurgeryFormRepository: saveReferral(benId, referredTo, reasons)
EyeSurgeryFormRepository->>InAppDb: Save referral record
end
deactivate EyeSurgeryFormRepository
deactivate EyeSurgeryFormViewModel
EyeSurgeryFormFragment->>User: Show success / Navigate back
deactivate EyeSurgeryFormFragment
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
app/src/main/java/org/piramalswasthya/sakhi/model/dynamicEntity/eye_surgery/EyeSurgeryFormResponseJsonEntity.kt (1)
10-20:⚠️ Potential issue | 🟠 MajorDon't default the unique-key column to an empty eye side.
eyeSideis now part of the row identity, but this entity still defaults it to""whileMIGRATION_57_58backfills legacy rows to'LEFT'. That makesbenId + eyeSidelookups/upserts miss migrated records and create parallel rows instead of updating the existing one.🛠️ Safer shape for the entity
- val eyeSide: String = "", + val eyeSide: String,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/org/piramalswasthya/sakhi/model/dynamicEntity/eye_surgery/EyeSurgeryFormResponseJsonEntity.kt` around lines 10 - 20, The entity EyeSurgeryFormResponseJsonEntity sets eyeSide default to an empty string even though eyeSide is part of the unique index and MIGRATION_57_58 backfills legacy rows to 'LEFT'; change the model so eyeSide has no empty-string default (remove the default value) or set its default to match the migration ('LEFT') so lookups/upserts that use benId + eyeSide will match migrated rows; update EyeSurgeryFormResponseJsonEntity's eyeSide property accordingly and ensure any callers constructing instances without specifying eyeSide are updated to provide a correct value.app/src/main/java/org/piramalswasthya/sakhi/ui/home_activity/all_ben/AllBenFragment.kt (2)
228-240:⚠️ Potential issue | 🟠 MajorKeep the deactivated-beneficiary guard before showing the eye-surgery sheet.
This path now opens the cataract flow for every non-IFA row. The earlier
!item.isDeactivatecheck is gone, so deactivated beneficiaries can enter a new visit.Preserve the existing eligibility guard
- }else{ - - showEyeSurgeryBottomSheet(benId, hhId,item.benFullName, item.gender, item.age) + } else if (!item.isDeactivate) { + showEyeSurgeryBottomSheet( + benId, + hhId, + item.benFullName, + item.gender, + item.age + ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/org/piramalswasthya/sakhi/ui/home_activity/all_ben/AllBenFragment.kt` around lines 228 - 240, The current click handler in AllBenFragment now navigates to the eye-surgery flow for non-IFA rows without checking beneficiary deactivation; reintroduce the previous eligibility guard by checking item.isDeactivate (or !item.isDeactivate) before calling showEyeSurgeryBottomSheet so deactivated beneficiaries are blocked from starting a new visit, keeping the existing IFA branch (AllBenFragmentDirections.actionAllBenFragmentToBenIfaFormFragment) unchanged; update the lambda that currently receives (item, benId, hhId, isViewMode, isIFA) to first validate !item.isDeactivate and only then call showEyeSurgeryBottomSheet(benId, hhId, item.benFullName, item.gender, item.age).
108-116:⚠️ Potential issue | 🟠 MajorThe age-above-40 filter still drops non-death rows with
isDeath = 'undefined'.This branch routes to
filterType = 3, but the backing DAO predicate only checksisDeath = 0. Beneficiaries with the table default'undefined'will disappear from this filter even though they are not death records. Based on learnings, death-status queries inapp/src/main/java/org/piramalswasthya/sakhi/database/room/dao/BenDao.ktmust also treatisDeath = 'undefined'as non-death.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/org/piramalswasthya/sakhi/ui/home_activity/all_ben/AllBenFragment.kt` around lines 108 - 116, The age-above-40 filter (filterType = 3 from AllBenFragment::btnOk click) is currently excluded by the DAO because the BenDao query only checks isDeath = 0; update the DAO methods that filter on death status (the query methods in BenDao.kt that reference the isDeath column) to treat the string 'undefined' as non-death as well (e.g., include isDeath = 'undefined' in the predicate or check isDeath IN (...) / use coalesce/default handling) so beneficiaries with isDeath='undefined' are returned for non-death filters such as the AGE_ABOVE_40 branch. Ensure any helper/repository methods that call these DAO queries continue to use filterType 3 unchanged.app/src/main/java/org/piramalswasthya/sakhi/adapters/dynamicAdapter/FormRendererAdapter.kt (1)
1033-1126:⚠️ Potential issue | 🟠 MajorRevalidate dependent dates when the source field changes.
The active
"date"path now constrains the picker only when the dependent field is opened. If a user selects a validend_date/nrc_discharge_dateand later moves the source date forward, the stale dependent value stays in the model with no new error. Please keep that dependent-field revalidation in the active path, or move it into form-level validation before save.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/org/piramalswasthya/sakhi/adapters/dynamicAdapter/FormRendererAdapter.kt` around lines 1033 - 1126, The dependent-date constraints (e.g., end_date, nrc_discharge_date, follow_up_visit_date, visit_date) are only enforced when opening the dependent field's DatePicker, so when a source date moves forward the stale dependent values remain valid in the model; update the change handler that runs when a date is picked (the lambda passed to DatePickerDialog that calls onValueChanged(field, dateStr)) to revalidate and / or clear any dependent fields (use getDate("...") and field.fieldId names like "start_date","end_date","nrc_admission_date","nrc_discharge_date","follow_up_visit_date") after setting the new value, or alternatively add the same dependent-date validation to form-level validation invoked before save so stale dependent values are detected and errors set (field.errorMessage) consistently.app/src/main/java/org/piramalswasthya/sakhi/repositories/dynamicRepo/EyeSurgeryFormRepository.kt (2)
67-72:⚠️ Potential issue | 🟠 MajorOffline fallback still points at the HBNC schema.
If schema fetch/cache miss happens, this repository falls back to
hbnc_form_1stday.json. That will render the wrong form for eye surgery instead of recovering offline.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/org/piramalswasthya/sakhi/repositories/dynamicRepo/EyeSurgeryFormRepository.kt` around lines 67 - 72, The offline fallback currently used by loadSchemaFromAssets() in EyeSurgeryFormRepository reads "hbnc_form_1stday.json" which is the wrong schema; update loadSchemaFromAssets() to load the correct eye surgery offline schema (e.g., "eye_surgery_form.json" or the canonical eye surgery asset name), keep using FormSchemaDto.fromJson(json) and preserve the existing try/catch behavior, and ensure the asset filename reference is used consistently so the repository recovers to the eye surgery form on cache miss.
171-177:⚠️ Potential issue | 🟠 MajorDownloaded visits are still being deduped by month.
With the new eye-side model, continuing to funnel server visits through
insertAllByMonth()can overwrite one eye with the other when both surgeries fall in the same month. This path also never persistseyeSideon the imported entity above, soEyeSurgeryListViewModel.getAvailableEyes()cannot reliably treat synced visits as completed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/org/piramalswasthya/sakhi/repositories/dynamicRepo/EyeSurgeryFormRepository.kt` around lines 171 - 177, The current flow dedupes imported surgery visits by month via insertAllByMonth which calls jsonResponseDao.upsertByMonth, causing different eyeSide records in the same month to overwrite each other and never persisting eyeSide on EyeSurgeryFormResponseJsonEntity; change the import path to upsert each entity individually (avoid insertAllByMonth/jsonResponseDao.upsertByMonth) so every EyeSurgeryFormResponseJsonEntity is persisted with its eyeSide populated before save (or add a dedicated upsert method that keys by visit+eyeSide), and ensure EyeSurgeryListViewModel.getAvailableEyes() can rely on eyeSide being present on synced records.
🧹 Nitpick comments (6)
app/src/main/res/layout/rv_item_eye_surgery_visit.xml (1)
32-37: Move the fallback button label into a string resource.The adapter may overwrite this later, but keeping
VIEWin XML leaves the layout with an English-only fallback for previews and any pre-bind render.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/res/layout/rv_item_eye_surgery_visit.xml` around lines 32 - 37, Replace the hardcoded button text "VIEW" in the MaterialButton with a string resource: update the MaterialButton (id btnAction) in rv_item_eye_surgery_visit.xml to use android:text="@string/..." and add the corresponding entry in strings.xml (e.g., name="view_action" value="View") so the fallback label is localized and can still be overwritten by the adapter at runtime.app/src/main/res/layout/fragment_eye_surgery_form.xml (1)
39-48: Clamp the beneficiary name to one line.Long names will wrap here and push the gender/age cells out of alignment. A single-line, ellipsized header keeps the banner stable.
🧩 Keep the header compact
<TextView android:id="@+id/tv_ben_name" android:layout_width="0dp" android:layout_height="wrap_content" android:layout_weight="2" + android:ellipsize="end" android:textAppearance="@style/TextAppearance.Material3.TitleMedium" + android:maxLines="1" android:textColor="@android:color/white" android:drawablePadding="6dp" android:gravity="center_vertical" tools:text="" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/res/layout/fragment_eye_surgery_form.xml` around lines 39 - 48, The TextView with id tv_ben_name should be constrained to a single line to prevent long beneficiary names from wrapping and misaligning the gender/age cells; update the TextView attributes (tv_ben_name) to use android:maxLines="1" and android:ellipsize="end" (avoid deprecated android:singleLine) so overflowing names are truncated with an ellipsis while preserving the existing layout_weight and gravity.app/src/main/java/org/piramalswasthya/sakhi/adapters/BenListAdapter.kt (1)
102-107: Collapse the now-dead text branch.Both branches return
R.string.cataract_surgery, so the conditional and the commented-out alternatives just obscure that only the tint changes here.♻️ Simplify the assignment
- binding.btnAbove30.text = if (isMatched) { - binding.root.context.getString(R.string.cataract_surgery) -// binding.root.context.getString(R.string.view_edit_eye_surgery) - } else { -// binding.root.context.getString(R.string.add_eye_surgery) - binding.root.context.getString(R.string.cataract_surgery) - } + binding.btnAbove30.text = + binding.root.context.getString(R.string.cataract_surgery)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/org/piramalswasthya/sakhi/adapters/BenListAdapter.kt` around lines 102 - 107, The code in BenListAdapter.kt currently sets binding.btnAbove30.text using a conditional on isMatched with commented alternatives, but both branches use R.string.cataract_surgery; replace the entire conditional with a single assignment binding.btnAbove30.text = binding.root.context.getString(R.string.cataract_surgery), remove the commented-out strings and the if/else block, and keep any existing tint or style changes that still depend on isMatched (do not remove those).app/src/main/java/org/piramalswasthya/sakhi/ui/home_activity/immunization_due/child_immunization/list/ChildImmunizationListViewModel.kt (1)
43-59: Drop the commented-outbenWithVaccineDetailsimplementations.The file now keeps two dead versions of the same flow around the live one. That makes future fixes harder to audit and easy to apply to the wrong block.
Also applies to: 97-117
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/org/piramalswasthya/sakhi/ui/home_activity/immunization_due/child_immunization/list/ChildImmunizationListViewModel.kt` around lines 43 - 59, Remove the two commented-out implementations of benWithVaccineDetails so only the active flow remains: delete the commented block that defines benWithVaccineDetails (the one combining pastRecords and vaccinesFlow) and the duplicate commented block around the later occurrence (lines referenced in the review), leaving the live benWithVaccineDetails implementation untouched; this eliminates dead code and prevents confusion when editing pastRecords, vaccinesFlow or ImmunizationDetailsDomain transformations.app/src/main/res/navigation/nav_home.xml (1)
2233-2240: Align the bottom-sheet destination with how it is actually launched.The sheet is opened via
show(...), notNavController.navigate(...), so this<fragment>entry/action is detached from the real runtime flow. IfEyeSurgeryBottomSheetFragmentis aBottomSheetDialogFragment, model it as a<dialog>destination with the required args and navigate to it; otherwise this nav node/action is just dead wiring.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/res/navigation/nav_home.xml` around lines 2233 - 2240, The nav graph declares EyeSurgeryBottomSheetFragment as a <fragment> and an action (action_eyeSurgeryBottomSheet_to_eyeSurgeryFormFragment) but the sheet is actually shown via show(...), so the nav node is mismatched; update the nav to model EyeSurgeryBottomSheetFragment as a <dialog> destination if it extends BottomSheetDialogFragment (preserve any required arguments on that <dialog>), and remove or adjust the action/action target (action_eyeSurgeryBottomSheet_to_eyeSurgeryFormFragment -> eyeSurgeryFormFragment) since the sheet is not navigated via NavController; alternatively, if you intend to navigate, change the fragment launch call to NavController.navigate(...) instead of show(...) and keep the <fragment> entry.app/src/main/java/org/piramalswasthya/sakhi/ui/home_activity/all_ben/eye_surgery_registration/EyeSurgeryFormViewModel.kt (1)
79-87: Make the saved-data path eye-scoped.
repository.loadFormResponseJson(benId, formId)can only return one payload, but the table now allows multiple visits for the same beneficiary/form separated byeyeSide. LeavingloadSavedData = trueas the default makes it easy to prefill the wrong eye.#!/bin/bash # Verify that eye-surgery callers do not rely on the benId/formId-only load path. rg -n -C4 '\bloadFormSchema\s*\(' --type=kt rg -n -C4 '\bloadFormSchemaFromJson\s*\(' --type=kt rg -n -C4 '\bloadFormResponseJson\s*\(' --type=ktExpected result: new-visit flows should call
loadFormSchema(..., loadSavedData = false, ...), and edit flows should useloadFormSchemaFromJson(...)or another eye-specific loader.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/org/piramalswasthya/sakhi/ui/home_activity/all_ben/eye_surgery_registration/EyeSurgeryFormViewModel.kt` around lines 79 - 87, The current saved-data path in EyeSurgeryFormViewModel uses repository.loadFormResponseJson(benId, formId) which is not eye-scoped and can prefill the wrong eye; update the logic so saved data is looked up by eyeSide (e.g. call a repository method that accepts eyeSide or add eyeSide to loadFormResponseJson parameters) or disable loadSavedData for new-visit flows and ensure edit flows use loadFormSchemaFromJson(...) or an eye-specific loader; change references in EyeSurgeryFormViewModel to use the eye-aware loader and/or set loadSavedData = false for new-visit callers so benId+formId-only lookups are not used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/org/piramalswasthya/sakhi/database/room/dao/BenDao.kt`:
- Line 218: The age-40+ branch (the SQL fragment using OR (:filterType = 3 AND
CAST((strftime('%s','now') - dob/1000)/60/60/24/365 AS INTEGER) >= 40 AND
isDeath = 0)) and the equivalent branches for BENEFICIARY/BEN_BASIC_CACHE must
treat non-death as isDeath = 0 OR isDeath IS NULL OR isDeath = 'undefined';
update the predicate in the queries that currently use isDeath = 0 (including
the filterType = 3 branch and the matching queries around the other occurrences)
to use a combined condition such as (isDeath = 0 OR isDeath IS NULL OR isDeath =
'undefined') so beneficiaries with NULL/'undefined' alive state are included.
In
`@app/src/main/java/org/piramalswasthya/sakhi/database/room/dao/dynamicSchemaDao/EyeSurgeryFormResponseJsonDao.kt`:
- Around line 50-69: The upsertByEye() currently resolves existing rows only by
benId and eyeSide which can clash across different form versions; update the
lookup to include formId too: either add a new DAO query (e.g.,
getByBenEyeAndForm(benId: Long, eyeSide: String, formId: String)) or modify an
existing query to filter by benId, eyeSide and formId, then use that method
inside upsertByEye(entity: EyeSurgeryFormResponseJsonEntity) to fetch existing
before copying id and calling insertFormResponse.
In `@app/src/main/java/org/piramalswasthya/sakhi/database/room/InAppDb.kt`:
- Around line 318-333: MIGRATION_57_58 currently backfills eyeSide as 'LEFT'
(NOT NULL DEFAULT) then creates a unique index on (benId, formId, eyeSide),
which will conflict for legacy rows—change the migration in MIGRATION_57_58 so
the new column is created nullable (no NOT NULL DEFAULT and use "ADD COLUMN
eyeSide TEXT"), do not bulk backfill to a single value, and create the new
unique index that preserves the prior uniqueness granularity by including
visitMonth (e.g., CREATE UNIQUE INDEX
index_ALL_EYE_SURGERY_VISIT_HISTORY_benId_formId_visitMonth_eyeSide ON
ALL_EYE_SURGERY_VISIT_HISTORY(benId, formId, visitMonth, eyeSide)); this avoids
UNIQUE constraint failures while retaining per-visit uniqueness and lets you
populate or tighten eyeSide semantics in a later migration.
In
`@app/src/main/java/org/piramalswasthya/sakhi/repositories/dynamicRepo/EyeSurgeryFormRepository.kt`:
- Around line 206-220: The saveReferral function currently swallows all
exceptions (catching Exception and only calling e.printStackTrace()), which
masks failures and allows EyeSurgeryFormViewModel.saveFormResponses() to report
success even when ncdReferalRepo.saveReferedNCD(...) failed; remove or rework
the broad try/catch in saveReferral (or change its signature to return a
Result/Boolean or suspend that throws) so that exceptions from
ncdReferalRepo.saveReferedNCD are propagated to the caller (or return a failure
result) and update EyeSurgeryFormViewModel.saveFormResponses() to handle the
propagated exception/result accordingly; reference symbols: function
saveReferral, repository call ncdReferalRepo.saveReferedNCD, and caller
EyeSurgeryFormViewModel.saveFormResponses.
In `@app/src/main/java/org/piramalswasthya/sakhi/ui/BindingUtils.kt`:
- Around line 528-531: The visibility check in
Button.visibleIfAgeAbove30AndAlive currently only treats isDeath=="false" as
alive and hides the CTA when isDeath comes back as "undefined"; update the
condition in visibleIfAgeAbove30AndAlive so that isDeath values of null, "false"
or "undefined" (case-insensitive) are treated as alive before hiding the button.
Locate the visibleIfAgeAbove30AndAlive function and change the isDeath check to
consider isAlive when isDeath is null OR equals "false" OR equals "undefined"
(ignoreCase), then compute shouldShow = (age ?: 0) >= 40 && isAlive and set
visibility accordingly.
In
`@app/src/main/java/org/piramalswasthya/sakhi/ui/home_activity/all_ben/eye_surgery_registration/eyeBottomsheet/adapter/EyeSurgeryVisitListAdapter.kt`:
- Around line 37-40: Replace hardcoded labels "Add" and "View/Edit" in
EyeSurgeryVisitListAdapter (where tvVisitTitle/tvVisitDate/btnAction are bound)
with localized string resources; use the view/context to call getString(...)
(e.g., btnAction.context.getString(R.string.action_add) and
btnAction.context.getString(R.string.action_view_edit)) and add corresponding
entries (action_add, action_view_edit) to strings.xml (with Hindi translations)
so the UI uses resource-based localization.
In
`@app/src/main/java/org/piramalswasthya/sakhi/ui/home_activity/all_ben/eye_surgery_registration/eyeBottomsheet/EyeSurgeryBottomSheetFragment.kt`:
- Around line 202-214: The current navigateToForm in
EyeSurgeryBottomSheetFragment calls
navigationCallback?.navigateToEyeSurgeryForm(...) and then unconditionally calls
dismiss(), which will close the sheet even when navigationCallback is null;
change it to guard the dismissal so that you only call dismiss() if
navigationCallback is non-null and navigation actually started (e.g., check
navigationCallback != null before invoking navigateToEyeSurgeryForm and only
call dismiss() inside that branch), leaving the sheet open when the callback is
absent to avoid silently swallowing the tap after fragment recreation.
In
`@app/src/main/java/org/piramalswasthya/sakhi/ui/home_activity/all_ben/eye_surgery_registration/eyeBottomsheet/EyeSurgeryListViewModel.kt`:
- Around line 21-36: The per-visit debug logging in EyeSurgeryListViewModel (the
Log.d calls that iterate visits and print each it.id and it.eyeSide) is noisy
and should be removed; delete the visits.forEach { Log.d("EyeList", "visit:
id=${it.id} eyeSide=${it.eyeSide}") } (and optionally the initial Log.d of
visits size if you want no per-open logging), keeping only aggregate logs like
the visits.size, completed, or available as needed so no individual medical rows
are emitted to logs.
In
`@app/src/main/java/org/piramalswasthya/sakhi/ui/home_activity/all_ben/eye_surgery_registration/EyeSurgeryFormFragment.kt`:
- Around line 298-314: The code sets "form_submitted" and pops the back stack
even when save fails; change the flow so that
findNavController().previousBackStackEntry?.savedStateHandle?.set("form_submitted",
true) and findNavController().popBackStack() are executed only when
viewModel.saveFormResponses(benId, hhId, actualEyeSide, recordId) returns true
(isSaved). Modify the block around saveFormResponses and the Toasts so that on
isSaved == true you set the savedStateHandle key and pop the back stack, and on
failure you only show the failure Toast and do not modify the
previousBackStackEntry or navigate away.
- Around line 180-192: When the FAB edit click handler
(binding.fabEdit.setOnClickListener) flips isViewMode to false but formDataJson
is null/empty, recreate the form adapter in edit mode so fields become editable;
currently the adapter was initially created with isViewOnly = true so the UI
remains read‑only. In the else branch (when formDataJson.isNullOrEmpty()),
reinitialize or replace the existing adapter instance (the one constructed with
isViewOnly = true) with a new adapter configured with isViewOnly = false (or
call the fragment's adapter setup method), and attach it to the RecyclerView so
the form switches into editable mode immediately.
- Around line 328-331: The fragment currently clears the view binding in
onDestroy(), which retains the destroyed view hierarchy; move the binding
cleanup into onDestroyView() by overriding onDestroyView() in
EyeSurgeryFormFragment and setting _binding = null there, and remove the
_binding = null call from onDestroy() so the fragment follows the standard
view-binding lifecycle.
In
`@app/src/main/java/org/piramalswasthya/sakhi/ui/home_activity/all_household/household_members/HouseholdMembersFragment.kt`:
- Around line 496-513: The current showEyeSurgeryBottomSheet registers a
transient NavigationCallback on EyeSurgeryBottomSheetFragment which is lost on
recreation; replace this pattern so navigation survives rotation by switching to
a persistent communication mechanism (either emit results from
EyeSurgeryBottomSheetFragment via setFragmentResult with a clear requestKey and
observe it from HouseholdMembersFragment using
getFragmentResultListener/getFragmentResultLiveData, or use an activity-scoped
ViewModel shared between EyeSurgeryBottomSheetFragment and
HouseholdMembersFragment to post an event); update EyeSurgeryBottomSheetFragment
to stop storing a nullable navigationCallback and instead call setFragmentResult
or post to the shared ViewModel when triggering navigateToEyeSurgeryForm, and
update showEyeSurgeryBottomSheet (and its navigateToEyeSurgeryForm handling) to
observe the fragment result or ViewModel event and perform the
findNavController().navigate(...) call there.
In `@app/src/main/res/layout/fragment_eye_surgery_bottom_sheet.xml`:
- Around line 8-22: The layout renders the same heading twice because the static
TextView and the TextView with id txtName both default to
`@string/cataract_surgery_visits`; update the
fragment_eye_surgery_bottom_sheet.xml so txtName does not duplicate the static
title (e.g., remove the android:text value from the TextView with id txtName or
set it to empty/default placeholder and/or set its visibility to gone until
populated) and ensure any code that populates txtName (referenced by id txtName)
still sets the desired text at runtime.
In `@app/src/main/res/values-as/strings.xml`:
- Around line 884-886: The three new string resources (cataract_surgery,
cataract_surgery_visits, due_date_label) are still English; replace their
English values with Assamese translations for this locale file and preserve the
format placeholder in due_date_label (keep %1$s intact). Update the string
values for cataract_surgery and cataract_surgery_visits to Assamese phrases and
ensure due_date_label becomes the Assamese equivalent of "Due: %1$s" while
retaining the %1$s placeholder.
In `@app/src/main/res/values-hi/strings.xml`:
- Around line 765-767: The three new string resources cataract_surgery,
cataract_surgery_visits, and due_date_label are still in English; translate them
into Hindi and replace the English literals in this locale file with their Hindi
equivalents, keeping the resource names unchanged and preserving the placeholder
token %1$s exactly in due_date_label (e.g., "देय: %1$s") so formatting remains
correct at runtime.
---
Outside diff comments:
In
`@app/src/main/java/org/piramalswasthya/sakhi/adapters/dynamicAdapter/FormRendererAdapter.kt`:
- Around line 1033-1126: The dependent-date constraints (e.g., end_date,
nrc_discharge_date, follow_up_visit_date, visit_date) are only enforced when
opening the dependent field's DatePicker, so when a source date moves forward
the stale dependent values remain valid in the model; update the change handler
that runs when a date is picked (the lambda passed to DatePickerDialog that
calls onValueChanged(field, dateStr)) to revalidate and / or clear any dependent
fields (use getDate("...") and field.fieldId names like
"start_date","end_date","nrc_admission_date","nrc_discharge_date","follow_up_visit_date")
after setting the new value, or alternatively add the same dependent-date
validation to form-level validation invoked before save so stale dependent
values are detected and errors set (field.errorMessage) consistently.
In
`@app/src/main/java/org/piramalswasthya/sakhi/model/dynamicEntity/eye_surgery/EyeSurgeryFormResponseJsonEntity.kt`:
- Around line 10-20: The entity EyeSurgeryFormResponseJsonEntity sets eyeSide
default to an empty string even though eyeSide is part of the unique index and
MIGRATION_57_58 backfills legacy rows to 'LEFT'; change the model so eyeSide has
no empty-string default (remove the default value) or set its default to match
the migration ('LEFT') so lookups/upserts that use benId + eyeSide will match
migrated rows; update EyeSurgeryFormResponseJsonEntity's eyeSide property
accordingly and ensure any callers constructing instances without specifying
eyeSide are updated to provide a correct value.
In
`@app/src/main/java/org/piramalswasthya/sakhi/repositories/dynamicRepo/EyeSurgeryFormRepository.kt`:
- Around line 67-72: The offline fallback currently used by
loadSchemaFromAssets() in EyeSurgeryFormRepository reads "hbnc_form_1stday.json"
which is the wrong schema; update loadSchemaFromAssets() to load the correct eye
surgery offline schema (e.g., "eye_surgery_form.json" or the canonical eye
surgery asset name), keep using FormSchemaDto.fromJson(json) and preserve the
existing try/catch behavior, and ensure the asset filename reference is used
consistently so the repository recovers to the eye surgery form on cache miss.
- Around line 171-177: The current flow dedupes imported surgery visits by month
via insertAllByMonth which calls jsonResponseDao.upsertByMonth, causing
different eyeSide records in the same month to overwrite each other and never
persisting eyeSide on EyeSurgeryFormResponseJsonEntity; change the import path
to upsert each entity individually (avoid
insertAllByMonth/jsonResponseDao.upsertByMonth) so every
EyeSurgeryFormResponseJsonEntity is persisted with its eyeSide populated before
save (or add a dedicated upsert method that keys by visit+eyeSide), and ensure
EyeSurgeryListViewModel.getAvailableEyes() can rely on eyeSide being present on
synced records.
In
`@app/src/main/java/org/piramalswasthya/sakhi/ui/home_activity/all_ben/AllBenFragment.kt`:
- Around line 228-240: The current click handler in AllBenFragment now navigates
to the eye-surgery flow for non-IFA rows without checking beneficiary
deactivation; reintroduce the previous eligibility guard by checking
item.isDeactivate (or !item.isDeactivate) before calling
showEyeSurgeryBottomSheet so deactivated beneficiaries are blocked from starting
a new visit, keeping the existing IFA branch
(AllBenFragmentDirections.actionAllBenFragmentToBenIfaFormFragment) unchanged;
update the lambda that currently receives (item, benId, hhId, isViewMode, isIFA)
to first validate !item.isDeactivate and only then call
showEyeSurgeryBottomSheet(benId, hhId, item.benFullName, item.gender, item.age).
- Around line 108-116: The age-above-40 filter (filterType = 3 from
AllBenFragment::btnOk click) is currently excluded by the DAO because the BenDao
query only checks isDeath = 0; update the DAO methods that filter on death
status (the query methods in BenDao.kt that reference the isDeath column) to
treat the string 'undefined' as non-death as well (e.g., include isDeath =
'undefined' in the predicate or check isDeath IN (...) / use coalesce/default
handling) so beneficiaries with isDeath='undefined' are returned for non-death
filters such as the AGE_ABOVE_40 branch. Ensure any helper/repository methods
that call these DAO queries continue to use filterType 3 unchanged.
---
Nitpick comments:
In `@app/src/main/java/org/piramalswasthya/sakhi/adapters/BenListAdapter.kt`:
- Around line 102-107: The code in BenListAdapter.kt currently sets
binding.btnAbove30.text using a conditional on isMatched with commented
alternatives, but both branches use R.string.cataract_surgery; replace the
entire conditional with a single assignment binding.btnAbove30.text =
binding.root.context.getString(R.string.cataract_surgery), remove the
commented-out strings and the if/else block, and keep any existing tint or style
changes that still depend on isMatched (do not remove those).
In
`@app/src/main/java/org/piramalswasthya/sakhi/ui/home_activity/all_ben/eye_surgery_registration/EyeSurgeryFormViewModel.kt`:
- Around line 79-87: The current saved-data path in EyeSurgeryFormViewModel uses
repository.loadFormResponseJson(benId, formId) which is not eye-scoped and can
prefill the wrong eye; update the logic so saved data is looked up by eyeSide
(e.g. call a repository method that accepts eyeSide or add eyeSide to
loadFormResponseJson parameters) or disable loadSavedData for new-visit flows
and ensure edit flows use loadFormSchemaFromJson(...) or an eye-specific loader;
change references in EyeSurgeryFormViewModel to use the eye-aware loader and/or
set loadSavedData = false for new-visit callers so benId+formId-only lookups are
not used.
In
`@app/src/main/java/org/piramalswasthya/sakhi/ui/home_activity/immunization_due/child_immunization/list/ChildImmunizationListViewModel.kt`:
- Around line 43-59: Remove the two commented-out implementations of
benWithVaccineDetails so only the active flow remains: delete the commented
block that defines benWithVaccineDetails (the one combining pastRecords and
vaccinesFlow) and the duplicate commented block around the later occurrence
(lines referenced in the review), leaving the live benWithVaccineDetails
implementation untouched; this eliminates dead code and prevents confusion when
editing pastRecords, vaccinesFlow or ImmunizationDetailsDomain transformations.
In `@app/src/main/res/layout/fragment_eye_surgery_form.xml`:
- Around line 39-48: The TextView with id tv_ben_name should be constrained to a
single line to prevent long beneficiary names from wrapping and misaligning the
gender/age cells; update the TextView attributes (tv_ben_name) to use
android:maxLines="1" and android:ellipsize="end" (avoid deprecated
android:singleLine) so overflowing names are truncated with an ellipsis while
preserving the existing layout_weight and gravity.
In `@app/src/main/res/layout/rv_item_eye_surgery_visit.xml`:
- Around line 32-37: Replace the hardcoded button text "VIEW" in the
MaterialButton with a string resource: update the MaterialButton (id btnAction)
in rv_item_eye_surgery_visit.xml to use android:text="@string/..." and add the
corresponding entry in strings.xml (e.g., name="view_action" value="View") so
the fallback label is localized and can still be overwritten by the adapter at
runtime.
In `@app/src/main/res/navigation/nav_home.xml`:
- Around line 2233-2240: The nav graph declares EyeSurgeryBottomSheetFragment as
a <fragment> and an action
(action_eyeSurgeryBottomSheet_to_eyeSurgeryFormFragment) but the sheet is
actually shown via show(...), so the nav node is mismatched; update the nav to
model EyeSurgeryBottomSheetFragment as a <dialog> destination if it extends
BottomSheetDialogFragment (preserve any required arguments on that <dialog>),
and remove or adjust the action/action target
(action_eyeSurgeryBottomSheet_to_eyeSurgeryFormFragment ->
eyeSurgeryFormFragment) since the sheet is not navigated via NavController;
alternatively, if you intend to navigate, change the fragment launch call to
NavController.navigate(...) instead of show(...) and keep the <fragment> entry.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 66dcff2e-37f3-4b58-926f-672c2166e6a1
📒 Files selected for processing (28)
app/src/main/java/org/piramalswasthya/sakhi/adapters/BenListAdapter.ktapp/src/main/java/org/piramalswasthya/sakhi/adapters/dynamicAdapter/FormRendererAdapter.ktapp/src/main/java/org/piramalswasthya/sakhi/database/room/InAppDb.ktapp/src/main/java/org/piramalswasthya/sakhi/database/room/dao/BenDao.ktapp/src/main/java/org/piramalswasthya/sakhi/database/room/dao/dynamicSchemaDao/EyeSurgeryFormResponseJsonDao.ktapp/src/main/java/org/piramalswasthya/sakhi/model/Immunization.ktapp/src/main/java/org/piramalswasthya/sakhi/model/dynamicEntity/eye_surgery/EyeSurgeryFormResponseJsonEntity.ktapp/src/main/java/org/piramalswasthya/sakhi/repositories/dynamicRepo/EyeSurgeryFormRepository.ktapp/src/main/java/org/piramalswasthya/sakhi/ui/BindingUtils.ktapp/src/main/java/org/piramalswasthya/sakhi/ui/home_activity/all_ben/AllBenFragment.ktapp/src/main/java/org/piramalswasthya/sakhi/ui/home_activity/all_ben/eye_surgery_registration/EyeSurgeryFormFragment.ktapp/src/main/java/org/piramalswasthya/sakhi/ui/home_activity/all_ben/eye_surgery_registration/EyeSurgeryFormViewModel.ktapp/src/main/java/org/piramalswasthya/sakhi/ui/home_activity/all_ben/eye_surgery_registration/eyeBottomsheet/EyeSurgeryBottomSheetFragment.ktapp/src/main/java/org/piramalswasthya/sakhi/ui/home_activity/all_ben/eye_surgery_registration/eyeBottomsheet/EyeSurgeryListViewModel.ktapp/src/main/java/org/piramalswasthya/sakhi/ui/home_activity/all_ben/eye_surgery_registration/eyeBottomsheet/adapter/EyeSurgeryVisitListAdapter.ktapp/src/main/java/org/piramalswasthya/sakhi/ui/home_activity/all_ben/eye_surgery_registration/eyeBottomsheet/model/EyeSurgeryVisitOption.ktapp/src/main/java/org/piramalswasthya/sakhi/ui/home_activity/all_household/household_members/HouseholdMembersFragment.ktapp/src/main/java/org/piramalswasthya/sakhi/ui/home_activity/immunization_due/child_immunization/list/ChildImmunizationListViewModel.ktapp/src/main/res/drawable/ic_eye.xmlapp/src/main/res/layout/fragment_eye_surgery_bottom_sheet.xmlapp/src/main/res/layout/fragment_eye_surgery_form.xmlapp/src/main/res/layout/rv_item_eye_surgery_visit.xmlapp/src/main/res/layout/rv_item_imm_vaccine.xmlapp/src/main/res/navigation/nav_home.xmlapp/src/main/res/values-as/strings.xmlapp/src/main/res/values-hi/strings.xmlapp/src/main/res/values/strings.xmlapp/src/main/res/values/strings_hrp.xml
💤 Files with no reviewable changes (1)
- app/src/main/res/values/strings_hrp.xml
app/src/main/java/org/piramalswasthya/sakhi/database/room/dao/BenDao.kt
Outdated
Show resolved
Hide resolved
...rg/piramalswasthya/sakhi/database/room/dao/dynamicSchemaDao/EyeSurgeryFormResponseJsonDao.kt
Outdated
Show resolved
Hide resolved
...src/main/java/org/piramalswasthya/sakhi/repositories/dynamicRepo/EyeSurgeryFormRepository.kt
Show resolved
Hide resolved
...alswasthya/sakhi/ui/home_activity/all_ben/eye_surgery_registration/EyeSurgeryFormFragment.kt
Show resolved
Hide resolved
...lswasthya/sakhi/ui/home_activity/all_household/household_members/HouseholdMembersFragment.kt
Show resolved
Hide resolved
|




📋 Description
JIRA ID: FLW-829
Cataract surgery changes done
✅ Type of Change
ℹ️ Additional Information
Please describe how the changes were tested, and include any relevant screenshots, logs, or other information that provides additional context.
Summary by CodeRabbit
Release Notes
New Features
Updates