Merge ~ltrager/maas:fix_script_result_triggers into maas:master

[Lee Trager]

LP: #1727568, #1727575 - Create migration to show old storage tests on all devices.

MAAS 2.2 did not support assoicated a ScriptResult with a hardware_type or
physical_blockdevice. When the UI was updated in 2.3 it was decided that
these old results should be assoicated with all known PhysicalBlockDevices.
This was implemented in Javascript but has been causing issues with the
triggers as some ScriptResults are associated with a PhyicalBlockDevice while
others aren't.

Old ScriptResults are now assoicated with the node's known
PhysicalBlockDevices in a migration so the UI gets a similar data set.

[Andres Rodriguez (andreserl)]

lgtm!

review: Approve

[MAAS Lander (maas-lander)]

LANDING
-b fix_script_result_triggers lp:~ltrager/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/566/consoleText

[MAAS Lander (maas-lander)]

UNIT TESTS
-b fix_script_result_triggers lp:~ltrager/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/567/console
COMMIT: 8a382b39679f0a1b0a501ddebdac6687227c5d2f

review: Needs Fixing

b7bbb5d... by Lee Trager on 2017-10-27

Fix test failure

diff --git a/src/maasserver/static/js/angular/factories/node_results.js b/src/maasserver/static/js/angular/factories/node_results.js
index eba3c70..1f51330 100644
--- a/src/maasserver/static/js/angular/factories/node_results.js
+++ b/src/maasserver/static/js/angular/factories/node_results.js
@@ -160,37 +160,23 @@ angular.module('MAAS').factory(
                 }
             }
 
-            if(result.hardware_type === 3) {
+            if(result.hardware_type === 3 &&
+               result.physical_blockdevice !== null) {
                 // Storage results are split into individual components.
                 var disk, subtext;
-                if(result.physical_blockdevice !== null) {
-                    // If the storage result is assoicated with a specific
-                    // component generate subtext for that component.
-                    for(i = 0; i < this._node.disks.length; i++) {
-                        disk = this._node.disks[i];
-                        if(disk.id === result.physical_blockdevice) {
-                            subtext = this._getStorageSubtext(disk);
-                            if(!angular.isArray(
-                                hardware_type_results[subtext])) {
-                                    hardware_type_results[subtext] = [];
-                            }
-                            this._addOrReplace(
-                                hardware_type_results[subtext], result);
-                            break;
-                        }
-                    }
-                }else{
-                    // Storage results which do not have an associated physical
-                    // block device are associated with all known storage
-                    // devices.
-                    for(i = 0; i < this._node.disks.length; i++) {
-                        disk = this._node.disks[i];
+                // If the storage result is associated with a specific
+                // component generate subtext for that component.
+                for(i = 0; i < this._node.disks.length; i++) {
+                    disk = this._node.disks[i];
+                    if(disk.id === result.physical_blockdevice) {
                         subtext = this._getStorageSubtext(disk);
-                        if(!angular.isArray(hardware_type_results[subtext])) {
-                            hardware_type_results[subtext] = [];
+                        if(!angular.isArray(
+                            hardware_type_results[subtext])) {
+                                hardware_type_results[subtext] = [];
                         }
                         this._addOrReplace(
                             hardware_type_results[subtext], result);
+                        break;
                     }
                 }
             }else{
diff --git a/src/maasserver/static/js/angular/factories/tests/test_node_results.js b/src/maasserver/static/js/angular/factories/tests/test_node_results.js
index c94fcc1..55fccae 100644
--- a/src/maasserver/static/js/angular/factories/tests/test_node_results.js
+++ b/src/maasserver/static/js/angular/factories/tests/test_node_results.js
@@ -280,127 +280,6 @@ describe("NodeResultsManagerFactory", function() {
                     $selected: old_result.$selected
                 }]);
             });
-
-            it("add " + result_type_name + " storage result no blockdevice",
-                function() {
-                // Regression test for LP: #1721524
-                var node = makenode();
-                node.disks = [];
-                var i;
-                for(i = 0; i < 3; i++) {
-                    node.disks.push({
-                        id: makeInteger(0, 100),
-                        name: makeName("name"),
-                        model: makeName("model"),
-                        serial: makeName("serial")
-                    });
-                }
-                var manager = NodeResultsManagerFactory.getManager(
-                    node, result_type_name);
-                var result = {
-                    name: makeName("name"),
-                    status: makeInteger(0, 100),
-                    status_name: makeName("status_name"),
-                    result_type: parseInt(result_type, 10),
-                    hardware_type: 3,
-                    physical_blockdevice: null,
-                    showing_results: false,
-                    showing_menu: false,
-                    showing_history: false,
-                    $selected: false
-                };
-                var results_list = [];
-                for(i = 0; i < manager.results.length; i++) {
-                    if(manager.results[i].hardware_type === 3) {
-                        var j;
-                        for(j = 0; j < node.disks.length; j++) {
-                            var subtext = "/dev/" + node.disks[j].name +
-                                " (Model: " + node.disks[j].model +
-                                ", Serial: " + node.disks[j].serial + ")";
-                            var results = manager.results[i].results[
-                                subtext] = [];
-                            results_list.push(results);
-                        }
-                        break;
-                    }
-                }
-
-                manager._processItem(result);
-                for(i = 0; i < results_list.length; i++) {
-                    expect(results_list[i]).toEqual([result]);
-                }
-            });
-
-            it("update " + result_type_name + " storage result no blockdevice",
-                function() {
-                var node = makenode();
-                node.disks = [];
-                var i;
-                for(i = 0; i < 3; i++) {
-                    node.disks.push({
-                        id: makeInteger(0, 100),
-                        name: makeName("name"),
-                        model: makeName("model"),
-                        serial: makeName("serial")
-                    });
-                }
-                var manager = NodeResultsManagerFactory.getManager(
-                    node, result_type_name);
-                var old_result = {
-                    name: makeName("name"),
-                    status: makeInteger(0, 100),
-                    status_name: makeName("status_name"),
-                    result_type: parseInt(result_type, 10),
-                    hardware_type: 3,
-                    physical_blockdevice: null,
-                    showing_results: makeBoolean(),
-                    showing_menu: makeBoolean(),
-                    showing_history: makeBoolean(),
-                    $selected: makeBoolean()
-                };
-                var results_list = [];
-                for(i = 0; i < manager.results.length; i++) {
-                    if(manager.results[i].hardware_type === 3) {
-                        var j;
-                        for(j = 0; j < node.disks.length; j++) {
-                            var subtext = "/dev/" + node.disks[j].name +
-                                " (Model: " + node.disks[j].model +
-                                ", Serial: " + node.disks[j].serial + ")";
-                            var results = manager.results[i].results[
-                                subtext] = [old_result];
-                            results_list.push(results);
-                        }
-                        break;
-                    }
-                }
-                var result = {
-                    name: old_result.name,
-                    status: makeInteger(0, 100),
-                    status_name: makeName("status_name"),
-                    result_type: parseInt(result_type, 10),
-                    hardware_type: 3,
-                    physical_blockdevice: null,
-                    showing_results: false,
-                    showing_menu: false,
-                    showing_history: false,
-                    $selected: false
-                };
-                manager._processItem(result);
-                for(i = 0; i < results_list.length; i++) {
-                    expect(results_list[i]).toEqual([{
-                        name: old_result.name,
-                        status: result.status,
-                        status_name: result.status_name,
-                        result_type: parseInt(result_type, 10),
-                        hardware_type: 3,
-                        physical_blockdevice: null,
-                        showing_results: old_result.showing_results,
-                        showing_menu: old_result.showing_menu,
-                        showing_history: old_result.showing_history,
-                        $selected: old_result.$selected
-                    }]);
-                }
-            });
         });
 
         // Installation results are stored in a signal list.
