vorburger commented on a change in pull request #1024:
URL: https://github.com/apache/fineract/pull/1024#discussion_r443119349
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanArrearsAgingServiceImpl.java
##########
@@ -423,18 +423,14 @@ public OriginalScheduleExtractor(final String
loanIdsAsString) {
while (rs.next()) {
Long loanId = rs.getLong("loanId");
- List<LoanSchedulePeriodData> periodDatas = new ArrayList<>();
- LoanSchedulePeriodData loanSchedulePeriodData =
fetchLoanSchedulePeriodData(rs);
- periodDatas.add(loanSchedulePeriodData);
- while (rs.next()) {
- Long tempLoanId = rs.getLong("loanId");
- if (loanId.equals(tempLoanId)) {
Review comment:
@ptuomola sorry for delay in reviewing this (I wanted to do it with a
clear mind instead of clicking through it half asleep a night after day job
work...) and after staring at this a little bit, I get it now - this makes good
sense to me as well, and I have no objection to merging this even without
having any way to functionally test this myself either (no idea if existing
ITs, recently re-enable, would catch this, but doesn't matter, it "makes
sense").
One very small feedback I would have is that, now that I understand this
logic, it seems to me that moving that `scheduleDate.put(loanId, periodDatas);`
now in line 449 (originally 434) up into the `if(periodDatas == null)` right
after the `periodDatas = new ArrayList<>();` would make it a tiny bit more
clear and explicit what we're doing here? It's too minor to hold back merging
this important fix, so I'll just go ahead and merge it ASAP, and let you raise
a very small minor follow-up PR, if you like (or not, fine). _I originally
wrote this before seeing the Spotless style failure, if you're going over this
again, might as well do that too here - if you agree that makes sense?_
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
