-
Notifications
You must be signed in to change notification settings - Fork 153
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
Redirect /opportunity pages to /initiatives #4248
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to go to http://localhost:8000/opportunity and ended up on http://localhost:8000/en/initiatives/initiatives/initiatives/initiatives/initiatives/initiatives/initiatives/initiatives/initiatives/initiatives/initiatives/initiatives/initiatives/initiatives/initiatives/initiatives/initiatives/initiatives/initiatives/ =S
It's also doing pretty weird things when comparing |
Ummm both |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. We'll have to test on prod for edge cases.
We have to manually move opportunity pages to initiatives right?
r+ once you fix the cypress test.
PR updated! @Pomax I can't reproduce the issues you were having. Can you try out this PR again and let me know if you still run into issues?
What kind of edge cases?
Yea, we'll have to manually move those pages using the "move" button on Wagtail admin first before landing this PR. |
edge cases that we don't know about haha. Basically I mean, run on prod and monitor if anything breaks. I don't see any issues with the implementation so the only thing I can think of that will break this is some random edge case we haven't thought about. |
Looks like one thing we're still missing is argument capture. I.e. this: https://foundation-s-issue-2971-2zw1sx.herokuapp.com/pt/opportunity/test/tewo?filter=ckae redirects to: https://foundation-s-issue-2971-2zw1sx.herokuapp.com/pt/initiatives/test/tewo instead of : https://foundation-s-issue-2971-2zw1sx.herokuapp.com/pt/initiatives/test/tewo?filter=ckae |
Co-Authored-By: Pomax <pomax@nihongoresources.com>
Related to #2971
Wagtail page has been moved to
/initiatives
English
https://foundation-s-issue-2971-2zw1sx.herokuapp.com/en/initiatives/page-has-been-moved-initiatives/ (link should redirect to /en/initiatives/...)
French
https://foundation-s-issue-2971-2zw1sx.herokuapp.com/fr/initiatives/page-has-been-moved-initiatives/ (link should redirect to /fr/initiatives/...)
Wagtail page has * not * been moved to
/initiatives
yetEnglish
https://foundation-s-issue-2971-2zw1sx.herokuapp.com/en/opportunity/page-still-under-opportunity/ (link redirects to /en/initiatives/... but it's a 404 since we haven't moved the page yet)
French
https://foundation-s-issue-2971-2zw1sx.herokuapp.com/fr/opportunity/page-still-under-opportunity/ (link redirects to /fr/initiatives/... but it's a 404 since we haven't moved the page yet)