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

Sync is not tracking & merging changes between pyramids which creates a loss of hierarchy #8454

Closed
alexwykoff opened this issue Apr 23, 2017 · 21 comments · Fixed by brave/sync#84 or #9187

Comments

@alexwykoff
Copy link
Contributor

alexwykoff commented Apr 23, 2017

Test plan

#9187 (comment)


  • Did you search for similar issues before submitting this one?
    Yes

  • Describe the issue you encountered:
    When adding bookmarks and folders to Pyramid 0 prior to adding Pyramid 1 to the sync group, those changes were not being tracked appropriately so Pyramid 1 does not have a matching hierarchy to Pyramid 0.

  • Platform (Win7, 8, 10? macOS? Linux distro?):
    OS X for both.

  • Brave Version (revision SHA):
    Pyramid 0 - 0.15.0 RC3 (clean profile)
    Pyramid 1 - 0.14.1 (current prod, upgraded from 0.14.0)

  • Steps to reproduce:

    1. Pyramid 0 - With a clean profile, launch 0.15.0, create a sync group, create a bookmark, then create a folder, then move the bookmark into the folder. Create one more bookmark.
    2. Pyramid 1 - Join the sync group of Pyramid 0. After sync is complete, create a bookmark.
  • Actual result:
    Pyramid 0 - (from left to right on bookmarks bar) Bookmark folder with first bookmark in folder, second bookmark next to folder, Pyramid 1's bookmark next to second bookmark.
    Pyramid 1 - (from left to right on bookmarks bar) Pyramid 0's first bookmark, Pyramid 0's bookmark folder (empty), Pyramid 0's second bookmark, Pyramid 1's bookmark.

  • Expected result:
    They should be identical and should match Pyramid 0.

  • Will the steps above reproduce in a fresh profile? If not what other info can be added?
    Yes

  • Is this an issue in the currently released version?
    Not checked yet.

  • Can this issue be consistently reproduced?
    Not checked yet.

  • Extra QA steps:
    1.
    2.
    3.

  • Screenshot if needed:
    Pyramid 0:

screen shot 2017-04-23 at 5 34 33 pm

Pyramid 1:
screen shot 2017-04-23 at 5 34 47 pm

  • Any related issues:
@diracdeltas
Copy link
Member

in the screenshots above, "Pyramid 1's bookmark" appears to actually be a bookmarks folder. Is that correct?

@alexwykoff
Copy link
Contributor Author

indeed it is a folder

@diracdeltas
Copy link
Member

I confirmed that the test plan works as expected with brave/sync#84

@alexwykoff
Copy link
Contributor Author

alexwykoff commented May 25, 2017

I was able to reproduce this issue in 0.15.308 as pyramid 0 and 0.15.2 as pyramid 1.

STR:

  1. With clean profiles on both machines, start sync on pyramid 0, then start sync on pyramid 1.
  2. I created a folder on pyramid 0, it appeared on pyramid 1, but also duplicated on pyramid 0.
  3. I created a folder on pyramid 1, it appeared on pyramid 0.
  4. I added a bookmark on pyramid 0, it appeared on pyramid 1.
  5. I added a bookmark on pyramid 1, it appeared in pyramid 0.
  6. For pyramid 0, I imported all of my info from Chrome. The Brave instances on both devices became unresponsive. Folder structure was not maintained on pyramid 1.

duplication

@diracdeltas
Copy link
Member

@alexwykoff i wouldn't necessarily expect it to work with 0.15.2 since that still uses the buggy sync code before this fix

@alexwykoff
Copy link
Contributor Author

Confirmed the issue with 0.15.308 to 0.15.308, video documentation is available via company drive.

@alexwykoff
Copy link
Contributor Author

Loss of hierarchy has boomeranged back to pyramid 0. Pyramid 1 is missing at least 1 bookmark.
Details in photo:
Bookmarks Manager page, searched for circus on both machines 1 hour after import.
img_6336

@alexwykoff alexwykoff reopened this May 26, 2017
@alexwykoff alexwykoff modified the milestones: 0.16.200, 0.15.300 May 26, 2017
@alexwykoff alexwykoff added this to the 0.16.100 milestone May 26, 2017
@ayumi
Copy link
Contributor

ayumi commented Jun 2, 2017

PR is approved – need to pull into somewhere @bsclifton @alexwykoff

@diracdeltas
Copy link
Member

diracdeltas commented Jun 2, 2017

yeah the fix has been approved already; see above link

@NejcZdovc
Copy link
Contributor

if PR is ready for merge please add ready-for-merge label to it and one of the release team will merge it in all branches that are necessary

@diracdeltas
Copy link
Member

@NejcZdovc i understand that but i was asking what milestone this should be moved into. 0.16.x or 0.17.x at the latest IMO

@NejcZdovc
Copy link
Contributor

I would say 0.17, because 0.16 should be c59 and regression only I guess

@diracdeltas
Copy link
Member

diracdeltas commented Jun 2, 2017

i just tested and found that the duplicating-folders issue is a regression that exists in 0.15.310. if you create a bookmark folder with sync enabled in the last release, the folder will be duplicated after 1 minute.

i believe the regression was introduced with 8b7c008, so it may have also been in 0.15.2. i don't think it existed before that.

given that nobody has noticed this problem until now, it may be ok for 0.17.x.

@bsclifton
Copy link
Member

Moving to 0.17.x (will cherry-pick the merge into there)

@bsclifton bsclifton modified the milestones: 0.17.x (Frozen, only critical adds from here), 0.18.x Jun 2, 2017
@bsclifton bsclifton modified the milestones: 0.16.x (Frozen, only critical adds from here), 0.17.x (Frozen, only critical adds from here) Jun 5, 2017
@bsclifton
Copy link
Member

Moved to 0.16.x

@alexwykoff
Copy link
Contributor Author

Confirmed still an issue in 0.16.4.

STR:
0. Delete the /brave folders on either machine to ensure clean profiles

  1. On pyramid 0 - start a sync group
  2. On pyramid 1 - join the sync group
  3. On pyramid 0 - import bookmarks
  4. On pyramid 1 - wait for results

screen shot 2017-06-07 at 4 29 39 pm

@alexwykoff alexwykoff reopened this Jun 7, 2017
@bsclifton bsclifton modified the milestones: 0.17.x (Frozen, only critical adds from here), 0.16.x (Frozen, only critical adds from here) Jun 7, 2017
@diracdeltas
Copy link
Member

diracdeltas commented Jun 7, 2017

@alexwykoff this issue is getting very confusing. Originally I was addressing sync hierarchy between two pyramids. Then it was re-opened to address the duplicating folders issue on a single pyramid. It appears your latest issue is about importing bookmarks, which is more related to #8892 than this.

Please confirm that the STR for the original issue you posted passes. If it does, then this can be closed.

@alexwykoff
Copy link
Contributor Author

Confirmed original STR are solved, sorry for the confusion @diracdeltas 😸

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.