MarianaDmytrivBinariks commented on PR #6662: URL: https://github.com/apache/fineract/pull/6662#issuecomment-6343688815
> Thanks @mariiaKraievska, both blockers from last round are fixed. The breach name in `WorkingCapitalLoanApplicationValidationTest` is random now and IT shard 6 is green on PG, MySQL and MariaDB. The `clear*` logic moved onto `WorkingCapitalLoanProductRelatedDetail(s)` with its own unit test, and the TestRail IDs are in. The current red CI is infra (`spotlessGroovyGradle` fails on `Failed to provision P2 dependencies`, which takes down spotless, the WAR smoke test and all E2E shards), so the E2E changes in [cef7889](https://github.com/apache/fineract/commit/cef78893dfd599d3b026fe89b34b3d96a787b856) haven't run yet. Pls re-run and let's merge only after E2E is green. > > A few things on the new push: > > * On loan create an explicit `breachId` / `nearBreachId` / `delinquencyStartType` / `breachStartType: null` now clears the value instead of inheriting it from the product. That's an API behavior change, is it intended? There's no IT for it. > * [cef7889](https://github.com/apache/fineract/commit/cef78893dfd599d3b026fe89b34b3d96a787b856) removes an existing step from C115359 (`WorkingCapitalBreachEvaluation.feature`, around line 448) without explanation. The path should still return `override.not.allowed`, can you confirm why it's gone? That commit is also labelled [FINERACT-2684](https://issues.apache.org/jira/browse/FINERACT-2684) instead of [FINERACT-2968](https://issues.apache.org/jira/browse/FINERACT-2968). > * Still no IT/e2e for clearing on parent removal (near-breach included), and the breach defaults differ between create (`0` / `DISBURSEMENT`) and update (null). > * #6542 also adds `parts/0090_*` to the WC module changelog. Different changeset ids so Liquibase is fine, but whichever PR lands second should rename to 0091. thanks for review @galovics , I would like to clarify reason for this changes: > On loan create an explicit breachId / nearBreachId / delinquencyStartType / breachStartType: null now clears the value instead of inheriting it from the product. That's an API behavior change, is it intended? There's no IT for it. this fixes were made to make it possible to have some defined breach/delinquency values on WCLP level, then inherit them to WC loan account level, and just in case user doesn't want to have breachId / nearBreachId / delinquencyStartType / breachStartType at all - to allow clear those values while create/modify loan account IF overrides are allowed, so user is free to inherit OR to have no values per breach/delinquency, that means to override them actually, like value -> no value(empty); in case override is disallowed - in case use tries to override those values, then corresponding error message is shown on BE(on UI - those fields aren't editable and greyed out with inherited values from WCLP level), could you please let me know is that sounds reasonable to you? please review, I will appreciate your answer, thanks in advance! -- 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]
