Initial commits for change of checkbox logic for multiselection case …#255
Initial commits for change of checkbox logic for multiselection case …#255Kunalbizbrolly wants to merge 1 commit intorelease-5.0from
Conversation
📝 WalkthroughWalkthroughThis PR introduces a new reactive force-refresh mechanism for form mutations, refactors ANC patient querying from count-based to patient-list-based filtering, implements locale-neutral "None" option handling for checkbox selections, converts birth weight representation to milliliter units, and updates localized string arrays. ChangesForm Element Mutation & Refresh Handling
ANC Patient Querying Refactoring
Sequence DiagramsequenceDiagram
participant FormElement as FormElement<br/>(Checkbox)
participant Dataset as Dataset<br/>(ChildRegistration)
participant Fragment as ChildRegistration<br/>Fragment
participant Adapter as FormAdapter<br/>(RecyclerView)
participant UI as UI<br/>(rendered row)
rect rgba(100, 150, 200, 0.5)
Note over FormElement,UI: User selects checkbox in a form row
FormElement->>Dataset: handleListOnValueChanged(element)
Dataset->>FormElement: mutate element.value in-place<br/>(clear/add options)
Dataset->>Dataset: forceRefreshId(elementId)
Dataset->>Dataset: _forceRefreshIdFlow.tryEmit(elementId)
end
rect rgba(150, 100, 200, 0.5)
Note over Dataset,UI: Fragment detects mutation signal
Dataset-->>Fragment: emit(elementId) via forceRefreshIdFlow
Fragment->>Fragment: collect emission in lifecycleScope
Fragment->>Adapter: notifyItemChanged(position)
end
rect rgba(100, 200, 150, 0.5)
Note over Adapter,UI: Adapter rebinds row with updated value
Adapter->>FormElement: onBindViewHolder (rebind)
FormElement->>UI: render updated element.value
UI-->>Fragment: user sees new state
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/org/piramalswasthya/cho/ui/home/rmncha/maternal_health/ANCVisitsFragment.kt (1)
128-152:⚠️ Potential issue | 🟠 Major
observePatients()lacks thepregnantWomanDeliveredguard thatgetAncDueList()uses, creating an inconsistency.
getAncDueList()explicitly skips delivered women:if (ancRecords.any { it.pregnantWomanDelivered == true }) return@mapNotNull nullBut
observePatients()andgetANCCount()rely exclusively on the DAO'sstatusOfWomanID IN (2)predicate, with no check for delivery. IfstatusOfWomanIDis not reliably updated when a delivery outcome is recorded, delivered women could still appear in the ANC listing.Note: The field
pregnantWomanDeliveredexists only onPregnantWomanAncCache, not on thePregnantWomanRegistrationCache(PWR) domain model, so the suggested guard would need to either:
- Access the ANC records directly (like
getAncDueList()does), or- Check
patient.patient.statusOfWomanID != 3(post-natal status) to avoid the field path issue.Verify that
statusOfWomanIDis consistently updated to 3 on delivery; if not, add a defensive check similar togetAncDueList()or use the status ID condition to suppress post-delivery women from the ANC list.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/org/piramalswasthya/cho/ui/home/rmncha/maternal_health/ANCVisitsFragment.kt` around lines 128 - 152, observePatients() currently doesn't exclude delivered women like getAncDueList() does; update observePatients() (and similarly getANCCount()) to defensively filter out post-delivery patients by either fetching the patient's ANC records and skipping when any PregnantWomanAncCache has pregnantWomanDelivered == true (same approach as getAncDueList()), or by ensuring patient.patient.statusOfWomanID != 3 before including them (if you can guarantee statusOfWomanID is updated). Locate the filtering block in observePatients() (the map/filter pipeline that builds allPatients) and add the chosen guard there; mirror the same guard in getANCCount() to keep listing/counts consistent. Ensure you reference PregnantWomanAncCache when using the ANC-records approach so you can check pregnantWomanDelivered.
🧹 Nitpick comments (1)
app/src/main/java/org/piramalswasthya/cho/database/room/dao/MaternalHealthDao.kt (1)
104-115: ⚡ Quick win
LEFT OUTER JOINallows patients without a PWR record into the ANC result set, contradicting the KDoc.The
WHERE (pwr.patientID IS NULL OR pwr.active = 1)condition lets anyPATIENTrow withstatusOfWomanID = 2but no pregnancy registration pass through (theNULLbranch). The KDoc at line 87 states "Returns only patients who have an active pregnancy registration," which is incorrect for the null-join case.These unregistered patients are ultimately discarded by the
hasActivePWRpredicate in bothANCVisitsFragment.observePatients()andgetANCCount(), so there is no visible bug. However, Room still hydrates the extra rows and the@Relationfetches their (empty) PWR lists.Since ANC eligibility explicitly requires an active PWR, consider replacing the
LEFT OUTER JOINwith anINNER JOINand dropping theNULLbranch:♻️ Proposed refactor
`@Transaction` `@Query`(""" SELECT DISTINCT p.* FROM PATIENT p - LEFT OUTER JOIN PREGNANCY_REGISTER pwr ON p.patientID = pwr.patientID - WHERE (pwr.patientID IS NULL OR pwr.active = 1) - AND p.genderID = 2 + INNER JOIN PREGNANCY_REGISTER pwr ON p.patientID = pwr.patientID + WHERE pwr.active = 1 + AND p.genderID = 2 AND p.maritalStatusID = 2 AND p.statusOfWomanID IN (2) AND p.age BETWEEN 15 AND 49 ORDER BY p.registrationDate DESC """) fun getAllPatientsWithANC(): Flow<List<PatientWithPwrCache>>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/org/piramalswasthya/cho/database/room/dao/MaternalHealthDao.kt` around lines 104 - 115, The query in getAllPatientsWithANC currently uses a LEFT OUTER JOIN and a WHERE clause that allows rows with no pregnancy registration (pwr.patientID IS NULL), which contradicts the KDoc saying only patients with an active pregnancy registration should be returned; change the JOIN to an INNER JOIN and remove the NULL branch so the predicate becomes (pwr.active = 1) only, keeping the other filters (p.genderID, p.maritalStatusID, p.statusOfWomanID, p.age) and the ORDER BY intact to ensure Room only hydrates patients that have an active PREGNANCY_REGISTER record.
🤖 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/cho/configuration/ChildRegistrationDataset.kt`:
- Around line 774-783: The checkbox callback currently conflates position and
checked-state by using signed index values (as enforced by FormInputAdapter's
contract) which makes position 0 ambiguous; update the encoding/decoding so
position 0 is distinguishable—either change the adapter/callback signature to
accept (position: Int, checked: Boolean) or switch to encoding as value =
(position + 1) * (if checked then 1 else -1) and decode with realIndex =
abs(value) - 1 and isChecked = value > 0; apply the corresponding decoding in
ChildRegistrationDataset where birthDoseVaccinesGiven handlers compute
realIndex/isChecked (replace current realIndex/ isChecked logic) and update any
places that construct the signed index to use the new (position+1) scheme or the
separate parameters.
In `@app/src/main/java/org/piramalswasthya/cho/configuration/Dataset.kt`:
- Around line 117-126: The updateList path currently mutates a FormElement's
errorText in place and emits only a shallow-copied list so validation-only
changes can remain visually stale; update updateList (or the caller that sets
errorStateChanged and updateIndex) to call forceRefreshId(id) when
errorStateChanged is true and updateIndex == -1 (or alternatively replace the
mutated FormElement instance before emitting the list) so the existing
_forceRefreshIdFlow/forceRefreshIdFlow path is triggered; reference the existing
helper forceRefreshId(id), the boolean errorStateChanged, and the updateIndex
check to implement this wiring.
In
`@app/src/main/java/org/piramalswasthya/cho/configuration/NeonatalOutcomeDataset.kt`:
- Around line 354-364: The checkbox state encoding is ambiguous for the first
option because FormInputAdapter's callback uses index*(if(b)1 else -1),
producing 0 for index=0 whether checked or not; update the call to
onValueChanged in FormInputAdapter (where formValueListener?.onValueChanged is
invoked) to encode as (index + 1) * (if (b) 1 else -1) so position 0 maps to ±1
and checked/unchecked are distinguishable, and ensure NeonatalOutcomeDataset.kt
logic that reads the encoded value (variables like realIndex, isChecked)
continues to decode using sign and absolute value as before.
- Around line 409-415: The restore path is still checking
saved.birthDoseVaccinesGiven?.contains("None") which breaks for non-English
locales; update restoreConditionalFields() to perform locale-neutral comparisons
by mapping the saved CSV values through the Dataset's resources/englishResources
lookup (the same way noneLocalized is derived) or by comparing against the
englishResources token instead of hardcoded "None", so that the logic that
computes noneIsSelected (using noneLocalized, birthDoseVaccinesGiven and
toggleDependant) and the restoreConditionalFields() branch that toggles
reasonForNoVaccines use the same resource-backed token resolution (use
Dataset.resources or englishResources and the same lookup function used
elsewhere) to detect the saved "None" value in any locale.
In
`@app/src/main/java/org/piramalswasthya/cho/repositories/MaternalHealthRepo.kt`:
- Around line 373-390: getANCCount() should guard against records where
pregnantWomanDelivered is true before counting; inside the Flow mapping over
getAllPatientsWithANC() (the lambda receiving "list"), add a check like if
(list.any { it.pregnantWomanDelivered == true }) return 0 (use the proper
return-from-lambda form, e.g. return@map 0) so delivered women are excluded even
if DAO/Room updates are inconsistent; update the logic in getANCCount() (the
mapping lambda over getAllPatientsWithANC()) to perform this check before
converting items with asDomainModel() and counting.
In
`@app/src/main/java/org/piramalswasthya/cho/ui/commons/eligible_couple/tracking/form/EligibleCoupleTrackingFormViewModel.kt`:
- Around line 106-115: When getLoggedInUser() returns null the code currently
writes empty strings into EligibleCoupleTrackingCache.createdBy/updatedBy which
can cause silent sync failures; change the logic in the block that builds
eligibleCoupleTracking (around getLoggedInUser(), EligibleCoupleTrackingCache,
createdBy/updatedBy) to: if asha is null, emit a warning log (using the app
logger or Log/TAG) noting missing logged-in user and do NOT construct/persist a
cache record with empty audit fields — instead set the form/save state to a
failure state (e.g., SyncState.SAVE_FAILED or a local SAVE_FAILED equivalent)
and short-circuit the save flow so the form cannot be submitted without
attribution; if asha is present, continue to populate createdBy/updatedBy with
asha.userName as before.
---
Outside diff comments:
In
`@app/src/main/java/org/piramalswasthya/cho/ui/home/rmncha/maternal_health/ANCVisitsFragment.kt`:
- Around line 128-152: observePatients() currently doesn't exclude delivered
women like getAncDueList() does; update observePatients() (and similarly
getANCCount()) to defensively filter out post-delivery patients by either
fetching the patient's ANC records and skipping when any PregnantWomanAncCache
has pregnantWomanDelivered == true (same approach as getAncDueList()), or by
ensuring patient.patient.statusOfWomanID != 3 before including them (if you can
guarantee statusOfWomanID is updated). Locate the filtering block in
observePatients() (the map/filter pipeline that builds allPatients) and add the
chosen guard there; mirror the same guard in getANCCount() to keep
listing/counts consistent. Ensure you reference PregnantWomanAncCache when using
the ANC-records approach so you can check pregnantWomanDelivered.
---
Nitpick comments:
In
`@app/src/main/java/org/piramalswasthya/cho/database/room/dao/MaternalHealthDao.kt`:
- Around line 104-115: The query in getAllPatientsWithANC currently uses a LEFT
OUTER JOIN and a WHERE clause that allows rows with no pregnancy registration
(pwr.patientID IS NULL), which contradicts the KDoc saying only patients with an
active pregnancy registration should be returned; change the JOIN to an INNER
JOIN and remove the NULL branch so the predicate becomes (pwr.active = 1) only,
keeping the other filters (p.genderID, p.maritalStatusID, p.statusOfWomanID,
p.age) and the ORDER BY intact to ensure Room only hydrates patients that have
an active PREGNANCY_REGISTER record.
🪄 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: bb88ecd7-0d63-444d-ac5c-470a48ba29d1
📒 Files selected for processing (12)
app/src/main/java/org/piramalswasthya/cho/configuration/ChildBeneficiaryRegistrationDataset.ktapp/src/main/java/org/piramalswasthya/cho/configuration/ChildRegistrationDataset.ktapp/src/main/java/org/piramalswasthya/cho/configuration/Dataset.ktapp/src/main/java/org/piramalswasthya/cho/configuration/NeonatalOutcomeDataset.ktapp/src/main/java/org/piramalswasthya/cho/configuration/RMNCHAIconDataset.ktapp/src/main/java/org/piramalswasthya/cho/database/room/dao/MaternalHealthDao.ktapp/src/main/java/org/piramalswasthya/cho/repositories/MaternalHealthRepo.ktapp/src/main/java/org/piramalswasthya/cho/ui/commons/eligible_couple/tracking/form/EligibleCoupleTrackingFormViewModel.ktapp/src/main/java/org/piramalswasthya/cho/ui/home/rmncha/child_care/ChildRegistrationFragment.ktapp/src/main/java/org/piramalswasthya/cho/ui/home/rmncha/maternal_health/ANCVisitsFragment.ktapp/src/main/res/values-as/strings.xmlapp/src/main/res/values-hi/strings.xml
💤 Files with no reviewable changes (2)
- app/src/main/res/values-hi/strings.xml
- app/src/main/res/values-as/strings.xml
| val realIndex = if (index < 0) -index else index | ||
| val localizedEntries = birthDoseVaccinesGiven.entries | ||
| // Locale-neutral "None" identification. | ||
| val noneIndex = englishResources | ||
| .getStringArray(birthDoseVaccinesGiven.arrayId) | ||
| .indexOf("None") | ||
| val noneLocalized = localizedEntries?.getOrNull(noneIndex) | ||
| val clickedOption = localizedEntries?.getOrNull(realIndex) | ||
| val isNoneOption = noneIndex >= 0 && realIndex == noneIndex | ||
| val isChecked = index > 0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -i 'FormInputAdapter.kt' | while read -r f; do
echo "== $f =="
rg -n -C4 'FormValueListener|CheckBox|onCheckedChangeListener|setOnCheckedChangeListener|index' "$f"
doneRepository: PSMRI/HWC-Mobile-App
Length of output: 11782
🏁 Script executed:
cat -n app/src/main/java/org/piramalswasthya/cho/configuration/ChildRegistrationDataset.kt | sed -n '770,800p'Repository: PSMRI/HWC-Mobile-App
Length of output: 1762
🏁 Script executed:
cat -n app/src/main/java/org/piramalswasthya/cho/configuration/ChildRegistrationDataset.kt | sed -n '840,860p'Repository: PSMRI/HWC-Mobile-App
Length of output: 1260
Fix position–checked-state ambiguity in checkbox handlers.
Lines 783 and 855 use index > 0 to detect a checked event, but the callback contract from FormInputAdapter (line 546) encodes both position and checked-state using the sign of the same integer: index * (if (b) 1 else -1). This makes position 0 impossible to distinguish: both a checked and unchecked first option produce 0, so isChecked = index > 0 is always false when position is 0. The selection handler at lines 785–797 and 857–870 then silently skips the first checkbox option.
Switch the callback contract to separate position and checked-state, either using (position, checked) as distinct parameters or encoding as ±(position + 1) to shift position space away from zero.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/org/piramalswasthya/cho/configuration/ChildRegistrationDataset.kt`
around lines 774 - 783, The checkbox callback currently conflates position and
checked-state by using signed index values (as enforced by FormInputAdapter's
contract) which makes position 0 ambiguous; update the encoding/decoding so
position 0 is distinguishable—either change the adapter/callback signature to
accept (position: Int, checked: Boolean) or switch to encoding as value =
(position + 1) * (if checked then 1 else -1) and decode with realIndex =
abs(value) - 1 and isChecked = value > 0; apply the corresponding decoding in
ChildRegistrationDataset where birthDoseVaccinesGiven handlers compute
realIndex/isChecked (replace current realIndex/ isChecked logic) and update any
places that construct the signed index to use the new (position+1) scheme or the
separate parameters.
| /** Signals that a specific FormElement's row needs the adapter to call | ||
| * notifyItemChanged. Used when we mutate a FormElement's `value` in place | ||
| * (e.g. "None" mutual exclusion) — DiffUtil cannot detect it because the | ||
| * list still holds the same reference, so the fragment must force a | ||
| * rebind explicitly. */ | ||
| private val _forceRefreshIdFlow = MutableSharedFlow<Int>(extraBufferCapacity = 8) | ||
| val forceRefreshIdFlow = _forceRefreshIdFlow.asSharedFlow() | ||
|
|
||
| protected fun forceRefreshId(id: Int) { | ||
| _forceRefreshIdFlow.tryEmit(id) |
There was a problem hiding this comment.
Wire this refresh path into validation-only row updates too.
This helper only fixes the branches that explicitly call it. updateList() still mutates errorText in place and emits only a shallow copy of list, so rows whose state changed only because of validation can still stay visually stale. Please also trigger forceRefreshId(formId) when errorStateChanged is true and updateIndex == -1, or replace the mutated FormElement instance before emitting the list.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/org/piramalswasthya/cho/configuration/Dataset.kt` around
lines 117 - 126, The updateList path currently mutates a FormElement's errorText
in place and emits only a shallow-copied list so validation-only changes can
remain visually stale; update updateList (or the caller that sets
errorStateChanged and updateIndex) to call forceRefreshId(id) when
errorStateChanged is true and updateIndex == -1 (or alternatively replace the
mutated FormElement instance before emitting the list) so the existing
_forceRefreshIdFlow/forceRefreshIdFlow path is triggered; reference the existing
helper forceRefreshId(id), the boolean errorStateChanged, and the updateIndex
check to implement this wiring.
| val realIndex = if (index < 0) -index else index | ||
| val localizedEntries = newbornComplications.entries | ||
| // Identify "None" by its position in the English array so the | ||
| // logic works in every locale (Hindi, Assamese, etc.). | ||
| val noneIndex = englishResources | ||
| .getStringArray(newbornComplications.arrayId) | ||
| .indexOf("None") | ||
| val noneLocalized = localizedEntries?.getOrNull(noneIndex) | ||
| val clickedOption = localizedEntries?.getOrNull(realIndex) ?: return -1 | ||
| val isNoneOption = noneIndex >= 0 && realIndex == noneIndex | ||
| val isChecked = index > 0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -i 'FormInputAdapter.kt' | while read -r f; do
echo "== $f =="
rg -n -C4 'FormValueListener|CheckBox|onCheckedChangeListener|setOnCheckedChangeListener|index' "$f"
doneRepository: PSMRI/HWC-Mobile-App
Length of output: 11782
Fix the checkbox callback encoding to distinguish the first option's checked state.
The checkbox callback in FormInputAdapter.kt (line 546) uses index * (if (b) 1 else -1), which is ambiguous for the first option (index=0): both checked and unchecked produce 0. This causes NeonatalOutcomeDataset.kt (lines 364 and 393) to misinterpret the state, since isChecked = index > 0 treats 0 as always unchecked.
Change the callback to encode position as (index + 1) * (if (b) 1 else -1) so position 0 maps to ±1, making checked and unchecked states distinguishable.
FormInputAdapter.kt line 546
formValueListener?.onValueChanged(
item, (index + 1) * (if (b) 1 else -1) // Changed from: index * (if (b) 1 else -1)
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/org/piramalswasthya/cho/configuration/NeonatalOutcomeDataset.kt`
around lines 354 - 364, The checkbox state encoding is ambiguous for the first
option because FormInputAdapter's callback uses index*(if(b)1 else -1),
producing 0 for index=0 whether checked or not; update the call to
onValueChanged in FormInputAdapter (where formValueListener?.onValueChanged is
invoked) to encode as (index + 1) * (if (b) 1 else -1) so position 0 maps to ±1
and checked/unchecked are distinguishable, and ensure NeonatalOutcomeDataset.kt
logic that reads the encoded value (variables like realIndex, isChecked)
continues to decode using sign and absolute value as before.
| val noneIsSelected = noneLocalized != null && (birthDoseVaccinesGiven.value ?: "") | ||
| .split(",") | ||
| .map { it.trim() } | ||
| .any { it == noneLocalized } | ||
| toggleDependant( | ||
| source = birthDoseVaccinesGiven, | ||
| condition = birthDoseVaccinesGiven.value?.contains("None") == true, | ||
| condition = noneIsSelected, |
There was a problem hiding this comment.
Keep the saved-value restore path locale-neutral too.
This branch now toggles reasonForNoVaccines with noneLocalized, but restoreConditionalFields() at Line 234 still checks saved.birthDoseVaccinesGiven?.contains("None"). Reopening a saved non-English form will skip the dependant field even when localized “None” was selected.
Suggested fix
- if (saved.birthDoseVaccinesGiven?.contains("None") == true) {
+ val noneIndex = englishResources
+ .getStringArray(birthDoseVaccinesGiven.arrayId)
+ .indexOf("None")
+ val noneLocalized = birthDoseVaccinesGiven.entries?.getOrNull(noneIndex)
+ if (!noneLocalized.isNullOrBlank() &&
+ saved.birthDoseVaccinesGiven
+ ?.split(",")
+ ?.any { it.trim() == noneLocalized } == true
+ ) {
formElements.add(formElements.indexOf(birthDoseVaccinesGiven) + 1, reasonForNoVaccines)
}Based on learnings: In Dataset implementations in org.piramalswasthya.cho.configuration (Kotlin/Android), always use Dataset.resources (and englishResources where applicable) for all user-facing titles/options/alerts/comparisons.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/org/piramalswasthya/cho/configuration/NeonatalOutcomeDataset.kt`
around lines 409 - 415, The restore path is still checking
saved.birthDoseVaccinesGiven?.contains("None") which breaks for non-English
locales; update restoreConditionalFields() to perform locale-neutral comparisons
by mapping the saved CSV values through the Dataset's resources/englishResources
lookup (the same way noneLocalized is derived) or by comparing against the
englishResources token instead of hardcoded "None", so that the logic that
computes noneIsSelected (using noneLocalized, birthDoseVaccinesGiven and
toggleDependant) and the restoreConditionalFields() branch that toggles
reasonForNoVaccines use the same resource-backed token resolution (use
Dataset.resources or englishResources and the same lookup function used
elsewhere) to detect the saved "None" value in any locale.
| fun getANCCount(): Flow<Int> { | ||
| val ancEligibilityWindowMillis = 35L * 24 * 60 * 60 * 1000 | ||
| return maternalHealthDao.getANCEligibleCount(System.currentTimeMillis() - ancEligibilityWindowMillis) | ||
| return getAllPatientsWithANC().map { list -> | ||
| val now = System.currentTimeMillis() | ||
| val cutoff = now - ancEligibilityWindowMillis | ||
| list.map { it.asDomainModel() } | ||
| .count { domain -> | ||
| val hasActivePWR = domain.pwr != null && domain.isActive() | ||
| val isFemale = domain.patient.genderID == 2 | ||
| val age = domain.patient.age ?: 0 | ||
| val isReproductiveAge = age in 15..49 | ||
| val isEligibleForANC = domain.pwr?.lmpDate?.let { lmp -> | ||
| lmp <= cutoff | ||
| } ?: false | ||
| hasActivePWR && isFemale && isReproductiveAge && isEligibleForANC | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find every place statusOfWomanID is written/updated in Kotlin source
rg -n "statusOfWomanID" --type=kt -A 3 -B 1Repository: PSMRI/HWC-Mobile-App
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Find Kotlin files first, then search for statusOfWomanID usage
echo "=== Searching for statusOfWomanID in Kotlin files ==="
fd -e kt | head -50
echo ""
echo "=== Searching for statusOfWomanID mentions ==="
rg -n "statusOfWomanID" -A 3 -B 1 | head -100Repository: PSMRI/HWC-Mobile-App
Length of output: 16927
🏁 Script executed:
#!/bin/bash
# Search for delivery-related code and pregnantWomanDelivered usage
echo "=== Searching for pregnantWomanDelivered ==="
rg -n "pregnantWomanDelivered" -A 2 -B 1
echo ""
echo "=== Searching for delivery recording/outcome code ==="
rg -n "delivery|Delivery|DELIVERY" --type kotlin -i | grep -i "record\|save\|update\|insert" | head -20Repository: PSMRI/HWC-Mobile-App
Length of output: 12150
🏁 Script executed:
#!/bin/bash
# Find the MaternalHealthRepo file and examine delivery-related methods
echo "=== Finding MaternalHealthRepo file ==="
fd -n "MaternalHealthRepo.kt"
echo ""
echo "=== Searching for methods that update delivery status ==="
rg -n "fun.*[Dd]eliver" -A 5Repository: PSMRI/HWC-Mobile-App
Length of output: 50625
🏁 Script executed:
#!/bin/bash
# Look at the getANCCount method and getAllPatientsWithANC method
echo "=== Examining MaternalHealthRepo.kt around getANCCount ==="
cat -n app/src/main/java/org/piramalswasthya/cho/repositories/MaternalHealthRepo.kt | sed -n '370,410p'
echo ""
echo "=== Find getAllPatientsWithANC definition ==="
rg -n "fun getAllPatientsWithANC" -A 20Repository: PSMRI/HWC-Mobile-App
Length of output: 7090
🏁 Script executed:
#!/bin/bash
# Find the STATUS_POST_NATAL_MOTHER constant
echo "=== Find STATUS_POST_NATAL_MOTHER constant ==="
rg -n "STATUS_POST_NATAL_MOTHER" -B 2 -A 2
echo ""
echo "=== Check what value statusOfWomanID should be for pregnant women ==="
rg -n "statusOfWomanID.*=.*2\|IN.*2\)" -B 1 -A 1 | head -40Repository: PSMRI/HWC-Mobile-App
Length of output: 3163
🏁 Script executed:
#!/bin/bash
# Find getAncDueList and getAncCompletedList to see the pregnantWomanDelivered check
echo "=== Examine getAncDueList implementation ==="
rg -n "fun getAncDueList" -A 40 | head -60
echo ""
echo "=== Examine getAncCompletedList implementation ==="
rg -n "fun getAncCompletedList" -A 40 | head -60Repository: PSMRI/HWC-Mobile-App
Length of output: 11100
🏁 Script executed:
#!/bin/bash
# Look at where pregnantWomanDelivered is set to understand if it's atomic with statusOfWomanID
echo "=== Check PwAncFormViewModel where pregnantWomanDelivered and statusOfWomanID are updated ==="
rg -n "pregnantWomanDelivered.*true" -B 5 -A 5 | head -50
echo ""
echo "=== Verify getAllPatientsWithANC DAO query - check if it filters statusOfWomanID ==="
cat -n app/src/main/java/org/piramalswasthya/cho/database/room/dao/MaternalHealthDao.kt | sed -n '100,135p'Repository: PSMRI/HWC-Mobile-App
Length of output: 8273
getANCCount() lacks pregnantWomanDelivered check—delivered women risk being incorrectly included if delivery status updates are not reliable.
The DAO query getAllPatientsWithANC() does filter by statusOfWomanID IN (2), which should exclude delivered women when statusOfWomanID is updated to 3. However, getANCCount() itself performs no pregnantWomanDelivered check, unlike getAncDueList() (line 182) and getAncCompletedList() (line 233), which both explicitly filter delivered women.
The concern is valid: if the delivery recording flow fails to update statusOfWomanID atomically or reliably in Room, delivered women will pass the DAO filter and be counted. The pattern of checking pregnantWomanDelivered exists elsewhere in the codebase precisely because this is recognized as necessary.
Add if (ancRecords.any { it.pregnantWomanDelivered == true }) return 0 at the start of getANCCount() to match the pattern used in due/completed lists and guard against incomplete delivery status updates.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/org/piramalswasthya/cho/repositories/MaternalHealthRepo.kt`
around lines 373 - 390, getANCCount() should guard against records where
pregnantWomanDelivered is true before counting; inside the Flow mapping over
getAllPatientsWithANC() (the lambda receiving "list"), add a check like if
(list.any { it.pregnantWomanDelivered == true }) return 0 (use the proper
return-from-lambda form, e.g. return@map 0) so delivered women are excluded even
if DAO/Room updates are inconsistent; update the logic in getANCCount() (the
mapping lambda over getAllPatientsWithANC()) to perform this check before
converting items with asDomainModel() and counting.
| val asha = userRepo.getLoggedInUser() | ||
| val ben = patientRepo.getPatientDisplay(patientID)?.also { ben -> | ||
| _benName.value = | ||
| "${ben.patient.firstName} ${if (ben.patient.lastName == null) "" else ben.patient.lastName}" | ||
| _benAgeGender.value = "${ben.patient.age} ${ben.ageUnit?.name} | ${ben.gender?.genderName}" | ||
| eligibleCoupleTracking = EligibleCoupleTrackingCache( | ||
| patientID = patientID, | ||
| syncState = SyncState.UNSYNCED, | ||
| createdBy = asha.userName, | ||
| updatedBy = asha.userName, | ||
| createdBy = asha?.userName ?: "", | ||
| updatedBy = asha?.userName ?: "", |
There was a problem hiding this comment.
Silent empty-string audit fields when getLoggedInUser() returns null may cause undetectable sync failures.
The null-safe guard prevents the crash that !! would cause, but substituting "" for createdBy/updatedBy silently persists records with no attribution. In a healthcare sync context:
- Backend APIs typically validate
createdByis non-empty; a blank value will likely cause a silent server-side rejection during sync, with no user-visible feedback. - There is no log entry to diagnose when or why the user was unavailable, making field-level debugging impossible.
At minimum, emit a warning log when asha is null and consider posting a SAVE_FAILED-equivalent state to prevent the form from being submitted with empty audit fields.
🛡️ Proposed fix — warn on null user and guard record creation
- val asha = userRepo.getLoggedInUser()
+ val asha = userRepo.getLoggedInUser()
+ if (asha == null) {
+ Timber.w("getLoggedInUser() returned null during ECT init for patient $patientID — createdBy/updatedBy will be empty")
+ }
val ben = patientRepo.getPatientDisplay(patientID)?.also { ben ->
_benName.value = ...
eligibleCoupleTracking = EligibleCoupleTrackingCache(
patientID = patientID,
syncState = SyncState.UNSYNCED,
- createdBy = asha?.userName ?: "",
- updatedBy = asha?.userName ?: "",
+ createdBy = asha?.userName ?: "", // empty only if session is unexpectedly null
+ updatedBy = asha?.userName ?: "",
)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val asha = userRepo.getLoggedInUser() | |
| val ben = patientRepo.getPatientDisplay(patientID)?.also { ben -> | |
| _benName.value = | |
| "${ben.patient.firstName} ${if (ben.patient.lastName == null) "" else ben.patient.lastName}" | |
| _benAgeGender.value = "${ben.patient.age} ${ben.ageUnit?.name} | ${ben.gender?.genderName}" | |
| eligibleCoupleTracking = EligibleCoupleTrackingCache( | |
| patientID = patientID, | |
| syncState = SyncState.UNSYNCED, | |
| createdBy = asha.userName, | |
| updatedBy = asha.userName, | |
| createdBy = asha?.userName ?: "", | |
| updatedBy = asha?.userName ?: "", | |
| val asha = userRepo.getLoggedInUser() | |
| if (asha == null) { | |
| Timber.w("getLoggedInUser() returned null during ECT init for patient $patientID — createdBy/updatedBy will be empty") | |
| } | |
| val ben = patientRepo.getPatientDisplay(patientID)?.also { ben -> | |
| _benName.value = | |
| "${ben.patient.firstName} ${if (ben.patient.lastName == null) "" else ben.patient.lastName}" | |
| _benAgeGender.value = "${ben.patient.age} ${ben.ageUnit?.name} | ${ben.gender?.genderName}" | |
| eligibleCoupleTracking = EligibleCoupleTrackingCache( | |
| patientID = patientID, | |
| syncState = SyncState.UNSYNCED, | |
| createdBy = asha?.userName ?: "", // empty only if session is unexpectedly null | |
| updatedBy = asha?.userName ?: "", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/org/piramalswasthya/cho/ui/commons/eligible_couple/tracking/form/EligibleCoupleTrackingFormViewModel.kt`
around lines 106 - 115, When getLoggedInUser() returns null the code currently
writes empty strings into EligibleCoupleTrackingCache.createdBy/updatedBy which
can cause silent sync failures; change the logic in the block that builds
eligibleCoupleTracking (around getLoggedInUser(), EligibleCoupleTrackingCache,
createdBy/updatedBy) to: if asha is null, emit a warning log (using the app
logger or Log/TAG) noting missing logged-in user and do NOT construct/persist a
cache record with empty audit fields — instead set the form/save state to a
failure state (e.g., SyncState.SAVE_FAILED or a local SAVE_FAILED equivalent)
and short-circuit the save flow so the form cannot be submitted without
attribution; if asha is present, continue to populate createdBy/updatedBy with
asha.userName as before.
1 participant
…(none),
📋 Description
JIRA ID: MHWC-281, MHWC-266
Infant Registration Form Validation Issue (Birth Dose Vaccine Given),Even after the woman has delivered, it shows up in the ANC listing
✅ 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
Bug Fixes
Changes