Builds: don't delete them when a version is deleted

stsewd · stsewd · commit 664a27a47231 · 2020-11-18T17:11:53.000-05:00
We lose information when a version is deleted. Setting the version to null allow us to keep the builds around. But now we need to save more data to be able to reproduce some links. A data migration is needed to update old builds, but isn't required, as we fallback to the value from the version. Close #7674
diff --git a/readthedocs/api/v2/serializers.py b/readthedocs/api/v2/serializers.py
@@ -123,8 +123,8 @@ class BuildSerializer(serializers.ModelSerializer):
 
     commands = BuildCommandSerializer(many=True, read_only=True)
     project_slug = serializers.ReadOnlyField(source='project.slug')
-    version_slug = serializers.ReadOnlyField(source='version.slug')
-    docs_url = serializers.ReadOnlyField(source='version.get_absolute_url')
+    version_slug = serializers.ReadOnlyField(source='get_version_slug')
+    docs_url = serializers.SerializerMethodField()
     state_display = serializers.ReadOnlyField(source='get_state_display')
     commit_url = serializers.ReadOnlyField(source='get_commit_url')
     # Jsonfield needs an explicit serializer
@@ -136,6 +136,11 @@ class Meta:
         # `_config` should be excluded to avoid conflicts with `config`
         exclude = ('builder', '_config')
 
+    def get_docs_url(self, obj):
+        if obj.version:
+            return obj.version.get_absolute_url()
+        return ''
+
 
 class BuildAdminSerializer(BuildSerializer):
 
diff --git a/readthedocs/api/v3/serializers.py b/readthedocs/api/v3/serializers.py
@@ -69,14 +69,16 @@ def get__self(self, obj):
         return self._absolute_url(path)
 
     def get_version(self, obj):
-        path = reverse(
-            'projects-versions-detail',
-            kwargs={
-                'parent_lookup_project__slug': obj.project.slug,
-                'version_slug': obj.version.slug,
-            },
-        )
-        return self._absolute_url(path)
+        if obj.version:
+            path = reverse(
+                'projects-versions-detail',
+                kwargs={
+                    'parent_lookup_project__slug': obj.project.slug,
+                    'version_slug': obj.version.slug,
+                },
+            )
+            return self._absolute_url(path)
+        return ''
 
     def get_project(self, obj):
         path = reverse(
@@ -103,14 +105,16 @@ def get_project(self, obj):
         return self._absolute_url(path)
 
     def get_version(self, obj):
-        path = reverse(
-            'project_version_detail',
-            kwargs={
-                'project_slug': obj.project.slug,
-                'version_slug': obj.version.slug
-            }
-        )
-        return self._absolute_url(path)
+        if obj.version:
+            path = reverse(
+                'project_version_detail',
+                kwargs={
+                    'project_slug': obj.project.slug,
+                    'version_slug': obj.version.slug
+                }
+            )
+            return self._absolute_url(path)
+        return ""
 
 
 class BuildConfigSerializer(FlexFieldsSerializerMixin, serializers.Serializer):
diff --git a/readthedocs/builds/migrations/0030_dont_cascade_delete_build.py b/readthedocs/builds/migrations/0030_dont_cascade_delete_build.py
@@ -0,0 +1,34 @@
+# Generated by Django 2.2.16 on 2020-11-18 21:52
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('builds', '0029_add_time_fields'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='build',
+            name='version_name',
+            field=models.CharField(blank=True, max_length=255, null=True, verbose_name='Version name'),
+        ),
+        migrations.AddField(
+            model_name='build',
+            name='version_slug',
+            field=models.CharField(blank=True, max_length=255, null=True, verbose_name='Version slug'),
+        ),
+        migrations.AddField(
+            model_name='build',
+            name='version_type',
+            field=models.CharField(blank=True, choices=[('branch', 'Branch'), ('tag', 'Tag'), ('external', 'External'), ('unknown', 'Unknown')], max_length=32, null=True, verbose_name='Version type'),
+        ),
+        migrations.AlterField(
+            model_name='build',
+            name='version',
+            field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='builds', to='builds.Version', verbose_name='Version'),
+        ),
+    ]
diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py
@@ -62,6 +62,7 @@
     get_bitbucket_username_repo,
     get_github_username_repo,
     get_gitlab_username_repo,
