Sync versions: filter by type on update

readthedocs/api/v2/utils.py

@@ -1,8 +1,8 @@
 """Utility functions that are used by both views and celery tasks."""
 
 import itertools
-import structlog
 
+import structlog
 from rest_framework.pagination import PageNumberPagination
 
 from readthedocs.builds.constants import (
@@ -79,9 +79,11 @@ def sync_versions_to_db(project, versions, type):  # pylint: disable=redefined-b
             Version.objects.filter(
                 project=project,
                 verbose_name=version_name,
+                # Always filter by type, a tag and a branch
+                # can share the same verbose_name.
+                type=type,
             ).update(
                 identifier=version_id,
-                type=type,
                 machine=False,
             )
 

readthedocs/rtd_tests/tests/test_backend.py

@@ -75,8 +75,8 @@ def test_git_lsremote(self):
         repo_branches, repo_tags = repo.lsremote
 
         self.assertEqual(
-            set(branches + default_branches),
-            {branch.verbose_name for branch in repo_branches},
+            {branch: branch for branch in default_branches + branches},
+            {branch.verbose_name: branch.identifier for branch in repo_branches},
         )
 
         self.assertEqual(
@@ -111,8 +111,8 @@ def test_git_branches(self, checkout_path):
         repo.clone()
 
         self.assertEqual(
-            set(branches + default_branches),
-            {branch.verbose_name for branch in repo.branches},
+            {branch: branch for branch in default_branches + branches},
+            {branch.verbose_name: branch.identifier for branch in repo.branches},
         )
 
     @patch('readthedocs.projects.models.Project.checkout_path')

readthedocs/rtd_tests/tests/test_sync_versions.py

@@ -1,4 +1,3 @@
-import json
 from unittest import mock
 
 from django.conf import settings
@@ -14,7 +13,6 @@
 )
 from readthedocs.builds.tasks import sync_versions_task
 from readthedocs.organizations.models import Organization, OrganizationOwner
-from readthedocs.projects.constants import PUBLIC
 from readthedocs.projects.models import Project
 
 
@@ -726,6 +724,57 @@ def test_deletes_version_with_same_identifier(self):
             1,
         )
 
+    def test_versions_with_same_verbose_name(self):
+        get(
+            Version,
+            project=self.pip,
+            identifier="v2",
+            verbose_name="v2",
+            active=True,
+            type=BRANCH,
+        )
+        get(
+            Version,
+            project=self.pip,
+            identifier="1234abc",
+            verbose_name="v2",
+            active=True,
+            type=TAG,
+        )
+        branches_data = [
+            {
+                "identifier": "v2",
+                "verbose_name": "v2",
+            },
+        ]
+        tags_data = [
+            {
+                # THe identifier has changed!
+                "identifier": "12345abc",
+                "verbose_name": "v2",
+            },
+        ]
+
+        sync_versions_task(
+            self.pip.pk,
+            branches_data=branches_data,
+            tags_data=tags_data,
+        )
+
+        self.assertEqual(
+            self.pip.versions.filter(
+                verbose_name="v2", identifier="v2", type=BRANCH
+            ).count(),
+            1,
+        )
+        self.assertEqual(
+            self.pip.versions.filter(
+                verbose_name="v2", identifier="12345abc", type=TAG
+            ).count(),
+            1,
+        )
+
+
     @mock.patch('readthedocs.builds.tasks.run_automation_rules')
     def test_automation_rules_are_triggered_for_new_versions(self, run_automation_rules):
         Version.objects.create(

readthedocs/vcs_support/backends/git.py

@@ -287,11 +287,17 @@ def branches(self):
 
         for branch in branches:
             verbose_name = branch.name
-            if verbose_name.startswith('origin/'):
-                verbose_name = verbose_name.replace('origin/', '')
-            if verbose_name == 'HEAD':
+            if verbose_name.startswith("origin/"):
+                verbose_name = verbose_name.replace("origin/", "", 1)
+            if verbose_name == "HEAD":
                 continue
-            versions.append(VCSVersion(self, str(branch), verbose_name))
+            versions.append(
+                VCSVersion(
+                    repository=self,
+                    identifier=verbose_name,
+                    verbose_name=verbose_name,
+                )
+            )
         return versions
 
     @property