Hi everyone, The removal PR is now open:
PR: https://github.com/apache/fineract/pull/5764 JIRA: https://issues.apache.org/jira/browse/FINERACT-2587 Phase 1 removes the write path only — API routing branch, interface method, stub implementation, builder method, two constants, Liquibase changeset deleting INACTIVATE_CLIENTCHARGE and INACTIVATE_CLIENTCHARGE_CHECKER, and corresponding sample SQL entries. The GET response fields isActive and inactivationDate are left untouched. On 2026/04/09 11:08:01 Ashhar Ahmad Khan wrote: > Hi everyone, > > A few weeks ago while going through the client module I came across POST > /clients/{clientId}/charges/{chargeId}?command=inactivate and noticed it > always threw an exception. I traced it back to a missing command handler > and a service method that was just returning null with a "functionality not > yet supported" comment. It looked like an incomplete implementation, so I > opened PR #5655 [1] to implement it. > > After I raised this on the dev list [2], Victor pointed out that charges > should be account-associated rather than client-associated, and James asked > that we properly validate whether this path had ever worked and whether > anything depended on it before any removal decision. > > The inactivateCharge() service method has been returning null since its > very first commit on August 17 2015 (d0fd3e4a6c, Vishwas Babu A J). A few > weeks after that, commit 8f1eb39d5d updated the comment from "// TODO > Auto-generated method stub" to "// functionality not yet supported" — still > returning null. > > Searching the full git history: > git log --all -S "clientCharge.inactivate" --oneline > # zero results > > There has never been an implementation in MifosX or Apache Fineract. The > savings equivalent has been working since July 2014 (MIFOSX-1408), but > client charges were not introduced until August 2015, so the handler was > simply never written when the feature arrived. > > I also verified this against apache/fineract:latest: > POST /fineract-provider/api/v1/clients/1/charges/1?command=inactivate > HTTP/1.1 400 > { "errors": [{ "userMessageGlobalisationCode": > "error.msg.command.unsupported" }] } > > The m_portfolio_command_source entry for that call shows made_on_date as > NULL and status 5, meaning the request was rejected before any processing > began and nothing was written to m_client_charge. Before running this, the > table had zero rows for action_name=INACTIVATE and > entity_name=CLIENTCHARGE. The one entry that now exists is from this > validation run. > > On the dependency question James raised, I checked the full surface area: > > - No inactivateClientCharge method in the generated SDK client > - Swagger @Operation on that endpoint documents only paycharge and waive > - Built fineract.yaml has no inactivate entry for client charges > - No frontend file in the repository references command=inactivate for > client charges > - No integration test has ever exercised this endpoint > - m_portfolio_command_source had zero prior attempts before this > validation run > > Based on all of this, and on the direction from Victor and James, I would > like to proceed with a write-path-only removal. Concretely that means: > > - Remove the else-if branch for command=inactivate in > ClientChargesApiResource > - Remove inactivateCharge() from the interface and its null stub in the > implementation > - Remove inactivateClientCharge() from CommandWrapperBuilder > - Remove CLIENT_CHARGE_ACTION_INACTIVATE and > CLIENT_CHARGE_COMMAND_INACTIVATE_CHARGE from ClientApiConstants > - Add a Liquibase changeset deleting INACTIVATE_CLIENTCHARGE and > INACTIVATE_CLIENTCHARGE_CHECKER from m_permission > - Update barebones_db.sql and load_sample_data.sql accordingly > > The shared ACTION_INACTIVATE constant in CommandWrapperConstants stays > untouched as savings charge inactivation depends on it. > > The GET response for /clients/{clientId}/charges keeps isActive and > inactivationDate for now since dropping those fields changes the response > contract. That is a separate conversation if the community wants it. > > For any existing caller, ?command=inactivate goes from HTTP 400 > (error.msg.command.unsupported) to HTTP 400 (error.msg.unrecognized.param). > The status code does not change. > > Backward compatibility will not be preserved for ?command=inactivate. This > command has never returned a successful response in any version of Fineract. > > I will hold off at least 72 hours before opening the removal PR. If anyone > sees a reason to preserve this path, or knows of downstream usage I may > have missed, please reply on-thread. > > Thanks, > Ashhar > > [1] https://github.com/apache/fineract/pull/5655 > [2] https://lists.apache.org/thread/lhh0hx0t2y7vzdrs98fg3qyfx1xo9d0y > JIRA: https://issues.apache.org/jira/browse/FINERACT-2545 >
