# HG changeset patch
# User Andrew Azores <aazores@redhat.com>
# Date 1506369673 14400
# Node ID c37b4d997182685b63f414b72878a65ed4077dbe
# Parent  970066338150429f9f0a94b88aa766cafc63786d
Restore reopen system entry functionality to jvm-list

Reviewed-by: jkang
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2017-September/025185.html
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2017-October/025438.html

diff -r 970066338150 -r c37b4d997182 src/app/components/jvm-list/jvm-list.controller.js
--- a/src/app/components/jvm-list/jvm-list.controller.js	Mon Oct 23 09:55:08 2017 -0400
+++ b/src/app/components/jvm-list/jvm-list.controller.js	Mon Sep 25 16:01:13 2017 -0400
@@ -26,19 +26,32 @@
  */
 
 import filters from 'shared/filters/filters.module.js';
+import services from 'shared/services/services.module.js';
 import components from 'shared/components/components.module.js';
 import jvmListService from './jvm-list.service.js';
 import systemInfoService from 'components/system-info/system-info.service.js';
 
+const LOCAL_STORAGE_KEY = 'jvmListSystemsOpen';
+
 class JvmListController {
-  constructor (jvmListService, systemInfoService, $location, $state, $timeout, $translate) {
+  constructor (jvmListService, systemInfoService, localStorage, $state, $timeout, $translate) {
     'ngInject';
     this.jvmListService = jvmListService;
     this.systemInfoService = systemInfoService;
-    this.location = $location;
     this.timeout = $timeout;
     this.translate = $translate;
+
+    this.localStorage = localStorage;
     this.systemsOpen = {};
+    if (localStorage.hasItem(LOCAL_STORAGE_KEY)) {
+      try {
+        this.systemsOpen = JSON.parse(localStorage.getItem(LOCAL_STORAGE_KEY));
+      } catch (e) {
+        // This is noncritical, so just leave systemsOpen as an empty object if
+        // stored state is unparseable. When this controller is destroyed, any
+        // built up systemsOpen state at that time will be persisted again.
+      }
+    }
 
     this.aliveOnly = true;
 
@@ -46,7 +59,7 @@
     this.listConfig = {
       showSelectBox: false,
       useExpandingRows: true,
-      onClick: item => $location.hash(this.changeLocationHash(item))
+      onClick: item => this.toggleOpenState(item)
     };
     this.jvmConfig = {
       showSelectBox: false,
@@ -65,7 +78,6 @@
     this.translate('jvmList.ERR_TITLE').then(s => this.emptyStateConfig.title = s);
     this.translate('jvmList.ERR_MESSAGE').then(s => this.emptyStateConfig.info = s);
 
-    this.location.hash('');
     let aliveOnlySwitch = angular.element('#aliveOnlyState');
     aliveOnlySwitch.bootstrapSwitch();
     aliveOnlySwitch.on('switchChange.bootstrapSwitch', (event, state) => {
@@ -76,37 +88,12 @@
     this.loadData();
   }
 
-  /**
-   * Given an item, it will append or remove the item from the location
-   * hash depending on the result of (systemsOpen[item.systemId])
-   * E.g., if systemsOpen[item.systemId] is true, then the item is
-   * currently expanded in the pf-list-view, and the URL hash should be
-   * appended with the systemId of the item. Subsequently calling
-   * changeLocationHash with the same item will toggle it's boolean
-   * state in systemsOpen, and the location hash will be rebuilt to
-   * include only systems that are currently open.
-   * @param {Object} item
-   * @param {String || Number} hash
-   */
-  changeLocationHash (item, hash = this.location.hash()) {
+  $onDestroy () {
+    this.localStorage.setItem(LOCAL_STORAGE_KEY, JSON.stringify(this.systemsOpen));
+  }
+
+  toggleOpenState (item) {
     this.systemsOpen[item.systemId] = !this.systemsOpen[item.systemId];
-    if (this.systemsOpen[item.systemId]) {
-      if (hash === '') {
-        hash = item.hostname;
-      } else {
-        hash += '+' + item.hostname;
-      }
-    } else { // rebuild the location hash string
-      hash = '';
-      for (let index in this.systemsOpen) {
-        if (hash === '' && this.systemsOpen[index]) {
-          hash = index;
-        } else if (this.systemsOpen[index]) {
-          hash += '+' + index;
-        }
-      }
-    }
-    return hash;
   }
 
   loadData () {
@@ -116,28 +103,32 @@
       resp => {
         this.showErr = false;
         this.systems = resp.data.response;
+        let systemIds = [];
+        if (this.systems.length === 1) {
+          this.systemsOpen[this.systems[0].systemId] = true;
+        }
         for (let i = 0; i < this.systems.length; i++) {
           let system = this.systems[i];
-          this.systemsOpen[system.systemId] = false;
+          systemIds.push(system.systemId);
           this.systemInfoService.getSystemInfo(system.systemId).then(
             resp => {
-              let jvms = system.jvms;
-              jvms.forEach(jvm => {
-                jvm.systemId = system.systemId;
-              });
-              this.allItems.push({
+              system.jvms.forEach(jvm => jvm.systemId = system.systemId);
+              let item = {
                 systemId: system.systemId,
                 hostname: resp.data.response[0].hostname,
-                jvms: jvms,
+                jvms: system.jvms,
                 timeCreated: resp.data.response[0].timeCreated,
                 pageConfig: {
                   pageNumber: 1,
                   pageSize: 5
                 }
-              });
-              if (this.systems.length === 1) {
-                this.systemsOpen[this.systems[0].systemId] = true;
+              };
+              if (this.systemsOpen[system.systemId]) {
+                item.isExpanded = true;
               }
+
+              this.allItems.push(item);
+
               this.pageConfig.numTotalItems = this.allItems.length;
               this.pageConfig.pageSize = this.allItems.length;
               this.pageConfig.pageNumber = 1;
@@ -147,6 +138,11 @@
             }
           );
         }
+        Object.keys(this.systemsOpen).forEach(key => {
+          if (!systemIds.includes(key)) {
+            delete this.systemsOpen[key];
+          }
+        });
       },
       () => {
         this.listConfig.itemsAvailable = false;
@@ -293,6 +289,7 @@
     'patternfly',
     'patternfly.toolbars',
     filters,
+    services,
     components,
     jvmListService,
     systemInfoService
diff -r 970066338150 -r c37b4d997182 src/app/components/jvm-list/jvm-list.controller.spec.js
--- a/src/app/components/jvm-list/jvm-list.controller.spec.js	Mon Oct 23 09:55:08 2017 -0400
+++ b/src/app/components/jvm-list/jvm-list.controller.spec.js	Mon Sep 25 16:01:13 2017 -0400
@@ -31,7 +31,7 @@
 
   beforeEach(angular.mock.module(controllerModule));
 
-  let rootScope, controller, jvmListSvc, systemInfoSvc, promise, location, state, timeout, translate;
+  let rootScope, controller, jvmListSvc, systemInfoSvc, promise, localStorage, state, timeout, translate;
 
   let fooItem = {
     hostname: 'foo',
@@ -86,8 +86,10 @@
       on: sinon.spy()
     });
     promise = $q.defer();
-    location = {
-      hash: sinon.stub().returns('')
+    localStorage = {
+      hasItem: sinon.stub().returns(true),
+      getItem: sinon.stub().returns(JSON.stringify({ foo: true })),
+      setItem: sinon.spy()
     };
     state = {
       go: sinon.spy()
@@ -106,14 +108,14 @@
     controller = $controller('JvmListController', {
       jvmListService: jvmListSvc,
       systemInfoService: systemInfoSvc,
-      $location: location,
+      localStorage: localStorage,
       $state: state,
       $timeout: timeout,
       $translate: translate
     });
     controller.$onInit();
     sinon.spy(controller, 'applyFilters');
-    sinon.spy(controller, 'changeLocationHash');
+    sinon.spy(controller, 'toggleOpenState');
     sinon.spy(controller, 'compareFn');
     sinon.spy(controller, 'matchesFilter');
     sinon.spy(controller, 'matchesFilters');
@@ -129,12 +131,46 @@
     should.exist(controller);
   });
 
+  describe('$onDestroy', () => {
+    it('should copy systemsOpen to localStorage', () => {
+      localStorage.setItem.should.not.be.called();
+      controller.$onDestroy();
+      localStorage.setItem.should.be.calledOnce();
+      localStorage.setItem.should.be.calledWith('jvmListSystemsOpen', JSON.stringify({ foo: true }));
+    });
+  });
+
+  describe('localStorage', () => {
+    it('should populate systemsOpen from storage', () => {
+      controller.should.have.ownProperty('systemsOpen');
+      controller.systemsOpen.should.deepEqual({ foo: true });
+    });
+
+    it('should default to empty systemsOpen if localStorage has none', () => {
+      angular.mock.inject($controller => {
+        'ngInject';
+        localStorage.hasItem.returns(false);
+        localStorage.getItem.returns(null);
+        controller = $controller('JvmListController', {
+          jvmListService: jvmListSvc,
+          systemInfoService: systemInfoSvc,
+          localStorage: localStorage,
+          $state: state,
+          $timeout: timeout,
+          $translate: translate
+        });
+        controller.should.have.ownProperty('systemsOpen');
+        controller.systemsOpen.should.deepEqual({});
+      });
+    });
+  });
+
   describe('listConfig', () => {
-    it('should use a fn (changeLocationHash) for onClick attribute', () => {
+    it('should use a fn for onClick attribute', () => {
       let fn = controller.listConfig.onClick;
       fn.should.be.a.Function();
       fn('');
-      controller.changeLocationHash.should.be.calledOnce();
+      controller.toggleOpenState.should.be.calledOnce();
     });
   });
 
@@ -152,30 +188,12 @@
     });
   });
 
-  describe('changeLocationHash', () => {
-    it('should add system hostname to url when opened', () => {
-      let prevLocationHash = '';
+  describe('toggleOpenState', () => {
+    it('should toggle systemsOpen value', () => {
       controller.systemsOpen[fooItem.systemId] = false;
-      let result = controller.changeLocationHash(fooItem, prevLocationHash);
-      result.should.equal(fooItem.hostname);
+      controller.toggleOpenState(fooItem);
+      controller.systemsOpen[fooItem.systemId].should.be.true();
     });
-
-    it('should append system hostname if more than one open', () => {
-      let prevLocationHash = 'foo';
-      controller.systemsOpen[fooItem.systemId] = true;
-      let result = controller.changeLocationHash(barbazItem, prevLocationHash);
-      result.should.equal(fooItem.hostname + '+' + barbazItem.hostname);
-    });
-
-    it('should rebuild location hashes when list-view rows are closed', () => {
-      let prevLocationHash = 'foo+bar+baz';
-      controller.systemsOpen.foo = true;
-      controller.systemsOpen.bar = true;
-      controller.systemsOpen.baz = true;
-      let result = controller.changeLocationHash(fooItem, prevLocationHash);
-      result.should.equal('bar+baz');
-    });
-
   });
 
   describe('aliveOnly', () => {
@@ -209,14 +227,29 @@
   });
 
   describe('loadData', () => {
-    it('should set JVMs list and systemsOpen when service resolves', done => {
+    it('should not set systemOpen for multiple results', () => {
       let data = {
         response: [
           {
-            systemId: 'foo',
+            systemId: 'bar',
             jvms: []
           },
           {
+            systemIf: 'baz',
+            jvms: []
+          }
+        ]
+      };
+      promise.resolve({ data: data });
+      rootScope.$apply();
+      controller.systemsOpen.should.deepEqual({});
+      controller.listConfig.itemsAvailable.should.be.True();
+    });
+
+    it('should set systemsOpen to true for singleton result', () => {
+      let data = {
+        response: [
+          {
             systemId: 'bar',
             jvms: []
           }
@@ -224,68 +257,19 @@
       };
       promise.resolve({ data: data });
       rootScope.$apply();
-      controller.should.have.ownProperty('systems');
-      controller.systems.should.deepEqual(data.response);
-      controller.showErr.should.equal(false);
-      controller.systemsOpen.should.deepEqual({
-        foo: false,
-        bar: false
-      });
+      controller.systemsOpen.should.deepEqual({ bar: true });
       controller.listConfig.itemsAvailable.should.be.True();
-      done();
     });
 
-    it('should set systemsOpen to false for multiple results with no hash', done => {
-      let data = {
-        response: [
-          {
-            systemId: 'foo',
-            jvms: []
-          },
-          {
-            systemId: 'bar',
-            jvms: []
-          }
-        ]
-      };
-      promise.resolve({ data: data });
-      rootScope.$apply();
-      controller.systemsOpen.should.deepEqual({
-        foo: false,
-        bar: false
-      });
-      controller.listConfig.itemsAvailable.should.be.True();
-      done();
-    });
-
-    it('should set systemsOpen to true for singleton result', done => {
-      let data = {
-        response: [
-          {
-            systemId: 'foo',
-            jvms: []
-          }
-        ]
-      };
-      promise.resolve({ data: data });
-      rootScope.$apply();
-      controller.systemsOpen.should.deepEqual({
-        foo: true
-      });
-      controller.listConfig.itemsAvailable.should.be.True();
-      done();
-    });
-
-    it('should set error flag when service rejects', done => {
+    it('should set error flag when service rejects', () => {
       promise.reject();
       rootScope.$apply();
       controller.should.have.ownProperty('showErr');
       controller.showErr.should.equal(true);
       controller.listConfig.itemsAvailable.should.be.False();
-      done();
     });
 
-    it('should append systemId to jvm items', done => {
+    it('should append systemId to jvm items', () => {
       let data = {
         response: [
           {
@@ -299,7 +283,6 @@
       controller.allItems[0].jvms[0].should.have.ownProperty('systemId');
       controller.allItems[0].jvms[0].systemId.should.equal('foo');
       controller.listConfig.itemsAvailable.should.be.True();
-      done();
     });
   });