From 40720ab83d0148fab28eba8218ae5fdc4b3e01f9 Mon Sep 17 00:00:00 2001 From: Andre Klapper Date: Mon, 3 Jun 2024 13:34:49 +0200 Subject: [PATCH] Expose initially set values of Maniphest custom date field and point values when removing them Summary: After removing or resetting a custom date field value or points value initially set at task creation there is no way to find and restore the previous value. As Phorge adds a transaction entry anyway, include the previous value in the transaction title. This makes it easier to revert an accidental removal of such data and is also more aligned with the behavior of other custom field types. Closes T15846 Test Plan: * Define a custom date field in Maniphest via http://phorge.localhost/config/edit/maniphest.custom-field-definitions/ and also enable http://phorge.localhost/config/edit/maniphest.points/ * Create a task with those fields set to non-custom values via a create task form which allows editing/changing these values * Edit the task and remove/reset the values of these custom fields * Look at the task and its transaction entries Reviewers: O1 Blessed Committers, avivey Reviewed By: O1 Blessed Committers, avivey Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15846 Differential Revision: https://we.phorge.it/D25678 --- .../maniphest/xaction/ManiphestTaskPointsTransaction.php | 5 +++++ .../standard/PhabricatorStandardCustomFieldDate.php | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/src/applications/maniphest/xaction/ManiphestTaskPointsTransaction.php b/src/applications/maniphest/xaction/ManiphestTaskPointsTransaction.php index 3b24dff590..fae125dccd 100644 --- a/src/applications/maniphest/xaction/ManiphestTaskPointsTransaction.php +++ b/src/applications/maniphest/xaction/ManiphestTaskPointsTransaction.php @@ -33,6 +33,11 @@ final class ManiphestTaskPointsTransaction '%s set the point value for this task to %s.', $this->renderAuthor(), $this->renderNewValue()); + } else if ($new === null && $old !== null) { + return pht( + '%s removed the point value %s for this task.', + $this->renderAuthor(), + $this->renderOldValue()); } else if ($new === null) { return pht( '%s removed the point value for this task.', diff --git a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldDate.php b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldDate.php index 5988970ca6..c437df82fb 100644 --- a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldDate.php +++ b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldDate.php @@ -177,6 +177,12 @@ final class PhabricatorStandardCustomFieldDate $xaction->renderHandleLink($author_phid), $this->getFieldName(), $new_date); + } else if (!$new && $old) { + return pht( + '%s removed %s which was set to %s.', + $xaction->renderHandleLink($author_phid), + $this->getFieldName(), + $old_date); } else if (!$new) { return pht( '%s removed %s.', -- 2.51.2