From 6bfb101aff20a1f1f0435178e2de044a945676ba Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 4 Dec 2015 06:37:36 -0800 Subject: [PATCH] Replace all Maniphest commenting code with EditEngine commenting code Summary: Ref T9132. Like D14659, I'll hold this until after the cut. This swaps commenting in Maniphest over to EditEngine / stackable actions. New code doesn't have parity yet, although none of the things we're missing should technically be //strictly mandatory//. There's a list inline. I'll restore these in the next diffs. Briefly -- comments, subscribers and projects work. Status, owners and priority do not yet. Test Plan: - Made comments and added subscribers and projects. - Read through the old code to look for missing features and tried to document them all. Reviewers: chad Reviewed By: chad Maniphest Tasks: T9132 Differential Revision: https://secure.phabricator.com/D14663 --- resources/celerity/map.php | 30 +-- resources/celerity/packages.php | 3 - src/__phutil_library_map__.php | 4 - .../ManiphestTaskDetailController.php | 225 +----------------- .../ManiphestTransactionPreviewController.php | 128 ---------- .../ManiphestTransactionSaveController.php | 209 ---------------- .../maniphest/editor/ManiphestEditEngine.php | 12 +- .../behavior-transaction-controls.js | 35 --- .../maniphest/behavior-transaction-expand.js | 29 --- .../maniphest/behavior-transaction-preview.js | 76 ------ 10 files changed, 20 insertions(+), 731 deletions(-) delete mode 100644 src/applications/maniphest/controller/ManiphestTransactionPreviewController.php delete mode 100644 src/applications/maniphest/controller/ManiphestTransactionSaveController.php delete mode 100644 webroot/rsrc/js/application/maniphest/behavior-transaction-controls.js delete mode 100644 webroot/rsrc/js/application/maniphest/behavior-transaction-expand.js delete mode 100644 webroot/rsrc/js/application/maniphest/behavior-transaction-preview.js diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 026d46f6df..4bec955da1 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -15,7 +15,7 @@ return array( 'diffusion.pkg.css' => 'f45955ed', 'diffusion.pkg.js' => 'ca1c8b5a', 'maniphest.pkg.css' => '4845691a', - 'maniphest.pkg.js' => '3ec6a6d5', + 'maniphest.pkg.js' => '949a7498', 'rsrc/css/aphront/aphront-bars.css' => '231ac33c', 'rsrc/css/aphront/dark-console.css' => '6378ef3d', 'rsrc/css/aphront/dialog-view.css' => 'be0e3a46', @@ -407,9 +407,6 @@ return array( 'rsrc/js/application/maniphest/behavior-line-chart.js' => '88f0c5b3', 'rsrc/js/application/maniphest/behavior-list-edit.js' => 'a9f88de2', 'rsrc/js/application/maniphest/behavior-subpriorityeditor.js' => '71237763', - 'rsrc/js/application/maniphest/behavior-transaction-controls.js' => '44168bad', - 'rsrc/js/application/maniphest/behavior-transaction-expand.js' => '5fefb143', - 'rsrc/js/application/maniphest/behavior-transaction-preview.js' => '4c95d29e', 'rsrc/js/application/owners/OwnersPathEditor.js' => 'aa1733d0', 'rsrc/js/application/owners/owners-path-editor.js' => '7a68dda3', 'rsrc/js/application/passphrase/passphrase-credential-control.js' => '3cb0b2fc', @@ -624,9 +621,6 @@ return array( 'javelin-behavior-maniphest-batch-selector' => '7b98d7c5', 'javelin-behavior-maniphest-list-editor' => 'a9f88de2', 'javelin-behavior-maniphest-subpriority-editor' => '71237763', - 'javelin-behavior-maniphest-transaction-controls' => '44168bad', - 'javelin-behavior-maniphest-transaction-expand' => '5fefb143', - 'javelin-behavior-maniphest-transaction-preview' => '4c95d29e', 'javelin-behavior-owners-path-editor' => '7a68dda3', 'javelin-behavior-passphrase-credential-control' => '3cb0b2fc', 'javelin-behavior-persona-login' => '9414ff18', @@ -1095,11 +1089,6 @@ return array( 'javelin-dom', 'javelin-request', ), - '44168bad' => array( - 'javelin-behavior', - 'javelin-dom', - 'phabricator-prefab', - ), '44959b73' => array( 'javelin-util', 'javelin-uri', @@ -1141,14 +1130,6 @@ return array( 'javelin-request', 'javelin-util', ), - '4c95d29e' => array( - 'javelin-behavior', - 'javelin-dom', - 'javelin-util', - 'javelin-json', - 'javelin-stratcom', - 'phabricator-shaped-request', - ), '4cebc641' => array( 'javelin-install', ), @@ -1267,12 +1248,6 @@ return array( 'phabricator-prefab', 'javelin-json', ), - '5fefb143' => array( - 'javelin-behavior', - 'javelin-dom', - 'javelin-workflow', - 'javelin-stratcom', - ), 60479091 => array( 'phabricator-busy', 'javelin-behavior', @@ -2291,9 +2266,6 @@ return array( ), 'maniphest.pkg.js' => array( 'javelin-behavior-maniphest-batch-selector', - 'javelin-behavior-maniphest-transaction-controls', - 'javelin-behavior-maniphest-transaction-preview', - 'javelin-behavior-maniphest-transaction-expand', 'javelin-behavior-maniphest-subpriority-editor', 'javelin-behavior-maniphest-list-editor', ), diff --git a/resources/celerity/packages.php b/resources/celerity/packages.php index adc001d0ec..bb97cb889e 100644 --- a/resources/celerity/packages.php +++ b/resources/celerity/packages.php @@ -187,9 +187,6 @@ return array( ), 'maniphest.pkg.js' => array( 'javelin-behavior-maniphest-batch-selector', - 'javelin-behavior-maniphest-transaction-controls', - 'javelin-behavior-maniphest-transaction-preview', - 'javelin-behavior-maniphest-transaction-expand', 'javelin-behavior-maniphest-subpriority-editor', 'javelin-behavior-maniphest-list-editor', ), diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 3142b3399c..dad5140e91 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1328,9 +1328,7 @@ phutil_register_library_map(array( 'ManiphestTransaction' => 'applications/maniphest/storage/ManiphestTransaction.php', 'ManiphestTransactionComment' => 'applications/maniphest/storage/ManiphestTransactionComment.php', 'ManiphestTransactionEditor' => 'applications/maniphest/editor/ManiphestTransactionEditor.php', - 'ManiphestTransactionPreviewController' => 'applications/maniphest/controller/ManiphestTransactionPreviewController.php', 'ManiphestTransactionQuery' => 'applications/maniphest/query/ManiphestTransactionQuery.php', - 'ManiphestTransactionSaveController' => 'applications/maniphest/controller/ManiphestTransactionSaveController.php', 'ManiphestUpdateConduitAPIMethod' => 'applications/maniphest/conduit/ManiphestUpdateConduitAPIMethod.php', 'ManiphestView' => 'applications/maniphest/view/ManiphestView.php', 'MetaMTAEmailTransactionCommand' => 'applications/metamta/command/MetaMTAEmailTransactionCommand.php', @@ -5320,9 +5318,7 @@ phutil_register_library_map(array( 'ManiphestTransaction' => 'PhabricatorApplicationTransaction', 'ManiphestTransactionComment' => 'PhabricatorApplicationTransactionComment', 'ManiphestTransactionEditor' => 'PhabricatorApplicationTransactionEditor', - 'ManiphestTransactionPreviewController' => 'ManiphestController', 'ManiphestTransactionQuery' => 'PhabricatorApplicationTransactionQuery', - 'ManiphestTransactionSaveController' => 'ManiphestController', 'ManiphestUpdateConduitAPIMethod' => 'ManiphestConduitAPIMethod', 'ManiphestView' => 'AphrontView', 'MetaMTAEmailTransactionCommand' => 'Phobject', diff --git a/src/applications/maniphest/controller/ManiphestTaskDetailController.php b/src/applications/maniphest/controller/ManiphestTaskDetailController.php index d5fab7f9f5..3d6c0df65d 100644 --- a/src/applications/maniphest/controller/ManiphestTaskDetailController.php +++ b/src/applications/maniphest/controller/ManiphestTaskDetailController.php @@ -118,229 +118,17 @@ final class ManiphestTaskDetailController extends ManiphestController { new ManiphestTransactionQuery(), $engine); - $resolution_types = ManiphestTaskStatus::getTaskStatusMap(); - - $transaction_types = array( - PhabricatorTransactions::TYPE_COMMENT => pht('Comment'), - ManiphestTransaction::TYPE_STATUS => pht('Change Status'), - ManiphestTransaction::TYPE_OWNER => pht('Reassign / Claim'), - PhabricatorTransactions::TYPE_SUBSCRIBERS => pht('Add CCs'), - ManiphestTransaction::TYPE_PRIORITY => pht('Change Priority'), - PhabricatorTransactions::TYPE_EDGE => pht('Associate Projects'), - ); - - // Remove actions the user doesn't have permission to take. - - $requires = array( - ManiphestTransaction::TYPE_OWNER => - ManiphestEditAssignCapability::CAPABILITY, - ManiphestTransaction::TYPE_PRIORITY => - ManiphestEditPriorityCapability::CAPABILITY, - PhabricatorTransactions::TYPE_EDGE => - ManiphestEditProjectsCapability::CAPABILITY, - ManiphestTransaction::TYPE_STATUS => - ManiphestEditStatusCapability::CAPABILITY, - ); - - foreach ($transaction_types as $type => $name) { - if (isset($requires[$type])) { - if (!$this->hasApplicationCapability($requires[$type])) { - unset($transaction_types[$type]); - } - } - } - - // Don't show an option to change to the current status, or to change to - // the duplicate status explicitly. - unset($resolution_types[$task->getStatus()]); - unset($resolution_types[ManiphestTaskStatus::getDuplicateStatus()]); - - // Don't show owner/priority changes for closed tasks, as they don't make - // much sense. - if ($task->isClosed()) { - unset($transaction_types[ManiphestTransaction::TYPE_PRIORITY]); - unset($transaction_types[ManiphestTransaction::TYPE_OWNER]); - } - - $default_claim = array( - $viewer->getPHID() => $viewer->getUsername(). - ' ('.$viewer->getRealName().')', - ); - - $draft = id(new PhabricatorDraft())->loadOneWhere( - 'authorPHID = %s AND draftKey = %s', - $viewer->getPHID(), - $task->getPHID()); - if ($draft) { - $draft_text = $draft->getDraft(); - } else { - $draft_text = null; - } - - $projects_source = new PhabricatorProjectDatasource(); - $users_source = new PhabricatorPeopleDatasource(); - $mailable_source = new PhabricatorMetaMTAMailableDatasource(); - - $comment_form = new AphrontFormView(); - $comment_form - ->setUser($viewer) - ->setWorkflow(true) - ->setAction('/maniphest/transaction/save/') - ->setEncType('multipart/form-data') - ->addHiddenInput('taskID', $task->getID()) - ->appendChild( - id(new AphrontFormSelectControl()) - ->setLabel(pht('Action')) - ->setName('action') - ->setOptions($transaction_types) - ->setID('transaction-action')) - ->appendChild( - id(new AphrontFormSelectControl()) - ->setLabel(pht('Status')) - ->setName('resolution') - ->setControlID('resolution') - ->setControlStyle('display: none') - ->setOptions($resolution_types)) - ->appendControl( - id(new AphrontFormTokenizerControl()) - ->setLabel(pht('Assign To')) - ->setName('assign_to') - ->setControlID('assign_to') - ->setControlStyle('display: none') - ->setID('assign-tokenizer') - ->setDisableBehavior(true) - ->setDatasource($users_source)) - ->appendControl( - id(new AphrontFormTokenizerControl()) - ->setLabel(pht('CCs')) - ->setName('ccs') - ->setControlID('ccs') - ->setControlStyle('display: none') - ->setID('cc-tokenizer') - ->setDisableBehavior(true) - ->setDatasource($mailable_source)) - ->appendChild( - id(new AphrontFormSelectControl()) - ->setLabel(pht('Priority')) - ->setName('priority') - ->setOptions($priority_map) - ->setControlID('priority') - ->setControlStyle('display: none') - ->setValue($task->getPriority())) - ->appendControl( - id(new AphrontFormTokenizerControl()) - ->setLabel(pht('Projects')) - ->setName('projects') - ->setControlID('projects') - ->setControlStyle('display: none') - ->setID('projects-tokenizer') - ->setDisableBehavior(true) - ->setDatasource($projects_source)) - ->appendChild( - id(new AphrontFormFileControl()) - ->setLabel(pht('File')) - ->setName('file') - ->setControlID('file') - ->setControlStyle('display: none')) - ->appendChild( - id(new PhabricatorRemarkupControl()) - ->setUser($viewer) - ->setLabel(pht('Comments')) - ->setName('comments') - ->setValue($draft_text) - ->setID('transaction-comments') - ->setUser($viewer)) - ->appendChild( - id(new AphrontFormSubmitControl()) - ->setValue(pht('Submit'))); - - $control_map = array( - ManiphestTransaction::TYPE_STATUS => 'resolution', - ManiphestTransaction::TYPE_OWNER => 'assign_to', - PhabricatorTransactions::TYPE_SUBSCRIBERS => 'ccs', - ManiphestTransaction::TYPE_PRIORITY => 'priority', - PhabricatorTransactions::TYPE_EDGE => 'projects', - ); - - $tokenizer_map = array( - PhabricatorTransactions::TYPE_EDGE => array( - 'id' => 'projects-tokenizer', - 'src' => $projects_source->getDatasourceURI(), - 'placeholder' => $projects_source->getPlaceholderText(), - ), - ManiphestTransaction::TYPE_OWNER => array( - 'id' => 'assign-tokenizer', - 'src' => $users_source->getDatasourceURI(), - 'value' => $default_claim, - 'limit' => 1, - 'placeholder' => $users_source->getPlaceholderText(), - ), - PhabricatorTransactions::TYPE_SUBSCRIBERS => array( - 'id' => 'cc-tokenizer', - 'src' => $mailable_source->getDatasourceURI(), - 'placeholder' => $mailable_source->getPlaceholderText(), - ), - ); - - // TODO: Initializing these behaviors for logged out users fatals things. - if ($viewer->isLoggedIn()) { - Javelin::initBehavior('maniphest-transaction-controls', array( - 'select' => 'transaction-action', - 'controlMap' => $control_map, - 'tokenizers' => $tokenizer_map, - )); - - Javelin::initBehavior('maniphest-transaction-preview', array( - 'uri' => '/maniphest/transaction/preview/'.$task->getID().'/', - 'preview' => 'transaction-preview', - 'comments' => 'transaction-comments', - 'action' => 'transaction-action', - 'map' => $control_map, - 'tokenizers' => $tokenizer_map, - )); - } - - $is_serious = PhabricatorEnv::getEnvConfig('phabricator.serious-business'); - $comment_header = $is_serious - ? pht('Add Comment') - : pht('Weigh In'); - - $preview_panel = phutil_tag_div( - 'aphront-panel-preview', - phutil_tag( - 'div', - array('id' => 'transaction-preview'), - phutil_tag_div( - 'aphront-panel-preview-loading-text', - pht('Loading preview...')))); - - $object_name = 'T'.$task->getID(); $actions = $this->buildActionView($task); + $monogram = $task->getMonogram(); $crumbs = $this->buildApplicationCrumbs() - ->addTextCrumb($object_name, '/'.$object_name); + ->addTextCrumb($monogram, '/'.$monogram); $header = $this->buildHeaderView($task); $properties = $this->buildPropertyView( $task, $field_list, $edges, $actions, $handles); $description = $this->buildDescriptionView($task, $engine); - if (!$viewer->isLoggedIn()) { - // TODO: Eventually, everything should run through this. For now, we're - // only using it to get a consistent "Login to Comment" button. - $comment_box = id(new PhabricatorApplicationTransactionCommentView()) - ->setUser($viewer) - ->setRequestURI($request->getRequestURI()); - $preview_panel = null; - } else { - $comment_box = id(new PHUIObjectBoxView()) - ->setFlush(true) - ->setHeaderText($comment_header) - ->setForm($comment_form); - $timeline->setQuoteTargetID('transaction-comments'); - $timeline->setQuoteRef($object_name); - } - $object_box = id(new PHUIObjectBoxView()) ->setHeader($header) ->addPropertyList($properties); @@ -349,7 +137,11 @@ final class ManiphestTaskDetailController extends ManiphestController { $object_box->addPropertyList($description); } - $title = 'T'.$task->getID().' '.$task->getTitle(); + $title = pht('%s %s', $monogram, $task->getTitle()); + + $comment_view = id(new ManiphestEditEngine()) + ->setViewer($viewer) + ->buildEditEngineCommentView($task); return $this->newPage() ->setTitle($title) @@ -363,8 +155,7 @@ final class ManiphestTaskDetailController extends ManiphestController { $info_view, $object_box, $timeline, - $comment_box, - $preview_panel, + $comment_view, )); } diff --git a/src/applications/maniphest/controller/ManiphestTransactionPreviewController.php b/src/applications/maniphest/controller/ManiphestTransactionPreviewController.php deleted file mode 100644 index 65756ca8e4..0000000000 --- a/src/applications/maniphest/controller/ManiphestTransactionPreviewController.php +++ /dev/null @@ -1,128 +0,0 @@ -getViewer(); - $id = $request->getURIData('id'); - - $comments = $request->getStr('comments'); - - $task = id(new ManiphestTaskQuery()) - ->setViewer($viewer) - ->withIDs(array($id)) - ->executeOne(); - if (!$task) { - return new Aphront404Response(); - } - - id(new PhabricatorDraft()) - ->setAuthorPHID($viewer->getPHID()) - ->setDraftKey($task->getPHID()) - ->setDraft($comments) - ->replaceOrDelete(); - - $action = $request->getStr('action'); - - $transaction = new ManiphestTransaction(); - $transaction->setAuthorPHID($viewer->getPHID()); - $transaction->setTransactionType($action); - - // This should really be split into a separate transaction, but it should - // all come out in the wash once we fully move to modern stuff. - $transaction->attachComment( - id(new ManiphestTransactionComment()) - ->setContent($comments)); - - $value = $request->getStr('value'); - // grab phids for handles and set transaction values based on action and - // value (empty or control-specific format) coming in from the wire - switch ($action) { - case ManiphestTransaction::TYPE_PRIORITY: - $transaction->setOldValue($task->getPriority()); - $transaction->setNewValue($value); - break; - case ManiphestTransaction::TYPE_OWNER: - if ($value) { - $value = current(json_decode($value)); - $phids = array($value); - } else { - $phids = array(); - } - $transaction->setNewValue($value); - break; - case PhabricatorTransactions::TYPE_SUBSCRIBERS: - if ($value) { - $value = json_decode($value); - } - if (!$value) { - $value = array(); - } - $phids = array(); - foreach ($value as $cc_phid) { - $phids[] = $cc_phid; - } - $transaction->setOldValue(array()); - $transaction->setNewValue($phids); - break; - case PhabricatorTransactions::TYPE_EDGE: - if ($value) { - $value = phutil_json_decode($value); - } - if (!$value) { - $value = array(); - } - - $phids = array(); - $value = array_fuse($value); - foreach ($value as $project_phid) { - $phids[] = $project_phid; - $value[$project_phid] = array('dst' => $project_phid); - } - - $project_type = PhabricatorProjectObjectHasProjectEdgeType::EDGECONST; - $transaction - ->setTransactionType(PhabricatorTransactions::TYPE_EDGE) - ->setMetadataValue('edge:type', $project_type) - ->setOldValue(array()) - ->setNewValue($value); - break; - case ManiphestTransaction::TYPE_STATUS: - $phids = array(); - $transaction->setOldValue($task->getStatus()); - $transaction->setNewValue($value); - break; - default: - $phids = array(); - $transaction->setNewValue($value); - break; - } - $phids[] = $viewer->getPHID(); - - $handles = $this->loadViewerHandles($phids); - - $transactions = array(); - $transactions[] = $transaction; - - $engine = new PhabricatorMarkupEngine(); - $engine->setViewer($viewer); - $engine->setContextObject($task); - if ($transaction->hasComment()) { - $engine->addObject( - $transaction->getComment(), - PhabricatorApplicationTransactionComment::MARKUP_FIELD_COMMENT); - } - $engine->process(); - - $transaction->setHandles($handles); - - $view = id(new PhabricatorApplicationTransactionView()) - ->setUser($viewer) - ->setTransactions($transactions) - ->setIsPreview(true); - - return id(new AphrontAjaxResponse()) - ->setContent((string)phutil_implode_html('', $view->buildEvents())); - } - -} diff --git a/src/applications/maniphest/controller/ManiphestTransactionSaveController.php b/src/applications/maniphest/controller/ManiphestTransactionSaveController.php deleted file mode 100644 index b060b18cab..0000000000 --- a/src/applications/maniphest/controller/ManiphestTransactionSaveController.php +++ /dev/null @@ -1,209 +0,0 @@ -getViewer(); - - $task = id(new ManiphestTaskQuery()) - ->setViewer($viewer) - ->withIDs(array($request->getStr('taskID'))) - ->needSubscriberPHIDs(true) - ->needProjectPHIDs(true) - ->executeOne(); - if (!$task) { - return new Aphront404Response(); - } - - $task_uri = '/'.$task->getMonogram(); - - $transactions = array(); - - $action = $request->getStr('action'); - - $implicit_ccs = array(); - $explicit_ccs = array(); - - $transaction = new ManiphestTransaction(); - $transaction - ->setTransactionType($action); - - switch ($action) { - case ManiphestTransaction::TYPE_STATUS: - $transaction->setNewValue($request->getStr('resolution')); - break; - case ManiphestTransaction::TYPE_OWNER: - $assign_to = $request->getArr('assign_to'); - $assign_to = reset($assign_to); - $transaction->setNewValue($assign_to); - break; - case PhabricatorTransactions::TYPE_EDGE: - $projects = $request->getArr('projects'); - $projects = array_merge($projects, $task->getProjectPHIDs()); - $projects = array_filter($projects); - $projects = array_unique($projects); - - $project_type = PhabricatorProjectObjectHasProjectEdgeType::EDGECONST; - $transaction - ->setMetadataValue('edge:type', $project_type) - ->setNewValue( - array( - '+' => array_fuse($projects), - )); - break; - case PhabricatorTransactions::TYPE_SUBSCRIBERS: - // Accumulate the new explicit CCs into the array that we'll add in - // the CC transaction later. - $explicit_ccs = $request->getArr('ccs'); - - // Throw away the primary transaction. - $transaction = null; - break; - case ManiphestTransaction::TYPE_PRIORITY: - $transaction->setNewValue($request->getInt('priority')); - break; - case PhabricatorTransactions::TYPE_COMMENT: - // Nuke this, we're going to create it below. - $transaction = null; - break; - default: - throw new Exception(pht("Unknown action '%s'!", $action)); - } - - if ($transaction) { - $transactions[] = $transaction; - } - - - // When you interact with a task, we add you to the CC list so you get - // further updates, and possibly assign the task to you if you took an - // ownership action (closing it) but it's currently unowned. We also move - // previous owners to CC if ownership changes. Detect all these conditions - // and create side-effect transactions for them. - - $implicitly_claimed = false; - if ($action == ManiphestTransaction::TYPE_OWNER) { - if ($task->getOwnerPHID() == $transaction->getNewValue()) { - // If this is actually no-op, don't generate the side effect. - } else { - // Otherwise, when a task is reassigned, move the previous owner to CC. - if ($task->getOwnerPHID()) { - $implicit_ccs[] = $task->getOwnerPHID(); - } - } - } - - if ($action == ManiphestTransaction::TYPE_STATUS) { - $resolution = $request->getStr('resolution'); - if (!$task->getOwnerPHID() && - ManiphestTaskStatus::isClosedStatus($resolution)) { - // Closing an unassigned task. Assign the user as the owner of - // this task. - $assign = new ManiphestTransaction(); - $assign->setTransactionType(ManiphestTransaction::TYPE_OWNER); - $assign->setNewValue($viewer->getPHID()); - $transactions[] = $assign; - - $implicitly_claimed = true; - } - } - - $user_owns_task = false; - if ($implicitly_claimed) { - $user_owns_task = true; - } else { - if ($action == ManiphestTransaction::TYPE_OWNER) { - if ($transaction->getNewValue() == $viewer->getPHID()) { - $user_owns_task = true; - } - } else if ($task->getOwnerPHID() == $viewer->getPHID()) { - $user_owns_task = true; - } - } - - if (!$user_owns_task) { - // If we aren't making the user the new task owner and they aren't the - // existing task owner, add them to CC unless they're aleady CC'd. - if (!in_array($viewer->getPHID(), $task->getSubscriberPHIDs())) { - $implicit_ccs[] = $viewer->getPHID(); - } - } - - if ($implicit_ccs || $explicit_ccs) { - - // TODO: These implicit CC rules should probably be handled inside the - // Editor, eventually. - - $all_ccs = array_fuse($implicit_ccs) + array_fuse($explicit_ccs); - - $cc_transaction = id(new ManiphestTransaction()) - ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS) - ->setNewValue(array('+' => $all_ccs)); - - if (!$explicit_ccs) { - $cc_transaction->setIgnoreOnNoEffect(true); - } - - $transactions[] = $cc_transaction; - } - - $comments = $request->getStr('comments'); - if (strlen($comments) || !$transactions) { - $transactions[] = id(new ManiphestTransaction()) - ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT) - ->attachComment( - id(new ManiphestTransactionComment()) - ->setContent($comments)); - } - - $event = new PhabricatorEvent( - PhabricatorEventType::TYPE_MANIPHEST_WILLEDITTASK, - array( - 'task' => $task, - 'new' => false, - 'transactions' => $transactions, - )); - $event->setUser($viewer); - $event->setAphrontRequest($request); - PhutilEventEngine::dispatchEvent($event); - - $task = $event->getValue('task'); - $transactions = $event->getValue('transactions'); - - $editor = id(new ManiphestTransactionEditor()) - ->setActor($viewer) - ->setContentSourceFromRequest($request) - ->setContinueOnMissingFields(true) - ->setContinueOnNoEffect($request->isContinueRequest()); - - try { - $editor->applyTransactions($task, $transactions); - } catch (PhabricatorApplicationTransactionNoEffectException $ex) { - return id(new PhabricatorApplicationTransactionNoEffectResponse()) - ->setCancelURI($task_uri) - ->setException($ex); - } - - $draft = id(new PhabricatorDraft())->loadOneWhere( - 'authorPHID = %s AND draftKey = %s', - $viewer->getPHID(), - $task->getPHID()); - if ($draft) { - $draft->delete(); - } - - $event = new PhabricatorEvent( - PhabricatorEventType::TYPE_MANIPHEST_DIDEDITTASK, - array( - 'task' => $task, - 'new' => false, - 'transactions' => $transactions, - )); - $event->setUser($viewer); - $event->setAphrontRequest($request); - PhutilEventEngine::dispatchEvent($event); - - return id(new AphrontRedirectResponse())->setURI($task_uri); - } - -} diff --git a/src/applications/maniphest/editor/ManiphestEditEngine.php b/src/applications/maniphest/editor/ManiphestEditEngine.php index 2e436b33e2..a0f235b3a9 100644 --- a/src/applications/maniphest/editor/ManiphestEditEngine.php +++ b/src/applications/maniphest/editor/ManiphestEditEngine.php @@ -67,11 +67,21 @@ final class ManiphestEditEngine $priority_map = ManiphestTaskPriority::getTaskPriorityMap(); + // TODO: Restore these or toss them: + // - Require a single owner. + // - Default owner to viewer. + // - Don't show "change status" for closed tasks. + // - Don't show "change owner" for closed tasks. + // - Don't let users change a task status to "Duplicate". + // - Make sure "Quote" works. + // - When closing an unassigned task, assign the closing user. + // - Make sure implicit CCs on actions are working reasonably. + return array( id(new PhabricatorTextEditField()) ->setKey('title') ->setLabel(pht('Title')) - ->setDescription(pht('Name of the paste.')) + ->setDescription(pht('Name of the task.')) ->setTransactionType(ManiphestTransaction::TYPE_TITLE) ->setIsRequired(true) ->setValue($object->getTitle()), diff --git a/webroot/rsrc/js/application/maniphest/behavior-transaction-controls.js b/webroot/rsrc/js/application/maniphest/behavior-transaction-controls.js deleted file mode 100644 index 48e6c43a83..0000000000 --- a/webroot/rsrc/js/application/maniphest/behavior-transaction-controls.js +++ /dev/null @@ -1,35 +0,0 @@ -/** - * @provides javelin-behavior-maniphest-transaction-controls - * @requires javelin-behavior - * javelin-dom - * phabricator-prefab - */ - -JX.behavior('maniphest-transaction-controls', function(config) { - - var tokenizers = {}; - - for (var k in config.tokenizers) { - var tconfig = config.tokenizers[k]; - tokenizers[k] = JX.Prefab.buildTokenizer(tconfig).tokenizer; - tokenizers[k].start(); - } - - JX.DOM.listen( - JX.$(config.select), - 'change', - null, - function() { - for (var k in config.controlMap) { - if (k == JX.$(config.select).value) { - JX.DOM.show(JX.$(config.controlMap[k])); - if (tokenizers[k]) { - tokenizers[k].refresh(); - } - } else { - JX.DOM.hide(JX.$(config.controlMap[k])); - } - } - }); - -}); diff --git a/webroot/rsrc/js/application/maniphest/behavior-transaction-expand.js b/webroot/rsrc/js/application/maniphest/behavior-transaction-expand.js deleted file mode 100644 index ad0d3af122..0000000000 --- a/webroot/rsrc/js/application/maniphest/behavior-transaction-expand.js +++ /dev/null @@ -1,29 +0,0 @@ -/** - * @provides javelin-behavior-maniphest-transaction-expand - * @requires javelin-behavior - * javelin-dom - * javelin-workflow - * javelin-stratcom - */ - -/** - * When the user clicks "show details" in a Maniphest transaction, replace the - * summary rendering with a detailed rendering. - */ -JX.behavior('maniphest-transaction-expand', function() { - - JX.Stratcom.listen( - 'click', - 'maniphest-expand-transaction', - function(e) { - e.kill(); - JX.Workflow.newFromLink(e.getTarget(), {}) - .setHandler(function(r) { - JX.DOM.setContent( - e.getNode('maniphest-transaction-description'), - JX.$H(r)); - }) - .start(); - }); - -}); diff --git a/webroot/rsrc/js/application/maniphest/behavior-transaction-preview.js b/webroot/rsrc/js/application/maniphest/behavior-transaction-preview.js deleted file mode 100644 index 66b58b1f4b..0000000000 --- a/webroot/rsrc/js/application/maniphest/behavior-transaction-preview.js +++ /dev/null @@ -1,76 +0,0 @@ -/** - * @provides javelin-behavior-maniphest-transaction-preview - * @requires javelin-behavior - * javelin-dom - * javelin-util - * javelin-json - * javelin-stratcom - * phabricator-shaped-request - */ - -JX.behavior('maniphest-transaction-preview', function(config) { - - var comments = JX.$(config.comments); - var action = JX.$(config.action); - - var callback = function(r) { - var panel = JX.$(config.preview); - var data = getdata(); - var hide = true; - for (var field in data) { - if (field == 'action') { - continue; - } - if (data[field]) { - hide = false; - } - } - if (hide) { - JX.DOM.hide(panel); - } else { - JX.DOM.setContent(panel, JX.$H(r)); - JX.DOM.show(panel); - } - }; - - var getdata = function() { - var selected = action.value; - - var value = null; - try { - var control = JX.$(config.map[selected]); - var input = ([] - .concat(JX.DOM.scry(control, 'select')) - .concat(JX.DOM.scry(control, 'input')))[0]; - if (JX.DOM.isType(input, 'input')) { - // Avoid reading 'value'(s) out of the tokenizer free text input. - if (input.type != 'hidden') { - value = null; - // Get the tokenizer and all that delicious data - } else { - var tokenizer_dom = JX.$(config.tokenizers[selected].id); - var tokenizer = JX.Stratcom.getData(tokenizer_dom).tokenizer; - value = JX.JSON.stringify(JX.keys(tokenizer.getTokens())); - } - } else { - value = input.value; - } - } catch (_ignored_) { - // Ignored. - } - - return { - comments : comments.value, - action : selected, - value : value || '' - }; - }; - - var request = new JX.PhabricatorShapedRequest(config.uri, callback, getdata); - var trigger = JX.bind(request, request.trigger); - - JX.DOM.listen(comments, 'keydown', null, trigger); - JX.DOM.listen(action, 'change', null, trigger); - - request.start(); -});