Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15443176
D8376.id19907.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
10 KB
Referenced Files
None
Subscribers
None
D8376.id19907.diff
View Options
diff --git a/resources/sql/autopatches/20140228.dxcomment.1.sql b/resources/sql/autopatches/20140228.dxcomment.1.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20140228.dxcomment.1.sql
@@ -0,0 +1,4 @@
+/* Make this column nullable. */
+
+ALTER TABLE {$NAMESPACE}_differential.differential_transaction_comment
+ CHANGE revisionPHID revisionPHID VARCHAR(64) COLLATE utf8_bin;
diff --git a/src/applications/differential/controller/DifferentialDiffViewController.php b/src/applications/differential/controller/DifferentialDiffViewController.php
--- a/src/applications/differential/controller/DifferentialDiffViewController.php
+++ b/src/applications/differential/controller/DifferentialDiffViewController.php
@@ -58,7 +58,8 @@
array(
'value' => $revision->getID(),
),
- 'D'.$revision->getID().' '.$revision->getTitle());
+ phutil_utf8_shorten(
+ 'D'.$revision->getID().' '.$revision->getTitle(), 128));
}
$select[] = hsprintf('</optgroup>');
}
diff --git a/src/applications/differential/controller/DifferentialRevisionEditControllerPro.php b/src/applications/differential/controller/DifferentialRevisionEditControllerPro.php
--- a/src/applications/differential/controller/DifferentialRevisionEditControllerPro.php
+++ b/src/applications/differential/controller/DifferentialRevisionEditControllerPro.php
@@ -23,6 +23,7 @@
->withIDs(array($this->id))
->needRelationships(true)
->needReviewerStatus(true)
+ ->needActiveDiffs(true)
->requireCapabilities(
array(
PhabricatorPolicyCapability::CAN_VIEW,
@@ -34,6 +35,7 @@
}
} else {
$revision = DifferentialRevision::initializeNewRevision($viewer);
+ $revision->attachReviewerStatus(array());
}
$diff_id = $request->getInt('diffID');
@@ -53,6 +55,15 @@
$diff = null;
}
+ if (!$diff) {
+ if (!$revision->getID()) {
+ throw new Exception(
+ pht('You can not create a new revision without a diff!'));
+ }
+ } else {
+ // TODO: It would be nice to show the diff being attached in the UI.
+ }
+
$field_list = PhabricatorCustomField::getObjectFields(
$revision,
PhabricatorCustomField::ROLE_EDIT);
@@ -66,6 +77,21 @@
new DifferentialTransaction(),
$request);
+ if ($diff) {
+ $xactions[] = id(new DifferentialTransaction())
+ ->setTransactionType(DifferentialTransaction::TYPE_UPDATE)
+ ->setNewValue($diff->getPHID());
+ }
+
+ $comments = $request->getStr('comments');
+ if (strlen($comments)) {
+ $xactions[] = id(new DifferentialTransaction())
+ ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
+ ->attachComment(
+ id(new DifferentialTransactionComment())
+ ->setContent($comments));
+ }
+
$editor = id(new DifferentialTransactionEditor())
->setActor($viewer)
->setContentSourceFromRequest($request)
diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php
--- a/src/applications/differential/controller/DifferentialRevisionViewController.php
+++ b/src/applications/differential/controller/DifferentialRevisionViewController.php
@@ -881,11 +881,6 @@
->setRightDiff($right_diff)
->setTransactions($xactions);
- // TODO: Make this work and restore edit links. We need to copy
- // `revisionPHID` to the new version of the comment. This should be simple,
- // but can happen post-merge. See T2222.
- $timeline->setShowEditActions(false);
-
return $timeline;
}
diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -14,11 +14,7 @@
$types[] = DifferentialTransaction::TYPE_ACTION;
$types[] = DifferentialTransaction::TYPE_INLINE;
$types[] = DifferentialTransaction::TYPE_STATUS;
-
-/*
-
$types[] = DifferentialTransaction::TYPE_UPDATE;
-*/
return $types;
}
@@ -36,6 +32,12 @@
return null;
case DifferentialTransaction::TYPE_INLINE:
return null;
+ case DifferentialTransaction::TYPE_UPDATE:
+ if ($this->getIsNewObject()) {
+ return null;
+ } else {
+ return $object->getActiveDiff()->getPHID();
+ }
}
return parent::getCustomTransactionOldValue($object, $xaction);
@@ -49,6 +51,7 @@
case PhabricatorTransactions::TYPE_VIEW_POLICY:
case PhabricatorTransactions::TYPE_EDIT_POLICY:
case DifferentialTransaction::TYPE_ACTION:
+ case DifferentialTransaction::TYPE_UPDATE:
return $xaction->getNewValue();
case DifferentialTransaction::TYPE_INLINE:
return null;
@@ -147,6 +150,9 @@
return;
case PhabricatorTransactions::TYPE_EDGE:
return;
+ case DifferentialTransaction::TYPE_UPDATE:
+ // TODO: Update the `diffPHID` once we add that.
+ return;
case DifferentialTransaction::TYPE_ACTION:
$status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW;
$status_revision = ArcanistDifferentialRevisionStatus::NEEDS_REVISION;
@@ -336,6 +342,29 @@
case DifferentialTransaction::TYPE_ACTION:
case DifferentialTransaction::TYPE_INLINE:
return;
+ case DifferentialTransaction::TYPE_UPDATE:
+ // Now that we're inside the transaction, do a final check.
+ $diff = $this->loadDiff($xaction->getNewValue());
+
+ // TODO: It would be slightly cleaner to just revalidate this
+ // transaction somehow using the same validation code, but that's
+ // not easy to do at the moment.
+
+ if (!$diff) {
+ throw new Exception(pht('Diff does not exist!'));
+ } else {
+ $revision_id = $diff->getRevisionID();
+ if ($revision_id && ($revision_id != $object->getID())) {
+ throw new Exception(
+ pht(
+ 'Diff is already attached to another revision. You lost '.
+ 'a race?'));
+ }
+ }
+
+ $diff->setRevisionID($object->getID());
+ $diff->save();
+ return;
}
return parent::applyCustomExternalTransaction($object, $xaction);
@@ -471,6 +500,25 @@
foreach ($xactions as $xaction) {
switch ($type) {
+ case DifferentialTransaction::TYPE_UPDATE:
+ $diff = $this->loadDiff($xaction->getNewValue());
+ if (!$diff) {
+ $errors[] = new PhabricatorApplicationTransactionValidationError(
+ $type,
+ pht('Invalid'),
+ pht('The specified diff does not exist.'),
+ $xaction);
+ } else if (($diff->getRevisionID()) &&
+ ($diff->getRevisionID() != $object->getID())) {
+ $errors[] = new PhabricatorApplicationTransactionValidationError(
+ $type,
+ pht('Invalid'),
+ pht(
+ 'You can not update this revision to the specified diff, '.
+ 'because the diff is already attached to another revision.'),
+ $xaction);
+ }
+ break;
case DifferentialTransaction::TYPE_ACTION:
$error = $this->validateDifferentialAction(
$object,
@@ -893,6 +941,12 @@
return implode("\n", $result);
}
+ private function loadDiff($phid) {
+ return id(new DifferentialDiffQuery())
+ ->withPHIDs(array($phid))
+ ->setViewer($this->getActor())
+ ->executeOne();
+ }
}
diff --git a/src/applications/differential/phid/DifferentialPHIDTypeDiff.php b/src/applications/differential/phid/DifferentialPHIDTypeDiff.php
--- a/src/applications/differential/phid/DifferentialPHIDTypeDiff.php
+++ b/src/applications/differential/phid/DifferentialPHIDTypeDiff.php
@@ -35,6 +35,7 @@
$id = $diff->getID();
$handle->setName(pht('Diff %d', $id));
+ $handle->setURI("/differential/diff/{$id}/");
}
}
diff --git a/src/applications/differential/storage/DifferentialTransaction.php b/src/applications/differential/storage/DifferentialTransaction.php
--- a/src/applications/differential/storage/DifferentialTransaction.php
+++ b/src/applications/differential/storage/DifferentialTransaction.php
@@ -70,6 +70,22 @@
return parent::getBodyForMail();
}
+ public function getRequiredHandlePHIDs() {
+ $phids = parent::getRequiredHandlePHIDs();
+
+ $old = $this->getOldValue();
+ $new = $this->getNewValue();
+
+ switch ($this->getTransactionType()) {
+ case self::TYPE_UPDATE:
+ if ($new) {
+ $phids[] = $new;
+ }
+ break;
+ }
+
+ return $phids;
+ }
public function getTitle() {
$author_phid = $this->getAuthorPHID();
@@ -87,10 +103,16 @@
if ($new) {
// TODO: Migrate to PHIDs and use handles here?
// TODO: Link this?
- return pht(
- '%s updated this revision to Diff #%d.',
- $author_handle,
- $new);
+ if (phid_get_type($new) == 'DIFF') {
+ return pht(
+ '%s updated this revision to %s.',
+ $author_handle,
+ $this->renderHandleLink($new));
+ } else {
+ return pht(
+ '%s updated this revision.',
+ $author_handle);
+ }
} else {
return pht(
'%s updated this revision.',
diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
--- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
+++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
@@ -30,6 +30,7 @@
private $transactionGroup = array();
private $viewer = self::ATTACHABLE;
private $object = self::ATTACHABLE;
+ private $editorData = array();
private $ignoreOnNoEffect;
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Fri, Mar 28, 2:51 AM (1 w, 4 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7725062
Default Alt Text
D8376.id19907.diff (10 KB)
Attached To
Mode
D8376: Make "EditPro" controller work with diff updates
Attached
Detach File
Event Timeline
Log In to Comment