From 4a14c5647072a5e306ab17fd6f63b2df5a8e9200 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 28 Nov 2017 16:47:06 -0500 Subject: [PATCH 1/9] Fix isort settings --- .isort.cfg | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.isort.cfg b/.isort.cfg index fc9d00977de..d619e783262 100644 --- a/.isort.cfg +++ b/.isort.cfg @@ -2,8 +2,8 @@ line_length=80 indent=' ' multi_line_output=4 -default_section=FIRSTPARTY -known_firstparty=readthedocs,readthedocsinc -known_third_party=celery,stripe,requests,pytz,builtins,django,annoying,readthedocs_build +default_section=THIRDPARTY +known_first_party=readthedocs,readthedocsinc +known_third_party=mock sections=FUTURE,STDLIB,THIRDPARTY,FIRSTPARTY,LOCALFOLDER add_imports=from __future__ import division, from __future__ import print_function, from __future__ import unicode_literals From 5adbdd1fae909b4ef66adea66f6dc7ee0d839fbf Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 28 Nov 2017 19:45:18 -0500 Subject: [PATCH 2/9] Improve docstrings and style --- .flake8 | 2 +- readthedocs/projects/version_handling.py | 109 ++++++++++++++++------- 2 files changed, 80 insertions(+), 31 deletions(-) diff --git a/.flake8 b/.flake8 index a229cfef0ad..79a57b86ca8 100644 --- a/.flake8 +++ b/.flake8 @@ -1,3 +1,3 @@ [flake8] -ignore = E125,D100,D101,D102,D105,D107,D200,D211,P101,FI15,FI16,FI12,FI11,FI17,FI50,FI53,FI54 +ignore = E125,D100,D101,D102,D105,D107,D200,D211,P101,FI15,FI16,FI12,FI11,FI17,FI50,FI53,FI54,T000,MQ101 max-line-length = 80 diff --git a/readthedocs/projects/version_handling.py b/readthedocs/projects/version_handling.py index 77984b74ddd..9a30628edc5 100644 --- a/readthedocs/projects/version_handling.py +++ b/readthedocs/projects/version_handling.py @@ -1,24 +1,37 @@ -"""Project version handling""" -from __future__ import absolute_import +# -*- coding: utf-8 -*- +"""Project version handling.""" +from __future__ import ( + absolute_import, division, print_function, unicode_literals) import unicodedata +from builtins import object, range from collections import defaultdict -from builtins import (object, range) -from packaging.version import Version -from packaging.version import InvalidVersion import six +from packaging.version import InvalidVersion, Version -from readthedocs.builds.constants import LATEST_VERBOSE_NAME -from readthedocs.builds.constants import STABLE_VERBOSE_NAME +from readthedocs.builds.constants import ( + LATEST_VERBOSE_NAME, STABLE_VERBOSE_NAME) def get_major(version): + """ + Return the major version. + + :param version: version to get the major + :type version: packaging.version.Version + """ # pylint: disable=protected-access return version._version.release[0] def get_minor(version): + """ + Return the minor version. + + :param version: version to get the minor + :type version: packaging.version.Version + """ # pylint: disable=protected-access try: return version._version.release[1] @@ -28,7 +41,7 @@ def get_minor(version): class VersionManager(object): - """Prune list of versions based on version windows""" + """Prune list of versions based on version windows.""" def __init__(self): self._state = defaultdict(lambda: defaultdict(list)) @@ -72,13 +85,13 @@ def get_version_list(self): versions.extend(version_list) versions = sorted(versions) return [ - version.public - for version in versions - if not version.is_prerelease] + version.public for version in versions if not version.is_prerelease + ] def version_windows(versions, major=1, minor=1, point=1): - """Return list of versions that have been pruned to version windows + """ + Return list of versions that have been pruned to version windows. Uses :py:class:`VersionManager` to prune the list of versions @@ -111,6 +124,18 @@ def version_windows(versions, major=1, minor=1, point=1): def parse_version_failsafe(version_string): + """ + Parse a version in string form and return Version object. + + If there is an error parsing the string, ``None`` is returned. + + :param version_string: version as string object (e.g. '3.10.1') + :type version_string: str or unicode + + :returns: version object created from a string object + + :rtype: packaging.version.Version + """ if not isinstance(version_string, six.text_type): uni_version = version_string.decode('utf-8') else: @@ -126,11 +151,19 @@ def parse_version_failsafe(version_string): def comparable_version(version_string): - """This can be used as ``key`` argument to ``sorted``. + """ + Can be used as ``key`` argument to ``sorted``. The ``LATEST`` version shall always beat other versions in comparison. ``STABLE`` should be listed second. If we cannot figure out the version number then we sort it to the bottom of the list. + + :param version_string: version as string object (e.g. '3.10.1' or 'latest') + :type version_string: str or unicode + + :returns: a comparable version object (e.g. 'latest' -> Version('99999.0')) + + :rtype: packaging.version.Version """ comparable = parse_version_failsafe(version_string) if not comparable: @@ -144,12 +177,16 @@ def comparable_version(version_string): def sort_versions(version_list): - """Takes a list of ``Version`` models and return a sorted list, + """ + Take a list of Version models and return a sorted list. - The returned value is a list of two-tuples. The first is the actual - ``Version`` model instance, the second is an instance of - ``packaging.version.Version``. They are ordered in descending order (latest - version first). + :param version_list: list of Version models + :type version_list: list(readthedocs.builds.models.Version) + + :returns: sorted list in descending order (latest version first) of versions + + :rtype: list(tupe(readthedocs.builds.models.Version, + packaging.version.Version)) """ versions = [] for version_obj in version_list: @@ -158,13 +195,20 @@ def sort_versions(version_list): if comparable_version: versions.append((version_obj, comparable_version)) - return list(sorted( - versions, - key=lambda version_info: version_info[1], - reverse=True)) + return list( + sorted( + versions, + key=lambda version_info: version_info[1], + reverse=True, + )) def highest_version(version_list): + """ + Return the highest version for a given ``version_list``. + + :rtype: tupe(readthedocs.builds.models.Version, packaging.version.Version) + """ versions = sort_versions(version_list) if versions: return versions[0] @@ -172,17 +216,22 @@ def highest_version(version_list): def determine_stable_version(version_list): - """Determine a stable version for version list + """ + Determine a stable version for version list. - Takes a list of ``Version`` model instances and returns the version - instance which can be considered the most recent stable one. It will return - ``None`` if there is no stable version in the list. + :param version_list: list of versions + :type version_list: list(readthedocs.builds.models.Version) + + :returns: version considered the most recent stable one or ``None`` if there + is no stable version in the list + + :rtype: readthedocs.builds.models.Version """ versions = sort_versions(version_list) - versions = [ - (version_obj, comparable) - for version_obj, comparable in versions - if not comparable.is_prerelease] + versions = [(version_obj, comparable) + for version_obj, comparable in versions + if not comparable.is_prerelease] + if versions: version_obj, comparable = versions[0] return version_obj From f9c567e9c81ce48b9cb81aafd512b1ec2aa447d6 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 29 Nov 2017 10:29:28 -0500 Subject: [PATCH 3/9] Style rules applied to test_sync_versions --- .../rtd_tests/tests/test_sync_versions.py | 117 +++++++++++------- 1 file changed, 71 insertions(+), 46 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_sync_versions.py b/readthedocs/rtd_tests/tests/test_sync_versions.py index 317b574bf79..7f3c7811bee 100644 --- a/readthedocs/rtd_tests/tests/test_sync_versions.py +++ b/readthedocs/rtd_tests/tests/test_sync_versions.py @@ -1,26 +1,36 @@ # -*- coding: utf-8 -*- -from __future__ import absolute_import +from __future__ import ( + absolute_import, division, print_function, unicode_literals) + import json from django.test import TestCase -from readthedocs.builds.models import Version from readthedocs.builds.constants import STABLE +from readthedocs.builds.models import Version from readthedocs.projects.models import Project class TestSyncVersions(TestCase): - fixtures = ["eric", "test_data"] + fixtures = ['eric', 'test_data'] def setUp(self): self.client.login(username='eric', password='test') self.pip = Project.objects.get(slug='pip') - Version.objects.create(project=self.pip, identifier='origin/master', - verbose_name='master', active=True, - machine=True) - Version.objects.create(project=self.pip, identifier='to_delete', - verbose_name='to_delete', active=False) + Version.objects.create( + project=self.pip, + identifier='origin/master', + verbose_name='master', + active=True, + machine=True, + ) + Version.objects.create( + project=self.pip, + identifier='to_delete', + verbose_name='to_delete', + active=False, + ) def test_proper_url_no_slash(self): version_post_data = { @@ -33,10 +43,11 @@ def test_proper_url_no_slash(self): 'identifier': 'origin/to_add', 'verbose_name': 'to_add', }, - ]} + ], + } r = self.client.post( - '/api/v2/project/%s/sync_versions/' % self.pip.pk, + '/api/v2/project/{}/sync_versions/'.format(self.pip.pk), data=json.dumps(version_post_data), content_type='application/json', ) @@ -46,8 +57,12 @@ def test_proper_url_no_slash(self): def test_new_tag_update_active(self): - Version.objects.create(project=self.pip, identifier='0.8.3', - verbose_name='0.8.3', active=True) + Version.objects.create( + project=self.pip, + identifier='0.8.3', + verbose_name='0.8.3', + active=True, + ) version_post_data = { 'branches': [ @@ -69,12 +84,11 @@ def test_new_tag_update_active(self): 'identifier': '0.8.3', 'verbose_name': '0.8.3', }, - - ] + ], } self.client.post( - '/api/v2/project/%s/sync_versions/' % self.pip.pk, + '/api/v2/project/{}/sync_versions/'.format(self.pip.pk), data=json.dumps(version_post_data), content_type='application/json', ) @@ -83,8 +97,12 @@ def test_new_tag_update_active(self): def test_new_tag_update_inactive(self): - Version.objects.create(project=self.pip, identifier='0.8.3', - verbose_name='0.8.3', active=False) + Version.objects.create( + project=self.pip, + identifier='0.8.3', + verbose_name='0.8.3', + active=False, + ) version_post_data = { 'branches': [ @@ -106,12 +124,11 @@ def test_new_tag_update_inactive(self): 'identifier': '0.8.3', 'verbose_name': '0.8.3', }, - - ] + ], } self.client.post( - '/api/v2/project/%s/sync_versions/' % self.pip.pk, + '/api/v2/project/{}/sync_versions/'.format(self.pip.pk), data=json.dumps(version_post_data), content_type='application/json', ) @@ -120,7 +137,7 @@ def test_new_tag_update_inactive(self): class TestStableVersion(TestCase): - fixtures = ["eric", "test_data"] + fixtures = ['eric', 'test_data'] def setUp(self): self.client.login(username='eric', password='test') @@ -147,17 +164,16 @@ def test_stable_versions(self): 'identifier': '0.8', 'verbose_name': '0.8', }, - - ] + ], } self.assertRaises( Version.DoesNotExist, Version.objects.get, - slug=STABLE + slug=STABLE, ) self.client.post( - '/api/v2/project/%s/sync_versions/' % self.pip.pk, + '/api/v2/project/{}/sync_versions/'.format(self.pip.pk), data=json.dumps(version_post_data), content_type='application/json', ) @@ -174,11 +190,11 @@ def test_pre_release_are_not_stable(self): {'identifier': '0.9b1', 'verbose_name': '0.9b1'}, {'identifier': '0.8', 'verbose_name': '0.8'}, {'identifier': '0.8rc2', 'verbose_name': '0.8rc2'}, - ] + ], } self.client.post( - '/api/v2/project/%s/sync_versions/' % self.pip.pk, + '/api/v2/project/{}/sync_versions/'.format(self.pip.pk), data=json.dumps(version_post_data), content_type='application/json', ) @@ -192,11 +208,11 @@ def test_post_releases_are_stable(self): 'tags': [ {'identifier': '1.0', 'verbose_name': '1.0'}, {'identifier': '1.0.post1', 'verbose_name': '1.0.post1'}, - ] + ], } self.client.post( - '/api/v2/project/%s/sync_versions/' % self.pip.pk, + '/api/v2/project/{}/sync_versions/'.format(self.pip.pk), data=json.dumps(version_post_data), content_type='application/json', ) @@ -210,12 +226,15 @@ def test_invalid_version_numbers_are_not_stable(self): version_post_data = { 'branches': [], 'tags': [ - {'identifier': 'this.is.invalid', 'verbose_name': 'this.is.invalid'}, - ] + { + 'identifier': 'this.is.invalid', + 'verbose_name': 'this.is.invalid' + }, + ], } self.client.post( - '/api/v2/project/%s/sync_versions/' % self.pip.pk, + '/api/v2/project/{}/sync_versions/'.format(self.pip.pk), data=json.dumps(version_post_data), content_type='application/json', ) @@ -224,13 +243,19 @@ def test_invalid_version_numbers_are_not_stable(self): version_post_data = { 'branches': [], 'tags': [ - {'identifier': '1.0', 'verbose_name': '1.0'}, - {'identifier': 'this.is.invalid', 'verbose_name': 'this.is.invalid'}, - ] + { + 'identifier': '1.0', + 'verbose_name': '1.0', + }, + { + 'identifier': 'this.is.invalid', + 'verbose_name': 'this.is.invalid' + }, + ], } self.client.post( - '/api/v2/project/%s/sync_versions/' % self.pip.pk, + '/api/v2/project/{}/sync_versions/'.format(self.pip.pk), data=json.dumps(version_post_data), content_type='application/json', ) @@ -255,11 +280,11 @@ def test_update_stable_version(self): 'identifier': '0.8', 'verbose_name': '0.8', }, - ] + ], } self.client.post( - '/api/v2/project/%s/sync_versions/' % self.pip.pk, + '/api/v2/project/{}/sync_versions/'.format(self.pip.pk), data=json.dumps(version_post_data), content_type='application/json', ) @@ -278,7 +303,7 @@ def test_update_stable_version(self): } self.client.post( - '/api/v2/project/%s/sync_versions/' % self.pip.pk, + '/api/v2/project/{}/sync_versions/'.format(self.pip.pk), data=json.dumps(version_post_data), content_type='application/json', ) @@ -293,11 +318,11 @@ def test_update_stable_version(self): 'identifier': '0.7', 'verbose_name': '0.7', }, - ] + ], } self.client.post( - '/api/v2/project/%s/sync_versions/' % self.pip.pk, + '/api/v2/project/{}/sync_versions/'.format(self.pip.pk), data=json.dumps(version_post_data), content_type='application/json', ) @@ -319,11 +344,11 @@ def test_update_inactive_stable_version(self): 'identifier': '0.9', 'verbose_name': '0.9', }, - ] + ], } self.client.post( - '/api/v2/project/%s/sync_versions/' % self.pip.pk, + '/api/v2/project/{}/sync_versions/'.format(self.pip.pk), data=json.dumps(version_post_data), content_type='application/json', ) @@ -339,7 +364,7 @@ def test_update_inactive_stable_version(self): }) self.client.post( - '/api/v2/project/%s/sync_versions/' % self.pip.pk, + '/api/v2/project/{}/sync_versions/'.format(self.pip.pk), data=json.dumps(version_post_data), content_type='application/json', ) @@ -353,11 +378,11 @@ def test_unicode(self): 'branches': [], 'tags': [ {'identifier': 'foo-£', 'verbose_name': 'foo-£'}, - ] + ], } resp = self.client.post( - '/api/v2/project/%s/sync_versions/' % self.pip.pk, + '/api/v2/project/{}/sync_versions/'.format(self.pip.pk), data=json.dumps(version_post_data), content_type='application/json', ) From 682a7fa7080ea68766d424acda526d468486ba42 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 29 Nov 2017 10:30:18 -0500 Subject: [PATCH 4/9] Add pdbpp for local debugging --- requirements/testing.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/requirements/testing.txt b/requirements/testing.txt index 529de546728..c406f42daf2 100644 --- a/requirements/testing.txt +++ b/requirements/testing.txt @@ -5,3 +5,6 @@ pytest-django==3.1.2 pytest-xdist==1.20.1 apipkg==1.4 execnet==1.5.0 + +# local debugging tools +pdbpp From fd0b4062176854a0ac18266be6a92d3113011886 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 29 Nov 2017 10:33:16 -0500 Subject: [PATCH 5/9] Take preference of tags over branches to get stable version --- readthedocs/projects/version_handling.py | 8 +++++++- .../rtd_tests/tests/test_sync_versions.py | 17 ++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/readthedocs/projects/version_handling.py b/readthedocs/projects/version_handling.py index 9a30628edc5..897bfa1ad97 100644 --- a/readthedocs/projects/version_handling.py +++ b/readthedocs/projects/version_handling.py @@ -11,7 +11,7 @@ from packaging.version import InvalidVersion, Version from readthedocs.builds.constants import ( - LATEST_VERBOSE_NAME, STABLE_VERBOSE_NAME) + LATEST_VERBOSE_NAME, STABLE_VERBOSE_NAME, TAG) def get_major(version): @@ -233,6 +233,12 @@ def determine_stable_version(version_list): if not comparable.is_prerelease] if versions: + # We take preference for tags over branches. If we don't find any tag, + # we just return the first branch found. + for version_obj, comparable in versions: + if version_obj.type == TAG: + return version_obj + version_obj, comparable = versions[0] return version_obj return None diff --git a/readthedocs/rtd_tests/tests/test_sync_versions.py b/readthedocs/rtd_tests/tests/test_sync_versions.py index 7f3c7811bee..ce429a20489 100644 --- a/readthedocs/rtd_tests/tests/test_sync_versions.py +++ b/readthedocs/rtd_tests/tests/test_sync_versions.py @@ -95,6 +95,12 @@ def test_new_tag_update_active(self): version_9 = Version.objects.get(slug='0.9') self.assertTrue(version_9.active) + # Version 0.9 becomes the stable version + self.assertEqual( + version_9.identifier, + self.pip.get_stable_version().identifier, + ) + def test_new_tag_update_inactive(self): Version.objects.create( @@ -132,8 +138,17 @@ def test_new_tag_update_inactive(self): data=json.dumps(version_post_data), content_type='application/json', ) + # Version 0.9 becomes the stable version and active version_9 = Version.objects.get(slug='0.9') - self.assertTrue(version_9.active is False) + self.assertEqual( + version_9.identifier, + self.pip.get_stable_version().identifier, + ) + self.assertTrue(version_9.active) + + # Version 0.8.3 is still inactive + version_8 = Version.objects.get(slug='0.8.3') + self.assertFalse(version_8.active) class TestStableVersion(TestCase): From 073b0c07f057da83c41f8bdb7f3052860f54865f Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 29 Nov 2017 12:33:21 -0500 Subject: [PATCH 6/9] Do not update version type when sync versions This allows us to have the same `verbose_name` for different type of versions (for example, '2.0' could both a tag and a branch). If this happen, we want to keep them both. In the UI, it will say just `"2.0"` for the branch and `"2.0 (c0d25453)"` for the tag. This could be improved later if necessary. --- readthedocs/restapi/utils.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/readthedocs/restapi/utils.py b/readthedocs/restapi/utils.py index 41bae9b4236..4f655f8ffdc 100644 --- a/readthedocs/restapi/utils.py +++ b/readthedocs/restapi/utils.py @@ -13,12 +13,9 @@ def sync_versions(project, versions, type): # pylint: disable=redefined-builtin """Update the database with the current versions from the repository.""" - # Bookkeeping for keeping tag/branch identifies correct - verbose_names = [v['verbose_name'] for v in versions] - project.versions.filter(verbose_name__in=verbose_names).update(type=type) - old_versions = {} - old_version_values = project.versions.values('identifier', 'verbose_name') + old_version_values = project.versions.filter(type=type).values( + 'identifier', 'verbose_name') for version in old_version_values: old_versions[version['verbose_name']] = version['identifier'] From 1d76453f3aad8dd8b09c565c233d88694a041a52 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 29 Nov 2017 12:34:01 -0500 Subject: [PATCH 7/9] Test for tags over branches versions --- .../rtd_tests/tests/test_sync_versions.py | 68 ++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/readthedocs/rtd_tests/tests/test_sync_versions.py b/readthedocs/rtd_tests/tests/test_sync_versions.py index ce429a20489..79d6caf472f 100644 --- a/readthedocs/rtd_tests/tests/test_sync_versions.py +++ b/readthedocs/rtd_tests/tests/test_sync_versions.py @@ -7,7 +7,7 @@ from django.test import TestCase -from readthedocs.builds.constants import STABLE +from readthedocs.builds.constants import BRANCH, STABLE, TAG from readthedocs.builds.models import Version from readthedocs.projects.models import Project @@ -24,12 +24,14 @@ def setUp(self): verbose_name='master', active=True, machine=True, + type=BRANCH, ) Version.objects.create( project=self.pip, identifier='to_delete', verbose_name='to_delete', active=False, + type=TAG, ) def test_proper_url_no_slash(self): @@ -388,6 +390,70 @@ def test_update_inactive_stable_version(self): self.assertFalse(version_stable.active) self.assertEqual(version_stable.identifier, '1.0.0') + def test_stable_version_tags_over_branches(self): + version_post_data = { + 'branches': [ + # 2.0 development + {'identifier': 'origin/2.0', 'verbose_name': '2.0'}, + {'identifier': 'origin/0.9.1rc1', 'verbose_name': '0.9.1rc1'}, + ], + 'tags': [ + {'identifier': '1.0rc1', 'verbose_name': '1.0rc1'}, + {'identifier': '0.9', 'verbose_name': '0.9'}, + ], + } + + self.client.post( + '/api/v2/project/{}/sync_versions/'.format(self.pip.pk), + data=json.dumps(version_post_data), + content_type='application/json', + ) + + # If there is a branch with a higher version, tags takes preferences + # over the branches to select the stable version + version_stable = Version.objects.get(slug=STABLE) + self.assertTrue(version_stable.active) + self.assertEqual(version_stable.identifier, '0.9') + + version_post_data['tags'].append({ + 'identifier': '1.0', + 'verbose_name': '1.0', + }) + + self.client.post( + '/api/v2/project/{}/sync_versions/'.format(self.pip.pk), + data=json.dumps(version_post_data), + content_type='application/json', + ) + + version_stable = Version.objects.get(slug=STABLE) + self.assertTrue(version_stable.active) + self.assertEqual(version_stable.identifier, '1.0') + + def test_stable_version_same_id_tag_branch(self): + version_post_data = { + 'branches': [ + # old 1.0 development branch + {'identifier': 'origin/1.0', 'verbose_name': '1.0'}, + ], + 'tags': [ + # tagged 1.0 final version + {'identifier': '1.0', 'verbose_name': '1.0'}, + {'identifier': '0.9', 'verbose_name': '0.9'}, + ], + } + + self.client.post( + '/api/v2/project/{}/sync_versions/'.format(self.pip.pk), + data=json.dumps(version_post_data), + content_type='application/json', + ) + + version_stable = Version.objects.get(slug=STABLE) + self.assertTrue(version_stable.active) + self.assertEqual(version_stable.identifier, '1.0') + self.assertEqual(version_stable.type, 'tag') + def test_unicode(self): version_post_data = { 'branches': [], From bcfbca88291c1035322de0ac756100100a4f9e7c Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 29 Nov 2017 16:25:24 -0500 Subject: [PATCH 8/9] 'most up to date version' note prefers tags Now, this logic is like this: - if the project has any Version that is a TAG, the highest version will only be a TAG also (BRANCH are not considered) - if the project doesn't have any TAG the highest version will be a BRANCH (or UNKNOWN in the worst case) --- readthedocs/restapi/views/footer_views.py | 9 +- readthedocs/rtd_tests/tests/test_footer.py | 119 ++++++++++++++++++++- 2 files changed, 125 insertions(+), 3 deletions(-) diff --git a/readthedocs/restapi/views/footer_views.py b/readthedocs/restapi/views/footer_views.py index f805ca62d75..e08e28429a5 100644 --- a/readthedocs/restapi/views/footer_views.py +++ b/readthedocs/restapi/views/footer_views.py @@ -28,8 +28,13 @@ def get_version_compare_data(project, base_version=None): :param base_version: We assert whether or not the base_version is also the highest version in the resulting "is_highest" value. """ - highest_version_obj, highest_version_comparable = highest_version( - project.versions.public().filter(active=True)) + versions_qs = project.versions.public().filter(active=True) + + # Take preferences over tags only if the project has at least one tag + if versions_qs.filter(type=TAG).exists(): + versions_qs = versions_qs.filter(type=TAG) + + highest_version_obj, highest_version_comparable = highest_version(versions_qs) ret_val = { 'project': six.text_type(highest_version_obj), 'version': six.text_type(highest_version_comparable), diff --git a/readthedocs/rtd_tests/tests/test_footer.py b/readthedocs/rtd_tests/tests/test_footer.py index 8a7e99e750b..cb27320fd22 100644 --- a/readthedocs/rtd_tests/tests/test_footer.py +++ b/readthedocs/rtd_tests/tests/test_footer.py @@ -1,12 +1,15 @@ from __future__ import absolute_import import mock +from django.test import TestCase from rest_framework.test import APIRequestFactory, APITestCase from readthedocs.core.middleware import FooterNoSessionMiddleware from readthedocs.rtd_tests.mocks.paths import fake_paths_by_regex +from readthedocs.builds.constants import TAG, BRANCH, LATEST +from readthedocs.builds.models import Version from readthedocs.projects.models import Project -from readthedocs.restapi.views.footer_views import footer_html +from readthedocs.restapi.views.footer_views import footer_html, get_version_compare_data class Testmaker(APITestCase): @@ -85,3 +88,117 @@ def test_no_session_logged_out(self): home_request = self.factory.get('/') mid.process_request(home_request) self.assertEqual(home_request.session.TEST_COOKIE_NAME, 'testcookie') + + +class TestVersionCompareFooter(TestCase): + fixtures = ['test_data'] + + def setUp(self): + self.pip = Project.objects.get(slug='pip') + + 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/', + 'slug': ('0.8.1',), + 'version': '0.8.1', + 'is_highest': True, + } + returned_data = get_version_compare_data(self.pip, base_version) + self.assertDictEqual(valid_data, returned_data) + + 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/', + 'slug': ('0.8.1',), + 'version': '0.8.1', + 'is_highest': False, + } + returned_data = get_version_compare_data(self.pip, base_version) + self.assertDictEqual(valid_data, returned_data) + + def test_highest_version_from_latest(self): + Version.objects.create_latest(project=self.pip) + 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/', + 'slug': ('0.8.1',), + 'version': '0.8.1', + 'is_highest': True, + } + returned_data = get_version_compare_data(self.pip, base_version) + self.assertDictEqual(valid_data, returned_data) + + def test_highest_version_over_branches(self): + Version.objects.create( + project=self.pip, + verbose_name='2.0.0', + identifier='2.0.0', + type=BRANCH, + active=True, + ) + + version = Version.objects.create( + project=self.pip, + verbose_name='1.0.0', + identifier='1.0.0', + type=TAG, + active=True, + ) + + 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/', + 'slug': ('1.0.0',), + 'version': '1.0.0', + 'is_highest': False, + } + returned_data = get_version_compare_data(self.pip, base_version) + self.assertDictEqual(valid_data, returned_data) + + def test_highest_version_without_tags(self): + self.pip.versions.filter(type=TAG).update(type=BRANCH) + + 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/', + 'slug': ('0.8.1',), + 'version': '0.8.1', + 'is_highest': True, + } + returned_data = get_version_compare_data(self.pip, base_version) + self.assertDictEqual(valid_data, returned_data) + + 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/', + 'slug': ('0.8.1',), + 'version': '0.8.1', + 'is_highest': False, + } + returned_data = get_version_compare_data(self.pip, base_version) + self.assertDictEqual(valid_data, returned_data) + + version = Version.objects.create( + project=self.pip, + verbose_name='2.0.0', + identifier='2.0.0', + type=BRANCH, + active=True, + ) + valid_data = { + 'project': 'Version 2.0.0 of Pip ({})'.format(version.pk), + 'url': '/dashboard/pip/version/2.0.0/', + 'slug': ('2.0.0',), + 'version': '2.0.0', + 'is_highest': False, + } + returned_data = get_version_compare_data(self.pip, base_version) + self.assertDictEqual(valid_data, returned_data) From 20ce7aa5c590ca37f76a8920f2818f6f80313dab Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 29 Nov 2017 16:38:40 -0500 Subject: [PATCH 9/9] More styling --- readthedocs/restapi/views/footer_views.py | 86 +++++++++++++--------- readthedocs/rtd_tests/tests/test_footer.py | 19 +++-- 2 files changed, 61 insertions(+), 44 deletions(-) diff --git a/readthedocs/restapi/views/footer_views.py b/readthedocs/restapi/views/footer_views.py index e08e28429a5..295d0486155 100644 --- a/readthedocs/restapi/views/footer_views.py +++ b/readthedocs/restapi/views/footer_views.py @@ -1,29 +1,29 @@ +# -*- coding: utf-8 -*- """Endpoint to generate footer HTML.""" -from __future__ import absolute_import +from __future__ import ( + absolute_import, division, print_function, unicode_literals) -from django.shortcuts import get_object_or_404 -from django.template import RequestContext, loader as template_loader +import six from django.conf import settings - - +from django.shortcuts import get_object_or_404 +from django.template import loader as template_loader from rest_framework import decorators, permissions from rest_framework.renderers import JSONRenderer from rest_framework.response import Response from rest_framework_jsonp.renderers import JSONPRenderer -from readthedocs.builds.constants import LATEST -from readthedocs.builds.constants import TAG +from readthedocs.builds.constants import LATEST, TAG from readthedocs.builds.models import Version from readthedocs.projects.models import Project -from readthedocs.projects.version_handling import highest_version -from readthedocs.projects.version_handling import parse_version_failsafe +from readthedocs.projects.version_handling import ( + highest_version, parse_version_failsafe) from readthedocs.restapi.signals import footer_response -import six def get_version_compare_data(project, base_version=None): - """Retrieve metadata about the highest version available for this project. + """ + Retrieve metadata about the highest version available for this project. :param base_version: We assert whether or not the base_version is also the highest version in the resulting "is_highest" value. @@ -34,7 +34,8 @@ def get_version_compare_data(project, base_version=None): if versions_qs.filter(type=TAG).exists(): versions_qs = versions_qs.filter(type=TAG) - highest_version_obj, highest_version_comparable = highest_version(versions_qs) + highest_version_obj, highest_version_comparable = highest_version( + versions_qs) ret_val = { 'project': six.text_type(highest_version_obj), 'version': six.text_type(highest_version_comparable), @@ -74,32 +75,30 @@ def footer_html(request): subproject = request.GET.get('subproject', False) source_suffix = request.GET.get('source_suffix', '.rst') - new_theme = (theme == "sphinx_rtd_theme") - using_theme = (theme == "default") + new_theme = (theme == 'sphinx_rtd_theme') + using_theme = (theme == 'default') project = get_object_or_404(Project, slug=project_slug) version = get_object_or_404( - Version.objects.public(request.user, project=project, only_active=False), + Version.objects.public( + request.user, project=project, only_active=False), slug=version_slug) main_project = project.main_language_project or project - if page_slug and page_slug != "index": - if ( - main_project.documentation_type == "sphinx_htmldir" or - main_project.documentation_type == "mkdocs"): - path = page_slug + "/" - elif main_project.documentation_type == "sphinx_singlehtml": - path = "index.html#document-" + page_slug + if page_slug and page_slug != 'index': + if (main_project.documentation_type == 'sphinx_htmldir' or + main_project.documentation_type == 'mkdocs'): + path = page_slug + '/' + elif main_project.documentation_type == 'sphinx_singlehtml': + path = 'index.html#document-' + page_slug else: - path = page_slug + ".html" + path = page_slug + '.html' else: - path = "" + path = '' if version.type == TAG and version.project.has_pdf(version.slug): print_url = ( - 'https://keminglabs.com/print-the-docs/quote?project={project}&version={version}' - .format( - project=project.slug, - version=version.slug)) + 'https://keminglabs.com/print-the-docs/quote?project={project}&version={version}' # noqa + .format(project=project.slug, version=version.slug)) else: print_url = None @@ -120,14 +119,28 @@ def footer_html(request): 'settings': settings, 'subproject': subproject, 'print_url': print_url, - 'github_edit_url': version.get_github_url(docroot, page_slug, source_suffix, 'edit'), - 'github_view_url': version.get_github_url(docroot, page_slug, source_suffix, 'view'), - 'bitbucket_url': version.get_bitbucket_url(docroot, page_slug, source_suffix), + 'github_edit_url': version.get_github_url( + docroot, + page_slug, + source_suffix, + 'edit', + ), + 'github_view_url': version.get_github_url( + docroot, + page_slug, + source_suffix, + 'view', + ), + 'bitbucket_url': version.get_bitbucket_url( + docroot, + page_slug, + source_suffix, + ), 'theme': theme, } - html = template_loader.get_template('restapi/footer.html').render(context, - request) + html = template_loader.get_template('restapi/footer.html').render( + context, request) resp_data = { 'html': html, 'version_active': version.active, @@ -135,8 +148,9 @@ def footer_html(request): 'version_supported': version.supported, } - # Allow folks to hook onto the footer response for various information collection, - # or to modify the resp_data. - footer_response.send(sender=None, request=request, context=context, resp_data=resp_data) + # Allow folks to hook onto the footer response for various information + # collection, or to modify the resp_data. + footer_response.send( + sender=None, request=request, context=context, resp_data=resp_data) return Response(resp_data) diff --git a/readthedocs/rtd_tests/tests/test_footer.py b/readthedocs/rtd_tests/tests/test_footer.py index cb27320fd22..bbea7c58a24 100644 --- a/readthedocs/rtd_tests/tests/test_footer.py +++ b/readthedocs/rtd_tests/tests/test_footer.py @@ -1,15 +1,18 @@ -from __future__ import absolute_import -import mock +# -*- coding: utf-8 -*- +from __future__ import ( + absolute_import, division, print_function, unicode_literals) +import mock from django.test import TestCase from rest_framework.test import APIRequestFactory, APITestCase -from readthedocs.core.middleware import FooterNoSessionMiddleware -from readthedocs.rtd_tests.mocks.paths import fake_paths_by_regex -from readthedocs.builds.constants import TAG, BRANCH, LATEST +from readthedocs.builds.constants import BRANCH, LATEST, TAG from readthedocs.builds.models import Version +from readthedocs.core.middleware import FooterNoSessionMiddleware from readthedocs.projects.models import Project -from readthedocs.restapi.views.footer_views import footer_html, get_version_compare_data +from readthedocs.restapi.views.footer_views import ( + footer_html, get_version_compare_data) +from readthedocs.rtd_tests.mocks.paths import fake_paths_by_regex class Testmaker(APITestCase): @@ -43,10 +46,10 @@ def test_footer(self): self.assertEqual(r.status_code, 200) def test_footer_uses_version_compare(self): - version_compare = 'readthedocs.restapi.views.footer_views.get_version_compare_data' + version_compare = 'readthedocs.restapi.views.footer_views.get_version_compare_data' # noqa with mock.patch(version_compare) as get_version_compare_data: get_version_compare_data.return_value = { - 'MOCKED': True + 'MOCKED': True, } r = self.render() self.assertEqual(r.status_code, 200)