Page MenuHomePhabricator
Files F13993920

D8306.id.diff
No OneTemporary
Actions

D8306.id.diff
View Options

Index: src/applications/differential/controller/DifferentialCommentSaveControllerPro.php
===================================================================
--- src/applications/differential/controller/DifferentialCommentSaveControllerPro.php
+++ src/applications/differential/controller/DifferentialCommentSaveControllerPro.php
@@ -86,8 +86,30 @@
->setNewValue(array('+' => $reviewer_edges));
}
+ $inline_phids = $this->loadUnsubmittedInlinePHIDs($revision);
+ if ($inline_phids) {
+ $inlines = id(new PhabricatorApplicationTransactionCommentQuery())
+ ->setTemplate(new DifferentialTransactionComment())
+ ->setViewer($viewer)
+ ->withPHIDs($inline_phids)
+ ->execute();
+ } else {
+ $inlines = null;
+ }
+
+ foreach ($inlines as $inline) {
+ $xactions[] = id(new DifferentialTransaction())
+ ->setTransactionType($type_inline)
+ ->attachComment($inline);
+ }
+
+ // NOTE: If there are no other transactions, add an empty comment
+ // transaction so that we'll raise a more user-friendly error message,
+ // to the effect of "you can not post an empty comment".
+ $no_xactions = !$xactions;
+
$comment = $request->getStr('comment');
- if (strlen($comment)) {
+ if (strlen($comment) || $no_xactions) {
$xactions[] = id(new DifferentialTransaction())
->setTransactionType($type_comment)
->attachComment(
@@ -96,7 +118,6 @@
->setContent($comment));
}
- // TODO: Inlines!
$editor = id(new DifferentialTransactionEditor())
->setActor($viewer)
@@ -114,8 +135,6 @@
->setException($ex);
}
- // TODO: Diff change detection?
-
$user = $request->getUser();
$draft = id(new PhabricatorDraft())->loadOneWhere(
'authorPHID = %s AND draftKey = %s',
@@ -130,4 +149,30 @@
->setURI('/D'.$revision->getID());
}
+
+ private function loadUnsubmittedInlinePHIDs(DifferentialRevision $revision) {
+ $viewer = $this->getRequest()->getUser();
+
+ // TODO: This probably needs to move somewhere more central as we move
+ // away from DifferentialInlineCommentQuery, but
+ // PhabricatorApplicationTransactionCommentQuery is currently `final` and
+ // I'm not yet decided on how to approach that. For now, just get the PHIDs
+ // and then execute a PHID-based query through the standard stack.
+
+ $table = new DifferentialTransactionComment();
+ $conn_r = $table->establishConnection('r');
+
+ $phids = queryfx_all(
+ $conn_r,
+ 'SELECT phid FROM %T
+ WHERE revisionPHID = %s
+ AND authorPHID = %s
+ AND transactionPHID IS NULL',
+ $table->getTableName(),
+ $revision->getPHID(),
+ $viewer->getPHID());
+
+ return ipull($phids, 'phid');
+ }
+
}
Index: src/applications/differential/editor/DifferentialTransactionEditor.php
===================================================================
--- src/applications/differential/editor/DifferentialTransactionEditor.php
+++ src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -12,10 +12,10 @@
$types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
$types[] = DifferentialTransaction::TYPE_ACTION;
+ $types[] = DifferentialTransaction::TYPE_INLINE;
/*
- $types[] = DifferentialTransaction::TYPE_INLINE;
$types[] = DifferentialTransaction::TYPE_UPDATE;
*/
@@ -33,6 +33,8 @@
return $object->getEditPolicy();
case DifferentialTransaction::TYPE_ACTION:
return null;
+ case DifferentialTransaction::TYPE_INLINE:
+ return null;
}
return parent::getCustomTransactionOldValue($object, $xaction);
@@ -47,6 +49,8 @@
case PhabricatorTransactions::TYPE_EDIT_POLICY:
case DifferentialTransaction::TYPE_ACTION:
return $xaction->getNewValue();
+ case DifferentialTransaction::TYPE_INLINE:
+ return null;
}
return parent::getCustomTransactionNewValue($object, $xaction);
@@ -57,6 +61,8 @@
PhabricatorApplicationTransaction $xaction) {
switch ($xaction->getTransactionType()) {
+ case DifferentialTransaction::TYPE_INLINE:
+ return $xaction->hasComment();
}
return parent::transactionHasEffect($object, $xaction);
@@ -76,6 +82,8 @@
return;
case PhabricatorTransactions::TYPE_SUBSCRIBERS:
case PhabricatorTransactions::TYPE_COMMENT:
+ case DifferentialTransaction::TYPE_INLINE:
+ return;
case PhabricatorTransactions::TYPE_EDGE:
// TODO: When removing reviewers, we may be able to move the revision
// to "Accepted".
@@ -101,6 +109,7 @@
case PhabricatorTransactions::TYPE_EDGE:
case PhabricatorTransactions::TYPE_COMMENT:
case DifferentialTransaction::TYPE_ACTION:
+ case DifferentialTransaction::TYPE_INLINE:
return;
}
@@ -120,6 +129,23 @@
return $errors;
}
+ protected function sortTransactions(array $xactions) {
+ $head = array();
+ $tail = array();
+
+ // Move bare comments to the end, so the actions precede them.
+ foreach ($xactions as $xaction) {
+ $type = $xaction->getTransactionType();
+ if ($type == DifferentialTransaction::TYPE_INLINE) {
+ $tail[] = $xaction;
+ } else {
+ $head[] = $xaction;
+ }
+ }
+
+ return array_values(array_merge($head, $tail));
+ }
+
protected function requireCapabilities(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
Index: src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
===================================================================
--- src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -1335,12 +1335,7 @@
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
- switch ($xaction->getTransactionType()) {
- case PhabricatorTransactions::TYPE_COMMENT:
- return true;
- default:
- return false;
- }
+ return $xaction->isCommentTransaction();
}
Index: src/applications/transactions/storage/PhabricatorApplicationTransaction.php
===================================================================
--- src/applications/transactions/storage/PhabricatorApplicationTransaction.php
+++ src/applications/transactions/storage/PhabricatorApplicationTransaction.php
@@ -528,6 +528,19 @@
return 1.0;
}
+ public function isCommentTransaction() {
+ if ($this->hasComment()) {
+ return true;
+ }
+
+ switch ($this->getTransactionType()) {
+ case PhabricatorTransactions::TYPE_COMMENT:
+ return true;
+ }
+
+ return false;
+ }
+
public function getActionName() {
switch ($this->getTransactionType()) {
case PhabricatorTransactions::TYPE_COMMENT:
@@ -605,8 +618,6 @@
* @return bool True to display in a group with the other transactions.
*/
public function shouldDisplayGroupWith(array $group) {
- $type_comment = PhabricatorTransactions::TYPE_COMMENT;
-
$this_source = null;
if ($this->getContentSource()) {
$this_source = $this->getContentSource()->getSource();
@@ -624,7 +635,7 @@
}
// Don't group anything into a group which already has a comment.
- if ($xaction->getTransactionType() == $type_comment) {
+ if ($xaction->isCommentTransaction()) {
return false;
}

File Metadata

Mime Type
text/plain
Expires
Thu, Oct 24, 2:18 AM (3 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6717763
Default Alt Text
D8306.id.diff (7 KB)

Event Timeline

Phabricator · Built by Phacility