adamsaghy commented on PR #6512: URL: https://github.com/apache/fineract/pull/6512#issuecomment-6415868659
@AshharAhmadKhan thanks for picking this up. I went through it against the description and FINERACT-1183. Most of it works as described: the validity dates go through create, update and read; opening a new account and reinvesting on manual closure are both blocked; and archived products still show in the template and lookup. A few things need fixing before this can go in: **1. Maturity job: existing FD accounts set to reinvest at maturity** If an FD account is set to reinvest at maturity and its product is archived before it matures, the "Update Deposit Accounts Maturity details" job calls `handleFDAccountMaturityClosure` ([here](https://github.com/apache/fineract/blob/abe8e835a5bf6293de5683c1a6eceeda147cbc02/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/DepositAccountWritePlatformServiceJpaRepositoryImpl.java#L1214)), and the new `validateProductApplicationDate` call throws there ([here](https://github.com/apache/fineract/blob/abe8e835a5bf6293de5683c1a6eceeda147cbc02/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/DepositAccountDomainServiceJpa.java#L268-L272)). The tasklet runs as a single transaction ([config](https://github.com/apache/fineract/blob/abe8e835a5bf6293de5683c1a6eceeda147cbc02/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/jobs/updatedepositsaccountmaturitydetails/UpdateDepositsAccountMaturityDetailsConfig.java#L49)). The exception leaves the `@Transactional` domain-service method, which marks that transaction rollback-only. The tasklet's catch only logs it, so the commit at the end of the step fails and the maturity updates for every other account in that run are rolled back too. The affected account never matures and is never paid out, and the same thing happens on every run. That contradicts "existing accounts are unaffected". Please decide what should happen at maturity when the product is archived (for example, mature the account but skip the reinvestment), and check this before entering the transactional method. `testArchivedDepositProductsRejectReinvestment` archives the product after the job has already run and then only tries a manual close, so it doesn't cover this path. Please add a test for it. **2. Modifying an application can get around the archive** `modifyFDApplication` / `modifyRDApplication` can switch `productId` to an archived product ([here](https://github.com/apache/fineract/blob/abe8e835a5bf6293de5683c1a6eceeda147cbc02/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/DepositApplicationProcessWritePlatformServiceJpaRepositoryImpl.java#L527-L532)) or move `submittedOnDate` past `closeDate`, and nothing checks either change. Loan applications re-check the product dates on modify (`LoanApplicationValidator#validateSubmittedOnDate`), so the same check is needed here. **3. Date ordering on partial updates** `validateStartDateBeforeCloseDate` only compares the dates in the request. A `PUT` with only `closeDate`, which is the usual way to archive a product, is never compared with the stored `startDate`. Since the entity-level check was removed, `closeDate < startDate` can now be saved. On update, please compare against the stored value when only one of the two dates is sent. **Description** Please update the PR description and commit message. The `depositAmount` Swagger fields are already `BigDecimal` on `develop`, and `retrieveAllForLookup` on `develop` never had a `close_date` filter, so neither of those points is part of this diff. The charts change is better described as "charts required on create, optional on update". That is what makes a dates-only update possible. -- 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]
