Skip to content

FINERACT-2587: Remove CLIENTCHARGE|INACTIVATE write path (never implemented since 2015)#5764

Merged
adamsaghy merged 1 commit intoapache:developfrom
AshharAhmadKhan:FINERACT-2587-remove-clientcharge-inactivate-writepath
Apr 22, 2026
Merged

FINERACT-2587: Remove CLIENTCHARGE|INACTIVATE write path (never implemented since 2015)#5764
adamsaghy merged 1 commit intoapache:developfrom
AshharAhmadKhan:FINERACT-2587-remove-clientcharge-inactivate-writepath

Conversation

@AshharAhmadKhan
Copy link
Copy Markdown
Contributor

Description

POST /clients/{clientId}/charges/{chargeId}?command=inactivate has thrown
error.msg.command.unsupported on every call since August 2015 and has never
returned a successful response in any version of Fineract or its predecessor MifosX.

The service method inactivateCharge() was a stub returning null from its very
first commit (d0fd3e4a6c, August 17 2015, Vishwas Babu A J). No command handler
for CLIENTCHARGE|INACTIVATE was ever registered in any commit across the full
repository history. git log --all -S "clientCharge.inactivate" returns zero results.

Runtime confirmation against apache/fineract:latest (April 2026): HTTP 400,
error.msg.command.unsupported. m_portfolio_command_source shows
made_on_date=NULL, status=5 — rejected before any processing began.
m_client_charge.is_active was never written. Historical record showed zero
prior attempts before the validation run.

Following community discussion on the DEV list, architectural direction from
Victor (charges should be account-associated, not client-associated) and
confirmation from PMC Chair James Dailey, this PR removes the write path
scaffolding (Phase 1 only).

What this PR removes:

  • else if routing branch for command=inactivate in ClientChargesApiResource
  • inactivateCharge() interface method and null stub implementation in ClientChargeWritePlatformService and ClientChargeWritePlatformServiceImpl
  • inactivateClientCharge() builder method in CommandWrapperBuilder
  • CLIENT_CHARGE_ACTION_INACTIVATE and CLIENT_CHARGE_COMMAND_INACTIVATE_CHARGE constants from ClientApiConstants
  • Liquibase changeset 0228 deleting INACTIVATE_CLIENTCHARGE and INACTIVATE_CLIENTCHARGE_CHECKER from m_permission
  • Corresponding rows in barebones_db.sql and load_sample_data.sql

What this PR does NOT remove (Phase 2 decision):

  • isActive and inactivationDate from the GET /clients/{clientId}/charges response — removing these fields changes the response contract and requires a separate deprecation notice

Observable behavior change for callers:

  • Before: ?command=inactivate → HTTP 400, error.msg.command.unsupported
  • After: ?command=inactivate → HTTP 400, error.msg.unrecognized.param
  • HTTP status code does not change. This command has never produced a successful response.

References:

@AshharAhmadKhan
Copy link
Copy Markdown
Contributor Author

@adamsaghy failure unrelated to my code. Please re run it when you get a chance!

Copy link
Copy Markdown
Contributor

@IOhacker IOhacker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@AshharAhmadKhan
Copy link
Copy Markdown
Contributor Author

Hi @adamsaghy , could you take a look at this when you have time? I’ve rechecked everything and it looks good from my side. If you have any suggestions, please let me know.

@AshharAhmadKhan AshharAhmadKhan force-pushed the FINERACT-2587-remove-clientcharge-inactivate-writepath branch 2 times, most recently from c5200b9 to 4fd57a8 Compare April 17, 2026 11:54
@AshharAhmadKhan
Copy link
Copy Markdown
Contributor Author

Hi @adamsaghy , rebased onto latest develop, changeset renumbered from 0228 to 0229. Also failure unrelated to my code, so please re - trigger it when you get a chance!

Copy link
Copy Markdown
Member

@Aman-Mittal Aman-Mittal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems OK, Implementation are properly removed and liquibase migration script is maintained

@Aman-Mittal Aman-Mittal requested a review from adamsaghy April 20, 2026 12:22
@AshharAhmadKhan
Copy link
Copy Markdown
Contributor Author

Hi @adamsaghy , just circling back on this, would value your feedback whenever you get the chance.

Copy link
Copy Markdown
Contributor

@IOhacker IOhacker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@IOhacker
Copy link
Copy Markdown
Contributor

@AshharAhmadKhan there is a conflict to be solved... could you please help us?

@AshharAhmadKhan AshharAhmadKhan force-pushed the FINERACT-2587-remove-clientcharge-inactivate-writepath branch 2 times, most recently from 0058bb8 to b14a429 Compare April 20, 2026 15:30
Copy link
Copy Markdown
Contributor

@IOhacker IOhacker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AshharAhmadKhan
Copy link
Copy Markdown
Contributor Author

Hi @adamsaghy @IOhacker , changeset renumbered from 0229 to 0230, all conflicts resolved!

@adamsaghy
Copy link
Copy Markdown
Contributor

@AshharAhmadKhan Please rebase it one more time (some earlier PRs were merged)

