ptuomola commented on a change in pull request #1179:
URL: https://github.com/apache/fineract/pull/1179#discussion_r480722022



##########
File path: 
fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/AccountingScenarioIntegrationTest.java
##########
@@ -786,18 +787,18 @@ public void 
checkPeriodicAccrualAccountingTillCurrentDateFlow() throws Interrupt
         final ArrayList<HashMap> loanSchedule = 
this.loanTransactionHelper.getLoanRepaymentSchedule(requestSpec, responseSpec, 
loanID);
         // MAKE 1
         List fromDateList = (List) loanSchedule.get(1).get("fromDate");
-        LocalDate fromDateLocal = LocalDate.now();
+        LocalDate fromDateLocal = LocalDate.now(ZoneId.systemDefault());

Review comment:
       Just out of interest - why was it needed to explicitly specify ZoneId? 
Wouldn't just calling now() have worked as before?

##########
File path: 
fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientLoanIntegrationTest.java
##########
@@ -755,7 +757,7 @@ public void testLoanCharges_DISBURSEMENT_WITH_TRANCHES() {
         final Integer loanProductID = createLoanProduct(true, NONE);
 
         List<HashMap> tranches = new ArrayList<>();
-        tranches.add(createTrancheDetail("1 March 2014", "25000"));
+        tranches.add(createTrancheDetail("01 March 2014", "25000"));

Review comment:
       Instead of changing all of these inputs, should we not consider changing 
the pattern to be "d M y" instead? My worry would be that there may be many 
clients not using the leading zero. 
   
   But of course changing the pattern is not going to be error-free either as 
this is also being sent by the client. Can you see any way of retaining 
backward compatibility?

##########
File path: 
fineract-provider/src/main/java/org/apache/fineract/adhocquery/data/AdHocData.java
##########
@@ -46,13 +46,13 @@
     @SuppressWarnings("unused")
     private final boolean isActive;
     @SuppressWarnings("unused")
-    private final DateTime createdOn;
+    private final ZonedDateTime createdOn;
     @SuppressWarnings("unused")

Review comment:
       Why do we use a mix of LocalDate (no timezone) and ZonedDateTime (with 
timezone) - what's the logic here?




----------------------------------------------------------------
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]


Reply via email to