Skip to content

Redirects: allow editing of redirects #9418

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

Closed
agjohnson opened this issue Jul 7, 2022 · 9 comments · Fixed by #9593
Closed

Redirects: allow editing of redirects #9418

agjohnson opened this issue Jul 7, 2022 · 9 comments · Fixed by #9593
Assignees
Labels
Improvement Minor improvement to code

Comments

@agjohnson
Copy link
Contributor

agjohnson commented Jul 7, 2022

Currently, redirects cannot be edited. The view and forms should allow for editing, and we should add a form view and UI in addition to the existing views. I don't think we have any reason for keeping redirects immutable, so this seems like it might have been an oversight or a set of views that was never updated.

@agjohnson agjohnson added the Improvement Minor improvement to code label Jul 7, 2022
@benjaoming
Copy link
Contributor

See also this comment from @SimonBiggs

#2904 (comment)

@ericholscher
Copy link
Member

ericholscher commented Aug 30, 2022

Putting this in the sprint, because we got bogged down with a grand redirects project. Let's just start making the current version better in obvious ways for now :)

@SimonBiggs
Copy link

Thanks @ericholscher 🙂

@SimonBiggs
Copy link

SimonBiggs commented Aug 30, 2022

Let's just start making the current version better in obvious ways for now :)

Also, I know it's potentially a significant difference but the following approach has the potential to be quite powerful:

Another alternative, is that we do have the capacity for the redirects to be defined within the readthedocs.yml, but we make it so that only the readthedocs.yml within the main default branch is utilised for these "project level" configuration items.

There is a huge maintenance benefit in having the redirects defined within the repository's version control (redirects working within repo forks, undos, distributed responsibility, review).


Also, if there was capacity for a regex based redirect (with group resolution) so that the following could work, that would be epic:

\/a\/b\/c\/([^\/]+)\/e.html -> /a/b/c/some_prefix_$1/e.html

Or something like the following for cases where directory nesting gets re-ordered:

\/([^\/]+)\/([^\/]+)\/index.html -> /$2/$1/index.html

-- https://regex101.com/r/8fbKLv/2

@agjohnson
Copy link
Contributor Author

There is a huge maintenance benefit in having the redirects defined within the repository's version control

We definitely all would like to see this eventually, but this is actually the point that we wrestled with the most. The experience around putting per-project redirects in a per-version configuration is quite awkward.

But, if you are looking for an immediate way to accomplish this, this is an interesting community project that I've pointed folks towards in the past:

https://pypi.org/project/readthedocs-cli/

It uses our API and a file you can check in to your repository to manage redirects.

Also, if there was capacity for a regex based redirect (with group resolution)

Regex rules have always seemed like a really obvious experience here, but we've had unfortunate experiences implementing similar advanced rules in the past. One constraint we have here is that redirects get evaluated in our 404 handling. Coupled with a moderate amount of malicious/bot scans, activating 404 handling, regex rules and complex conditional rules have strong potential to overwhelm our web servers.

@SimonBiggs
Copy link

SimonBiggs commented Aug 30, 2022

regex rules and complex conditional rules have strong potential to overwhelm our web servers.

Yup, fair point.

I wonder if regex resolutions only happen just prior to 404, as in all other redirects run first, and, any time a redirect based on a regex is found, then it is cached. And those cached redirects are utilised prior to any regex being utilised. The regex cache gets invalidated if the maintainer changes the original regex. And then, on top of that, implement a quota on the number of "new regex resolutions" that one documentation base url can handle globally in a given second. If the user is about to see a 404 and the regex tests would be run, but if the documentation is over its "regex resolution quota" for that second (/minute/hour), the regex evaluation gets skipped, and the user is sent straight to 404.

Might something like that have a chance to work?

Essentially if a bot is spamming various 404s of a page, the regex quota of that page will quickly expire. But as long as a user clicks a link when a bot isn't spamming the website, then the new regex will be found, and cached, and that user's particular redirect won't need to be run again. Given the likely limited number of redirects, I would expect most useful redirects would be populated in the cache quite readily, and users wouldn't need to compete with bots for the regex quota in the long term.

But, if you are looking for an immediate way to accomplish this, this is an interesting community project that I've pointed folks towards in the past:

https://pypi.org/project/readthedocs-cli/

I will definitely check that out! Thank you! 🙂

@ssbarnea
Copy link

Please make this happen. Putting this configuration inside the default repository branch is the normal approach, same taken by other automation systems that had the same dilemma, examples github actions and zuul ci, on both the main branch has more authority than others and there are specific configuration options that must happen first on default branch before the build system would use them.

@TheTripleV
Copy link

With editable redirects and an api for it now, is there a plan for version level redirects? That would be really useful and is the main reason for https://github.com/wpilibsuite/sphinxext-rediraffe . Project level redirects are just infeasible for versioned documentation. As links for multiple versions pop up on Google, with only project level redirects, links to old versions will break as the "new" paths won't exist.

@ericholscher
Copy link
Member

@TheTripleV We do support version-level redirects here: https://docs.readthedocs.io/en/stable/user-defined-redirects.html#page-redirects -- but they apply to all versions. Can you clarify the exact redirect you'd like to see, and how it would be configured?

@stsewd stsewd assigned stsewd and unassigned humitos Sep 8, 2022
@stsewd stsewd moved this from Planned to Needs review in 📍Roadmap Sep 12, 2022
Repository owner moved this from Needs review to Done in 📍Roadmap Sep 14, 2022
stsewd added a commit that referenced this issue Sep 14, 2022
Looks like we already allow updating redirects via the API https://docs.readthedocs.io/en/stable/api/v3.html#redirect-update, so this allows updating redirects from the UI. I have moved the form to its own page as we do with more of our CRUD operations.

Closes #9418.

Co-authored-by: Manuel Kaufmann <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement Minor improvement to code
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

8 participants