-
Notifications
You must be signed in to change notification settings - Fork 32
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
RFC 34: Automatic redirects #34
base: main
Are you sure you want to change the base?
Changes from 4 commits
687343d
987c425
1c1c418
a7697f1
e585d78
9fb26d7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
# RFC: Redirects changes | ||
|
||
## Models changes | ||
|
||
### Prefix redirects | ||
|
||
We will add a new type of redirect that matches on prefixes rather than full URLs. The rest of the URL path that's not part of the redirect's ``old_path`` field will be appended to the destination path. | ||
|
||
For example, say we have the following prefix redirect: | ||
|
||
- ``/articles`` to ``/news`` | ||
|
||
When a user then requests, ``/articles/the-article/`` this will be matched by the prefix redirect and the user will be redirected to ``/news/the-article/``. | ||
|
||
To allow creating redirects of this type, a new "Match prefix" checkbox/boolean field will be added to redirects. | ||
|
||
#### Choosing the most specific redirect | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One other issue is if a page is moved from We probably want to exclude prefix redirects which point to a page that lives at the source path, as this would turn all sub paths into redirect loops. |
||
|
||
If multiple prefix redirects and/or a single redirect matches the same URL, the most specific redirect is used. | ||
|
||
For example, say we have the following redirects: | ||
|
||
- prefix redirect from ``/articles`` to ``/news`` | ||
- prefix redirect from ``/articles/events`` to ``/events`` | ||
- direct redirect from ``/articles/events/christmas`` to ``/christmas`` | ||
|
||
Requests will be redirected as follows: | ||
|
||
- ``/articles/events/christmas/`` to ``/christmas/`` | ||
- ``/articles/events/other-event/`` to ``/events/other-event/`` | ||
- ``/articles/the-article/`` to ``/news/the-article/`` | ||
|
||
## Admin changes | ||
|
||
### Adding new redirects automatically | ||
|
||
#### On page move | ||
|
||
We will add a new checkbox to the page move UI which asks the user if they would like to create a redirect from the old location to the new one. | ||
|
||
 | ||
|
||
If this is checked, a redirect to the page will be created from the previous URL of the page. If the page has any children, this redirect will be a prefix redirect. | ||
|
||
#### On page slug change | ||
|
||
When a page’s slug is changed, a checkbox will appear underneath the slug field asking the user if they would like to create a redirect from the old slug. | ||
|
||
If this is checked when the page is published with the new slug, a redirect to the page will be created from the previous URL of the page. If the page has any children, this redirect will be a prefix redirect. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe a slug change and move functionality are essentially the same and should be merged? Such that a once published page doesn't have a text input field for the slug but a Move button? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems to me like a reasonable solution to the question at line 51
|
||
|
||
**Q: What if the page is saved as draft? The redirect cannot be created yet but the user who publishes it may not be aware that there is a checkbox on the promote tab.** | ||
|
||
#### When unpublishing or deleting a page | ||
|
||
When unpublishing or deleting a page, the user can add a redirect from that page’s URL to another page. If they create one, they will be given the option to repoint existing redirects at the new page, delete the existing redirects, or (when not deleting) do nothing. | ||
|
||
### Redirect management | ||
|
||
#### Searching for prefix redirects | ||
|
||
The search will return any prefix redirect that matched the searched path. If multiple redirects match, the results will be displayed in order of specificity with most specific first. | ||
|
||
#### Management from the page explorer interface | ||
|
||
 | ||
|
||
We will add a “redirects” icon at the top right of the page explorer. This will indicate the number of redirects where their “from path” is from within this section. Clicking this will link the user to the redirect management which would be filtered to show only redirects from this section. This button will only be visible to users who have permission to manage redirects. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could alternatively show the redirects in the page list without actually making them a page type. I think that it would be a bit more intuitive (since redirects do feel a little like pages) and make it less likely that people would forget that they have lots of redirect clutter. I might show them like pages but with a much smaller height, to indicate that they are not actually pages (and so that they don't entirely clutter the page list). And then maybe have a toggle for whether they're shown in the list. |
||
|
||
We will also add a new “Add redirect here” button in the “more” menu. This takes the user to the add redirect UI with the prefix pre-filled (and undeletable). | ||
|
||
An alternative to this would be to create a “RedirectPage” type which is a page type that acts as a redirect when served, but these aren’t as flexible and are more heavier to handle in the database than redirects are. | ||
|
||
#### Bulk actions | ||
|
||
Removing many redirects at once is quite tedious. This will be more of a problem when we start adding new ways to add redirects. | ||
|
||
We should improve the redirect search to allow filtering by prefix, and add a bulk delete action which can use checkboxes by each row or all redirects that match the current query. | ||
|
||
## Drawbacks | ||
|
||
### Redirect chains and loops | ||
|
||
It's possible with both prefix and direct redirects to create "redirect chains" where one redirect just leads to another redirect. These are much more likely to happen after we introduce the automatic creation of prefix redirects. For example: | ||
|
||
Say we have a section within a section: ``/articles/events`` | ||
|
||
If the user renames ``articles`` to ``news``, then moves ``events`` out into its own section, this will create two prefix redirects: | ||
|
||
- ``/articles`` to ``/news`` | ||
- ``/news/events`` to ``/events`` | ||
|
||
A request to ``/articles/events/christmas/`` would be redirected to ``/news/events/christmas/`` then redirected again to ``/events/christmas/``. | ||
|
||
No attempt will be made to resolve redirect chains/loops on the server-side as this depends on the knowledge that the each step of the redirect chain will result in a 404. While we can figure out if a URL points to a Wagtail page, it's impossible to know for sure that the redirect URL will result in a 404, as there may be URLs configured in the web server that Wagtail does not know about. | ||
|
||
The same goes for redirect loops, we will rely on web browsers to limit the number of redirects the user receives to break out of any loops. |
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.
Could we achieve this by simply allowing the user to add an asterisk to the end of the path? We could use a regex validator to ensure they aren't added elsewhere, and we can set a flag on save if we need a convenient way to tell the two types apart.
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 agree with the current RFC text. Wagtail assumes that a redirect always implies the full nested hierarchy, i.e.
*
. So if one movesarticle/
tonews/
then nothing can be nested inarticle/
anymore, since any request forarticle/<slug>
redirects tonews/<slug>
... if I understand it correctly? This way users won't be burdened with exceptions, priorities and the whole idea of URL resolution.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.
@benjaoming My point refers specifically to how this could be handled in the UI... so, instead of having a separate "Match prefix" checkbox/boolean, the user would simply add an asterisk to the end of the path. They could do the same thing for the target path too (or leave it off to direct all matches to a specific path).
Not always... only if the "Match prefix" checkbox/boolean field
Redirects are only queried if no page can be found matching the requested URL... so, if you added a page to a location covered by a previous redirect, the page would be served at that URL (no redirection would take place)
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 think that the asterisk option is likely to confuse people, especially non-technical users. I think it might be best to explicit write out the behavior of each option, maybe in a radio select. Maybe something along these lines:
/old/foo
→/new
), e.g. if deleting a whole tree/old/foo
→/new/foo
), e.g. if moving a whole tree