avikganguly01 commented on pull request #1179:
URL: https://github.com/apache/fineract/pull/1179#issuecomment-895470295


   @ptuomola @percyashu @awasum @vorburger :
   
   FINERACT-826
   actualDisbursementDate.toDate() got replaced by
   
java.util.Date.from(actualDisbursementDate.atStartOfDay(ZoneId.systemDefault()).toInstant());
   
   Before FINERACT-1112
   actualDisbursementDate = T
   Server Timezone : +05:30Z, Tenant Timezone : +6Z
   ZoneId zone = ZoneId.systemDefault(); //+05:30Z 
   ZonedDateTime dateTime = actualDisbursementDate.atStartOfDay(zone); 
//Midnight +5:30Z
   Instant instant = dateTime.toInstant(); // T-1 Midnight + 18:30 0Z
   Date dt = Date.from(instant); // T-1 Midnight + 18:30 + 5:30 = T 00:00 
+05:30Z
   
   FINERACT-1112
   If tenant timezone is ahead of server timezone, then :-
   
   Server Timezone : +05:30Z, Tenant Timezone : +6Z
   actualDisbursementDate = T
   ZoneId zone = DateUtils.getDateTimeZoneOfTenant(); //+6Z // Previously 
ZoneId.systemDefault()
   ZonedDateTime dateTime = actualDisbursementDate.atStartOfDay(zone); 
//Midnight +6Z
   Instant instant = dateTime.toInstant(); // T-1 Midnight + 18 0Z
   Date dt = Date.from(instant); // T-1 Midnight + 18 + 5:30 = T-1 23:30 +05:30Z
   
   If tenant timezone and server timezones are different, then this creates a 
T-1 issue for all txn dates populated by application. 
   
   > Hi - in all the places where this currently calls ZoneId.systemDefault(), 
should we not be retrieving the appropriate tenant timezone instead? I know 
that's not what the previous code does, but wouldn't that be the right 
behaviour for any logic that is related to a specific tenant - i.e. we should 
be using the tenant's timezone for any dates?
   
   > actualDisbursementDate.toDate() got replaced by
   
java.util.Date.from(actualDisbursementDate.atStartOfDay(ZoneId.systemDefault()).toInstant());
   This part looks mostly correct as this is the most popular way 
(https://stackoverflow.com/questions/22929237/convert-java-time-localdate-into-java-util-date-type/22929420)
 to convert time.date to util.date. Note that the popular answer is questioned 
in it's comments section because atStartOfDay changes the time but overall the 
answer fixes the question.
   
   Problem is with using DateUtils.getDateTimeZoneOfTenant() instead of 
ZoneId.systemDefault() as this deviates from the solution provided for 
Fineract-826. A tenant is already passing the correct tenant-specific date in 
the API. There is no need to mess with the date by applying tenant timezone all 
over again.
   
   We have reverted DateUtils.getDateTimeZoneOfTenant() to 
ZoneId.systemDefault() wherever date was being parsed with atStartOfDay logic 
and so far SIT is successful. Please approve the corresponding PR if you 
approve of this logic reversal.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to