revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-26 16:52:41 +01:00
Code Issues Releases Wiki Activity

Correct an issue where Maniphest's awful legacy "reports" UI was extra broken on merges

Browse source 
Summary:
See PHI66. See that issue for context. This UI is bad broken legacy junk, but was especially broken when reporting merges.

These do not currently generate a "status" transaction, so they were never counted as task closures. Pretend they're normal closures.

This is still wrong, but should be much closer to the real numbers. Specifically, if you merge a closed task into another task, it will incorrectly be counted as an extra close. This could result in negative tasks, but the numbers should be much closer to reality than they are today even so.

The "Facts" application (T1562) is the real pathway forward here in the longer term.

Test Plan:
  - Moved my `maniphest_transactions` table aside with `RENAME TABLE ...`.
  - Created a new empty table with `CREATE TABLE ... LIKE ...`.
  - Reloaded reports UI, saw empty chart.
  - Created, closed, and reopened tasks while reloading the chart, saw accurate reporting.
  - Merged an open task into another task, saw bad reporting.
  - Applied patch, saw the right chart again.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18601
This commit is contained in:
epriestley 2017-09-14 04:23:53 -07:00
parent c310f08b7a
commit 939008e64c
1 changed files with 24 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

25
src/applications/maniphest/controller/ManiphestReportController.php
View file

@ -88,23 +88,40 @@ final class ManiphestReportController extends ManiphestController {
$data = queryfx_all( $data = queryfx_all(
$conn, $conn,
'SELECT x.oldValue, x.newValue, x.dateCreated FROM %T x %Q 'SELECT x.transactionType, x.oldValue, x.newValue, x.dateCreated
WHERE transactionType = %s FROM %T x %Q
WHERE transactionType IN (%Ls)
ORDER BY x.dateCreated ASC', ORDER BY x.dateCreated ASC',
$table->getTableName(), $table->getTableName(),
$joins, $joins,
ManiphestTaskStatusTransaction::TRANSACTIONTYPE); array(
ManiphestTaskStatusTransaction::TRANSACTIONTYPE,
ManiphestTaskMergedIntoTransaction::TRANSACTIONTYPE,
));
$stats = array(); $stats = array();
$day_buckets = array(); $day_buckets = array();
$open_tasks = array(); $open_tasks = array();
$default_status = ManiphestTaskStatus::getDefaultStatus();
$duplicate_status = ManiphestTaskStatus::getDuplicateStatus();
foreach ($data as $key => $row) { foreach ($data as $key => $row) {
switch ($row['transactionType']) {
case ManiphestTaskStatusTransaction::TRANSACTIONTYPE:
// NOTE: Hack to avoid json_decode(). // NOTE: Hack to avoid json_decode().
$oldv = trim($row['oldValue'], '"'); $oldv = trim($row['oldValue'], '"');
$newv = trim($row['newValue'], '"'); $newv = trim($row['newValue'], '"');
break;
case ManiphestTaskMergedIntoTransaction::TRANSACTIONTYPE:
// NOTE: Merging a task does not generate a "status" transaction.
// We pretend it did. Note that this is not always accurate: it is
// possble to merge a task which was previously closed, but this
// fake transaction always counts a merge as a closure.
$oldv = $default_status;
$newv = $duplicate_status;
break;
}
if ($oldv == 'null') { if ($oldv == 'null') {
$old_is_open = false; $old_is_open = false;