rymghosn commented on PR #6237:
URL: https://github.com/apache/fineract/pull/6237#issuecomment-6337201185

   @adamsaghy Thanks, all confirmed. Rather than patching the gross-up model, I 
reworked the PR (a2e37eabb0, rebased on the latest `develop`) so savings 
charges are taxed the way loan charges already are on `develop` since 
FINERACT-1289: the tax is part of the charge amount. Point by point:
   
   - **`payCharge` paying the gross amount / charge never marked paid / 
recurring due dates stuck:** the charge is paid exactly its own amount now, and 
the tax is only split out of that amount and stored on the transaction 
(`m_savings_account_transaction_tax_details`). So the outstanding reaches zero, 
the charge is marked paid, and recurring fees move on as before. The IT checks 
amount paid 100 and outstanding 0.
   - **Overdraft split posting the tax twice:** each leg now takes its pro-rata 
share of the tax (`TaxPaymentDTO.share` / `remainder`, unit-tested including 
rounding). In the IT, a 100 fee with 40 in the account gives income 54 + 36 and 
tax 6 + 4.
   - **Tax component without a credit account:** such a component is no longer 
split out, so its tax stays in income and the entries balance (the IT checks 
income 100). That matches how loan charge tax treats a component without a 
credit account.
   - **No tests:** I added `SavingsChargeTaxIntegrationTest` (cash, undo, 
overdraft, missing credit account, accrual). On unmodified `develop` the three 
tax cases fail, and with the change all four pass. The existing loan charge 
tax, accounting scenario, savings charge and `ClientSavingsIntegrationTest` 
suites (withholding tax included) still pass against PostgreSQL.
   - **Stale description:** rewritten. `calculateChargeAmountWithTax`, the 
TaxComponent/History mapping and the empty-components validation are gone from 
the PR; the latter two are on `develop` through FINERACT-2766 / FINERACT-2748. 
The `TaxGroupComponent` DTO change is gone too.
   - **Conflict with #6285:** #6285 is closed, since it turned out `develop` 
already covers it, so there's no shared `TaxUtils` method any more.
   
   Could you please re-review? Thanks!
   


-- 
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]

Reply via email to