search

thoughtbot / Tropos

Weather and Forecasts for Humans
troposweather.com
MIT License
1.51k stars 201 forks source link

Use GMT offset as Courier channel name #187

Closed klaaspieter closed 8 years ago

klaaspieter commented 8 years ago

This partially fixes the issue where certain time zones will never receive push notifications because the time zone names used by Rails are different from those used by iOS (See thoughtbot/Tropos#185 for more information)

This fixes it by not using the names, but the relative offset from UTC as the channel name. Currently we subscribe to us-pacific when the iOS time zone is set to San Francisco.

This PR changes it to subscribe to the channel minus-0700 instead. This is a more reliable way of identifying time zones across platforms.

calleluks commented 8 years ago

I had one question, other than that this looks good to merge.

cpytel commented 8 years ago

This is a good change because it also gets us closer to being able to have the user select the time for the notification.

sharplet commented 8 years ago

Will this code handle daylight savings changes gracefully?

klaaspieter commented 8 years ago

@cpytel hadn't thought of that, but that's absolutely true!

@sharplet there is some logic in place that automatically unsubscribes from a channel if the time zone changes. So assuming all goes well this deals with DST.

It'll only try once so worst case scenario it's possible for someone to be subscribe to multiple channels. I created #188 to track this issue.