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

Remove unused code in ManiphestReportController::renderBurn()

Summary:
`ManiphestReportController` (which renders global `/maniphest/report/burn/`) wastes CPU cycles. Thus remove useless code.

It constructs table rows never to be shown: The code calculates rows and adds them via `$table = new AphrontTableView($rows)`, then defines `$panel = new PHUIObjectBoxView()` and does `$panel->setTable($table)` but a bit later `$panel = id(new PhabricatorProjectBurndownChartEngine())` overwrites that table and all its data.
This re-definition of `$panel` was introduced in rP5c1b91ab457db9f3db10d8cc5e07831512645ebb.

Its code creates a `PhabricatorProjectBurndownChartEngine` anyway, similar to `PhabricatorProjectReportsController`. So none of those expensive database queries are needed at all. Variables like `list($burn_x, $burn_y) = $this->buildSeries($data);` are never read since rPf8ebc71b8f217ed156f416ddb4cd028dcaa28174.

Bug: T16005

Test Plan:
1. Make sure `/applications/view/PhabricatorFactApplication/` is installed
2. Go to `/maniphest/report/burn/` for global view, go to some URL like `/maniphest/report/burn/?project=PHID-PROJ-1234567890abcdef` for per-project view
3. See that the "Burnup Rate" chart is rendered in the same way before and after applying this patch
4. Optionally, run `./bin/cache purge --all` (but seems not relevant here) and reload
5. Optionally, go to `/project/reports/1/` and as expected the burndown chart is the same as in step 2

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T16005

Differential Revision: https://we.phorge.it/D25902

