From cf9dc5d1894551e815c1132176e62f92a29e297b Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 28 Jul 2013 18:21:22 -0700 Subject: [PATCH] Fix bug when multiple comment forms appear on a single page Summary: Ref T3373. The submit listener doesn't properly scope the form it listens to right now, so several forms on the page mean that comments post to one of them more or less at random. Scope it properly by telling it which object PHID it is associated with. Test Plan: Made Question comments, saw comments Ajax in on the question itself rather than on an arbitrary answer. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T3373 Differential Revision: https://secure.phabricator.com/D6611 --- src/__celerity_resource_map__.php | 2 +- .../config/PhabricatorAuthEditController.php | 1 + .../PhabricatorConfigEditController.php | 1 + .../DiffusionRepositoryEditController.php | 1 + .../LegalpadDocumentViewController.php | 2 ++ .../PhabricatorMacroViewController.php | 2 ++ .../phlux/controller/PhluxViewController.php | 1 + .../controller/PholioMockViewController.php | 2 ++ .../PhortuneAccountViewController.php | 1 + .../PhortuneProductViewController.php | 1 + .../PonderQuestionViewController.php | 4 ++++ .../request/ReleephRequestViewController.php | 2 ++ .../PhabricatorSlowvotePollController.php | 2 ++ ...tionTransactionCommentHistoryController.php | 1 + ...icatorApplicationTransactionCommentView.php | 18 ++++++++++++++++++ .../PhabricatorApplicationTransactionView.php | 15 +++++++++++++++ src/view/form/AphrontFormView.php | 12 ++++++++++++ .../transactions/behavior-transaction-list.js | 9 ++++++++- 18 files changed, 75 insertions(+), 2 deletions(-) diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index b51268d22a..707ad5a948 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -2194,7 +2194,7 @@ celerity_register_resource_map(array( ), 'javelin-behavior-phabricator-transaction-list' => array( - 'uri' => '/res/e7a015a7/rsrc/js/application/transactions/behavior-transaction-list.js', + 'uri' => '/res/8d602093/rsrc/js/application/transactions/behavior-transaction-list.js', 'type' => 'js', 'requires' => array( diff --git a/src/applications/auth/controller/config/PhabricatorAuthEditController.php b/src/applications/auth/controller/config/PhabricatorAuthEditController.php index 5c5d0328da..fcd619a2c0 100644 --- a/src/applications/auth/controller/config/PhabricatorAuthEditController.php +++ b/src/applications/auth/controller/config/PhabricatorAuthEditController.php @@ -266,6 +266,7 @@ final class PhabricatorAuthEditController $xaction_view = id(new PhabricatorApplicationTransactionView()) ->setUser($viewer) + ->setObjectPHID($config->getPHID()) ->setTransactions($xactions); } diff --git a/src/applications/config/controller/PhabricatorConfigEditController.php b/src/applications/config/controller/PhabricatorConfigEditController.php index 5db9f813b4..0d7736d7a6 100644 --- a/src/applications/config/controller/PhabricatorConfigEditController.php +++ b/src/applications/config/controller/PhabricatorConfigEditController.php @@ -219,6 +219,7 @@ final class PhabricatorConfigEditController $xaction_view = id(new PhabricatorApplicationTransactionView()) ->setUser($user) + ->setObjectPHID($config_entry->getPHID()) ->setTransactions($xactions); return $this->buildApplicationPage( diff --git a/src/applications/diffusion/controller/DiffusionRepositoryEditController.php b/src/applications/diffusion/controller/DiffusionRepositoryEditController.php index 4078849385..f15e9ea6fa 100644 --- a/src/applications/diffusion/controller/DiffusionRepositoryEditController.php +++ b/src/applications/diffusion/controller/DiffusionRepositoryEditController.php @@ -51,6 +51,7 @@ final class DiffusionRepositoryEditController extends DiffusionController { $xaction_view = id(new PhabricatorApplicationTransactionView()) ->setUser($user) + ->setObjectPHID($repository->getPHID()) ->setTransactions($xactions) ->setMarkupEngine($engine); diff --git a/src/applications/legalpad/controller/LegalpadDocumentViewController.php b/src/applications/legalpad/controller/LegalpadDocumentViewController.php index 8aa70d2364..81597d3d4a 100644 --- a/src/applications/legalpad/controller/LegalpadDocumentViewController.php +++ b/src/applications/legalpad/controller/LegalpadDocumentViewController.php @@ -71,6 +71,7 @@ final class LegalpadDocumentViewController extends LegalpadController { $xaction_view = id(new LegalpadTransactionView()) ->setUser($this->getRequest()->getUser()) + ->setObjectPHID($document->getPHID()) ->setTransactions($xactions) ->setMarkupEngine($engine); @@ -209,6 +210,7 @@ final class LegalpadDocumentViewController extends LegalpadController { $form = id(new PhabricatorApplicationTransactionCommentView()) ->setUser($user) + ->setObjectPHID($document->getPHID()) ->setFormID($comment_form_id) ->setDraft($draft) ->setSubmitButtonName($button_name) diff --git a/src/applications/macro/controller/PhabricatorMacroViewController.php b/src/applications/macro/controller/PhabricatorMacroViewController.php index b521a67585..129b7de352 100644 --- a/src/applications/macro/controller/PhabricatorMacroViewController.php +++ b/src/applications/macro/controller/PhabricatorMacroViewController.php @@ -55,6 +55,7 @@ final class PhabricatorMacroViewController $timeline = id(new PhabricatorApplicationTransactionView()) ->setUser($user) + ->setObjectPHID($macro->getPHID()) ->setTransactions($xactions) ->setMarkupEngine($engine); @@ -85,6 +86,7 @@ final class PhabricatorMacroViewController $add_comment_form = id(new PhabricatorApplicationTransactionCommentView()) ->setUser($user) + ->setObjectPHID($macro->getPHID()) ->setDraft($draft) ->setAction($this->getApplicationURI('/comment/'.$macro->getID().'/')) ->setSubmitButtonName($submit_button_name); diff --git a/src/applications/phlux/controller/PhluxViewController.php b/src/applications/phlux/controller/PhluxViewController.php index 1df396747c..845e7a18f0 100644 --- a/src/applications/phlux/controller/PhluxViewController.php +++ b/src/applications/phlux/controller/PhluxViewController.php @@ -79,6 +79,7 @@ final class PhluxViewController extends PhluxController { $xaction_view = id(new PhabricatorApplicationTransactionView()) ->setUser($user) + ->setObjectPHID($var->getPHID()) ->setTransactions($xactions) ->setMarkupEngine($engine); diff --git a/src/applications/pholio/controller/PholioMockViewController.php b/src/applications/pholio/controller/PholioMockViewController.php index f13a2d534d..409736854b 100644 --- a/src/applications/pholio/controller/PholioMockViewController.php +++ b/src/applications/pholio/controller/PholioMockViewController.php @@ -88,6 +88,7 @@ final class PholioMockViewController extends PholioController { $xaction_view = id(new PholioTransactionView()) ->setUser($this->getRequest()->getUser()) + ->setObjectPHID($mock->getPHID()) ->setTransactions($xactions) ->setMarkupEngine($engine); @@ -249,6 +250,7 @@ final class PholioMockViewController extends PholioController { $form = id(new PhabricatorApplicationTransactionCommentView()) ->setUser($user) + ->setObjectPHID($mock->getPHID()) ->setFormID($comment_form_id) ->setDraft($draft) ->setSubmitButtonName($button_name) diff --git a/src/applications/phortune/controller/PhortuneAccountViewController.php b/src/applications/phortune/controller/PhortuneAccountViewController.php index 2faeb83c7b..de2d6a50cd 100644 --- a/src/applications/phortune/controller/PhortuneAccountViewController.php +++ b/src/applications/phortune/controller/PhortuneAccountViewController.php @@ -168,6 +168,7 @@ final class PhortuneAccountViewController extends PhortuneController { $xaction_view = id(new PhabricatorApplicationTransactionView()) ->setUser($user) + ->setObjectPHID($account->getPHID()) ->setTransactions($xactions) ->setMarkupEngine($engine); diff --git a/src/applications/phortune/controller/PhortuneProductViewController.php b/src/applications/phortune/controller/PhortuneProductViewController.php index 9481c7147b..2e9be65229 100644 --- a/src/applications/phortune/controller/PhortuneProductViewController.php +++ b/src/applications/phortune/controller/PhortuneProductViewController.php @@ -77,6 +77,7 @@ final class PhortuneProductViewController extends PhortuneController { $xaction_view = id(new PhabricatorApplicationTransactionView()) ->setUser($user) + ->setObjectPHID($product->getPHID()) ->setTransactions($xactions) ->setMarkupEngine($engine); diff --git a/src/applications/ponder/controller/PonderQuestionViewController.php b/src/applications/ponder/controller/PonderQuestionViewController.php index 287b710ea8..c006fd7692 100644 --- a/src/applications/ponder/controller/PonderQuestionViewController.php +++ b/src/applications/ponder/controller/PonderQuestionViewController.php @@ -172,11 +172,13 @@ final class PonderQuestionViewController extends PonderController { $timeline = id(new PhabricatorApplicationTransactionView()) ->setUser($viewer) + ->setObjectPHID($question->getPHID()) ->setTransactions($xactions) ->setMarkupEngine($engine); $add_comment = id(new PhabricatorApplicationTransactionCommentView()) ->setUser($viewer) + ->setObjectPHID($question->getPHID()) ->setShowPreview(false) ->setAction($this->getApplicationURI("/question/comment/{$id}/")) ->setSubmitButtonName(pht('Comment')); @@ -230,11 +232,13 @@ final class PonderQuestionViewController extends PonderController { $out[] = id(new PhabricatorApplicationTransactionView()) ->setUser($viewer) + ->setObjectPHID($answer->getPHID()) ->setTransactions($xactions) ->setMarkupEngine($engine); $out[] = id(new PhabricatorApplicationTransactionCommentView()) ->setUser($viewer) + ->setObjectPHID($answer->getPHID()) ->setShowPreview(false) ->setAction($this->getApplicationURI("/answer/comment/{$id}/")) ->setSubmitButtonName(pht('Comment')); diff --git a/src/applications/releeph/controller/request/ReleephRequestViewController.php b/src/applications/releeph/controller/request/ReleephRequestViewController.php index 1c62203dda..e81a021ed4 100644 --- a/src/applications/releeph/controller/request/ReleephRequestViewController.php +++ b/src/applications/releeph/controller/request/ReleephRequestViewController.php @@ -50,6 +50,7 @@ final class ReleephRequestViewController extends ReleephProjectController { $timeline = id(new PhabricatorApplicationTransactionView()) ->setUser($request->getUser()) + ->setObjectPHID($releeph_request->getPHID()) ->setTransactions($xactions) ->setMarkupEngine($engine); @@ -62,6 +63,7 @@ final class ReleephRequestViewController extends ReleephProjectController { $add_comment_form = id(new PhabricatorApplicationTransactionCommentView()) ->setUser($user) + ->setObjectPHID($releeph_request->getPHID()) ->setDraft($draft) ->setAction($this->getApplicationURI( '/request/comment/'.$releeph_request->getID().'/')) diff --git a/src/applications/slowvote/controller/PhabricatorSlowvotePollController.php b/src/applications/slowvote/controller/PhabricatorSlowvotePollController.php index a61577c22b..03e60cf050 100644 --- a/src/applications/slowvote/controller/PhabricatorSlowvotePollController.php +++ b/src/applications/slowvote/controller/PhabricatorSlowvotePollController.php @@ -155,6 +155,7 @@ final class PhabricatorSlowvotePollController $timeline = id(new PhabricatorApplicationTransactionView()) ->setUser($viewer) + ->setObjectPHID($poll->getPHID()) ->setTransactions($xactions) ->setMarkupEngine($engine); @@ -180,6 +181,7 @@ final class PhabricatorSlowvotePollController $add_comment_form = id(new PhabricatorApplicationTransactionCommentView()) ->setUser($viewer) + ->setObjectPHID($poll->getPHID()) ->setDraft($draft) ->setAction($this->getApplicationURI('/comment/'.$poll->getID().'/')) ->setSubmitButtonName($submit_button_name); diff --git a/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentHistoryController.php b/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentHistoryController.php index 2819f743de..11b2c97157 100644 --- a/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentHistoryController.php +++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentHistoryController.php @@ -61,6 +61,7 @@ final class PhabricatorApplicationTransactionCommentHistoryController $view = id(new PhabricatorApplicationTransactionView()) ->setUser($user) + ->setObjectPHID($obj_phid) ->setTransactions($xactions) ->setShowEditActions(false); diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php index 167b0a31e6..767b75d779 100644 --- a/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php +++ b/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php @@ -17,6 +17,16 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView { private $draft; private $requestURI; private $showPreview = true; + private $objectPHID; + + public function setObjectPHID($object_phid) { + $this->objectPHID = $object_phid; + return $this; + } + + public function getObjectPHID() { + return $this->objectPHID; + } public function setShowPreview($show_preview) { $this->showPreview = $show_preview; @@ -130,11 +140,19 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView { $draft_comment = $this->getDraft()->getDraft(); } + if (!$this->getObjectPHID()) { + throw new Exception("Call setObjectPHID() before render()!"); + } + return id(new AphrontFormView()) ->setUser($this->getUser()) ->setFlexible(true) ->addSigil('transaction-append') ->setWorkflow(true) + ->setMetadata( + array( + 'objectPHID' => $this->getObjectPHID(), + )) ->setAction($this->getAction()) ->setID($this->getFormID()) ->appendChild( diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionView.php index 84dac0fdd0..bb151e4206 100644 --- a/src/applications/transactions/view/PhabricatorApplicationTransactionView.php +++ b/src/applications/transactions/view/PhabricatorApplicationTransactionView.php @@ -10,6 +10,16 @@ class PhabricatorApplicationTransactionView extends AphrontView { private $anchorOffset = 1; private $showEditActions = true; private $isPreview; + private $objectPHID; + + public function setObjectPHID($object_phid) { + $this->objectPHID = $object_phid; + return $this; + } + + public function getObjectPHID() { + return $this->objectPHID; + } public function setIsPreview($is_preview) { $this->isPreview = $is_preview; @@ -131,6 +141,10 @@ class PhabricatorApplicationTransactionView extends AphrontView { } public function render() { + if (!$this->getObjectPHID()) { + throw new Exception("Call setObjectPHID() before render()!"); + } + $view = new PhabricatorTimelineView(); $events = $this->buildEvents(); foreach ($events as $event) { @@ -146,6 +160,7 @@ class PhabricatorApplicationTransactionView extends AphrontView { 'phabricator-transaction-list', array( 'listID' => $list_id, + 'objectPHID' => $this->getObjectPHID(), 'nextAnchor' => $this->anchorOffset + count($events), )); } diff --git a/src/view/form/AphrontFormView.php b/src/view/form/AphrontFormView.php index 3f1555f52c..25c872671b 100644 --- a/src/view/form/AphrontFormView.php +++ b/src/view/form/AphrontFormView.php @@ -12,6 +12,17 @@ final class AphrontFormView extends AphrontView { private $flexible; private $noShading; private $sigils = array(); + private $metadata; + + + public function setMetadata($metadata) { + $this->metadata = $metadata; + return $this; + } + + public function getMetadata() { + return $this->metadata; + } public function setFlexible($flexible) { $this->flexible = $flexible; @@ -111,6 +122,7 @@ final class AphrontFormView extends AphrontView { 'method' => $this->method, 'enctype' => $this->encType, 'sigil' => $sigils ? implode(' ', $sigils) : null, + 'meta' => $this->metadata, 'id' => $this->id, ), $layout->render()); diff --git a/webroot/rsrc/js/application/transactions/behavior-transaction-list.js b/webroot/rsrc/js/application/transactions/behavior-transaction-list.js index c1b290cedb..41d14a61eb 100644 --- a/webroot/rsrc/js/application/transactions/behavior-transaction-list.js +++ b/webroot/rsrc/js/application/transactions/behavior-transaction-list.js @@ -79,8 +79,15 @@ JX.behavior('phabricator-transaction-list', function(config) { ['submit', 'didSyntheticSubmit'], 'transaction-append', function(e) { - e.kill(); var form = e.getTarget(); + if (JX.Stratcom.getData(form).objectPHID != config.objectPHID) { + // This indicates there are several forms on the page, and the user + // submitted a different one than the one we're in control of. + return; + } + + e.kill(); + JX.DOM.invoke(form, 'willSubmit'); JX.Workflow.newFromForm(form, { anchor : next_anchor })