Skip to content

Merge 3.8.4 3.9 - #182

Merged
snehar-nd merged 30 commits into
release-3.9.0from
vb/3.8.4-3.9.0
Sep 10, 2026
Merged

Merge 3.8.4 3.9#182
snehar-nd merged 30 commits into
release-3.9.0from
vb/3.8.4-3.9.0

Conversation

@vishwab1

Copy link
Copy Markdown
Member

📋 Description

JIRA ID:

Please provide a summary of the change and the motivation behind it. Include relevant context and details.


✅ Type of Change

  • 🐞 Bug fix (non-breaking change which resolves an issue)
  • New feature (non-breaking change which adds functionality)
  • 🔥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 🛠 Refactor (change that is neither a fix nor a new feature)
  • ⚙️ Config change (configuration file or build script updates)
  • 📚 Documentation (updates to docs or readme)
  • 🧪 Tests (adding new or updating existing tests)
  • 🎨 UI/UX (changes that affect the user interface)
  • 🚀 Performance (improves performance)
  • 🧹 Chore (miscellaneous changes that don't modify src or test files)

ℹ️ Additional Information

Please describe how the changes were tested, and include any relevant screenshots, logs, or other information that provides additional context.

vishwab1 and others added 30 commits June 4, 2026 12:25
After MMU user selects their service point/van via getUserVanSpDetails,
write camp:vanID and camp:parkingPlaceID to Redis with 30-day TTL.
FLW-API and TM-API read these keys to stamp correct vanID on all
Stop TB records, enabling MMU DataSync to push data to central.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…l data sync

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…_opd to VALID_TABLES

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…zation

Central RoleAuthenticationFilter reads Jwttoken header, not Authorization.
Sending as Authorization caused 401 on all central data sync calls.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add /dataSync to skip list in JwtUserIdValidationFilter
- Add /dataSync/** to permitAll in SecurityConfig
- Revert RestTemplateUtil to send Authorization header for datasync

Restores release-3.4.2 behavior for datasync path while keeping
WASA security for all other endpoints.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Consistent with release-3.4.2 behavior. Security maintained by
@RequestMapping(headers="Authorization") requirement and skip/permitAll
configuration for /dataSync/** endpoints.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… records on central

Stop TB tables use vanID (camelCase) while standard AMRIT tables use VanID (PascalCase).
HashMap lookup was case-sensitive causing cleanRecord.get("VanID") to return null for
Stop TB tables, resulting in duplicate check always returning 0 and every sync run
inserting a new record instead of updating the existing one.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Identity-API's saveGeneratedBenIDToLocalServer expects a plain JSON array,
not a wrapped object with vanID and benIDList fields.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…to Identity-API

Central response does not include vanID in each record. Added loop to
inject vanID from the original request into each element of the array.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…BLES

These tables were registered in m_synctabledetail but rejected by
central's sync endpoint because DataSyncRepositoryCentral's VALID_TABLES
whitelist didn't include them, causing "Invalid identifiers provided"
errors on every sync attempt.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ALID_TABLES

Central's isValidTableName() silently rejects any table not in this
allowlist before it ever tries to build SQL against it - a defense
against SQL injection via unparameterizable table/column identifiers.

Full gap analysis against the van-side m_synctabledetail registration
found 5 tables that were registered to push (or about to be) but would
still get rejected on arrival at central:

- tb_diagnostic_order, tb_diagnostic_result, tb_diagnostic_document:
  the diagnostic-device (X-ray/TrueNat) integration tables added to
  FLW-API after this allowlist was last updated. FLW-API already
  stamps VanID/VanSerialNo correctly on all three.
- i_beneficiarydetails_rmnch, i_bornbirthdeatils: never added despite
  being core Stop TB beneficiary tables.

tb_diagnostic_provider_token deliberately excluded - local EMRLite
auth/session state, not beneficiary data, shouldn't sync to central.

tb_stoptb_visit and i_householddetails were already present from an
earlier fix; this closes out the rest of the gap.

Companion van-side fix (not in this repo): register the same 7 tables
in m_synctabledetail (SyncTableGroupID=11, Stop TB) so the van side
actually attempts to push them - see stoptb_sync_tables.sql in ADMIN-UI.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
fix(datasync): add missing Stop TB diagnostic/beneficiary tables to V…
t_form_response, t_section_response, t_question_response hold actual
per-beneficiary submitted counselling/contact-tracing answers and need to
sync to central, same as every other beneficiary-data table. They were
never registered here since the Dynamic Form module (V87) postdates this
allowlist's last update - identical gap to the diagnostic-device tables
fixed earlier.

The 7 form-definition/structure tables (t_dynamic_form, t_form_version,
t_form_section, t_question_option, t_question_validation,
t_option_condition, t_section_question) are deliberately NOT registered -
they're seeded once at app startup, not per-van transactional data.

Companion fixes (not in this repo): FLW-API now stamps VanSerialNo on all
3 response tables (was previously only stamping VanID), and the van side
needs these 3 tables registered in m_synctabledetail.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
fix(datasync): add Dynamic Form response tables to VALID_TABLES
FLW-API/TM-API/Identity-API no longer read vanID from Redis — they now read
stoptb.van.id from properties instead. This write was the source side of
that removed mechanism; now dead code with nothing left to consume it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
chore(stoptb): remove camp:vanID/parkingPlaceID Redis write on MMU login
Adds a date-range-scoped export of Stop TB beneficiaries as a CSV
formatted for the Nikshay ID Generator app, and an import endpoint to
write the portal-generated Nikshay IDs back onto those beneficiaries.

Scoped by visit-date range only, no vanID/servicePointID — MMU runs
one local database per van, so everything in it already belongs to
the current van/service point.

Location columns (village/healthFacility/tu/district/state) are
resolved against Nikshay's own, isolated location hierarchy
(m_nikshay_village -> m_nikshay_facility -> m_nikshay_tu ->
m_nikshay_district -> m_nikshay_state), not AMRIT's standard masters
— a Stop TB beneficiary's I_bendemographics.DistrictBranchID actually
holds their Nikshay Village ID (the same overload Common-API's
NikshayAddressResolver relies on), so every other location field is
derived by walking up that one chain. A beneficiary whose village
doesn't resolve all the way to a state is silently left out of the
export, same as one who already has a Nikshay ID.

New endpoints:
- GET /stopTb/nikshay/exportBeneficiariesCsv?fromDate=&toDate=
- POST /stopTb/nikshay/importResultsCsv?visitDate=&file=

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…hone+name

Two corrections after checking real data:

1. nikshay_id now lives on tb_suspected, not tb_stoptb_diagnostics — both
   tables have the column, but tb_suspected is the one this feature is
   meant to use going forward. Both the export's already-has-an-ID check
   and the import's write path now target tb_suspected.benRegID.

2. A real Nikshay ID Generator results file has no beneficiary ID column
   of any kind — only the original template columns plus its own
   generatedId/status/durationSec/error. The import can no longer assume
   benRegId round-trips through untouched. Rows are now matched back to a
   beneficiary by normalized primaryPhone + case-insensitive firstName
   (NikshayExportRepository.findMatchingBeneficiaryIds); anything other
   than exactly one match is left for manual review rather than guessed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Every table the export previously used for beneficiary/visit data turned
out not to exist on a real server (i_beneficiary, I_bendemographics,
tb_stoptb_visit) — confirmed via BadSqlGrammarException in production
logs and direct schema inspection. Rewritten against tables and columns
verified with real data:

- Visit filter: t_benvisitdetail (VisitCategory = 'Stop TB'), not
  tb_stoptb_visit — confirmed 511 real rows vs. zero.
- Beneficiary identity: MMU's datasource only connects to db_iemr, but
  beneficiary data lives in db_identity on the same physical server —
  reached via fully-qualified cross-schema table names:
  i_beneficiarymapping (BenRegId, unique) -> BenDetailsId ->
  i_beneficiarydetails (name/DOB/gender/caste/occupation/income/HIV,
  already denormalized, no extra master-table joins needed) and
  BenAddressId -> i_beneficiaryaddress (address/village/pincode), plus
  BenContactsId -> i_beneficiarycontacts (phone).
- New confirmed real gap: many van-registered beneficiaries
  (i_beneficiarymapping.CreatedBy = 'reglocal') have a mapping row but
  their detail/address rows never synced to db_identity, sometimes for
  days. These are now silently skipped (no FirstName resolved), same
  category as an unresolved location — renamed
  countUnresolvedLocation -> countNotReadyToExport to reflect both cases.
- Import's phone+name matching updated to the same verified join path.

Tested against real data on a live server: 15 of 20 today's-date
beneficiaries correctly skipped (identity never synced), 5 correctly
resolved end-to-end including the full Nikshay location chain, producing
valid CSV rows.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…..])

exportBeneficiariesCsv was declared ResponseEntity<?> so it could return
either a StreamingResponseBody (success) or a plain String (bad request/
error). Spring's StreamingResponseBodyReturnValueHandler decides whether
to special-case the response based on the method's *declared* generic
return type, not the runtime object — a wildcard resolves to Object,
StreamingResponseBody isn't assignable from Object, so it falls through
to normal HttpMessageConverters, which can't serialize a raw lambda:
"No converter for [...NikshayExportController$$Lambda...] with preset
Content-Type 'text/csv'" (confirmed in production logs, 2026-08-18).

Fixed by keeping the return type consistently
ResponseEntity<StreamingResponseBody> and wrapping error messages as a
StreamingResponseBody that writes the message as bytes, instead of
returning a plain String on any path.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@PreAuthorize only listed clinical roles (Nurse/Doctor/Pharmacist/Lab
Technician/etc.), copied from sn/nikshaya's original controller without
verifying against real Stop TB camp roles. Confirmed in production:
AccessDeniedException for a real Stop TB user (stoptb1) whose roles
(checked directly via v_userservicerolemapping) include Registrar,
Nurse, Doctor, Pharmacist, Lab Technician, Data Sync — Registrar was the
one missing from the allow-list. Downloading/uploading a beneficiary CSV
is fundamentally a registrar task, so this was very likely the actual
active session role denied by AccessDeniedException in the logs.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Role-based gating proved fragile — wrong role list on the first attempt,
still AccessDeniedException after adding REGISTRAR. SecurityConfig's
anyRequest().authenticated() still requires a logged-in user; dropping
the role-specific check per explicit request rather than guessing a
third time.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…Body

Root cause of the persistent AccessDeniedException (survived removing
@PreAuthorize entirely): StreamingResponseBody makes Spring process the
response body on a Servlet async re-dispatch. Confirmed from production
logs (2026-08-18) — two AccessDeniedExceptions per request, the second
via ApplicationDispatcher/AsyncContextImpl. Spring Security's own core
filter chain is registered for ASYNC dispatch by default and re-runs,
but this app's custom JwtUserIdValidationFilter/RoleAuthenticationFilter
(which read the JWT cookie and establish who's logged in) are not —
AnonymousAuthenticationFilter fires instead on the second pass, and the
global anyRequest().authenticated() rule denies it.

Fixed by writing the CSV directly and synchronously onto
HttpServletResponse instead of returning a StreamingResponseBody —
avoids async dispatch entirely, so there's no second security pass to
fail.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Verified against real data: PhoneNum1 and CurrAddressValue are empty on
every real beneficiary checked — the actual data lives in
PreferredPhoneNum (phone) and CurrAddrLine1/2/3 + CurrHabitation
(address), confirmed by direct query on real records. Falls back through
PhoneNum1/PhoneNum2 if PreferredPhoneNum is empty. Address line parts are
individually NULLIF'd before CONCAT_WS, since blank (not NULL) line
fields otherwise leave stray ", ," artifacts.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
findMatchingBeneficiaryIds only checked c.PhoneNum1 — same bug already
fixed on the export side (commit 79d8422) but missed here. Confirmed
in production: two real beneficiaries (17877 'K', 17875 'U') with
PhoneNum1 IS NULL but PreferredPhoneNum populated ('9999999999') both
came back "no beneficiary found" on upload. Now checks
PreferredPhoneNum/PhoneNum1/PhoneNum2, same as the export's column
preference order. Verified against real data: 17877 now matches
correctly for phone '9999999999' + firstName 'K'.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…name

If a results row carries a parseable benRegId (e.g. a manually-rebuilt
test file, or a future ID Generator version that keeps it), use it
directly — exact match, no ambiguity possible, skips the phone+name
lookup entirely. Falls back to the existing phone+name matching only
when benRegId is absent or unparseable, which is what a genuine Nikshay
portal results file looks like (confirmed no ID column at all).

Makes test files with benRegId (e.g. a rebuilt export) match instantly
and unambiguously, while real portal files keep working exactly as
before via the phone+name path.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 10, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: fddc18ce-537c-405b-bd46-ecebdecb5a6b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sonarqubecloud

Copy link
Copy Markdown

@snehar-nd
snehar-nd merged commit 4f56d8a into release-3.9.0 Sep 10, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants