From 9c5203276f3a2698c802b8f6f4eaf32a70933afb Mon Sep 17 00:00:00 2001
From: spalger <email@spalger.com>
Date: Wed, 7 Sep 2016 10:29:12 -0700
Subject: [PATCH] [state] don't make extra $location.replace() calls

Issue:
In order to ensure BWC in the new state-hashing changes, rison encoded query string parameters are automatically converted into hashes and placed back into the URL via `$location.search().replace()`. This ensures that extra history entries don't get created, but this is still happening when hashing is disabled (which is now the default). This causes every state-caused history change to become a replacement, which mutilates the history stack.

Fix:
Added a `#isHashingEnabled()` method to the state objects that is called before trying to convert rison encoded query string states and replacing them in the URL.
---
 .../public/state_management/__tests__/state.js | 17 +++++++++++++++++
 src/ui/public/state_management/state.js        | 18 ++++++++++++++----
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/src/ui/public/state_management/__tests__/state.js b/src/ui/public/state_management/__tests__/state.js
index 28b04c9dcc27a..ae8153f0d326d 100644
--- a/src/ui/public/state_management/__tests__/state.js
+++ b/src/ui/public/state_management/__tests__/state.js
@@ -187,6 +187,23 @@ describe('State Management', function () {
       stateObj = state.toObject();
       expect(stateObj).to.eql({});
     });
+
+    it('does not replace the state value on read', () => {
+      const { state } = setup();
+      sinon.stub($location, 'search', (newSearch) => {
+        if (newSearch) {
+          return $location;
+        } else {
+          return {
+            [state.getQueryParamName()]: '(a:1)'
+          };
+        }
+      });
+      const replaceStub = sinon.stub($location, 'replace').returns($location);
+
+      state.fetch();
+      sinon.assert.notCalled(replaceStub);
+    });
   });
 
   describe('Hashing', () => {
diff --git a/src/ui/public/state_management/state.js b/src/ui/public/state_management/state.js
index 835f0e198cbcf..84f5f6128ca1b 100644
--- a/src/ui/public/state_management/state.js
+++ b/src/ui/public/state_management/state.js
@@ -80,13 +80,19 @@ export default function StateProvider(Private, $rootScope, $location, config) {
       $location.search(search).replace();
     }
 
-    if (risonEncoded) {
+    if (!risonEncoded) {
+      return null;
+    }
+
+    if (this.isHashingEnabled()) {
+      // RISON can find its way into the URL any number of ways, including the navbar links or
+      // shared urls with the entire state embedded. These values need to be translated into
+      // hashes and replaced in the browser history when state-hashing is enabled
       search[this._urlParam] = this.toQueryParam(risonEncoded);
       $location.search(search).replace();
-      return risonEncoded;
     }
 
-    return null;
+    return risonEncoded;
   };
 
   /**
@@ -212,13 +218,17 @@ export default function StateProvider(Private, $rootScope, $location, config) {
     return rison.encode(this._parseQueryParamValue(hash));
   };
 
+  State.prototype.isHashingEnabled = function () {
+    return !!config.get('state:storeInSessionStorage');
+  };
+
   /**
    *  Produce the hash version of the state in it's current position
    *
    *  @return {string}
    */
   State.prototype.toQueryParam = function (state = this.toObject()) {
-    if (!config.get('state:storeInSessionStorage')) {
+    if (!this.isHashingEnabled()) {
       return rison.encode(state);
     }