Page MenuHomePhabricator
Differential D18601

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

Authored by epriestley on Sep 14 2017, 11:29 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Apr 25, 3:10 AM
Unknown Object (File)
Wed, Apr 24, 1:34 PM
Unknown Object (File)
Fri, Apr 19, 2:42 AM
Unknown Object (File)
Fri, Apr 19, 2:42 AM
Unknown Object (File)
Fri, Apr 19, 2:42 AM
Unknown Object (File)
Thu, Apr 11, 10:38 AM
Unknown Object (File)
Thu, Apr 11, 9:47 AM
Unknown Object (File)
Thu, Apr 11, 4:03 AM
View All Files
Subscribers
• sshannin

Details

Reviewers
amckinley
Commits
rP939008e64c26: Correct an issue where Maniphest's awful legacy "reports" UI was extra broken…
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.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley created this revision.Sep 14 2017, 11:29 AM
Harbormaster completed remote builds in B18445: Diff 44664.Sep 14 2017, 11:30 AM
amckinley accepted this revision.Sep 14 2017, 4:39 PM
This revision is now accepted and ready to land.Sep 14 2017, 4:39 PM
Closed by commit rP939008e64c26: Correct an issue where Maniphest's awful legacy "reports" UI was extra broken… (authored by epriestley). · Explain WhySep 14 2017, 5:09 PM
This revision was automatically updated to reflect the committed changes.
• sshannin added a subscriber: • sshannin.Nov 23 2017, 3:21 AM

For completeness's sake, this sounds like the issue that hit me in T9025 where the charts accumulated task counts greater than the number of open tasks (and indeed, the counts in a maniphest search by project now seem to pretty closely match the end state of the corresponding chart).

Revision Contents
Changeset List

PathSize
src/
applications/
maniphest/
controller/
31 lines

Diff 44666

View Options

src/applications/maniphest/controller/ManiphestReportController.php

Loading...
Phabricator · Built by Phacility