From 0a62a7e74ce6edb96254cde67685c96d468f4fd4 Mon Sep 17 00:00:00 2001
From: Brian Clifton <brian@clifton.me>
Date: Thu, 11 Jan 2018 22:29:41 -0700
Subject: [PATCH] Merge pull request #12589 from NejcZdovc/hotfix/#12484-other

Fixes edit flow for bookmarks and folders
---
 app/browser/reducers/bookmarkFoldersReducer.js     | 13 ++++++++++++-
 app/browser/reducers/bookmarksReducer.js           |  3 +++
 app/common/state/bookmarkFoldersState.js           |  5 ++---
 .../app/common/state/bookmarkFoldersStateTest.js   | 14 +++++++++-----
 4 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/app/browser/reducers/bookmarkFoldersReducer.js b/app/browser/reducers/bookmarkFoldersReducer.js
index 07533f620ba..9bc48632343 100644
--- a/app/browser/reducers/bookmarkFoldersReducer.js
+++ b/app/browser/reducers/bookmarkFoldersReducer.js
@@ -58,10 +58,21 @@ const bookmarkFoldersReducer = (state, action, immutableAction) => {
           break
         }
 
-        state = bookmarkFoldersState.editFolder(state, key, folder)
+        const oldFolder = bookmarkFoldersState.getFolder(state, key)
+
+        if (oldFolder.isEmpty()) {
+          return state
+        }
+
+        state = bookmarkFoldersState.editFolder(state, key, oldFolder, folder)
         state = syncUtil.updateObjectCache(state, folder, STATE_SITES.BOOKMARK_FOLDERS)
         const folderDetails = bookmarkFoldersState.getFolder(state, key)
         textCalc.calcText(folderDetails, siteTags.BOOKMARK_FOLDER)
+
+        if (folder.has('parentFolderId') && oldFolder.get('parentFolderId') !== folder.get('parentFolderId')) {
+          state = bookmarkToolbarState.setToolbars(state)
+        }
+
         break
       }
     case appConstants.APP_MOVE_BOOKMARK_FOLDER:
diff --git a/app/browser/reducers/bookmarksReducer.js b/app/browser/reducers/bookmarksReducer.js
index da9d783e6bd..aed8d035b84 100644
--- a/app/browser/reducers/bookmarksReducer.js
+++ b/app/browser/reducers/bookmarksReducer.js
@@ -75,6 +75,9 @@ const bookmarksReducer = (state, action, immutableAction) => {
         textCalc.calcText(bookmarkDetail, siteTags.BOOKMARK)
 
         state = bookmarkUtil.updateActiveTabBookmarked(state)
+        if (oldBookmark.get('parentFolderId') !== bookmarkDetail.get('parentFolderId')) {
+          state = bookmarkToolbarState.setToolbars(state)
+        }
         break
       }
     case appConstants.APP_MOVE_BOOKMARK:
diff --git a/app/common/state/bookmarkFoldersState.js b/app/common/state/bookmarkFoldersState.js
index d29d2ec6f3e..4db477cacd4 100644
--- a/app/common/state/bookmarkFoldersState.js
+++ b/app/common/state/bookmarkFoldersState.js
@@ -74,11 +74,10 @@ const bookmarkFoldersState = {
     return state
   },
 
-  editFolder: (state, editKey, folderDetails) => {
+  editFolder: (state, editKey, oldFolder, folderDetails) => {
     state = validateState(state)
-    const oldFolder = bookmarkFoldersState.getFolder(state, editKey)
 
-    if (oldFolder.isEmpty()) {
+    if (oldFolder == null) {
       return state
     }
 
diff --git a/test/unit/app/common/state/bookmarkFoldersStateTest.js b/test/unit/app/common/state/bookmarkFoldersStateTest.js
index b071b4f91f7..7c5dcf36421 100644
--- a/test/unit/app/common/state/bookmarkFoldersStateTest.js
+++ b/test/unit/app/common/state/bookmarkFoldersStateTest.js
@@ -216,6 +216,10 @@ describe('bookmarkFoldersState unit test', function () {
 
   describe('editFolder', function () {
     let addFolderToCacheSpy, removeCacheKeySpy
+
+    const oldFolder = stateWithData.getIn(['bookmarkFolders', '1'])
+    const editKey = '1'
+
     before(function () {
       addFolderToCacheSpy = sinon.spy(bookmarkOrderCache, 'addFolderToCache')
       removeCacheKeySpy = sinon.spy(bookmarkOrderCache, 'removeCacheKey')
@@ -239,13 +243,13 @@ describe('bookmarkFoldersState unit test', function () {
     })
 
     it('old parent id is not the same as provided one', function () {
-      const result = bookmarkFoldersState.editFolder(stateWithData, '1', Immutable.fromJS({
+      const result = bookmarkFoldersState.editFolder(stateWithData, editKey, oldFolder, Immutable.fromJS({
         title: 'New folder name',
         parentFolderId: 1
       }))
       const expectedState = stateWithData
-        .setIn(['bookmarkFolders', '1', 'parentFolderId'], 1)
-        .setIn(['bookmarkFolders', '1', 'title'], 'New folder name')
+        .setIn(['bookmarkFolders', editKey, 'parentFolderId'], 1)
+        .setIn(['bookmarkFolders', editKey, 'title'], 'New folder name')
         .setIn(['cache', 'bookmarkOrder', '0'], Immutable.fromJS([
           {
             key: '69',
@@ -271,11 +275,11 @@ describe('bookmarkFoldersState unit test', function () {
     })
 
     it('old parent id is the same as provided one', function () {
-      const result = bookmarkFoldersState.editFolder(stateWithData, '1', Immutable.fromJS({
+      const result = bookmarkFoldersState.editFolder(stateWithData, editKey, oldFolder, Immutable.fromJS({
         title: 'New folder name'
       }))
       const expectedState = stateWithData
-        .setIn(['bookmarkFolders', '1', 'title'], 'New folder name')
+        .setIn(['bookmarkFolders', editKey, 'title'], 'New folder name')
       assert.deepEqual(result.toJS(), expectedState.toJS())
       assert(addFolderToCacheSpy.notCalled)
       assert(removeCacheKeySpy.notCalled)