+    get_vcs_url,
 )
 from readthedocs.builds.version_slug import VersionSlugField
 from readthedocs.config import LATEST_CONFIGURATION_VERSION
@@ -73,12 +74,10 @@
     GITHUB_BRAND,
     GITHUB_COMMIT_URL,
     GITHUB_PULL_REQUEST_COMMIT_URL,
-    GITHUB_PULL_REQUEST_URL,
     GITHUB_URL,
     GITLAB_BRAND,
     GITLAB_COMMIT_URL,
     GITLAB_MERGE_REQUEST_COMMIT_URL,
-    GITLAB_MERGE_REQUEST_URL,
     GITLAB_URL,
     MEDIA_TYPES,
     PRIVACY_CHOICES,
@@ -212,47 +211,22 @@ def ref(self):
 
     @property
     def vcs_url(self):
-        """
-        Generate VCS (github, gitlab, bitbucket) URL for this version.
-
-        Example: https://github.com/rtfd/readthedocs.org/tree/3.4.2/.
-        External Version Example: https://github.com/rtfd/readthedocs.org/pull/99/.
-        """
-        if self.type == EXTERNAL:
-            if 'github' in self.project.repo:
-                user, repo = get_github_username_repo(self.project.repo)
-                return GITHUB_PULL_REQUEST_URL.format(
-                    user=user,
-                    repo=repo,
-                    number=self.verbose_name,
-                )
-            if 'gitlab' in self.project.repo:
-                user, repo = get_gitlab_username_repo(self.project.repo)
-                return GITLAB_MERGE_REQUEST_URL.format(
-                    user=user,
-                    repo=repo,
-                    number=self.verbose_name,
-                )
-            # TODO: Add VCS URL for BitBucket.
-            return ''
-
-        url = ''
-        if self.slug == STABLE:
-            slug_url = self.ref
-        elif self.slug == LATEST:
-            slug_url = self.project.get_default_branch()
-        else:
-            slug_url = self.slug
-
-        if ('github' in self.project.repo) or ('gitlab' in self.project.repo):
-            url = f'/tree/{slug_url}/'
-
-        if 'bitbucket' in self.project.repo:
-            slug_url = self.identifier
-            url = f'/src/{slug_url}'
-
-        # TODO: improve this replacing
-        return self.project.repo.replace('git://', 'https://').replace('.git', '') + url
+        version_name = self.verbose_name
+        if not self.is_external:
+            if self.slug == STABLE:
+                version_name = self.ref
+            elif self.slug == LATEST:
+                version_name = self.project.get_default_branch()
+            else:
+                version_name = self.slug
+            if 'bitbucket' in self.project.repo:
+                version_name = self.identifier
+
+        return get_vcs_url(
+            project=self.project,
+            version_type=self.type,
+            version_name=version_name,
+        )
 
     @property
     def last_build(self):
@@ -623,7 +597,7 @@ class Build(models.Model):
         verbose_name=_('Version'),
         null=True,
         related_name='builds',
-        on_delete=models.CASCADE,
+        on_delete=models.SET_NULL,
     )
     type = models.CharField(
         _('Type'),
@@ -662,12 +636,33 @@ class Build(models.Model):
     output = models.TextField(_('Output'), default='', blank=True)
     error = models.TextField(_('Error'), default='', blank=True)
     exit_code = models.IntegerField(_('Exit code'), null=True, blank=True)
+
+    # Metadata from were the build happened.
     commit = models.CharField(
         _('Commit'),
         max_length=255,
         null=True,
         blank=True,
     )
+    version_slug = models.CharField(
+        _('Version slug'),
+        max_length=255,
+        null=True,
+        blank=True,
+    )
+    version_name = models.CharField(
+        _('Version name'),
+        max_length=255,
+        null=True,
+        blank=True,
+    )
+    version_type = models.CharField(
+        _('Version type'),
+        max_length=32,
+        choices=VERSION_TYPES,
+        null=True,
+        blank=True,
+    )
     _config = JSONField(_('Configuration used in the build'), default=dict)
 
     length = models.IntegerField(_('Build Length'), null=True, blank=True)
@@ -772,6 +767,11 @@ def save(self, *args, **kwargs):  # noqa
                 # yapf: enable
                 previous_pk = previous._config.get(self.CONFIG_KEY, previous.pk)
                 self._config = {self.CONFIG_KEY: previous_pk}
+
+        if self.version:
+            self.version_name = self.version.verbose_name
+            self.version_slug = self.version.slug
+            self.version_type = self.version.type
         super().save(*args, **kwargs)
         self._config_changed = False
 
@@ -803,6 +803,31 @@ def get_full_url(self):
         )
         return full_url
 
