From 5a3e3994f5824c4e793c34252e0121cab5bd1432 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 28 Jul 2013 15:02:18 -0700 Subject: [PATCH] Make PonderQuestionEditor use ApplicationTransactions Summary: Ref T3373. Make PonderQuestions editable and use transactions. This temporarily disables some stuff: - email; - feed; - comments; - voting. I'll restore those in followups and wait to land this until they're at least mostly back online. The transactions themselves also need more string/color/icon work. Test Plan: Created and edited questions. Viewed transactions. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T3373 Differential Revision: https://secure.phabricator.com/D6601 --- src/__phutil_library_map__.php | 8 +- .../PhabricatorApplicationPonder.php | 2 +- .../ponder/controller/PonderController.php | 2 +- ...r.php => PonderQuestionEditController.php} | 92 +++++++++++---- .../PonderQuestionViewController.php | 105 ++++++++++++----- .../ponder/editor/PonderQuestionEditor.php | 111 +++++++++++------- .../query/PonderQuestionTransactionQuery.php | 10 ++ .../storage/PonderQuestionTransaction.php | 3 + 8 files changed, 231 insertions(+), 102 deletions(-) rename src/applications/ponder/controller/{PonderQuestionAskController.php => PonderQuestionEditController.php} (51%) create mode 100644 src/applications/ponder/query/PonderQuestionTransactionQuery.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 8ca5efe709..757c4b10a7 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1895,8 +1895,8 @@ phutil_register_library_map(array( 'PonderPHIDTypeQuestion' => 'applications/ponder/phid/PonderPHIDTypeQuestion.php', 'PonderPostBodyView' => 'applications/ponder/view/PonderPostBodyView.php', 'PonderQuestion' => 'applications/ponder/storage/PonderQuestion.php', - 'PonderQuestionAskController' => 'applications/ponder/controller/PonderQuestionAskController.php', 'PonderQuestionDetailView' => 'applications/ponder/view/PonderQuestionDetailView.php', + 'PonderQuestionEditController' => 'applications/ponder/controller/PonderQuestionEditController.php', 'PonderQuestionEditor' => 'applications/ponder/editor/PonderQuestionEditor.php', 'PonderQuestionListController' => 'applications/ponder/controller/PonderQuestionListController.php', 'PonderQuestionMailReceiver' => 'applications/ponder/mail/PonderQuestionMailReceiver.php', @@ -1907,6 +1907,7 @@ phutil_register_library_map(array( 'PonderQuestionStatusController' => 'applications/ponder/controller/PonderQuestionStatusController.php', 'PonderQuestionTransaction' => 'applications/ponder/storage/PonderQuestionTransaction.php', 'PonderQuestionTransactionComment' => 'applications/ponder/storage/PonderQuestionTransactionComment.php', + 'PonderQuestionTransactionQuery' => 'applications/ponder/query/PonderQuestionTransactionQuery.php', 'PonderQuestionViewController' => 'applications/ponder/controller/PonderQuestionViewController.php', 'PonderRemarkupRule' => 'applications/ponder/remarkup/PonderRemarkupRule.php', 'PonderReplyHandler' => 'applications/ponder/mail/PonderReplyHandler.php', @@ -4025,9 +4026,9 @@ phutil_register_library_map(array( 4 => 'PhabricatorPolicyInterface', 5 => 'PhabricatorTokenReceiverInterface', ), - 'PonderQuestionAskController' => 'PonderController', 'PonderQuestionDetailView' => 'AphrontView', - 'PonderQuestionEditor' => 'PhabricatorEditor', + 'PonderQuestionEditController' => 'PonderController', + 'PonderQuestionEditor' => 'PhabricatorApplicationTransactionEditor', 'PonderQuestionListController' => array( 0 => 'PonderController', @@ -4041,6 +4042,7 @@ phutil_register_library_map(array( 'PonderQuestionStatusController' => 'PonderController', 'PonderQuestionTransaction' => 'PhabricatorApplicationTransaction', 'PonderQuestionTransactionComment' => 'PhabricatorApplicationTransactionComment', + 'PonderQuestionTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'PonderQuestionViewController' => 'PonderController', 'PonderRemarkupRule' => 'PhabricatorRemarkupRuleObject', 'PonderReplyHandler' => 'PhabricatorMailReplyHandler', diff --git a/src/applications/ponder/application/PhabricatorApplicationPonder.php b/src/applications/ponder/application/PhabricatorApplicationPonder.php index bae4d86d55..38b9c3d9ef 100644 --- a/src/applications/ponder/application/PhabricatorApplicationPonder.php +++ b/src/applications/ponder/application/PhabricatorApplicationPonder.php @@ -52,7 +52,7 @@ final class PhabricatorApplicationPonder extends PhabricatorApplication { '(?:query/(?P[^/]+)/)?' => 'PonderQuestionListController', 'answer/add/' => 'PonderAnswerSaveController', 'answer/preview/' => 'PonderAnswerPreviewController', - 'question/ask/' => 'PonderQuestionAskController', + 'question/edit/(?:(?P\d+)/)?' => 'PonderQuestionEditController', 'question/preview/' => 'PonderQuestionPreviewController', 'question/(?Popen|close)/(?P[1-9]\d*)/' => 'PonderQuestionStatusController', diff --git a/src/applications/ponder/controller/PonderController.php b/src/applications/ponder/controller/PonderController.php index f2941d7e84..fbefd35b33 100644 --- a/src/applications/ponder/controller/PonderController.php +++ b/src/applications/ponder/controller/PonderController.php @@ -23,7 +23,7 @@ abstract class PonderController extends PhabricatorController { ->addAction( id(new PHUIListItemView()) ->setName(pht('Create Question')) - ->setHref('/ponder/question/ask/') + ->setHref('/ponder/question/edit/') ->setIcon('create')); return $crumbs; diff --git a/src/applications/ponder/controller/PonderQuestionAskController.php b/src/applications/ponder/controller/PonderQuestionEditController.php similarity index 51% rename from src/applications/ponder/controller/PonderQuestionAskController.php rename to src/applications/ponder/controller/PonderQuestionEditController.php index b2293119ae..aa6165ff9c 100644 --- a/src/applications/ponder/controller/PonderQuestionAskController.php +++ b/src/applications/ponder/controller/PonderQuestionEditController.php @@ -1,25 +1,49 @@ id = idx($data, 'id'); + } public function processRequest() { $request = $this->getRequest(); $user = $request->getUser(); - $question = id(new PonderQuestion()) - ->setAuthorPHID($user->getPHID()) - ->setVoteCount(0) - ->setAnswerCount(0) - ->setHeat(0.0); + if ($this->id) { + $question = id(new PonderQuestionQuery()) + ->setViewer($user) + ->withIDs(array($this->id)) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) + ->executeOne(); + if (!$question) { + return new Aphront404Response(); + } + } else { + $question = id(new PonderQuestion()) + ->setStatus(PonderQuestionStatus::STATUS_OPEN) + ->setAuthorPHID($user->getPHID()) + ->setVoteCount(0) + ->setAnswerCount(0) + ->setHeat(0.0); + } + + $v_title = $question->getTitle(); + $v_content = $question->getContent(); $errors = array(); $e_title = true; if ($request->isFormPost()) { - $question->setTitle($request->getStr('title')); - $question->setContent($request->getStr('content')); - $question->setStatus(PonderQuestionStatus::STATUS_OPEN); + $v_title = $request->getStr('title'); + $v_content = $request->getStr('content'); - $len = phutil_utf8_strlen($question->getTitle()); + $len = phutil_utf8_strlen($v_title); if ($len < 1) { $errors[] = pht('Title must not be empty.'); $e_title = pht('Required'); @@ -29,17 +53,23 @@ final class PonderQuestionAskController extends PonderController { } if (!$errors) { - $content_source = PhabricatorContentSource::newForSource( - PhabricatorContentSource::SOURCE_WEB, - array( - 'ip' => $request->getRemoteAddr(), - )); - $question->setContentSource($content_source); + $template = id(new PonderQuestionTransaction()); + $xactions = array(); - id(new PonderQuestionEditor()) - ->setQuestion($question) + $xactions[] = id(clone $template) + ->setTransactionType(PonderQuestionTransaction::TYPE_TITLE) + ->setNewValue($v_title); + + $xactions[] = id(clone $template) + ->setTransactionType(PonderQuestionTransaction::TYPE_CONTENT) + ->setNewValue($v_content); + + $editor = id(new PonderQuestionEditor()) ->setActor($user) - ->save(); + ->setContentSourceFromRequest($request) + ->setContinueOnNoEffect(true); + + $editor->applyTransactions($question, $xactions); return id(new AphrontRedirectResponse()) ->setURI('/Q'.$question->getID()); @@ -60,13 +90,13 @@ final class PonderQuestionAskController extends PonderController { id(new AphrontFormTextControl()) ->setLabel(pht('Question')) ->setName('title') - ->setValue($question->getTitle()) + ->setValue($v_title) ->setError($e_title)) ->appendChild( id(new PhabricatorRemarkupControl()) ->setName('content') ->setID('content') - ->setValue($question->getContent()) + ->setValue($v_content) ->setLabel(pht('Description')) ->setUser($user)) ->appendChild( @@ -92,9 +122,21 @@ final class PonderQuestionAskController extends PonderController { )); $crumbs = $this->buildApplicationCrumbs(); - $crumbs->addCrumb( - id(new PhabricatorCrumbView()) - ->setName(pht('Ask Question'))); + + $id = $question->getID(); + if ($id) { + $crumbs->addCrumb( + id(new PhabricatorCrumbView()) + ->setName("Q{$id}") + ->setHref("/Q{$id}")); + $crumbs->addCrumb( + id(new PhabricatorCrumbView()) + ->setName(pht('Edit'))); + } else { + $crumbs->addCrumb( + id(new PhabricatorCrumbView()) + ->setName(pht('Ask Question'))); + } return $this->buildApplicationPage( array( @@ -104,9 +146,9 @@ final class PonderQuestionAskController extends PonderController { $preview, ), array( + 'title' => pht('Ask a Question'), 'device' => true, 'dust' => true, - 'title' => pht('Ask a Question'), )); } diff --git a/src/applications/ponder/controller/PonderQuestionViewController.php b/src/applications/ponder/controller/PonderQuestionViewController.php index 9086d6184e..089dafc5de 100644 --- a/src/applications/ponder/controller/PonderQuestionViewController.php +++ b/src/applications/ponder/controller/PonderQuestionViewController.php @@ -41,11 +41,7 @@ final class PonderQuestionViewController extends PonderController { $this->loadHandles($object_phids); $handles = $this->getLoadedHandles(); - $detail_panel = new PonderQuestionDetailView(); - $detail_panel - ->setQuestion($question) - ->setUser($user) - ->setHandles($handles); + $question_xactions = $this->buildQuestionTransactions($question); $responses_panel = new PonderAnswerListView(); $responses_panel @@ -79,7 +75,7 @@ final class PonderQuestionViewController extends PonderController { $header, $actions, $properties, - $detail_panel, + $question_xactions, $responses_panel, $answer_add_panel ), @@ -92,34 +88,48 @@ final class PonderQuestionViewController extends PonderController { private function buildActionListView(PonderQuestion $question) { $request = $this->getRequest(); - $user = $request->getUser(); + $viewer = $request->getUser(); - $action_list = id(new PhabricatorActionListView()) - ->setUser($user) + $id = $question->getID(); + + $can_edit = PhabricatorPolicyFilter::hasCapability( + $viewer, + $question, + PhabricatorPolicyCapability::CAN_EDIT); + + $view = id(new PhabricatorActionListView()) + ->setUser($request->getUser()) ->setObject($question) ->setObjectURI($request->getRequestURI()); - if ($user->getPhid() === $question->getAuthorPhid()) { - if ($question->getStatus() == PonderQuestionStatus::STATUS_OPEN) { - $name = pht("Close Question"); - $icon = "delete"; - $href = "close"; - } else { - $name = pht("Open Question"); - $icon = "enable"; - $href = "open"; - } - $action_list->addAction( - id(new PhabricatorActionView()) - ->setName($name) - ->setIcon($icon) - ->setRenderAsForm(true) - ->setHref( - $this->getApplicationURI( - "/question/{$href}/{$this->questionID}/"))); + $view->addAction( + id(new PhabricatorActionView()) + ->setIcon('edit') + ->setName(pht('Edit Question')) + ->setHref($this->getApplicationURI("/question/edit/{$id}/")) + ->setDisabled(!$can_edit) + ->setWorkflow(!$can_edit)); + + if ($question->getStatus() == PonderQuestionStatus::STATUS_OPEN) { + $name = pht("Close Question"); + $icon = "delete"; + $href = "close"; + } else { + $name = pht("Reopen Question"); + $icon = "enable"; + $href = "open"; } - return $action_list; + $view->addAction( + id(new PhabricatorActionView()) + ->setName($name) + ->setIcon($icon) + ->setRenderAsForm($can_edit) + ->setWorkflow(!$can_edit) + ->setDisabled(!$can_edit) + ->setHref($this->getApplicationURI("/question/{$href}/{$id}/"))); + + return $view; } private function buildPropertyListView( @@ -142,6 +152,45 @@ final class PonderQuestionViewController extends PonderController { pht('Created'), phabricator_datetime($question->getDateCreated(), $viewer)); + $view->invokeWillRenderEvent(); + + $view->addTextContent( + PhabricatorMarkupEngine::renderOneObject( + $question, + $question->getMarkupField(), + $viewer)); + + return $view; } + + private function buildQuestionTransactions(PonderQuestion $question) { + $viewer = $this->getRequest()->getUser(); + + $xactions = id(new PonderQuestionTransactionQuery()) + ->setViewer($viewer) + ->withObjectPHIDs(array($question->getPHID())) + ->execute(); + + $engine = id(new PhabricatorMarkupEngine()) + ->setViewer($viewer); + foreach ($xactions as $xaction) { + if ($xaction->getComment()) { + $engine->addObject( + $xaction->getComment(), + PhabricatorApplicationTransactionComment::MARKUP_FIELD_COMMENT); + } + } + $engine->process(); + + $timeline = id(new PhabricatorApplicationTransactionView()) + ->setUser($viewer) + ->setTransactions($xactions) + ->setMarkupEngine($engine); + + // TODO: Add comment form. + + return $timeline; + } + } diff --git a/src/applications/ponder/editor/PonderQuestionEditor.php b/src/applications/ponder/editor/PonderQuestionEditor.php index d99970b3c8..3106b4250f 100644 --- a/src/applications/ponder/editor/PonderQuestionEditor.php +++ b/src/applications/ponder/editor/PonderQuestionEditor.php @@ -1,53 +1,76 @@ question = $question; - return $this; + $types[] = PhabricatorTransactions::TYPE_COMMENT; + $types[] = PonderQuestionTransaction::TYPE_TITLE; + $types[] = PonderQuestionTransaction::TYPE_CONTENT; + + return $types; } + protected function getCustomTransactionOldValue( + PhabricatorLiskDAO $object, + PhabricatorApplicationTransaction $xaction) { - public function setShouldEmail($se) { - $this->shouldEmail = $se; - return $this; - } - - public function save() { - $actor = $this->requireActor(); - if (!$this->question) { - throw new Exception("Must set question before saving it"); - } - - $question = $this->question; - $question->save(); - - $question->attachRelated(); - id(new PhabricatorSearchIndexer()) - ->indexDocumentByPHID($question->getPHID()); - - // subscribe author and @mentions - $subeditor = id(new PhabricatorSubscriptionsEditor()) - ->setObject($question) - ->setActor($actor); - - $subeditor->subscribeExplicit(array($question->getAuthorPHID())); - - $content = $question->getContent(); - $at_mention_phids = PhabricatorMarkupEngine::extractPHIDsFromMentions( - array($content)); - $subeditor->subscribeImplicit($at_mention_phids); - $subeditor->save(); - - if ($this->shouldEmail && $at_mention_phids) { - id(new PonderMentionMail( - $question, - $question, - $actor)) - ->setToPHIDs($at_mention_phids) - ->send(); + switch ($xaction->getTransactionType()) { + case PonderQuestionTransaction::TYPE_TITLE: + return $object->getTitle(); + case PonderQuestionTransaction::TYPE_CONTENT: + return $object->getContent(); } } + + protected function getCustomTransactionNewValue( + PhabricatorLiskDAO $object, + PhabricatorApplicationTransaction $xaction) { + + switch ($xaction->getTransactionType()) { + case PonderQuestionTransaction::TYPE_TITLE: + case PonderQuestionTransaction::TYPE_CONTENT: + return $xaction->getNewValue(); + } + } + + protected function applyCustomInternalTransaction( + PhabricatorLiskDAO $object, + PhabricatorApplicationTransaction $xaction) { + + switch ($xaction->getTransactionType()) { + case PonderQuestionTransaction::TYPE_TITLE: + $object->setTitle($xaction->getNewValue()); + break; + case PonderQuestionTransaction::TYPE_CONTENT: + $object->setContent($xaction->getNewValue()); + break; + } + } + + protected function applyCustomExternalTransaction( + PhabricatorLiskDAO $object, + PhabricatorApplicationTransaction $xaction) { + return; + } + + protected function mergeTransactions( + PhabricatorApplicationTransaction $u, + PhabricatorApplicationTransaction $v) { + + $type = $u->getTransactionType(); + switch ($type) { + case PonderQuestionTransaction::TYPE_TITLE: + case PonderQuestionTransaction::TYPE_CONTENT: + return $v; + } + + return parent::mergeTransactions($u, $v); + } + + // TODO: Feed support + // TODO: Mail support + // TODO: Add/remove answers + } diff --git a/src/applications/ponder/query/PonderQuestionTransactionQuery.php b/src/applications/ponder/query/PonderQuestionTransactionQuery.php new file mode 100644 index 0000000000..6d36c0f540 --- /dev/null +++ b/src/applications/ponder/query/PonderQuestionTransactionQuery.php @@ -0,0 +1,10 @@ +