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);