Skip to content
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

Exact Redirects changed behavior from working, now 404s #4008

Closed
stevepiercy opened this issue Apr 24, 2018 · 13 comments
Closed

Exact Redirects changed behavior from working, now 404s #4008

stevepiercy opened this issue Apr 24, 2018 · 13 comments
Assignees
Labels
Bug A bug
Milestone

Comments

@stevepiercy
Copy link
Contributor

Expected Result

All of our previously working and configured Exact Redirects should continue to work.

Actual Result

All of our configured Exact Redirects now result in 404s.

Details

At some point within the last few days, perhaps with the recent changes to Exact Redirects in #3965 or #3593, all of our configured Exact Redirects now result in 404s. We expected them to continue to function. A complete list of our redirects is available at https://readthedocs.org/dashboard/pylons/redirects/

Example configuration:

Redirect Type:
Exact Redirect

From URL:
/docs/pyramid.html

To URL:
https://trypyramid.com/resources.html

https://docs.pylonsproject.org/en/latest/docs/pyramid.html used to redirect to https://trypyramid.com/resources.html. It now 404s.

This is a backward incompatible change, and it caught us by surprise. Is it possible to restore backward compatibility so that the previously entered redirects would still work without the now required "language/version" path preface? Or must I remove the redirects and add them all back with the preface?

@stsewd stsewd added the Bug A bug label Apr 24, 2018
@agjohnson agjohnson added this to the 2.4 milestone Apr 24, 2018
@humitos
Copy link
Member

humitos commented Apr 24, 2018

Hi @stevepiercy, thanks for your report.

I think that you were affected by #3965. That PR fixes a bug and I understand that it's incompatible with your setup. Unfornately (for your setup), how is working now it's the correct way and you/we need to solve your problem in a different way now: re-writing your rules.

I need to understand what are your exact needing here:

  1. if you need to fix any /en/latest/(you From URL defined here) to the to_url, I could prepend the /en/latest to all of your redirects in the database
  2. if you need to redirect version independent /<any lang>/<any version>/docs/pyramid.html to to_url we should change the Exact redirect to a Page redirect.

Let me know this. In the meantime I will keep taking a look/talking to the other folks.

@stevepiercy
Copy link
Contributor Author

@humitos thank you! This is not a rush. Let's see what the team comes up with in its internal discussion.

This project has only one version and language. It's primary purpose is to serve as redirects and mile markers to direct users to the correct resources and avoid 404s from old bookmarks, pages, and search engine indices.

Ideally backward compatibility would be maintained without configuration changes, and the redirects without specifying language/version would be implicit across all languages/versions for the project. In fact, that might be a desirable feature. IMO, DRY > explicit > implicit.

Pending the outcome of your internal discussion, and if there is no better alternative, then I would appreciate your execution of option 2 above.

Side note for option 2, the UI for Page Redirects make it appear that they would not work for external URLs (even though they do):

Outcome: /$lang/$version/docs/pyramid.html -> /$lang/$version/https://trypyramid.com/resources.html

I can file a separate issue for that, if you think it merits it.

@humitos
Copy link
Member

humitos commented Apr 24, 2018

Side note for option 2, the UI for Page Redirects make it appear that they would not work for external URLs (even though they do):

Yes. I think the Redirects section from the admin needs some love. It's very confusing how it works.

I just created a sample project locally to debug this and I finally understood the UI by debugging the code :)

Look at this example:

captura de pantalla_2018-04-24_18-03-08

Then, if you open the browser at:
http://rtd-project-a.readthedocs.io/en/latest/external you will be redirected to an external link and also http://rtd-project-a.readthedocs.io/external works

@stevepiercy
Copy link
Contributor Author

@humitos agreed. It's the point immediately before clicking the Add button that is misleading with its "Outcome" grey box.

Outcome

If I click the Add button, it then displays fine and works fine.

Anyway, should I submit a separate issue for that UI bit?

@stsewd
Copy link
Member

stsewd commented Apr 25, 2018

@humitos I thought that page redirects can only redirect to pages inside rtd 🤔 https://docs.readthedocs.io/en/latest/user-defined-redirects.html#page-redirects

@humitos
Copy link
Member

humitos commented Apr 25, 2018

Anyway, should I submit a separate issue for that UI bit?

Yes, please. So we track this specific thing in a separate issue (it may be a similar one, if you can take a minute to search before opening the new one, that would be great :D )

Also, when you know, please let us know what is the decision you made regarding this original issue and the redirects that do not work as you expect.

Thanks!

@stevepiercy
Copy link
Contributor Author

Submitted #4023.

I was awaiting further response from your internal discussion:

Pending the outcome of your internal discussion, and if there is no better alternative, then I would appreciate your execution of option 2 above.

Thank you!

@humitos
Copy link
Member

humitos commented Apr 25, 2018

@stevepiercy hey! I just updated our database to use Page Redirects for your redirects. I tried some of them and they work as you want. Please confirm and close this issue if it's solved.

@humitos humitos self-assigned this Apr 25, 2018
@stevepiercy
Copy link
Contributor Author

@humitos the redirects work, however the Admin UI does not reflect the changes you made in the database:
https://readthedocs.org/dashboard/pylons/redirects/

I think all the redirects should display the prefix of /en/latest to remove ambiguity.

google chrome005

Thank you!

@humitos
Copy link
Member

humitos commented Apr 25, 2018

however the Admin UI does not reflect the changes you made in the database:

I changed the Exact Redirect to Page Redirect (I see the change in that image), so it work for any language and any version. Examples of redirects:

It works as I described in my previuos comment.

Otherwise, I'm not following you.

@stsewd
Copy link
Member

stsewd commented Apr 25, 2018

@humitos just for clarification, all redirects also work for external links? If so, that isn't specified in the docs.

@humitos
Copy link
Member

humitos commented Apr 25, 2018

@stsewd I don't know if all. At least Page redirects. For the other, we need to check the source code.

@stevepiercy
Copy link
Contributor Author

@humitos You're right. Please ignore my comment. I'm closing this issue as complete. Thank you for your assistance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug
Projects
None yet
Development

No branches or pull requests

4 participants