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

Fix enabling multiple languages if the APP_NAME in the db was "Arches" #11660 #11687

Open
wants to merge 3 commits into
base: dev/7.6.x
Choose a base branch
from

Conversation

jacobtylerwalls
Copy link
Member

@jacobtylerwalls jacobtylerwalls commented Dec 13, 2024

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Description of Change

setup_db creates a tile for the system setting APP_NAME with a value of "Arches". If the application was running under wsgi.py, then it's likely that the arches system settings object fetched tiles, thus merging the APP_NAME setting tile into the django settings, causing the check against the django settings in urls.py to unexpectedly evaluate True, breaking the ability of a project to enable multiple languages. This wasn't caught in development because the dev workflow doesn't run wsgi.py and thus update_from_db() never ran.

Two fixes:

  • avoid checking APP_NAME in core urls.py. Instead use the same check that's working well in projects to determine "is this project the project being served"
  • merge system settings into the django settings in more cases, not just under wsgi.py but also during development. This was discussed at a recent standup. Fix the docstring that implies that this is not wanted. EDIT: will defer this fix, caused issues with setup_db.

Testing instructions

Issues Solved

Closes #11660

Checklist

  • I targeted one of these branches:
    • dev/8.0.x (under development): features, bugfixes not covered below
    • dev/7.6.x (main support): regressions, crashing bugs, security issues, major bugs in new features
    • dev/6.2.x (extended support): major security issues, data loss issues
  • I added a changelog in arches/releases
  • I submitted a PR to arches-docs (if appropriate)
  • Unit tests pass locally with my changes
  • I added tests that prove my fix is effective or that my feature works
  • My test fails on the target branch

Ticket Background

  • Sponsored by: Getty Conservation Institute
  • Found by: @ekansa

This check sniffs for whether this urls.py is the one being run as a project
(not included by any further apps). For core arches, this would only be true if running core arches without a project. This was failing on a Django exception
inside i18n_patterns().

This check is the same check that we're using in project-level settings.py
@aarongundel
Copy link
Contributor

@jacobtylerwalls the tests seem pretty angry here. I'm guessing related?

@jacobtylerwalls
Copy link
Member Author

I'll back-burner that fix for bringing the dev experience into alignment with prod. was Worth a shot but too hot for a patch.

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.

Unexpected SystemSettings interaction causing failure to run via wsgi.py
2 participants