diff --git a/js/state/siteUtil.js b/js/state/siteUtil.js index d438c24ab7b..e31b3acb859 100644 --- a/js/state/siteUtil.js +++ b/js/state/siteUtil.js @@ -109,6 +109,16 @@ module.exports.addSite = function (sites, siteDetail, tag, originalSiteDetail) { folderId = module.exports.getNextFolderId(sites) } + // Remve duplicate folder + if (!oldSite && tag === siteTags.BOOKMARK_FOLDER) { + const dupFolder = sites.find((site) => isBookmarkFolder(site.get('tags')) && + site.get('parentFolderId') === siteDetail.get('parentFolderId') && + site.get('title') === siteDetail.get('title')) + if (dupFolder) { + sites = module.exports.removeSite(sites, dupFolder, siteTags.BOOKMARK_FOLDER) + } + } + let tags = index !== -1 && sites.getIn([index, 'tags']) || new Immutable.List() if (tag) { tags = tags.toSet().add(tag).toList() @@ -118,20 +128,26 @@ module.exports.addSite = function (sites, siteDetail, tag, originalSiteDetail) { let site = Immutable.fromJS({ lastAccessedTime: siteDetail.get('lastAccessedTime') || new Date().getTime(), tags, - location: siteDetail.get('location'), title: siteDetail.get('title') }) + if (siteDetail.get('location')) { + site = site.set('location', siteDetail.get('location')) + } if (folderId) { site = site.set('folderId', Number(folderId)) } if (typeof customTitle === 'string') { site = site.set('customTitle', customTitle) } - if (siteDetail.get('parentFolderId') || oldSite && oldSite.get('parentFolderId')) { - site = site.set('parentFolderId', Number(siteDetail.get('parentFolderId') || oldSite.get('parentFolderId'))) + if (siteDetail.get('parentFolderId') !== undefined || oldSite && oldSite.get('parentFolderId')) { + let parentFolderId = siteDetail.get('parentFolderId') !== undefined + ? siteDetail.get('parentFolderId') : oldSite.get('parentFolderId') + site = site.set('parentFolderId', Number(parentFolderId)) } - if (siteDetail.get('partitionNumber') || oldSite && oldSite.get('partitionNumber')) { - site = site.set('partitionNumber', Number(siteDetail.get('partitionNumber') || oldSite.get('partitionNumber'))) + if (siteDetail.get('partitionNumber') !== undefined || oldSite && oldSite.get('partitionNumber')) { + let partitionNumber = siteDetail.get('partitionNumber') !== undefined + ? siteDetail.get('partitionNumber') : oldSite.get('partitionNumber') + site = site.set('partitionNumber', Number(partitionNumber)) } if (siteDetail.get('favicon') || oldSite && oldSite.get('favicon')) { site = site.set('favicon', siteDetail.get('favicon') || oldSite.get('favicon')) diff --git a/test/unit/state/siteUtilTest.js b/test/unit/state/siteUtilTest.js index c34a4dbf559..172123a8e06 100644 --- a/test/unit/state/siteUtilTest.js +++ b/test/unit/state/siteUtilTest.js @@ -161,7 +161,9 @@ describe('siteUtil', function () { lastAccessedTime: 123, tags: [siteTags.BOOKMARK], location: testUrl1, - title: 'sample' + title: 'sample', + parentFolderId: 0, + partitionNumber: 0 }) const processedSites = siteUtil.addSite(sites, siteDetail, siteTags.BOOKMARK) const expectedSites = sites.push(siteDetail) @@ -223,6 +225,45 @@ describe('siteUtil', function () { // toJS needed because immutable ownerID :( assert.deepEqual(processedSites.toJS(), expectedSites.toJS()) }) + it('remove duplicate folder', function () { + const sites = Immutable.fromJS([ + { + lastAccessedTime: 123, + title: 'folder1', + folderId: 1, + parentFolderId: 0, + tags: [siteTags.BOOKMARK_FOLDER] + }, + { + lastAccessedTime: 123, + title: 'folder2', + folderId: 2, + parentFolderId: 1, + tags: [siteTags.BOOKMARK_FOLDER] + }, + { + lastAccessedTime: 123, + title: 'bookmark1', + parentFolderId: 1, + location: testUrl1, + tags: [siteTags.BOOKMARK] + }, + { + lastAccessedTime: 123, + title: 'bookmark2', + parentFolderId: 2, + location: testUrl2, + tags: [siteTags.BOOKMARK] + } + ]) + let processedSites = sites + sites.forEach((site) => { + processedSites = siteUtil.addSite(processedSites, site) + }) + const expectedSites = sites + // toJS needed because immutable ownerID :( + assert.deepEqual(processedSites.toJS(), expectedSites.toJS()) + }) }) })