Build: add a new "Cancelled" final state

This commit introduces a new build final state: "Cancelled".
This state is shown in the build listing page and also in the build details
page. The build will end up on this state when:

- The user cancels the build by clicking on the button
- The build is cancelled due to inactivity
- The build is cancelled because it was duplicated

Ther are a lot of changes required for this:

 - HTML template logic for build details and listing pages
 - API changes to add a new value
 - Knockout.js logic for build details page
 - `Build` model changes to support a new state
 - Update different places of core logic to change checking for `finished` state
   to `BUILD_FINAL_STATES` instead
 - Documentation changes to mention the new state
 - Celery build handler `after_return` to decide if it's `finished` or
   `cancelled` the ending state
 - Logic to decide if the build should be killed due to inactivity

Reference #9100

docs/user/api/v3.rst

@@ -694,7 +694,7 @@ Build details
     :>json string created: The ISO-8601 datetime when the build was created.
     :>json string finished: The ISO-8601 datetime when the build has finished.
     :>json integer duration: The length of the build in seconds.
-    :>json string state: The state of the build (one of ``triggered``, ``building``, ``installing``, ``cloning``, or ``finished``)
+    :>json string state: The state of the build (one of ``triggered``, ``building``, ``installing``, ``cloning``, ``finished`` or ``cancelled``)
     :>json string error: An error message if the build was unsuccessful
 
     :query string expand: allows to add/expand some extra fields in the response.

media/javascript/build_updater.js

