Merge ~steverydz/maas:fix-machine-listing-js-error into maas:master

[Steve Rydz]

LP: #1811377 - Fix JS error on machine list page

If an array isn't passed into the `removeDuplicates` method an error is shown in the console. This change adds a type check to ensure the array exists.

QA steps:
1. Go to machines list view
2. Click through to a machine detail view
3. Click the navigation link back to machines list view
4. Check the browser console
5. There should not be an error for `filter of undefined` in the console

[Anthony Dillon (ya-bo-ng)]

Code and QA +1

review: Approve

[MAAS Lander (maas-lander)]

UNIT TESTS
-b fix-machine-listing-js-error lp:~steverydz/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: f420eabf3ca6a2c86663f26fe783915b965a97f7

review: Approve

[Björn Tillenius (bjornt)]

I can confirm that there's no error shown in the JS console with this branch.

However, I'm curious as what's actually going on there.

For example, on current master, why is there no error on the initial loading of the table? And why is removeDuplicates given an undefined value?

I feel that there's an error somewhere else, and this branch would just hide it even further. Or do you have a good explanation for where the undefined value is coming from?

review: Needs Information

[Björn Tillenius (bjornt)]

Ah, confirmed that this branch indeed hides the real problem further.

Try clicking on a machine for which an IP address is shown in the listing. Now switch back to the listing. You'll see that the IP address is no longer shown.

review: Needs Fixing

[Björn Tillenius (bjornt)]

I suspect that the problem is in the websocket handler. It seems like the IP addresses are included only in the listing, and not when viewing only the node itself. I suspect that's causing problem, since it's unexpected that a single node has less information that the listing.

This seems to fix the issue, but I'd have to investigate further to see that it actually makes sense:

  http://paste.ubuntu.com/p/2TCYHb3Vk5/

[Björn Tillenius (bjornt)]

I couldn't find out exactly what's happening, but I think that there is a cache for each object that gets updated when you go and view the object individually, since you generally need to get more information. Then when switching back to the list, the cache isn't updated, since it's expected that you don't loose information when viewing the object individually.

I've created a new MP for the proper fix:

  https://code.launchpad.net/~bjornt/maas/+git/maas/+merge/362031

review: Disapprove

[MAAS Lander (maas-lander)]

UNIT TESTS
-b fix-machine-listing-js-error lp:~steverydz/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/4962/console
COMMIT: null

review: Needs Fixing

[MAAS Lander (maas-lander)]

UNIT TESTS
-b fix-machine-listing-js-error lp:~steverydz/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/4964/console
COMMIT: null

review: Needs Fixing

[MAAS Lander (maas-lander)]

UNIT TESTS
-b fix-machine-listing-js-error lp:~steverydz/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/4965/console
COMMIT: null

review: Needs Fixing

[MAAS Lander (maas-lander)]

UNIT TESTS
-b fix-machine-listing-js-error lp:~steverydz/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/4966/console
COMMIT: null

review: Needs Fixing

[MAAS Lander (maas-lander)]

UNIT TESTS
-b fix-machine-listing-js-error lp:~steverydz/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/4967/console
COMMIT: null

review: Needs Fixing

[MAAS Lander (maas-lander)]

UNIT TESTS
-b fix-machine-listing-js-error lp:~steverydz/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/4968/console
COMMIT: null

review: Needs Fixing

[MAAS Lander (maas-lander)]

UNIT TESTS
-b fix-machine-listing-js-error lp:~steverydz/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/4969/console
COMMIT: null

review: Needs Fixing

diff --git a/src/maasserver/static/js/angular/directives/machines_table.js b/src/maasserver/static/js/angular/directives/machines_table.js
index 6c0619e..a1aafa5 100644
--- a/src/maasserver/static/js/angular/directives/machines_table.js
+++ b/src/maasserver/static/js/angular/directives/machines_table.js
@@ -253,6 +253,10 @@ angular.module('MAAS').directive('maasMachinesTable', [
         };
 
         scope.removeDuplicates = function(ipArray, prop) {
+          if (!angular.isArray(ipArray)) {
+            return;
+          }
+
           return ipArray.filter((obj, pos, arr) => {
             return arr.map(mapObj => mapObj[prop]).indexOf(obj[prop]) === pos;
           });
diff --git a/src/maasserver/static/js/angular/directives/tests/test_machines_table.js b/src/maasserver/static/js/angular/directives/tests/test_machines_table.js
index b29d4af..41aff3e 100644
--- a/src/maasserver/static/js/angular/directives/tests/test_machines_table.js
+++ b/src/maasserver/static/js/angular/directives/tests/test_machines_table.js
@@ -576,5 +576,29 @@ describe("maasMachinesTable", function() {
             var actual = scope.removeDuplicates(ipAddresses, 'ip');
             expect(actual.length).toEqual(3);
         });
+
+        it("returns undefined if no array given", function() {
+            var directive = compileDirective();
+            var scope = directive.isolateScope();
+
+            var actual = scope.removeDuplicates(undefined, 'ip');
+            expect(actual).toBeUndefined();
+        });
+
+        it("returns undefined if first argument not array", function() {
+            var directive = compileDirective();
+            var scope = directive.isolateScope();
+
+            var actual = scope.removeDuplicates('foo', 'ip');
+            expect(actual).toBeUndefined();
+        });
+
+        it("returns undefined if no prop given", function() {
+            var directive = compileDirective();
+            var scope = directive.isolateScope();
+
+            var actual = scope.removeDuplicates('foo');
+            expect(actual).toBeUndefined();
+        });
     });
 });