Skip to content

Conversation

@feanil
Copy link
Contributor

@feanil feanil commented Oct 20, 2025

If ENABLE_MKTG_SITE is True, MKTG_URLS['ROOT'] must be set. However if
it is set to the same value as the LMS_ROOT_URL (which points to this
view), you can end up in an infinite redirect loop. If the two URLs do
match, don't redirect, just fall through to the content that this page
would have responded with instead.

This impacts #37053 since that would cause the ENABLE_MKTG_SITE=True path to always be called.

These are out of date and while they still work with the proxy object we
don't need them anymore.  Just look up the setting directly.
If ENABLE_MKTG_SITE is True, MKTG_URLS['ROOT'] must be set.  However if
it is set to the same value as the LMS_ROOT_URL (which points to this
view), you can end up in an infinite redirect loop.  If the two URLs do
match, don't redirect, just fall through to the content that this page
would have responded with instead.
@feanil feanil force-pushed the feanil/fix_branding_redirect_loop branch from 1e5b7f8 to a0ab489 Compare October 21, 2025 16:12
@feanil feanil marked this pull request as ready for review October 21, 2025 16:12
@feanil feanil requested a review from a team as a code owner October 21, 2025 16:12
@feanil feanil requested a review from a team October 21, 2025 16:12
@feanil feanil merged commit b55c44a into master Oct 21, 2025
53 checks passed
@feanil feanil deleted the feanil/fix_branding_redirect_loop branch October 21, 2025 17:27
@robrap
Copy link
Contributor

robrap commented Oct 23, 2025

I see. You are leaving the legacy code in place for now. Thanks.
Hopefully we can get in some sort of fix before you delete the legacy code.

@feanil
Copy link
Contributor Author

feanil commented Oct 23, 2025

Longer term I think all of the implementation in this year is going to be replaced with just the redirects to the new MFE. In that situation I would expect you guys to have your own either catalog, MFE or other landing page that you want to send people to.

See #36785 for more details.

@robrap
Copy link
Contributor

robrap commented Oct 23, 2025

Thanks @feanil. We'll see how this shakes out during the next named release. I'm unclear whether or not edge will want the new MFE, etc. But, that's an issue for another time. I think we're all set right now with your implementation.

ktyagiapphelix2u pushed a commit to edx/edx-platform that referenced this pull request Oct 29, 2025
…irect_loop

feanil/fix branding redirect loop
Akanshu-2u pushed a commit to edx/edx-platform that referenced this pull request Oct 29, 2025
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.

4 participants