adamsaghy commented on code in PR #3973:
URL: https://github.com/apache/fineract/pull/3973#discussion_r1680883629


##########
fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/AccrualBasedAccountingProcessorForSavings.java:
##########
@@ -217,48 +224,48 @@ else if 
(savingsTransactionDTO.getTransactionType().isFeeDeduction() && savingsT
                             feePayments);
                     if (isPositive) {
                         
this.helper.createAccrualBasedJournalEntriesAndReversalsForSavingsCharges(office,
 currencyCode,
-                                AccrualAccountsForSavings.SAVINGS_CONTROL, 
AccrualAccountsForSavings.INCOME_FROM_FEES, savingsProductId,
-                                paymentTypeId, savingsId, transactionId, 
transactionDate, amount.subtract(overdraftAmount), isReversal,
-                                feePayments);
+                                accountTypeToBeDebited, 
AccrualAccountsForSavings.INCOME_FROM_FEES, savingsProductId, paymentTypeId,
+                                savingsId, transactionId, transactionDate, 
amount.subtract(overdraftAmount), isReversal, feePayments);
                     }
                 }
             }
 
             else if 
(savingsTransactionDTO.getTransactionType().isFeeDeduction()) {
+                AccrualAccountsForSavings accountTypeToBeCredited = 
AccrualAccountsForSavings.INCOME_FROM_PENALTIES;
                 // Is the Charge a penalty?
                 if (penaltyPayments.size() > 0) {
                     
this.helper.createAccrualBasedJournalEntriesAndReversalsForSavingsCharges(office,
 currencyCode,
-                            AccrualAccountsForSavings.SAVINGS_CONTROL, 
AccrualAccountsForSavings.INCOME_FROM_PENALTIES, savingsProductId,
-                            paymentTypeId, savingsId, transactionId, 
transactionDate, amount, isReversal, penaltyPayments);
+                            AccrualAccountsForSavings.SAVINGS_CONTROL, 
accountTypeToBeCredited, savingsProductId, paymentTypeId, savingsId,
+                            transactionId, transactionDate, amount, 
isReversal, penaltyPayments);
                 } else {
                     
this.helper.createAccrualBasedJournalEntriesAndReversalsForSavingsCharges(office,
 currencyCode,
-                            AccrualAccountsForSavings.SAVINGS_CONTROL, 
AccrualAccountsForSavings.INCOME_FROM_FEES, savingsProductId,
-                            paymentTypeId, savingsId, transactionId, 
transactionDate, amount, isReversal, feePayments);
+                            AccrualAccountsForSavings.SAVINGS_CONTROL, 
accountTypeToBeCredited, savingsProductId, paymentTypeId, savingsId,
+                            transactionId, transactionDate, amount, 
isReversal, feePayments);
                 }
             }
 
             /** Handle Transfers proposal **/
             else if 
(savingsTransactionDTO.getTransactionType().isInitiateTransfer()) {
-                
this.helper.createCashBasedJournalEntriesAndReversalsForSavings(office, 
currencyCode,

Review Comment:
   Why do you override the cash based journal entry handling?



##########
fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/AccrualBasedAccountingProcessorForSavings.java:
##########
@@ -217,48 +224,48 @@ else if 
(savingsTransactionDTO.getTransactionType().isFeeDeduction() && savingsT
                             feePayments);
                     if (isPositive) {
                         
this.helper.createAccrualBasedJournalEntriesAndReversalsForSavingsCharges(office,
 currencyCode,
-                                AccrualAccountsForSavings.SAVINGS_CONTROL, 
AccrualAccountsForSavings.INCOME_FROM_FEES, savingsProductId,
-                                paymentTypeId, savingsId, transactionId, 
transactionDate, amount.subtract(overdraftAmount), isReversal,
-                                feePayments);
+                                accountTypeToBeDebited, 
AccrualAccountsForSavings.INCOME_FROM_FEES, savingsProductId, paymentTypeId,
+                                savingsId, transactionId, transactionDate, 
amount.subtract(overdraftAmount), isReversal, feePayments);
                     }
                 }
             }
 
             else if 
(savingsTransactionDTO.getTransactionType().isFeeDeduction()) {
+                AccrualAccountsForSavings accountTypeToBeCredited = 
AccrualAccountsForSavings.INCOME_FROM_PENALTIES;
                 // Is the Charge a penalty?
                 if (penaltyPayments.size() > 0) {
                     
this.helper.createAccrualBasedJournalEntriesAndReversalsForSavingsCharges(office,
 currencyCode,
-                            AccrualAccountsForSavings.SAVINGS_CONTROL, 
AccrualAccountsForSavings.INCOME_FROM_PENALTIES, savingsProductId,
-                            paymentTypeId, savingsId, transactionId, 
transactionDate, amount, isReversal, penaltyPayments);
+                            AccrualAccountsForSavings.SAVINGS_CONTROL, 
accountTypeToBeCredited, savingsProductId, paymentTypeId, savingsId,
+                            transactionId, transactionDate, amount, 
isReversal, penaltyPayments);
                 } else {
                     
this.helper.createAccrualBasedJournalEntriesAndReversalsForSavingsCharges(office,
 currencyCode,
-                            AccrualAccountsForSavings.SAVINGS_CONTROL, 
AccrualAccountsForSavings.INCOME_FROM_FEES, savingsProductId,
-                            paymentTypeId, savingsId, transactionId, 
transactionDate, amount, isReversal, feePayments);
+                            AccrualAccountsForSavings.SAVINGS_CONTROL, 
accountTypeToBeCredited, savingsProductId, paymentTypeId, savingsId,
+                            transactionId, transactionDate, amount, 
isReversal, feePayments);
                 }
             }
 
             /** Handle Transfers proposal **/
             else if 