@@ -30,18 +30,23 @@
                 var el = $(this.buildDiv + ' span#build-' + prop);
 
                 if (prop == 'success') {
-                    if (data.hasOwnProperty('state') && data['state'] != 'finished') {
+                    if (data.hasOwnProperty('state') && data['state'] != 'finished') && data['state'] != 'cancelled' {
                         val = "Not yet finished";
                     }
                     else {
+                        // TODO: I'm not sure what to do with these. We are
+                        // adding a third option here ("Cancelled") that's not
+                        // "Passed" nor "Failed". There are many other places in
+                        // the code where we are assuming only two possible
+                        // options.
                         val = val ? "Passed" : "Failed";
                     }
                 }
 
                 if (prop == 'state') {
                     val = val.charAt(0).toUpperCase() + val.slice(1);
 
-                    if (val == 'Finished') {
+                    if (val == 'Finished' || val == 'Cancelled') {
                         _this.stopPolling();
                     }
                 }
@@ -55,26 +60,26 @@
 
     BuildUpdater.prototype.getBuild = function() {
         _this = this;
-        
+
         $.get(this.buildUrl, function(data) {
             _this.render(data);
         });
     };
 
-    // If the build with ID `this.buildId` has a state other than finished, poll
-    // the server every 5 seconds for the current status. Update the details
-    // page with the latest values from the server, to keep the user informed of
-    // progress.
+    // If the build with ID `this.buildId` has a state other than finished or
+    // cancelled, poll the server every 5 seconds for the current status. Update
+    // the details page with the latest values from the server, to keep the user
+    // informed of progress.
     //
-    // If we haven't received a 'finished' state back the server in 10 minutes,
+    // If we haven't received a 'finished'/'cancelled' state back the server in 10 minutes,
     // stop polling.
     BuildUpdater.prototype.startPolling = function() {
         var stateSpan = $(this.buildDiv + ' span#build-state');
         var _this = this;
 
         // If the build is already finished, or it isn't displayed on the page,
         // ignore it.
-        if (stateSpan.text() == 'Finished' || stateSpan.length === 0) {
+        if (stateSpan.text() == 'Finished' || stateSpan.text() == 'Cancelled' || stateSpan.length === 0) {
             return;
         }
 
@@ -117,7 +122,7 @@
                 if (prop == 'state') {
                     // Show the success value ("Passed" or "Failed") if the build
                     // finished. Otherwise, show the state value.
-                    if (val == 'Finished') {
+                    if (val == 'Finished' || val == 'Cancelled') {
                         val = data['success'];
                         _this.stopPolling();
                     } else {
@@ -134,4 +139,3 @@
 
 
 }).call(this);
-

readthedocs/api/v2/templates/restapi/log.txt

@@ -5,7 +5,7 @@ Version: {{ build.version_slug }}
 Commit: {{ build.commit }}
 Date: {{ build.date }}
 State: {{ build.state }}
-Success: {% if build.state == 'finished' %}{{ build.success }}{% else %}Unknown{% endif %}
+Success: {% if build.state == 'finished' or build.state == 'cancelled' %}{{ build.success }}{% else %}Unknown{% endif %}
 
 {% for command in build.commands %}
 [rtd-command-info] start-time: {{ command.start_time }}, end-time: {{ command.end_time }}, duration: {{ command.run_time }}, exit-code: {{ command.exit_code }}

readthedocs/api/v3/filters.py

@@ -1,8 +1,8 @@
 import django_filters.rest_framework as filters
 
-from readthedocs.builds.constants import BUILD_STATE_FINISHED
+from readthedocs.builds.constants import BUILD_FINAL_STATES
 from readthedocs.builds.models import Build, Version
-from readthedocs.oauth.models import RemoteRepository, RemoteOrganization
+from readthedocs.oauth.models import RemoteOrganization, RemoteRepository
 from readthedocs.projects.models import Project
 
 
@@ -45,9 +45,9 @@ class Meta:
 
     def get_running(self, queryset, name, value):
         if value:
-            return queryset.exclude(state=BUILD_STATE_FINISHED)
+            return queryset.exclude(state__in=BUILD_FINAL_STATES)
 
-        return queryset.filter(state=BUILD_STATE_FINISHED)
+        return queryset.filter(state__in=BUILD_FINAL_STATES)
 
 
 class RemoteRepositoryFilter(filters.FilterSet):

readthedocs/builds/constants.py

@@ -3,20 +3,27 @@
 from django.conf import settings
 from django.utils.translation import gettext_lazy as _
 
-BUILD_STATE_TRIGGERED = 'triggered'
-BUILD_STATE_CLONING = 'cloning'
-BUILD_STATE_INSTALLING = 'installing'
-BUILD_STATE_BUILDING = 'building'
-BUILD_STATE_UPLOADING = 'uploading'
-BUILD_STATE_FINISHED = 'finished'
+BUILD_STATE_TRIGGERED = "triggered"
+BUILD_STATE_CLONING = "cloning"
+BUILD_STATE_INSTALLING = "installing"
+BUILD_STATE_BUILDING = "building"
+BUILD_STATE_UPLOADING = "uploading"
+BUILD_STATE_FINISHED = "finished"
+BUILD_STATE_CANCELLED = "cancelled"
 
 BUILD_STATE = (
-    (BUILD_STATE_TRIGGERED, _('Triggered')),
-    (BUILD_STATE_CLONING, _('Cloning')),
-    (BUILD_STATE_INSTALLING, _('Installing')),
-    (BUILD_STATE_BUILDING, _('Building')),
-    (BUILD_STATE_UPLOADING, _('Uploading')),
-    (BUILD_STATE_FINISHED, _('Finished')),
+    (BUILD_STATE_TRIGGERED, _("Triggered")),
+    (BUILD_STATE_CLONING, _("Cloning")),
+    (BUILD_STATE_INSTALLING, _("Installing")),
+    (BUILD_STATE_BUILDING, _("Building")),
+    (BUILD_STATE_UPLOADING, _("Uploading")),
+    (BUILD_STATE_FINISHED, _("Finished")),
+    (BUILD_STATE_CANCELLED, _("Cancelled")),
+)
+
+BUILD_FINAL_STATES = (
+    BUILD_STATE_FINISHED,
+    BUILD_STATE_CANCELLED,
 )
 
 BUILD_TYPES = (

readthedocs/builds/filters.py

@@ -1,10 +1,9 @@
 import structlog
-
 from django.forms.widgets import HiddenInput
 from django.utils.translation import gettext_lazy as _
 from django_filters import CharFilter, ChoiceFilter, FilterSet
 
-from readthedocs.builds.constants import BUILD_STATE_FINISHED
+from readthedocs.builds.constants import BUILD_FINAL_STATES, BUILD_STATE_FINISHED
 
 log = structlog.get_logger(__name__)
 
@@ -37,7 +36,7 @@ def get_state(self, queryset, name, value):
             queryset = queryset.filter(state=BUILD_STATE_FINISHED, success=True)
         elif value == self.STATE_FAILED:
             queryset = queryset.filter(
-                state=BUILD_STATE_FINISHED,
+                state__in=BUILD_FINAL_STATES,
                 success=False,
             )
         return queryset

readthedocs/builds/migrations/0043_add_cancelled_state.py

@@ -0,0 +1,32 @@
+# Generated by Django 3.2.13 on 2022-05-04 11:03
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("builds", "0042_version_state"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="build",
+            name="state",
+            field=models.CharField(
+                choices=[
+                    ("triggered", "Triggered"),
+                    ("cloning", "Cloning"),
+                    ("installing", "Installing"),
+                    ("building", "Building"),
+                    ("uploading", "Uploading"),
+                    ("finished", "Finished"),
+                    ("cancelled", "Cancelled"),
+                ],
+                db_index=True,
+                default="finished",
+                max_length=55,
+                verbose_name="State",
+            ),
+        ),
+    ]

readthedocs/builds/models.py

@@ -21,6 +21,7 @@
 import readthedocs.builds.automation_actions as actions
 from readthedocs.builds.constants import (
     BRANCH,
+    BUILD_FINAL_STATES,
     BUILD_STATE,
     BUILD_STATE_FINISHED,
     BUILD_STATE_TRIGGERED,
@@ -933,8 +934,8 @@ def get_commit_url(self):
 
     @property
     def finished(self):
-        """Return if build has a finished state."""
-        return self.state == BUILD_STATE_FINISHED
+        """Return if build has an end state."""
+        return self.state in BUILD_FINAL_STATES
 
     @property
     def is_stale(self):

readthedocs/builds/querysets.py

@@ -1,12 +1,13 @@
 """Build and Version QuerySet classes."""
 import datetime
-import structlog
 
+import structlog
 from django.db import models
 from django.db.models import Q
 from django.utils import timezone
 
 from readthedocs.builds.constants import (
+    BUILD_STATE_CANCELLED,
     BUILD_STATE_FINISHED,
     BUILD_STATE_TRIGGERED,
     EXTERNAL,
@@ -225,9 +226,18 @@ def concurrent(self, project):
             query |= Q(project__in=organization.projects.all())
 
         concurrent = (
-            self.filter(query)
-            .exclude(state__in=[BUILD_STATE_TRIGGERED, BUILD_STATE_FINISHED])
-        ).distinct().count()
+            (
+                self.filter(query).exclude(
+                    state__in=[
+                        BUILD_STATE_TRIGGERED,
+                        BUILD_STATE_FINISHED,
+                        BUILD_STATE_CANCELLED,
+                    ]
+                )
+            )
+            .distinct()
+            .count()
+        )
 
         max_concurrent = Project.objects.max_concurrent_builds(project)
         log.info(

readthedocs/builds/static-src/builds/js/detail.js

@@ -32,8 +32,11 @@ function BuildDetailView(instance) {
     /* Instance variables */
     self.state = ko.observable(instance.state);
     self.state_display = ko.observable(instance.state_display);
+    self.cancelled = ko.computed(function () {
+        return self.state() === 'cancelled';
+    });
     self.finished = ko.computed(function () {
-        return self.state() === 'finished';
+        return self.state() === 'finished' || self.state() === 'cancelled';
     });
     self.date = ko.observable(instance.date);
     self.success = ko.observable(instance.success);

readthedocs/builds/static/builds/css/detail.css

@@ -19,7 +19,8 @@ div.build-detail div.build-state {
 }
 
 div.build-detail div.build-state > span.build-state-successful,
-div.build-detail div.build-state > span.build-state-failed {
+div.build-detail div.build-state > span.build-state-failed,
+div.build-detail div.build-state > span.build-state-cancelled {
     padding: .3em .5em;
     border-radius: .3em;
     -moz-border-radius: .3em;
@@ -28,7 +29,8 @@ div.build-detail div.build-state > span.build-state-failed {
     color: white;
 }
 
-div.build-detail div.build-state > span.build-state-failed {
+div.build-detail div.build-state > span.build-state-failed,
+div.build-detail div.build-state > span.build-state-cancelled {
     background: #a55;
 }
 div.build-detail div.build-state > span.build-state-successful {

readthedocs/builds/tests/test_build_queryset.py

@@ -1,12 +1,9 @@
-import pytest
-
 import django_dynamic_fixture as fixture
-from django.conf import settings
+import pytest
 
-from readthedocs.builds.querysets import BuildQuerySet
-from readthedocs.builds.models import Build, Version
+from readthedocs.builds.models import Build
 from readthedocs.organizations.models import Organization
-from readthedocs.projects.models import Project, Feature
+from readthedocs.projects.models import Project
 
 
 @pytest.mark.django_db
@@ -18,7 +15,7 @@ def test_concurrent_builds(self):
             max_concurrent_builds=None,
             main_language_project=None,
         )
-        for state in ('triggered', 'building', 'cloning', 'finished'):
+        for state in ("triggered", "building", "cloning", "finished", "cancelled"):
             fixture.get(
                 Build,
                 project=project,
@@ -39,7 +36,7 @@ def test_concurrent_builds_project_limited(self):
             max_concurrent_builds=2,
             main_language_project=None,
         )
-        for state in ('triggered', 'building', 'cloning', 'finished'):
+        for state in ("triggered", "building", "cloning", "finished", "cancelled"):
             fixture.get(
                 Build,
                 project=project,
@@ -58,7 +55,7 @@ def test_concurrent_builds_translations(self):
             max_concurrent_builds=None,
             main_language_project=project,
         )
-        for state in ('triggered', 'building', 'cloning', 'finished'):
+        for state in ("triggered", "building", "cloning", "finished", "cancelled"):
             fixture.get(
                 Build,
                 project=project,
@@ -90,7 +87,7 @@ def test_concurrent_builds_organization(self):
             organization.projects.add(project)
 
         for project in organization.projects.all():
-            for state in ('triggered', 'building', 'cloning', 'finished'):
+            for state in ("triggered", "building", "cloning", "finished", "cancelled"):
                 fixture.get(
                     Build,
                     project=project,
@@ -124,7 +121,7 @@ def test_concurrent_builds_organization_limited(self):
         )
         organization.projects.add(project_with_builds)
         organization.projects.add(project_without_builds)
-        for state in ('triggered', 'building', 'cloning', 'finished'):
+        for state in ("triggered", "building", "cloning", "finished", "cancelled"):
             fixture.get(
                 Build,
                 project=project_with_builds,
@@ -151,7 +148,7 @@ def test_concurrent_builds_organization_and_project_limited(self):
         )
         organization.projects.add(project_limited)
         organization.projects.add(project_not_limited)
-        for state in ('triggered', 'building', 'cloning', 'finished'):
+        for state in ("triggered", "building", "cloning", "finished", "cancelled"):
             fixture.get(
                 Build,
                 project=project_limited,