Skip to content

Proxito: standardize how to handle redirects from subprojects #10049

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

Open
stsewd opened this issue Feb 20, 2023 · 11 comments
Open

Proxito: standardize how to handle redirects from subprojects #10049

stsewd opened this issue Feb 20, 2023 · 11 comments
Labels
Needed: design decision A core team decision is required Needed: documentation Documentation is required

Comments

@stsewd
Copy link
Member

stsewd commented Feb 20, 2023

Currently, we don't have tests nor documentation for this.

We do check the redirects from subprojects when a matching /projects/subproject/ path 404.
The current behavior is as follows:

Exact redirects

We try to match the subproject redirects using the full path /projects/subproject/foo.html, we don't check for the redirects of the parent project. The generated redirect will be absolute to the current domain.

For example, the following redirect defined in the subproject

  • from: /projects/subproject/foo.html
  • to: /en/latest/

Will redirect docs.example.com/projects/subproject/foo.html to docs.example.com/en/latest/

Page redirect

We try to match the redirects using the captured filename foo.html, we don't check for the redirects of the parent project. The generated redirect will be relative to the subproject.

For example, the following redirect defined in the subproject

  • from: foo.html
  • to: /

Will redirect docs.example.com/projects/subproject/en/latest/foo.html to docs.example.com/projects/subproject/en/latest/.

Prefix redirect

We try to match the redirects using the captured filename foo.html, we don't check for the redirects of the parent project. The generated redirect will be relative to the subproject.

For example, the following redirect defined in the subproject

  • from: /prefix/

Will redirect docs.example.com/projects/subproject/prefix/foo.html to docs.example.com/projects/subproject/en/latest/foo.html.

Changes

I think page and prefix redirect are ok, exact redirects need to change to a less confusing implementation. Something like:

Match the path after the /projects/subproject/ part, the generated redirect will still be absolute to the current domain.

For example, the following redirect defined in the subproject

  • from: /foo.html
  • to: /en/latest/

Will redirect docs.example.com/projects/subproject/foo.html to docs.example.com/en/latest/.

I think we should be fine ignoring redirects from the parent if we are handling a 404 from a subproject.

@stsewd stsewd added the Needed: design decision A core team decision is required label Feb 20, 2023
@ericholscher
Copy link
Member

ericholscher commented Feb 21, 2023

I think page and prefix redirect are ok, exact redirects need to change to a less confusing implementation. Something like:

Match the path after the /projects/subproject/ part, the generated redirect will still be absolute to the current domain.

This seems confusing to me. I think we want Exact redirects to match the exact file path? So it seems like they are doing what they should do?

That said, exact redirects outside of the subproject's path shouldn't be evaluated on the subproject, and we should raise an error if someone tries to create them, I think? But I do think there's reasonable use cases for redirecting a subproject URL to the top-level project (/projects/sub/old.html to /projects/sub2/new.html for example)

Though perhaps that should be set on the parent project? Seems fine to support it both places tho.. 🤔

@humitos
Copy link
Member

humitos commented Mar 1, 2023

IMO, subproject redirects shold always relative to the path prefix. If users want to redirect outside the subproject, they should use a full URL for the to argument. I think that's the easiest way to explain.

With that in mind,

  • Type: Exact
  • From: /foo.html
  • To: /new/bar.html

Will redirect:

  • https://docs.example.com/projects/subproject/foo.html
  • https://docs.example.com/projects/subproject/new/bar.html

If the user wants to go outsite the subroject, they would need to write the full URL as:

  • Type: Exact
  • From: /foo.html
  • To: https://docs.example.com/en/latest/new/bar.html

Following this idea, all the redirects will be inside their own projects unless explicitly set via a full URL it has to go outsite it. Besides, this logic follows the current implementation for the other 2 types: Page and Prefix.

Extra points: if we really want to go outside the subproject without writing a full URL, we should have an extra argument/field for these exact redirect where we can "force" to start considering the path from /. The previous example would be:

  • Type: Exact
  • From: /foo.html
  • To: /new/bar.html
  • Force from /: True

Will redirect:

  • https://docs.example.com/projects/subproject/foo.html
  • https://docs.example.com/new/bar.html

This allows subprojects to be "self-contained" and move them from being a subproject to main project and keep their redirects always working.

@ericholscher
Copy link
Member

ericholscher commented Mar 1, 2023

I'm still -1 on exact redirects not matching the exact URL.

With that in mind,

  • Type: Exact
  • From: /foo.html
  • To: /new/bar.html

Will redirect:

  • https://docs.example.com/projects/subproject/foo.html
  • https://docs.example.com/projects/subproject/new/bar.html

Isn't this what Page redirects are for?

If the user wants to go outsite the subroject, they would need to write the full URL as:

  • Type: Exact
  • From: /foo.html
  • To: https://docs.example.com/en/latest/new/bar.html

I think this also makes sense. I think saying Exact redirects only match inside the subproject, but can redirect to any fully formed URL seems reasonable? Having them work differently between parent projects and subprojects feels really confusing.