(savingsTransactionDTO.getTransactionType().isInitiateTransfer()) {
-                
this.helper.createCashBasedJournalEntriesAndReversalsForSavings(office, 
currencyCode,
+                
this.helper.createAccrualBasedJournalEntriesAndReversalsForSavings(office, 
currencyCode,
                         AccrualAccountsForSavings.SAVINGS_CONTROL.getValue(), 
AccrualAccountsForSavings.TRANSFERS_SUSPENSE.getValue(),
                         savingsProductId, paymentTypeId, savingsId, 
transactionId, transactionDate, amount, isReversal);
             }
 
             /** Handle Transfer Withdrawal or Acceptance **/
             else if 
(savingsTransactionDTO.getTransactionType().isWithdrawTransfer()
                     || 
savingsTransactionDTO.getTransactionType().isApproveTransfer()) {
-                
this.helper.createCashBasedJournalEntriesAndReversalsForSavings(office, 
currencyCode,
+                
this.helper.createAccrualBasedJournalEntriesAndReversalsForSavings(office, 
currencyCode,

Review Comment:
   Why do you override the cash based journal entry handling?



##########
fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/AccrualBasedAccountingProcessorForSavings.java:
##########
@@ -217,48 +224,48 @@ else if 
(savingsTransactionDTO.getTransactionType().isFeeDeduction() && savingsT
                             feePayments);
                     if (isPositive) {
                         
this.helper.createAccrualBasedJournalEntriesAndReversalsForSavingsCharges(office,
 currencyCode,
-                                AccrualAccountsForSavings.SAVINGS_CONTROL, 
AccrualAccountsForSavings.INCOME_FROM_FEES, savingsProductId,
-                                paymentTypeId, savingsId, transactionId, 
transactionDate, amount.subtract(overdraftAmount), isReversal,
-                                feePayments);
+                                accountTypeToBeDebited, 
AccrualAccountsForSavings.INCOME_FROM_FEES, savingsProductId, paymentTypeId,
+                                savingsId, transactionId, transactionDate, 
amount.subtract(overdraftAmount), isReversal, feePayments);
                     }
                 }
             }
 
             else if 
(savingsTransactionDTO.getTransactionType().isFeeDeduction()) {
+                AccrualAccountsForSavings accountTypeToBeCredited = 
AccrualAccountsForSavings.INCOME_FROM_PENALTIES;
                 // Is the Charge a penalty?
                 if (penaltyPayments.size() > 0) {
                     
this.helper.createAccrualBasedJournalEntriesAndReversalsForSavingsCharges(office,
 currencyCode,
-                            AccrualAccountsForSavings.SAVINGS_CONTROL, 
AccrualAccountsForSavings.INCOME_FROM_PENALTIES, savingsProductId,
-                            paymentTypeId, savingsId, transactionId, 
transactionDate, amount, isReversal, penaltyPayments);
+                            AccrualAccountsForSavings.SAVINGS_CONTROL, 
accountTypeToBeCredited, savingsProductId, paymentTypeId, savingsId,
+                            transactionId, transactionDate, amount, 
isReversal, penaltyPayments);
                 } else {
                     
this.helper.createAccrualBasedJournalEntriesAndReversalsForSavingsCharges(office,
 currencyCode,
-                            AccrualAccountsForSavings.SAVINGS_CONTROL, 
AccrualAccountsForSavings.INCOME_FROM_FEES, savingsProductId,
-                            paymentTypeId, savingsId, transactionId, 
transactionDate, amount, isReversal, feePayments);
+                            AccrualAccountsForSavings.SAVINGS_CONTROL, 
accountTypeToBeCredited, savingsProductId, paymentTypeId, savingsId,
+                            transactionId, transactionDate, amount, 
isReversal, feePayments);
                 }
             }
 
             /** Handle Transfers proposal **/
             else if 
(savingsTransactionDTO.getTransactionType().isInitiateTransfer()) {
-                
this.helper.createCashBasedJournalEntriesAndReversalsForSavings(office, 
currencyCode,
+                
this.helper.createAccrualBasedJournalEntriesAndReversalsForSavings(office, 
currencyCode,
                         AccrualAccountsForSavings.SAVINGS_CONTROL.getValue(), 
AccrualAccountsForSavings.TRANSFERS_SUSPENSE.getValue(),
                         savingsProductId, paymentTypeId, savingsId, 
transactionId, transactionDate, amount, isReversal);
             }
 
             /** Handle Transfer Withdrawal or Acceptance **/
             else if 
(savingsTransactionDTO.getTransactionType().isWithdrawTransfer()
                     || 
savingsTransactionDTO.getTransactionType().isApproveTransfer()) {
-                
this.helper.createCashBasedJournalEntriesAndReversalsForSavings(office, 
currencyCode,
+                
this.helper.createAccrualBasedJournalEntriesAndReversalsForSavings(office, 
currencyCode,
                         
AccrualAccountsForSavings.TRANSFERS_SUSPENSE.getValue(), 
AccrualAccountsForSavings.SAVINGS_CONTROL.getValue(),
                         savingsProductId, paymentTypeId, savingsId, 
transactionId, transactionDate, amount, isReversal);
             }
 
             /** overdraft **/
             else if 
(savingsTransactionDTO.getTransactionType().isOverdraftInterest()) {
-                
this.helper.createCashBasedJournalEntriesAndReversalsForSavings(office, 
currencyCode,
+                
this.helper.createAccrualBasedJournalEntriesAndReversalsForSavings(office, 
currencyCode,

Review Comment:
   Why do you override the cash based journal entry handling?



-- 
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: commits-unsubscr...@fineract.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to