diff --git a/app/browser/reducers/bookmarkFoldersReducer.js b/app/browser/reducers/bookmarkFoldersReducer.js index 824699343ec..07533f620ba 100644 --- 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 index 7f7b7271d4b..da9d783e6bd 100644 --- 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 index 52488c4222f..d29d2ec6f3e 100644 --- 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 index 878e30e7144..1053e7a0378 100644 --- 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 index 914c8753fa6..b071b4f91f7 100644 --- 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 index b64c34a9765..7016cbb8e06 100644 --- 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))