@adamsaghy adamsaghy closed this Apr 22, 2026
@adamsaghy adamsaghy reopened this Apr 22, 2026
@AshharAhmadKhan AshharAhmadKhan force-pushed the FINERACT-2587-remove-clientcharge-inactivate-writepath branch from b14a429 to 49d94eb Compare April 22, 2026 15:08
@AshharAhmadKhan
Copy link
Copy Markdown
Contributor Author

Hi @adamsaghy , changeset renumbered from 0230 to 0231. Happy to accommodate any further changes if needed!

@adamsaghy adamsaghy merged commit 737f3e1 into apache:develop Apr 22, 2026
43 checks passed
@IOhacker
Copy link
Copy Markdown
Contributor

@AshharAhmadKhan I notice two items after trying to run this on an existing tenant,

Change set author must be "fineract" and there is an issue with a Foreing keys, could you please raise a PR for fixing the issues reported?

fineract-server-1 | Caused by: liquibase.exception.MigrationFailedException: Migration failed for changeset db/changelog/tenant/parts/0231_remove_clientcharge_inactivate_permissions.xml::1::ashharahmadkhan:
fineract-server-1 | Reason: liquibase.exception.DatabaseException: ERROR: update o delete en «m_permission» viola la llave foránea «fk8dedb048103b544b» en la tabla «m_role_permission»
fineract-server-1 | Detail: La llave (id)=(672) todavía es referida desde la tabla «m_role_permission». [Failed SQL: (0) DELETE FROM public.m_permission WHERE code='INACTIVATE_CLIENTCHARGE']
fineract-server-1 | at liquibase.changelog.ChangeSet.execute(ChangeSet.java:873)
fineract-server-1 | at liquibase.changelog.visitor.UpdateVisitor.executeAcceptedChange(UpdateVisitor.java:127)
fineract-server-1 | at liquibase.changelog.visitor.UpdateVisitor.visit(UpdateVisitor.java:71)
fineract-server-1 | at liquibase.changelog.ChangeLogIterator.lambda$run$0(ChangeLogIterator.java:133)
fineract-server-1 | at liquibase.Scope.lambda$child$0(Scope.java:216)
fineract-server-1 | at liquibase.Scope.child(Scope.java:225)
fineract-server-1 | at liquibase.Scope.child(Scope.java:215)
fineract-server-1 | at liquibase.Scope.child(Scope.java:194)
fineract-server-1 | at liquibase.changelog.ChangeLogIterator.lambda$run$1(ChangeLogIterator.java:122)
fineract-server-1 | at liquibase.Scope.lambda$child$0(Scope.java:216)
fineract-server-1 | at liquibase.Scope.child(Scope.java:225)
fineract-server-1 | at liquibase.Scope.child(Scope.java:215)
fineract-server-1 | at liquibase.Scope.child(Scope.java:194)
fineract-server-1 | at liquibase.Scope.child(Scope.java:282)
fineract-server-1 | at liquibase.Scope.child(Scope.java:286)
fineract-server-1 | at liquibase.changelog.ChangeLogIterator.run(ChangeLogIterator.java:91)
fineract-server-1 | ... 32 common frames omitted
fineract-server-1 | Caused by: liquibase.exception.DatabaseException: ERROR: update o delete en «m_permission» viola la llave foránea «fk8dedb048103b544b» en la tabla «m_role_permission»
fineract-server-1 | Detail: La llave (id)=(672) todavía es referida desde la tabla «m_role_permission». [Failed SQL: (0) DELETE FROM public.m_permission WHERE code='INACTIVATE_CLIENTCHARGE']

fineract-server-1 | Detail: La llave (id)=(678) todavía es referida desde la tabla «m_role_permission». [Failed SQL: (0) DELETE FROM public.m_permission WHERE code='INACTIVATE_CLIENTCHARGE_CHECKER']
fineract-server-1 | at liquibase.executor.jvm.JdbcExecutor$ExecuteStatementCallback.doInStatement(JdbcExecutor.java:520)
fineract-server-1 | at liquibase.executor.jvm.JdbcExecutor.execute(JdbcExecutor.java:85)
fineract-server-1 | at liquibase.executor.jvm.JdbcExecutor.execute(JdbcExecutor.java:188)
fineract-server-1 | at liquibase.executor.AbstractExecutor.execute(AbstractExecutor.java:148)
fineract-server-1 | at liquibase.database.AbstractJdbcDatabase.executeStatements(AbstractJdbcDatabase.java:1198)
fineract-server-1 | at liquibase.changelog.ChangeSet.execute(ChangeSet.java:816)
fineract-server-1 | ... 47 common frames omitted
fineract-server-1 | Caused by: org.postgresql.util.PSQLException: ERROR: update o delete en «m_permission» viola la llave foránea «fk8dedb048103b544b» en la tabla «m_role_permission»
fineract-server-1 | Detail: La llave (id)=(678) todavía es referida desde la tabla «m_role_permission».

@AshharAhmadKhan
Copy link
Copy Markdown
Contributor Author

Thanks for catching this, @IOhacker

I’ll put up a follow-up PR to address both issues shortly. Appreciate you validating this on an existing tenant and reporting it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants