Page MenuHomePhabricator

Allow the workboard backlog column to be reordered
ClosedPublic

Authored by epriestley on Aug 8 2014, 3:39 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 12, 5:49 PM
Unknown Object (File)
Oct 23 2024, 5:01 AM
Unknown Object (File)
Oct 21 2024, 11:20 PM
Unknown Object (File)
Oct 21 2024, 8:37 PM
Unknown Object (File)
Oct 21 2024, 2:57 PM
Unknown Object (File)
Oct 20 2024, 5:05 PM
Unknown Object (File)
Oct 20 2024, 4:28 PM
Unknown Object (File)
Oct 19 2024, 1:42 AM
Subscribers

Details

Summary

Fixes T5677.

  • Instead of using sequence == 0 to mean "this is the backlog column", flag the column explicitly.
  • Migrate existing sequence 0 columns to have the flag.
  • Add the flag when initializing or copying a board.
  • Remove special backlog logic when reordering columns.
Test Plan
  • Migrated columns, viewed some boards, they looked identical.
  • Reordered the backlog column a bunch of times (first, last, middle, dragged other stuff around).
  • Added tasks to a project, saw them show up in the reordered backlog.
  • Initialized a new board and saw a backlog column show up.
  • Copied an existing board and saw the backlog column come over.
  • Tried to hide a backlog column.

Diff Detail

Repository
rP Phabricator
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

epriestley retitled this revision from to Allow the workboard backlog column to be reordered.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
btrahan edited edge metadata.

Awesome!

src/applications/project/controller/PhabricatorProjectBoardViewController.php
169–180

I think $defaut_phid needs to be updated to? This code reads to me that the "0th position column gets all the tasks by default". That said, your test plan covers the case I'd expect to fail - having tasks show up in a "default" column that is not in position 0.

Maybe we're explicitly writing relationships for the "default" case now and this code can be delated? (i.e. on line 174 the isset() always returns true in modern times?)

This revision is now accepted and ready to land.Aug 8 2014, 10:49 PM
epriestley updated this revision to Diff 24543.

Closed by commit rP24a6eeb8d8a5 (authored by @epriestley).

src/applications/project/controller/PhabricatorProjectBoardViewController.php
169–180

Oops, jumped the gun on this. Nice catch! Your second paragraph is right (we should never be able to miss on $positions now, or if we can it's surprising / a bug) -- I'll clean this up.