Changeset View
Changeset View
Standalone View
Standalone View
src/applications/maniphest/controller/ManiphestReportController.php
| Show First 20 Lines • Show All 180 Lines • ▼ Show 20 Lines | public function renderBurn() { | |||||||||||
| $day_buckets = array(); | $day_buckets = array(); | |||||||||||
| $open_tasks = array(); | $open_tasks = array(); | |||||||||||
| foreach ($data as $key => $row) { | foreach ($data as $key => $row) { | |||||||||||
| switch ($row['transactionType']) { | switch ($row['transactionType']) { | |||||||||||
| case ManiphestTaskStatusTransaction::TRANSACTIONTYPE: | case ManiphestTaskStatusTransaction::TRANSACTIONTYPE: | |||||||||||
| // NOTE: Hack to avoid json_decode(). | // NOTE: Hack to avoid json_decode(). | |||||||||||
| $oldv = trim($row['oldValue'], '"'); | $oldv = $row['oldValue']; | |||||||||||
| if ($oldv !== null) { | ||||||||||||
| $oldv = trim($oldv, '"'); | ||||||||||||
| } | ||||||||||||
valerio.bozzolan: Thanks but this is not so obvious for me to review since previously the `trim(null, '"')` was… | ||||||||||||
| $newv = trim($row['newValue'], '"'); | $newv = trim($row['newValue'], '"'); | |||||||||||
| break; | break; | |||||||||||
| case ManiphestTaskMergedIntoTransaction::TRANSACTIONTYPE: | case ManiphestTaskMergedIntoTransaction::TRANSACTIONTYPE: | |||||||||||
| // NOTE: Merging a task does not generate a "status" transaction. | // NOTE: Merging a task does not generate a "status" transaction. | |||||||||||
| // We pretend it did. Note that this is not always accurate: it is | // We pretend it did. Note that this is not always accurate: it is | |||||||||||
| // possible to merge a task which was previously closed, but this | // possible to merge a task which was previously closed, but this | |||||||||||
| // fake transaction always counts a merge as a closure. | // fake transaction always counts a merge as a closure. | |||||||||||
| $oldv = $default_status; | $oldv = $default_status; | |||||||||||
| $newv = $duplicate_status; | $newv = $duplicate_status; | |||||||||||
| break; | break; | |||||||||||
| } | } | |||||||||||
| if ($oldv == 'null') { | if ($oldv == 'null') { | |||||||||||
| $old_is_open = false; | $old_is_open = false; | |||||||||||
Not Done Inline Actions
Unrealated: for performance reasons we may like in the future to also add a strict check here to avoid the unuseful call to ManiphestTaskStatus::isOpenStatus(null) that returns false after comparing that in a loop with each possible known status valerio.bozzolan: Unrealated: for performance reasons we may like in the future to also add a strict check here… | ||||||||||||
Done Inline Actions@aklapper If you want to relax proposing another easy-peasy patch, feel free to take inspiration from this small comment ↑ Easy +1 valerio.bozzolan: @aklapper If you want to relax proposing another easy-peasy patch, feel free to take… | ||||||||||||
| } else { | } else { | |||||||||||
| $old_is_open = ManiphestTaskStatus::isOpenStatus($oldv); | $old_is_open = ManiphestTaskStatus::isOpenStatus($oldv); | |||||||||||
| } | } | |||||||||||
| $new_is_open = ManiphestTaskStatus::isOpenStatus($newv); | $new_is_open = ManiphestTaskStatus::isOpenStatus($newv); | |||||||||||
| $is_open = ($new_is_open && !$old_is_open); | $is_open = ($new_is_open && !$old_is_open); | |||||||||||
| $is_close = ($old_is_open && !$new_is_open); | $is_close = ($old_is_open && !$new_is_open); | |||||||||||
| ▲ Show 20 Lines • Show All 679 Lines • Show Last 20 Lines | ||||||||||||
Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under Apache 2.0 or other open source licenses. · CC BY-SA 4.0 · Apache 2.0
Thanks but this is not so obvious for me to review since previously the trim(null, '"') was just returning an empty string and now it will return null. I will double-check.