Page MenuHomePhabricator
Files F15437254

D8376.diff
No OneTemporary
Actions

D8376.diff
View Options

Index: resources/sql/autopatches/20140228.dxcomment.1.sql
===================================================================
--- /dev/null
+++ 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;
Index: src/applications/differential/controller/DifferentialDiffViewController.php
===================================================================
--- src/applications/differential/controller/DifferentialDiffViewController.php
+++ 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>');
}
Index: src/applications/differential/controller/DifferentialRevisionEditControllerPro.php
===================================================================
--- src/applications/differential/controller/DifferentialRevisionEditControllerPro.php
+++ 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)
Index: src/applications/differential/controller/DifferentialRevisionViewController.php
===================================================================
--- src/applications/differential/controller/DifferentialRevisionViewController.php
+++ 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;
}
Index: src/applications/differential/editor/DifferentialTransactionEditor.php
===================================================================
--- src/applications/differential/editor/DifferentialTransactionEditor.php
+++ 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,
@@ -966,6 +1014,12 @@
return implode("\n", $result);
}
+ private function loadDiff($phid) {
+ return id(new DifferentialDiffQuery())
+ ->withPHIDs(array($phid))
+ ->setViewer($this->getActor())
+ ->executeOne();
+ }
}
Index: src/applications/differential/phid/DifferentialPHIDTypeDiff.php
===================================================================
--- src/applications/differential/phid/DifferentialPHIDTypeDiff.php
+++ src/applications/differential/phid/DifferentialPHIDTypeDiff.php
@@ -35,6 +35,7 @@
$id = $diff->getID();
$handle->setName(pht('Diff %d', $id));
+ $handle->setURI("/differential/diff/{$id}/");
}
}
Index: src/applications/differential/storage/DifferentialTransaction.php
===================================================================
--- src/applications/differential/storage/DifferentialTransaction.php
+++ 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.',

File Metadata

Mime Type
text/plain
Expires
Wed, Mar 26, 6:34 PM (1 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7688900
Default Alt Text
D8376.diff (9 KB)

Event Timeline

Phabricator · Built by Phacility