Skip to content

Commit ca79cdb

Browse files
committed
Don't compare inactive or non build versions
This is a better approach for readthedocs#6353
1 parent 2f97d8b commit ca79cdb

File tree

3 files changed

+12
-11
lines changed

3 files changed

+12
-11
lines changed

readthedocs/api/v2/views/footer_views.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,10 @@ def get_version_compare_data(project, base_version=None):
2626
:param base_version: We assert whether or not the base_version is also the
2727
highest version in the resulting "is_highest" value.
2828
"""
29-
versions_qs = Version.internal.public(project=project)
29+
versions_qs = (
30+
Version.internal.public(project=project)
31+
.filter(built=True, active=True)
32+
)
3033

3134
# Take preferences over tags only if the project has at least one tag
3235
if versions_qs.filter(type=TAG).exists():
@@ -46,7 +49,7 @@ def get_version_compare_data(project, base_version=None):
4649
if highest_version_obj:
4750
# Never link to the dashboard,
4851
# users reading the docs may don't have access to the dashboard.
49-
ret_val['url'] = highest_version_obj.get_absolute_url(link_to_dashboard=False)
52+
ret_val['url'] = highest_version_obj.get_absolute_url()
5053
ret_val['slug'] = highest_version_obj.slug
5154
if base_version and base_version.slug != LATEST:
5255
try:

readthedocs/builds/models.py

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -289,13 +289,8 @@ def commit_name(self):
289289
)
290290
return self.identifier
291291

292-
def get_absolute_url(self, link_to_dashboard=True):
293-
"""
294-
Get absolute url to the docs of the version.
295-
296-
:param link_to_dashboard: If `False` we never try to link to the dashboard,
297-
we link to the docs even if they result in a 404.
298-
"""
292+
def get_absolute_url(self):
293+
"""Get absolute url to the docs of the version."""
299294
# Hack external versions for now.
300295
# TODO: We can integrate them into the resolver
301296
# but this is much simpler to handle since we only link them a couple places for now
@@ -305,7 +300,7 @@ def get_absolute_url(self, link_to_dashboard=True):
305300
f'{self.project.slug}/{self.slug}/index.html'
306301
return url
307302

308-
if not self.built and not self.uploaded and link_to_dashboard:
303+
if not self.built and not self.uploaded:
309304
return reverse(
310305
'project_version_detail',
311306
kwargs={

readthedocs/rtd_tests/tests/test_footer.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ class TestVersionCompareFooter(TestCase):
124124

125125
def setUp(self):
126126
self.pip = Project.objects.get(slug='pip')
127+
self.pip.versions.update(built=True)
127128

128129
def test_highest_version_from_stable(self):
129130
base_version = self.pip.get_stable_version()
@@ -150,7 +151,7 @@ def test_highest_version_from_lower(self):
150151
self.assertDictEqual(valid_data, returned_data)
151152

152153
def test_highest_version_from_latest(self):
153-
Version.objects.create_latest(project=self.pip)
154+
Version.objects.create_latest(project=self.pip, built=True)
154155
base_version = self.pip.versions.get(slug=LATEST)
155156
valid_data = {
156157
'project': 'Version 0.8.1 of Pip (19)',
@@ -177,6 +178,7 @@ def test_highest_version_over_branches(self):
177178
identifier='1.0.0',
178179
type=TAG,
179180
active=True,
181+
built=True,
180182
)
181183

182184
base_version = self.pip.versions.get(slug='0.8.1')
@@ -221,6 +223,7 @@ def test_highest_version_without_tags(self):
221223
identifier='2.0.0',
222224
type=BRANCH,
223225
active=True,
226+
built=True,
224227
)
225228
valid_data = {
226229
'project': 'Version 2.0.0 of Pip ({})'.format(version.pk),

0 commit comments

Comments
 (0)