From ea554af476493bd27017017c230b8e8a76b8dd4d Mon Sep 17 00:00:00 2001 From: Aviv Eyal Date: Sat, 11 May 2024 13:22:24 +0300 Subject: [PATCH] Add (Advanced) Custom Fields to Item List Summary: Allow PHP-coded Custom Fields to show things in Lists. Also add Repository to Revision List and Flags to Maniphest lists. Closes T15133. Ref T15512, T15750 Test Plan: Look at Repository List and Task lists that have flags. Reviewers: O1 Blessed Committers, aklapper, valerio.bozzolan Reviewed By: O1 Blessed Committers, aklapper, valerio.bozzolan Subscribers: aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15750, T15512, T15133 Differential Revision: https://we.phorge.it/D25548 --- src/__phutil_library_map__.php | 6 +++ .../DifferentialRepositoryField.php | 12 +++++ .../DifferentialRevisionSearchEngine.php | 5 ++ .../view/DifferentialRevisionListView.php | 12 +++++ .../PhorgeFlagFlaggedObjectCustomField.php | 46 +++++++++++++++++++ .../PhorgeFlagFlaggedObjectFieldStorage.php | 36 +++++++++++++++ .../field/ManiphestFlagCustomField.php | 18 ++++++++ .../query/ManiphestTaskSearchEngine.php | 24 ++++++++++ .../maniphest/view/ManiphestTaskListView.php | 15 ++++++ .../view/ManiphestTaskResultListView.php | 18 ++++++-- .../PhabricatorApplicationSearchEngine.php | 32 ++++++++++++- .../field/PhabricatorCustomField.php | 13 +++++- .../field/PhabricatorCustomFieldList.php | 13 ++++++ 13 files changed, 244 insertions(+), 6 deletions(-) create mode 100644 src/applications/flag/customfield/PhorgeFlagFlaggedObjectCustomField.php create mode 100644 src/applications/flag/customfield/PhorgeFlagFlaggedObjectFieldStorage.php create mode 100644 src/applications/maniphest/field/ManiphestFlagCustomField.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 17b1e837a6..b21f29d1b4 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1802,6 +1802,7 @@ phutil_register_library_map(array( 'ManiphestEditConduitAPIMethod' => 'applications/maniphest/conduit/ManiphestEditConduitAPIMethod.php', 'ManiphestEditEngine' => 'applications/maniphest/editor/ManiphestEditEngine.php', 'ManiphestEmailCommand' => 'applications/maniphest/command/ManiphestEmailCommand.php', + 'ManiphestFlagCustomField' => 'applications/maniphest/field/ManiphestFlagCustomField.php', 'ManiphestGetTaskTransactionsConduitAPIMethod' => 'applications/maniphest/conduit/ManiphestGetTaskTransactionsConduitAPIMethod.php', 'ManiphestHovercardEngineExtension' => 'applications/maniphest/engineextension/ManiphestHovercardEngineExtension.php', 'ManiphestInfoConduitAPIMethod' => 'applications/maniphest/conduit/ManiphestInfoConduitAPIMethod.php', @@ -5388,6 +5389,8 @@ phutil_register_library_map(array( 'PholioTransactionView' => 'applications/pholio/view/PholioTransactionView.php', 'PholioUploadedImageView' => 'applications/pholio/view/PholioUploadedImageView.php', 'PhorgeCodeWarningSetupCheck' => 'applications/config/check/PhorgeCodeWarningSetupCheck.php', + 'PhorgeFlagFlaggedObjectCustomField' => 'applications/flag/customfield/PhorgeFlagFlaggedObjectCustomField.php', + 'PhorgeFlagFlaggedObjectFieldStorage' => 'applications/flag/customfield/PhorgeFlagFlaggedObjectFieldStorage.php', 'PhorgeSystemDeprecationWarningListener' => 'applications/system/events/PhorgeSystemDeprecationWarningListener.php', 'PhortuneAccount' => 'applications/phortune/storage/PhortuneAccount.php', 'PhortuneAccountAddManagerController' => 'applications/phortune/controller/account/PhortuneAccountAddManagerController.php', @@ -8008,6 +8011,7 @@ phutil_register_library_map(array( 'ManiphestEditConduitAPIMethod' => 'PhabricatorEditEngineAPIMethod', 'ManiphestEditEngine' => 'PhabricatorEditEngine', 'ManiphestEmailCommand' => 'MetaMTAEmailTransactionCommand', + 'ManiphestFlagCustomField' => 'ManiphestCustomField', 'ManiphestGetTaskTransactionsConduitAPIMethod' => 'ManiphestConduitAPIMethod', 'ManiphestHovercardEngineExtension' => 'PhabricatorHovercardEngineExtension', 'ManiphestInfoConduitAPIMethod' => 'ManiphestConduitAPIMethod', @@ -12212,6 +12216,8 @@ phutil_register_library_map(array( 'PholioTransactionView' => 'PhabricatorApplicationTransactionView', 'PholioUploadedImageView' => 'AphrontView', 'PhorgeCodeWarningSetupCheck' => 'PhabricatorSetupCheck', + 'PhorgeFlagFlaggedObjectCustomField' => 'PhabricatorCustomField', + 'PhorgeFlagFlaggedObjectFieldStorage' => 'Phobject', 'PhorgeSystemDeprecationWarningListener' => 'PhabricatorEventListener', 'PhortuneAccount' => array( 'PhortuneDAO', diff --git a/src/applications/differential/customfield/DifferentialRepositoryField.php b/src/applications/differential/customfield/DifferentialRepositoryField.php index 1d403d160a..9cdc29fcb3 100644 --- a/src/applications/differential/customfield/DifferentialRepositoryField.php +++ b/src/applications/differential/customfield/DifferentialRepositoryField.php @@ -63,4 +63,16 @@ final class DifferentialRepositoryField $repository->getMonogram().' '.$repository->getName()); } + public function shouldAppearInListView() { + return true; + } + + public function renderOnListItem(PHUIObjectItemView $view) { + if ($this->getValue()) { + $handle = $this->getViewer()->renderHandle($this->getValue()); + $view->addByLine(pht('Repository: %s', $handle)); + } + } + + } diff --git a/src/applications/differential/query/DifferentialRevisionSearchEngine.php b/src/applications/differential/query/DifferentialRevisionSearchEngine.php index b69b764bb8..9e18ba8e24 100644 --- a/src/applications/differential/query/DifferentialRevisionSearchEngine.php +++ b/src/applications/differential/query/DifferentialRevisionSearchEngine.php @@ -199,6 +199,10 @@ final class DifferentialRevisionSearchEngine $unlanded = $this->loadUnlandedDependencies($revisions); + $custom_field_lists = $this->loadCustomFields( + $revisions, + PhabricatorCustomField::ROLE_LIST); + $views = array(); if ($bucket) { $bucket->setViewer($viewer); @@ -231,6 +235,7 @@ final class DifferentialRevisionSearchEngine foreach ($views as $view) { $view->setUnlandedDependencies($unlanded); + $view->setCustomFieldLists($custom_field_lists); } if (count($views) == 1) { diff --git a/src/applications/differential/view/DifferentialRevisionListView.php b/src/applications/differential/view/DifferentialRevisionListView.php index ab0abcc386..018f640c6c 100644 --- a/src/applications/differential/view/DifferentialRevisionListView.php +++ b/src/applications/differential/view/DifferentialRevisionListView.php @@ -11,6 +11,7 @@ final class DifferentialRevisionListView extends AphrontView { private $noBox; private $background = null; private $unlandedDependencies = array(); + private $customFieldLists = array(); public function setUnlandedDependencies(array $unlanded_dependencies) { $this->unlandedDependencies = $unlanded_dependencies; @@ -47,6 +48,11 @@ final class DifferentialRevisionListView extends AphrontView { return $this; } + public function setCustomFieldLists(array $lists) { + $this->customFieldLists = $lists; + return $this; + } + public function render() { $viewer = $this->getViewer(); @@ -181,6 +187,12 @@ final class DifferentialRevisionListView extends AphrontView { "{$icon} {$color}", $revision->getStatusDisplayName()); + $field_list = idx($this->customFieldLists, $revision->getPHID()); + if ($field_list) { + $field_list + ->addFieldsToListViewItem($revision, $viewer, $item); + } + $list->addItem($item); } diff --git a/src/applications/flag/customfield/PhorgeFlagFlaggedObjectCustomField.php b/src/applications/flag/customfield/PhorgeFlagFlaggedObjectCustomField.php new file mode 100644 index 0000000000..8d2f358efd --- /dev/null +++ b/src/applications/flag/customfield/PhorgeFlagFlaggedObjectCustomField.php @@ -0,0 +1,46 @@ +flag) { + return; + } + // I'm very open to improvements in the way a Flag is displayed + $icon = PhabricatorFlagColor::getIcon($this->flag->getColor()); + $view->addIcon($icon); + } + + + public function shouldUseStorage() { + return true; + } + + public function setValueFromStorage($value) { + $this->flag = $value; + } + + // The parent function is defined to return a PhabricatorCustomFieldStorage, + // but that assumes a DTO with a particular form; That doesn't apply here. + // Maybe the function needs to be re-defined with a suitable interface. + // For now, PhorgeFlagFlaggedObjectFieldStorage just duck-types into the + // right shape. + public function newStorageObject() { + return id(new PhorgeFlagFlaggedObjectFieldStorage()) + ->setViewer($this->getViewer()); + } + +} diff --git a/src/applications/flag/customfield/PhorgeFlagFlaggedObjectFieldStorage.php b/src/applications/flag/customfield/PhorgeFlagFlaggedObjectFieldStorage.php new file mode 100644 index 0000000000..b8f28a2fac --- /dev/null +++ b/src/applications/flag/customfield/PhorgeFlagFlaggedObjectFieldStorage.php @@ -0,0 +1,36 @@ +viewer = $viewer; + return $this; + } + + public function getStorageSourceKey() { + return 'flags/flag'; + } + + public function loadStorageSourceData(array $fields) { + + $objects = mpull($fields, 'getObject'); + $object_phids = mpull($objects, 'getPHID'); + $flags = (new PhabricatorFlagQuery()) + ->setViewer($this->viewer) + ->withOwnerPHIDs(array($this->viewer->getPHID())) + ->withObjectPHIDs($object_phids) + ->execute(); + $flags = mpull($flags, null, 'getObjectPHID'); + + $result = array(); + foreach ($fields as $key => $field) { + $target_phid = $field->getObject()->getPHID(); + $result[$key] = idx($flags, $target_phid); + } + + return $result; + } + +} diff --git a/src/applications/maniphest/field/ManiphestFlagCustomField.php b/src/applications/maniphest/field/ManiphestFlagCustomField.php new file mode 100644 index 0000000000..febf0aea8d --- /dev/null +++ b/src/applications/maniphest/field/ManiphestFlagCustomField.php @@ -0,0 +1,18 @@ +setProxy(new PhorgeFlagFlaggedObjectCustomField()); + } + + public function canSetProxy() { + return true; + } + + public function newStorageObject() { + return $this->getProxy()->newStorageObject(); + } +} diff --git a/src/applications/maniphest/query/ManiphestTaskSearchEngine.php b/src/applications/maniphest/query/ManiphestTaskSearchEngine.php index 4f3e9c0462..541f7f3831 100644 --- a/src/applications/maniphest/query/ManiphestTaskSearchEngine.php +++ b/src/applications/maniphest/query/ManiphestTaskSearchEngine.php @@ -374,11 +374,17 @@ final class ManiphestTaskSearchEngine ManiphestBulkEditCapability::CAPABILITY); } + $custom_field_lists = $this->loadCustomFields( + $tasks, + PhabricatorCustomField::ROLE_LIST); + $list = id(new ManiphestTaskResultListView()) ->setUser($viewer) ->setTasks($tasks) + ->setHandles($handles) ->setSavedQuery($saved) ->setCanBatchEdit($can_bulk_edit) + ->setCustomFieldLists($custom_field_lists) ->setShowBatchControls($this->showBatchControls); $result = new PhabricatorApplicationSearchResultView(); @@ -387,6 +393,24 @@ final class ManiphestTaskSearchEngine return $result; } + protected function getRequiredHandlePHIDsForResultList( + array $objects, + PhabricatorSavedQuery $query) { + + $phids = array(); + foreach ($objects as $task) { + $assigned_phid = $task->getOwnerPHID(); + if ($assigned_phid) { + $phids[] = $assigned_phid; + } + foreach ($task->getProjectPHIDs() as $project_phid) { + $phids[] = $project_phid; + } + } + + return $phids; + } + protected function willUseSavedQuery(PhabricatorSavedQuery $saved) { // The 'withUnassigned' parameter may be present in old saved queries from diff --git a/src/applications/maniphest/view/ManiphestTaskListView.php b/src/applications/maniphest/view/ManiphestTaskListView.php index f9ad9e6046..d5d3d1e9c5 100644 --- a/src/applications/maniphest/view/ManiphestTaskListView.php +++ b/src/applications/maniphest/view/ManiphestTaskListView.php @@ -4,6 +4,7 @@ final class ManiphestTaskListView extends ManiphestView { private $tasks; private $handles; + private $customFieldLists = array(); private $showBatchControls; private $noDataString; @@ -19,6 +20,11 @@ final class ManiphestTaskListView extends ManiphestView { return $this; } + public function setCustomFieldLists(array $lists) { + $this->customFieldLists = $lists; + return $this; + } + public function setShowBatchControls($show_batch_controls) { $this->showBatchControls = $show_batch_controls; return $this; @@ -132,12 +138,21 @@ final class ManiphestTaskListView extends ManiphestView { ->setHref($href)); } + + $field_list = idx($this->customFieldLists, $task->getPHID()); + if ($field_list) { + $field_list + ->addFieldsToListViewItem($task, $this->getViewer(), $item); + } + $list->addItem($item); } return $list; } + // This method should be removed, and all call-sites switch + // to use ManiphestSearchEngine public static function loadTaskHandles( PhabricatorUser $viewer, array $tasks) { diff --git a/src/applications/maniphest/view/ManiphestTaskResultListView.php b/src/applications/maniphest/view/ManiphestTaskResultListView.php index cc2a135292..6ee2025265 100644 --- a/src/applications/maniphest/view/ManiphestTaskResultListView.php +++ b/src/applications/maniphest/view/ManiphestTaskResultListView.php @@ -3,6 +3,8 @@ final class ManiphestTaskResultListView extends ManiphestView { private $tasks; + private $handles; + private $customFieldLists = array(); private $savedQuery; private $canBatchEdit; private $showBatchControls; @@ -17,6 +19,16 @@ final class ManiphestTaskResultListView extends ManiphestView { return $this; } + public function setHandles(array $handles) { + $this->handles = $handles; + return $this; + } + + public function setCustomFieldLists(array $lists) { + $this->customFieldLists = $lists; + return $this; + } + public function setCanBatchEdit($can_batch_edit) { $this->canBatchEdit = $can_batch_edit; return $this; @@ -42,11 +54,10 @@ final class ManiphestTaskResultListView extends ManiphestView { $group_parameter = nonempty($query->getParameter('group'), 'priority'); $order_parameter = nonempty($query->getParameter('order'), 'priority'); - $handles = ManiphestTaskListView::loadTaskHandles($viewer, $tasks); $groups = $this->groupTasks( $tasks, $group_parameter, - $handles); + $this->handles); $result = array(); @@ -56,7 +67,8 @@ final class ManiphestTaskResultListView extends ManiphestView { $task_list->setShowBatchControls($this->showBatchControls); $task_list->setUser($viewer); $task_list->setTasks($list); - $task_list->setHandles($handles); + $task_list->setHandles($this->handles); + $task_list->setCustomFieldLists($this->customFieldLists); $header = id(new PHUIHeaderView()) ->addSigil('task-group') diff --git a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php index cad397b0ac..9094ead227 100644 --- a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php +++ b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php @@ -13,6 +13,7 @@ * @task read Reading Utilities * @task exec Paging and Executing Queries * @task render Rendering Results + * @task custom Custom Fields */ abstract class PhabricatorApplicationSearchEngine extends Phobject { @@ -1071,7 +1072,7 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject { if ($phids) { $handles = id(new PhabricatorHandleQuery()) ->setViewer($this->requireViewer()) - ->witHPHIDs($phids) + ->withPHIDs($phids) ->execute(); } else { $handles = array(); @@ -1626,4 +1627,33 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject { return $supported; } + /** + * Load from object and from storage, and updates Custom Fields instances + * that are attached to each object. + * + * @return mapPhabricatorCustomFieldList> of loaded fields. + * @task custom + */ + protected function loadCustomFields(array $objects, $role) { + assert_instances_of($objects, 'PhabricatorCustomFieldInterface'); + + $query = new PhabricatorCustomFieldStorageQuery(); + $lists = array(); + + foreach ($objects as $object) { + $field_list = PhabricatorCustomField::getObjectFields($object, $role); + $field_list->readFieldsFromObject($object); + foreach ($field_list->getFields() as $field) { + // TODO move $viewer into PhabricatorCustomFieldStorageQuery + $field->setViewer($this->viewer); + } + $lists[$object->getPHID()] = $field_list; + $query->addFields($field_list->getFields()); + } + // This updates the field_list objects. + $query->execute(); + + return $lists; + } + } diff --git a/src/infrastructure/customfield/field/PhabricatorCustomField.php b/src/infrastructure/customfield/field/PhabricatorCustomField.php index 4e55348c36..774dbb6630 100644 --- a/src/infrastructure/customfield/field/PhabricatorCustomField.php +++ b/src/infrastructure/customfield/field/PhabricatorCustomField.php @@ -384,6 +384,7 @@ abstract class PhabricatorCustomField extends Phobject { * * @param PhabricatorCustomField Field implementation. * @return this + * @task proxy */ final public function setProxy(PhabricatorCustomField $proxy) { if (!$this->canSetProxy()) { @@ -400,12 +401,20 @@ abstract class PhabricatorCustomField extends Phobject { * @{method:canSetProxy}. * * @return PhabricatorCustomField|null Proxy field, if one is set. + * @task proxy */ final public function getProxy() { return $this->proxy; } - + /** + * @task proxy + */ + public function __clone() { + if ($this->proxy) { + $this->proxy = clone $this->proxy; + } + } /* -( Contextual Data )---------------------------------------------------- */ @@ -827,7 +836,7 @@ abstract class PhabricatorCustomField extends Phobject { /** - * Appearing in ApplicationTrasactions allows a field to be edited using + * Appearing in ApplicationTransactions allows a field to be edited using * standard workflows. * * @return bool True to appear in ApplicationTransactions. diff --git a/src/infrastructure/customfield/field/PhabricatorCustomFieldList.php b/src/infrastructure/customfield/field/PhabricatorCustomFieldList.php index 60a5171cdd..369bed2297 100644 --- a/src/infrastructure/customfield/field/PhabricatorCustomFieldList.php +++ b/src/infrastructure/customfield/field/PhabricatorCustomFieldList.php @@ -200,6 +200,19 @@ final class PhabricatorCustomFieldList extends Phobject { } } + public function addFieldsToListViewItem( + PhabricatorCustomFieldInterface $object, + PhabricatorUser $viewer, + PHUIObjectItemView $view) { + + foreach ($this->fields as $field) { + if ($field->shouldAppearInListView()) { + $field->setViewer($viewer); + $field->renderOnListItem($view); + } + } + } + public function buildFieldTransactionsFromRequest( PhabricatorApplicationTransaction $template, AphrontRequest $request) {