+1 On Thu, 9 Apr 2026, 17:48 James Dailey, <[email protected]> wrote:
> +1 > > Sent from Gmail Mobile > > > On Thu, Apr 9, 2026 at 4:09 AM Ashhar Ahmad Khan <[email protected]> > 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 >> >
