From 3e1899b5f07cdcde0934a38761ce7eb388f2744e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EF=BD=81=EF=BD=99=EF=BD=95=EF=BD=8D=EF=BD=89=C2=A0=20?= =?UTF-8?q?=EF=BD=99=EF=BD=95?= Date: Tue, 1 Aug 2017 06:40:08 +0000 Subject: [PATCH] More Sync sites refactor fixes and remove siteUtil.js --- app/common/lib/historyUtil.js | 5 +- app/common/state/bookmarkFoldersState.js | 5 +- app/common/state/bookmarksState.js | 16 +- app/sync.js | 32 ++-- js/state/siteUtil.js | 14 -- js/state/syncUtil.js | 230 +++++++++++++---------- test/misc-components/syncTest.js | 8 +- test/unit/state/syncUtilTest.js | 223 ++++++++++------------ 8 files changed, 276 insertions(+), 257 deletions(-) delete mode 100644 js/state/siteUtil.js diff --git a/app/common/lib/historyUtil.js b/app/common/lib/historyUtil.js index 297dc04d48f..7a1bfc6e2e5 100644 --- a/app/common/lib/historyUtil.js +++ b/app/common/lib/historyUtil.js @@ -75,14 +75,15 @@ const prepareHistoryEntry = (siteDetail) => { return makeImmutable({ lastAccessedTime: time, - objectId: undefined, + objectId: siteDetail.get('objectId', null), title: siteDetail.get('title'), location: siteDetail.get('location'), partitionNumber: ~~siteDetail.get('partitionNumber', 0), count: 1, themeColor: siteDetail.get('themeColor'), favicon: siteDetail.get('favicon', siteDetail.get('icon')), - key: getKey(siteDetail) + key: getKey(siteDetail), + skipSync: siteDetail.get('skipSync', null) }) } diff --git a/app/common/state/bookmarkFoldersState.js b/app/common/state/bookmarkFoldersState.js index 426c2c3f938..a2445c29249 100644 --- a/app/common/state/bookmarkFoldersState.js +++ b/app/common/state/bookmarkFoldersState.js @@ -63,8 +63,9 @@ const bookmarkFoldersState = { key: key.toString(), parentFolderId: ~~folderDetails.get('parentFolderId', 0), partitionNumber: ~~folderDetails.get('partitionNumber', 0), - objectId: null, - type: siteTags.BOOKMARK_FOLDER + objectId: folderDetails.get('objectId', null), + type: siteTags.BOOKMARK_FOLDER, + skipSync: folderDetails.get('skipSync', null) }) state = state.setIn([STATE_SITES.BOOKMARK_FOLDERS, key.toString()], newFolder) diff --git a/app/common/state/bookmarksState.js b/app/common/state/bookmarksState.js index 3ff7a0b1e1f..61e31f49046 100644 --- a/app/common/state/bookmarksState.js +++ b/app/common/state/bookmarksState.js @@ -117,11 +117,12 @@ const bookmarksState = { location: bookmarkDetail.get('location'), parentFolderId: ~~bookmarkDetail.get('parentFolderId', 0), partitionNumber: ~~dataItem.get('partitionNumber', 0), - objectId: null, + objectId: bookmarkDetail.get('objectId', null), favicon: dataItem.get('favicon'), themeColor: dataItem.get('themeColor'), type: siteTags.BOOKMARK, - key: key + key: key, + skipSync: bookmarkDetail.get('skipSync', null) }) if (key === null) { @@ -204,8 +205,17 @@ const bookmarksState = { return state } + const syncEnabled = getSetting(settings.SYNC_ENABLED) === true const bookmarks = bookmarksState.getBookmarks(state) - .filter(bookmark => bookmark.get('parentFolderId') !== ~~parentFolderId) + .filter(bookmark => { + if (bookmark.get('parentFolderId') !== ~~parentFolderId) { + return true + } + if (syncEnabled) { + syncActions.removeSite(bookmark) + } + return false + }) return state.set(STATE_SITES.BOOKMARKS, bookmarks) }, diff --git a/app/sync.js b/app/sync.js index 56448191d39..92a2b04758b 100644 --- a/app/sync.js +++ b/app/sync.js @@ -25,6 +25,7 @@ const getSetting = require('../js/settings').getSetting const settings = require('../js/constants/settings') const extensions = require('./extensions') const {unescapeJSONPointer} = require('./common/lib/jsonUtil') +const bookmarkUtil = require('./common/lib/bookmarkUtil') const bookmarkFoldersUtil = require('./common/lib/bookmarkFoldersUtil') const bookmarkFoldersState = require('./common/state/bookmarkFoldersState') const bookmarksState = require('./common/state/bookmarksState') @@ -125,8 +126,15 @@ const appStoreChangeCallback = function (diffs) { const entryJS = entry.toJS() entryJS.objectId = entryJS.objectId || syncUtil.newObjectId(statePath) - sendSyncRecords(backgroundSender, action, - [isSite ? syncUtil.createSiteData(entryJS) : syncUtil.createSiteSettingsData(statePath[1], entryJS)]) + let record = null + if (type === STATE_SITES.BOOKMARKS || type === STATE_SITES.BOOKMARK_FOLDERS) { + record = syncUtil.createBookmarkData(entryJS) + } else if (type === STATE_SITES.HISTORY_SITES) { + record = syncUtil.createHistorySiteData(entryJS) + } else { + record = syncUtil.createSiteSettingsData(statePath[1], entryJS) + } + sendSyncRecords(backgroundSender, action, [record]) } } }) @@ -212,9 +220,10 @@ const dispatcherCallback = (action) => { return } switch (action.actionType) { + // Currently triggered only by bookmarksState and bookmarkFoldersState. case syncConstants.SYNC_REMOVE_SITE: sendSyncRecords(backgroundSender, writeActions.DELETE, - [syncUtil.createSiteData(action.item.toJS())]) + [syncUtil.createBookmarkData(action.item.toJS())]) break case syncConstants.SYNC_CLEAR_HISTORY: backgroundSender.send(syncMessages.DELETE_SYNC_CATEGORY, CATEGORY_MAP.historySite.categoryName) @@ -273,7 +282,7 @@ module.exports.onSyncReady = (isFirstRun, e) => { * Sync a bookmark that has not been synced yet, first syncing the parent * folder if needed. For folders, set and memoize folderId to ensure * consistent parentFolderObjectIds. - * Otherwise siteUtil.createSiteData() will generate new objectIds every + * Otherwise syncUtil.createBookmarkData() will generate new objectIds every * call; there's not enough time to dispatch id updates to appStore.sites. * @param {Immutable.Map} site */ @@ -300,13 +309,13 @@ module.exports.onSyncReady = (isFirstRun, e) => { if (!folderToObjectId[parentFolderId]) { const folderResult = bookmarkFoldersState.getFolder(appState, parentFolderId) if (!folderResult.isEmpty()) { - syncBookmark(folderResult[1], appState) + syncBookmark(folderResult, appState) } } bookmarkJS.parentFolderObjectId = folderToObjectId[parentFolderId] } - const record = syncUtil.createSiteData(bookmarkJS, appState) + const record = syncUtil.createBookmarkData(bookmarkJS, appState) const folderId = bookmark.get('folderId') if (typeof folderId === 'number') { folderToObjectId[folderId] = record.objectId @@ -320,10 +329,11 @@ module.exports.onSyncReady = (isFirstRun, e) => { const bookmarks = bookmarksState.getBookmarksWithFolders(appState, parentFolderId) bookmarks.forEach((bookmark) => { if (shouldSyncBookmark(bookmark) === true) { - syncBookmark(bookmark, appState) - } - if (bookmarkFoldersUtil.isFolder(bookmark)) { - syncBookmarkFolder(bookmark.get('folderId'), appState) + if (bookmarkUtil.isBookmark(bookmark)) { + syncBookmark(bookmark, appState) + } else if (bookmarkFoldersUtil.isFolder(bookmark)) { + syncBookmarkFolder(bookmark.get('folderId'), appState) + } } }) } @@ -338,7 +348,7 @@ module.exports.onSyncReady = (isFirstRun, e) => { // might not be synced. const siteSettings = appState.get('siteSettings').filter((value, key) => { - return !value.get('objectId') && syncUtil.isSyncable('siteSetting', value) + return !value.get('objectId') && syncUtil.isSyncableSiteSetting(value) }).toJS() if (siteSettings) { const siteSettingsData = Object.keys(siteSettings).map((item) => { diff --git a/js/state/siteUtil.js b/js/state/siteUtil.js deleted file mode 100644 index d8642302364..00000000000 --- a/js/state/siteUtil.js +++ /dev/null @@ -1,14 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public * License, v. 2.0. If a copy of the MPL was not distributed with this file, - * You can obtain one at http://mozilla.org/MPL/2.0/. */ - -'use strict' - -/** - * Determines if the site detail is a history entry. - * @param siteDetail The site detail to check. - * @return true if the site detail is a history entry. - */ -// TODO remove when sync is refactored -module.exports.isHistoryEntry = function (siteDetail) { - return false -} diff --git a/js/state/syncUtil.js b/js/state/syncUtil.js index 049836d4457..9bf4fe9fc05 100644 --- a/js/state/syncUtil.js +++ b/js/state/syncUtil.js @@ -7,7 +7,6 @@ const Immutable = require('immutable') const appActions = require('../actions/appActions') const crypto = require('crypto') const writeActions = require('../constants/sync/proto').actions -const siteUtil = require('./siteUtil') const {getSetting} = require('../settings') const {isDataUrl} = require('../lib/urlutil') const bookmarkUtil = require('../../app/common/lib/bookmarkUtil') @@ -58,7 +57,6 @@ const SITE_FIELDS = [ 'objectId', 'location', 'title', - 'tags', 'favicon', 'themeColor', 'lastAccessedTime', @@ -99,6 +97,24 @@ let objectToFolderMap = new Immutable.Map() // Used when sending folder records const folderToObjectMap = {} +const getBookmarkSiteDataFromRecord = (siteProps, appState, records, existingObjectData) => { + const newSiteProps = Object.assign({}, siteProps) + const existingFolderId = existingObjectData && existingObjectData.get('folderId') + if (existingFolderId) { + newSiteProps.folderId = existingFolderId + } + const parentFolderObjectId = newSiteProps.parentFolderObjectId + if (parentFolderObjectId && parentFolderObjectId.length > 0) { + newSiteProps.parentFolderId = + getFolderIdByObjectId(new Immutable.List(parentFolderObjectId), appState, records) + } else { + // Null or empty parentFolderObjectId on a record corresponds to + // a top-level bookmark. -1 indicates a hidden bookmark (Other bookmarks). + newSiteProps.parentFolderId = siteProps.hideInToolbar ? -1 : 0 + } + return newSiteProps +} + /** * Converts sync bookmark and history records into an object that can be * consumed by AppStore actions. @@ -118,7 +134,7 @@ module.exports.getSiteDataFromRecord = (record, appState, records) => { existingObjectData = existingObject && existingObject[1] } - const siteProps = Object.assign( + let siteProps = Object.assign( {}, existingObjectData && existingObjectData.toJS(), record.historySite, @@ -126,21 +142,13 @@ module.exports.getSiteDataFromRecord = (record, appState, records) => { record.bookmark && record.bookmark.site, {objectId} ) + // customTitle was previously used in browser-laptop + if (siteProps.customTitle) { + siteProps.title = siteProps.customTitle + } delete siteProps.customTitle if (record.objectData === 'bookmark') { - const existingFolderId = existingObjectData && existingObjectData.get('folderId') - if (existingFolderId) { - siteProps.folderId = existingFolderId - } - const parentFolderObjectId = siteProps.parentFolderObjectId - if (parentFolderObjectId && parentFolderObjectId.length > 0) { - siteProps.parentFolderId = - getFolderIdByObjectId(new Immutable.List(parentFolderObjectId), appState, records) - } else { - // Null or empty parentFolderObjectId on a record corresponds to - // a top-level bookmark. -1 indicates a hidden bookmark (Other bookmarks). - siteProps.parentFolderId = siteProps.hideInToolbar ? -1 : 0 - } + siteProps = getBookmarkSiteDataFromRecord(siteProps, appState, records, existingObjectData) } return new Immutable.Map(pickFields(siteProps, SITE_FIELDS)) } @@ -294,7 +302,7 @@ module.exports.applySyncRecords = (records) => { return true } if (record.objectData === 'bookmark') { - const siteData = module.exports.getSiteDataFromRecord(record, appState, records) + const siteData = module.exports.getSiteDataFromRecord(record, appState, records).set('skipSync', true) if (shouldAddRecord(record)) { if (record.bookmark.isFolder) { bufferAppAction(appActions.addBookmarkFolder, siteData) @@ -302,11 +310,13 @@ module.exports.applySyncRecords = (records) => { bufferAppAction(appActions.addBookmark, siteData) } } else if (shouldRemoveRecord(record)) { - const folderKey = bookmarkFoldersUtil.getKey(siteData) - // TODO: removeBookmarkFolder doesn't support List removal - setImmediate(() => { - appActions.removeBookmarkFolder(folderKey) - }) + if (record.bookmark.isFolder) { + const folderKey = bookmarkFoldersUtil.getKey(siteData) + bufferAppAction(appActions.removeBookmarkFolder, folderKey) + } else { + const siteKey = bookmarkUtil.getKey(siteData) + bufferAppAction(appActions.removeBookmark, siteKey) + } } } else if (record.objectData === 'historySite') { const siteData = module.exports.getSiteDataFromRecord(record, appState, records) @@ -343,8 +353,10 @@ module.exports.getExistingObject = (categoryName, syncRecord) => { let item switch (categoryName) { case 'BOOKMARKS': + item = module.exports.createBookmarkData(existingObjectData, appState) + break case 'HISTORY_SITES': - item = module.exports.createSiteData(existingObjectData, appState) + item = module.exports.createHistorySiteData(existingObjectData) break case 'PREFERENCES': const hostPattern = existingObjectKeyPath[existingObjectKeyPath.length - 1] @@ -500,19 +512,39 @@ module.exports.now = () => { } /** - * Checks whether an object is syncable as a record of the given type - * @param {string} type + * Checks whether an object is syncable as a bookmark. * @param {Immutable.Map} item * @returns {boolean} */ -module.exports.isSyncable = (type, item) => { - if (type === 'bookmark') { - return bookmarkUtil.isBookmark(item) || bookmarkFoldersUtil.isFolder(item) - } else if (type === 'siteSetting') { - for (let field in module.exports.siteSettingDefaults) { - if (item.has(field)) { - return true - } +module.exports.isSyncableBookmark = (site) => { + return bookmarkUtil.isBookmark(site) || bookmarkFoldersUtil.isFolder(site) +} + +/** + * Checks whether an object is syncable as a history site. + * @param {Immutable.Map} item + * @returns {boolean} + */ +module.exports.isSyncableHistorySite = (site) => { + if (!site) { + return false + } + const order = site.get('order') + const location = site.get('location') + return !order && + typeof location === 'string' && + !location.startsWith('about:') +} + +/** + * Checks whether an object is syncable as a site setting. + * @param {Immutable.Map} item + * @returns {boolean} + */ +module.exports.isSyncableSiteSetting = (item) => { + for (let field in module.exports.siteSettingDefaults) { + if (item.has(field)) { + return true } } return false @@ -552,13 +584,7 @@ const findOrCreateFolderObjectId = (folderId, appState) => { } } -/** - * Converts a site object into input for sendSyncRecords - * @param {Object} site - * @param {Immutable.Map} appState - * @returns {{name: string, value: object, objectId: Array.}} - */ -module.exports.createSiteData = (site, appState) => { +const pickSiteData = (site) => { const siteData = { location: '', title: '', @@ -571,69 +597,79 @@ module.exports.createSiteData = (site, appState) => { siteData[field] = site[field] } } + return siteData +} + +/** + * Converts a bookmark site object into input for sendSyncRecords. + * @param {Object} site + * @param {Immutable.Map} appState + * @returns {{name: string, value: object, objectId: Array.}} + */ +module.exports.createBookmarkData = (site, appState) => { + const siteData = pickSiteData(site) const immutableSite = Immutable.fromJS(site) - let name - let objectId - let parentFolderObjectId - let value - if (module.exports.isSyncable('bookmark', immutableSite)) { - name = 'bookmark' - const isFolder = bookmarkFoldersUtil.isFolder(immutableSite) - let siteKey - if (isFolder) { - siteKey = bookmarkFoldersUtil.getKey(immutableSite) || bookmarkFoldersUtil.getKey(Immutable.fromJS(siteData)) - } else { - siteKey = bookmarkUtil.getKey(immutableSite) || bookmarkUtil.getKey(Immutable.fromJS(siteData)) - } - if (siteKey === null) { - // May happen if this is called before the appStore object has its location - // field populated - console.log(`Ignoring entry because we can't create site key: ${JSON.stringify(site)}`) - return - } + const isFolder = bookmarkFoldersUtil.isFolder(immutableSite) + const collectionUtil = isFolder ? bookmarkFoldersUtil : bookmarkUtil + const siteKey = collectionUtil.getKey(immutableSite) || + collectionUtil.getKey(Immutable.fromJS(siteData)) + if (siteKey === null) { + // May happen if this is called before the appStore object has its location + // field populated + console.log(`createBookmarkData ignoring entry because we can't create site key: ${JSON.stringify(site)}`) + return + } - const sitesCollection = isFolder ? STATE_SITES.BOOKMARK_FOLDERS : STATE_SITES.BOOKMARKS - objectId = site.objectId || - folderToObjectMap[site.folderId] || - module.exports.newObjectId([sitesCollection, siteKey]) - parentFolderObjectId = site.parentFolderObjectId || - folderToObjectMap[site.parentFolderId] || - findOrCreateFolderObjectId(site.parentFolderId, appState) - value = { - site: siteData, - isFolder, - hideInToolbar: site.parentFolderId === -1, - parentFolderObjectId - } - } else if (siteUtil.isHistoryEntry(immutableSite)) { - // TODO create new function in historyUtil, because siteUtil.isHistoryEntry is deprecated - const siteKey = historyUtil.getKey(immutableSite) || historyUtil.getKey(Immutable.fromJS(siteData)) - if (siteKey === null) { - // May happen if this is called before the appStore object has its location - // field populated - console.log(`Ignoring entry because we can't create site key: ${JSON.stringify(site)}`) - return - } + const sitesCollection = isFolder ? STATE_SITES.BOOKMARK_FOLDERS : STATE_SITES.BOOKMARKS + const objectId = site.objectId || + folderToObjectMap[site.folderId] || + module.exports.newObjectId([sitesCollection, siteKey]) + if (!objectId) { + console.log(`Warning: createBookmarkData can't create site data: ${JSON.stringify(site)}`) + } + + const parentFolderObjectId = site.parentFolderObjectId || + folderToObjectMap[site.parentFolderId] || + findOrCreateFolderObjectId(site.parentFolderId, appState) + const value = { + site: siteData, + isFolder, + hideInToolbar: site.parentFolderId === -1, + parentFolderObjectId + } + return { + name: 'bookmark', + objectId, + value + } +} + +/** + * Converts a bookmark site object into input for sendSyncRecords. + * @param {Object} site + * @returns {{name: string, value: object, objectId: Array.}} + */ +module.exports.createHistorySiteData = (site) => { + const siteData = pickSiteData(site) + const siteKey = historyUtil.getKey(Immutable.fromJS(site)) || + historyUtil.getKey(Immutable.fromJS(siteData)) + if (siteKey === null) { + // May happen if this is called before the appStore object has its location + // field populated + console.log(`createHistorySiteData ignoring site because we can't create site key: ${JSON.stringify(site)}`) + return + } - objectId = site.objectId || module.exports.newObjectId([STATE_SITES.HISTORY_SITES, siteKey]) - name = 'historySite' - value = siteData + const objectId = site.objectId || module.exports.newObjectId([STATE_SITES.HISTORY_SITES, siteKey]) + if (!objectId) { + console.log(`Warning: createHistorySiteData can't create site data: ${JSON.stringify(site)}`) } - if (objectId) { - if (typeof site.folderId === 'number') { - folderToObjectMap[site.folderId] = objectId - } - if (typeof site.parentFolderId === 'number' && parentFolderObjectId) { - folderToObjectMap[site.parentFolderId] = parentFolderObjectId - } - return { - name, - objectId, - value - } + return { + name: 'historySite', + objectId, + value: siteData } - console.log(`Warning: Can't create site data: ${JSON.stringify(site)}`) } /** diff --git a/test/misc-components/syncTest.js b/test/misc-components/syncTest.js index 021d0a09f43..0ce04aabab4 100644 --- a/test/misc-components/syncTest.js +++ b/test/misc-components/syncTest.js @@ -442,7 +442,7 @@ describe('Syncing bookmarks', function () { }) it('create bookmark in folder', function * () { - const pageNthChild = 2 // TODO: This should be 1. See https://github.com/brave/sync/issues/104 + const pageNthChild = 1 const folderTitle = this.folder1Title const pageTitle = this.folder1Page1Title const folder = `[data-test-id="bookmarkToolbarButton"][title="${folderTitle}"]` @@ -454,7 +454,7 @@ describe('Syncing bookmarks', function () { }) it('update bookmark, moving it into the folder', function * () { - const pageNthChild = 1 // TODO: This should be 2 + const pageNthChild = 2 const folderTitle = this.folder1Title const pageTitle = this.folder1Page2Title const folder = `[data-test-id="bookmarkToolbarButton"][title="${folderTitle}"]` @@ -598,7 +598,7 @@ describe('Syncing bookmarks from an existing profile', function () { }) it('create bookmark in folder', function * () { - const pageNthChild = 2 // TODO: This should be 1 + const pageNthChild = 1 const folderTitle = this.folder1Title const pageTitle = this.folder1Page1Title const folder = `[data-test-id="bookmarkToolbarButton"][title="${folderTitle}"]` @@ -610,7 +610,7 @@ describe('Syncing bookmarks from an existing profile', function () { }) it('update bookmark, moving it into the folder', function * () { - const pageNthChild = 1 // TODO: This should be 2 + const pageNthChild = 2 const folderTitle = this.folder1Title const pageTitle = this.folder1Page2Title const folder = `[data-test-id="bookmarkToolbarButton"][title="${folderTitle}"]` diff --git a/test/unit/state/syncUtilTest.js b/test/unit/state/syncUtilTest.js index 3bb5c3fc4b2..b9439e02da4 100644 --- a/test/unit/state/syncUtilTest.js +++ b/test/unit/state/syncUtilTest.js @@ -7,7 +7,7 @@ const Immutable = require('immutable') const writeActions = require('../../../js/constants/sync/proto').actions // TODO re-enable when fixes @ayumi -describe.skip('syncUtil', () => { +describe('syncUtil', () => { let syncUtil let appAction let crypto @@ -54,42 +54,22 @@ describe.skip('syncUtil', () => { objectId, order: 9, folderId: 2, - customTitle: 'Folder1', // XXX: Android uses title whereas laptop uses customTitle parentFolderId: 1 } const appState = { - sites: { + bookmarkFolders: { '2': existingObject }, sync: { objectsById: { - [objectId.join('|')]: ['sites', '2'] + [objectId.join('|')]: ['bookmarkFolders', '2'] } } } - describe('with action', function () { - describe('when NOT writeActions.CREATE', function () { - it('looks up / returns existing object', function () { - const result = syncUtil.getSiteDataFromRecord(records[0], - Immutable.fromJS(appState), Immutable.fromJS(records)) - assert.deepEqual(result.existingObjectData.toJS(), existingObject) - }) - }) - - describe('when writeActions.CREATE', function () { - it('does not set existingObjectData', function () { - const recordsCopy = Object.assign({}, records[0]) - recordsCopy.action = writeActions.CREATE - const result = syncUtil.getSiteDataFromRecord(recordsCopy, - Immutable.fromJS(appState), Immutable.fromJS([recordsCopy])) - assert.equal(result.existingObjectData, undefined) - }) - }) - }) - - describe('with customTitle', function () { - it('keeps field if assigned value', function () { + // customTitle deprecated in browser-laptop, however we should support old records with it. + describe.skip('with customTitle', function () { + it('if assigned value, it copies field to title', function () { // deep copy and assign a different custom title const recordsCopy = Object.assign({}, records[0]) recordsCopy.bookmark = Object.assign({}, records[0].bookmark) @@ -97,14 +77,15 @@ describe.skip('syncUtil', () => { recordsCopy.bookmark.site.customTitle = 'demo value' const result = syncUtil.getSiteDataFromRecord(recordsCopy, Immutable.fromJS(appState), Immutable.fromJS([recordsCopy])) - assert.equal(result.siteDetail.has('customTitle'), true) - assert.equal(result.siteDetail.get('customTitle'), 'demo value') + assert.equal(result.has('customTitle'), false) + assert.equal(result.has('title'), true) + assert.equal(result.get('title'), 'demo value') }) it('deletes field if empty', function () { const result = syncUtil.getSiteDataFromRecord(records[0], Immutable.fromJS(appState), Immutable.fromJS(records)) - assert.equal(result.siteDetail.has('customTitle'), false) + assert.equal(result.has('customTitle'), false) }) }) @@ -113,8 +94,7 @@ describe.skip('syncUtil', () => { it('update bookmark parent folder to null -> overrides existing parent folder', () => { const result = syncUtil.getSiteDataFromRecord(records[0], Immutable.fromJS(appState), Immutable.fromJS(records)) - assert.equal(result.tag, 'bookmark-folder') - assert.deepEqual(result.siteDetail.toJS(), + assert.deepEqual(result.toJS(), { objectId, title: 'Folder1', @@ -122,12 +102,10 @@ describe.skip('syncUtil', () => { location: '', parentFolderId: 0, folderId: 2, - tags: ['bookmark-folder'], lastAccessedTime: 0, creationTime: 0 } ) - assert.deepEqual(result.existingObjectData.toJS(), existingObject) }) }) @@ -163,7 +141,13 @@ describe.skip('syncUtil', () => { }) }) - describe('isSyncable', function () { + describe('isSyncableBookmark', function () { + }) + + describe('isSyncableHistorySite', function () { + }) + + describe('isSyncableSiteSetting', function () { }) describe('newObjectId', function () { @@ -191,7 +175,7 @@ describe.skip('syncUtil', () => { }) }) - describe('createSiteData', function () { + describe('create{collection}SiteData', function () { const objectId = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15] const site = { favicon: 'https://calendar.google.com/googlecalendar/images/favicon_v2014_18.ico', @@ -199,7 +183,6 @@ describe.skip('syncUtil', () => { location: 'https://calendar.google.com/calendar/render#main_7', objectId, partitionNumber: 0, - tags: [], themeColor: 'rgb(255, 255, 255)', title: 'Google Calendar' } @@ -210,112 +193,104 @@ describe.skip('syncUtil', () => { favicon: 'https://calendar.google.com/googlecalendar/images/favicon_v2014_18.ico', location: 'https://calendar.google.com/calendar/render#main_7', title: 'Google Calendar', - customTitle: '', lastAccessedTime: 1484792353816, creationTime: 0 } } - it('history sites', () => { - assert.deepEqual(syncUtil.createSiteData(site), expectedSite) - }) - - // TODO: Not explicitly supported; falls back to history item - it('pinned sites', () => { - const pinnedSite = Object.assign({}, site, {tags: ['pinned']}) - assert.deepEqual(syncUtil.createSiteData(pinnedSite), expectedSite) - }) - - it('bookmarks', () => { - const bookmark = Object.assign({}, site, {tags: ['bookmark']}) - const expectedBookmark = { - name: 'bookmark', - objectId, - value: { - site: expectedSite.value, - isFolder: false, - hideInToolbar: false, - parentFolderObjectId: undefined + describe('createBookmarkData', function () { + it('bookmarks', () => { + const expectedBookmark = { + name: 'bookmark', + objectId, + value: { + site: expectedSite.value, + isFolder: false, + hideInToolbar: false, + parentFolderObjectId: undefined + } } - } - assert.deepEqual(syncUtil.createSiteData(bookmark), expectedBookmark) - }) + assert.deepEqual(syncUtil.createBookmarkData(site), expectedBookmark) + }) - it('bookmark with undefined custom title', () => { - const bookmark = Object.assign({}, site, {tags: ['bookmark'], customTitle: undefined}) - const newValue = Object.assign({}, expectedSite.value, {customTitle: undefined}) - const expectedBookmark = { - name: 'bookmark', - objectId, - value: { - site: newValue, - isFolder: false, - hideInToolbar: false, - parentFolderObjectId: undefined + it('bookmark with undefined custom title', () => { + const expectedBookmark = { + name: 'bookmark', + objectId, + value: { + site: expectedSite.value, + isFolder: false, + hideInToolbar: false, + parentFolderObjectId: undefined + } } - } - assert.deepEqual(syncUtil.createSiteData(bookmark), expectedBookmark) - }) + assert.deepEqual(syncUtil.createBookmarkData(site), expectedBookmark) + }) - it('bookmark containing data url', () => { - const bookmark = Object.assign({}, site, {tags: ['bookmark'], favicon: 'data:foo'}) - const newValue = Object.assign({}, expectedSite.value, {favicon: ''}) - const expectedBookmark = { - name: 'bookmark', - objectId, - value: { - site: newValue, - isFolder: false, - hideInToolbar: false, - parentFolderObjectId: undefined + it('bookmark containing data url', () => { + const bookmark = Object.assign({}, site, {favicon: 'data:foo'}) + const newValue = Object.assign({}, expectedSite.value, {favicon: ''}) + const expectedBookmark = { + name: 'bookmark', + objectId, + value: { + site: newValue, + isFolder: false, + hideInToolbar: false, + parentFolderObjectId: undefined + } } - } - assert.deepEqual(syncUtil.createSiteData(bookmark), expectedBookmark) - }) + assert.deepEqual(syncUtil.createBookmarkData(bookmark), expectedBookmark) + }) - it('bookmark in Other Bookmarks folder', () => { - const bookmark = Object.assign({}, site, {tags: ['bookmark'], parentFolderId: -1}) - const expectedBookmark = { - name: 'bookmark', - objectId, - value: { - site: expectedSite.value, - isFolder: false, - hideInToolbar: true, - parentFolderObjectId: undefined + it('bookmark in Other Bookmarks folder', () => { + const bookmark = Object.assign({}, site, {parentFolderId: -1}) + const expectedBookmark = { + name: 'bookmark', + objectId, + value: { + site: expectedSite.value, + isFolder: false, + hideInToolbar: true, + parentFolderObjectId: undefined + } } - } - assert.deepEqual(syncUtil.createSiteData(bookmark), expectedBookmark) + assert.deepEqual(syncUtil.createBookmarkData(bookmark), expectedBookmark) + }) }) - it('site without lastAccessedTime', () => { - const site = { - order: 1207, - count: 15, - partitionNumber: 0, - location: 'https://parsecpizzadelivery.com/', - title: "Parsec Pizza Delivery trailer - A pixelated deliver 'em up", - tags: [], - objectId: [0, 63, 197, 156, 48, 17, 112, 109, 247, 175, 79, 57, 151, 123, 29, 198], - themeColor: 'rgb(5, 5, 5)' - } - const expectedSite = { - name: 'historySite', - objectId: [0, 63, 197, 156, 48, 17, 112, 109, 247, 175, 79, 57, 151, 123, 29, 198], - value: { - creationTime: 0, - customTitle: '', - favicon: '', - lastAccessedTime: 0, + describe('createHistorySiteData', function () { + it('history sites', () => { + assert.deepEqual(syncUtil.createHistorySiteData(site), expectedSite) + }) + + it('site without lastAccessedTime', () => { + const site = { + order: 1207, + count: 15, + partitionNumber: 0, location: 'https://parsecpizzadelivery.com/', - title: "Parsec Pizza Delivery trailer - A pixelated deliver 'em up" + title: "Parsec Pizza Delivery trailer - A pixelated deliver 'em up", + objectId: [0, 63, 197, 156, 48, 17, 112, 109, 247, 175, 79, 57, 151, 123, 29, 198], + themeColor: 'rgb(5, 5, 5)' } - } - assert.deepEqual(syncUtil.createSiteData(site), expectedSite) + const expectedSite = { + name: 'historySite', + objectId: [0, 63, 197, 156, 48, 17, 112, 109, 247, 175, 79, 57, 151, 123, 29, 198], + value: { + creationTime: 0, + favicon: '', + lastAccessedTime: 0, + location: 'https://parsecpizzadelivery.com/', + title: "Parsec Pizza Delivery trailer - A pixelated deliver 'em up" + } + } + assert.deepEqual(syncUtil.createHistorySiteData(site), expectedSite) + }) }) - }) - describe('createSiteSettingsData', function () { + describe('createSiteSettingsData', function () { + }) }) describe('deepArrayify', function () {