Farooq Ayoade created FINERACT-2778:
---------------------------------------
Summary: Financial activity account update rejects cashAtTeller
and cashAtMainVault, which create accepts
Key: FINERACT-2778
URL: https://issues.apache.org/jira/browse/FINERACT-2778
Project: Apache Fineract
Issue Type: Bug
Components: Accounting
Reporter: Farooq Ayoade
h3. Summary
{{FinancialActivityAccountDataValidator}} validates the same parameter against
two different lists.
{{validateForCreate}} accepts all seven members of the {{FinancialActivity}}
enum, {{CASH_AT_MAINVAULT}}
(101) and {{CASH_AT_TELLER}} (102) among them. {{validateForUpdate}} omits
exactly those two and accepts
only {{[100, 200, 300, 103, 201]}}.
So the platform will happily create a teller or main-vault mapping and then
refuse every attempt to change
it:
{code}
PUT /financialactivityaccounts/{id}
{"financialActivityId":102,"glAccountId":805}
400
validation.msg.financialactivityaccount.financialActivityId.is.not.one.of.expected.enumerations
The parameter `financialActivityId` must be one of [ 100, 200, 300, 103,
201 ] .
{code}
The two halves write the same column and the mapping is read back the same way
whichever route created it
({{TellerWritePlatformServiceJpaImpl}} resolves both through
{{findByFinancialActivityTypeWithNotFoundDetection}}). There is no stated
reason for them to disagree, and
no note in the code suggesting the omission is deliberate.
h3. Impact
These two activities are not decorative: they are the GL accounts the
teller/cashier module posts cash
against. A deployment that maps them to the wrong account — easy to do, since
the mapping is set once at
configuration time — has every cashier and vault movement landing on the wrong
GL account, and no supported
way to correct it.
The remaining options are both bad:
* delete the mapping and create a replacement, which discards the row and its
identity, or
* {{UPDATE}} {{acc_gl_financial_activity_account}} directly in the database,
bypassing the platform's
validation and its audit trail.
On the tenant where this was found, {{cashAtTeller}} resolved to an account
named "Sms Unpaid" and
{{cashAtMainVault}} to "LOAN - PROVIDER'S ADVANCE - UNPAID INTEREST". Neither
could be repointed through the
API.
h3. Steps to reproduce
# {{POST /financialactivityaccounts}} with {{{}"financialActivityId": 102{}}}
(cashAtTeller) and any asset
GL account id. It succeeds — create accepts 102.
# {{PUT /financialactivityaccounts/{id\}}} with {{{}"financialActivityId":
102{}}} and a different asset GL
account id.
# The request fails with HTTP 400 and
{{validation.msg.financialactivityaccount.financialActivityId.is.not.one.of.expected.enumerations}},
naming
an expected set that does not contain the value the resource was created with.
The same holds for 101 (cashAtMainVault).
h3. Root cause
{{fineract-accounting/src/main/java/org/apache/fineract/accounting/financialactivityaccount/serialization/FinancialActivityAccountDataValidator.java}}
{{validateForCreate}} lists seven constants:
{code}
FinancialActivity.ASSET_TRANSFER, FinancialActivity.LIABILITY_TRANSFER,
FinancialActivity.CASH_AT_MAINVAULT, FinancialActivity.CASH_AT_TELLER,
FinancialActivity.OPENING_BALANCES_TRANSFER_CONTRA,
FinancialActivity.ASSET_FUND_SOURCE,
FinancialActivity.PAYABLE_DIVIDENDS
{code}
{{validateForUpdate}} lists five — the same set with {{CASH_AT_MAINVAULT}} and
{{CASH_AT_TELLER}} dropped.
Both lists are written out by hand rather than derived from the enum, which is
how they came to differ.
h3. Why it was not caught
{{FinancialActivityAccountsTest}} exercises the update path only with
{{LIABILITY_TRANSFER}}, and its
negative case uses activity id {{232}} — a value that is not in the enum at
all. A value that is in the
enum, is accepted by create, and is rejected by update is precisely the case
the test never covers.
h3. Proposed fix
Add the two missing constants to {{validateForUpdate}} so both halves accept
the same set, and extend
{{FinancialActivityAccountsTest}} to create a {{CASH_AT_TELLER}} mapping and
update it, which fails before
the change and passes after.
Deriving both lists from {{FinancialActivity.values()}} would prevent a
recurrence and is the tidier change,
but it widens the diff beyond the defect; happy to do it that way if reviewers
prefer.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
