Dhanno98 commented on PR #5940: URL: https://github.com/apache/fineract/pull/5940#issuecomment-4852432541
> Technically, I see one real blocker: the PR fixes `ShareAccountTransaction.createChargeTransaction()` to use the rounded/applied amount, but activation charges still create `ShareAccountChargePaidBy` with `charge.percentageOrAmount()` in `ShareAccountDataSerializer`. > > That means for a flat activation charge like 19.8 rounded to 20: `share transaction amount = 20` `chargesPaid.amount = 19.8` > > That is dangerous because share accounting consumes `chargesPaid.amount` and checks that the charge-payment breakdown totals match the transaction amount. With accounting enabled, this can trip the “charges for shares have broken accounting code” integrity exception. > > To be fixed: in the activation charge path, pass the derived amount into `ShareAccountChargePaidBy`, not `charge.percentageOrAmount()`. > > Please also add a test covering activation share charges with accounting enabled, because the current rounding tests appear to use non-accounting share products and won’t catch this. Thanks for catching that! I'll update the activation charge path to use the derived amount and add an accounting-enabled test. -- 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]
