From 52f7446eeaa36ee1716fd1ed6e90b26b964e11ac Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 8 Dec 2015 06:26:30 -0800 Subject: [PATCH] Remove "Create Empty Task" workflow callouts and some other clutter Summary: Ref T9908. This removes the "Create Another Empty Task", "Create A Similar Task" and "Create Another Subtask of Same Parent Task" workflow callouts on the task detail page, which are actions that show up after creating a task or creating a subtask. - I think "Create Empty" isn't relevant now that we have "Create Task" nearby and the quick create menu? - I'm not sure if "Create Similar" is worth keeping. If we do want to retain it, I'd maybe like to find a way to do it generically. - I'm likewise not sure if "Create another subtask" is worth keeping. Overall, these actions are weird/unusual and I'm not sure how valuable they are. I'm open to keeping "Similar" and/or "Subtask" but I'd like to verify that they're still valuable and make sure we have a reasonable design for them if we retain them. For example, if we want to retain "Similar", maybe a better approach is just to add "Create Similar Object" to every action menu (which is now possible for EditEngine applications)? There's at least some interest in "Create Similar Repository" in Diffusion. Also removes a very very old piece of "attached files" code. Test Plan: Created and viewed some tasks. Reviewers: chad Reviewed By: chad Maniphest Tasks: T9908 Differential Revision: https://secure.phabricator.com/D14705 --- src/__phutil_library_map__.php | 2 - .../ManiphestTaskDetailController.php | 86 +------------------ .../maniphest/storage/ManiphestTask.php | 4 +- .../layout/PhabricatorFileLinkListView.php | 36 -------- 4 files changed, 7 insertions(+), 121 deletions(-) delete mode 100644 src/view/layout/PhabricatorFileLinkListView.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 8504af9f5c..d125ecc233 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2238,7 +2238,6 @@ phutil_register_library_map(array( 'PhabricatorFileImageMacro' => 'applications/macro/storage/PhabricatorFileImageMacro.php', 'PhabricatorFileImageTransform' => 'applications/files/transform/PhabricatorFileImageTransform.php', 'PhabricatorFileInfoController' => 'applications/files/controller/PhabricatorFileInfoController.php', - 'PhabricatorFileLinkListView' => 'view/layout/PhabricatorFileLinkListView.php', 'PhabricatorFileLinkView' => 'view/layout/PhabricatorFileLinkView.php', 'PhabricatorFileListController' => 'applications/files/controller/PhabricatorFileListController.php', 'PhabricatorFileQuery' => 'applications/files/query/PhabricatorFileQuery.php', @@ -6396,7 +6395,6 @@ phutil_register_library_map(array( ), 'PhabricatorFileImageTransform' => 'PhabricatorFileTransform', 'PhabricatorFileInfoController' => 'PhabricatorFileController', - 'PhabricatorFileLinkListView' => 'AphrontView', 'PhabricatorFileLinkView' => 'AphrontView', 'PhabricatorFileListController' => 'PhabricatorFileController', 'PhabricatorFileQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', diff --git a/src/applications/maniphest/controller/ManiphestTaskDetailController.php b/src/applications/maniphest/controller/ManiphestTaskDetailController.php index 886bbbadcf..456efd275e 100644 --- a/src/applications/maniphest/controller/ManiphestTaskDetailController.php +++ b/src/applications/maniphest/controller/ManiphestTaskDetailController.php @@ -10,10 +10,6 @@ final class ManiphestTaskDetailController extends ManiphestController { $viewer = $this->getViewer(); $id = $request->getURIData('id'); - $e_title = null; - - $priority_map = ManiphestTaskPriority::getTaskPriorityMap(); - $task = id(new ManiphestTaskQuery()) ->setViewer($viewer) ->withIDs(array($id)) @@ -23,15 +19,6 @@ final class ManiphestTaskDetailController extends ManiphestController { return new Aphront404Response(); } - $workflow = $request->getStr('workflow'); - $parent_task = null; - if ($workflow && is_numeric($workflow)) { - $parent_task = id(new ManiphestTaskQuery()) - ->setViewer($viewer) - ->withIDs(array($workflow)) - ->executeOne(); - } - $field_list = PhabricatorCustomField::getObjectFields( $task, PhabricatorCustomField::ROLE_VIEW); @@ -65,53 +52,13 @@ final class ManiphestTaskDetailController extends ManiphestController { } $phids[$task->getAuthorPHID()] = true; - $attached = $task->getAttached(); - foreach ($attached as $type => $list) { - foreach ($list as $phid => $info) { - $phids[$phid] = true; - } - } - - if ($parent_task) { - $phids[$parent_task->getPHID()] = true; - } - $phids = array_keys($phids); $handles = $viewer->loadHandles($phids); - $info_view = null; - if ($parent_task) { - $info_view = new PHUIInfoView(); - $info_view->setSeverity(PHUIInfoView::SEVERITY_NOTICE); - $info_view->addButton( - id(new PHUIButtonView()) - ->setTag('a') - ->setHref('/maniphest/task/create/?parent='.$parent_task->getID()) - ->setText(pht('Create Another Subtask'))); - - $info_view->appendChild(hsprintf( - 'Created a subtask of %s.', - $handles->renderHandle($parent_task->getPHID()))); - } else if ($workflow == 'create') { - $info_view = new PHUIInfoView(); - $info_view->setSeverity(PHUIInfoView::SEVERITY_NOTICE); - $info_view->addButton( - id(new PHUIButtonView()) - ->setTag('a') - ->setHref('/maniphest/task/create/?template='.$task->getID()) - ->setText(pht('Similar Task'))); - $info_view->addButton( - id(new PHUIButtonView()) - ->setTag('a') - ->setHref('/maniphest/task/create/') - ->setText(pht('Empty Task'))); - $info_view->appendChild(pht('New task created. Create another?')); - } - - $engine = new PhabricatorMarkupEngine(); - $engine->setViewer($viewer); - $engine->setContextObject($task); - $engine->addObject($task, ManiphestTask::MARKUP_FIELD_DESCRIPTION); + $engine = id(new PhabricatorMarkupEngine()) + ->setViewer($viewer) + ->setContextObject($task) + ->addObject($task, ManiphestTask::MARKUP_FIELD_DESCRIPTION); $timeline = $this->buildTransactionTimeline( $task, @@ -155,7 +102,6 @@ final class ManiphestTaskDetailController extends ManiphestController { )) ->appendChild( array( - $info_view, $object_box, $timeline, $comment_view, @@ -325,30 +271,6 @@ final class ManiphestTaskDetailController extends ManiphestController { phutil_implode_html(phutil_tag('br'), $revisions_commits)); } - $attached = $task->getAttached(); - if (!is_array($attached)) { - $attached = array(); - } - - $file_infos = idx($attached, PhabricatorFileFilePHIDType::TYPECONST); - if ($file_infos) { - $file_phids = array_keys($file_infos); - - // TODO: These should probably be handles or something; clean this up - // as we sort out file attachments. - $files = id(new PhabricatorFileQuery()) - ->setViewer($viewer) - ->withPHIDs($file_phids) - ->execute(); - - $file_view = new PhabricatorFileLinkListView(); - $file_view->setFiles($files); - - $view->addProperty( - pht('Files'), - $file_view->render()); - } - $view->invokeWillRenderEvent(); $field_list->appendFieldsToPropertyList( diff --git a/src/applications/maniphest/storage/ManiphestTask.php b/src/applications/maniphest/storage/ManiphestTask.php index 7fbc806bae..4a78978442 100644 --- a/src/applications/maniphest/storage/ManiphestTask.php +++ b/src/applications/maniphest/storage/ManiphestTask.php @@ -32,7 +32,6 @@ final class ManiphestTask extends ManiphestDAO protected $viewPolicy = PhabricatorPolicies::POLICY_USER; protected $editPolicy = PhabricatorPolicies::POLICY_USER; - protected $attached = array(); protected $projectPHIDs = array(); protected $ownerOrdering; @@ -43,6 +42,9 @@ final class ManiphestTask extends ManiphestDAO private $customFields = self::ATTACHABLE; private $edgeProjectPHIDs = self::ATTACHABLE; + // TODO: This field is unused and should eventually be removed. + protected $attached = array(); + public static function initializeNewTask(PhabricatorUser $actor) { $app = id(new PhabricatorApplicationQuery()) ->setViewer($actor) diff --git a/src/view/layout/PhabricatorFileLinkListView.php b/src/view/layout/PhabricatorFileLinkListView.php deleted file mode 100644 index 5824b6a8c2..0000000000 --- a/src/view/layout/PhabricatorFileLinkListView.php +++ /dev/null @@ -1,36 +0,0 @@ -files = $files; - return $this; - } - private function getFiles() { - return $this->files; - } - - public function render() { - $files = $this->getFiles(); - if (!$files) { - return ''; - } - - require_celerity_resource('phabricator-remarkup-css'); - - $file_links = array(); - foreach ($this->getFiles() as $file) { - $view = id(new PhabricatorFileLinkView()) - ->setFilePHID($file->getPHID()) - ->setFileName($file->getName()) - ->setFileDownloadURI($file->getDownloadURI()) - ->setFileViewURI($file->getBestURI()) - ->setFileViewable($file->isViewableImage()); - $file_links[] = $view->render(); - } - - return phutil_implode_html(phutil_tag('br'), $file_links); - } -}