FINERACT-2778: Allow teller and vault financial activity mappings to be updated

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

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