@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.

Continue lifting column layout logic out of ColumnPositionQuery

Summary:
Ref T10010. See D15174. This gets rid of the "actually apply the change" callsite and moves it to layout engine.

Next up is to make the board view use the layout engine, then throw away all the whack code in ColumnPositionQuery, then move forward with D15171.

Test Plan:
- Dragged tasks within a column.
- Dragged tasks between columns.
- Dragged tasks to empty columns.
- Created a task in a column.
- Swapped board to priority sort, dragged a bunch of stuff all over.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10010

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

+274 -108
+59 -106
src/applications/maniphest/editor/ManiphestTransactionEditor.php
··· 200 200 'columnPHIDs')); 201 201 } 202 202 203 - $columns = id(new PhabricatorProjectColumnQuery()) 204 - ->setViewer($this->requireActor()) 205 - ->withPHIDs($new_phids) 206 - ->execute(); 207 - $columns = mpull($columns, null, 'getPHID'); 208 - 209 - $positions = id(new PhabricatorProjectColumnPositionQuery()) 210 - ->setViewer($this->requireActor()) 211 - ->withObjectPHIDs(array($object->getPHID())) 212 - ->withBoardPHIDs(array($board_phid)) 213 - ->execute(); 214 - 215 203 $before_phid = idx($xaction->getNewValue(), 'beforePHID'); 216 204 $after_phid = idx($xaction->getNewValue(), 'afterPHID'); 217 205 ··· 227 215 // object's position in the "natural" ordering, so we do need to update 228 216 // some rows. 229 217 230 - // Remove all existing column positions on the board. 218 + $object_phid = $object->getPHID(); 231 219 232 - foreach ($positions as $position) { 233 - $position->delete(); 234 - } 220 + // We're doing layout with the ominpotent viewer to make sure we don't 221 + // remove positions in columns that exist, but which the actual actor 222 + // can't see. 223 + $omnipotent_viewer = PhabricatorUser::getOmnipotentUser(); 235 224 236 - // Add the new column positions. 225 + $board_tasks = id(new ManiphestTaskQuery()) 226 + ->setViewer($omnipotent_viewer) 227 + ->withEdgeLogicPHIDs( 228 + PhabricatorProjectObjectHasProjectEdgeType::EDGECONST, 229 + PhabricatorQueryConstraint::OPERATOR_AND, 230 + array($board_phid)) 231 + ->execute(); 237 232 238 - foreach ($new_phids as $phid) { 239 - $column = idx($columns, $phid); 240 - if (!$column) { 241 - throw new Exception( 242 - pht('No such column "%s" exists!', $phid)); 243 - } 233 + $object_phids = mpull($board_tasks, 'getPHID'); 234 + $object_phids[] = $object_phid; 244 235 245 - // Load the other object positions in the column. Note that we must 246 - // skip implicit column creation to avoid generating a new position 247 - // if the target column is a backlog column. 236 + $engine = id(new PhabricatorBoardLayoutEngine()) 237 + ->setViewer($omnipotent_viewer) 238 + ->setBoardPHIDs(array($board_phid)) 239 + ->setObjectPHIDs($object_phids) 240 + ->executeLayout(); 248 241 249 - $other_positions = id(new PhabricatorProjectColumnPositionQuery()) 250 - ->setViewer($this->requireActor()) 251 - ->withColumns(array($column)) 252 - ->withBoardPHIDs(array($board_phid)) 253 - ->setSkipImplicitCreate(true) 254 - ->execute(); 255 - $other_positions = msort($other_positions, 'getOrderingKey'); 242 + // TODO: This logic needs to be revised if we legitimately support 243 + // multiple column positions. 256 244 257 - // Set up the new position object. We're going to figure out the 258 - // right sequence number and then persist this object with that 259 - // sequence number. 260 - $new_position = id(new PhabricatorProjectColumnPosition()) 261 - ->setBoardPHID($board_phid) 262 - ->setColumnPHID($column->getPHID()) 263 - ->setObjectPHID($object->getPHID()); 245 + // NOTE: When a task is newly created, it's implicitly added to the 246 + // backlog but we don't currently record that in the "$old_phids". Just 247 + // clean it up for now. 248 + $columns = $engine->getObjectColumns($board_phid, $object_phid); 249 + foreach ($columns as $column) { 250 + $engine->queueRemovePosition( 251 + $board_phid, 252 + $column->getPHID(), 253 + $object_phid); 254 + } 264 255 265 - $updates = array(); 266 - $sequence = 0; 256 + // Remove all existing column positions on the board. 257 + foreach ($old_phids as $column_phid) { 258 + $engine->queueRemovePosition( 259 + $board_phid, 260 + $column_phid, 261 + $object_phid); 262 + } 267 263 268 - // If we're just dropping this into the column without any specific 269 - // position information, put it at the top. 270 - if (!$before_phid && !$after_phid) { 271 - $new_position->setSequence($sequence)->save(); 272 - $sequence++; 273 - } 274 - 275 - foreach ($other_positions as $position) { 276 - $object_phid = $position->getObjectPHID(); 277 - 278 - // If this is the object we're moving before and we haven't 279 - // saved yet, insert here. 280 - if (($before_phid == $object_phid) && !$new_position->getID()) { 281 - $new_position->setSequence($sequence)->save(); 282 - $sequence++; 283 - } 284 - 285 - // This object goes here in the sequence; we might need to update 286 - // the row. 287 - if ($sequence != $position->getSequence()) { 288 - $updates[$position->getID()] = $sequence; 289 - } 290 - $sequence++; 291 - 292 - // If this is the object we're moving after and we haven't saved 293 - // yet, insert here. 294 - if (($after_phid == $object_phid) && !$new_position->getID()) { 295 - $new_position->setSequence($sequence)->save(); 296 - $sequence++; 297 - } 298 - } 299 - 300 - // We should have found a place to put it. 301 - if (!$new_position->getID()) { 302 - throw new Exception( 303 - pht('Unable to find a place to insert object on column!')); 264 + // Add new positions. 265 + foreach ($new_phids as $column_phid) { 266 + if ($before_phid) { 267 + $engine->queueAddPositionBefore( 268 + $board_phid, 269 + $column_phid, 270 + $object_phid, 271 + $before_phid); 272 + } else if ($after_phid) { 273 + $engine->queueAddPositionAfter( 274 + $board_phid, 275 + $column_phid, 276 + $object_phid, 277 + $after_phid); 278 + } else { 279 + $engine->queueAddPosition( 280 + $board_phid, 281 + $column_phid, 282 + $object_phid); 304 283 } 305 - 306 - // If we changed other objects' column positions, bulk reorder them. 284 + } 307 285 308 - if ($updates) { 309 - $position = new PhabricatorProjectColumnPosition(); 310 - $conn_w = $position->establishConnection('w'); 286 + $engine->applyPositionUpdates(); 311 287 312 - $pairs = array(); 313 - foreach ($updates as $id => $sequence) { 314 - // This is ugly because MySQL gets upset with us if it is 315 - // configured strictly and we attempt inserts which can't work. 316 - // We'll never actually do these inserts since they'll always 317 - // collide (triggering the ON DUPLICATE KEY logic), so we just 318 - // provide dummy values in order to get there. 319 - 320 - $pairs[] = qsprintf( 321 - $conn_w, 322 - '(%d, %d, "", "", "")', 323 - $id, 324 - $sequence); 325 - } 326 - 327 - queryfx( 328 - $conn_w, 329 - 'INSERT INTO %T (id, sequence, boardPHID, columnPHID, objectPHID) 330 - VALUES %Q ON DUPLICATE KEY UPDATE sequence = VALUES(sequence)', 331 - $position->getTableName(), 332 - implode(', ', $pairs)); 333 - } 334 - } 335 288 break; 336 289 default: 337 290 break;
+214 -1
src/applications/project/engine/PhabricatorBoardLayoutEngine.php
··· 10 10 private $objectColumnMap = array(); 11 11 private $boardLayout = array(); 12 12 13 + private $remQueue = array(); 14 + private $addQueue = array(); 15 + 13 16 public function setViewer(PhabricatorUser $viewer) { 14 17 $this->viewer = $viewer; 15 18 return $this; ··· 76 79 return array_select_keys($this->columnMap, $column_phids); 77 80 } 78 81 82 + public function queueRemovePosition( 83 + $board_phid, 84 + $column_phid, 85 + $object_phid) { 86 + 87 + $board_layout = idx($this->boardLayout, $board_phid, array()); 88 + $positions = idx($board_layout, $column_phid, array()); 89 + $position = idx($positions, $object_phid); 90 + 91 + if ($position) { 92 + $this->remQueue[] = $position; 93 + 94 + // If this position hasn't been saved yet, get it out of the add queue. 95 + if (!$position->getID()) { 96 + foreach ($this->addQueue as $key => $add_position) { 97 + if ($add_position === $position) { 98 + unset($this->addQueue[$key]); 99 + } 100 + } 101 + } 102 + } 103 + 104 + unset($this->boardLayout[$board_phid][$column_phid][$object_phid]); 105 + 106 + return $this; 107 + } 108 + 109 + public function queueAddPositionBefore( 110 + $board_phid, 111 + $column_phid, 112 + $object_phid, 113 + $before_phid) { 114 + 115 + return $this->queueAddPositionRelative( 116 + $board_phid, 117 + $column_phid, 118 + $object_phid, 119 + $before_phid, 120 + true); 121 + } 122 + 123 + public function queueAddPositionAfter( 124 + $board_phid, 125 + $column_phid, 126 + $object_phid, 127 + $after_phid) { 128 + 129 + return $this->queueAddPositionRelative( 130 + $board_phid, 131 + $column_phid, 132 + $object_phid, 133 + $after_phid, 134 + false); 135 + } 136 + 137 + public function queueAddPosition( 138 + $board_phid, 139 + $column_phid, 140 + $object_phid) { 141 + return $this->queueAddPositionRelative( 142 + $board_phid, 143 + $column_phid, 144 + $object_phid, 145 + null, 146 + true); 147 + } 148 + 149 + private function queueAddPositionRelative( 150 + $board_phid, 151 + $column_phid, 152 + $object_phid, 153 + $relative_phid, 154 + $is_before) { 155 + 156 + $board_layout = idx($this->boardLayout, $board_phid, array()); 157 + $positions = idx($board_layout, $column_phid, array()); 158 + 159 + // Check if the object is already in the column, and remove it if it is. 160 + $object_position = idx($positions, $object_phid); 161 + unset($positions[$object_phid]); 162 + 163 + if (!$object_position) { 164 + $object_position = id(new PhabricatorProjectColumnPosition()) 165 + ->setBoardPHID($board_phid) 166 + ->setColumnPHID($column_phid) 167 + ->setObjectPHID($object_phid); 168 + } 169 + 170 + $found = false; 171 + if (!$positions) { 172 + $object_position->setSequence(0); 173 + } else { 174 + foreach ($positions as $position) { 175 + if (!$found) { 176 + if ($relative_phid === null) { 177 + $is_match = true; 178 + } else { 179 + $position_phid = $position->getObjectPHID(); 180 + $is_match = ($relative_phid == $position_phid); 181 + } 182 + 183 + if ($is_match) { 184 + $found = true; 185 + 186 + $sequence = $position->getSequence(); 187 + 188 + if (!$is_before) { 189 + $sequence++; 190 + } 191 + 192 + $object_position->setSequence($sequence++); 193 + 194 + if (!$is_before) { 195 + // If we're inserting after this position, continue the loop so 196 + // we don't update it. 197 + continue; 198 + } 199 + } 200 + } 201 + 202 + if ($found) { 203 + $position->setSequence($sequence++); 204 + $this->addQueue[] = $position; 205 + } 206 + } 207 + } 208 + 209 + if ($relative_phid && !$found) { 210 + throw new Exception( 211 + pht( 212 + 'Unable to find object "%s" in column "%s" on board "%s".', 213 + $relative_phid, 214 + $column_phid, 215 + $board_phid)); 216 + } 217 + 218 + $this->addQueue[] = $object_position; 219 + 220 + $positions[$object_phid] = $object_position; 221 + $positions = msort($positions, 'getOrderingKey'); 222 + 223 + $this->boardLayout[$board_phid][$column_phid] = $positions; 224 + 225 + return $this; 226 + } 227 + 228 + public function applyPositionUpdates() { 229 + foreach ($this->remQueue as $position) { 230 + if ($position->getID()) { 231 + $position->delete(); 232 + } 233 + } 234 + $this->remQueue = array(); 235 + 236 + $adds = array(); 237 + $updates = array(); 238 + 239 + foreach ($this->addQueue as $position) { 240 + $id = $position->getID(); 241 + if ($id) { 242 + $updates[$id] = $position; 243 + } else { 244 + $adds[] = $position; 245 + } 246 + } 247 + $this->addQueue = array(); 248 + 249 + $table = new PhabricatorProjectColumnPosition(); 250 + $conn_w = $table->establishConnection('w'); 251 + 252 + $pairs = array(); 253 + foreach ($updates as $id => $position) { 254 + // This is ugly because MySQL gets upset with us if it is configured 255 + // strictly and we attempt inserts which can't work. We'll never actually 256 + // do these inserts since they'll always collide (triggering the ON 257 + // DUPLICATE KEY logic), so we just provide dummy values in order to get 258 + // there. 259 + 260 + $pairs[] = qsprintf( 261 + $conn_w, 262 + '(%d, %d, "", "", "")', 263 + $id, 264 + $position->getSequence()); 265 + } 266 + 267 + if ($pairs) { 268 + queryfx( 269 + $conn_w, 270 + 'INSERT INTO %T (id, sequence, boardPHID, columnPHID, objectPHID) 271 + VALUES %Q ON DUPLICATE KEY UPDATE sequence = VALUES(sequence)', 272 + $table->getTableName(), 273 + implode(', ', $pairs)); 274 + } 275 + 276 + foreach ($adds as $position) { 277 + $position->save(); 278 + } 279 + 280 + return $this; 281 + } 282 + 79 283 private function loadBoards() { 80 284 $viewer = $this->getViewer(); 81 285 $board_phids = $this->getBoardPHIDs(); ··· 113 317 114 318 private function loadPositions(array $boards) { 115 319 $viewer = $this->getViewer(); 320 + 321 + $object_phids = $this->getObjectPHIDs(); 322 + if (!$object_phids) { 323 + return array(); 324 + } 116 325 117 326 $positions = id(new PhabricatorProjectColumnPositionQuery()) 118 327 ->setViewer($viewer) 119 328 ->withBoardPHIDs(array_keys($boards)) 120 - ->withObjectPHIDs($this->getObjectPHIDs()) 329 + ->withObjectPHIDs($object_phids) 121 330 ->execute(); 122 331 $positions = msort($positions, 'getOrderingKey'); 123 332 $positions = mgroup($positions, 'getBoardPHID'); ··· 150 359 foreach ($positions as $key => $position) { 151 360 $column_phid = $position->getColumnPHID(); 152 361 if (empty($columns[$column_phid])) { 362 + $this->remQueue[] = $position; 153 363 unset($positions[$key]); 154 364 } 155 365 } ··· 161 371 ->setColumnPHID($default_phid) 162 372 ->setObjectPHID($object_phid) 163 373 ->setSequence(0); 374 + 375 + $this->addQueue[] = $new_position; 376 + 164 377 $positions = array( 165 378 $new_position, 166 379 );
+1 -1
src/applications/project/storage/PhabricatorProjectColumnPosition.php
··· 41 41 } 42 42 43 43 public function getOrderingKey() { 44 - if (!$this->getID()) { 44 + if (!$this->getID() && !$this->getSequence()) { 45 45 return 0; 46 46 } 47 47