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

Option to clear Sync server data and local browser state #7349

Merged
merged 1 commit into from
Feb 22, 2017
Merged

Option to clear Sync server data and local browser state #7349

merged 1 commit into from
Feb 22, 2017

Conversation

ayumi
Copy link
Contributor

@ayumi ayumi commented Feb 21, 2017 •
edited
Loading

Fix #7275

Fix brave/sync#51

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).

Test Plan:

  1. Spawn a clean pyramid 0. Enable Sync and add bookmarks.
  2. Spawn a clean pyramid 1 (npm run start2). Add it to 0's Sync profile and see the bookmarks sync over.
  3. Save the Sync code words. We'll use this to demonstrate data was purged.
  4. In Pyramid 0, go to Preferences > Sync and click Reset data. In the console you should see "Deleting user!!".
  5. Close Pyramid 1, delete its user data, and spawn it clean again. Using the saved code words, try to re-add it to 0's deleted Sync profile. No bookmarks should sync over.
  6. Back in Pyramid 0, you can use Sync freshly. Create a new profile and you should get a new set of code words.

Pix
screen shot 2017-02-16 at 16 48 00
screen shot 2017-02-16 at 16 48 15

diracdeltas
diracdeltas reviewed Feb 21, 2017
View reviewed changes
app/renderer/components/preferences/syncTab.js Outdated
@@ -242,6 +274,14 @@ class SyncTab extends ImmutableComponent {
</div>
: null
}
{
this.isSetup
? <div class='syncClearData'>
Copy link
Member

@diracdeltas diracdeltas Feb 21, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

i think react is throwing a warning because it expects className here

diracdeltas
diracdeltas reviewed Feb 21, 2017
View reviewed changes
app/sync.js Outdated
if (dispatcherCallback) {
dispatcherCallback({actionType: syncConstants.SYNC_DELETE_USER})
}
appActions.changeSetting(settings.SYNC_ENABLED, false)
Copy link
Member

Choose a reason for hiding this comment

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

i would wait a short amount of time to call this to make sure that SYNC_DELETE_USER gets processed before the extension is disabled

Copy link
Contributor Author

Choose a reason for hiding this comment

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

could also add an IPC message for DELETED_USER and wait for that

Copy link
Member

Choose a reason for hiding this comment

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

that works too

@diracdeltas
Copy link
Member

test plan worked for me. 👍

@diracdeltas diracdeltas added this to the 0.13.5 milestone Feb 21, 2017
ayumi added a commit to brave/sync that referenced this pull request Feb 22, 2017
@ayumi
Add IPC webview to browser: DELETED_SYNC_USER
7f2bbb8 
To address
brave/browser-laptop#7349 (review)

Auditors: @diracdeltas
@ayumi
Copy link
Contributor Author

ayumi commented Feb 22, 2017

@diracdeltas have addressed comments.

deleting data requires sync to be enabled because it needs a valid AWS credential to issue the delete request. i've added a little message to explain:

screen shot 2017-02-21 at 21 29 03

@ayumi ayumi merged commit 8a3a133 into master Feb 22, 2017
@ayumi ayumi deleted the feature/clear-sync-data branch February 22, 2017 20:25
@ayumi ayumi mentioned this pull request Feb 22, 2017
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@diracdeltas diracdeltas diracdeltas approved these changes

Assignees
No one assigned
Labels
feature/sync
Projects
None yet
Milestone
0.13.5
Development

Successfully merging this pull request may close these issues.
2 participants
@ayumi @diracdeltas