From cabe0c9c431f55ae9d21d494c6bf461aa30c9a5b Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 6 Nov 2019 15:08:49 -0500 Subject: [PATCH 1/3] Never link to the dashboard from the footer Closes #6137 We use the `get_absolute_url` in other parts of the code where it makes sense to link to the dashboard. So I'm adding an additional parameter. --- readthedocs/api/v2/views/footer_views.py | 4 +++- readthedocs/builds/models.py | 11 ++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/readthedocs/api/v2/views/footer_views.py b/readthedocs/api/v2/views/footer_views.py index 1dd433ec91b..bea5235a6f3 100644 --- a/readthedocs/api/v2/views/footer_views.py +++ b/readthedocs/api/v2/views/footer_views.py @@ -44,7 +44,9 @@ def get_version_compare_data(project, base_version=None): 'is_highest': True, } if highest_version_obj: - ret_val['url'] = highest_version_obj.get_absolute_url() + # Never link to the dashboard, + # users reading the docs may don't have access to the dashboard. + ret_val['url'] = highest_version_obj.get_absolute_url(link_to_dashboard=False) ret_val['slug'] = highest_version_obj.slug if base_version and base_version.slug != LATEST: try: diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py index 82b96c70ce8..97c47fef57c 100644 --- a/readthedocs/builds/models.py +++ b/readthedocs/builds/models.py @@ -77,7 +77,6 @@ from readthedocs.projects.models import APIProject, Project from readthedocs.projects.version_handling import determine_stable_version - log = logging.getLogger(__name__) @@ -289,7 +288,13 @@ def commit_name(self): ) return self.identifier - def get_absolute_url(self): + def get_absolute_url(self, link_to_dashboard=True): + """ + Get absolute url to the docs of the version. + + :param link_to_dashboard: If `False` we never try to link to the dashboard, + we link to the docs even if they result in a 404. + """ # Hack external versions for now. # TODO: We can integrate them into the resolver # but this is much simpler to handle since we only link them a couple places for now @@ -299,7 +304,7 @@ def get_absolute_url(self): f'{self.project.slug}/{self.slug}/index.html' return url - if not self.built and not self.uploaded: + if not self.built and not self.uploaded and link_to_dashboard: return reverse( 'project_version_detail', kwargs={ From 2b4fddb150e3168e16a95592a2dfb34d73f5d836 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 6 Nov 2019 17:53:13 -0500 Subject: [PATCH 2/3] Fix test --- readthedocs/rtd_tests/tests/test_footer.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_footer.py b/readthedocs/rtd_tests/tests/test_footer.py index 0c48c2ad3b9..059ff63b0e5 100644 --- a/readthedocs/rtd_tests/tests/test_footer.py +++ b/readthedocs/rtd_tests/tests/test_footer.py @@ -1,6 +1,7 @@ import mock from django.contrib.sessions.backends.base import SessionBase from django.test import TestCase +from django.test.utils import override_settings from rest_framework.test import APIRequestFactory, APITestCase from readthedocs.api.v2.views.footer_views import ( @@ -114,6 +115,10 @@ def test_not_show_edit_on_github(self): self.assertNotIn('Edit', response.data['html']) +@override_settings( + USE_SUBDOMAIN=True, + PUBLIC_DOMAIN='readthedocs.io', +) class TestVersionCompareFooter(TestCase): fixtures = ['test_data'] @@ -124,7 +129,7 @@ def test_highest_version_from_stable(self): base_version = self.pip.get_stable_version() valid_data = { 'project': 'Version 0.8.1 of Pip (19)', - 'url': '/dashboard/pip/version/0.8.1/', + 'url': 'http://pip.readthedocs.io/en/0.8.1/', 'slug': '0.8.1', 'version': '0.8.1', 'is_highest': True, @@ -136,7 +141,7 @@ def test_highest_version_from_lower(self): base_version = self.pip.versions.get(slug='0.8') valid_data = { 'project': 'Version 0.8.1 of Pip (19)', - 'url': '/dashboard/pip/version/0.8.1/', + 'url': 'http://pip.readthedocs.io/en/0.8.1/', 'slug': '0.8.1', 'version': '0.8.1', 'is_highest': False, @@ -149,7 +154,7 @@ def test_highest_version_from_latest(self): base_version = self.pip.versions.get(slug=LATEST) valid_data = { 'project': 'Version 0.8.1 of Pip (19)', - 'url': '/dashboard/pip/version/0.8.1/', + 'url': 'http://pip.readthedocs.io/en/0.8.1/', 'slug': '0.8.1', 'version': '0.8.1', 'is_highest': True, @@ -177,7 +182,7 @@ def test_highest_version_over_branches(self): base_version = self.pip.versions.get(slug='0.8.1') valid_data = { 'project': 'Version 1.0.0 of Pip ({})'.format(version.pk), - 'url': '/dashboard/pip/version/1.0.0/', + 'url': 'http://pip.readthedocs.io/en/1.0.0/', 'slug': '1.0.0', 'version': '1.0.0', 'is_highest': False, @@ -191,7 +196,7 @@ def test_highest_version_without_tags(self): base_version = self.pip.versions.get(slug='0.8.1') valid_data = { 'project': 'Version 0.8.1 of Pip (19)', - 'url': '/dashboard/pip/version/0.8.1/', + 'url': 'http://pip.readthedocs.io/en/0.8.1/', 'slug': '0.8.1', 'version': '0.8.1', 'is_highest': True, @@ -202,7 +207,7 @@ def test_highest_version_without_tags(self): base_version = self.pip.versions.get(slug='0.8') valid_data = { 'project': 'Version 0.8.1 of Pip (19)', - 'url': '/dashboard/pip/version/0.8.1/', + 'url': 'http://pip.readthedocs.io/en/0.8.1/', 'slug': '0.8.1', 'version': '0.8.1', 'is_highest': False, @@ -219,7 +224,7 @@ def test_highest_version_without_tags(self): ) valid_data = { 'project': 'Version 2.0.0 of Pip ({})'.format(version.pk), - 'url': '/dashboard/pip/version/2.0.0/', + 'url': 'http://pip.readthedocs.io/en/2.0.0/', 'slug': '2.0.0', 'version': '2.0.0', 'is_highest': False, From e90ea847846c70ad562751be25e3a0ec0e7fbee6 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 6 Nov 2019 18:17:39 -0500 Subject: [PATCH 3/3] Fix tests We were testing with non-build versions. But in production is more common to hit this with build versions. --- readthedocs/rtd_tests/tests/test_footer.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/readthedocs/rtd_tests/tests/test_footer.py b/readthedocs/rtd_tests/tests/test_footer.py index 059ff63b0e5..7a5d8586439 100644 --- a/readthedocs/rtd_tests/tests/test_footer.py +++ b/readthedocs/rtd_tests/tests/test_footer.py @@ -240,11 +240,12 @@ class TestFooterPerformance(APITestCase): # The expected number of queries for generating the footer # This shouldn't increase unless we modify the footer API - EXPECTED_QUERIES = 9 + EXPECTED_QUERIES = 13 def setUp(self): self.pip = Project.objects.get(slug='pip') self.pip.versions.create_latest() + self.pip.versions.update(built=True) def render(self): request = self.factory.get(self.url)