Builds: don't delete them when a version is deleted (#7679)

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

Author: stsewd

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 None
+
 
 class BuildAdminSerializer(BuildSerializer):
 

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 None
 
     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 None
 
 
 class BuildConfigSerializer(FlexFieldsSerializerMixin, serializers.Serializer):

readthedocs/builds/migrations/0031_add_version_fields_to_build.py

@@ -0,0 +1,29 @@
+# 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', '0030_add_automation_rule_matches'),
+    ]
+
+    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'),
+        ),
+    ]

readthedocs/builds/migrations/0032_migrate_version_data_to_build.py

@@ -0,0 +1,22 @@
+# Generated by Django 2.2.17 on 2020-11-25 15:56
+
+from django.db import migrations
+
+
+def forwards_func(apps, schema_editor):
+    """Migrate all data from versions to builds."""
+    Build = apps.get_model('builds', 'Build')
+    for build in Build.objects.all().iterator():
+        # When the build is saved, the fields are updated.
+        build.save()
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('builds', '0031_add_version_fields_to_build'),
+    ]
+
+    operations = [
+        migrations.RunPython(forwards_func),
+    ]

readthedocs/builds/migrations/0033_dont_cascade_delete_builds.py

@@ -0,0 +1,19 @@
+# Generated by Django 2.2.17 on 2020-11-25 16:01
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('builds', '0032_migrate_version_data_to_build'),
+    ]
+
+    operations = [
+        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'),
+        ),
+    ]

readthedocs/builds/models.py

@@ -63,6 +63,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
@@ -74,12 +75,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,
@@ -213,47 +212,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):
@@ -622,7 +596,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'),
@@ -661,12 +635,34 @@ 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.
+    # This is also used after the version is deleted.
     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)
@@ -763,14 +759,18 @@ def save(self, *args, **kwargs):  # noqa
         """
         if self.pk is None or self._config_changed:
             previous = self.previous
-            # yapf: disable
             if (
-                previous is not None and self._config and
-                self._config == previous.config
+                previous is not None
+                and self._config
+                and self._config == previous.config
             ):
-                # 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
 
@@ -802,6 +802,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
@@ -814,7 +839,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:
@@ -825,7 +850,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.
@@ -876,7 +901,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):