budaidev commented on PR #6264: URL: https://github.com/apache/fineract/pull/6264#issuecomment-5454040993
Thanks for the go-ahead on the shape — `asBookedPreviousRate` is implemented in this push, and the three rounding threads are answered above. Amended into the existing dev commit, so the two-commit structure is unchanged. ### `asBookedPreviousRate` Written once in `WorkingCapitalLoanPeriodPaymentRateChange.create(...)` from the predecessor resolved at booking time, and never touched again — `@Column(updatable = false)` plus `@Setter(AccessLevel.NONE)`, so `restatePreviousRates` can't reach it even by accident. That gives the row the two self-consistent pairs we discussed instead of one confusable mixture: - **as-booked audit** — `asBookedPreviousRate` + `newRate` + `eir` and the derived snapshot - **current effective chain** — `previousRate` + `newRate` + `effectiveDate` It lands in a new Liquibase changeset (`wcl-0073-2`, additive nullable `DECIMAL(19,6)`), the entity, `WorkingCapitalLoanPeriodPaymentRateChangeData`, the read service, and the Avro record. The REST client model is generated, so it picks the field up automatically. Legacy rows carry null, and the Avro/Swagger docs say why rather than leaving a silent null: for a change booked before the column existed, the booking-time predecessor genuinely isn't recoverable. I put it in its own changeset rather than extending `wcl-0073-1`, so anyone who has already applied the first one doesn't get a checksum mismatch locally. ### Test coverage The existing backdating integration test is where the two fields actually diverge, so that's where the assertions went. On the backdated loan the 20% row now reports `previousRate = 17` (restated) against `asBookedPreviousRate = 18` (what its EIR was computed against); the same change booked in effective-date order reports 17 for both. Asserting the in-order case too is deliberate — it's what makes the divergence attributable to the backdating rather than to the field simply differing everywhere. The mapper unit test carries a row where the two differ, plus a pre-feature row that must map to null rather than being backfilled from the restated value. ### The earlier items — re-checked against the current diff, not assumed 1. `segmentOpenedOn(LocalDate)` replaces `rateSegmentAt`, matching on `startDayIndex() == split` exactly as you proposed, with the graceful `null` fallback retained in `recordCalculatedValues`. 2. EIR rounding pinned, now `HALF_EVEN` per the threads above. 3. `recordCalculatedValues` ends in an explicit `rateChangeRepository.save(rateChange)`. 4. Annualisation uses the product's `npvDayCount` via `annualiseEir(...)`, not a hardcoded 365. 5. `calculated_annual_eir` stores the annual figure as `DECIMAL(19,6)`. One caveat I raised last round and would rather restate than let disappear: the exact-match guard removes the neighbour case, but `splitDayIndexFor` still clamps to `scheduleTerm()`, so two changes dated at or after the current schedule end map to the same index, and `RateSegment` has no identity of its own to tell them apart. Narrow edge case, and I've left it since you resolved the thread — but the clean fix is to have `applyRateChange` return the segment it just created so the caller never looks it up by index at all. Happy to raise it as a follow-up ticket. ### Verification - 7/7 integration tests in `FeignWorkingCapitalLoanRateChangeEirHistoryTest` green - Mapper unit tests green - Liquibase migrations validated fresh on **both** PostgreSQL and MariaDB; `as_booked_previous_rate` created as nullable `decimal(19,6)` on both - Full quality build (checkstyle, spotbugs, error-prone, modernizer, license, javadoc, spotless) clean E2E coverage of the new field isn't in this push — those scenarios live in the second commit, which is Mariana's, and the additive nullable column leaves the existing tables passing. Say the word and I'll add a column there too. Ready for another look. -- 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]
