Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #15285 - save browser timezone in User #3886

Closed
wants to merge 1 commit into from

Conversation

shlomizadok
Copy link
Member

@shlomizadok shlomizadok commented Sep 25, 2016

Not sure about this pull request:
Currently when a user chooses "Browser timezone" it is set to nil in the User model. And because it is empty we set it to the cookie's timezone.
The change in this pull request will enforce that the cookie's timezone will be saved for the user.

Would love to hear your comments

@dLobatog
Copy link
Member

dLobatog commented Sep 26, 2016

@shlomizadok Thanks for the effort but app/controllers/concerns/application_shared.rb does the job already, why do you want to save it? It's set on every request and it allows us to not have to take care of yet another property on user. You wrote that code so I guess you have reasons to want to save it..?

@shlomizadok
Copy link
Member Author

I am conflicted to be honest.
On one hand, as you have said: We don't really need it application_shared.rb is handling it correctly.
On the other hand, if a user picks the "Browser timezone", it is never really saved anywhere.

I'll let you decide 😄 - I'm good either way

@dLobatog
Copy link
Member

I prefer to close it - it means zero extra work & risk for a feature that already works pretty well (which you yourself did 😄 )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants