Proxito: redirect to default version from root language #10313

stsewd · 2023-05-10T02:03:33Z

stsewd · 2023-05-10T18:27:00Z

docs/user/user-defined-redirects.rst

+as set in your project settings.
+
+This works for readthedocs.io (|org_brand|) and :doc:`custom domains </custom-domains>`.
+For |com_brand| this can be enabled by contacting :doc:`support </support>`.


This is since this "feature" is only available when using the new proxito implementation.

benjaoming

This looks great!! Happy to close an old issue so efficiently.

I wondered about cases where a project contains:

a) No translations
b) A file called /es/test.html
c) ..or maybe a file called /es/latest/test.html

But I concluded that we aren't raising the "TranslationNotFound" error unless the path also doesn't exist? Is that correct?

docs/user/user-defined-redirects.rst

benjaoming · 2023-05-11T15:37:15Z

readthedocs/proxito/tests/test_redirects.py

+            "/en",
+            "/en/",


stsewd · 2023-05-11T23:26:17Z

But I concluded that we aren't raising the "TranslationNotFound" error unless the path also doesn't exist? Is that correct?

Not sure if I understand the question, but for the cases that you listed:

a) No translations: since the project doesn't have translations (just an en language), a TranslationNotFound error will be raised for a path like /es/latest/
b) A file called /es/test.html: If the project is a multiversion project and has the es translation, test.html will be interpreted as a version, and since the that version doesn't exist a version not found will be raised.
c) ..or maybe a file called /es/latest/test.html: If the project is a multiversion project and has the es translation, that will be resolved correctly.

Co-authored-by: Benjamin Balder Bach <[email protected]>

benjaoming · 2023-05-12T12:55:09Z

@stsewd

a) No translations: since the project doesn't have translations (just an en language), a TranslationNotFound error will be raised for a path like /es/latest/

If a project is a multiversion project, it cannot use paths like /es/file.html or /es/latest/file.html for regular files at all, because they will just return TranslationNotFound? Or can a project have a file stored in this path if it doesn't have an es translation?

I'm wondering if strictly always resolving [a-z]{2}/ as a language slug without a fallback to a file path is too strict. There are A LOT of language codes, for instance Norwegian is no and might be useful for other things if the project doesn't want a Norwegian translation :)

We can definitely treat this as a separate issue.

stsewd · 2023-05-12T14:33:33Z

If a project is a multiversion project, it cannot use paths like /es/file.html or /es/latest/file.html for regular files at all, because they will just return TranslationNotFound? Or can a project have a file stored in this path if it doesn't have an es translation?

If a project is a multiversion project, it always needs to serve a file from a language subpath. If it's a single version project it can have any path in the url, it will resolve just fine.

benjaoming · 2023-05-12T15:16:30Z

Garh of course 🤦 I managed to ask a very silly question. I think where I got it wrong initially was thinking that we do redirects to a default language like how Django i18n url patterns does. Which we don't - we use our own /page redirects.

I don't have anymore inputs or questions :)

humitos

Thanks for the whole work you have done on El Proxito lately. This PR is a lot easier to review now that we all have a better understanding of how things work.

humitos · 2023-05-24T07:37:57Z

docs/user/user-defined-redirects.rst

+A link to the root language of your documentation (``<slug>.readthedocs.io/en/``)
+will redirect to the  :term:`default version` of that translation.


Suggested change

A link to the root language of your documentation (``<slug>.readthedocs.io/en/``)

will redirect to the :term:`default version` of that translation.

A link to the root language of your documentation (``<slug>.readthedocs.io/<lang>/``)

will redirect to the :term:`default version` of that language.

humitos · 2023-05-24T07:38:58Z

docs/user/user-defined-redirects.rst

+
+Root language redirects on |com_brand| can be enabled by contacting :doc:`support </support>`.
+
+For example::


Suggested change

For example::

For example, accessing the English language of the project will redirect you to the ``stable`` version::

readthedocs/proxito/views/serve.py

humitos · 2023-05-24T07:43:14Z

readthedocs/proxito/tests/test_redirects.py

+            "/en",
+            "/en/",
+        ]
+        for path in paths:


Instead of a for here, shouldn't these be pytest.parametrize? (I know you love that one 😉 ). It would be clearer when the test fail to know what is the exact case we are testing.

Note there are more cases like this in the other tests we can change to pytest.parametrize.

I think parametrize doesn't work with tests that inherit from TestCase, I'll give it a try

yep, they don't work there https://docs.pytest.org/en/latest/how-to/unittest.html#pytest-features-in-unittest-testcase-subclasses.

Proxito: redirect to default version from root language

0bcb26a

Closes #2968

stsewd requested a review from a team as a code owner May 10, 2023 02:03

stsewd requested a review from humitos May 10, 2023 02:03

auto-assign bot assigned stsewd May 10, 2023

Docs

d125fe0

stsewd requested a review from a team as a code owner May 10, 2023 02:11

stsewd requested a review from benjaoming May 10, 2023 02:11

Handle exception in 404 view

f403c3c

stsewd commented May 10, 2023

View reviewed changes

benjaoming approved these changes May 11, 2023

View reviewed changes

Apply suggestions from code review

bbcdc05

Co-authored-by: Benjamin Balder Bach <[email protected]>

humitos approved these changes May 24, 2023

View reviewed changes

stsewd added 2 commits May 24, 2023 10:28

Updates from review

8182b5b

Merge branch 'main' into redirect-from-root-language

75988b1

stsewd merged commit c9884a0 into main May 24, 2023

stsewd deleted the redirect-from-root-language branch May 24, 2023 15:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Proxito: redirect to default version from root language #10313

Proxito: redirect to default version from root language #10313

stsewd commented May 10, 2023

stsewd May 10, 2023

benjaoming left a comment

benjaoming May 11, 2023

stsewd commented May 11, 2023

benjaoming commented May 12, 2023 •

edited

Loading

stsewd commented May 12, 2023

benjaoming commented May 12, 2023 •

edited

Loading

humitos left a comment

humitos May 24, 2023

humitos May 24, 2023

humitos May 24, 2023

humitos May 24, 2023

stsewd May 24, 2023

stsewd May 24, 2023

		A link to the root language of your documentation (``<slug>.readthedocs.io/en/``)
		will redirect to the :term:`default version` of that translation.


		Root language redirects on \|com_brand\| can be enabled by contacting :doc:`support </support>`.

		For example::

	For example::
	For example, accessing the English language of the project will redirect you to the ``stable`` version::

Proxito: redirect to default version from root language #10313

Proxito: redirect to default version from root language #10313

Conversation

stsewd commented May 10, 2023

Choose a reason for hiding this comment

benjaoming left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stsewd commented May 11, 2023

benjaoming commented May 12, 2023 • edited Loading

stsewd commented May 12, 2023

benjaoming commented May 12, 2023 • edited Loading

humitos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benjaoming commented May 12, 2023 •

edited

Loading

benjaoming commented May 12, 2023 •

edited

Loading