Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[iOS-Sync] Delete Other Devices (Uplift to 1.18.x) #7576

Closed
wants to merge 1 commit into from

Conversation

Brandon-T
Copy link
Contributor

@Brandon-T Brandon-T commented Jan 13, 2021

Implementation Details

  • Adds support for deleting other devices from the sync chain.
  • Adds support for the current device being deleted from the chain, by another device.
  • Fixes missing brave_services_key

Resolves brave/brave-ios#3194 and brave/brave-ios#3195

Submitter Checklist:

  • There is a ticket for my issue.
  • Used Github auto-closing keywords in the commit message.
  • Wrote a good PR/commit description
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed).
  • Requested a security/privacy review as needed.

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

…s was removed).

Adding back deleting other devices using the 1.20.x API (profile_helper).
Changed ResetSync to use the brave_sync::ResetSync from profile_helper.
@Brandon-T Brandon-T changed the base branch from master to 1.18.x January 13, 2021 00:02
@bsclifton bsclifton changed the title [iOS-Sync] Delete Other Devices [iOS-Sync] Delete Other Devices (Uplift to 1.18.x) Jan 13, 2021
auto device_value = base::Value::FromUniquePtrValue(device->ToValue());
bool is_current_device =
local_device_info ? local_device_info->guid() == device->guid() : false;
device_value.SetBoolKey("isCurrentDevice", is_current_device);
device_value.SetStringKey("guid", device->guid());
device_value.SetBoolKey("supportsSelfDelete",
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't find where supportsSelfDelete is used neither in branch ios/sync-delete-devices nor in https://github.com/brave/brave-ios . Or this is supposed in upcoming PRs?

Copy link
Member

Choose a reason for hiding this comment

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

It would decide whether we allow users to delete remote devices from device list

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, so this is for future use?

Copy link
Contributor

@AlexeyBarabash AlexeyBarabash left a comment

Choose a reason for hiding this comment

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

Looks good

@Brandon-T Brandon-T closed this Jan 13, 2021
@kjozwiak
Copy link
Member

kjozwiak commented Jan 13, 2021

Quick context: Closed as we're going to base 1.23 off of 1.19.x via #7583 and not 1.18.x.

@Brandon-T Brandon-T deleted the ios/sync-delete-devices branch December 16, 2022 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Sync] Crash Deleting bookmarks
4 participants