bharathcgowda commented on PR #5780:
URL: https://github.com/apache/fineract/pull/5780#issuecomment-4341580674
> Hi @alberto-art3ch
>
> In Colombia, VAT (IVA, 19%) on insurance and similar loan charges is
**passed through to the customer**: the customer pays `base + VAT` as a single
line in the repayment schedule. The bank collects the full amount and remits
the VAT portion to the tax authority.
>
> With the current PR, after `applyTaxIfConfigured`:
>
> * `m_loan_charge.amount` = base
> * `m_loan_charge.tax_amount` = computed tax
> * `feeChargesDue` on the schedule = base only
>
> This works perfectly for tax models where the bank absorbs the tax (e.g.
savings withholding tax). But for pass-through tax, the customer's
`feeChargesDue` is short by the tax amount — the VAT lives only in journal
entries and doesn't surface in what the customer must pay in the schedule.
>
> ### Proposal
> A `tax_added_to_amount` boolean on `m_tax_group` (default `false` to
preserve current behavior). When `true`, `applyTaxIfConfigured` sums the tax
into `amount` so the outstanding reflects what the customer actually owes:
>
> ```java
> // LoanChargeService.applyTaxIfConfigured
> loanCharge.setTaxAmount(totalTax);
> if (taxGroup.isTaxAddedToAmount()) {
> }
> loanCharge.setAmountOutstanding(loanCharge.calculateOutstanding());
> loanCharge.getTaxDetails().clear();
> taxSplit.forEach((component, taxAmt) ->
> loanCharge.getTaxDetails().add(new LoanChargeTaxDetails(loanCharge,
component, taxAmt)));
> ```
>
> Schema delta:
>
> ```
> <changeSet author="fineract" id="loan-charge-tax-3">
> <addColumn tableName="m_tax_group">
> <column name="tax_added_to_amount" type="BOOLEAN"
defaultValueBoolean="false">
> <constraints nullable="false"/>
> </column>
> </addColumn>
> </changeSet>
> ```
>
> The flag is queryable via `taxGroup.isTaxAddedToAmount()`
(Lombok-generated getter) and exposed in `TaxGroupData` for the API. The
accounting separation (your `LoanCommonAccountingHelper` and
`ChargeTaxPaymentDTO` work) stays unchanged — `tax_amount` is still persisted
separately, journal entries still split fee vs tax. Only the customer-facing
outstanding changes.
>
> I'd be happy to open a follow-up PR with the migration, the flag, the API
exposure on `TaxGroupData`, and tests covering both modes — once this PR lands,
or earlier if you'd prefer a single combined PR.
>
> Independent issue, much smaller. The call is:
>
> ```java
> chargeTaxApplicationService.computeTax(taxGroup, loanCharge.getAmount(),
effectiveDate, 6);
> ```
>
> The `6` is the scale of the underlying `DECIMAL(19,6)` column, but
`splitTax` uses it to round the **value** itself. For a currency like COP with
`digitsAfterDecimal=0` (no cents), tax ends up as `39527.917300` instead of
`39528`. The customer-visible amount in the API shows 6 decimals that the
currency doesn't support.
>
> ### Proposal
> Use the loan currency's scale instead, with a sensible fallback:
>
> ```java
> int scale = (loanCharge.getLoan() != null &&
loanCharge.getLoan().getCurrency() != null)
> ? loanCharge.getLoan().getCurrency().getDigitsAfterDecimal()
> : 2;
> Map<TaxComponent, BigDecimal> taxSplit =
> chargeTaxApplicationService.computeTax(taxGroup,
loanCharge.getAmount(), effectiveDate, scale);
> ```
>
> This change is risk-free and aligns with how Money/MoneyHelper rounds
elsewhere in the core. Could land as a small commit on this PR or as a tiny
separate fix.
@jonattanvargas-habi The enhancement you have mentioned is not part of this
ticket and I believe we can handle it through different story. Could you please
create a new enhancement ticket on fineract JIRA?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
