Merge pull request #12517 from NejcZdovc/hotfix/#12481-dnd

Fixes dnd for bookmark toolbar
brave · Jan 5, 2018 · 81346ee · 81346ee
1 parent 2cef685
commit 81346ee
Show file tree

Hide file tree

Showing 6 changed files with 99 additions and 4 deletions.
diff --git a/app/browser/reducers/bookmarkFoldersReducer.js b/app/browser/reducers/bookmarkFoldersReducer.js
@@ -5,6 +5,7 @@
 const Immutable = require('immutable')
 
 // State
+const bookmarksState = require('../../common/state/bookmarksState')
 const bookmarkFoldersState = require('../../common/state/bookmarkFoldersState')
 const bookmarkToolbarState = require('../../common/state/bookmarkToolbarState')
 
@@ -81,7 +82,7 @@ const bookmarkFoldersReducer = (state, action, immutableAction) => {
           action.get('moveIntoParent')
         )
 
-        const destinationDetail = bookmarkFoldersState.getFolder(state, action.get('destinationKey'))
+        const destinationDetail = bookmarksState.findBookmark(state, action.get('destinationKey'))
         state = syncUtil.updateObjectCache(state, destinationDetail, STATE_SITES.BOOKMARK_FOLDERS)
         if (
           destinationDetail.get('parentFolderId') === 0 ||

diff --git a/app/browser/reducers/bookmarksReducer.js b/app/browser/reducers/bookmarksReducer.js
@@ -95,7 +95,7 @@ const bookmarksReducer = (state, action, immutableAction) => {
           action.get('moveIntoParent')
         )
 
-        const destinationDetail = bookmarksState.getBookmark(state, action.get('destinationKey'))
+        const destinationDetail = bookmarksState.findBookmark(state, action.get('destinationKey'))
         state = syncUtil.updateObjectCache(state, destinationDetail, STATE_SITES.BOOKMARKS)
 
         if (

diff --git a/app/common/state/bookmarkFoldersState.js b/app/common/state/bookmarkFoldersState.js
@@ -173,6 +173,11 @@ const bookmarkFoldersState = {
         ? destinationItem.get('parentFolderId')
         : destinationItem.get('folderId')
 
+      // always use parent ID when we are not moving into the folder
+      if (!moveIntoParent) {
+        parentFolderId = destinationItem.get('parentFolderId')
+      }
+
       if (parentFolderId == null) {
         parentFolderId = destinationKey
       }

diff --git a/app/common/state/bookmarksState.js b/app/common/state/bookmarksState.js
@@ -244,6 +244,11 @@ const bookmarksState = {
         ? destinationItem.get('parentFolderId')
         : destinationItem.get('folderId')
 
+      // always use parent ID when we are not moving into the folder
+      if (!moveIntoParent) {
+        parentFolderId = destinationItem.get('parentFolderId')
+      }
+
       if (parentFolderId == null) {
         parentFolderId = destinationKey
       }

diff --git a/test/unit/app/common/state/bookmarkFoldersStateTest.js b/test/unit/app/common/state/bookmarkFoldersStateTest.js
@@ -665,7 +665,46 @@ describe('bookmarkFoldersState unit test', function () {
     })
 
     it('destination parent ID is different then current parent ID', function () {
-      const result = bookmarkFoldersState.moveFolder(moveState, '69', '2')
+      const result = bookmarkFoldersState.moveFolder(moveState, '69', '2', false, false)
+      const expectedState = moveState
+        .setIn(['bookmarkFolders', '69', 'parentFolderId'], 1)
+        .setIn(['cache', 'bookmarkOrder', '0'], Immutable.fromJS([
+          {
+            key: '1',
+            order: 0,
+            type: siteTags.BOOKMARK_FOLDER
+          },
+          {
+            key: '70',
+            order: 1,
+            type: siteTags.BOOKMARK_FOLDER
+          }
+        ]))
+        .setIn(['cache', 'bookmarkOrder', '1'], Immutable.fromJS([
+          {
+            key: '2',
+            order: 0,
+            type: siteTags.BOOKMARK_FOLDER
+          },
+          {
+            key: 'https://brave.com/|0|0',
+            order: 1,
+            type: siteTags.BOOKMARK
+          },
+          {
+            key: '69',
+            order: 2,
+            type: siteTags.BOOKMARK_FOLDER
+          }
+        ]))
+      assert.deepEqual(result.toJS(), expectedState.toJS())
+      assert(removeCacheKeySpy.calledOnce)
+      assert(addFolderToCacheSpy.calledOnce)
+      assert(findBookmarkSpy.calledOnce)
+    })
+
+    it('we want to move folder into the parent', function () {
+      const result = bookmarkFoldersState.moveFolder(moveState, '69', '2', false, true)
       const expectedState = moveState
         .setIn(['bookmarkFolders', '69', 'parentFolderId'], 2)
         .setIn(['cache', 'bookmarkOrder', '0'], Immutable.fromJS([

diff --git a/test/unit/app/common/state/bookmarksStateTest.js b/test/unit/app/common/state/bookmarksStateTest.js
@@ -895,7 +895,52 @@ describe('bookmarkState unit test', function () {
     })
 
     it('destination parent ID is different then current parent ID', function () {
-      const result = bookmarksState.moveBookmark(stateWithData, 'https://brave.com/|0|0', '2')
+      const result = bookmarksState.moveBookmark(stateWithData, 'https://brave.com/|0|0', '2', false, false)
+      const expectedState = stateWithData
+        .deleteIn(['bookmarks', 'https://brave.com/|0|0'])
+        .setIn(['bookmarks', 'https://brave.com/|0|1'], Immutable.fromJS(bookmark1))
+        .setIn(['bookmarks', 'https://brave.com/|0|1', 'parentFolderId'], 1)
+        .setIn(['bookmarks', 'https://brave.com/|0|1', 'key'], 'https://brave.com/|0|1')
+        .setIn(['cache', 'bookmarkOrder', '0'], Immutable.fromJS([
+          {
+            key: 'https://clifton.io/|0|0',
+            order: 0,
+            type: siteTags.BOOKMARK
+          },
+          {
+            key: '1',
+            order: 1,
+            type: siteTags.BOOKMARK_FOLDER
+          }
+        ]))
+        .setIn(['cache', 'bookmarkOrder', '1'], Immutable.fromJS([
+          {
+            key: 'https://brianbondy.com/|0|1',
+            order: 0,
+            type: siteTags.BOOKMARK
+          },
+          {
+            key: '2',
+            order: 1,
+            type: siteTags.BOOKMARK_FOLDER
+          },
+          {
+            key: 'https://brave.com/|0|1',
+            order: 2,
+            type: siteTags.BOOKMARK
+          }
+        ]))
+        .setIn(['cache', 'bookmarkLocation', 'https://brave.com/'], Immutable.fromJS([
+          'https://brave.com/|0|1'
+        ]))
+      assert.deepEqual(result.toJS(), expectedState.toJS())
+      assert(removeCacheKeySpy.calledOnce)
+      assert(addFolderToCacheSpy.calledOnce)
+      assert(findBookmarkSpy.calledOnce)
+    })
+
+    it('we want to move it into the parent folder', function () {
+      const result = bookmarksState.moveBookmark(stateWithData, 'https://brave.com/|0|0', '2', false, true)
       const expectedState = stateWithData
         .deleteIn(['bookmarks', 'https://brave.com/|0|0'])
         .setIn(['bookmarks', 'https://brave.com/|0|2'], Immutable.fromJS(bookmark1))