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 Django to version 3.2 #11974

Merged
merged 44 commits into from
May 6, 2024

Conversation

rtibbles
Copy link
Member

@rtibbles rtibbles commented Mar 7, 2024

Summary

  • Updates all Django related dependencies to ones compatible with 3.2
  • Updates translation and language code previously vendored from Django for modification
  • Updates all url function invocations to re_path
  • Updates filterset to filterset_class for DRF viewsets
  • Use the new header attribute for response objects to access header values
  • Explicitly mark tests that access more than one of the test databases with __all__
  • Updates tests that were sending None as a value for a POST - the Django test client no longer allows this
  • Changes is_authenticated check from method invocation to property check
  • Fixes imports
  • Changes function signatures for field methods
  • Updates how we serialize and deserialize JSONFields and DateTimeTzFields to match change in Morango - now both serialization and deserialization behaviour are explicitly flagged - parallel to how Django's own serialization framework handles it except for specific reserved types
  • Adds the now required --all argument to the makemessages command
  • Updates static lib usage
  • Adds setting to set the automatic PK field to AutoIncrementingInteger
  • Stops deletion of querysets that have had distinct called on them
  • Removes python 2 unicode decorator
  • Cleans up all swappable dependency cruft from migrations for FacilityUser
  • Update Django model fields based on warnings/errors from Django
  • Removes bugfixes that are now in Django
  • Handle badly formed GET parameters now raising 400s
  • Update how we do inheritance for anonymous user and facility user, as anonymous users can no longer be an abstract Model class
  • For some bulk operations (channel import, db restore) turn off SQLite referential integrity checks, as they are stricter than PostgreSQL and get verified at statement execution, not transaction commit.
  • Update postres and cryptography dependencies
  • Removes collections and translations monkey patching that was previously required
  • Updates html5lib to not need collections monkey patching

References

Fixes #11726

Reviewer guidance

The urls migration could have been done more selectively by using path instead of re_path in cases where regex is not strictly needed, but that seemed like a task better suited for follow up.

The database attribute of test classes could also have been more selectively applied, but it was easier to use __all__.

Do tests pass?

I didn't update any documentation, as I don't think anything hugely changed.


Testing checklist

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Critical and brittle code paths are covered by unit tests

PR process

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

@rtibbles rtibbles added the TODO: needs review Waiting for review label Mar 7, 2024
@github-actions github-actions bot added DEV: backend Python, databases, networking, filesystem... APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) APP: Facility Re: Facility App (user/class management, facility settings, csv import/export, etc.) APP: Learn Re: Learn App (content, quizzes, lessons, etc.) APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) APP: User Re: User app (sign-in, sign-up, user profile, etc.) APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) SIZE: medium labels Mar 7, 2024
@pcenov
Copy link
Member

pcenov commented Mar 8, 2024

Hi @rtibbles, after a clean installation of the .deb build Kolibri is not loading the setup wizard. Logs and DB:
logs.zip

@rtibbles
Copy link
Member Author

rtibbles commented Mar 8, 2024

Ah, thanks @pcenov! Looks like none of the frontend files are being properly loaded - guessing Django is bypassing our previous setup somehow.

@rtibbles
Copy link
Member Author

rtibbles commented Mar 8, 2024

Latest commit should have fixed this - updated assets will be along in a little while!

@rtibbles rtibbles mentioned this pull request Mar 9, 2024
9 tasks
@pcenov
Copy link
Member

pcenov commented Mar 11, 2024

Hi @rtibbles now the Setup Wizard is working correctly but it's not possible import a channel in any of the supported ways:

2024-03-11_12-26-58.mp4

Logs and DB: logs.zip

@rtibbles
Copy link
Member Author

Who knew that changing everything would be so difficult?! :)

@rtibbles
Copy link
Member Author

Looks like there's some more issues with how we parse arguments for the import channel command (and probably the import content command too).

@rtibbles
Copy link
Member Author

@pcenov this should now be fixed. I also looked for other possible instances of the same error. If you could also test the facility import, single user import + syncing, on my own setup + merge to single user syncing, and facility syncing that should cover the places that I updated.

@pcenov
Copy link
Member

pcenov commented Mar 13, 2024

Hi @rtibbles, I found two additional issues while regression testing everything:

  1. The Android app can be installed but closes immediately after I launch it. Logs: android-logs.zip
  2. When migrating a user, the process gets stuck at "Remotely integrating data". If I sign out and sign in again with the same user I can see that the user is actually migrated. Logs: migrate-user-logs.zip
2024-03-13_17-56-28.mp4

@rtibbles
Copy link
Member Author

Hi @pcenov - the android app kolibri log is exceptionally short. Could you rerun the app and grab the logcat output? That might give me some more clues.

@rtibbles
Copy link
Member Author

Some very interesting bugs in the other log, can take a closer look at them!

@pcenov
Copy link
Member

pcenov commented Mar 14, 2024

Hi @rtibbles here's the logcat log: logcat.txt

@rtibbles
Copy link
Member Author

Thank you @pcenov - hah, I think this might be a result of our not having merged 0.16.x into develop recently enough rather than anything I've done here, so I'll get that sorted too. Thank you!

@rtibbles
Copy link
Member Author

Oh, on further investigation, that doesn't seem to be the issue - it just looks very similar to an error we had late on in the 0.16.0 release process.

@rtibbles
Copy link
Member Author

Hi @pcenov - I think I've addressed all the errors that I saw in the logs.

@pcenov
Copy link
Member

pcenov commented Mar 18, 2024

Hi @rtibbles - I confirm that the Android app installs and runs correctly now. I'm still testing the critical workflows with all installers and will let you know if I find anything else that's not working.

@pcenov
Copy link
Member

pcenov commented Mar 19, 2024

Hi @rtibbles the only new issues I was able to identify are for the Mac app. There I am getting a server error at Device > Channels > Select a source which remains in a 'Loading connections' state and it's not possible to import resources.

2024-03-18_17-10-59

Also no libraries are being displayed in the 'Other libraries' section of the Library page:

2024-03-19_12-01-33

logs.zip

rtibbles added 27 commits May 6, 2024 11:20
as Django's anonymous user can no longer be an abstract model.
…t transaction to allow for bulk row insertion of self referential FKs, and optimized deletion.
Don't pass to non-resume command.
Add dummy get_queryset and get_serializer_class methods otherwise.
@rtibbles rtibbles force-pushed the django_unchained branch from 078dad9 to 2418619 Compare May 6, 2024 18:20
@rtibbles rtibbles merged commit 142b0a9 into learningequality:develop May 6, 2024
31 checks passed
@rtibbles rtibbles deleted the django_unchained branch May 6, 2024 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) APP: Facility Re: Facility App (user/class management, facility settings, csv import/export, etc.) APP: Learn Re: Learn App (content, quizzes, lessons, etc.) APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) APP: User Re: User app (sign-in, sign-up, user profile, etc.) DEV: backend Python, databases, networking, filesystem... SIZE: medium TODO: needs review Waiting for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade Django to 3.2
3 participants