Page MenuHomePhabricator

Fix an issue where editing cards on a workboard with implicit column ordering could reorder cards improperly
ClosedPublic

Authored by epriestley on Jul 24 2019, 6:11 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Apr 11, 4:28 AM
Unknown Object (File)
Thu, Apr 11, 4:28 AM
Unknown Object (File)
Thu, Apr 11, 4:28 AM
Unknown Object (File)
Thu, Apr 11, 4:28 AM
Unknown Object (File)
Wed, Apr 10, 6:29 PM
Unknown Object (File)
Sun, Apr 7, 10:29 AM
Unknown Object (File)
Sat, Apr 6, 11:14 PM
Unknown Object (File)
Sat, Mar 30, 5:14 PM
Subscribers
None

Details

Summary

Depends on D20680. Ref T4900. The "BoardLayoutEngine" operates on PHIDs without knowledge of the underlying objects, but this means it has to be sensitive to PHID input order when falling back to a default layout order.

We use "default layout order" on workboards which are sorted by "Natual" order but which have one or more cards which no user has ever reordered. For example, if you add 10 tasks to a project, then create a board, there's no existing order for those tasks in the "Backlog" column. The layout engine uses the input order to place them in the column, with the expectation that input order is ID/creation order, so new cards will end up on top.

I think this code never really made an explicit effort to guarantee that the LayoutEngine received objects in ID order, and it just sort of happened to by coincidence and good fortune. Some recent change has disrupted this, so the edit operation can end up with the PHIDs arranged in arbitrary order.

Explicitly put them in ID order so we always get an implicit default layout order to fall back to. Also, update to msortv().

Test Plan
  • Tagged several tasks with project X, a project without a board yet.
  • Created the project X workboard.
  • (Did not drag any tasks around on the project X board!)
  • Viewed the board in "Natural" order.

This creates a view of the board where tasks are ordered by implicit/virtual/input order. The expectation, and "view" behavior of this board, is that this order is "newest on top".

  • Edited one of the cards on the board, changing the title (don't reorder it!)
  • Before: page state synchronized with cards in arbitrary/random/different order.
  • After: page state synchronized with cards in the same order as before ("newest on top").

Diff Detail

Repository
rP Phabricator
Branch
pnotify2
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 23192
Build 31856: Run Core Tests
Build 31855: arc lint + arc unit

Event Timeline

Some recent change has disrupted this...

Actually, I think this was pre-existing. I made an attempt to git bisect the bad rev, but this was still a bug as long ago as April 15th.

This operation is somewhat unusual since you usually do some kind of drag on a board sooner or later, and as soon as you do we convert the positions from virtual positions to concrete positions.

This revision was not accepted when it landed; it landed in state Needs Review.Jul 30 2019, 8:17 PM
This revision was automatically updated to reflect the committed changes.