+1 On Thu, Apr 9, 2026 at 7:14 PM Kigred Developer <[email protected]> wrote:
> +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 >>> >>
