launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-fix-js-status into lp:launchpad.

Commit message:
Fix AJAX update of snap builds table to handle all build statuses.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-fix-js-status/+merge/330015

I suspect the original confusion was because it was cloned-and-hacked from lib/lp/soyuz/javascript/update_archive_build_statuses.js, which is dealing with BuildSetStatus values rather than BuildStatus values.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-fix-js-status into lp:launchpad.

=== renamed file 'lib/canonical/launchpad/images/build-failed.gif' => 'lib/canonical/launchpad/images/build-failed.png'
=== modified file 'lib/lp/app/browser/tales.py'
--- lib/lp/app/browser/tales.py	2016-12-02 12:04:11 +0000
+++ lib/lp/app/browser/tales.py	2017-08-31 14:38:35 +0000
@@ -1097,6 +1097,7 @@
         return '<img height="14" width="14" alt="" src="/@@/milestone" />'
 
 
+# Keep this in sync with lib/lp/buildmaster/javascript/build_statuses.js.
 class BuildImageDisplayAPI(ObjectImageDisplayAPI):
     """Adapter for IBuild objects to an image.
 
@@ -1119,8 +1120,8 @@
             BuildStatus.CHROOTWAIT: {'src': "/@@/build-chrootwait"},
             BuildStatus.SUPERSEDED: {'src': "/@@/build-superseded"},
             BuildStatus.BUILDING: {'src': "/@@/processing"},
+            BuildStatus.FAILEDTOUPLOAD: {'src': "/@@/build-failedtoupload"},
             BuildStatus.UPLOADING: {'src': "/@@/processing"},
-            BuildStatus.FAILEDTOUPLOAD: {'src': "/@@/build-failedtoupload"},
             BuildStatus.CANCELLING: {'src': "/@@/processing"},
             BuildStatus.CANCELLED: {'src': "/@@/build-failed"},
             }

=== added directory 'lib/lp/buildmaster/javascript'
=== added file 'lib/lp/buildmaster/javascript/buildstatus.js'
--- lib/lp/buildmaster/javascript/buildstatus.js	1970-01-01 00:00:00 +0000
+++ lib/lp/buildmaster/javascript/buildstatus.js	2017-08-31 14:38:35 +0000
@@ -0,0 +1,54 @@
+/* Copyright 2017 Canonical Ltd.  This software is licensed under the
+ * GNU Affero General Public License version 3 (see the file LICENSE).
+ *
+ * Build status handling.
+ *
+ * @module Y.lp.buildmaster.buildstatus
+ */
+YUI.add('lp.buildmaster.buildstatus', function(Y) {
+    var module = Y.namespace('lp.buildmaster.buildstatus');
+
+    // Keep this in sync with
+    // lib/lp/app/browser/tales.py:BuildImageDisplayAPI.
+    var icon_map = {
+        NEEDSBUILD: { src: '/@@/build-needed' },
+        FULLYBUILT: { src: '/@@/build-success' },
+        FAILEDTOBUILD: {
+            src: '/@@/build-failed',
+            width: 16
+        },
+        MANUALDEPWAIT: { src: '/@@/build-depwait' },
+        CHROOTWAIT: { src: '/@@/build-chrootwait' },
+        SUPERSEDED: { src: '/@@/build-superseded' },
+        BUILDING: { src: '/@@/processing' },
+        FAILEDTOUPLOAD: { src: '/@@/build-failedtoupload' },
+        UPLOADING: { src: '/@@/processing' },
+        CANCELLING: { src: '/@@/processing' },
+        CANCELLED: { src: '/@@/build-failed' }
+    };
+
+    module.update_build_status = function(node, build_summary) {
+        var link_node = node.one('a');
+        var img_node = node.one('img');
+
+        if (link_node === null || img_node === null) {
+            return false;
+        }
+
+        if (node.hasClass(build_summary.status)) {
+            return false;
+        }
+        if (!(build_summary.status in icon_map)) {
+            return false;
+        }
+        icon_props = icon_map[build_summary.status];
+
+        node.setAttribute('class', 'build_status');
+        node.addClass(build_summary.status);
+        link_node.set('innerHTML', build_summary.buildstate);
+        img_node.setAttribute('alt', '[' + build_summary.status + ']');
+        img_node.setAttribute('title', build_summary.buildstate);
+        img_node.setAttribute('src', icon_props.src);
+        img_node.setAttribute('width', icon_props.width || 14);
+    };
+}, '0.1', {'requires': []});

=== modified file 'lib/lp/snappy/javascript/snap.update_build_statuses.js'
--- lib/lp/snappy/javascript/snap.update_build_statuses.js	2016-05-14 00:25:07 +0000
+++ lib/lp/snappy/javascript/snap.update_build_statuses.js	2017-08-31 14:38:35 +0000
@@ -5,7 +5,8 @@
  * LP DynamicDomUpdater plugin for updating the latest builds table of a snap.
  *
  * @module Y.lp.snappy.snap.update_build_statuses
- * @requires anim, node, lp.anim, lp.soyuz.dynamic_dom_updater
+ * @requires anim, node, lp.anim, lp.buildmaster.buildstatus,
+ *           lp.soyuz.dynamic_dom_updater
  */
 YUI.add('lp.snappy.snap.update_build_statuses', function(Y) {
     Y.log('loading lp.snappy.snap.update_build_statuses');
@@ -30,40 +31,9 @@
                 return;
             }
 
-            var link_node = td_build_status.one("a");
-            var img_node = td_build_status.one("img");
-
-            if (link_node === null || img_node === null) {
-                return;
-            }
-
-            if (!td_build_status.hasClass(build_summary.status)) {
+            if (Y.lp.buildmaster.buildstatus.update_build_status(
+                    td_build_status, build_summary)) {
                 ui_changed = true;
-                var new_src = null;
-                switch(build_summary.status) {
-                case 'BUILDING':
-                case 'UPLOADING':
-                    new_src = '/@@/processing';
-                    break;
-                case 'NEEDSBUILD':
-                    new_src = '/@@/build-needed';
-                    break;
-                case 'FAILEDTOBUILD':
-                    new_src = '/@@/build-failed';
-                    break;
-                case 'FULLYBUILT_PENDING':
-                    new_src = '/@@/build-success-publishing';
-                    break;
-                default:
-                    new_src = '/@@/build-success';
-                }
-
-                td_build_status.setAttribute("class", "build_status");
-                td_build_status.addClass(build_summary.status);
-                link_node.set("innerHTML", build_summary.buildstate);
-                img_node.setAttribute("src", new_src);
-                img_node.setAttribute("title", build_summary.buildstate);
-                img_node.setAttribute("alt", "[" + build_summary.status + "]");
             }
 
             if (build_summary.when_complete !== null) {
@@ -137,4 +107,5 @@
 }, "0.1", {"requires":["anim",
                        "node",
                        "lp.anim",
+                       "lp.buildmaster.buildstatus",
                        "lp.soyuz.dynamic_dom_updater"]});

=== modified file 'lib/lp/snappy/javascript/tests/test_snap.update_build_statuses.html'
--- lib/lp/snappy/javascript/tests/test_snap.update_build_statuses.html	2016-05-14 00:25:07 +0000
+++ lib/lp/snappy/javascript/tests/test_snap.update_build_statuses.html	2017-08-31 14:38:35 +0000
@@ -34,6 +34,8 @@
       <script type="text/javascript"
           src="../../../../../build/js/lp/app/extras/extras.js"></script>
       <script type="text/javascript"
+          src="../../../../../build/js/lp/buildmaster/buildstatus.js"></script>
+      <script type="text/javascript"
           src="../../../../../build/js/lp/soyuz/lp_dynamic_dom_updater.js"></script>
       <script type="text/javascript"
           src="../../../../../build/js/lp/app/testing/assert.js"></script>

=== modified file 'lib/lp/snappy/javascript/tests/test_snap.update_build_statuses.js'
--- lib/lp/snappy/javascript/tests/test_snap.update_build_statuses.js	2017-07-21 17:35:14 +0000
+++ lib/lp/snappy/javascript/tests/test_snap.update_build_statuses.js	2017-08-31 14:38:35 +0000
@@ -63,28 +63,110 @@
             this.td_status.setAttribute("class", this.td_status_class);
         },
 
-        test_update_build_status_dom: function() {
-            var original_a_href = this.td_status_a.get("href");
-            var data = {
-                "1": {
-                    "status": "BUILDING",
-                    "build_log_url": null,
-                    "when_complete_estimate": true,
-                    "buildstate": "Currently building",
-                    "build_log_size": null,
-                    "when_complete": "in 1 minute"
-                }
-            };
-            module.domUpdate(this.table, data);
-            Y.Assert.areEqual(
-                "build_status BUILDING", this.td_status.getAttribute("class"));
-            Y.Assert.areEqual(
-                "Currently building", this.td_status.get("text").trim());
-            Y.Assert.areEqual("[BUILDING]", this.td_status_img.get("alt"));
-            Y.Assert.areEqual(
-                "Currently building", this.td_status_img.get("title"));
-            Y.Assert.areEqual(
-                "file:///@@/processing", this.td_status_img.get("src"));
+        test_update_build_status_dom_building: function() {
+            var original_a_href = this.td_status_a.get("href");
+            var data = {
+                "1": {
+                    "status": "BUILDING",
+                    "build_log_url": null,
+                    "when_complete_estimate": true,
+                    "buildstate": "Currently building",
+                    "build_log_size": null,
+                    "when_complete": "in 1 minute"
+                }
+            };
+            module.domUpdate(this.table, data);
+            Y.Assert.areEqual(
+                "build_status BUILDING", this.td_status.getAttribute("class"));
+            Y.Assert.areEqual(
+                "Currently building", this.td_status.get("text").trim());
+            Y.Assert.areEqual("[BUILDING]", this.td_status_img.get("alt"));
+            Y.Assert.areEqual(
+                "Currently building", this.td_status_img.get("title"));
+            Y.Assert.areEqual(
+                "file:///@@/processing", this.td_status_img.get("src"));
+            Y.Assert.areEqual("14", this.td_status_img.get("width"));
+            Y.Assert.areEqual(original_a_href, this.td_status_a.get("href"));
+        },
+
+        test_update_build_status_dom_building: function() {
+            var original_a_href = this.td_status_a.get("href");
+            var data = {
+                "1": {
+                    "status": "BUILDING",
+                    "build_log_url": null,
+                    "when_complete_estimate": true,
+                    "buildstate": "Currently building",
+                    "build_log_size": null,
+                    "when_complete": "in 1 minute"
+                }
+            };
+            module.domUpdate(this.table, data);
+            Y.Assert.areEqual(
+                "build_status BUILDING", this.td_status.getAttribute("class"));
+            Y.Assert.areEqual(
+                "Currently building", this.td_status.get("text").trim());
+            Y.Assert.areEqual("[BUILDING]", this.td_status_img.get("alt"));
+            Y.Assert.areEqual(
+                "Currently building", this.td_status_img.get("title"));
+            Y.Assert.areEqual(
+                "file:///@@/processing", this.td_status_img.get("src"));
+            Y.Assert.areEqual("14", this.td_status_img.get("width"));
+            Y.Assert.areEqual(original_a_href, this.td_status_a.get("href"));
+        },
+
+        test_update_build_status_dom_failedtobuild: function() {
+            var original_a_href = this.td_status_a.get("href");
+            var data = {
+                "1": {
+                    "status": "FAILEDTOBUILD",
+                    "build_log_url": null,
+                    "when_complete_estimate": false,
+                    "buildstate": "Failed to build",
+                    "build_log_size": null,
+                    "when_complete": "1 minute ago"
+                }
+            };
+            module.domUpdate(this.table, data);
+            Y.Assert.areEqual(
+                "build_status FAILEDTOBUILD",
+                this.td_status.getAttribute("class"));
+            Y.Assert.areEqual(
+                "Failed to build", this.td_status.get("text").trim());
+            Y.Assert.areEqual(
+                "[FAILEDTOBUILD]", this.td_status_img.get("alt"));
+            Y.Assert.areEqual(
+                "Failed to build", this.td_status_img.get("title"));
+            Y.Assert.areEqual(
+                "file:///@@/build-failed", this.td_status_img.get("src"));
+            Y.Assert.areEqual("16", this.td_status_img.get("width"));
+            Y.Assert.areEqual(original_a_href, this.td_status_a.get("href"));
+        },
+
+        test_update_build_status_dom_chrootwait: function() {
+            var original_a_href = this.td_status_a.get("href");
+            var data = {
+                "1": {
+                    "status": "CHROOTWAIT",
+                    "build_log_url": null,
+                    "when_complete_estimate": false,
+                    "buildstate": "Chroot problem",
+                    "build_log_size": null,
+                    "when_complete": "1 minute ago"
+                }
+            };
+            module.domUpdate(this.table, data);
+            Y.Assert.areEqual(
+                "build_status CHROOTWAIT",
+                this.td_status.getAttribute("class"));
+            Y.Assert.areEqual(
+                "Chroot problem", this.td_status.get("text").trim());
+            Y.Assert.areEqual("[CHROOTWAIT]", this.td_status_img.get("alt"));
+            Y.Assert.areEqual(
+                "Chroot problem", this.td_status_img.get("title"));
+            Y.Assert.areEqual(
+                "file:///@@/build-chrootwait", this.td_status_img.get("src"));
+            Y.Assert.areEqual("14", this.td_status_img.get("width"));
             Y.Assert.areEqual(original_a_href, this.td_status_a.get("href"));
         },
 

=== modified file 'lib/lp/snappy/model/snap.py'
--- lib/lp/snappy/model/snap.py	2017-06-29 12:02:11 +0000
+++ lib/lp/snappy/model/snap.py	2017-08-31 14:38:35 +0000
@@ -537,7 +537,7 @@
 
             result[build.id] = {
                 "status": build.status.name,
-                "buildstate": build.status,
+                "buildstate": build.status.title,
                 "when_complete": when_complete,
                 "when_complete_estimate": build.estimate,
                 "build_log_url": build.log_url,

=== modified file 'lib/lp/snappy/tests/test_snap.py'
--- lib/lp/snappy/tests/test_snap.py	2017-04-24 13:38:04 +0000
+++ lib/lp/snappy/tests/test_snap.py	2017-08-31 14:38:35 +0000
@@ -23,6 +23,8 @@
 from storm.locals import Store
 from testtools.matchers import (
     Equals,
+    Is,
+    MatchesDict,
     MatchesSetwise,
     MatchesStructure,
     )
@@ -420,8 +422,19 @@
         summary1 = snap1.getBuildSummariesForSnapBuildIds(
             [build11.id, build12.id])
         summary2 = snap2.getBuildSummariesForSnapBuildIds([build2.id])
-        self.assertContentEqual([build11.id, build12.id], summary1.keys())
-        self.assertContentEqual([build2.id], summary2.keys())
+        summary_matcher = MatchesDict({
+            "status": Equals("NEEDSBUILD"),
+            "buildstate": Equals("Needs building"),
+            "when_complete": Is(None),
+            "when_complete_estimate": Is(False),
+            "build_log_url": Is(None),
+            "build_log_size": Is(None),
+            })
+        self.assertThat(summary1, MatchesDict({
+            build11.id: summary_matcher,
+            build12.id: summary_matcher,
+            }))
+        self.assertThat(summary2, MatchesDict({build2.id: summary_matcher}))
 
     def test_getBuildSummariesForSnapBuildIds_empty_input(self):
         snap = self.factory.makeSnap()