diff --git a/components/brave_sync/brave_sync_service_impl.cc b/components/brave_sync/brave_sync_service_impl.cc index 697f251a19f5..b125155f533f 100644 --- a/components/brave_sync/brave_sync_service_impl.cc +++ b/components/brave_sync/brave_sync_service_impl.cc @@ -469,6 +469,7 @@ BraveSyncServiceImpl::PrepareResolvedPreferences(const RecordsList& records) { void BraveSyncServiceImpl::OnResolvedPreferences(const RecordsList& records) { const std::string this_device_id = sync_prefs_->GetThisDeviceId(); bool this_device_deleted = false; + bool contains_only_one_device = false; auto sync_devices = sync_prefs_->GetSyncDevices(); for (const auto &record : records) { @@ -486,12 +487,14 @@ void BraveSyncServiceImpl::OnResolvedPreferences(const RecordsList& records) { (record->deviceId == this_device_id && record->action == jslib::SyncRecord::Action::A_DELETE && actually_merged); + contains_only_one_device = sync_devices->size() < 2 && + record->action == jslib::SyncRecord::Action::A_DELETE; } } // for each device sync_prefs_->SetSyncDevices(*sync_devices); - if (this_device_deleted) + if (this_device_deleted || contains_only_one_device) OnResetSync(); } diff --git a/components/brave_sync/brave_sync_service_impl.h b/components/brave_sync/brave_sync_service_impl.h index cdd56139d40c..06c1bad4b38c 100644 --- a/components/brave_sync/brave_sync_service_impl.h +++ b/components/brave_sync/brave_sync_service_impl.h @@ -20,6 +20,8 @@ FORWARD_DECLARE_TEST(BraveSyncServiceTest, BookmarkDeleted); FORWARD_DECLARE_TEST(BraveSyncServiceTest, GetSyncWords); FORWARD_DECLARE_TEST(BraveSyncServiceTest, GetSeed); FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnDeleteDevice); +FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnDeleteDeviceWhenOneDevice); +FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnDeleteDeviceWhenSelfDeleted); FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnResetSync); FORWARD_DECLARE_TEST(BraveSyncServiceTest, ClientOnGetInitData); FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnGetInitData); @@ -94,6 +96,9 @@ class BraveSyncServiceImpl FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, GetSyncWords); FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, GetSeed); FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, OnDeleteDevice); + FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, OnDeleteDeviceWhenOneDevice); + FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, + OnDeleteDeviceWhenSelfDeleted); FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, OnResetSync); FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, ClientOnGetInitData); FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, OnSaveBookmarksBaseOrder); diff --git a/components/brave_sync/brave_sync_service_unittest.cc b/components/brave_sync/brave_sync_service_unittest.cc index 3a0f9a99fa6a..5cc2e70a56da 100644 --- a/components/brave_sync/brave_sync_service_unittest.cc +++ b/components/brave_sync/brave_sync_service_unittest.cc @@ -323,7 +323,7 @@ TEST_F(BraveSyncServiceTest, GetSettingsAndDevices) { EXPECT_FALSE(settings->sync_bookmarks_); EXPECT_FALSE(settings->sync_settings_); EXPECT_FALSE(settings->sync_history_); - EXPECT_EQ(devices->devices_.size(), 0u); + EXPECT_EQ(devices->size(), 0u); } ); sync_service()->GetSettingsAndDevices(callback1); @@ -425,24 +425,88 @@ TEST_F(BraveSyncServiceTest, OnDeleteDevice) { EXPECT_CALL(*sync_client(), SendSyncRecords("PREFERENCES", ContainsDeviceRecord(SyncRecord::Action::A_DELETE, "device3"))).Times(1); sync_service()->OnDeleteDevice("3"); + + RecordsList resolved_records; + auto resolved_record = SyncRecord::Clone(*records.at(2)); + resolved_record->action = jslib::SyncRecord::Action::A_DELETE; + resolved_records.push_back(std::move(resolved_record)); + EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(1); + sync_service()->OnResolvedPreferences(resolved_records); + + auto devices_final = sync_service()->sync_prefs_->GetSyncDevices(); + EXPECT_TRUE(DevicesContains(devices_final.get(), "1", "device1")); + EXPECT_TRUE(DevicesContains(devices_final.get(), "2", "device2")); + EXPECT_FALSE(DevicesContains(devices_final.get(), "3", "device3")); +} + +TEST_F(BraveSyncServiceTest, OnDeleteDeviceWhenOneDevice) { + RecordsList records; + records.push_back(SimpleDeviceRecord( + jslib::SyncRecord::Action::A_CREATE, + "1", "device1")); + records.push_back(SimpleDeviceRecord( + jslib::SyncRecord::Action::A_CREATE, + "2", "device2")); + EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(1); + sync_service()->OnResolvedPreferences(records); + + auto devices = sync_service()->sync_prefs_->GetSyncDevices(); + + EXPECT_TRUE(DevicesContains(devices.get(), "1", "device1")); + EXPECT_TRUE(DevicesContains(devices.get(), "2", "device2")); + + using brave_sync::jslib::SyncRecord; EXPECT_CALL(*sync_client(), SendSyncRecords("PREFERENCES", ContainsDeviceRecord(SyncRecord::Action::A_DELETE, "device2"))).Times(1); sync_service()->OnDeleteDevice("2"); RecordsList resolved_records; - auto resolved_record1 = SyncRecord::Clone(*records.at(2)); - resolved_record1->action = jslib::SyncRecord::Action::A_DELETE; - resolved_records.push_back(std::move(resolved_record1)); - auto resolved_record2 = SyncRecord::Clone(*records.at(1)); - resolved_record2->action = jslib::SyncRecord::Action::A_DELETE; - resolved_records.push_back(std::move(resolved_record2)); + auto resolved_record = SyncRecord::Clone(*records.at(1)); + resolved_record->action = jslib::SyncRecord::Action::A_DELETE; + resolved_records.push_back(std::move(resolved_record)); + EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(3); + sync_service()->OnResolvedPreferences(resolved_records); + + auto devices_final = sync_service()->sync_prefs_->GetSyncDevices(); + EXPECT_FALSE(DevicesContains(devices_final.get(), "1", "device1")); + EXPECT_FALSE(DevicesContains(devices_final.get(), "2", "device2")); + + EXPECT_FALSE(sync_service()->IsSyncConfigured()); +} + +TEST_F(BraveSyncServiceTest, OnDeleteDeviceWhenSelfDeleted) { + RecordsList records; + records.push_back(SimpleDeviceRecord( + jslib::SyncRecord::Action::A_CREATE, + "1", "device1")); + records.push_back(SimpleDeviceRecord( + jslib::SyncRecord::Action::A_CREATE, + "2", "device2")); EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(1); + sync_service()->OnResolvedPreferences(records); + + auto devices = sync_service()->sync_prefs_->GetSyncDevices(); + + EXPECT_TRUE(DevicesContains(devices.get(), "1", "device1")); + EXPECT_TRUE(DevicesContains(devices.get(), "2", "device2")); + + using brave_sync::jslib::SyncRecord; + EXPECT_CALL(*sync_client(), SendSyncRecords("PREFERENCES", + ContainsDeviceRecord(SyncRecord::Action::A_DELETE, "device1"))).Times(1); + sync_service()->OnDeleteDevice("1"); + + RecordsList resolved_records; + auto resolved_record = SyncRecord::Clone(*records.at(0)); + resolved_record->action = jslib::SyncRecord::Action::A_DELETE; + resolved_records.push_back(std::move(resolved_record)); + EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(3); sync_service()->OnResolvedPreferences(resolved_records); auto devices_final = sync_service()->sync_prefs_->GetSyncDevices(); - EXPECT_TRUE(DevicesContains(devices_final.get(), "1", "device1")); + EXPECT_FALSE(DevicesContains(devices_final.get(), "1", "device1")); EXPECT_FALSE(DevicesContains(devices_final.get(), "2", "device2")); - EXPECT_FALSE(DevicesContains(devices_final.get(), "3", "device3")); + + EXPECT_FALSE(sync_service()->IsSyncConfigured()); } TEST_F(BraveSyncServiceTest, OnResetSync) { diff --git a/components/brave_sync/sync_devices.h b/components/brave_sync/sync_devices.h index 9b153434ff6a..10c35135c970 100644 --- a/components/brave_sync/sync_devices.h +++ b/components/brave_sync/sync_devices.h @@ -42,6 +42,7 @@ class SyncDevices { std::unique_ptr ToValue() const; std::unique_ptr ToValueArrOnly() const; std::string ToJson() const; + size_t size() const { return devices_.size(); } void FromJson(const std::string &str_json); void Merge(const SyncDevice& device, int action, bool* actually_merged); diff --git a/components/brave_sync/ui/actions/sync_actions.ts b/components/brave_sync/ui/actions/sync_actions.ts index 4640c625513f..d74a81a12bef 100644 --- a/components/brave_sync/ui/actions/sync_actions.ts +++ b/components/brave_sync/ui/actions/sync_actions.ts @@ -50,22 +50,9 @@ export const onSetupNewToSync = (thisDeviceName: string) => { return action(types.SYNC_ON_SETUP_NEW_TO_SYNC, { thisDeviceName }) } -/** - * Dispatches a message telling the back-end that the user requested the QR code - */ -export const onRequestQRCode = () => { - return action(types.SYNC_ON_REQUEST_QR_CODE) -} - export const onGenerateQRCodeImageSource = (imageSource: string) => { return action(types.SYNC_ON_GENERATE_QR_CODE_IMAGE_SOURCE, { imageSource }) } -/** - * Dispatches a message telling the back-end that the user requested the sync words - */ -export const onRequestSyncWords = () => { - return action(types.SYNC_ON_REQUEST_SYNC_WORDS) -} /** * Dispatches a message telling the back-end that the user removed a given device diff --git a/components/brave_sync/ui/components/enabledContent.tsx b/components/brave_sync/ui/components/enabledContent.tsx index b79af12f442d..d41fbcc927e0 100644 --- a/components/brave_sync/ui/components/enabledContent.tsx +++ b/components/brave_sync/ui/components/enabledContent.tsx @@ -60,17 +60,6 @@ export default class SyncEnabledContent extends React.PureComponent { if (!devices) { return diff --git a/components/brave_sync/ui/components/modals/addNewChainCameraOption.tsx b/components/brave_sync/ui/components/modals/addNewChainCameraOption.tsx index 2b01d7630484..f2b5aecf69ed 100644 --- a/components/brave_sync/ui/components/modals/addNewChainCameraOption.tsx +++ b/components/brave_sync/ui/components/modals/addNewChainCameraOption.tsx @@ -105,9 +105,9 @@ export default class AddNewChainCameraOptionModal extends React.PureComponent { } } - componentDidUpdate () { - // once this screen is rendered and component is updated, - // request sync qr code and words so it can be seen immediately - const { seedQRImageSource, syncWords } = this.props.syncData - if (!syncWords) { - this.props.actions.onRequestSyncWords() - } - if (!seedQRImageSource) { - this.props.actions.onRequestQRCode() - } - } - onUserNoticedError = () => { this.props.actions.resetSyncSetupError() this.props.onClose() } onClickClose = () => { - const { devices } = this.props.syncData + const { devices, isSyncConfigured } = this.props.syncData // sync is enabled when at least 2 devices are in the chain. // this modal works both with sync enabled and disabled states. // in case user opens it in the enabled content screen, // check there are 2 devices in chain before reset - if (devices.length < 2) { + if (isSyncConfigured && devices.length < 2) { this.props.actions.onSyncReset() } this.props.onClose() diff --git a/components/brave_sync/ui/components/modals/scanCode.tsx b/components/brave_sync/ui/components/modals/scanCode.tsx index e1b7382c126a..9718ab736b64 100644 --- a/components/brave_sync/ui/components/modals/scanCode.tsx +++ b/components/brave_sync/ui/components/modals/scanCode.tsx @@ -96,9 +96,9 @@ export default class ScanCodeModal extends React.PureComponent { type='accent' size='medium' onClick={onClose} - disabled={syncData.devices.length < 2} + disabled={!syncData.isSyncConfigured} text={ - syncData.devices.length < 2 + !syncData.isSyncConfigured ? getLocale('lookingForDevice') : getLocale('ok') } diff --git a/components/brave_sync/ui/constants/sync_types.ts b/components/brave_sync/ui/constants/sync_types.ts index 3570e8835810..ac14364769ad 100644 --- a/components/brave_sync/ui/constants/sync_types.ts +++ b/components/brave_sync/ui/constants/sync_types.ts @@ -8,9 +8,7 @@ export const enum types { SYNC_ON_HAVE_SYNC_WORDS = '@@sync/SYNC_ON_HAVE_SYNC_WORDS', SYNC_ON_HAVE_SEED_FOR_QR_CODE = '@@sync/SYNC_ON_HAVE_SEED_FOR_QR_CODE', SYNC_ON_SETUP_NEW_TO_SYNC = '@@sync/SYNC_ON_SETUP_NEW_TO_SYNC', - SYNC_ON_REQUEST_QR_CODE = '@@sync/SYNC_ON_REQUEST_QR_CODE', SYNC_ON_GENERATE_QR_CODE_IMAGE_SOURCE = '@@sync/SYNC_ON_GENERATE_QR_CODE_IMAGE_SOURCE', - SYNC_ON_REQUEST_SYNC_WORDS = '@@sync/SYNC_ON_REQUEST_SYNC_WORDS', SYNC_ON_REMOVE_DEVICE = '@@sync/SYNC_ON_REMOVE_DEVICE', SYNC_ON_RESET = '@@sync/SYNC_ON_RESET', SYNC_BOOKMARKS = '@@sync/SYNC_BOOKMARKS', diff --git a/components/brave_sync/ui/reducers/sync_reducer.ts b/components/brave_sync/ui/reducers/sync_reducer.ts index 70418c33d947..24fed6521156 100644 --- a/components/brave_sync/ui/reducers/sync_reducer.ts +++ b/components/brave_sync/ui/reducers/sync_reducer.ts @@ -44,6 +44,20 @@ const syncReducer: Reducer = (state: Sync.State | undefi syncSavedSiteSettings: payload.settings.sync_settings, syncBrowsingHistory: payload.settings.sync_history } + + if (state.isSyncConfigured && !state.syncWords) { + chrome.send('needSyncWords') + } + if (state.isSyncConfigured && !state.seedQRImageSource) { + chrome.send('needSyncQRcode') + } + + // if this device is deleted or only one device on sync chain, + // BraveSyncService will do reset but we must clear + // previous sync state + if (!state.isSyncConfigured) { + state = { ...storage.defaultState } + } break case types.SYNC_ON_HAVE_SEED_FOR_QR_CODE: @@ -68,14 +82,6 @@ const syncReducer: Reducer = (state: Sync.State | undefi chrome.send('setupSyncNewToSync', [payload.thisDeviceName]) break - case types.SYNC_ON_REQUEST_QR_CODE: - chrome.send('needSyncQRcode') - break - - case types.SYNC_ON_REQUEST_SYNC_WORDS: - chrome.send('needSyncWords') - break - case types.SYNC_ON_RESET: chrome.send('resetSync') // sync is reset. clear all data @@ -86,18 +92,6 @@ const syncReducer: Reducer = (state: Sync.State | undefi if (typeof payload.id === 'undefined' || typeof payload.deviceName === 'undefined') { break } - - // if the device removed is the this device, reset sync - if (payload.id === state.thisDeviceId) { - state = { ...storage.defaultState } - chrome.send('resetSync') - break - } - - state = { - ...state, - devices: [ ...state.devices.filter((device: Sync.Devices) => device.id !== payload.id) ] - } chrome.send('deleteDevice', [payload.id]) break diff --git a/components/test/brave_sync_ui/actions/sync_actions_test.ts b/components/test/brave_sync_ui/actions/sync_actions_test.ts index fbddb9fb70f1..b21b9dc95305 100644 --- a/components/test/brave_sync_ui/actions/sync_actions_test.ts +++ b/components/test/brave_sync_ui/actions/sync_actions_test.ts @@ -58,10 +58,6 @@ describe('sync_actions', () => { }) }) - it('onRequestQRCode', () => { - expect(actions.onRequestQRCode).toEqual(expect.any(Function)) - }) - it('onGenerateQRCodeImageSource', () => { expect(actions.onGenerateQRCodeImageSource('someImageConvertedToBase64')).toEqual({ type: types.SYNC_ON_GENERATE_QR_CODE_IMAGE_SOURCE, @@ -70,10 +66,6 @@ describe('sync_actions', () => { }) }) - it('onRequestSyncWords', () => { - expect(actions.onRequestSyncWords).toEqual(expect.any(Function)) - }) - it('onRemoveDevice', () => { expect(actions.onRemoveDevice(1337)).toEqual({ type: types.SYNC_ON_REMOVE_DEVICE, diff --git a/components/test/brave_sync_ui/reducers/sync_reducer_test.ts b/components/test/brave_sync_ui/reducers/sync_reducer_test.ts index faffced5ef38..59cb67856372 100644 --- a/components/test/brave_sync_ui/reducers/sync_reducer_test.ts +++ b/components/test/brave_sync_ui/reducers/sync_reducer_test.ts @@ -31,14 +31,6 @@ describe.skip('syncReducer', () => { // TODO }) - describe('SYNC_ON_REQUEST_QR_CODE', () => { - // TODO - }) - - describe('SYNC_ON_REQUEST_SYNC_WORDS', () => { - // TODO - }) - describe('SYNC_ON_RESET', () => { // TODO })