Upgrade back-end code from Django 4.2 to 5.2 LTS - #8052
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughChangesDjango platform upgrade
Frontend logout flow
Sequence Diagram(s)sequenceDiagram
participant User
participant Router
participant Logout
participant AccountsLogout
participant LoginPage
User->>Router: Open command/logout
Router->>Logout: Lazy-load Logout
Logout->>AccountsLogout: POST /accounts/logout/
AccountsLogout-->>Logout: Return response
Logout->>LoginPage: Redirect with nextUrl
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Warning One or more dependencies are approaching or past End-of-Life. |
Triggered by ffecf67 on branch refs/heads/issue-8046
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
specifyweb/frontend/js_src/lib/components/ChooseCollection/index.tsx (1)
175-177: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAvoid posting logout twice in this flow.
The
pingcall performs a POST, then the redirect mountsLogout, which POSTs to/accounts/logout/again. Let one layer own the logout request; verify whether the pre-redirect POST is still required.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@specifyweb/frontend/js_src/lib/components/ChooseCollection/index.tsx` around lines 175 - 177, Update the logout flow around the ping call and redirect to ensure /accounts/logout/ is POSTed only once. Determine whether Logout owns the required logout request, then remove the redundant pre-redirect ping POST if so, while preserving the redirect to formatUrl('/specify/command/logout/', { next: nextUrl }).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@requirements-testing.txt`:
- Around line 2-3: Update the mypy dependency pin in requirements-testing.txt to
a version supported by django-stubs==5.2.9, such as mypy==1.13.0; retain the
django-stubs pin and do not keep the incompatible mypy==1.5.1 combination.
In `@requirements.txt`:
- Line 14: Update the PyJWT dependency version in requirements.txt from 2.12.1
to 2.13.0 or later, ensuring the pinned version includes the PyJWKClient
scheme-validation fix.
In `@specifyweb/backend/context/testurls.py`:
- Line 12: Update the URL pattern for views.viewsets to use a string-compatible
path converter instead of <int:level>, so the level argument remains a string
when passed to testsviews.viewsets().
In `@specifyweb/frontend/js_src/lib/components/Logout/index.tsx`:
- Line 17: Update the logout redirect callback in the Logout component to read
and validate the route’s next value, preserving it when valid and falling back
to “/specify/” otherwise. Use that resolved value in the formatUrl call instead
of always passing the fixed “/specify/” target.
- Around line 16-22: Update the logout failure handler in the promise chain
around softError so a failed POST transitions away from the indefinitely
displayed LoadingScreen. Render an error/retry state or navigate to a terminal
fallback on rejection, while preserving the existing successful redirect
behavior.
- Around line 11-20: Move the logout request and its loading/redirect error
handling from the component render path into a mount-only effect, using the
component’s existing effect and loading utilities. Ensure the POST to
/accounts/logout/ starts once after mount, while preserving the current success
redirect and softError behavior.
---
Nitpick comments:
In `@specifyweb/frontend/js_src/lib/components/ChooseCollection/index.tsx`:
- Around line 175-177: Update the logout flow around the ping call and redirect
to ensure /accounts/logout/ is POSTed only once. Determine whether Logout owns
the required logout request, then remove the redundant pre-redirect ping POST if
so, while preserving the redirect to formatUrl('/specify/command/logout/', {
next: nextUrl }).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3501db84-1148-48c2-861a-22071020f1f5
📒 Files selected for processing (20)
pytest.inirequirements-testing.txtrequirements.txtspecifyweb/backend/batch_identify/urls.pyspecifyweb/backend/context/testurls.pyspecifyweb/backend/context/urls.pyspecifyweb/backend/inheritance/urls.pyspecifyweb/backend/setup_tool/urls.pyspecifyweb/backend/trees/urls.pyspecifyweb/frontend/js_src/lib/components/Atoms/Icons.tsxspecifyweb/frontend/js_src/lib/components/ChooseCollection/index.tsxspecifyweb/frontend/js_src/lib/components/Core/Main.tsxspecifyweb/frontend/js_src/lib/components/Header/userToolDefinitions.tsspecifyweb/frontend/js_src/lib/components/Logout/index.tsxspecifyweb/frontend/js_src/lib/components/Router/Routes.tsxspecifyweb/settings/__init__.pyspecifyweb/specify/migrations/0045_alter_many_to_many_through_fields.pyspecifyweb/specify/urls.pyspecifyweb/specify/utils/case_insensitive_bool.pyspecifyweb/urls.py
💤 Files with no reviewable changes (1)
- specifyweb/settings/init.py
github-project-automation
Bot
moved this from 📋Back Log
to Dev Attention Needed
in General Tester Board
level.xml int => string bump dependencies for `mypy` and `PyJWT`
…vior is preserved
Triggered by d68dac5 on branch refs/heads/issue-8046
There was a problem hiding this comment.
I completed the manual testing:
- Labels are localized correctly and schema-config labels are respected
- Switching languages works correctly
- Logging out from several locations works without a flashing error
- Logging back in takes me to the splash screen
- General UI/UX testing
- WorkBench
- Attachments and attachment bulk upload
Everything worked as expected except that opening the Attachments panel displays two error dialogs. Both report:
'datetime.date' object has no attribute 'utcoffset'
Not sure if it's related to the upgrade. The Attachments features still worked after dismissing the dialogs.
|
@rijulpoudel I think that is outside of the scope of this PR, I think it is happening here: as that is the only place we use datetime.UTC, which is not a part of django. Might be a different issue to write up.
|
There was a problem hiding this comment.
- Ensure strings are localized correctly
- verify that labels are not raw schema labels, i.e. ensure schema config labels are respected.
- Ensure that switching languages behaves correctly.
- Log out from several locations
- Ensure that no error dialogs flash on the screen
- Logging back in takes you to the splash screen
- General UI/UX testing
- especially the workbench and attachments/attachment bulk upload
It's a bit difficult to tell if switching languages is behaving correctly, as several are incomplete, but from what I could see all of the expected areas were still localized.
Edited since I hadn't spotted Rijul's testing before I submitted my review--apologies! There's a lot of screenshots that can be moved over to the issue
Rijul's issue didn't exist in main from my testing. It appears that date fields having no utcoffset is causing issues saving or accessing any newly created records, since I created a record set from Workbench, and found that the timestamps were off, and that I could not open the recordset(s). This affects other new records, such as Collection Objects.
We should wait until this issue is fixed regardless of whether it's within the scope of this PR or not to check general UI/UX, as it's affecting the basic functionality.
Below are all of the screenshots of the UTC offset issues I got.
From this branch:
When opening the record set(s):
Specify 7 Crash Report - 2026-07-29T16_47_28.993Z.txt
From main:
There are some oddities in timestamps because of this. Newly created record sets/queries appear to have a different timestamp from main. Above is an example--the two recordset created '5 hours ago' were actually created in main, recently, while the one recordset created an hour ago (ID 498) was created in the branch and appears to be accurate. On main, recordset 498 has a timestamp of 'now.'
Further evidence: from the branch, you can see that it the most recent query was created 15 minutes ago. This is accurate. On main, however, the timestamp created says 'now.'
Database is sdnhmherps_2026_07_17 with a login of sdnhmherp for verification/testing, as well. If this gets split into another issue, I'll move all of this over.
Thank you for the work on this Eos! 🫡
|
I don't think this should be a different issue actually. I combed through the Django release notes again, as well as our Django settings, saw something I missed. In 5.0, they set the default value for |
7 participants
Fixes #8046
Upgrade Django from 4.2 to LTS version 5.2.
https://docs.djangoproject.com/en/6.0/releases/5.2/
Changes:
USE_L10N(A localization setting, that is now defaulted to true)GETrequest to logout to aPOSTrequest to be in HTTP (and django) speclogOutuser tool (inHeader/userToolDefinitions) to point to a new front-end endpoint:/specify/command/logoutto match the cache clearing endpointRouter/Routes.tsx) renders theLogoutcomponentLogoutcomponent makes the POST request to the backend and returns a loading screenChecklist
self-explanatory (or properly documented)
Testing instructions
Note for dev testers
If you are testing on your local instance, change
specify7/docker-compose.yml
Line 61 in 4da3228
from
--mode developmentto--mode production. This is because in development mode, webpack will duplicate web requests, which when sending thePOSTrequest, the first one is aborted by the second, which causes ajax to flash an error on-screen before refreshing to the login page. This does not happen in production mode, as far as I have seen.Summary by CodeRabbit
New Features
Bug Fixes
Refactor