+    def get_version_name(self):
+        if self.version:
+            return self.version.verbose_name
+        return self.version_name
+
+    def get_version_slug(self):
+        if self.version:
+            return self.version.verbose_name
+        return self.version_name
+
+    def get_version_type(self):
+        if self.version:
+            return self.version.type
+        return self.version_type
+
+    @property
+    def vcs_url(self):
+        if self.version:
+            return self.version.vcs_url
+        return get_vcs_url(
+            project=self.project,
+            version_type=self.get_version_type(),
+            version_name=self.get_version_name(),
+        )
+
     def get_commit_url(self):
         """Return the commit URL."""
         repo_url = self.project.repo
@@ -815,7 +840,7 @@ def get_commit_url(self):
                 return GITHUB_PULL_REQUEST_COMMIT_URL.format(
                     user=user,
                     repo=repo,
-                    number=self.version.verbose_name,
+                    number=self.get_version_name(),
                     commit=self.commit
                 )
             if 'gitlab' in repo_url:
@@ -826,7 +851,7 @@ def get_commit_url(self):
                 return GITLAB_MERGE_REQUEST_COMMIT_URL.format(
                     user=user,
                     repo=repo,
-                    number=self.version.verbose_name,
+                    number=self.get_version_name(),
                     commit=self.commit
                 )
             # TODO: Add External Version Commit URL for BitBucket.
@@ -877,7 +902,10 @@ def is_stale(self):
 
     @property
     def is_external(self):
-        return self.version.type == EXTERNAL
+        type = self.version_type
+        if self.version:
+            type = self.version.type
+        return type == EXTERNAL
 
     @property
     def external_version_name(self):
diff --git a/readthedocs/builds/utils.py b/readthedocs/builds/utils.py
@@ -5,9 +5,12 @@
 from celery.five import monotonic
 from django.core.cache import cache
 
+from readthedocs.builds.constants import EXTERNAL
 from readthedocs.projects.constants import (
     BITBUCKET_REGEXS,
+    GITHUB_PULL_REQUEST_URL,
     GITHUB_REGEXS,
+    GITLAB_MERGE_REQUEST_URL,
     GITLAB_REGEXS,
 )
 
@@ -41,6 +44,41 @@ def get_gitlab_username_repo(url=None):
     return (None, None)
 
 
+def get_vcs_url(*, project, version_type, version_name):
+    """
+    Generate VCS (github, gitlab, bitbucket) URL for this version.
+
+    Example: https://github.com/rtfd/readthedocs.org/tree/3.4.2/.
+    External version example: https://github.com/rtfd/readthedocs.org/pull/99/.
+    """
+    if version_type == EXTERNAL:
+        if 'github' in project.repo:
+            user, repo = get_github_username_repo(project.repo)
+            return GITHUB_PULL_REQUEST_URL.format(
+                user=user,
+                repo=repo,
+                number=version_name,
+            )
+        if 'gitlab' in project.repo:
+            user, repo = get_gitlab_username_repo(project.repo)
+            return GITLAB_MERGE_REQUEST_URL.format(
+                user=user,
+                repo=repo,
+                number=version_name,
+            )
+        # TODO: Add VCS URL for BitBucket.
+        return ''
+
+    url = ''
+    if ('github' in project.repo) or ('gitlab' in project.repo):
+        url = f'/tree/{version_name}/'
+    elif 'bitbucket' in project.repo:
+        url = f'/src/{version_name}'
+
+    # TODO: improve this replacing
+    return project.repo.replace('git://', 'https://').replace('.git', '') + url
+
+
 @contextmanager
 def memcache_lock(lock_id, oid):
     """
diff --git a/readthedocs/templates/builds/build_detail.html b/readthedocs/templates/builds/build_detail.html
