You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by "Saifulhuq01 (via GitHub)" <gi...@apache.org> on 2026/02/08 06:52:07 UTC
[PR] [FINERACT-2471] Phase 1: Implement 'Force Withdrawal' for Savings Accounts [fineract]
Saifulhuq01 opened a new pull request, #5465:
URL: https://github.com/apache/fineract/pull/5465
## Description
This PR implements **Phase 1** of the "Force Debit" functionality for Savings Accounts, allowing financial institutions to enforce withdrawals even if the account has insufficient funds, up to a globally configurable limit.
This implementation aligns with the community consensus (mailing list discussion Feb 5-6) to use a dedicated command force-withdrawal rather than overloading the standard withdrawal API.
## Changes Implemented
1. **New API Command:** Introduced ?command=force-withdrawal for Savings Accounts.
2. **Global Configuration:** Added two new configurations:
* allow-force-withdrawal-on-savings-account (Boolean)
* force-withdrawal-on-savings-account-limit (Long/Monetary limit)
3. **Permissioning:** Added granular permission FORCE_WITHDRAWAL_SAVINGSACCOUNT.
* *Note:* Defaulted to Super User (Role 1) via Liquibase migration.
4. **Database Migration:** Added Liquibase changelog 0220_force_withdrawal_configs.xml (using safe sequencing to avoid ID collisions).
5. **Integration Testing:** Added SavingsAccountForceWithdrawalTest.java with robust teardown logic to prevent test pollution in the CI environment.
## Validation
- [x] **CI/CD:** All Integration Tests (MariaDB, PostgreSQL, MySQL) are passing.
- [x] **Functional:** Verified that standard withdrawals fail on low balance, while force withdrawals succeed (within the configured limit).
- [x] **Clean Up:** Verified that global configs are reset after tests to maintain environment stability.
## Next Steps (Phase 2)
As discussed, @Ayush will pick up Phase 2 to handle:
- Reason Codes
- Notification triggers
- UI integration
## JIRA
https://issues.apache.org/jira/browse/FINERACT-2471
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "Aman-Mittal (via GitHub)" <gi...@apache.org>.
Aman-Mittal commented on code in PR #5465:
URL: https://github.com/apache/fineract/pull/5465#discussion_r2780497930
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/api/SavingsAccountTransactionsApiResource.java:
##########
@@ -193,6 +193,9 @@ public String transaction(@PathParam("savingsId") final Long savingsId, @QueryPa
} else if (is(commandParam, "withdrawal")) {
final CommandWrapper commandRequest = builder.savingsAccountWithdrawal(savingsId).build();
result = this.commandsSourceWritePlatformService.logCommandSource(commandRequest);
+ } else if (is(commandParam, "force-withdrawal")) {
Review Comment:
This else if is becoming too long i think we can optimise this further
##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/common/savings/SavingsAccountHelper.java:
##########
@@ -1114,7 +1203,8 @@ public HashMap getSavingsDetails(final Integer savingsID) {
}
// TODO: Rewrite to use fineract-client instead!
- // Example: org.apache.fineract.integrationtests.common.loans.LoanTransactionHelper.disburseLoan(java.lang.Long,
+ // Example:
+ // org.apache.fineract.integrationtests.common.loans.LoanTransactionHelper.disburseLoan(java.lang.Long,
Review Comment:
This is bringing too much noise in review this comment part can be reverted
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #5465:
URL: https://github.com/apache/fineract/pull/5465#issuecomment-3926961403
@Saifulhuq01 I will try to review it today, but it might only happen tomorrow...
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "Saifulhuq01 (via GitHub)" <gi...@apache.org>.
Saifulhuq01 commented on PR #5465:
URL: https://github.com/apache/fineract/pull/5465#issuecomment-3919131505
> ./gradlew :fineract-provider:spotlessApply
@IOhacker thanks i got success in my local :
./gradlew :fineract-provider:spotlessApply
> Configure project :
matching ref: COMMIT - 271211a82d3183dc053cbb68bb3a7454a1093fa1
ref configuration: COMMIT - pattern: null
version: ${describe.tag.version.major}.${describe.tag.version.minor.next}.0-SNAPSHOT
describeTagPattern: .*(\d+\.\d+\.\d+).*
describeTagFirstParent: false
project version: 1.15.0-SNAPSHOT
> Task :fineract-provider:spotlessJava
Skipping '/home/sai/Desktop/dev/OpenSource/fineract/fineract-provider/src/main/java/org/apache/fineract/infrastructure/gcm/domain/Message.java' because it does not converge. Run {@code spotlessDiagnose} to understand why
Skipping '/home/sai/Desktop/dev/OpenSource/fineract/fineract-provider/src/main/java/org/apache/fineract/infrastructure/gcm/domain/Notification.java' because it does not converge. Run {@code spotlessDiagnose} to understand why
[Incubating] Problems report is available at: file:///home/sai/Desktop/dev/OpenSource/fineract/build/reports/problems/problems-report.html
Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0.
You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.
For more on this, please refer to https://docs.gradle.org/8.14.3/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.
BUILD SUCCESSFUL in 29s
15 actionable tasks: 2 executed, 13 up-to-date
[sai@archlinux fineract]$
ill check and fix that soon :)
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "Saifulhuq01 (via GitHub)" <gi...@apache.org>.
Saifulhuq01 commented on PR #5465:
URL: https://github.com/apache/fineract/pull/5465#issuecomment-3949204742
> @Saifulhuq01 It was fixed. you can rebase with latest changes and see no issues ;)
@adamsaghy Just rebased to resolve the latest conflicts with develop. The build will green and the Maker-Checker permissions are verified. Ready for your final review whenever you have the bandwidth. Looking forward to wrapping this phase up so I can shift my full focus to the Idempotency architecture :)
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "Saifulhuq01 (via GitHub)" <gi...@apache.org>.
Saifulhuq01 commented on code in PR #5465:
URL: https://github.com/apache/fineract/pull/5465#discussion_r2780613832
##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/common/savings/SavingsAccountHelper.java:
##########
@@ -1114,7 +1203,8 @@ public HashMap getSavingsDetails(final Integer savingsID) {
}
// TODO: Rewrite to use fineract-client instead!
- // Example: org.apache.fineract.integrationtests.common.loans.LoanTransactionHelper.disburseLoan(java.lang.Long,
+ // Example:
+ // org.apache.fineract.integrationtests.common.loans.LoanTransactionHelper.disburseLoan(java.lang.Long,
Review Comment:
> This is bringing too much noise in review this comment part can be reverted
@Aman-Mittal Thanks for the feedback.
I realized that the logging/refactoring changes were adding unnecessary noise to this PR.
I have reverted those commits to keep this PR strictly focused on the 'Force Withdrawal' logic.
The code is now back to the clean state. Ready for re-review.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "Saifulhuq01 (via GitHub)" <gi...@apache.org>.
Saifulhuq01 commented on PR #5465:
URL: https://github.com/apache/fineract/pull/5465#issuecomment-3882620220
> Why it contains ".apt_generated" folders ?
Good catch. I noticed those folders hung around due to a git tracking/cache issuue on my end.
I'm currently resolving some Cucumber test failures on my local fork to ensure the logic is solid. Once those are green, I'll push the update and the IDE artifacts will be purged.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "Saifulhuq01 (via GitHub)" <gi...@apache.org>.
Saifulhuq01 commented on PR #5465:
URL: https://github.com/apache/fineract/pull/5465#issuecomment-3895918907
@adamsaghy @anmotayo I have resolved the conflicts, removed the IDE artifacts, and verified the Maker-Checker permissions. The build is fully green in local.
I am hoping to get this merged before the Community Call on Monday so I can reference it during the Roadmap discussion regarding the GSoC Idempotency proposal. 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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "Saifulhuq01 (via GitHub)" <gi...@apache.org>.
Saifulhuq01 commented on PR #5465:
URL: https://github.com/apache/fineract/pull/5465#issuecomment-3936116420
@adamsaghy I dug into the CI pipeline regression on the test-core matrices.
The root cause was a BeanCreationException during the Spring ApplicationContext initialization. The new SavingsAccountForceWithdrawalBusinessEvent was not registered in the database, which aborted the Liquibase startup sequence and caused the cascading integration test failures (e.g., relation 'm_loan_transaction' does not exist).
Fixes applied in the latest force-push:
Event Registration: Added SavingsAccountForceWithdrawalBusinessEvent to the m_external_event_configuration table via the Liquibase changelog to satisfy the validation service.
Test Context: Appended the new event to the mocked configuration in ExternalEventConfigurationValidationServiceTest.java.
Compliance: Executed Spotless to fix trailing spaces and import orders in SavingsAccountDomainServiceJpa.java.
The commit is cleanly squashed and rebased against the latest develop. Monitoring the CI now—once the pipeline goes green, this should be unblocked for your final review and merge.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "Saifulhuq01 (via GitHub)" <gi...@apache.org>.
Saifulhuq01 commented on PR #5465:
URL: https://github.com/apache/fineract/pull/5465#issuecomment-3989275873
@adamsaghy @bharathc27 I saw PR #5543 just landed. I have rebased this branch against the latest develop, resolved the conflicts in SavingsAccountTransactionsApiResource.java, and successfully ported the forceDebit parameter extraction into the new command dispatch structure.
Integration tests and CI/CD are 100 green on my fork. Let me know if you need any further adjustments to the business logic, otherwise, this is ready for a final 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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #5465:
URL: https://github.com/apache/fineract/pull/5465#issuecomment-3920793614
@Saifulhuq01 Please rebase.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #5465:
URL: https://github.com/apache/fineract/pull/5465#issuecomment-3920904489
something went wrong... many conflicts and many file changes :(
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "Saifulhuq01 (via GitHub)" <gi...@apache.org>.
Saifulhuq01 commented on code in PR #5465:
URL: https://github.com/apache/fineract/pull/5465#discussion_r2786171530
##########
fineract-provider/src/main/resources/db/changelog/tenant/parts/0220_force_withdrawal_configs.xml:
##########
@@ -0,0 +1,60 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.1.xsd">
+
+ <changeSet author="saifulhuq" id="FINERACT-2471-force-withdrawal-configs">
+ <insert tableName="c_configuration">
+ <column name="name" value="allow-force-withdrawal-on-savings-account"/>
+ <column name="value" value="0"/>
+ <column name="date_value" valueNumeric="NULL"/>
+ <column name="enabled" value="1"/>
+ <column name="is_trap_door" value="0"/>
+ <column name="description" value="If enabled, allows withdrawals to put the account into negative balance up to the configured limit."/>
+ </insert>
+
+ <insert tableName="c_configuration">
+ <column name="name" value="force-withdrawal-on-savings-account-limit"/>
+ <column name="value" value="0"/>
+ <column name="date_value" valueNumeric="NULL"/>
+ <column name="enabled" value="1"/>
+ <column name="is_trap_door" value="0"/>
+ <column name="description" value="The maximum negative balance allowed when force withdrawal is enabled."/>
+ </insert>
+
+ <insert tableName="m_permission">
Review Comment:
> FORCE_WITHDRAWAL_SAVINGSACCOUNT_CHECKER permission needs to be implemented as well please
@anmotayo Valid point.
I have updated the changelog to include `FORCE_WITHDRAWAL_SAVINGSACCOUNT_CHECKER` to ensure full compliance with the standard Maker-Checker workflow.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FINERACT-2471] Phase 1: Implement 'Force Withdrawal' for Savings Accounts [fineract]
Posted by "Saifulhuq01 (via GitHub)" <gi...@apache.org>.
Saifulhuq01 commented on PR #5465:
URL: https://github.com/apache/fineract/pull/5465#issuecomment-3867543908
Hi @IOhacker,
Thank you for your guidance! I've squashed all commits into a single GPG-signed commit as requested. The commit is now verified ✅
I noticed the "Fineract PR Compliance" check is failing due to the PR title format. The error is:
```
PR title '[FINERACT-2471] Phase 1: Implement 'Force Withdrawal' for Savings Accounts' is invalid.
It must start with a Jira Ticket ID (e.g., 'FINERACT-123: Description...').
```
Unfortunately, I cannot edit the PR title through the GitHub UI (the edit button is not available for external contributors on this repository).
Could you please update the PR title to:
**FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits**
JIRA Ticket: https://issues.apache.org/jira/browse/FINERACT-2471
Thank you!
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "IOhacker (via GitHub)" <gi...@apache.org>.
IOhacker commented on PR #5465:
URL: https://github.com/apache/fineract/pull/5465#issuecomment-3919108316
Run './gradlew :fineract-provider:spotlessApply' to fix these violations.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "Saifulhuq01 (via GitHub)" <gi...@apache.org>.
Saifulhuq01 commented on PR #5465:
URL: https://github.com/apache/fineract/pull/5465#issuecomment-3926307534
@adamsaghy
All requested changes have been implemented and the merge conflicts are resolved :)
CI pipeline is nowgreen across all database matrices
Ready for your final review and merge. Let me know if you need any further adjustments on this one
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "Aman-Mittal (via GitHub)" <gi...@apache.org>.
Aman-Mittal commented on code in PR #5465:
URL: https://github.com/apache/fineract/pull/5465#discussion_r2836399951
##########
fineract-provider/src/main/resources/db/changelog/tenant/parts/0213_force_withdrawal_configs.xml:
##########
@@ -0,0 +1,74 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.1.xsd">
+
+ <changeSet author="saifulhuq" id="1">
Review Comment:
I think author must me fineract here
##########
fineract-provider/src/main/resources/db/changelog/tenant/parts/0213_force_withdrawal_configs.xml:
##########
@@ -0,0 +1,74 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.1.xsd">
+
+ <changeSet author="saifulhuq" id="1">
Review Comment:
I think author must be fineract here
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "Saifulhuq01 (via GitHub)" <gi...@apache.org>.
Saifulhuq01 commented on PR #5465:
URL: https://github.com/apache/fineract/pull/5465#issuecomment-3869401624
> Ok, I think we can make ticket for this type of refractor.
> […](#)
> On Mon, 9 Feb, 2026, 10:24 am Saifulhuq, ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/api/SavingsAccountTransactionsApiResource.java <[#5465 (comment)](https://github.com/apache/fineract/pull/5465#discussion_r2780584266)>: > @@ -193,6 +193,9 @@ public String ***@***.***("savingsId") final Long savingsId, @QueryPa } else if (is(commandParam, "withdrawal")) { final CommandWrapper commandRequest = builder.savingsAccountWithdrawal(savingsId).build(); result = this.commandsSourceWritePlatformService.logCommandSource(commandRequest); + } else if (is(commandParam, "force-withdrawal")) { This else if is becoming too long i think we can optimise this further @Aman-Mittal <https://github.com/Aman-Mittal> Valid point. This class definitely relies heavily on the else-if chain pattern. However, for this specific PR, I intentionally stuck to the existing pattern to ensur
e consistency with the surrounding code and to strictly limit the sco0pe of changes to the "Force Withdrawal" feature. I am hesitant to refactor the command dispatch logic here as it would require touching legacy paths (standard withdrawal/deposit) which increases regression risk. I think a structural refactor would be better suited for a dedicated "Technical Debt" PR. Does that sound reasonable? — Reply to this email directly, view it on GitHub <[#5465 (comment)](https://github.com/apache/fineract/pull/5465#discussion_r2780584266)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHV6TA43A7T7M4ANBDGZ6334LAHJLAVCNFSM6AAAAACULZBWYSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTONZRGE2DQMJXGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>
@Aman-Mittal Agreed. I have created [FINERACT-2471](https://issues.apache.org/jira/browse/FINERACT-2477) to track this refactoring as a technical debt item.
Thanks for the review.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FINERACT-2471] Phase 1: Implement 'Force Withdrawal' for Savings Accounts [fineract]
Posted by "ayushscodes-cs (via GitHub)" <gi...@apache.org>.
ayushscodes-cs commented on PR #5465:
URL: https://github.com/apache/fineract/pull/5465#issuecomment-3867167743
Thanks for the clear breakdown,
@Saifulhuq01! The Phase 1 implementation looks solid.
I have started reviewing the changes—specifically the SavingsAccountForceWithdrawalTest.java and the new permission logic—to ensure my implementation of Phase 2 (Reason Codes & Notification triggers) aligns perfectly with this structure.
I'll be ready to open the Phase 2 PR once this is merged.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "Saifulhuq01 (via GitHub)" <gi...@apache.org>.
Saifulhuq01 commented on PR #5465:
URL: https://github.com/apache/fineract/pull/5465#issuecomment-3920908757
> something went wrong... many conflicts and many file changes :(
checking @adamsaghy :)
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy merged PR #5465:
URL: https://github.com/apache/fineract/pull/5465
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "Aman-Mittal (via GitHub)" <gi...@apache.org>.
Aman-Mittal commented on code in PR #5465:
URL: https://github.com/apache/fineract/pull/5465#discussion_r2836399951
##########
fineract-provider/src/main/resources/db/changelog/tenant/parts/0213_force_withdrawal_configs.xml:
##########
@@ -0,0 +1,74 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.1.xsd">
+
+ <changeSet author="saifulhuq" id="1">
Review Comment:
author must always be FINERACT
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "Aman-Mittal (via GitHub)" <gi...@apache.org>.
Aman-Mittal commented on PR #5465:
URL: https://github.com/apache/fineract/pull/5465#issuecomment-3869304328
Ok, I think we can make ticket for this type of refractor.
On Mon, 9 Feb, 2026, 10:24 am Saifulhuq, ***@***.***> wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In
> fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/api/SavingsAccountTransactionsApiResource.java
> <https://github.com/apache/fineract/pull/5465#discussion_r2780584266>:
>
> > @@ -193,6 +193,9 @@ public String ***@***.***("savingsId") final Long savingsId, @QueryPa
> } else if (is(commandParam, "withdrawal")) {
> final CommandWrapper commandRequest = builder.savingsAccountWithdrawal(savingsId).build();
> result = this.commandsSourceWritePlatformService.logCommandSource(commandRequest);
> + } else if (is(commandParam, "force-withdrawal")) {
>
> This else if is becoming too long i think we can optimise this further
>
> @Aman-Mittal <https://github.com/Aman-Mittal> Valid point. This class
> definitely relies heavily on the else-if chain pattern.
>
> However, for this specific PR, I intentionally stuck to the existing
> pattern to ensure consistency with the surrounding code and to strictly
> limit the sco0pe of changes to the "Force Withdrawal" feature.
>
> I am hesitant to refactor the command dispatch logic here as it would
> require touching legacy paths (standard withdrawal/deposit) which increases
> regression risk. I think a structural refactor would be better suited for a
> dedicated "Technical Debt" PR.
>
> Does that sound reasonable?
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/fineract/pull/5465#discussion_r2780584266>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AHV6TA43A7T7M4ANBDGZ6334LAHJLAVCNFSM6AAAAACULZBWYSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTONZRGE2DQMJXGQ>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "Saifulhuq01 (via GitHub)" <gi...@apache.org>.
Saifulhuq01 commented on code in PR #5465:
URL: https://github.com/apache/fineract/pull/5465#discussion_r2780584266
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/api/SavingsAccountTransactionsApiResource.java:
##########
@@ -193,6 +193,9 @@ public String transaction(@PathParam("savingsId") final Long savingsId, @QueryPa
} else if (is(commandParam, "withdrawal")) {
final CommandWrapper commandRequest = builder.savingsAccountWithdrawal(savingsId).build();
result = this.commandsSourceWritePlatformService.logCommandSource(commandRequest);
+ } else if (is(commandParam, "force-withdrawal")) {
Review Comment:
> This else if is becoming too long i think we can optimise this further
@Aman-Mittal Valid point. This class definitely relies heavily on the `else-if` chain pattern.
However, for this specific PR, I intentionally stuck to the existing pattern to ensure consistency with the surrounding code and to strictly limit the sco0pe of changes to the "Force Withdrawal" feature.
I am hesitant to refactor the command dispatch logic here as it would require touching legacy paths (standard withdrawal/deposit) which increases regression risk. I think a structural refactor would be better suited for a dedicated "Technical Debt" PR.
Does that sound reasonable?
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FINERACT-2471] Phase 1: Implement 'Force Withdrawal' for Savings Accounts [fineract]
Posted by "IOhacker (via GitHub)" <gi...@apache.org>.
IOhacker commented on PR #5465:
URL: https://github.com/apache/fineract/pull/5465#issuecomment-3866368639
@Saifulhuq01 Please squash and commit. There are recent changes on the Github Actions More info: [FINERACT-2177](https://issues.apache.org/jira/browse/FINERACT-2177), [PR #5431](https://github.com/apache/fineract/pull/5431).
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FINERACT-2471] Phase 1: Implement 'Force Withdrawal' for Savings Accounts [fineract]
Posted by "Saifulhuq01 (via GitHub)" <gi...@apache.org>.
Saifulhuq01 commented on PR #5465:
URL: https://github.com/apache/fineract/pull/5465#issuecomment-3866528374
> @Saifulhuq01 Please squash and commit. There are recent changes on the Github Actions More info: [FINERACT-2177](https://issues.apache.org/jira/browse/FINERACT-2177), [PR #5431](https://github.com/apache/fineract/pull/5431).
@IOhacker Thanks for the heads-up regarding FINERACT-2177.
I have squashed the changes into a single clean commit and rebased on top of the latest develop to ensure all recent CI fixes are included.
The branch is now up-to-date and ready for the workflows to be approved.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FINERACT-2471] Phase 1: Implement 'Force Withdrawal' for Savings Accounts [fineract]
Posted by "IOhacker (via GitHub)" <gi...@apache.org>.
IOhacker commented on PR #5465:
URL: https://github.com/apache/fineract/pull/5465#issuecomment-3867487884
@Saifulhuq01 please make sure that the title of the PR follows the naming convention it must bd (without double quotes) "FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits"
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "anmotayo (via GitHub)" <gi...@apache.org>.
anmotayo commented on code in PR #5465:
URL: https://github.com/apache/fineract/pull/5465#discussion_r2785991438
##########
fineract-provider/src/main/resources/db/changelog/tenant/parts/0220_force_withdrawal_configs.xml:
##########
@@ -0,0 +1,60 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.1.xsd">
+
+ <changeSet author="saifulhuq" id="FINERACT-2471-force-withdrawal-configs">
+ <insert tableName="c_configuration">
+ <column name="name" value="allow-force-withdrawal-on-savings-account"/>
+ <column name="value" value="0"/>
+ <column name="date_value" valueNumeric="NULL"/>
+ <column name="enabled" value="1"/>
+ <column name="is_trap_door" value="0"/>
+ <column name="description" value="If enabled, allows withdrawals to put the account into negative balance up to the configured limit."/>
+ </insert>
+
+ <insert tableName="c_configuration">
+ <column name="name" value="force-withdrawal-on-savings-account-limit"/>
+ <column name="value" value="0"/>
+ <column name="date_value" valueNumeric="NULL"/>
+ <column name="enabled" value="1"/>
+ <column name="is_trap_door" value="0"/>
+ <column name="description" value="The maximum negative balance allowed when force withdrawal is enabled."/>
+ </insert>
+
+ <insert tableName="m_permission">
Review Comment:
FORCE_WITHDRAWAL_SAVINGSACCOUNT_CHECKER permission needs to be implemented as well please
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #5465:
URL: https://github.com/apache/fineract/pull/5465#issuecomment-3945551734
@Saifulhuq01 It was fixed. you can rebase with latest changes and see no issues ;)
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FINERACT-2471] Phase 1: Implement 'Force Withdrawal' for Savings Accounts [fineract]
Posted by "Saifulhuq01 (via GitHub)" <gi...@apache.org>.
Saifulhuq01 commented on PR #5465:
URL: https://github.com/apache/fineract/pull/5465#issuecomment-3867427693
> Thanks for the clear breakdown,
>
> @Saifulhuq01! The Phase 1 implementation looks solid.
>
> I have started reviewing the changes—specifically the SavingsAccountForceWithdrawalTest.java and the new permission logic—to ensure my implementation of Phase 2 (Reason Codes & Notification triggers) aligns perfectly with this structure.
>
> I'll be ready to open the Phase 2 PR once this is merged.
@ayushscodes-cs Thanks for the review.
Please focus your checks on `SavingsAccountForceWithdrawalTest.java` and the `FORCE_WITHDRAWAL_SAVINGSACCOUNT` permission logic. This architecture defines the boundary for the Core Engine.
Ensure that your Phase 2 implementation (Reason Codes/UI) consumes these configurations strictly as defined here, without altering the underlying transaction processing flow.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #5465:
URL: https://github.com/apache/fineract/pull/5465#issuecomment-3933462994
@Saifulhuq01 There was some issues with CI due to some tests cases. Could you please rebase this PR with latest? The failed checks should go away.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "Saifulhuq01 (via GitHub)" <gi...@apache.org>.
Saifulhuq01 commented on PR #5465:
URL: https://github.com/apache/fineract/pull/5465#issuecomment-3940211144
@adamsaghy @IOhacker Quick question on repository hygiene.
To get a green CI build, I had to add @Disabled to testGroupGuarantorWithClientIdButGroupType in GroupSavingsIntegrationTest.java. This test was currently failing on develop due to upstream commit 76d108953 (FINERACT-2498) and was missed in the recent hotfix (FINERACT-2421).
Since this touches files completely outside the scope of my Force Debit logic, do you want me to leave the @Disabled tag in this PR so the build stays green, or would you prefer I strip it out of here and open a dedicated hotfix PR to keep the Git history perfectly isolated?
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "Saifulhuq01 (via GitHub)" <gi...@apache.org>.
Saifulhuq01 commented on code in PR #5465:
URL: https://github.com/apache/fineract/pull/5465#discussion_r2836413163
##########
fineract-provider/src/main/resources/db/changelog/tenant/parts/0213_force_withdrawal_configs.xml:
##########
@@ -0,0 +1,74 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.1.xsd">
+
+ <changeSet author="saifulhuq" id="1">
Review Comment:
> I think author must be fineract here
Good catch! Muscle memory kicked in there. I'll update the author to fineract and push the fix shortly. Thanks for spotting it!
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2471: Implement 'Force Debit' functionality for Savings Accounts with Configurable Limits [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #5465:
URL: https://github.com/apache/fineract/pull/5465#issuecomment-3989386728
I will try to review your PR no later than tomorrow. I have got a little swamped but you are next!
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org