Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Commit

Permalink
do not delete objectIds when sync is reset
Browse files Browse the repository at this point in the history
fix #7405
  • Loading branch information
diracdeltas committed Feb 28, 2017
1 parent 0dc505a commit aff4cf5
Showing 1 changed file with 0 additions and 8 deletions.
8 changes: 0 additions & 8 deletions js/stores/appStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -942,14 +942,6 @@ const handleAppAction = (action) => {
const sessionStore = require('../../app/sessionStore')
const syncDefault = Immutable.fromJS(sessionStore.defaultAppState().sync)
appState = appState.set('sync', syncDefault)
appState.get('sites').forEach((site, key) => {
if (!site.has('objectId')) { return }
appState = appState.setIn(['sites', key], site.delete('objectId'))
})
appState.get('siteSettings').forEach((site, key) => {
if (!site.has('objectId')) { return }
appState = appState.setIn(['siteSettings', key], site.delete('objectId'))
})
break
case appConstants.APP_SHOW_DOWNLOAD_DELETE_CONFIRMATION:
appState = appState.set('deleteConfirmationVisible', true)
Expand Down

6 comments on commit aff4cf5

@ayumi
Copy link
Contributor

@ayumi ayumi commented on aff4cf5 Feb 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@diracdeltas deleting objectIds is important here because that's how syncBookmark() determines to do the initial sync. if the user comes back to sync in +3 months, they would like all their existing bookmarks to be synced to a new profile.

@diracdeltas
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ayumi what if instead of deleting the objectId, we add a field to indicate which profile this was synced for in this code block? then if the user is syncing to a new profile, the bookmarks are re-synced and assigned new object IDs. otherwise nothing happens.

i'm assuming this is only needed for bookmarks right now

@ayumi
Copy link
Contributor

@ayumi ayumi commented on aff4cf5 Feb 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@diracdeltas i like it

@diracdeltas
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sweet will do

@ayumi
Copy link
Contributor

@ayumi ayumi commented on aff4cf5 Feb 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we set the sync profile field here or whenever syncing bookmarks?

@diracdeltas
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think here is ok, it is only a problem when sync is reset

Please sign in to comment.