recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

When selecting implicit column positions, actually create them

Summary:
Ref T4807. This is an alternative to D10179. The problem these diffs solve is that I want to be able to reorder a column's positions without having to load the actual objects, but that's difficutl because two positions may have the same sequence number (and I think it's good that we allow that, since it makes a bunch of other stuff way easier).

Instead of using the object ID (e.g., the task ID) to reorder positions with the same sequence, use the position itself. This is a little easier, is less ambiguous if columns eventually have several types of objects, and produces a better behavior when old objects are freshly added to a board. For example, if you tag `T300` with `#project`, this new rule will push it to the top of "Backlog" while the old rule might have buried it deep. I think this behavior is desirable and more "natural".

When creating a group of new rows, we do order the batch by ID, so a group of freshly-tagged objects float to the top togehter in ID order. This seems like the most natural rule, too.

Test Plan:
- Loaded some boards with implicit objects on them (freshly tagged tasks) and saw rows create.
- Verified new rows created in the right order.
- Dragged some tasks around.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4807

Differential Revision: https://secure.phabricator.com/D10180

epriestley 043e0db8 fdf6b562

+33 -5
-4
src/applications/maniphest/editor/ManiphestTransactionEditor.php
··· 205 205 // Remove all existing column positions on the board. 206 206 207 207 foreach ($positions as $position) { 208 - if (!$position->getID()) { 209 - // This is an ephemeral position, so don't try to destroy it. 210 - continue; 211 - } 212 208 $position->delete(); 213 209 } 214 210
+33 -1
src/applications/project/query/PhabricatorProjectColumnPositionQuery.php
··· 178 178 179 179 $positions = $table->loadAllFromArray($data); 180 180 181 + // Find the implied positions which don't exist yet. If there are any, 182 + // we're going to create them. 183 + $create = array(); 181 184 foreach ($positions as $position) { 182 185 if ($position->getColumnPHID() === null) { 183 - $position->makeEphemeral(); 184 186 $column_phid = idx($default_map, $position->getBoardPHID()); 185 187 $position->setColumnPHID($column_phid); 188 + 189 + $create[] = $position; 186 190 } 191 + } 192 + 193 + if ($create) { 194 + // If we're adding several objects to a column, insert the column 195 + // position objects in object ID order. This means that newly added 196 + // objects float to the top, and when a group of newly added objects 197 + // float up at the same time, the most recently created ones end up 198 + // highest in the list. 199 + 200 + $objects = id(new PhabricatorObjectQuery()) 201 + ->setViewer(PhabricatorUser::getOmnipotentUser()) 202 + ->withPHIDs(mpull($create, 'getObjectPHID')) 203 + ->execute(); 204 + $objects = mpull($objects, null, 'getPHID'); 205 + $objects = msort($objects, 'getID'); 206 + 207 + $create = mgroup($create, 'getObjectPHID'); 208 + $create = array_select_keys($create, array_keys($objects)) + $create; 209 + 210 + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); 211 + 212 + foreach ($create as $object_phid => $create_positions) { 213 + foreach ($create_positions as $create_position) { 214 + $create_position->save(); 215 + } 216 + } 217 + 218 + unset($unguarded); 187 219 } 188 220 189 221 return $positions;