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

set timezone automatically #1901

Merged
merged 14 commits into from
Aug 28, 2024
Merged

set timezone automatically #1901

merged 14 commits into from
Aug 28, 2024

Conversation

terrazoon
Copy link
Contributor

@terrazoon terrazoon commented Aug 23, 2024

Description

Set the timezone automatically for the user based on what the browser says the timezone is.

We were using US/Eastern and 7 or 8 similar timezones to represent the US, but it turns out these are "legacy" timezones and the correct forward-leaning timezones all look like "America/Los_Angeles". These work fine for scheduling jobs and so forth, but just look a little different.

Since are going to be setting the timezone automatically, the user will not have the option to set their timezone to something else, so I suppose we need to remove the Change Timezone feature on the User Profile.

TODO (optional)

  • TODO The setTimezone.js script is in the js folder instead of the javascripts folder because I didn't want it to disappear into all.js and get invoked everywhere all the time. Is that acceptable or what is the alternative?
  • TODO After the timezone is set in the cookie, we need to invoke the python side of things, and I have just made a random call to the set_timezone() method from another function in the dashboard. This works and doesn't break any tests but is a bit inelegant.
  • TODO Should we remove the Change Timezone feature on the User Profile? They can change it but it will get changed right back the next time the dashboard reloads.

Security Considerations

N/A

@terrazoon terrazoon marked this pull request as draft August 23, 2024 18:13
@terrazoon terrazoon changed the title initial set timezone automatically Aug 23, 2024
@terrazoon terrazoon marked this pull request as ready for review August 26, 2024 15:06
Copy link
Contributor

@jonathanbobel jonathanbobel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@heyitsmebev heyitsmebev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Ken, I left a couple of comments related to the js scripts placement

app/templates/views/user-profile.html Show resolved Hide resolved
app/templates/views/dashboard/dashboard.html Outdated Show resolved Hide resolved
@terrazoon terrazoon requested a review from heyitsmebev August 27, 2024 19:59
@terrazoon terrazoon self-assigned this Aug 27, 2024
@terrazoon terrazoon linked an issue Aug 27, 2024 that may be closed by this pull request
Copy link
Contributor

@heyitsmebev heyitsmebev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@heyitsmebev heyitsmebev merged commit b20c8ec into main Aug 28, 2024
11 checks passed
@heyitsmebev heyitsmebev deleted the notify-admin-1877 branch August 28, 2024 00:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatically set time zone for users
3 participants