You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2021/11/18 16:33:57 UTC
[GitHub] [fineract] avikganguly01 commented on a change in pull request #1933: Feat: Filter transactions from last interest posting date
avikganguly01 commented on a change in pull request #1933:
URL: https://github.com/apache/fineract/pull/1933#discussion_r741337636
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/data/SavingsAccountTransactionDataValidator.java
##########
@@ -67,10 +69,59 @@
private static final Set<String> SAVINGS_ACCOUNT_HOLD_AMOUNT_REQUEST_DATA_PARAMETERS = new HashSet<>(
Arrays.asList(transactionDateParamName, SavingsApiConstants.dateFormatParamName, SavingsApiConstants.localeParamName,
transactionAmountParamName));
+ private final ConfigurationDomainService configurationDomainService;
@Autowired
- public SavingsAccountTransactionDataValidator(final FromJsonHelper fromApiJsonHelper) {
+ public SavingsAccountTransactionDataValidator(final FromJsonHelper fromApiJsonHelper,
+ final ConfigurationDomainService configurationDomainService) {
this.fromApiJsonHelper = fromApiJsonHelper;
+ this.configurationDomainService = configurationDomainService;
+ }
+
+ public void validateLastPostingDate(final JsonCommand command, final SavingsAccount savingsAccount) {
+
+ final JsonElement element = command.parsedJson();
+ final LocalDate transactionDate = this.fromApiJsonHelper.extractLocalDateNamed(transactionDateParamName, element);
+ final boolean isPivotOn = this.configurationDomainService.retrievePivotDate();
+
+ final Date lastInterestPostingDate = savingsAccount.getSummary().getLastTransactionDate();
+
+ if (!isPivotOn) {
+ return;
+ }
+
+ if (lastInterestPostingDate != null) {
+ final LocalDate pivotDate = LocalDate.ofInstant(lastInterestPostingDate.toInstant(), DateUtils.getDateTimeZoneOfTenant());
+
+ boolean isBeforeLastPostingDate = pivotDate.isAfter(transactionDate) ? true : false;
+
+ if (isBeforeLastPostingDate) {
+ throw new TransactionBeforePivotDateNotAllowed(transactionDate, pivotDate);
+ }
+ }
+
+ }
+
+ public void validateLastPostingDate(final LocalDate transactionDate, final SavingsAccount savingsAccount) {
+
+ final boolean isPivotOn = this.configurationDomainService.retrievePivotDate();
+
+ final Date lastInterestPostingDate = savingsAccount.getSummary().getLastTransactionDate();
+
+ if (!isPivotOn) {
+ return;
+ }
+
+ if (lastInterestPostingDate != null) {
+ final LocalDate pivotDate = LocalDate.ofInstant(lastInterestPostingDate.toInstant(), DateUtils.getDateTimeZoneOfTenant());
+
+ boolean isBeforeLastPostingDate = pivotDate.isAfter(transactionDate) ? true : false;
+
+ if (isBeforeLastPostingDate) {
+ throw new TransactionBeforePivotDateNotAllowed(transactionDate, pivotDate);
+ }
Review comment:
Make functions to remove duplicate code.
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java
##########
@@ -1214,6 +1961,70 @@ public void validateAccountBalanceDoesNotBecomeNegative(final String transaction
}
}
+ public void validateSavingsAccountBalanceDoesNotBecomeNegative(final String transactionAction,
+ final List<DepositAccountOnHoldTransaction> depositAccountOnHoldTransactions) {
Review comment:
Business logic doesn't belong in the entity class. Put it in a write or domainService class. This also looks like a lot of duplicate code or code you're unnecessarily moving from a different file which you haven't written
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java
##########
@@ -891,17 +1407,74 @@ protected void resetAccountTransactionsEndOfDayBalances(final List<SavingsAccoun
endOfBalanceDate = transaction.transactionLocalDate().minusDays(1);
}
}
- }
+ }
+
+ public SavingsAccountTransaction deposit(final SavingsAccountTransactionDTO transactionDTO) {
+ return deposit(transactionDTO, SavingsAccountTransactionType.DEPOSIT);
+ }
+
+ public SavingsAccountTransaction dividendPayout(final SavingsAccountTransactionDTO transactionDTO) {
+ return deposit(transactionDTO, SavingsAccountTransactionType.DIVIDEND_PAYOUT);
+ }
+
+ public SavingsAccountTransaction deposit(final SavingsAccountTransactionDTO transactionDTO,
+ final SavingsAccountTransactionType savingsAccountTransactionType) {
+ final String resourceTypeName = depositAccountType().resourceName();
Review comment:
Business logic doesn't belong in the entity class. Put it in a write or domainService class. This also looks like a lot of duplicate code or code you're unnecessarily moving from a different file which you haven't written
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java
##########
@@ -1045,7 +1618,84 @@ public SavingsAccountTransaction withdraw(final SavingsAccountTransactionDTO tra
final SavingsAccountTransaction transaction = SavingsAccountTransaction.withdrawal(this, office(),
transactionDTO.getPaymentDetail(), transactionDTO.getTransactionDate(), transactionAmountMoney,
transactionDTO.getCreatedDate(), transactionDTO.getAppUser());
+
addTransaction(transaction);
+ // addTransactionToExisting(transaction);
+
+ if (this.sub_status.equals(SavingsAccountSubStatusEnum.INACTIVE.getValue())
+ || this.sub_status.equals(SavingsAccountSubStatusEnum.DORMANT.getValue())) {
+ this.sub_status = SavingsAccountSubStatusEnum.NONE.getValue();
+ }
+ return transaction;
+ }
+
+ public SavingsAccountTransaction savingsWithdraw(final SavingsAccountTransactionDTO transactionDTO, final boolean applyWithdrawFee) {
+
Review comment:
Business logic doesn't belong in the entity class. Put it in a write or domainService class. This also looks like a lot of duplicate code or code you're unnecessarily moving from a different file which you haven't written
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountWritePlatformServiceJpaRepositoryImpl.java
##########
@@ -253,11 +256,11 @@ public CommandProcessingResult activate(final Long savingsId, final JsonCommand
final Locale locale = command.extractLocale();
final DateTimeFormatter fmt = DateTimeFormatter.ofPattern(command.dateFormat()).withLocale(locale);
processPostActiveActions(account, fmt, existingTransactionIds, existingReversedTransactionIds);
-
+ // this.savingsAccountTransactionRepository.saveAll(account.getSavingsAccountTransactions());
Review comment:
Is this comment required?
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java
##########
@@ -544,9 +573,18 @@ public void postInterest(final MathContext mc, final LocalDate interestPostingUp
interestPostingTransactionDate, interestEarnedToBePostedForPeriod.negated(),
interestPostingPeriod.isUserPosting());
}
- addTransaction(newPostingTransaction);
- if (applyWithHoldTaxForOldTransaction) {
+ if (isPivotConfigOn) {
+ addTransactionToExisting(newPostingTransaction);
+ } else {
+ addTransaction(newPostingTransaction);
+ }
+ if (applyWithHoldTaxForOldTransaction && !isPivotConfigOn) {
createWithHoldTransaction(interestEarnedToBePostedForPeriod.getAmount(), interestPostingTransactionDate);
+ } else if (applyWithHoldTaxForOldTransaction && isPivotConfigOn) {
+ if (applyWithHoldTaxForOldTransaction) {
Review comment:
Double if. Already checked applyWithHoldTaxForOldTransaction is true in previous line.
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/data/SavingsAccountTransactionDataValidator.java
##########
@@ -67,10 +69,27 @@
private static final Set<String> SAVINGS_ACCOUNT_HOLD_AMOUNT_REQUEST_DATA_PARAMETERS = new HashSet<>(
Arrays.asList(transactionDateParamName, SavingsApiConstants.dateFormatParamName, SavingsApiConstants.localeParamName,
transactionAmountParamName));
+ private final ConfigurationDomainService configurationDomainService;
@Autowired
- public SavingsAccountTransactionDataValidator(final FromJsonHelper fromApiJsonHelper) {
+ public SavingsAccountTransactionDataValidator(final FromJsonHelper fromApiJsonHelper,
+ final ConfigurationDomainService configurationDomainService) {
this.fromApiJsonHelper = fromApiJsonHelper;
+ this.configurationDomainService = configurationDomainService;
+ }
+
+ public void validateTransactionWithPivotDate(final LocalDate transactionDate, final SavingsAccount savingsAccount) {
+
+ final boolean isPivotConfigOn = this.configurationDomainService.retrievePivotDateConfig();
Review comment:
@BLasan @fynmanoj : If Interest Posting is run daily, then pivot date will always be today's date. How is this configuration different from disallow backdated txn configuration?
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java
##########
@@ -558,15 +596,136 @@ public void postInterest(final MathContext mc, final LocalDate interestPostingUp
// no openingBalance concept supported yet but probably will to
// allow
// for migrations.
- final Money openingAccountBalance = Money.zero(this.currency);
+ Money openingAccountBalance = Money.zero(this.currency);
+
+ if (isPivotConfigOn) {
+ if (this.summary.getLastInterestCalculationDate() == null) {
+ openingAccountBalance = Money.zero(this.currency);
+ } else {
+ openingAccountBalance = Money.of(this.currency, this.summary.getRunningBalanceOnPivotDate());
+ }
+ }
// update existing transactions so derived balance fields are
// correct.
- recalculateDailyBalances(openingAccountBalance, interestPostingUpToDate);
+ recalculateDailyBalances(openingAccountBalance, interestPostingUpToDate, isPivotConfigOn);
}
- this.summary.updateSummary(this.currency, this.savingsAccountTransactionSummaryWrapper, this.transactions);
- }
+ if (!isPivotConfigOn) {
+ this.summary.updateSummary(this.currency, this.savingsAccountTransactionSummaryWrapper, this.transactions);
+ } else {
+ this.summary.updateSummaryWithPivotConfig(this.currency, this.savingsAccountTransactionSummaryWrapper, null,
+ this.savingsAccountTransactions);
+ }
+ }
+
+ // public void postInterestWithPivotConfig(final MathContext mc, final LocalDate interestPostingUpToDate, final
+ // boolean isInterestTransfer,
+ // final boolean isSavingsInterestPostingAtCurrentPeriodEnd, final Integer financialYearBeginningMonth,
+ // final LocalDate postInterestOnDate) {
+ // final List<PostingPeriod> postingPeriods = calculateInterestAfterPostingWithPivotConfig(mc,
+ // interestPostingUpToDate,
+ // isInterestTransfer, isSavingsInterestPostingAtCurrentPeriodEnd, financialYearBeginningMonth, postInterestOnDate,
+ // null);
+ //
+ // // Get last interest amount
+ // Money interestPostedToDate = Money.of(this.currency, this.summary.getTotalInterestPosted());
+ //
+ // boolean recalucateDailyBalanceDetails = false;
+ // boolean applyWithHoldTax = isWithHoldTaxApplicableForInterestPosting();
+ // final List<SavingsAccountTransaction> withholdTransactions = new ArrayList<>();
+ //
+ // withholdTransactions.addAll(findWithHoldSavingsTransactionsWithPivotConfig());
+ //
+ // for (final PostingPeriod interestPostingPeriod : postingPeriods) {
+ //
+ // final LocalDate interestPostingTransactionDate = interestPostingPeriod.dateOfPostingTransaction();
+ // final Money interestEarnedToBePostedForPeriod = interestPostingPeriod.getInterestEarned();
+ //
+ // if (!interestPostingTransactionDate.isAfter(interestPostingUpToDate)) {
+ // interestPostedToDate = interestPostedToDate.plus(interestEarnedToBePostedForPeriod);
+ //
+ // final SavingsAccountTransaction postingTransaction = findInterestPostingSavingsTransactionWithPivotConfig(
+ // interestPostingTransactionDate);
+ // if (postingTransaction == null) {
+ // SavingsAccountTransaction newPostingTransaction;
+ // if (interestEarnedToBePostedForPeriod.isGreaterThanOrEqualTo(Money.zero(currency))) {
+ //
+ // newPostingTransaction = SavingsAccountTransaction.interestPosting(this, office(), interestPostingTransactionDate,
+ // interestEarnedToBePostedForPeriod, interestPostingPeriod.isUserPosting());
+ // } else {
+ // newPostingTransaction = SavingsAccountTransaction.overdraftInterest(this, office(),
+ // interestPostingTransactionDate,
+ // interestEarnedToBePostedForPeriod.negated(), interestPostingPeriod.isUserPosting());
+ // }
+ // addTransactionToExisting(newPostingTransaction);
+ // if (applyWithHoldTax) {
+ // createWithHoldTransactionWithPivotConfig(interestEarnedToBePostedForPeriod.getAmount(),
+ // interestPostingTransactionDate);
+ // }
+ // recalucateDailyBalanceDetails = true;
+ // } else {
+ // boolean correctionRequired = false;
+ // if (postingTransaction.isInterestPostingAndNotReversed()) {
+ // correctionRequired = postingTransaction.hasNotAmount(interestEarnedToBePostedForPeriod);
+ // } else {
+ // correctionRequired = postingTransaction.hasNotAmount(interestEarnedToBePostedForPeriod.negated());
+ // }
+ // if (correctionRequired) {
+ // boolean applyWithHoldTaxForOldTransaction = false;
+ // postingTransaction.reverse();
+ // final SavingsAccountTransaction withholdTransaction = findTransactionFor(interestPostingTransactionDate,
+ // withholdTransactions);
+ // if (withholdTransaction != null) {
+ // withholdTransaction.reverse();
+ // applyWithHoldTaxForOldTransaction = true;
+ // }
+ // SavingsAccountTransaction newPostingTransaction;
+ // if (interestEarnedToBePostedForPeriod.isGreaterThanOrEqualTo(Money.zero(currency))) {
+ // newPostingTransaction = SavingsAccountTransaction.interestPosting(this, office(),
+ // interestPostingTransactionDate, interestEarnedToBePostedForPeriod,
+ // interestPostingPeriod.isUserPosting());
+ // } else {
+ // newPostingTransaction = SavingsAccountTransaction.overdraftInterest(this, office(),
+ // interestPostingTransactionDate, interestEarnedToBePostedForPeriod.negated(),
+ // interestPostingPeriod.isUserPosting());
+ // }
+ // addTransactionToExisting(newPostingTransaction);
+ // if (applyWithHoldTaxForOldTransaction) {
+ // createWithHoldTransactionWithPivotConfig(interestEarnedToBePostedForPeriod.getAmount(),
+ // interestPostingTransactionDate);
+ // }
+ // recalucateDailyBalanceDetails = true;
+ // }
+ // }
+ // }
+ // }
+ //
+ // if (recalucateDailyBalanceDetails) {
+ // // no openingBalance concept supported yet but probably will to
+ // // allow
+ // // for migrations.
+ // Money openingAccountBalance = null;
+ // if (this.summary.getLastInterestCalculationDate() == null) {
+ // openingAccountBalance = Money.zero(this.currency);
+ // } else {
+ // openingAccountBalance = Money.of(this.currency, this.summary.getRunningBalanceOnPivotDate());
+ // }
+ //
+ // recalculatesDailyBalancesWithPivotConfig(openingAccountBalance, interestPostingUpToDate);
+ // }
+ //
+ // this.summary.updateSummaryWithPivotConfig(this.currency, this.savingsAccountTransactionSummaryWrapper, null,
+ // this.savingsAccountTransactions);
+ //
+ // // List<SavingsAccountTransaction> sortedTransactions = retrieveSortedTransactions();
+ //
+ // // // This is unnecessary. AccountBalanceDerived would solve this issue
+ // // this.summary.runningBalance = sortedTransactions.get(0).getRunningBalance(this.currency).getAmount();
+ // //
+ // // // Get the latest posting from the above iteration
+ // // this.summary.lastInterestPostingDate = newPostingTransaction.getLastTransactionDate();
+ // }
Review comment:
Remove dead code.
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java
##########
@@ -1476,15 +2287,48 @@ private void validateLockinDetails(final DataValidatorBuilder baseDataValidator)
baseDataValidator.reset().parameter(lockinPeriodFrequencyParamName).value(this.lockinPeriodFrequency).notNull()
.integerZeroOrGreater();
}
- } else {
- baseDataValidator.reset().parameter(lockinPeriodFrequencyParamName).value(this.lockinPeriodFrequencyType)
- .integerZeroOrGreater();
- baseDataValidator.reset().parameter(lockinPeriodFrequencyTypeParamName).value(this.lockinPeriodFrequencyType).notNull()
- .inMinMaxRange(0, 3);
+ } else {
+ baseDataValidator.reset().parameter(lockinPeriodFrequencyParamName).value(this.lockinPeriodFrequencyType)
+ .integerZeroOrGreater();
+ baseDataValidator.reset().parameter(lockinPeriodFrequencyTypeParamName).value(this.lockinPeriodFrequencyType).notNull()
+ .inMinMaxRange(0, 3);
+ }
+ }
+
+ public Map<String, Object> deriveAccountingBridgeData(final CurrencyData currencyData, final Set<Long> existingTransactionIds,
+ final Set<Long> existingReversedTransactionIds, boolean isAccountTransfer) {
+
Review comment:
Business logic doesn't belong in the entity class. Put it in a write or domainService class. This also looks like a lot of duplicate code or code you're unnecessarily moving from a different file which you haven't written.
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/account/service/AccountTransfersReadPlatformServiceImpl.java
##########
@@ -265,6 +266,16 @@ public AccountTransferData retrieveOne(final Long transferId) {
return transactionId;
}
+ @Override
+ public Collection<Long> fetchPostInterestTransactionIdsWithPivotDate(final Long accountId, final Date pivotDate) {
+ final String sql = "select att.from_savings_transaction_id from m_account_transfer_transaction att inner join m_account_transfer_details atd on atd.id = att.account_transfer_details_id where atd.from_savings_account_id=? and att.is_reversed = 0 and atd.transfer_type = ? and att.transaction_date > ?";
Review comment:
Run the gradle code formatter task for spotlessApply. One should not have to horizontally scroll to read code.
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java
##########
@@ -1510,21 +2429,59 @@ private void validateLockinDetails(final DataValidatorBuilder baseDataValidator)
return accountingBridgeData;
}
- public Collection<Long> findExistingTransactionIds() {
+ public Map<String, Object> deriveAccountingBridgeDataWithPivotConfig(final CurrencyData currencyData,
+ final Set<Long> existingTransactionIds, final Set<Long> existingReversedTransactionIds, boolean isAccountTransfer) {
Review comment:
@fynmanoj @BLasan : Please revert on the need for a new deriveAccountingBridgeDataWithPivotConfig function. Signature looks same as old deriveAccountingBridgeDataWithPivotConfig.
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java
##########
@@ -877,6 +1326,73 @@ protected void recalculateDailyBalances(final Money openingAccountBalance, final
resetAccountTransactionsEndOfDayBalances(accountTransactionsSorted, interestPostingUpToDate);
}
+ // Interest Calculation for savings.
+ protected void recalculateSavingsDailyBalances(final Money openingAccountBalance, final LocalDate interestPostingUpToDate) {
+
Review comment:
Business logic doesn't belong in the entity class. Put it in a write or domainService class. This also looks like a lot of duplicate code or code you're unnecessarily moving from a different file which you haven't written
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java
##########
@@ -1510,21 +2429,59 @@ private void validateLockinDetails(final DataValidatorBuilder baseDataValidator)
return accountingBridgeData;
}
- public Collection<Long> findExistingTransactionIds() {
+ public Map<String, Object> deriveAccountingBridgeDataWithPivotConfig(final CurrencyData currencyData,
+ final Set<Long> existingTransactionIds, final Set<Long> existingReversedTransactionIds, boolean isAccountTransfer) {
Review comment:
@fynmanoj @BLasan : Not sure about the need for this separate function.
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java
##########
@@ -689,15 +909,22 @@ protected SavingsAccountTransaction findLastTransaction(final LocalDate date) {
public List<PostingPeriod> calculateInterestUsing(final MathContext mc, final LocalDate upToInterestCalculationDate,
boolean isInterestTransfer, final boolean isSavingsInterestPostingAtCurrentPeriodEnd, final Integer financialYearBeginningMonth,
- final LocalDate postInterestOnDate) {
+ final LocalDate postInterestOnDate, final boolean isPivotConfigOn) {
// no openingBalance concept supported yet but probably will to allow
// for migrations.
- final Money openingAccountBalance = Money.zero(this.currency);
+ Money openingAccountBalance = null;
+
+ // Check global configurations and 'pivot' date is null
+ if (isPivotConfigOn) {
+ openingAccountBalance = Money.of(this.currency, this.summary.getRunningBalanceOnPivotDate());
Review comment:
Is this the core logic part?
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java
##########
@@ -2729,16 +3901,54 @@ private void handlePayChargeTransactions(SavingsAccountCharge savingsAccountChar
} else {
chargeTransaction = SavingsAccountTransaction.charge(this, office(), transactionDate, transactionAmount, user);
}
-
handleChargeTransactions(savingsAccountCharge, chargeTransaction);
}
+ // private void handleSavingsPayChargeTransactions(SavingsAccountCharge savingsAccountCharge, Money
+ // transactionAmount,
+ // final LocalDate transactionDate, final AppUser user) {
+ // SavingsAccountTransaction chargeTransaction = null;
+ //
+ // if (savingsAccountCharge.isWithdrawalFee()) {
+ // chargeTransaction = SavingsAccountTransaction.withdrawalFee(this, office(), transactionDate, transactionAmount,
+ // user);
+ // } else if (savingsAccountCharge.isAnnualFee()) {
+ // chargeTransaction = SavingsAccountTransaction.annualFee(this, office(), transactionDate, transactionAmount,
+ // user);
+ // } else {
+ // chargeTransaction = SavingsAccountTransaction.charge(this, office(), transactionDate, transactionAmount, user);
+ // }
+ //
+ // handleSavingsChargeTransactions(savingsAccountCharge, chargeTransaction);
+ // }
+
+ private SavingsAccountTransaction handleAndReturnPayChargeTransactions(SavingsAccountCharge savingsAccountCharge,
+ Money transactionAmount, final LocalDate transactionDate, final AppUser user) {
Review comment:
Business logic doesn't belong in the entity class. Put it in a write or domainService class. This also looks like a lot of duplicate code or code you're unnecessarily moving from a different file which you haven't written
Also remove unnecessary comments when sending PRs for review.
##########
File path: fineract-provider/src/main/java/org/apache/fineract/interoperation/service/InteropServiceImpl.java
##########
@@ -436,10 +436,11 @@ public InteropTransferResponseData commitTransfer(@NotNull JsonCommand command)
SavingsTransactionBooleanValues transactionValues = new SavingsTransactionBooleanValues(false, true, true, false, false);
transaction = savingsAccountService.handleWithdrawal(savingsAccount, fmt, transactionDate, request.getAmount().getAmount(),
instance(findPaymentType(), savingsAccount.getExternalId(), null, getRoutingCode(), transferCode, null),
- transactionValues);
+ transactionValues, false);
} else {
Review comment:
First assign false to a variable named after parameter expected in function parameter and then pass that parameter to the function.
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsSchedularServiceImpl.java
##########
@@ -73,7 +73,7 @@ public void postInterestForAccounts() throws JobExecutionException {
this.savingAccountAssembler.assignSavingAccountHelpers(savingsAccount);
boolean postInterestAsOn = false;
LocalDate transactionDate = null;
- this.savingsAccountWritePlatformService.postInterest(savingsAccount, postInterestAsOn, transactionDate);
+ this.savingsAccountWritePlatformService.postInterest(savingsAccount, postInterestAsOn, transactionDate, false);
Review comment:
Missed using variable name here.
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java
##########
@@ -484,15 +496,25 @@ public boolean isClosed() {
public void postInterest(final MathContext mc, final LocalDate interestPostingUpToDate, final boolean isInterestTransfer,
final boolean isSavingsInterestPostingAtCurrentPeriodEnd, final Integer financialYearBeginningMonth,
- final LocalDate postInterestOnDate) {
+ final LocalDate postInterestOnDate, final boolean isPivotConfigOn) {
final List<PostingPeriod> postingPeriods = calculateInterestUsing(mc, interestPostingUpToDate, isInterestTransfer,
- isSavingsInterestPostingAtCurrentPeriodEnd, financialYearBeginningMonth, postInterestOnDate);
+ isSavingsInterestPostingAtCurrentPeriodEnd, financialYearBeginningMonth, postInterestOnDate, isPivotConfigOn);
+
Money interestPostedToDate = Money.zero(this.currency);
+ if (isPivotConfigOn) {
+ interestPostedToDate = Money.of(this.currency, this.summary.getTotalInterestPosted());
+ }
+
boolean recalucateDailyBalanceDetails = false;
boolean applyWithHoldTax = isWithHoldTaxApplicableForInterestPosting();
final List<SavingsAccountTransaction> withholdTransactions = new ArrayList<>();
- withholdTransactions.addAll(findWithHoldTransactions());
+
+ if (isPivotConfigOn) {
+ withholdTransactions.addAll(findWithHoldSavingsTransactionsWithPivotConfig());
+ } else {
+ withholdTransactions.addAll(findWithHoldTransactions());
+ }
Review comment:
Can you explain this part?
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/guarantor/service/GuarantorWritePlatformServiceJpaRepositoryIImpl.java
##########
@@ -111,12 +111,13 @@ private CommandProcessingResult createGuarantor(final Loan loan, final JsonComma
guarantorCommand.validateForCreate();
validateLoanStatus(loan);
final List<GuarantorFundingDetails> guarantorFundingDetails = new ArrayList<>();
+ final boolean isPivotConfigOn = false;
AccountAssociations accountAssociations = null;
if (guarantorCommand.getSavingsId() != null) {
- final SavingsAccount savingsAccount = this.savingsAccountAssembler.assembleFrom(guarantorCommand.getSavingsId());
+ final SavingsAccount savingsAccount = this.savingsAccountAssembler.assembleFrom(guarantorCommand.getSavingsId(), false);
Review comment:
Missed using the variable name here.
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java
##########
@@ -442,6 +446,30 @@ public void setHelpers(final SavingsAccountTransactionSummaryWrapper savingsAcco
this.savingsHelper = savingsHelper;
}
+ // public void setPivotDate(final Date date) {
+ // this.summary.lastInterestPostingDate = date;
+ // }
+
+ public void setSavingsAccountTransactions(final List<SavingsAccountTransaction> savingsAccountTransactions) {
+ this.savingsAccountTransactions.addAll(savingsAccountTransactions);
+ }
+
+ // public void setIsEmptyTransactions(final boolean isEmpty) {
+ // this.isEmpty = isEmpty;
+ // }
+
+ public List<SavingsAccountTransaction> getSavingsAccountTransactions() {
+ return this.savingsAccountTransactions;
+ }
+
+ // public Date getPivotDate() {
+ // return this.summary.lastInterestPostingDate;
+ // }
+
+ // public boolean getIsEmptyTransactions() {
+ // return this.isEmpty;
+ // }
+
Review comment:
Remove unnecessary comments.
##########
File path: fineract-provider/src/main/resources/sql/migrations/core_db/V376__savings_last_transaction_date.sql
##########
@@ -0,0 +1,24 @@
+--
Review comment:
Please check if migration script number is still valid since there have been a couple of PRs merged since this PR was sent.
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java
##########
@@ -1989,6 +2958,36 @@ public void undoTransaction(final Long transactionId) {
}
}
+ public void undoSavingsTransaction(final Long transactionId) {
+
+ SavingsAccountTransaction transactionToUndo = null;
+ for (final SavingsAccountTransaction transaction : this.savingsAccountTransactions) {
+ if (transaction.isIdentifiedBy(transactionId)) {
+ transactionToUndo = transaction;
+ }
+ }
+
+ if (transactionToUndo == null) {
+ throw new SavingsAccountTransactionNotFoundException(this.getId(), transactionId);
+ }
+
+ validateAttemptToUndoTransferRelatedTransactions(transactionToUndo);
+ validateActivityNotBeforeClientOrGroupTransferDate(SavingsEvent.SAVINGS_UNDO_TRANSACTION, transactionToUndo.transactionLocalDate());
+ transactionToUndo.reverse();
+ if (transactionToUndo.isChargeTransaction() || transactionToUndo.isWaiveCharge()) {
+ // undo charge
+ final Set<SavingsAccountChargePaidBy> chargesPaidBy = transactionToUndo.getSavingsAccountChargesPaid();
+ for (final SavingsAccountChargePaidBy savingsAccountChargePaidBy : chargesPaidBy) {
+ final SavingsAccountCharge chargeToUndo = savingsAccountChargePaidBy.getSavingsAccountCharge();
+ if (transactionToUndo.isChargeTransaction()) {
+ chargeToUndo.undoPayment(this.getCurrency(), transactionToUndo.getAmount(this.getCurrency()));
+ } else if (transactionToUndo.isWaiveCharge()) {
+ chargeToUndo.undoWaiver(this.getCurrency(), transactionToUndo.getAmount(this.getCurrency()));
+ }
Review comment:
Looks like copy/pasted duplicate code. From where is this copied and why?
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/account/domain/AccountTransferDetailAssembler.java
##########
@@ -64,10 +64,11 @@ public AccountTransferDetailAssembler(final ClientRepositoryWrapper clientReposi
public AccountTransferDetails assembleSavingsToSavingsTransfer(final JsonCommand command) {
final Long fromSavingsId = command.longValueOfParameterNamed(fromAccountIdParamName);
- final SavingsAccount fromSavingsAccount = this.savingsAccountAssembler.assembleFrom(fromSavingsId);
+ final SavingsAccount fromSavingsAccount = this.savingsAccountAssembler.assembleFrom(fromSavingsId, false);
+ final boolean isPivotConfigOn = false;
Review comment:
From a code readability standpoint, won't it make more sense to name it something like "backdatedTxnsAllowedTill" config?
##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/configuration/domain/ConfigurationDomainServiceJpa.java
##########
@@ -343,6 +343,31 @@ public Integer retrieveOTPLiveTime() {
return value;
}
+ @Override
+ public boolean retrievePivotDateConfig() {
Review comment:
Configuration name and function names are not in sync.
##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java
##########
@@ -776,11 +1003,250 @@ protected SavingsAccountTransaction findLastTransaction(final LocalDate date) {
isTransferInterestToOtherAccount());
this.summary.updateFromInterestPeriodSummaries(this.currency, allPostingPeriods);
- this.summary.updateSummary(this.currency, this.savingsAccountTransactionSummaryWrapper, this.transactions);
+
+ if (isPivotConfigOn) {
+ this.summary.updateSummaryWithPivotConfig(this.currency, this.savingsAccountTransactionSummaryWrapper, null,
+ this.savingsAccountTransactions);
+ } else {
+ this.summary.updateSummary(this.currency, this.savingsAccountTransactionSummaryWrapper, this.transactions);
+ }
+
+ // this.savingsHelper.calculateInterestForAllPostingPeriods(this.currency, allPostingPeriods,
+ // getLockedInUntilLocalDate(),
+ // isTransferInterestToOtherAccount());
+ //
+ // this.summary.updateFromInterestPeriodSummaries(this.currency, allPostingPeriods);
+ // this.summary.updateSummaryWithPivotConfig(this.currency, this.savingsAccountTransactionSummaryWrapper,
+ // savingsTransaction,
+ // this.savingsAccountTransactions);
return allPostingPeriods;
}
+ // public List<PostingPeriod> calculateInterestWithPivotDateConfig(final MathContext mc, final LocalDate
+ // upToInterestCalculationDate,
+ // boolean isInterestTransfer, final boolean isSavingsInterestPostingAtCurrentPeriodEnd, final Integer
+ // financialYearBeginningMonth,
+ // final LocalDate postInterestOnDate, final SavingsAccountTransaction savingsTransaction) {
+ //
+ // // no openingBalance concept supported yet but probably will to allow
+ // // for migrations.
+ //
+ // Money openingAccountBalance = null;
+ //
+ // // Check global configurations and 'pivot' date is null
+ // if (this.summary.getTransactionPivotDate() != null) {
+ // openingAccountBalance = Money.of(this.currency, this.summary.getRunningBalanceOnPivotDate());
+ // } else {
+ // openingAccountBalance = Money.zero(this.currency);
+ // }
+ //
+ // // update existing transactions so derived balance fields are
+ // // correct.
+ // // recalculateDailyBalances(openingAccountBalance, upToInterestCalculationDate);
+ //
+ // recalculatesDailyBalancesWithPivotConfig(openingAccountBalance, upToInterestCalculationDate);
+ //
+ // // 1. default to calculate interest based on entire history OR
+ // // 2. determine latest 'posting period' and find interest credited to
+ // // that period
+ //
+ // // A generate list of EndOfDayBalances (not including interest postings)
+ // final SavingsPostingInterestPeriodType postingPeriodType =
+ // SavingsPostingInterestPeriodType.fromInt(this.interestPostingPeriodType);
+ //
+ // final SavingsCompoundingInterestPeriodType compoundingPeriodType = SavingsCompoundingInterestPeriodType
+ // .fromInt(this.interestCompoundingPeriodType);
+ //
+ // final SavingsInterestCalculationDaysInYearType daysInYearType = SavingsInterestCalculationDaysInYearType
+ // .fromInt(this.interestCalculationDaysInYearType);
+ // List<LocalDate> postedAsOnDates = getManualPostingDatesWithPivotConfig();
+ // if (postInterestOnDate != null) {
+ // postedAsOnDates.add(postInterestOnDate);
+ // }
+ // final List<LocalDateInterval> postingPeriodIntervals = this.savingsHelper.determineInterestPostingPeriods(
+ // getStartInterestCalculationDate(), upToInterestCalculationDate, postingPeriodType, financialYearBeginningMonth,
+ // postedAsOnDates);
+ //
+ // final List<PostingPeriod> allPostingPeriods = new ArrayList<>();
+ //
+ // Money periodStartingBalance;
+ // if (this.startInterestCalculationDate != null) {
+ // LocalDate startInterestCalculationDate = LocalDate.ofInstant(this.startInterestCalculationDate.toInstant(),
+ // DateUtils.getDateTimeZoneOfTenant());
+ // final SavingsAccountTransaction transaction =
+ // findLastFilteredTransactionWithPivotConfig(startInterestCalculationDate);
+ //
+ // if (transaction == null) {
+ // final String defaultUserMessage = "No transactions were found on the specified date "
+ // + getStartInterestCalculationDate().toString() + " for account number " + this.accountNumber.toString()
+ // + " and resource id " + getId();
+ //
+ // final ApiParameterError error = ApiParameterError.parameterError(
+ // "error.msg.savingsaccount.transaction.incorrect.start.interest.calculation.date", defaultUserMessage,
+ // "transactionDate", getStartInterestCalculationDate().toString());
+ //
+ // final List<ApiParameterError> dataValidationErrors = new ArrayList<>();
+ // dataValidationErrors.add(error);
+ //
+ // throw new PlatformApiDataValidationException(dataValidationErrors);
+ // }
+ //
+ // periodStartingBalance = transaction.getRunningBalance(this.currency);
+ // } else {
+ // periodStartingBalance = Money.zero(this.currency);
+ // }
+ //
+ // final SavingsInterestCalculationType interestCalculationType =
+ // SavingsInterestCalculationType.fromInt(this.interestCalculationType);
+ // final BigDecimal interestRateAsFraction = getEffectiveInterestRateAsFraction(mc, upToInterestCalculationDate);
+ // final BigDecimal overdraftInterestRateAsFraction = getEffectiveOverdraftInterestRateAsFraction(mc);
+ //
+ // final Collection<Long> interestPostTransactions = this.savingsHelper.fetchPostInterestTransactionIds(getId());
+ // final Money minBalanceForInterestCalculation = Money.of(getCurrency(), minBalanceForInterestCalculation());
+ // final Money minOverdraftForInterestCalculation = Money.of(getCurrency(),
+ // this.minOverdraftForInterestCalculation);
+ //
+ // for (final LocalDateInterval periodInterval : postingPeriodIntervals) {
+ //
+ // boolean isUserPosting = false;
+ // if (postedAsOnDates.contains(periodInterval.endDate().plusDays(1))) {
+ // isUserPosting = true;
+ // }
+ //
+ // final PostingPeriod postingPeriod = PostingPeriod.createFrom(periodInterval, periodStartingBalance,
+ // retreiveOrderedNonInterestPostingSavingsTransactionsWithPivotConfig(), this.currency, compoundingPeriodType,
+ // interestCalculationType, interestRateAsFraction, daysInYearType.getValue(), upToInterestCalculationDate,
+ // interestPostTransactions, isInterestTransfer, minBalanceForInterestCalculation,
+ // isSavingsInterestPostingAtCurrentPeriodEnd, overdraftInterestRateAsFraction, minOverdraftForInterestCalculation,
+ // isUserPosting, financialYearBeginningMonth);
+ //
+ // periodStartingBalance = postingPeriod.closingBalance();
+ //
+ // allPostingPeriods.add(postingPeriod);
+ // }
+ //
+ // // Modify this based on the filtered transactions
+ // this.savingsHelper.calculateInterestForAllPostingPeriods(this.currency, allPostingPeriods,
+ // getLockedInUntilLocalDate(),
+ // isTransferInterestToOtherAccount());
+ //
+ // this.summary.updateFromInterestPeriodSummaries(this.currency, allPostingPeriods);
+ // this.summary.updateSummaryWithPivotConfig(this.currency, this.savingsAccountTransactionSummaryWrapper,
+ // savingsTransaction,
+ // this.savingsAccountTransactions);
+ //
+ // return allPostingPeriods;
+ // }
+
+ // public List<PostingPeriod> calculateInterestAfterPostingWithPivotConfig(final MathContext mc,
+ // final LocalDate upToInterestCalculationDate, boolean isInterestTransfer,
+ // final boolean isSavingsInterestPostingAtCurrentPeriodEnd, final Integer financialYearBeginningMonth,
+ // final LocalDate postInterestOnDate, final SavingsAccountTransaction savingsTransaction) {
+ //
+ // // no openingBalance concept supported yet but probably will to allow
+ // // for migrations.
+ //
+ // Money openingAccountBalance = null;
+ //
+ // // Check global configurations and 'pivot' date is null
+ // if (this.summary.getTransactionPivotDate() != null) {
+ // openingAccountBalance = Money.of(this.currency, this.summary.getRunningBalanceOnPivotDate());
+ // } else {
+ // openingAccountBalance = Money.zero(this.currency);
+ // }
+ //
+ // recalculatesDailyBalancesWithPivotConfig(openingAccountBalance, upToInterestCalculationDate);
+ //
+ // // 1. default to calculate interest based on entire history OR
+ // // 2. determine latest 'posting period' and find interest credited to
+ // // that period
+ //
+ // // A generate list of EndOfDayBalances (not including interest postings)
+ // final SavingsPostingInterestPeriodType postingPeriodType =
+ // SavingsPostingInterestPeriodType.fromInt(this.interestPostingPeriodType);
+ //
+ // final SavingsCompoundingInterestPeriodType compoundingPeriodType = SavingsCompoundingInterestPeriodType
+ // .fromInt(this.interestCompoundingPeriodType);
+ //
+ // final SavingsInterestCalculationDaysInYearType daysInYearType = SavingsInterestCalculationDaysInYearType
+ // .fromInt(this.interestCalculationDaysInYearType);
+ // List<LocalDate> postedAsOnDates = getManualPostingDatesWithPivotConfig();
+ // if (postInterestOnDate != null) {
+ // postedAsOnDates.add(postInterestOnDate);
+ // }
+ // final List<LocalDateInterval> postingPeriodIntervals = this.savingsHelper.determineInterestPostingPeriods(
+ // getStartInterestCalculationDate(), upToInterestCalculationDate, postingPeriodType, financialYearBeginningMonth,
+ // postedAsOnDates);
+ //
+ // final List<PostingPeriod> allPostingPeriods = new ArrayList<>();
+ //
+ // Money periodStartingBalance;
+ // if (this.startInterestCalculationDate != null) {
+ // LocalDate startInterestCalculationDate = LocalDate.ofInstant(this.startInterestCalculationDate.toInstant(),
+ // DateUtils.getDateTimeZoneOfTenant());
+ // final SavingsAccountTransaction transaction =
+ // findLastFilteredTransactionWithPivotConfig(startInterestCalculationDate);
+ //
+ // if (transaction == null) {
+ // final String defaultUserMessage = "No transactions were found on the specified date "
+ // + getStartInterestCalculationDate().toString() + " for account number " + this.accountNumber.toString()
+ // + " and resource id " + getId();
+ //
+ // final ApiParameterError error = ApiParameterError.parameterError(
+ // "error.msg.savingsaccount.transaction.incorrect.start.interest.calculation.date", defaultUserMessage,
+ // "transactionDate", getStartInterestCalculationDate().toString());
+ //
+ // final List<ApiParameterError> dataValidationErrors = new ArrayList<>();
+ // dataValidationErrors.add(error);
+ //
+ // throw new PlatformApiDataValidationException(dataValidationErrors);
+ // }
+ //
+ // periodStartingBalance = transaction.getRunningBalance(this.currency);
+ // } else {
+ // periodStartingBalance = Money.zero(this.currency);
+ // }
+ //
+ // final SavingsInterestCalculationType interestCalculationType =
+ // SavingsInterestCalculationType.fromInt(this.interestCalculationType);
+ // final BigDecimal interestRateAsFraction = getEffectiveInterestRateAsFraction(mc, upToInterestCalculationDate);
+ // final BigDecimal overdraftInterestRateAsFraction = getEffectiveOverdraftInterestRateAsFraction(mc);
+ //
+ // final Collection<Long> interestPostTransactions = this.savingsHelper.fetchPostInterestTransactionIds(getId());
+ //
+ // final Money minBalanceForInterestCalculation = Money.of(getCurrency(), minBalanceForInterestCalculation());
+ // final Money minOverdraftForInterestCalculation = Money.of(getCurrency(),
+ // this.minOverdraftForInterestCalculation);
+ //
+ // for (final LocalDateInterval periodInterval : postingPeriodIntervals) {
+ //
+ // boolean isUserPosting = false;
+ // if (postedAsOnDates.contains(periodInterval.endDate().plusDays(1))) {
+ // isUserPosting = true;
+ // }
+ //
+ // final PostingPeriod postingPeriod = PostingPeriod.createFrom(periodInterval, periodStartingBalance,
+ // retreiveOrderedNonInterestPostingSavingsTransactionsWithPivotConfig(), this.currency, compoundingPeriodType,
+ // interestCalculationType, interestRateAsFraction, daysInYearType.getValue(), upToInterestCalculationDate,
+ // interestPostTransactions, isInterestTransfer, minBalanceForInterestCalculation,
+ // isSavingsInterestPostingAtCurrentPeriodEnd, overdraftInterestRateAsFraction, minOverdraftForInterestCalculation,
+ // isUserPosting, financialYearBeginningMonth);
+ //
+ // periodStartingBalance = postingPeriod.closingBalance();
+ //
+ // allPostingPeriods.add(postingPeriod);
+ // }
+ //
+ // // Modify this based on the filtered transactions
+ // this.savingsHelper.calculateInterestForAllPostingPeriods(this.currency, allPostingPeriods,
+ // getLockedInUntilLocalDate(),
+ // isTransferInterestToOtherAccount());
+ //
+ // this.summary.updateFromInterestPeriodSummaries(this.currency, allPostingPeriods);
+ // return allPostingPeriods;
+ // }
+
Review comment:
Remove dead code.
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org