From e78df59ced7ca2fec68553f28ef6ee5b5fd29d87 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Tue, 4 Mar 2014 17:01:33 -0800 Subject: [PATCH] Maniphest Tasks + Project Boards - some polish Summary: Fixes T4550 by changing supportsFeed to shouldPublishFeedStory, so things can be more granular like that are with mail. Attempts to fix things generally too, filtering out xactions that have no business in feed, etc. Also return an updated Task HTML representation on drag and drop moves, etc. This is important so if the priority changes you can see it reflected in the UI. Test Plan: dragged tasks around. observed no feed stories on subpriority drags. observed feed stories and updated color bars on stories that changed priority Reviewers: epriestley, chad Reviewed By: epriestley CC: Korvin, epriestley, aran Maniphest Tasks: T4550 Differential Revision: https://secure.phabricator.com/D8399 --- .../conpherence/editor/ConpherenceEditor.php | 4 +++- .../editor/DifferentialTransactionEditor.php | 4 +++- .../files/editor/PhabricatorFileEditor.php | 4 +++- .../editor/LegalpadDocumentEditor.php | 4 +++- .../macro/editor/PhabricatorMacroEditor.php | 4 +++- .../editor/ManiphestTransactionEditor.php | 21 ++++++------------- .../storage/ManiphestTransaction.php | 2 +- .../paste/editor/PhabricatorPasteEditor.php | 4 +++- .../pholio/editor/PholioMockEditor.php | 4 +++- .../ponder/editor/PonderEditor.php | 4 +++- .../PhabricatorProjectMoveController.php | 19 +++++++++++++++-- ...habricatorApplicationTransactionEditor.php | 7 +++++-- .../PhabricatorApplicationTransaction.php | 4 ++++ .../projects/behavior-project-boards.js | 11 +++++----- 14 files changed, 62 insertions(+), 34 deletions(-) diff --git a/src/applications/conpherence/editor/ConpherenceEditor.php b/src/applications/conpherence/editor/ConpherenceEditor.php index 06f1537d87..249a87836e 100644 --- a/src/applications/conpherence/editor/ConpherenceEditor.php +++ b/src/applications/conpherence/editor/ConpherenceEditor.php @@ -400,7 +400,9 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { return PhabricatorEnv::getEnvConfig('metamta.conpherence.subject-prefix'); } - protected function supportsFeed() { + protected function shouldPublishFeedStory( + PhabricatorLiskDAO $object, + array $xactions) { return false; } diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php index a21bbf9794..86dde6bd94 100644 --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -785,7 +785,9 @@ final class DifferentialTransactionEditor return parent::requireCapabilities($object, $xaction); } - protected function supportsFeed() { + protected function shouldPublishFeedStory( + PhabricatorLiskDAO $object, + array $xactions) { return true; } diff --git a/src/applications/files/editor/PhabricatorFileEditor.php b/src/applications/files/editor/PhabricatorFileEditor.php index 7201322ffa..8d9c773cda 100644 --- a/src/applications/files/editor/PhabricatorFileEditor.php +++ b/src/applications/files/editor/PhabricatorFileEditor.php @@ -80,7 +80,9 @@ final class PhabricatorFileEditor return $body; } - protected function supportsFeed() { + protected function shouldPublishFeedStory( + PhabricatorLiskDAO $object, + array $xactions) { return true; } diff --git a/src/applications/legalpad/editor/LegalpadDocumentEditor.php b/src/applications/legalpad/editor/LegalpadDocumentEditor.php index 9a5c9e97bb..f24926c086 100644 --- a/src/applications/legalpad/editor/LegalpadDocumentEditor.php +++ b/src/applications/legalpad/editor/LegalpadDocumentEditor.php @@ -183,7 +183,9 @@ final class LegalpadDocumentEditor } - protected function supportsFeed() { + protected function shouldPublishFeedStory( + PhabricatorLiskDAO $object, + array $xactions) { return false; } diff --git a/src/applications/macro/editor/PhabricatorMacroEditor.php b/src/applications/macro/editor/PhabricatorMacroEditor.php index cbf7ccec83..ea99270cc7 100644 --- a/src/applications/macro/editor/PhabricatorMacroEditor.php +++ b/src/applications/macro/editor/PhabricatorMacroEditor.php @@ -156,7 +156,9 @@ final class PhabricatorMacroEditor return PhabricatorEnv::getEnvConfig('metamta.macro.subject-prefix'); } - protected function supportsFeed() { + protected function shouldPublishFeedStory( + PhabricatorLiskDAO $object, + array $xactions) { return true; } } diff --git a/src/applications/maniphest/editor/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/ManiphestTransactionEditor.php index 5225f5ea06..2c9463f928 100644 --- a/src/applications/maniphest/editor/ManiphestTransactionEditor.php +++ b/src/applications/maniphest/editor/ManiphestTransactionEditor.php @@ -251,19 +251,8 @@ final class ManiphestTransactionEditor PhabricatorLiskDAO $object, array $xactions) { - $should_mail = true; - if (count($xactions) == 1) { - $xaction = head($xactions); - switch ($xaction->getTransactionType()) { - case ManiphestTransaction::TYPE_SUBPRIORITY: - $should_mail = false; - break; - default: - $should_mail = true; - break; - } - } - return $should_mail; + $xactions = mfilter($xactions, 'shouldHide', true); + return $xactions; } protected function getMailSubjectPrefix() { @@ -318,8 +307,10 @@ final class ManiphestTransactionEditor return $body; } - protected function supportsFeed() { - return true; + protected function shouldPublishFeedStory( + PhabricatorLiskDAO $object, + array $xactions) { + return $this->shouldSendMail($object, $xactions); } protected function supportsSearch() { diff --git a/src/applications/maniphest/storage/ManiphestTransaction.php b/src/applications/maniphest/storage/ManiphestTransaction.php index d769c96831..4e20810c88 100644 --- a/src/applications/maniphest/storage/ManiphestTransaction.php +++ b/src/applications/maniphest/storage/ManiphestTransaction.php @@ -109,7 +109,7 @@ final class ManiphestTransaction return true; } - return false; + return parent::shouldHide(); } public function getActionStrength() { diff --git a/src/applications/paste/editor/PhabricatorPasteEditor.php b/src/applications/paste/editor/PhabricatorPasteEditor.php index 4ae98a7cf0..459f3b693c 100644 --- a/src/applications/paste/editor/PhabricatorPasteEditor.php +++ b/src/applications/paste/editor/PhabricatorPasteEditor.php @@ -157,7 +157,9 @@ final class PhabricatorPasteEditor ->addHeader('Thread-Topic', "P{$id}"); } - protected function supportsFeed() { + protected function shouldPublishFeedStory( + PhabricatorLiskDAO $object, + array $xactions) { return true; } diff --git a/src/applications/pholio/editor/PholioMockEditor.php b/src/applications/pholio/editor/PholioMockEditor.php index 3597c286cb..25a1369bec 100644 --- a/src/applications/pholio/editor/PholioMockEditor.php +++ b/src/applications/pholio/editor/PholioMockEditor.php @@ -402,7 +402,9 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor { return PhabricatorEnv::getEnvConfig('metamta.pholio.subject-prefix'); } - protected function supportsFeed() { + protected function shouldPublishFeedStory( + PhabricatorLiskDAO $object, + array $xactions) { return true; } diff --git a/src/applications/ponder/editor/PonderEditor.php b/src/applications/ponder/editor/PonderEditor.php index 94f75fd245..50bbd86645 100644 --- a/src/applications/ponder/editor/PonderEditor.php +++ b/src/applications/ponder/editor/PonderEditor.php @@ -3,7 +3,9 @@ abstract class PonderEditor extends PhabricatorApplicationTransactionEditor { - protected function supportsFeed() { + protected function shouldPublishFeedStory( + PhabricatorLiskDAO $object, + array $xactions) { return true; } diff --git a/src/applications/project/controller/PhabricatorProjectMoveController.php b/src/applications/project/controller/PhabricatorProjectMoveController.php index 105e81cdeb..906ec3a267 100644 --- a/src/applications/project/controller/PhabricatorProjectMoveController.php +++ b/src/applications/project/controller/PhabricatorProjectMoveController.php @@ -105,7 +105,22 @@ final class PhabricatorProjectMoveController $editor->applyTransactions($object, $xactions); - return id(new AphrontAjaxResponse())->setContent(array()); - } + $owner = null; + if ($object->getOwnerPHID()) { + $owner = id(new PhabricatorHandleQuery()) + ->setViewer($viewer) + ->withPHIDs(array($object->getOwnerPHID())) + ->executeOne(); + } + $card = id(new ProjectBoardTaskCard()) + ->setViewer($viewer) + ->setTask($object) + ->setOwner($owner) + ->setCanEdit(true) + ->getItem(); + + return id(new AphrontAjaxResponse())->setContent( + array('task' => $card)); + } } diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index 365a34ddde..0d9cca753a 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -597,7 +597,7 @@ abstract class PhabricatorApplicationTransactionEditor ->queueDocumentForIndexing($object->getPHID()); } - if ($this->supportsFeed()) { + if ($this->shouldPublishFeedStory($object, $xactions)) { $mailed = array(); if ($mail) { $mailed = $mail->buildRecipientList(); @@ -1664,7 +1664,9 @@ abstract class PhabricatorApplicationTransactionEditor /** * @task feed */ - protected function supportsFeed() { + protected function shouldPublishFeedStory( + PhabricatorLiskDAO $object, + array $xactions) { return false; } @@ -1729,6 +1731,7 @@ abstract class PhabricatorApplicationTransactionEditor array $xactions, array $mailed_phids) { + $xactions = mfilter($xactions, 'shouldHideForFeed', true); $related_phids = $this->getFeedRelatedPHIDs($object, $xactions); $subscribed_phids = $this->getFeedNotifyPHIDs($object, $xactions); diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php index 047a862c77..a0adf0f76e 100644 --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -354,6 +354,10 @@ abstract class PhabricatorApplicationTransaction return $this->shouldHide(); } + public function shouldHideForFeed() { + return $this->shouldHide(); + } + public function getTitleForMail() { return id(clone $this)->setRenderingTarget('text')->getTitle(); } diff --git a/webroot/rsrc/js/application/projects/behavior-project-boards.js b/webroot/rsrc/js/application/projects/behavior-project-boards.js index 3dde45eec6..5f9c66f900 100644 --- a/webroot/rsrc/js/application/projects/behavior-project-boards.js +++ b/webroot/rsrc/js/application/projects/behavior-project-boards.js @@ -18,8 +18,10 @@ JX.behavior('project-boards', function(config) { JX.DOM.alterClass(node, 'project-column-empty', !this.findItems().length); } - function onresponse(response) { - + function onresponse(response, item, list) { + list.unlock(); + JX.DOM.alterClass(item, 'drag-sending', false); + JX.DOM.replace(item, JX.$H(response.task)); } function ondrop(list, item, after, from) { @@ -37,10 +39,7 @@ JX.behavior('project-boards', function(config) { var workflow = new JX.Workflow(config.moveURI, data) .setHandler(function(response) { - onresponse(response); - list.unlock(); - - JX.DOM.alterClass(item, 'drag-sending', false); + onresponse(response, item, list); }); workflow.start(); -- 2.51.2