Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15439894
D8328.id19807.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
12 KB
Referenced Files
None
Subscribers
None
D8328.id19807.diff
View Options
diff --git a/resources/celerity/map.php b/resources/celerity/map.php
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -7,7 +7,7 @@
return array(
'names' =>
array(
- 'core.pkg.css' => '8c8b76a8',
+ 'core.pkg.css' => '76aa3fcd',
'core.pkg.js' => '8f7aa2c3',
'darkconsole.pkg.js' => 'ca8671ce',
'differential.pkg.css' => '6aef439e',
@@ -21,7 +21,7 @@
'rsrc/css/aphront/aphront-notes.css' => '6acadd3f',
'rsrc/css/aphront/context-bar.css' => '1c3b0529',
'rsrc/css/aphront/dark-console.css' => '6378ef3d',
- 'rsrc/css/aphront/dialog-view.css' => 'dd9db96c',
+ 'rsrc/css/aphront/dialog-view.css' => 'c01d24b4',
'rsrc/css/aphront/error-view.css' => '16cd9949',
'rsrc/css/aphront/lightbox-attachment.css' => '686f8885',
'rsrc/css/aphront/list-filter-view.css' => 'ef989c67',
@@ -483,7 +483,7 @@
'aphront-bars' => '231ac33c',
'aphront-contextbar-view-css' => '1c3b0529',
'aphront-dark-console-css' => '6378ef3d',
- 'aphront-dialog-view-css' => 'dd9db96c',
+ 'aphront-dialog-view-css' => 'c01d24b4',
'aphront-error-view-css' => '16cd9949',
'aphront-list-filter-view-css' => 'ef989c67',
'aphront-multi-column-view-css' => '12f65921',
diff --git a/src/applications/differential/controller/DifferentialCommentSaveControllerPro.php b/src/applications/differential/controller/DifferentialCommentSaveControllerPro.php
--- a/src/applications/differential/controller/DifferentialCommentSaveControllerPro.php
+++ b/src/applications/differential/controller/DifferentialCommentSaveControllerPro.php
@@ -94,7 +94,7 @@
->withPHIDs($inline_phids)
->execute();
} else {
- $inlines = null;
+ $inlines = array();
}
foreach ($inlines as $inline) {
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
@@ -82,6 +82,14 @@
return ($object->getStatus() != $status_revision);
case DifferentialAction::ACTION_REQUEST:
return ($object->getStatus() != $status_review);
+ case DifferentialAction::ACTION_RESIGN:
+ $actor_phid = $this->getActor()->getPHID();
+ foreach ($object->getReviewerStatus() as $reviewer) {
+ if ($reviewer->getReviewerPHID() == $actor_phid) {
+ return true;
+ }
+ }
+ return false;
}
}
@@ -113,6 +121,9 @@
$status_revision = ArcanistDifferentialRevisionStatus::NEEDS_REVISION;
switch ($xaction->getNewValue()) {
+ case DifferentialAction::ACTION_RESIGN:
+ // TODO: Update review status?
+ break;
case DifferentialAction::ACTION_ABANDON:
$object->setStatus(ArcanistDifferentialRevisionStatus::ABANDONED);
break;
@@ -150,6 +161,41 @@
return parent::applyCustomInternalTransaction($object, $xaction);
}
+ protected function expandTransaction(
+ PhabricatorLiskDAO $object,
+ PhabricatorApplicationTransaction $xaction) {
+
+ $results = parent::expandTransaction($object, $xaction);
+ switch ($xaction->getTransactionType()) {
+ case DifferentialTransaction::TYPE_ACTION:
+ switch ($xaction->getNewValue()) {
+ case DifferentialAction::ACTION_RESIGN:
+ // If the user is resigning, add a separate reviewer edit
+ // transaction which removes them as a reviewer.
+
+ $actor_phid = $this->getActor()->getPHID();
+ $type_edge = PhabricatorTransactions::TYPE_EDGE;
+ $edge_reviewer = PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER;
+
+ $results[] = id(new DifferentialTransaction())
+ ->setTransactionType($type_edge)
+ ->setMetadataValue('edge:type', $edge_reviewer)
+ ->setIgnoreOnNoEffect(true)
+ ->setNewValue(
+ array(
+ '-' => array(
+ $actor_phid => $actor_phid,
+ ),
+ ));
+
+ break;
+ }
+ break;
+ }
+
+ return $results;
+ }
+
protected function applyCustomExternalTransaction(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
@@ -221,6 +267,11 @@
$status_closed = ArcanistDifferentialRevisionStatus::CLOSED;
switch ($action) {
+ case DifferentialAction::ACTION_RESIGN:
+ // You can always resign from a revision if you're a reviewer. If you
+ // aren't, this is a no-op rather than invalid.
+ break;
+
case DifferentialAction::ACTION_ABANDON:
if (!$actor_is_author) {
return pht(
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
@@ -121,7 +121,8 @@
switch ($this->getMetadataValue('edge:type')) {
case PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER:
return pht(
- 'Those reviewers are already reviewing this revision.');
+ 'The reviewers you are trying to add are already reviewing '.
+ 'this revision.');
}
break;
case DifferentialTransaction::TYPE_ACTION:
@@ -142,6 +143,10 @@
return pht('This revision already requires changes.');
case DifferentialAction::ACTION_REQUEST:
return pht('Review is already requested for this revision.');
+ case DifferentialAction::ACTION_RESIGN:
+ return pht(
+ 'You can not resign from this revision because you are not '.
+ 'a reviewer.');
}
break;
}
diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -417,6 +417,7 @@
$xactions[] = $mention_xaction;
}
+ $xactions = $this->expandTransactions($object, $xactions);
$xactions = $this->combineTransactions($xactions);
foreach ($xactions as $xaction) {
@@ -853,6 +854,30 @@
return null;
}
+ /**
+ * Optionally expand transactions which imply other effects. For example,
+ * resigning from a revision in Differential implies removing yourself as
+ * a reviewer.
+ */
+ private function expandTransactions(
+ PhabricatorLiskDAO $object,
+ array $xactions) {
+
+ $results = array();
+ foreach ($xactions as $xaction) {
+ foreach ($this->expandTransaction($object, $xaction) as $expanded) {
+ $results[] = $expanded;
+ }
+ }
+
+ return $results;
+ }
+
+ protected function expandTransaction(
+ PhabricatorLiskDAO $object,
+ PhabricatorApplicationTransaction $xaction) {
+ return array($xaction);
+ }
/**
* Attempt to combine similar transactions into a smaller number of total
@@ -912,6 +937,12 @@
}
$u->setNewValue($result);
+ // When combining an "ignore" transaction with a normal transaction, make
+ // sure we don't propagate the "ignore" flag.
+ if (!$v->getIgnoreOnNoEffect()) {
+ $u->setIgnoreOnNoEffect(false);
+ }
+
return $u;
}
@@ -1128,6 +1159,8 @@
if ($xaction->getTransactionType() != $type_comment) {
$any_effect = true;
}
+ } else if ($xaction->getIgnoreOnNoEffect()) {
+ unset($xactions[$key]);
} else {
$no_effect[$key] = $xaction;
}
diff --git a/src/applications/transactions/response/PhabricatorApplicationTransactionNoEffectResponse.php b/src/applications/transactions/response/PhabricatorApplicationTransactionNoEffectResponse.php
--- a/src/applications/transactions/response/PhabricatorApplicationTransactionNoEffectResponse.php
+++ b/src/applications/transactions/response/PhabricatorApplicationTransactionNoEffectResponse.php
@@ -32,22 +32,28 @@
$only_empty_comment = (count($xactions) == 1) &&
(head($xactions)->getTransactionType() == $type_comment);
+ $count = new PhutilNumber(count($xactions));
+
if ($ex->hasAnyEffect()) {
- $title = pht('%d Action(s) With No Effect', count($xactions));
+ $title = pht('%d Action(s) With No Effect', $count);
+ $head = pht('Some of your %d action(s) have no effect:', $count);
$tail = pht('Apply remaining actions?');
- $continue = pht('Apply Other Actions');
+ $continue = pht('Apply Remaining Actions');
} else if ($ex->hasComment()) {
$title = pht('Post as Comment');
+ $head = pht('The %d action(s) you are taking have no effect:', $count);
$tail = pht('Do you want to post your comment anyway?');
$continue = pht('Post Comment');
} else if ($only_empty_comment) {
// Special case this since it's common and we can give the user a nicer
// dialog than "Action Has No Effect".
$title = pht('Empty Comment');
+ $head = null;
$tail = null;
$continue = null;
} else {
- $title = pht('%d Action(s) Have No Effect', count($xactions));
+ $title = pht('%d Action(s) Have No Effect', $count);
+ $head = pht('The %d action(s) you are taking have no effect:', $count);
$tail = null;
$continue = null;
}
@@ -56,10 +62,17 @@
->setUser($request->getUser())
->setTitle($title);
+ $dialog->appendChild($head);
+
+ $list = array();
foreach ($xactions as $xaction) {
- $dialog->appendChild(
- phutil_tag('p', array(), $xaction->getNoEffectDescription()));
+ $list[] = phutil_tag(
+ 'li',
+ array(),
+ $xaction->getNoEffectDescription());
}
+
+ $dialog->appendChild(phutil_tag('ul', array(), $list));
$dialog->appendChild($tail);
if ($continue) {
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
@@ -31,6 +31,24 @@
private $viewer = self::ATTACHABLE;
private $object = self::ATTACHABLE;
+ private $ignoreOnNoEffect;
+
+
+ /**
+ * Flag this transaction as a pure side-effect which should be ignored when
+ * applying transactions if it has no effect, even if transaction application
+ * would normally fail. This both provides users with better error messages
+ * and allows transactions to perform optional side effects.
+ */
+ public function setIgnoreOnNoEffect($ignore) {
+ $this->ignoreOnNoEffect = $ignore;
+ return $this;
+ }
+
+ public function getIgnoreOnNoEffect() {
+ return $this->ignoreOnNoEffect;
+ }
+
abstract public function getApplicationTransactionType();
private function getApplicationObjectTypeName() {
diff --git a/src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php
--- a/src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php
+++ b/src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php
@@ -215,6 +215,26 @@
'Actions With No Effect',
),
+ 'Some of your %d action(s) have no effect:' => array(
+ 'One of your actions has no effect:',
+ 'Some of your actions have no effect:',
+ ),
+
+ 'Apply remaining %d action(s)?' => array(
+ 'Apply remaining action?',
+ 'Apply remaining actions?',
+ ),
+
+ 'Apply %d Other Action(s)' => array(
+ 'Apply Remaining Action',
+ 'Apply Remaining Actions',
+ ),
+
+ 'The %d action(s) you are taking have no effect:' => array(
+ 'The action you are taking has no effect:',
+ 'The actions you are taking have no effect:',
+ ),
+
'%s edited post(s), added %d: %s; removed %d: %s.' =>
'%s edited posts, added: %3$s; removed: %5$s',
diff --git a/webroot/rsrc/css/aphront/dialog-view.css b/webroot/rsrc/css/aphront/dialog-view.css
--- a/webroot/rsrc/css/aphront/dialog-view.css
+++ b/webroot/rsrc/css/aphront/dialog-view.css
@@ -112,7 +112,7 @@
width: 50%;
}
-.aphront-access-dialog ul {
+.aphront-dialog-view ul {
margin: 12px 24px;
list-style: circle;
}
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Thu, Mar 27, 10:06 AM (2 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7724442
Default Alt Text
D8328.id19807.diff (12 KB)
Attached To
Mode
D8328: Implement "Resign" action against ApplicationTransactions
Attached
Detach File
Event Timeline
Log In to Comment