diff --git a/readthedocs/api/v2/utils.py b/readthedocs/api/v2/utils.py index 055e131954d..d7b3cd209c3 100644 --- a/readthedocs/api/v2/utils.py +++ b/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, ) diff --git a/readthedocs/rtd_tests/tests/test_backend.py b/readthedocs/rtd_tests/tests/test_backend.py index 30b144e45d3..af931c5c345 100644 --- a/readthedocs/rtd_tests/tests/test_backend.py +++ b/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') diff --git a/readthedocs/rtd_tests/tests/test_sync_versions.py b/readthedocs/rtd_tests/tests/test_sync_versions.py index 5c56935953d..226d18efea8 100644 --- a/readthedocs/rtd_tests/tests/test_sync_versions.py +++ b/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( diff --git a/readthedocs/vcs_support/backends/git.py b/readthedocs/vcs_support/backends/git.py index eb785729865..a5d27928e66 100644 --- a/readthedocs/vcs_support/backends/git.py +++ b/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