Skip to content

FINERACT-2545: Implement client charge inactivation#5655

Open
AshharAhmadKhan wants to merge 1 commit intoapache:developfrom
AshharAhmadKhan:FINERACT-2545-implement-client-charge-inactivate
Open

FINERACT-2545: Implement client charge inactivation#5655
AshharAhmadKhan wants to merge 1 commit intoapache:developfrom
AshharAhmadKhan:FINERACT-2545-implement-client-charge-inactivate

Conversation

@AshharAhmadKhan
Copy link
Contributor

Description

POST /clients/{clientId}/charges/{chargeId}?command=inactivate has never worked. The API accepted the command and built a CommandWrapper with entity=CLIENTCHARGE and action=INACTIVATE, but no command handler was ever registered for the key "CLIENTCHARGE|INACTIVATE". The dispatcher threw UnsupportedCommandException immediately, returning HTTP 500 to every caller. The service method ClientChargeWritePlatformServiceImpl.inactivateCharge() was also a stub returning null with the comment // functionality not yet supported.

All the surrounding infrastructure was already in place — the DB columns is_active and inactivated_on_date exist in m_client_charge, the permission INACTIVATE_CLIENTCHARGE is seeded in 0002_initial_data.xml, the builder method inactivateClientCharge() exists in CommandWrapperBuilder, and the API routing at ClientChargesApiResource line 212 correctly accepts the command. The handler and service implementation were simply never completed.

Savings charges have had a fully working equivalent (InactivateSavingsAccountChargeCommandHandler) since MIFOSX-1408. Client charges were left behind.

Changes:

InactivateClientChargeCommandHandler.java — new handler registered under CLIENTCHARGE|INACTIVATE, modelled on WaiveClientChargeCommandHandler.

ClientCharge.java — added inactivate(LocalDate) domain method setting status = false and inactivationDate. The DB columns already existed, the method was simply missing.

ClientChargeWritePlatformServiceImpl.java — replaced the return null stub with a full implementation. Guards for already-inactive, already-waived, and already-paid states, followed by clientCharge.inactivate() and saveAndFlush.

ClientHelper.java — added inactivateChargesForClients() and getClientChargeField() test helpers.

ClientChargesTest.java — added clientChargeInactivateTest() covering the success path, the isActive field assertion post-inactivation, and the idempotency failure (inactivating an already-inactive charge must return 400).

Checklist

  • Write the commit message as per our guidelines
  • Acknowledge that we will not review PRs that are not passing the build ("green") - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
  • Create/update unit or integration tests for verifying the changes made.
  • Follow our coding conventions.
  • Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
  • This PR must not be a "code dump". Large changes can be made in a branch, with assistance.

@AshharAhmadKhan
Copy link
Contributor Author

Pushed a follow-up commit after deeper review of state transitions and API completeness.

The first commit fixed the core bug - the missing handler and stub service. While verifying the full operation lifecycle I found three additional issues worth addressing in the same PR rather than leaving as separate tickets.

undoPayment() and undoWaiver() were unconditionally setting status = true, meaning undoing a prior payment on an inactivated charge would silently reactivate it. Fixed by checking inactivationDate == null before restoring active status, and added a service-level guard in ClientTransactionWritePlatformServiceJpaRepositoryImpl to block undo operations on inactive charges entirely - inactivation should be a terminal state.

isActive and inactivationDate were missing from CLIENT_CHARGES_RESPONSE_DATA_PARAMETERS, so ?fields=isActive returned nothing. Added both constants to the whitelist.

Updated the @Operation summary to document the inactivate command alongside pay and waive.

Tests added for the undo guard and the fields filter. All existing tests untouched.

POST /clients/{clientId}/charges/{chargeId}?command=inactivate was
broken end-to-end. The API accepted the command and built a
CommandWrapper with entity=CLIENTCHARGE action=INACTIVATE, but no
command handler was registered for that key, causing
CommandHandlerProvider to throw UnsupportedCommandException on every
call. The service method was also a stub returning null with a
'functionality not yet supported' comment.

This commit completes the originally intended implementation:
- Add InactivateClientChargeCommandHandler wired to CLIENTCHARGE|INACTIVATE
- Add ClientCharge.inactivate() domain method setting status=false
  and inactivationDate
- Implement ClientChargeWritePlatformServiceImpl.inactivateCharge()
  with guards for already-inactive, already-waived, and already-paid
- Add integration test covering success path and idempotency failure

FINERACT-2545: Fix state integrity and documentation gaps

Three follow-up fixes identified through deep domain analysis:

1. Add isActive and inactivationDate to CLIENT_CHARGES_RESPONSE_DATA_PARAMETERS
   so these fields are returned when using the ?fields= query parameter filter.

2. Fix ClientCharge.undoPayment() and undoWaiver() to not restore active
   status when a charge has been explicitly inactivated (inactivationDate != null).
   Previously, undoing a payment on an inactivated charge would silently
   reactivate it, violating state integrity.

3. Add service-level guard in ClientTransactionWritePlatformServiceJpaRepositoryImpl
   to block undo operations on inactive charges entirely, enforcing inactivation
   as a terminal state at both domain and service layers.

4. Update @operation Swagger annotation on payOrWaiveClientCharge to document
   the inactivate command alongside pay and waive.

Tests added: undo blocked on inactive charge, isActive returned correctly
via explicit ?fields= filter.
@AshharAhmadKhan AshharAhmadKhan force-pushed the FINERACT-2545-implement-client-charge-inactivate branch from 7524369 to fa37023 Compare March 20, 2026 16:00
@AshharAhmadKhan
Copy link
Contributor Author

All checks should now be passing after squashing into a single commit.

Happy to make any adjustments based on feedback or direction from the ongoing discussion.

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.

1 participant