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

Add FETCH_SYNC_DEVICES to get all device records #71

Merged
merged 1 commit into from Apr 10, 2017
Merged

Conversation

@ayumi
Copy link
Contributor

ayumi commented Apr 7, 2017

to support #41

Browser requests FETCH_SYNC_DEVICES when toggling sync from off -> on
It's a rebranded FETCH_SYNC_RECORDS which returns device records from all time (responses go through the existing down stream, GET_EXISTING_OBJECTS etc)

@ayumi ayumi requested a review from diracdeltas Apr 7, 2017
ayumi added a commit to brave/browser-laptop that referenced this pull request Apr 7, 2017
Requires brave/sync#71

Fixes brave/sync#41

Test plan:
1. Start Brave and enable Sync, naming the device "Pyramid 1".
2. `npm run start2` Brave and add it to Pyramid 1's Sync profile and name it "Pyramid 2".
3. Pyramid 2's device list should show both pyramids.
4. In Pyramid 1, wait a bit, 30-60s.
5. Pyramid 1's device list should also show both pyramids.
@ayumi ayumi mentioned this pull request Apr 7, 2017
3 of 4 tasks complete
@@ -54,6 +54,12 @@ const messages = {
FETCH_SYNC_RECORDS: _, /* @param Array.<string> categoryNames, @param {number} startAt (in seconds or milliseconds), @param {boolean=} limitResponse true to limit response to 1000 records */
/**
* webview -> browser
* sent to fetch all sync devices. webview responds with

This comment has been minimized.

Copy link
@diracdeltas

diracdeltas Apr 7, 2017

Member

shouldn't this be browser -> webview?

This comment has been minimized.

Copy link
@ayumi

ayumi Apr 10, 2017

Author Contributor

👍

})
const lastObject = s3Objects[s3Objects.length - 1]
const lastRecordTimestamp = s3Helper.parseS3Key(lastObject.Key).timestamp
ipc.send(messages.GET_EXISTING_OBJECTS, 'PREFERENCES', jsRecords, lastRecordTimestamp)

This comment has been minimized.

Copy link
@diracdeltas

diracdeltas Apr 7, 2017

Member

it makes slightly more sense to me if this were RESOLVED_SYNC_RECORDS instead of GET_EXISTING_OBJECTS, since sync doesn't appear to need or use the existing objects in this case.

Copy link
Member

diracdeltas left a comment

see above

@ayumi ayumi force-pushed the feature/list-devices branch 2 times, most recently from 1dc45da to e16af29 Apr 10, 2017
Copy link
Contributor Author

ayumi left a comment

@diracdeltas made some changes– can you re-review?

@@ -54,6 +54,12 @@ const messages = {
FETCH_SYNC_RECORDS: _, /* @param Array.<string> categoryNames, @param {number} startAt (in seconds or milliseconds), @param {boolean=} limitResponse true to limit response to 1000 records */
/**
* webview -> browser
* sent to fetch all sync devices. webview responds with

This comment has been minimized.

Copy link
@ayumi

ayumi Apr 10, 2017

Author Contributor

👍

@@ -53,6 +53,12 @@ const messages = {
*/
FETCH_SYNC_RECORDS: _, /* @param Array.<string> categoryNames, @param {number} startAt (in seconds or milliseconds), @param {boolean=} limitResponse true to limit response to 1000 records */
/**
* browser -> webview
* sent to fetch all sync devices. webview responds with
* GET_EXISTING_OBJECTS etc, as in FETCH_SYNC_RECORDS.

This comment has been minimized.

Copy link
@diracdeltas

diracdeltas Apr 10, 2017

Member

s/GET_EXISTING_OBJECTS/RESOLVED_SYNC_RECORDS

This comment has been minimized.

Copy link
@ayumi

ayumi Apr 10, 2017

Author Contributor

👍

@ayumi ayumi force-pushed the feature/list-devices branch from e16af29 to abf46c3 Apr 10, 2017
ayumi added a commit to brave/browser-laptop that referenced this pull request Apr 10, 2017
Requires brave/sync#71

Fixes brave/sync#41

Test plan:
1. Start Brave and enable Sync, naming the device "Pyramid 1".
2. `npm run start2` Brave and add it to Pyramid 1's Sync profile and name it "Pyramid 2".
3. Pyramid 2's device list should show both pyramids.
4. In Pyramid 1, wait a bit, 30-60s.
5. Pyramid 1's device list should also show both pyramids.
@ayumi ayumi merged commit f5fbc72 into staging Apr 10, 2017
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@ayumi ayumi deleted the feature/list-devices branch Apr 10, 2017
ayumi added a commit to brave/browser-laptop that referenced this pull request Apr 11, 2017
Requires brave/sync#71

Fixes brave/sync#41

Test plan:
1. Start Brave and enable Sync, naming the device "Pyramid 1".
2. `npm run start2` Brave and add it to Pyramid 1's Sync profile and name it "Pyramid 2".
3. Pyramid 2's device list should show both pyramids.
4. In Pyramid 1, wait a bit, 30-60s.
5. Pyramid 1's device list should also show both pyramids.
ayumi added a commit to brave/browser-laptop that referenced this pull request Apr 11, 2017
Requires brave/sync#71

Fixes brave/sync#41

Test plan:
1. Start Brave and enable Sync, naming the device "Pyramid 1".
2. `npm run start2` Brave and add it to Pyramid 1's Sync profile and name it "Pyramid 2".
3. Pyramid 2's device list should show both pyramids.
4. In Pyramid 1, wait a bit, 30-60s.
5. Pyramid 1's device list should also show both pyramids.
ayumi added a commit to brave/browser-laptop that referenced this pull request Apr 11, 2017
Requires brave/sync#71

Fixes brave/sync#41

Test plan:
1. Start Brave and enable Sync, naming the device "Pyramid 1".
2. `npm run start2` Brave and add it to Pyramid 1's Sync profile and name it "Pyramid 2".
3. Pyramid 2's device list should show both pyramids.
4. In Pyramid 1, wait a bit, 30-60s.
5. Pyramid 1's device list should also show both pyramids.
bsclifton added a commit to brave/browser-laptop that referenced this pull request Apr 11, 2017
Requires brave/sync#71

Fixes brave/sync#41

Test plan:
1. Start Brave and enable Sync, naming the device "Pyramid 1".
2. `npm run start2` Brave and add it to Pyramid 1's Sync profile and name it "Pyramid 2".
3. Pyramid 2's device list should show both pyramids.
4. In Pyramid 1, wait a bit, 30-60s.
5. Pyramid 1's device list should also show both pyramids.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.