Delete tags with same commit

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)

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']