rymghosn commented on PR #6235: URL: https://github.com/apache/fineract/pull/6235#issuecomment-5989495264
@adamsaghy Thanks for checking every claim; you were right on all of them. I've rebased onto the latest `develop` and reworked the PR, and the description is updated to match: 1. **Description:** it now says the SI was always created at disbursement and the defect was in listing by a loan account. 2. **Missing `WHERE`:** the account filter now counts towards the `WHERE` clause in `StandingInstructionReadPlatformServiceImpl#retrieveAll`, so `?fromAccountId=X&fromAccountType=1` on its own works instead of returning 500. 3. **History endpoint:** `StandingInstructionHistoryReadServiceImpl` uses the same loan filter now. 4. **Other transfer types:** a loan account filter is always `(toloanacc.id=? or fromloanacc.id=?)`, whatever the `transferType`. The per-type branching is gone. 5. **PostgreSQL boolean comparisons:** unchanged. 6. **Changeset (blocking):** reversed, and renumbered to `0256` because `0245` is taken upstream. It now keeps the trimmed codes the application checks and removes the trailing-space rows. First it moves their `m_role_permission` assignments over to the trimmed rows (no duplicates) and carries over an enabled `can_maker_checker` flag. Then it deletes the trailing-space rows, and trims `READ_STANDINGINSTRUCTION ` in place since it has no trimmed duplicate. The PostgreSQL-only precondition is gone. Rows are matched with `CHAR_LENGTH(code) > CHAR_LENGTH(TRIM(code))` rather than string equality, so it behaves the same under MySQL/MariaDB `PAD SPACE` collations. The self-referencing statements use derived tables, as MySQL requires. I verified it on an upgraded PostgreSQL tenant where a role held both variants. I haven't run it on MySQL/MariaDB locally. 7. **Tests:** new `StandingInstructionLoanAccountTest` lists the disbursement-created SI by loan account with and without `transferType`, and runs the "Execute Standing Instruction" job on the due date for a loan-dues SI. It checks that the job repays the loan and that the run history is found by the loan account. It passes on PostgreSQL, together with the existing `SchedulerJobsTestResults.testExecuteStandingInstructionsJobOutcome` and `LoanAccountDisbursementToSavingsWithAutoDownPaymentTest`. Could you please re-review? Thanks! -- 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]
