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

Update to Django 4.0 #429

Merged
merged 20 commits into from
Apr 1, 2022
Merged

Update to Django 4.0 #429

merged 20 commits into from
Apr 1, 2022

Conversation

ddabble
Copy link
Member

@ddabble ddabble commented Mar 24, 2022

This also updates some other requirements, and replaces some deprecated features (a84d45c).

Furthermore, a change in Django 4.0 made it necessary to move database queries from path converters to views (a5eaf62).

Additional highlights:

  • Made interpolated static files use absolute URLs (0423840)
  • Made incomplete cached user details add data from LDAP (0855485)
  • Fixed slow loading of the course registration list (1a80d62) - even though its speed can still be vastly improved by using e.g. pagination
  • Added django-extensions (0041dc1)
  • Added django-debug-toolbar (95102f0)
    • This will only be active if installed - either manually or through requirements_dev.txt
    • Adds a hideable sidebar that renders on top of other content, which contains multiple panels showing a lot of useful debug info. Activating some of the panels will make requesting pages noticeably slower, but these heavier panels are deactivated by default in the DEBUG_TOOLBAR_CONFIG setting.

ddabble added 16 commits March 24, 2022 14:04
* [test_reservation_extra.py] The `localize()` method has effectively been removed
  (see https://docs.djangoproject.com/en/4.0/releases/4.0/#zoneinfo-default-timezone-implementation)
* [requirements.txt] Also updated some other requirements - most notably `channels` from version 2 to 3, which has been long overdue
  * Also, replaced underscores in some of the package names with dashes, as that's what's displayed on the packages' PyPI pages
* [test_locale_utils.py] `datetime_safe` has been deprecated (see https://docs.djangoproject.com/en/4.0/releases/4.0/#miscellaneous-1)
* [routing.py] These changes were based on the `channels` 3.0.0 release notes (https://channels.readthedocs.io/en/stable/releases/3.0.0.html)
* [static.py] `ManifestStaticFilesStorage` now uses named regex groups in its `patterns` field
  (see django/django@781b442)
This fixes an issue with some browsers not being able to properly interpret relative URLs, and it also improves PyCharm's code suggestions in
the interpolated XML files (due to it interpreting the `{% static %}` token as a Django template tag).
Or, in other words, if the user data in the database is missing some values, the missing values are replaced with data from LDAP.
There was a change in Django 4.0 that made path converters run in an async context, which caused a `SynchronousOnlyOperation` error when querying
the database from a custom path converter (like subclasses of the previous `url_utils.SpecificObjectConverter`).
Using `sync_to_async()` did not work, but getting the view's "focused" object through path converters (instead of e.g. through views' `get_object()`
method) was an unconventional hack anyway, and so this was completely removed, and replaced by logic in the views.

It can also be noted that the previously mentioned error only happened when querying the database in the path converters' `to_python()` method,
not the `to_url()` method - where the former is usually invoked when Django receives a request and tries finding a view with a matching path,
and the latter is mainly invoked when reversing path names while constructing a response in views and templates.
This is why making database queries in `SpecificPageByTitle.to_url()` does not need to be changed.

Also removed the now unused `tests.utility.template_view_get_context_data()`.
This was caused by the `create` part of request URLs being matched by the `<PageTitle:title>` path parameter of `DocumentationPageDetailView`.
This could be reproduced by running both `contentbox/tests/test_urls.py` (but not `test_views.py`) and the docs tests in the same test run.
Not entirely sure where and why the tests failed, but it's possible it had something to do with the previous code in `docs/urls.py` being run
when the module was imported in an earlier run test, and `Page.objects.get_or_create()` not being re-invoked when running the docs tests.
Passing positional instead of keyword arguments, makes reversing the URL less error-prone, as one doesn't have to search through the whole codebase
to update the keyword argument names when the URL's path parameter is changed.

Exceptions to this include reversing e.g. `machine_detail`, as the URL contains multiple (and unconventionally named) parameters,
and it's easier to understand what the passed arguments are, if passed as keyword arguments.
Mainly that they're now class-based instead of function-based.
Also moved and refactored some common code between the previous `get_reservation_rules()` and `get_machine_data()` views to a method on
`ReservationRule` named `get_exact_start_and_end_times_list()`.
This was caused by a JavaScript error, due to the `CKEDITOR_CONFIG_FROM_DJANGO` variable not being defined,
because `config_from_django.js` is not linked when the widget is a `ckeditor_uploader/widgets/CKEditorUploadingWidget` instead of a
`web/widgets/CKEditorUploadingWidget` (as `Content.content` is a `RichTextUploadingField`, which is defined in the `django-ckeditor` package).
DST (daylight saving time) starts in Norway in less than a week, and so some of the tests comparing local datetimes
with UTC datetimes (which is what the database provides when getting model objects with datetime fields) fail.
Also added a missing translation for "shown".
@codecov
Copy link

codecov bot commented Mar 28, 2022

Codecov Report

Merging #429 (95102f0) into dev (8bb5fa0) will increase coverage by 0.52%.
The diff coverage is 88.47%.

@@            Coverage Diff             @@
##              dev     #429      +/-   ##
==========================================
+ Coverage   88.01%   88.53%   +0.52%     
==========================================
  Files         137      134       -3     
  Lines        5183     5209      +26     
==========================================
+ Hits         4562     4612      +50     
+ Misses        621      597      -24     
Impacted Files Coverage Δ
make_queue/templatetags/reservation_extra.py 90.69% <ø> (-2.33%) ⬇️
make_queue/tests/utility.py 100.00% <ø> (ø)
make_queue/urls.py 100.00% <ø> (ø)
make_queue/views/reservation/machine.py 100.00% <ø> (ø)
dataporten/ldap_utils.py 67.44% <43.75%> (+5.37%) ⬆️
make_queue/views/admin/course.py 55.84% <50.00%> (-0.74%) ⬇️
web/static.py 76.19% <62.96%> (-23.81%) ⬇️
util/url_utils.py 83.33% <83.33%> (+2.08%) ⬆️
docs/views.py 86.66% <85.71%> (+1.32%) ⬆️
make_queue/models/reservation.py 96.92% <85.71%> (-0.36%) ⬇️
... and 22 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8bb5fa0...95102f0. Read the comment docs.

ddabble added 3 commits March 29, 2022 11:44
This will only be active if installed - either manually or through `requirements_dev.txt`.
@ddabble ddabble merged commit 9644bb4 into dev Apr 1, 2022
@ddabble ddabble deleted the feature/django-4.0 branch April 1, 2022 16:00
@ddabble ddabble mentioned this pull request Apr 4, 2022
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