search

webdigi / GmailScheduler

Google app script system to allow gmail / google business apps users to schedule outgoing messages & to set messages to return to inbox
Other
102 stars 36 forks source link

This resolves the timezone issue #13

Closed thapar closed 9 years ago

thapar commented 9 years ago

This resolves the timezone issue (issue https://github.com/webdigi/GmailScheduler/issues/3) where some date/time entries miscalculated whether it was today or tomorrow.

The issue is that SugarJS doesn't take into account the timezone. This pull request forces the use of Google's Utilities.formatDate(), which takes into account the timezone. Previously, when when a date wasn't "converted", it simply used the SugarJS method to create the date (which didn't take into account the timezone). But with this change, the selected timezone will always be taken into account.

webdigi commented 9 years ago

Thanks. Can you please outline a few tests we can do verify the commit wont break anything.

thapar commented 9 years ago

Time is now 2:45pm (Eastern Standard Time, which is GMT -5) on Monday, February 17, 2015. Before pull request: (Timezone is not taken into account)

"3pm" -> Tuesday, February 18, 2015 3:00:00pm
"4pm" -> Tuesday, February 18, 2015 4:00:00pm
"8pm" -> Monday, February 17, 2015 8:00:00pm

After pull request: (Timezone is taken into account)

"3pm" -> Monday, February 17, 2015 3:00:00pm
"4pm" -> Monday, February 17, 2015 4:00:00pm
"8pm" -> Monday, February 17, 2015 8:00:00pm
webdigi commented 9 years ago

we are reviewing and will get back

webdigi commented 9 years ago

how would this work if user enters +1 day most users have 2 days later / + 3 days etc. removing the check might cause issues right?

thapar commented 9 years ago

You seem to still be apprehensive about this pull request. It's very clear that that using dateConversionRequired() was implemented before SugarJS was implemented. After implementing Date Sugar, dateConversionRequired() became useless. I'm not sure who put this code together, but they can definitely confirm this pull request as a fix for anyone not living at the GMT time zone.

webdigi commented 9 years ago

Thanks @thapar for your usual support. We do have several live users using the scheduler. So it might be best if we setup this in a separate copy to test? I am happy to commit this but we need to make sure it does not cause any errors.

The original developer will be back in a few weeks and we can investigate this further.

thapar commented 9 years ago

This will not break for ANY user, as I have already tested the change. In fact, after you merge this pull request, I can clean up some of the superfluous and unused lines of code as well. You probably don't notice the need for this pull request because you are in England (GMT +0). For everyone in the rest of the world using this script, it's tedious because of the timezone bug.

webdigi commented 9 years ago

Please review and confirm if this works as expected