- 
                Notifications
    
You must be signed in to change notification settings  - Fork 2.1k
 
FINERACT-2354: Introduce reaging preview API #5063
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
FINERACT-2354: Introduce reaging preview API #5063
Conversation
8d5cc27    to
    61b0528      
    Compare
  
    | 
           @oleksii-novikov-onix Please rebase.  | 
    
61b0528    to
    377db88      
    Compare
  
    
          
  | 
    
377db88    to
    ff13fb2      
    Compare
  
    cbb30b1    to
    3a8af86      
    Compare
  
            
          
                ...oanaccount/domain/transactionprocessor/impl/AdvancedPaymentScheduleTransactionProcessor.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
      | AtomicReference<Money> outstandingPrincipalBalance = new AtomicReference<>(Money.zero(currency)); | ||
| final LocalDate startDate = loanTransaction.getLoanReAgeParameter().getStartDate(); | ||
| final LocalDate endDate = calculateReAgedInstallmentEndDate(loanTransaction.getLoanReAgeParameter()); | ||
| final AtomicReference<Money> outstandingPrincipalBalance = new AtomicReference<>(Money.zero(currency)); | ||
| installments.forEach(i -> { | ||
| Money principalOutstanding = i.getPrincipalOutstanding(currency); | ||
| if (principalOutstanding.isGreaterThanZero()) { | ||
| outstandingPrincipalBalance.set(outstandingPrincipalBalance.get().add(principalOutstanding)); | ||
| i.addToPrincipal(loanTransaction.getTransactionDate(), principalOutstanding.negated()); | ||
| final boolean shouldInclude = !i.isAdditional() || i.getDueDate().isBefore(startDate) | ||
| || (!i.getDueDate().isBefore(startDate) && i.getDueDate().isBefore(endDate)); | ||
| if (shouldInclude) { | ||
| Money principalOutstanding = i.getPrincipalOutstanding(currency); | ||
| if (principalOutstanding.isGreaterThanZero()) { | ||
| outstandingPrincipalBalance.set(outstandingPrincipalBalance.get().add(principalOutstanding)); | ||
| i.addToPrincipal(loanTransaction.getTransactionDate(), principalOutstanding.negated()); | ||
| } | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need these changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was added to handle negative balances. I will review the logic again
| @Parameter(hidden = true) final String apiRequestBodyAsJson, @Context final UriInfo uriInfo) { | ||
| final LoanScheduleData result = previewReAgeScheduleInternal(loanId, null, apiRequestBodyAsJson); | ||
| final ApiRequestJsonSerializationSettings settings = this.apiRequestParameterHelper.process(uriInfo.getQueryParameters()); | ||
| return this.loanScheduleToApiJsonSerializer.serialize(settings, result, new HashSet<>()); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to serialize. Simply return the LoanScheduleData result. Also you can remove the ApiResponses annotation as well in that case.
| } | ||
| } | ||
| 
               | 
          ||
| @POST | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think GET would be better here. Lets provide the parameters as query params.
| } | ||
| 
               | 
          ||
| @Transactional(readOnly = true) | ||
| public LoanScheduleData previewReAge(final Long loanId, final JsonCommand command) { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need command. It's a read operation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please kindly see my review!
3a8af86    to
    c3b04b9      
    Compare
  
    c3b04b9    to
    5d47942      
    Compare
  
    5d47942    to
    cc7407d      
    Compare
  
    
Description
Describe the changes made and why they were made.
Ignore if these details are present on the associated Apache Fineract JIRA ticket.
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.