Merge pull request #12589 from NejcZdovc/hotfix/#12484-other

Fixes edit flow for bookmarks and folders
brave · Jan 12, 2018 · 999e451 · 999e451
1 parent a6727e8
commit 999e451
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 9 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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)