From 8b71afc47d9a2f2dc1dd4227f7119cba7cf30715 Mon Sep 17 00:00:00 2001 From: oluexpert99 Date: Sun, 23 Aug 2026 13:30:11 +0100 Subject: [PATCH] FINERACT-2778: Allow teller and vault financial activity mappings to be updated FinancialActivityAccountDataValidator validates the same parameter against two hand-written lists. validateForCreate accepts all seven FinancialActivity constants; validateForUpdate accepted five, omitting CASH_AT_MAINVAULT (101) and CASH_AT_TELLER (102). The platform will therefore create a teller or vault mapping and then refuse every attempt to change it: PUT /financialactivityaccounts/{id} {"financialActivityId":102,...} 400 validation.msg.financialactivityaccount.financialActivityId .is.not.one.of.expected.enumerations must be one of [ 100, 200, 300, 103, 201 ] Both halves write the same column, and the teller module resolves the mapping through findByFinancialActivityTypeWithNotFoundDetection whichever route created it, so the two lists have no reason to differ. These are the GL accounts the cashier module posts cash against. A deployment that maps them wrongly, which is easy since they are set once at configuration time, has every teller and vault movement landing on the wrong account and no supported way to correct it. The remaining options are to delete the mapping and create a replacement, discarding the row and its identity, or to update the database directly, bypassing validation and the audit trail. FinancialActivityAccountsTest exercised the update path only with LIABILITY_TRANSFER, and its negative case used activity id 232, which is not in the enum at all. A value that is in the enum, is accepted by create and is refused by update fell between the two cases. The test now creates a CASH_AT_TELLER mapping and updates it. Signed-off-by: oluexpert99 --- ...FinancialActivityAccountDataValidator.java | 8 +++++ .../FinancialActivityAccountsTest.java | 29 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/fineract-accounting/src/main/java/org/apache/fineract/accounting/financialactivityaccount/serialization/FinancialActivityAccountDataValidator.java b/fineract-accounting/src/main/java/org/apache/fineract/accounting/financialactivityaccount/serialization/FinancialActivityAccountDataValidator.java index e69cddba766..078a7d4d2cf 100644 --- a/fineract-accounting/src/main/java/org/apache/fineract/accounting/financialactivityaccount/serialization/FinancialActivityAccountDataValidator.java +++ b/fineract-accounting/src/main/java/org/apache/fineract/accounting/financialactivityaccount/serialization/FinancialActivityAccountDataValidator.java @@ -86,8 +86,16 @@ public void validateForUpdate(final String json) { if (this.fromApiJsonHelper.parameterExists(paramNameForFinancialActivity, element)) { final Integer financialActivityId = this.fromApiJsonHelper.extractIntegerSansLocaleNamed(paramNameForFinancialActivity, element); + // CASH_AT_MAINVAULT and CASH_AT_TELLER are accepted by validateForCreate above but were missing + // here, so a teller or vault mapping could be created and then never corrected: the update was + // rejected with "must be one of [ 100, 200, 300, 103, 201 ]". There is no reason for the two + // halves to disagree -- both write the same column, and the teller module reads the mapping via + // findByFinancialActivityTypeWithNotFoundDetection whichever way the row got there. Encountered on + // a tenant whose cashAtTeller pointed at an unrelated GL account, where the only ways out were to + // delete and recreate the row, losing its identity, or to UPDATE the database by hand. baseDataValidator.reset().parameter(paramNameForFinancialActivity).value(financialActivityId).ignoreIfNull().isOneOfTheseValues( FinancialActivity.ASSET_TRANSFER.getValue(), FinancialActivity.LIABILITY_TRANSFER.getValue(), + FinancialActivity.CASH_AT_MAINVAULT.getValue(), FinancialActivity.CASH_AT_TELLER.getValue(), FinancialActivity.OPENING_BALANCES_TRANSFER_CONTRA.getValue(), FinancialActivity.ASSET_FUND_SOURCE.getValue(), FinancialActivity.PAYABLE_DIVIDENDS.getValue()); } diff --git a/integration-tests/src/test/java/org/apache/fineract/integrationtests/FinancialActivityAccountsTest.java b/integration-tests/src/test/java/org/apache/fineract/integrationtests/FinancialActivityAccountsTest.java index 87ca0e0880f..5bccaad8d05 100644 --- a/integration-tests/src/test/java/org/apache/fineract/integrationtests/FinancialActivityAccountsTest.java +++ b/integration-tests/src/test/java/org/apache/fineract/integrationtests/FinancialActivityAccountsTest.java @@ -52,6 +52,7 @@ public class FinancialActivityAccountsTest { private FinancialActivityAccountHelper financialActivityAccountHelper; private final Integer assetTransferFinancialActivityId = FinancialActivity.ASSET_TRANSFER.getValue(); public static final Integer LIABILITY_TRANSFER_FINANCIAL_ACTIVITY_ID = FinancialActivity.LIABILITY_TRANSFER.getValue(); + public static final Integer CASH_AT_TELLER_FINANCIAL_ACTIVITY_ID = FinancialActivity.CASH_AT_TELLER.getValue(); @BeforeEach public void setup() { @@ -136,6 +137,34 @@ public void testFinancialActivityAccounts() { financialActivityAccountHelper.getFinancialActivityAccount(deletedFinancialActivityAccountId, responseSpecForResourceNotFoundError); } + /** + * A financial activity that create accepts must also be updatable. + * + * CASH_AT_TELLER and CASH_AT_MAINVAULT were accepted by validateForCreate and refused by validateForUpdate, so a + * teller or vault mapping could be created and then never corrected. The case above never caught it: it updates + * only LIABILITY_TRANSFER, and its negative case uses an id that is not in the enum at all, so a value that is in + * the enum, is accepted by create and is rejected by update fell exactly between the two. + */ + @Test + public void testTellerFinancialActivityAccountCanBeUpdated() { + Account tellerCashAccount = accountHelper.createAssetAccount(); + Account replacementTellerCashAccount = accountHelper.createAssetAccount(); + Assertions.assertNotNull(tellerCashAccount); + Assertions.assertNotNull(replacementTellerCashAccount); + + Integer financialActivityAccountId = (Integer) financialActivityAccountHelper.createFinancialActivityAccount( + CASH_AT_TELLER_FINANCIAL_ACTIVITY_ID, tellerCashAccount.getAccountID(), responseSpec, CommonConstants.RESPONSE_RESOURCE_ID); + Assertions.assertNotNull(financialActivityAccountId); + assertFinancialActivityAccountCreation(financialActivityAccountId, CASH_AT_TELLER_FINANCIAL_ACTIVITY_ID, tellerCashAccount); + + HashMap changes = (HashMap) financialActivityAccountHelper.updateFinancialActivityAccount(financialActivityAccountId, + CASH_AT_TELLER_FINANCIAL_ACTIVITY_ID, replacementTellerCashAccount.getAccountID(), responseSpec, + CommonConstants.RESPONSE_CHANGES); + Assertions.assertEquals(replacementTellerCashAccount.getAccountID(), changes.get("glAccountId")); + assertFinancialActivityAccountCreation(financialActivityAccountId, CASH_AT_TELLER_FINANCIAL_ACTIVITY_ID, + replacementTellerCashAccount); + } + private void assertFinancialActivityAccountCreation(Integer financialActivityAccountId, Integer financialActivityId, Account glAccount) { HashMap mappingDetails = financialActivityAccountHelper.getFinancialActivityAccount(financialActivityAccountId, responseSpec);