Merge pull request #12496 from NejcZdovc/hotfix/#12484-move

Fixes bookmark toolbar move

app/browser/reducers/bookmarkFoldersReducer.js

@@ -71,6 +71,8 @@ const bookmarkFoldersReducer = (state, action, immutableAction) => {
  break
  }
 
+ const oldFolder = bookmarkFoldersState.getFolder(state, key)
+
  state = bookmarkFoldersState.moveFolder(
  state,
  key,
@@ -81,7 +83,11 @@ const bookmarkFoldersReducer = (state, action, immutableAction) => {
 
  const destinationDetail = bookmarkFoldersState.getFolder(state, action.get('destinationKey'))
  state = syncUtil.updateObjectCache(state, destinationDetail, STATE_SITES.BOOKMARK_FOLDERS)
- if (destinationDetail.get('parentFolderId') === 0 || action.get('destinationKey') === 0) {
+ if (
+ destinationDetail.get('parentFolderId') === 0 ||
+ action.get('destinationKey') === 0 ||
+ oldFolder.get('parentFolderId') === 0
+ ) {
  state = bookmarkToolbarState.setToolbars(state)
  }
  break

app/browser/reducers/bookmarksReducer.js

@@ -85,6 +85,8 @@ const bookmarksReducer = (state, action, immutableAction) => {
  break
  }
 
+ const oldBookmark = bookmarksState.getBookmark(state, key)
+
  state = bookmarksState.moveBookmark(
  state,
  key,
@@ -96,7 +98,11 @@ const bookmarksReducer = (state, action, immutableAction) => {
  const destinationDetail = bookmarksState.getBookmark(state, action.get('destinationKey'))
  state = syncUtil.updateObjectCache(state, destinationDetail, STATE_SITES.BOOKMARKS)
 
- if (destinationDetail.get('parentFolderId') === 0 || action.get('destinationKey') === 0) {
+ if (
+ destinationDetail.get('parentFolderId') === 0 ||
+ action.get('destinationKey') === 0 ||
+ oldBookmark.get('parentFolderId') === 0
+ ) {
  state = bookmarkToolbarState.setToolbars(state)
  }
  break

test/unit/app/browser/reducers/bookmarkFoldersReducerTest.js

@@ -75,6 +75,15 @@ describe('bookmarkFoldersReducer unit test', function () {
  partitionNumber: 0,
  objectId: null,
  type: siteTags.BOOKMARK_FOLDER
+ },
+ '81': {
+ title: 'folder80',
+ folderId: 81,
+ key: '81',
+ parentFolderId: 1,
+ partitionNumber: 0,
+ objectId: null,
+ type: siteTags.BOOKMARK_FOLDER
  }
  },
  cache: {
@@ -91,6 +100,13 @@ describe('bookmarkFoldersReducer unit test', function () {
  type: siteTags.BOOKMARK_FOLDER
  }
  ],
+ '1': [
+ {
+ key: '81',
+ order: 0,
+ type: siteTags.BOOKMARK_FOLDER
+ }
+ ],
  '69': [
  {
  key: '80',
@@ -373,15 +389,25 @@ describe('bookmarkFoldersReducer unit test', function () {
  assert.deepEqual(newState.toJS(), expectedState.toJS())
  })
 
- it('destination key is not on bookmark toolbar', function () {
+ it('folder is moved from one folder to another', function () {
  spyToolbar = sinon.spy(bookmarkToolbarState, 'setToolbars')
  bookmarkFoldersReducer(stateWithData, {
  actionType: appConstants.APP_MOVE_BOOKMARK_FOLDER,
- folderKey: '1',
+ folderKey: '81',
  destinationKey: '80'
  })
  assert.equal(spyToolbar.notCalled, true)
  })
+
+ it('folder is moved from the toolbar into other bookmarks', function () {
+ spyToolbar = sinon.spy(bookmarkToolbarState, 'setToolbars')
+ bookmarkFoldersReducer(stateWithData, {
+ actionType: appConstants.APP_MOVE_BOOKMARK_FOLDER,
+ folderKey: '1',
+ destinationKey: '-1'
+ })
+ assert.equal(spyToolbar.calledOnce, true)
+ })
  })
 
  describe('APP_REMOVE_BOOKMARK_FOLDER', function () {
@@ -413,7 +439,7 @@ describe('bookmarkFoldersReducer unit test', function () {
  '69'
  ]
  })
- assert.equal(spy.callCount, 3)
+ assert.equal(spy.callCount, 4)
  assert.equal(spyToolbar.calledOnce, true)
  assert.deepEqual(newState.toJS(), state.toJS())
  })
@@ -433,11 +459,13 @@ describe('bookmarkFoldersReducer unit test', function () {
  type: siteTags.BOOKMARK_FOLDER
  }
  ]))
+ .deleteIn(['cache', 'bookmarkOrder', '1'])
  .deleteIn([STATE_SITES.BOOKMARK_FOLDERS, '1'])
+ .deleteIn([STATE_SITES.BOOKMARK_FOLDERS, '81'])
  .setIn(['windows', 0, 'bookmarksToolbar', 'toolbar'], Immutable.fromJS([
  '69'
  ]))
- assert.equal(spy.calledOnce, true)
+ assert.equal(spy.calledTwice, true)
  assert.equal(spyToolbar.calledOnce, true)
  assert.deepEqual(newState.toJS(), expectedState.toJS())
  })

test/unit/app/browser/reducers/bookmarksReducerTest.js

@@ -75,13 +75,24 @@ describe('bookmarksReducer unit test', function () {
  'https://brianbondy.com/|0|1': {
  favicon: undefined,
  title: 'Clifton',
- location: 'https://clifton.io/',
- key: 'https://clifton.io/|0|1',
+ location: 'https://brianbondy.com/',
+ key: 'https://brianbondy.com/|0|1',
  parentFolderId: 1,
  partitionNumber: 0,
  objectId: null,
  themeColor: undefined,
  type: siteTags.BOOKMARK
+ },
+ 'https://test.com/|0|2': {
+ favicon: undefined,
+ title: 'Clifton',
+ location: 'https://test.com/',
+ key: 'https://test.com/|0|2',
+ parentFolderId: 2,
+ partitionNumber: 0,
+ objectId: null,
+ themeColor: undefined,
+ type: siteTags.BOOKMARK
  }
  },
  bookmarkFolders: {},
@@ -105,6 +116,13 @@ describe('bookmarksReducer unit test', function () {
  order: 0,
  type: siteTags.BOOKMARK
  }
+ ],
+ '2': [
+ {
+ key: 'https://test.com/|0|2',
+ order: 0,
+ type: siteTags.BOOKMARK
+ }
  ]
  },
  bookmarkLocation: {
@@ -116,6 +134,9 @@ describe('bookmarksReducer unit test', function () {
  ],
  'https://brianbondy.com/': [
  'https://brianbondy.com/|0|1'
+ ],
+ 'https://test.com/': [
+ 'https://test.com/|0|2'
  ]
  }
  },
@@ -536,15 +557,25 @@ describe('bookmarksReducer unit test', function () {
  assert.deepEqual(newState.toJS(), expectedState.toJS())
  })
 
- it('destination key is not on bookmark toolbar', function () {
+ it('bookmark is moved from folder to another folder', function () {
  spyToolbar = sinon.spy(bookmarkToolbarState, 'setToolbars')
  bookmarksReducer(stateWithData, {
  actionType: appConstants.APP_MOVE_BOOKMARK,
- bookmarkKey: 'https://clifton.io/|0|0',
- destinationKey: 'https://brianbondy.com/|0|1'
+ bookmarkKey: 'https://brianbondy.com/|0|1',
+ destinationKey: 'https://test.com/|0|2'
  })
  assert.equal(spyToolbar.notCalled, true)
  })
+
+ it('bookmark is moved from toolbar to another folder', function () {
+ spyToolbar = sinon.spy(bookmarkToolbarState, 'setToolbars')
+ bookmarksReducer(stateWithData, {
+ actionType: appConstants.APP_MOVE_BOOKMARK,
+ bookmarkKey: 'https://clifton.io/|0|0',
+ destinationKey: 'https://test.com/|0|2'
+ })
+ assert.equal(spyToolbar.calledOnce, true)
+ })
  })
 
  describe('APP_REMOVE_BOOKMARK', function () {