SamaSVM commented on PR #6426:
URL: https://github.com/apache/fineract/pull/6426#issuecomment-6291411670

   @galovics Thank you for your review.
   
   **Snapshot incompleteness (flat ↔ percentage, charge swap)**
   
   Confirmed. `LoanChargeAssembler.createNewFromJson()` takes `amount` from the 
snapshot but `ChargeCalculationType` from the live `chargeDefinition`. On the 
read side, the `COALESCE`/`findFirst()` logic was also joined against the 
*current* product charge. As a result, when a charge was swapped, the new 
charge ID could be paired with the old snapshot amount.
   
   Fixed by snapshotting the charge identity alongside the amount:
   
   - Added `m_loan.overdue_charge_id` next to `overdue_charge_amount`.
   - `Loan.getOverdueChargeAmountOrDefault(Charge)` now uses the snapshot only 
when `overdueChargeId` still matches the charge being applied. Otherwise, it 
falls back to the live charge amount.
   - Applied this logic in both 
`LoanReadPlatformServiceImpl#retrieveAllOverdueInstallmentsForLoan` and the SQL 
mapper:
     `COALESCE(CASE WHEN ml.overdue_charge_id = mc.id THEN 
ml.overdue_charge_amount END, mc.amount)`.
   - Added an integration test, 
`overdueChargeSwappedInProductFallsBackToNewChargeAmount`, covering the swap 
case: the stale snapshot is ignored and the new charge's amount is used.
   
   **Migration (1036)**
   
   Reworked the backfill:
   
   - Scoped it to `loan_status_id = 300` (ACTIVE) only. Overdue charges are 
never applied to closed/written-off loans anyway: both 
`retrieveAllOverdueInstallmentsForLoan` and the scheduled job already filter on 
active loans, so there is no reason to lock every row.
   - Rewrote the comment to describe the actual behavior: the migration 
backfills *active* loans using the product's *current* charge. Historical 
amounts cannot be recovered, so existing loans are pinned to today's amount 
rather than the amount at loan creation time. This also means the fallback path 
only matters for loans whose product has no overdue charge, or where the charge 
is swapped later.
   - Prefixed changeset IDs with `1036-1`/`1036-2` to match the convention used 
in neighboring files (e.g. `1029-*`).
   - Added the missing `charge_applies_to_enum = 1` filter to match the runtime 
query.
   - Added a comment for `charge_time_enum = 9` 
(`ChargeTimeType.OVERDUE_INSTALLMENT`).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to