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

sync should only upload records when important fields are changed #8415

Closed
diracdeltas opened this issue Apr 20, 2017 · 0 comments · Fixed by #8480
Closed

sync should only upload records when important fields are changed #8415

diracdeltas opened this issue Apr 20, 2017 · 0 comments · Fixed by #8480
Assignees
@diracdeltas
Labels
feature/sync QA/checked-Linux QA/checked-macOS QA/checked-Win64 QA/test-plan-specified release-notes/include
Milestone
0.15.300 (Frozen,...

Comments

@diracdeltas
Copy link
Member

diracdeltas commented Apr 20, 2017 •
edited by luixxiul
Loading

Test plan

#8480 (comment)

at minimum it should not be sending an update every time that lastAccessedTime or creationTime in a site record is changed.

cc @SergeyZhukovsky @jhreis - this should also be implemented on mobile if not already. it does not require any changes to brave/sync.
@diracdeltas diracdeltas added this to the 0.15.1 milestone Apr 20, 2017
@diracdeltas diracdeltas self-assigned this Apr 20, 2017
diracdeltas added a commit that referenced this issue Apr 25, 2017
@diracdeltas
refactor sync to use appStore change listener
4850878 
and only trigger sync when important site fields are modified.
fix #8415
may also fix brave/sync#31

test plan:
0. automated sync tests should pass
1. enable sync and go to https://example.com
2. bookmark it. you should see a message in the console indicating a record was sent.
3. close the page and visit it again. you should not see a message in the console.
4. unbookmark it. you should see a message in the console indicating a record was sent.
diracdeltas added a commit that referenced this issue Apr 25, 2017
@diracdeltas
refactor sync to use appStore change listener
6ef7e4e 
and only trigger sync when important site fields are modified.
fix #8415
may also fix brave/sync#31

test plan:
0. automated sync tests should pass
1. enable sync and go to https://example.com
2. bookmark it. you should see a message in the console indicating a record was sent.
3. close the page and visit it again. you should not see a message in the console.
4. unbookmark it. you should see a message in the console indicating a record was sent.
@diracdeltas diracdeltas mentioned this issue Apr 25, 2017
Merged
4 tasks
diracdeltas added a commit that referenced this issue Apr 25, 2017
@diracdeltas
refactor sync to use appStore change listener
e24a939 
and only trigger sync when important site fields are modified.
fix #8415
may also fix brave/sync#31

test plan:
0. automated sync tests should pass
1. enable sync and go to https://example.com
2. bookmark it. you should see a message in the console indicating a record was sent.
3. close the page and visit it again. you should not see a message in the console.
4. unbookmark it. you should see a message in the console indicating a record was sent.
diracdeltas added a commit that referenced this issue Apr 27, 2017
@diracdeltas
refactor sync to use appStore change listener
374dc62 
and only trigger sync when important site fields are modified.
fix #8415
may also fix brave/sync#31

test plan:
0. automated sync tests should pass
1. enable sync and go to https://example.com
2. bookmark it. you should see a message in the console indicating a record was sent.
3. close the page and visit it again. you should not see a message in the console.
4. unbookmark it. you should see a message in the console indicating a record was sent.
This was referenced May 2, 2017
Closed
Closed
Closed
Closed
@bbondy bbondy mentioned this issue May 26, 2017
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@diracdeltas diracdeltas

Labels
feature/sync QA/checked-Linux QA/checked-macOS QA/checked-Win64 QA/test-plan-specified release-notes/include
Projects
None yet
Milestone
0.15.300 (Frozen, only critical adds ...
Development

Successfully merging a pull request may close this issue.

refactor sync to use appStore change listener brave/browser-laptop
4 participants
@diracdeltas @alexwykoff @luixxiul @srirambv