Feat: Add support for django5.2 by Talha-Rizwan · Pull Request #2773 · openedx/credentials

Talha-Rizwan · 2025-05-19T11:42:33Z

Resolves issue

Add Django 5.2 Support
This PR updates the codebase to be compatible with Django 5.2 while maintaining compatibility with Django 4.2.

Changes Made:

Update ci and tox files
Ran django-upgrade to apply necessary syntax updates for Django 5.2.
Run tests using tox command and resolved the warning
Ensured all modifications remain backward-compatible with Django 4.2.

django-upgrade usage:
I ran command git ls-files -z -- '*.py' | xargs -0r django-upgrade --target-version 5.2

awais786 · 2025-05-19T13:13:12Z

credentials/settings/test.py

 # Local Directories
 TEST_ROOT = path("test_root")
-DEFAULT_FILE_STORAGE = "django.core.files.storage.FileSystemStorage"
+STORAGES = {


It will effect django42 behaviour.

DEFAULT_FILE_STORAGE can't be defined with STORAGES dict, so the only option left is to go with the format of django52 (STORAGES {}) and search for any breakage in code and update to STORAGES['default']['BACKEND'] from this DEFAULT_FILE_STORAGE.
Also same for STATICFILES_STORAGE

awais786 · 2025-05-19T17:31:51Z

credentials/apps/api/v2/tests/test_serializers.py

        serializer = UserGradeSerializer(context={"request": Request(site=self.site)})
        updated_at_dt = datetime.now()
-        updated_at_utc = updated_at_dt.replace(tzinfo=pytz.UTC)
+        updated_at_utc = updated_at_dt.replace(tzinfo=ZoneInfo("UTC"))


from django.utils.timezone import datetime, timezone datetime.now(timezone.utc)

awais786 · 2025-05-19T17:40:36Z

requirements/base.in

 pillow
 pygments
 python-memcached
+python-slugify


why this new package require here ?

awais786 · 2025-05-20T11:20:53Z

.github/workflows/ci.yml

          make tests
      - name: Run code coverage
-        if: matrix.python-version == '3.12' && matrix.django-version == 'django42'
+        if: matrix.python-version == '3.12' && matrix.django-version == 'django52'


use django42

awais786 · 2025-05-22T13:12:48Z

credentials/settings/base.py

 EXTRA_APPS = []
 SESSION_EXPIRE_AT_BROWSER_CLOSE = False
-STATICFILES_STORAGE = "django.contrib.staticfiles.storage.ManifestStaticFilesStorage"
+


You should match the existing value here "django.contrib.staticfiles.storage.ManifestStaticFilesStorage"

awais786 · 2025-06-06T02:29:01Z

@feanil Please review.

Copilot

Pull Request Overview

Adds compatibility for Django 5.2 alongside existing Django 4.2 support by updating CI/tox configurations, code syntax, and tests.

Expanded CI and tox environments to include Django 5.2
Applied django-upgrade syntax changes (e.g., re_path → path, @admin.display)
Refactored tests and code to remove deprecated settings and switch to timezone‐aware imports

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tox.ini	Added `django52` to envlist and deps
credentials/urls.py	Swapped `re_path` for `path` in badge routes
credentials/settings/base.py	Removed deprecated `USE_DEPRECATED_PYTZ` and `USE_L10N`
credentials/apps/catalog/tests/factories.py	Switched to `django.utils.timezone` imports and added `skip_postgeneration_save`
credentials/apps/badges/admin.py	Replaced `short_description` with `@admin.display`
credentials/apps/api/v2/tests/test_views.py	Updated timezone logic to use `datetime.now(timezone.utc)`
credentials/apps/api/v2/tests/test_serializers.py	Same timezone import and usage update
credentials/apps/api/v2/decorators.py	Replaced `request.META` lookup with `request.headers`
.github/workflows/ci.yml	Expanded Django matrix to include 5.2

Comments suppressed due to low confidence (1)

credentials/apps/api/v2/decorators.py:29

Accessing host via request.headers.get('host') may not be reliable. Consider using request.get_host() to properly obtain the host header and respect Django’s ALLOWED_HOSTS setting.

f"[{request.user.username}] originating from [{request.headers.get('host', 'Unknown')}] "

Copilot · 2025-06-09T19:00:44Z

credentials/apps/api/v2/tests/test_views.py

        # simulate updating the existing record with the new field in the data
-        dt = datetime.datetime.now()
-        last_updated_at = dt.replace(tzinfo=pytz.UTC)
+        last_updated_at = datetime.now(timezone.utc)


Importing datetime from django.utils.timezone may not provide the standard datetime class. Consider from datetime import datetime and either use timezone.now() or from django.utils.timezone import utc for clarity.

Same, this is a good suggestion and we should switch to just calling timezone.now()

Copilot · 2025-06-09T19:00:45Z

credentials/apps/api/v2/tests/test_serializers.py

        serializer = UserGradeSerializer(context={"request": Request(site=self.site)})
-        updated_at_dt = datetime.now()
-        updated_at_utc = updated_at_dt.replace(tzinfo=pytz.UTC)
+        updated_at_utc = datetime.now(timezone.utc)


Same import concern here: datetime from django.utils.timezone may not map to the built-in datetime. Prefer from datetime import datetime or use timezone.now() to generate a UTC timestamp.

Suggested change

updated_at_utc = datetime.now(timezone.utc)

updated_at_utc = timezone.now()

This is a good suggestion, using timezone.now will give you zone aware times in UTC and we don't have to use the passthrough import of datetime which may go away in the future.

feanil · 2025-06-09T19:07:32Z

credentials/apps/api/v2/tests/test_serializers.py

        serializer = UserGradeSerializer(context={"request": Request(site=self.site)})
-        updated_at_dt = datetime.now()
-        updated_at_utc = updated_at_dt.replace(tzinfo=pytz.UTC)
+        updated_at_utc = datetime.now(timezone.utc)


This is a good suggestion, using timezone.now will give you zone aware times in UTC and we don't have to use the passthrough import of datetime which may go away in the future.

feanil · 2025-06-09T19:08:21Z

credentials/apps/api/v2/tests/test_views.py

        # simulate updating the existing record with the new field in the data
-        dt = datetime.datetime.now()
-        last_updated_at = dt.replace(tzinfo=pytz.UTC)
+        last_updated_at = datetime.now(timezone.utc)


Same, this is a good suggestion and we should switch to just calling timezone.now()

feanil · 2025-06-09T19:09:13Z

tox.ini

 [testenv]
 deps =
    django42: -r requirements/django.txt
+    django52: -r requirements/django.txt


This isn't actually testing Django 5.2, the django.txt file only pins to the current version of django.

Suggested change

django52: -r requirements/django.txt

django52: django>=5.2

feanil · 2025-06-10T15:08:23Z

credentials/apps/catalog/tests/factories.py

 class CourseFactory(factory.django.DjangoModelFactory):
    class Meta:
        model = Course
+        skip_postgeneration_save = True


What's the reason for this change?

reverted. it was by the package.

awais786 · 2025-06-16T17:14:16Z

I've updated all the code, and this PR looks good overall. However, the storage-related work is still pending. For django52, we need to use the STORAGES dictionary, which hasn't been addressed yet. We will follow edx-platform implementation for this change.

openedx/openedx-platform#36885 ( This PR will update the default file storage usage )

feanil · 2025-06-20T18:42:09Z

@awais786 that makes sense, should the helper function that Daniel added get moved to edx-django-utils? so it can also be used here?

awais786 · 2025-06-24T15:20:37Z

should the helper function that Daniel added get moved to edx-django-utils? so it can also be used here?

This repository doesn't use get_storage_class, so there's no usage here. However, the function could be moved to a shared location to support other packages like edx-val or similar.

awais786 · 2025-06-24T15:21:21Z

@feanil should we merge this now and fix the STORAGES later or wait for the owning team review ?

feanil · 2025-07-17T18:52:47Z

PR's been open long enough, if you're happy with it, I think it's okay to merge. Looks like there are some conflicts and it needs a rebase.

Let me know if you need me to press the merge button.

awais786 · 2025-07-23T19:17:20Z

@feanil I can't merge this. Please go ahead.

* feat: Add support for django5.2 * fix: updated timezone utc * fix: storage compatibility issue for django42 * fix: storage compatibility issue for django42 * fix: reverting few changes. * fix: reverting few changes. * fix: fixing timezone.now() changes. * fix: fixing timezone.now() changes. * fix: fixing timezone.now() changes. * fix: fixing timezone.now() changes. * fix: Update factories.py * fix: Update factories.py --------- Co-authored-by: awais qureshi <awais.qureshi@arbisoft.com>

Talha-Rizwan requested a review from a team as a code owner May 19, 2025 11:42

github-project-automation bot added this to Aperture-Maintained May 19, 2025

Talha-Rizwan marked this pull request as draft May 19, 2025 11:50

Talha-Rizwan force-pushed the django52support branch from f5c49bb to 4a4a551 Compare May 19, 2025 11:53

Talha-Rizwan changed the title ~~Add support for django5.2~~ Feat: Add support for django5.2 May 19, 2025

Talha-Rizwan force-pushed the django52support branch 4 times, most recently from 77c5b15 to 4a3b27a Compare May 19, 2025 13:01

Talha-Rizwan marked this pull request as ready for review May 19, 2025 13:11

awais786 reviewed May 19, 2025

View reviewed changes

requirements/base.in Outdated

pillow

pygments

python-memcached

python-slugify

Copy link

Contributor

awais786 May 19, 2025

Choose a reason for hiding this comment

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

why this new package require here ?

Talha-Rizwan force-pushed the django52support branch from 0f4556e to 2ab3cf0 Compare May 20, 2025 10:54

Talha-Rizwan requested a review from awais786 May 20, 2025 11:13

awais786 reviewed May 20, 2025

View reviewed changes

Talha-Rizwan force-pushed the django52support branch 2 times, most recently from ca0d71b to d9ea704 Compare May 22, 2025 11:44

Talha-Rizwan requested a review from awais786 May 22, 2025 11:51

awais786 reviewed May 22, 2025

View reviewed changes

Talha-Rizwan force-pushed the django52support branch from d9ea704 to 66ca851 Compare May 22, 2025 13:57

deborahgu moved this to Todo in Aperture-Maintained Jun 5, 2025

deborahgu added waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. waiting for eng review PR is ready for review. Review and merge it, or suggest changes. labels Jun 5, 2025

Talha-Rizwan and others added 6 commits June 5, 2025 22:17

feat: Add support for django5.2

31b7620

fix: updated timezone utc

0fe80ac

fix: storage compatibility issue for django42

a7e8df0

fix: storage compatibility issue for django42

2eb134d

fix: reverting few changes.

c064077

fix: reverting few changes.

c8b4341

awais786 force-pushed the django52support branch from b40bc24 to c8b4341 Compare June 6, 2025 02:17

feanil requested a review from Copilot June 9, 2025 18:58

Copilot AI reviewed Jun 9, 2025

View reviewed changes

feanil requested changes Jun 9, 2025

View reviewed changes

awais786 added 4 commits June 9, 2025 17:02

fix: fixing timezone.now() changes.

d995b0e

fix: fixing timezone.now() changes.

15b81e8

fix: fixing timezone.now() changes.

d6a520a

fix: fixing timezone.now() changes.

3ce2308

feanil reviewed Jun 10, 2025

View reviewed changes

awais786 added 2 commits June 10, 2025 11:59

fix: Update factories.py

5968282

fix: Update factories.py

7d0ba2b

feanil approved these changes Jun 20, 2025

View reviewed changes

awais786 requested review from a team and removed request for a team July 1, 2025 14:06

jsnwesson removed the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label Jul 1, 2025

awais786 requested a review from justinhynes July 2, 2025 13:15

deborahgu merged commit 7ceaf2c into openedx:master Jul 23, 2025
10 checks passed

github-project-automation bot moved this from Todo to Done in Aperture-Maintained Jul 23, 2025

	updated_at_utc = datetime.now(timezone.utc)
	updated_at_utc = timezone.now()

Conversation

Talha-Rizwan commented May 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

awais786 commented Jun 6, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

awais786 commented Jun 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

feanil commented Jun 20, 2025

Uh oh!

awais786 commented Jun 24, 2025

Uh oh!

awais786 commented Jun 24, 2025

Uh oh!

feanil commented Jul 17, 2025

Uh oh!

awais786 commented Jul 23, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

awais786 commented Jun 16, 2025 •

edited

Loading