Skip to content
This repository has been archived by the owner. 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

Conversation

@ayumi
Copy link
Contributor

ayumi commented Feb 21, 2017

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

@ayumi ayumi requested a review from diracdeltas Feb 21, 2017
@@ -242,6 +274,14 @@ class SyncTab extends ImmutableComponent {
</div>
: null
}
{
this.isSetup
? <div class='syncClearData'>

This comment has been minimized.

Copy link
@diracdeltas

diracdeltas Feb 21, 2017

Member

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

app/sync.js Outdated
if (dispatcherCallback) {
dispatcherCallback({actionType: syncConstants.SYNC_DELETE_USER})
}
appActions.changeSetting(settings.SYNC_ENABLED, false)

This comment has been minimized.

Copy link
@diracdeltas

diracdeltas Feb 21, 2017

Member

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

This comment has been minimized.

Copy link
@ayumi

ayumi Feb 21, 2017

Author Contributor

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

This comment has been minimized.

Copy link
@diracdeltas

diracdeltas Feb 21, 2017

Member

that works too

@diracdeltas
Copy link
Member

diracdeltas commented Feb 21, 2017

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 ayumi force-pushed the feature/clear-sync-data branch from b6307f3 to 4fc15b4 Feb 22, 2017
@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
0 of 3 checks passed
0 of 3 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details
@ayumi ayumi deleted the feature/clear-sync-data branch Feb 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.