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

[frontend] Timezone and date format for date display and datetime pickers #988

Open
wants to merge 104 commits into
base: master
Choose a base branch
from

Conversation

AlexandreDoneux
Copy link
Contributor

@AlexandreDoneux AlexandreDoneux commented Jan 11, 2024

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 :

  • Allow timezone selection for user
  • Adapt datetime pickers to timezone
    • settings.html : course accessibility and registration
    • accessibility.html : task accessibility
    • submission_query.html : submission filter
    • contests/admin.html
  • Select browser timezone if none given
  • Display dates adapted to timezone
  • Display countdowns adapted to timezone (contest, scoreboard plugins)
  • Adding date format choice
  • Adapt datetime pickers to date format
  • Display dates adapted to date format

This PR is based on PR #985. It will be rebased over time as changes on the base PR appear.

@AlexandreDoneux
Copy link
Contributor Author

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 .datetimepicker.('date').

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
Copy link
Contributor Author

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 ?

@AlexandreDoneux
Copy link
Contributor Author

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 .datetimepicker.('date').

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.

@AlexandreDoneux
Copy link
Contributor Author

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() %}
Copy link
Contributor Author

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.

@@ -569,7 +569,7 @@ def configure_authentication(self, database):
"password": UserManager.hash_password(password),
"bindings": {},
"language": "en",
"timezone": "UTC"})
"timezone": "None"})
Copy link
Contributor Author

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 ?

@AlexandreDoneux AlexandreDoneux marked this pull request as ready for review January 31, 2024 13:29
@AlexandreDoneux AlexandreDoneux changed the title [frontend] Take into account user timezone when displaying dates and using datetime pickers [frontend] Timezone and date format for date display and datetime pickers Feb 2, 2024
@AlexandreDoneux
Copy link
Contributor Author

Rebase on 4de3a17.

@anthonygego
Copy link
Member

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.
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.
Exchanging keys and values for easier manipulation in html.
…te() and adapt_database_date()

to fix test_parsable_text tests + fixes (not applying timezone to min and max dates)
@AlexandreDoneux
Copy link
Contributor Author

Rebase on 05daaf2.

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.

2 participants