+1 -348
+1 -348
src/applications/maniphest/controller/ManiphestReportController.php
··· 92 92 $handle = $handles[$project_phid]; 93 93 } 94 94 95 - $xtable = new ManiphestTransaction(); 96 - $conn = $xtable->establishConnection('r'); 97 - 98 - // Get legacy data: Querying the task transaction table is only needed for 99 - // code before rPd321cc81 got merged on 2017-11-22. 100 - if ($project_phid) { 101 - $legacy_joins = qsprintf( 102 - $conn, 103 - 'JOIN %T t ON x.objectPHID = t.phid 104 - JOIN %T p ON p.src = t.phid AND p.type = %d AND p.dst = %s', 105 - id(new ManiphestTask())->getTableName(), 106 - PhabricatorEdgeConfig::TABLE_NAME_EDGE, 107 - PhabricatorProjectObjectHasProjectEdgeType::EDGECONST, 108 - $project_phid); 109 - } else { 110 - $legacy_joins = qsprintf($conn, ''); 111 - } 112 - 113 - $legacy_data = queryfx_all( 114 - $conn, 115 - 'SELECT x.transactionType, x.oldValue, x.newValue, x.dateCreated 116 - FROM %T x %Q 117 - WHERE transactionType IN (%Ls) 118 - ORDER BY x.dateCreated ASC', 119 - $xtable->getTableName(), 120 - $legacy_joins, 121 - array( 122 - ManiphestTaskStatusTransaction::TRANSACTIONTYPE, 123 - ManiphestTaskMergedIntoTransaction::TRANSACTIONTYPE, 124 - )); 125 - 126 - // Remove any actual legacy status transactions which take status from 127 - // `null` to any open status. 128 - foreach ($legacy_data as $key => $row) { 129 - if ($row['transactionType'] != 'status') { 130 - continue; 131 - } 132 - 133 - $oldv = trim($row['oldValue'], '"'); 134 - $newv = trim($row['newValue'], '"'); 135 - 136 - // If this is a status change, preserve it. 137 - if ($oldv != 'null') { 138 - continue; 139 - } 140 - 141 - // If this task was created directly into a closed status, preserve 142 - // the transaction. 143 - if (!ManiphestTaskStatus::isOpenStatus($newv)) { 144 - continue; 145 - } 146 - 147 - // If this is a legacy "create" transaction, discard it in favor of the 148 - // synthetic transaction to be created below. 149 - unset($legacy_data[$key]); 150 - } 151 - 152 - // Since rPd321cc81, after the move to EditEngine, we no longer create a 153 - // "status" transaction if a task is created directly into the default 154 - // status. This likely impacted API/email tasks after 2016 and all other 155 - // tasks after deploying the Phorge codebase from 2017-11-22. 156 - // Until Facts can fix this properly, use the task creation dates to 157 - // generate synthetic transactions which look like the older transactions 158 - // that this page expects. 159 - 160 - $default_status = ManiphestTaskStatus::getDefaultStatus(); 161 - $duplicate_status = ManiphestTaskStatus::getDuplicateStatus(); 162 - 163 - if ($project_phid) { 164 - $synth_joins = qsprintf( 165 - $conn, 166 - 'JOIN %T p ON p.src = t.phid AND p.type = %d AND p.dst = %s', 167 - PhabricatorEdgeConfig::TABLE_NAME_EDGE, 168 - PhabricatorProjectObjectHasProjectEdgeType::EDGECONST, 169 - $project_phid); 170 - } else { 171 - $synth_joins = qsprintf($conn, ''); 172 - } 173 - 174 - // Build synthetic transactions which take status from `null` to the 175 - // default value. 176 - $synth_data = queryfx_all( 177 - $conn, 178 - 'SELECT t.dateCreated FROM %T t %Q', 179 - id(new ManiphestTask())->getTableName(), 180 - $synth_joins); 181 - foreach ($synth_data as $key => $synth_row) { 182 - $synth_data[$key] = array( 183 - 'transactionType' => 'status', 184 - 'oldValue' => null, 185 - 'newValue' => $default_status, 186 - 'dateCreated' => $synth_row['dateCreated'], 187 - ); 188 - } 189 - 190 - // Merge the synthetic transactions into the legacy transactions. 191 - $data = array_merge($synth_data, $legacy_data); 192 - $data = array_values($data); 193 - $data = isort($data, 'dateCreated'); 194 - 195 - $stats = array(); 196 - $day_buckets = array(); 197 - 198 - $open_tasks = array(); 199 - 200 - foreach ($data as $key => $row) { 201 - switch ($row['transactionType']) { 202 - case ManiphestTaskStatusTransaction::TRANSACTIONTYPE: 203 - // NOTE: Hack to avoid json_decode(). 204 - $oldv = $row['oldValue']; 205 - if ($oldv !== null) { 206 - $oldv = trim($oldv, '"'); 207 - } 208 - $newv = trim($row['newValue'], '"'); 209 - break; 210 - case ManiphestTaskMergedIntoTransaction::TRANSACTIONTYPE: 211 - // NOTE: Merging a task does not generate a "status" transaction. 212 - // We pretend it did. Note that this is not always accurate: it is 213 - // possible to merge a task which was previously closed, but this 214 - // fake transaction always counts a merge as a closure. 215 - $oldv = $default_status; 216 - $newv = $duplicate_status; 217 - break; 218 - } 219 - 220 - if ($oldv == 'null') { 221 - $old_is_open = false; 222 - } else { 223 - $old_is_open = ManiphestTaskStatus::isOpenStatus($oldv); 224 - } 225 - 226 - $new_is_open = ManiphestTaskStatus::isOpenStatus($newv); 227 - 228 - $is_open = ($new_is_open && !$old_is_open); 229 - $is_close = ($old_is_open && !$new_is_open); 230 - 231 - $data[$key]['_is_open'] = $is_open; 232 - $data[$key]['_is_close'] = $is_close; 233 - 234 - if (!$is_open && !$is_close) { 235 - // This is either some kind of bogus event, or a resolution change 236 - // (e.g., resolved -> invalid). Just skip it. 237 - continue; 238 - } 239 - 240 - $day_bucket = phabricator_format_local_time( 241 - $row['dateCreated'], 242 - $viewer, 243 - 'Yz'); 244 - $day_buckets[$day_bucket] = $row['dateCreated']; 245 - if (empty($stats[$day_bucket])) { 246 - $stats[$day_bucket] = array( 247 - 'open' => 0, 248 - 'close' => 0, 249 - ); 250 - } 251 - $stats[$day_bucket][$is_close ? 'close' : 'open']++; 252 - } 253 - 254 - $template = array( 255 - 'open' => 0, 256 - 'close' => 0, 257 - ); 258 - 259 - $rows = array(); 260 - $rowc = array(); 261 - $last_month = null; 262 - $last_month_epoch = null; 263 - $last_week = null; 264 - $last_week_epoch = null; 265 - $week = null; 266 - $month = null; 267 - 268 - $last = last_key($stats) - 1; 269 - $period = $template; 270 - 271 - foreach ($stats as $bucket => $info) { 272 - $epoch = $day_buckets[$bucket]; 273 - 274 - $week_bucket = phabricator_format_local_time( 275 - $epoch, 276 - $viewer, 277 - 'YW'); 278 - if ($week_bucket != $last_week) { 279 - if ($week) { 280 - $rows[] = $this->formatBurnRow( 281 - pht('Week of %s', phabricator_date($last_week_epoch, $viewer)), 282 - $week); 283 - $rowc[] = 'week'; 284 - } 285 - $week = $template; 286 - $last_week = $week_bucket; 287 - $last_week_epoch = $epoch; 288 - } 289 - 290 - $month_bucket = phabricator_format_local_time( 291 - $epoch, 292 - $viewer, 293 - 'Ym'); 294 - if ($month_bucket != $last_month) { 295 - if ($month) { 296 - $rows[] = $this->formatBurnRow( 297 - phabricator_format_local_time($last_month_epoch, $viewer, 'F, Y'), 298 - $month); 299 - $rowc[] = 'month'; 300 - } 301 - $month = $template; 302 - $last_month = $month_bucket; 303 - $last_month_epoch = $epoch; 304 - } 305 - 306 - $rows[] = $this->formatBurnRow(phabricator_date($epoch, $viewer), $info); 307 - $rowc[] = null; 308 - $week['open'] += $info['open']; 309 - $week['close'] += $info['close']; 310 - $month['open'] += $info['open']; 311 - $month['close'] += $info['close']; 312 - $period['open'] += $info['open']; 313 - $period['close'] += $info['close']; 314 - } 315 - 316 - if ($week) { 317 - $rows[] = $this->formatBurnRow( 318 - pht('Week To Date'), 319 - $week); 320 - $rowc[] = 'week'; 321 - } 322 - 323 - if ($month) { 324 - $rows[] = $this->formatBurnRow( 325 - pht('Month To Date'), 326 - $month); 327 - $rowc[] = 'month'; 328 - } 329 - 330 - $rows[] = $this->formatBurnRow( 331 - pht('All Time'), 332 - $period); 333 - $rowc[] = 'aggregate'; 334 - 335 - $rows = array_reverse($rows); 336 - $rowc = array_reverse($rowc); 337 - 338 - $table = new AphrontTableView($rows); 339 - $table->setRowClasses($rowc); 340 - $table->setHeaders( 341 - array( 342 - pht('Period'), 343 - pht('Opened'), 344 - pht('Closed'), 345 - pht('Change'), 346 - )); 347 - $table->setColumnClasses( 348 - array( 349 - 'right wide', 350 - 'n', 351 - 'n', 352 - 'n', 353 - )); 354 - 355 - if ($handle) { 356 - $inst = pht( 357 - 'NOTE: This table reflects tasks currently in '. 358 - 'the project. If a task was opened in the past but added to '. 359 - 'the project recently, it is counted on the day it was '. 360 - 'opened, not the day it was categorized. If a task was part '. 361 - 'of this project in the past but no longer is, it is not '. 362 - 'counted at all. This table may not agree exactly with the chart '. 363 - 'above.'); 364 - $header = pht('Task Burn Rate for Project %s', $handle->renderLink()); 365 - $caption = phutil_tag('p', array(), $inst); 366 - } else { 367 - $header = pht('Task Burn Rate for All Tasks'); 368 - $caption = null; 369 - } 370 - 371 - if ($caption) { 372 - $caption = id(new PHUIInfoView()) 373 - ->appendChild($caption) 374 - ->setSeverity(PHUIInfoView::SEVERITY_NOTICE); 375 - } 376 - 377 - $panel = new PHUIObjectBoxView(); 378 - $panel->setHeaderText($header); 379 - if ($caption) { 380 - $panel->setInfoView($caption); 381 - } 382 - $panel->setTable($table); 383 - 384 95 $tokens = array(); 385 96 if ($handle) { 386 97 $tokens = array($handle); 387 98 } 388 99 389 100 $filter = $this->renderReportFilters($tokens, $has_window = false); 390 - 391 - $id = celerity_generate_unique_node_id(); 392 - $chart = phutil_tag( 393 - 'div', 394 - array( 395 - 'id' => $id, 396 - 'style' => 'border: 1px solid #BFCFDA; '. 397 - 'background-color: #fff; '. 398 - 'margin: 8px 16px; '. 399 - 'height: 400px; ', 400 - ), 401 - ''); 402 - 403 - list($burn_x, $burn_y) = $this->buildSeries($data); 404 101 405 102 if ($project_phid) { 406 103 $projects = id(new PhabricatorProjectQuery()) ··· 471 168 return $filter; 472 169 } 473 170 474 - private function buildSeries(array $data) { 475 - $out = array(); 476 - 477 - $counter = 0; 478 - foreach ($data as $row) { 479 - $t = (int)$row['dateCreated']; 480 - if ($row['_is_close']) { 481 - --$counter; 482 - $out[$t] = $counter; 483 - } else if ($row['_is_open']) { 484 - ++$counter; 485 - $out[$t] = $counter; 486 - } 487 - } 488 - 489 - return array(array_keys($out), array_values($out)); 490 - } 491 - 492 171 /** 493 - * @param $label string Time representation for the row, e.g. "Feb 29 2024", 494 - * "All Time", "Week of May 10 2024", "Month To Date", etc. 495 - * @param $info array<string,int> open|close; number of tasks in timespan 496 - * @return array<string,string,string,PhutilSafeHTML> Row text label; number 497 - * of open tasks as string; number of closed tasks as string; 498 - * PhutilSafeHTML such as "<span class="red">+144</span>" 499 - */ 500 - private function formatBurnRow($label, $info) { 501 - $delta = $info['open'] - $info['close']; 502 - $fmt = number_format($delta); 503 - if ($delta > 0) { 504 - $fmt = '+'.$fmt; 505 - $fmt = phutil_tag('span', array('class' => 'red'), $fmt); 506 - } else { 507 - $fmt = phutil_tag('span', array('class' => 'green'), $fmt); 508 - } 509 - 510 - return array( 511 - $label, 512 - number_format($info['open']), 513 - number_format($info['close']), 514 - $fmt, 515 - ); 516 - } 517 - 518 - /** 519 - * @return int 50 172 + * @return int 50, the default value of the default "normal" Priority 520 173 */ 521 174 private function getAveragePriority() { 522 175 // TODO: This is sort of a hard-code for the default "normal" status.