diff --git a/src/maasserver/static/js/angular/services/manager.js b/src/maasserver/static/js/angular/services/manager.js
index b6d73b9..6703b52 100644
--- a/src/maasserver/static/js/angular/services/manager.js
+++ b/src/maasserver/static/js/angular/services/manager.js
@@ -112,22 +112,9 @@ angular.module('MAAS').service(
         Manager.prototype._replaceItemInArray = function(array, item) {
             var idx = this._getIndexOfItem(array, item[this._pk]);
             if(idx >= 0) {
-                // angular.copy deletes all fields on the original object
-                // before adding the new items. This causes a race conidtion
-                // in NodeResultsManager as it uses the node object to
-                // determine the subtext for storage results. Instead replace
-                // items atomically.
-                angular.forEach(item, function(value, key) {
-                    if(!angular.isDefined(array[idx][key]) ||
-                        (array[idx][key] !== value && key !== "$selected")) {
-                        array[idx][key] = value;
-                    }
-                });
-                angular.forEach(array[idx], function(value, key) {
-                    if(!angular.isDefined(item[key])) {
-                        delete item[key];
-                    }
-                });
+                // Keep the current selection on the item.
+                item.$selected = array[idx].$selected;
+                angular.copy(item, array[idx]);
             }
         };
 
diff --git a/src/metadataserver/migrations/builtin/0015_migrate_storage_tests.py b/src/metadataserver/migrations/builtin/0015_migrate_storage_tests.py
new file mode 100644
index 0000000..05ad0d6
--- /dev/null
+++ b/src/metadataserver/migrations/builtin/0015_migrate_storage_tests.py
@@ -0,0 +1,43 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import (
+    migrations,
+    models,
+)
+
+
+def run_migration(apps, schema_editor):
+    ScriptResult = apps.get_model('metadataserver', 'ScriptResult')
+    PhysicalBlockDevice = apps.get_model('maasserver', 'PhysicalBlockDevice')
+    # Previously ScriptResults were not assoicated with PhysicalBlockDevice as
+    # one test ran on all hardware. In 2.3 each storage device has its own
+    # ScriptResult. To properly show this in the UI make copies of each old
+    # result for all know storage devices.
+    for old_script_result in ScriptResult.objects.filter(
+            physical_blockdevice=None, script_name__in=[
+                'smartctl-validate', 'smartctl-short', 'smartctl-long',
+                'smartctl-conveyance', 'badblocks', 'badblocks-destructive']):
+        for bd in PhysicalBlockDevice.objects.filter(
+                node=old_script_result.script_set.node):
+            script_result = ScriptResult()
+            for field in ScriptResult._meta.fields:
+                if field.name == 'id':
+                    continue
+                setattr(
+                    script_result, field.name,
+                    getattr(old_script_result, field.name))
+            script_result.physical_blockdevice = bd
+            script_result.save()
+        old_script_result.delete()
+
+    
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('metadataserver', '0014_rename_dhcp_unconfigured_ifaces'),
+    ]
+
+    operations = [
+        migrations.RunPython(run_migration),
+    ]