I think they should work the same (match the whole URL path), but have restrictions on when they are matched. We should also not let a subproject create an Exact Redirect that doesn't start with the path, so it's validated on creation, not just evaluation.

Following this idea, all the redirects will be inside their own projects unless explicitly set via a full URL it has to go outsite it. Besides, this logic follows the current implementation for the other 2 types: Page and Prefix.

Right, but those redirect types are defined as understanding versions/projects, and not the exact path (it's in the name! Exact Redirect). I'm not saying Exact Redirects are the right abstraction, but if we want another solution, we should make it instead of changing how Exact Redirects work sometimes.

Extra points: if we really want to go outside the subproject without writing a full URL, we should have an extra argument/field for these exact redirect where we can "force" to start considering the path from /. The previous example would be:

  • Type: Exact
  • From: /foo.html
  • To: /new/bar.html
  • Force from /: True

Will redirect:

  • https://docs.example.com/projects/subproject/foo.html
  • https://docs.example.com/new/bar.html

This allows subprojects to be "self-contained" and move them from being a subproject to main project and keep their redirects always working.

This is adding more edge cases and confusion to the user, I think. I don't think this is the right abstraction for additional work on redirects, since I think Redirects v2 should likely remove all these "redirect type" concepts and just work with regex or similar as we've discussed in the past.

My thinking is still: Redirects work the same in subprojects as in normal projects, but they are restricted in the path they can be created/evaluated on.

@humitos
Copy link
Member

humitos commented May 24, 2023

@ericholscher

My thinking is still: Redirects work the same in subprojects as in normal projects, but they are restricted in the path they can be created/evaluated on.

👍🏼 -- can you add some detailed examples about how this implementation would work exactly? I think that will help us to be all in the same page here and avoid miss-understandings.

@ericholscher
Copy link
Member

ericholscher commented May 24, 2023

Basically:

Subproject redirect

A subproject that is served on docs.example.com/projects/subproject/en/latest/

  • Type: Exact
  • From: /projects/subproject/foo/
  • To: /projects/subproject/bar/
  • Result: This will work and redirect docs.example.com/projects/subproject/foo/ to docs.example.com/projects/subproject/bar/

--

--

  • Type: Exact
  • From: /foo/
  • To: /bar/
  • Result: Form validation error, subproject redirects must start with /projects/subproject/

Parent project redirect

  • From: /projects/subproject/foo/
  • To: /projects/subproject/bar/
  • Result: Form validation error, subproject redirects must start be set on the subproject

--

  • From: /foo
  • To: /projects/subproject/bar/
  • Result: This will work and redirect docs.example.com/foo/ to docs.example.com/projects/subproject/bar/

@stsewd
Copy link
Member Author

stsewd commented May 25, 2023

I think it could also be helpful if we match the exact redirects from the parent project, that way users can have all redirects in one central place, just an idea. For example, translations, subprojects, translations from subprojects, etc, all of those paths will check the exact redirects from the project that owns the domain (parent project) in addition to the redirects from the current project.

And maybe force to always have exact redirects in the parent project by disabling exact redirects from the translations/subprojects.

@humitos
Copy link
Member

humitos commented May 25, 2023

@ericholscher would this work?

  • Type: Exact
  • From: /projects/subproject/foo/
  • To: /projects/another-subproject/foo/
  • Result: This works and redirects 1 subproject to another

I guess it would. It's similar to your full URL example, but using a relative one.


I think your proposal makes everything pretty explicit and seems easy to explain to users. Also, validation errors make more sense and there is nothing hidden to users (as I was proposing with the "hidden scope"), 👍🏼

@humitos
Copy link
Member

humitos commented May 25, 2023

@stsewd

I think it could also be helpful if we match the exact redirects from the parent project, that way users can have all redirects in one central place, just an idea.

Hrm, isn't this opposed to what @ericholscher is suggesting with the form validation errors where "subproject redirects must be set on the subproject"?

@stsewd
Copy link
Member Author

stsewd commented May 25, 2023

@humitos yeah. There is also the problem if a subproject is renamed, then your redirects will no longer work, since the path will never match.

@humitos
Copy link
Member

humitos commented May 25, 2023

@stsewd yeah, that's the trade off of these solutions. The more explicit we make it for users, the less magic we can apply behind the scenes. However, if people want to change the alias and fix a bunch of redirects, they can always use the API for that

@ericholscher
Copy link
Member

I think I'm also OK with the parent project being able to set redirects on the subproject path. I agree it makes it easier to keep everything in one place -- I was a bit conflicted on which approach made sense in my above comment, so happy with either.

The main thing I want is to ensure subproject redirects only operate on the subproject namespace. That seems like the most important thing to me.

@stsewd stsewd added the Needed: documentation Documentation is required label Oct 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needed: design decision A core team decision is required Needed: documentation Documentation is required
Projects
None yet
Development

No branches or pull requests

3 participants