diff --git a/readthedocs/restapi/utils.py b/readthedocs/restapi/utils.py index 0c91de81618..8637cd1779b 100644 --- a/readthedocs/restapi/utils.py +++ b/readthedocs/restapi/utils.py @@ -2,16 +2,26 @@ """Utility functions that are used by both views and celery tasks.""" from __future__ import ( - absolute_import, division, print_function, unicode_literals) + absolute_import, + division, + print_function, + unicode_literals, +) import hashlib import logging from rest_framework.pagination import PageNumberPagination -from readthedocs.builds.constants import (LATEST, LATEST_VERBOSE_NAME, - NON_REPOSITORY_VERSIONS, STABLE, - STABLE_VERBOSE_NAME) +from readthedocs.builds.constants import ( + BRANCH, + LATEST, + LATEST_VERBOSE_NAME, + NON_REPOSITORY_VERSIONS, + STABLE, + STABLE_VERBOSE_NAME, + TAG, +) from readthedocs.builds.models import Version from readthedocs.search.indexes import PageIndex, ProjectIndex, SectionIndex @@ -133,15 +143,25 @@ def set_or_create_version(project, slug, version_id, verbose_name, type_): def delete_versions(project, version_data): """Delete all versions not in the current repo.""" - current_versions = [] - if 'tags' in version_data: - for version in version_data['tags']: - current_versions.append(version['identifier']) - if 'branches' in version_data: - for version in version_data['branches']: - current_versions.append(version['identifier']) + # We use verbose_name for tags + # because several tags can point to the same identifier. + versions_tags = [ + version['verbose_name'] + for version in version_data.get('tags', []) + ] + versions_branches = [ + version['identifier'] + for version in version_data.get('branches', []) + ] to_delete_qs = project.versions.all() - to_delete_qs = to_delete_qs.exclude(identifier__in=current_versions) + to_delete_qs = to_delete_qs.exclude( + type=TAG, + verbose_name__in=versions_tags, + ) + to_delete_qs = to_delete_qs.exclude( + type=BRANCH, + identifier__in=versions_branches, + ) to_delete_qs = to_delete_qs.exclude(uploaded=True) to_delete_qs = to_delete_qs.exclude(active=True) to_delete_qs = to_delete_qs.exclude(slug__in=NON_REPOSITORY_VERSIONS) diff --git a/readthedocs/rtd_tests/tests/test_sync_versions.py b/readthedocs/rtd_tests/tests/test_sync_versions.py index 7612ec49425..9b7516015e5 100644 --- a/readthedocs/rtd_tests/tests/test_sync_versions.py +++ b/readthedocs/rtd_tests/tests/test_sync_versions.py @@ -105,12 +105,13 @@ def test_new_tag_update_active(self): self.pip.get_stable_version().identifier, ) - def test_new_tag_update_inactive(self): + def test_new_tag_dont_update_inactive(self): Version.objects.create( project=self.pip, identifier='0.8.3', verbose_name='0.8.3', + type=TAG, active=False, ) @@ -142,13 +143,13 @@ 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') + # Version 0.9 becomes the stable version, but it's inactive + version_9 = self.pip.versions.get(slug='0.9') self.assertEqual( version_9.identifier, self.pip.get_stable_version().identifier, ) - self.assertTrue(version_9.active) + self.assertFalse(version_9.active) # Version 0.8.3 is still inactive version_8 = Version.objects.get(slug='0.8.3') @@ -651,6 +652,97 @@ def test_machine_attr_when_user_define_latest_branch_and_delete_it(self): ) self.assertTrue(version_latest.machine) + def test_deletes_version_with_same_identifier(self): + version_post_data = { + 'branches': [ + { + 'identifier': 'origin/master', + 'verbose_name': 'master', + }, + ], + 'tags': [ + { + 'identifier': '1234', + 'verbose_name': 'one', + }, + ], + } + + resp = self.client.post( + reverse('project-sync-versions', args=[self.pip.pk]), + data=json.dumps(version_post_data), + content_type='application/json', + ) + self.assertEqual(resp.status_code, 200) + + # We only have one version with an identifier `1234` + self.assertEqual( + self.pip.versions.filter(identifier='1234').count(), + 1 + ) + + # We add a new tag with the same identifier + version_post_data = { + 'branches': [ + { + 'identifier': 'origin/master', + 'verbose_name': 'master', + }, + ], + 'tags': [ + { + 'identifier': '1234', + 'verbose_name': 'two', + }, + { + 'identifier': '1234', + 'verbose_name': 'one', + }, + ], + } + + resp = self.client.post( + reverse('project-sync-versions', args=[self.pip.pk]), + data=json.dumps(version_post_data), + content_type='application/json', + ) + self.assertEqual(resp.status_code, 200) + + # We have two versions with an identifier `1234` + self.assertEqual( + self.pip.versions.filter(identifier='1234').count(), + 2 + ) + + # We delete one version with identifier `1234` + version_post_data = { + 'branches': [ + { + 'identifier': 'origin/master', + 'verbose_name': 'master', + }, + ], + 'tags': [ + { + 'identifier': '1234', + 'verbose_name': 'one', + }, + ], + } + + resp = self.client.post( + reverse('project-sync-versions', args=[self.pip.pk]), + data=json.dumps(version_post_data), + content_type='application/json', + ) + self.assertEqual(resp.status_code, 200) + + # We have only one version with an identifier `1234` + self.assertEqual( + self.pip.versions.filter(identifier='1234').count(), + 1 + ) + class TestStableVersion(TestCase): fixtures = ['eric', 'test_data']