-
Notifications
You must be signed in to change notification settings - Fork 139
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
[frontend] Timezone and date format for date display and datetime pickers #988
base: master
Are you sure you want to change the base?
[frontend] Timezone and date format for date display and datetime pickers #988
Conversation
b25bb75
to
531c1e6
Compare
We currently use Tempus Dominus Bootstrap4 v5.0.1 for the datetime pickers. However while adapting it's use to take into account the user's timezone I encountered what seems to be a bug of some sort : Modifying the picker's input a second time does not affect the value retrieved with The more recent versions of Tempus Dominus might fix this but does not support the integrated timezone option anymore. Because of this the date must be transformed before being passed to the picker. This raises the question of updating Tempus Dominus or using anoter picker. I have found this picker that contains date and time selection. It has a better documentation than Tempus Dominus and seems decent. |
@@ -97,7 +96,7 @@ def page(self, time_planner): | |||
else: | |||
del tasks_data[courseid][user_task["taskid"]] | |||
# Remove courses with no unfinished available tasks with deadline and lti courses | |||
if (len(tasks_data[courseid]) == 0) or course.is_lti(): | |||
if (len(tasks_data[courseid]) == 0): # or course.is_lti(): # removing lti check momentarily |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is_lti
is a method for a course object, however, here course is a taskset object.
Why was that check needed and is it still needed ? If yes any idea how I can do that ?
a8ff331
to
55b1d70
Compare
We currently use Tempus Dominus Bootstrap4 v5.0.1 for the datetime pickers. However while adapting it's use to take into account the user's timezone I encountered what seems to be a bug of some sort : Modifying the picker's input a second time does not affect the value retrieved with The more recent versions of Tempus Dominus might fix this but does not support the integrated timezone option anymore. Because of this the date must be transformed before being passed to the picker. This raises the question of updating Tempus Dominus or using anoter picker. I have found this picker that contains date and time selection. It has a better documentation than Tempus Dominus and seems decent. |
Rebase on 69317b1. |
@@ -20,7 +20,7 @@ <h3>Contest</h3> | |||
<i class="fa fa-info fa-fw"></i> | |||
</div> | |||
<div class="col-sm-11" style="margin-left:-10px;"> | |||
{% if start > start.now() %} | |||
{% if start > start.utcnow() %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that stored dates are UTC we need to check the actual date with .utcnow(). Previous versions using .now() returned the current date based on the user's machine timezone. Users could have accessed the contest outside it's given accessibility by changing their timezone.
Instead of checking on the frontend we could do like everywhere else and check with the AccessibleTime object stored in the course data.
inginious/frontend/installer.py
Outdated
@@ -569,7 +569,7 @@ def configure_authentication(self, database): | |||
"password": UserManager.hash_password(password), | |||
"bindings": {}, | |||
"language": "en", | |||
"timezone": "UTC"}) | |||
"timezone": "None"}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced "UTC" by another default string value to be sure it is well understood no timezone is selected.
We could replace with None value but it would be passed to html as a variable name and would need to be transformed into a string. It would be a bit more messy for the code. What's best ? Leave a string to represent no selected timezone ? Something else ?
0525160
to
bf06893
Compare
Rebase on 4de3a17. |
I will wait for #985 to be done before reviewing this one. |
Adding an "accessible_period" dict to define start and end period.
… accessibility and registration Courses will now use a dictionnary for "accessible_period" and "registration_period" with a start and end element. Tasks will have an "accessibility_period" dict with an extra soft_end element.
…ccessibility/registration structure
Putting boolean and period dict inside one element : "accessibility": {"is_open": bool, "period": {"start": ..., "soft_end": ..., "end": ...}}. This way dispenser_data["config"] init in TableOfContents does not have to be changed.
…nd DB To keep the same structure as the task accessibility (dict with bool and period dict).
When initiating task_config from yaml we give a default value if there are missing values. When these values are dictionnaries they are actually all the same. Modifying one will modify all of them. Therefor we give an new instance at each default value to allow transformation into strings before rendering. Also had a mixup on the transformation itself.
Adding representer to use !!timestamp tag in yaml for datetime objects. Moving string transformation in datetime before changing taskset yaml file. Fixing datetime transformation to datetime before toc.html render (was permanently changing task_config).
Structure made in previous commits was not valid due to the way other method of this class work. Values for the dates could be None. Resulting in errors when comparing NoneType and datetime objects.
Plus fixing bug : When deleting tasks in course edit, the missing tasks were added back (check_dispenser_data function) with default parameters. Default date parameter beeing None it wasn't correctly handled by the date conversion (dispenser_data_str_to_datetimes function).
+ fixing dispenser_config not passed to render
Changing their name and structure to allow transformation of strings and dates inside python list.
…me for use in templates Was setting up AccessibleTime to use datetime objects for the period dates. But in the templates the task_config is parsed in JSON and it can't parse datetime objects. So whe transform the dates into strings before render. AccessibleTime needs to be able to handle string dates because of it's use in the templates.
…y settings from task yaml Was checking the accessibility settings as they were using the new structure (in DB or taskset.yaml). But when checking if the tasks have legacy settings in their task.yaml it raises an exception because the legacy accessibility has the old structure.
The boolean "is_open" is not really used. Instead we now only use the start, soft_end and end date. We will differenciate the access types by setting some dates to a minimal or maximal value. For example : a task that is always open will have a minimal start date and maximal soft_end and end dates. In the custom acces mode, the dates are now optional. You can for example set an end date without setting a start date because it will automatically be set to the minimal value. The dates are stored in MongoDB as ISODate() objects and we will use datetime.min and datetime.max in python code for minimal and maximal values. However, MongoDB stores dates with a precision to the millisecond. Datetime objects have a precision to the nanosecond. Therefor we have some transformations when retrieving dates from mongoDB (in AccessibleTime).
…sform minimal date When using the timestamp representer to transform datetime objects into strings with "!!timestamp" tags, the date.min caused problems. The zeros in front of the 1 (0001-01-01 00:00:00) would not be considered when using datetime.strftime(). It would simply delete the zeros causing the date to not have the correct structure for the timestamp.
…led on taskset object We were storing still open courses as they were already past due.
Using datetime objects as dictionnary keys will change depending on if a timezone was given. If a timezone is given the key will have an offset in addition to the date and time. In our case, adding the utc timezone to the c key solves the problem.
Was transforming moment objects to Javascript Date objects. However these Date objects do not hold timezone data and define the date with a local timezone.
…pt scoreboard plugin to user's timezone
Exchanging keys and values for easier manipulation in html.
…ity under each task
…te() and adapt_database_date() to fix test_parsable_text tests + fixes (not applying timezone to min and max dates)
… string handling in accessibility.html
e13856d
to
cf2955c
Compare
Rebase on 05daaf2. |
This pull request aims to adapt displayed dates and form date inputs to the users timezone (see #526). This would allow better comprehension over courses accessibility and task deadlines for any users abroad/ in another timezone. Currently the displayed dates are based on the timezone of the machine where the INGInious is deployed.
Thanks to this PR the dates would be stored as UTC without offset and would be adapted on the frontend to whichever timezone the user has selected in their account preferences. Or, if none is selected, to the default timezone of their browser.
TO DO :
This PR is based on PR #985. It will be rebased over time as changes on the base PR appear.