Move RedirectMiddleware to appsembler.settings plugin - #817
Conversation
Use the django_sites_extensions RedirectMiddleware, but if TAHOE_ENABLE_DOMAIN_REDIRECT_MIDDLEWARE enabled (default ON), then insert related middleware classes before that standard redirect class.
|
Another possibly annoying but hopefully useful PR from me 😺 |
| # Soooo just in case | ||
| beeline.add_trace_field("redirect_middleware_exception", True) | ||
| pass | ||
| cache_key = '{prefix}-{site}'.format(prefix=settings.REDIRECT_CACHE_KEY_PREFIX, site=site.domain) |
There was a problem hiding this comment.
all this and below are in original django_sites_extensions.middleware.RedirectMiddleware
|
@bryanlandia can I take a moment and give big thanks to what you've been doing? I still didn't check this PR, but it looks like a straight-to merge PR. I will take a look soon. |
|
Sorry for the delay here. I've been focusing on a rather important bug: RED-1698. Please let me know if this is a blocker or could wait few more days. I've added two more reviewers to hopefully get a faster triage. |
thraxil
left a comment
thraxil
left a comment
There was a problem hiding this comment.
LGTM. The flag defaults to True, so existing Tahoe deployments shouldn't have to change their settings.
We just have to remember that this needs to move to production_lms.py when applying it to the juniper branch.
OmarIthawi
left a comment
OmarIthawi
left a comment
There was a problem hiding this comment.
I've added a suggestion to ensure the order is correct. Apart from that the PR looks good to me. Thanks @bryanlandia!
committing Omar's suggestion Co-authored-by: Omar Al-Ithawi <i@omardo.com>
4 participants
Split out original edX Django Extensions RedirectMiddleware (Redirect model-based 302s) from Tahoe-specific redirect middleware for default site minus whitelisted paths. Use appsembler.sites.middleware.RedirectMiddleware when enabled via settings. Default to enabled.
This change allows us to skip this behavior on standalone installs.
Add the first tests for this feature, too!