Page MenuHomePhabricator
Files F14081125

D17533.diff
No OneTemporary
Actions

D17533.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
@@ -9,7 +9,7 @@
'names' => array(
'conpherence.pkg.css' => '82aca405',
'conpherence.pkg.js' => '6249a1cf',
- 'core.pkg.css' => 'c012c648',
+ 'core.pkg.css' => 'dc689e29',
'core.pkg.js' => '1fa7c0c5',
'darkconsole.pkg.js' => 'e7393ebb',
'differential.pkg.css' => '90b30783',
@@ -145,7 +145,7 @@
'rsrc/css/phui/phui-document.css' => 'c32e8dec',
'rsrc/css/phui/phui-feed-story.css' => '44a9c8e9',
'rsrc/css/phui/phui-fontkit.css' => '1320ed01',
- 'rsrc/css/phui/phui-form-view.css' => 'cf198e10',
+ 'rsrc/css/phui/phui-form-view.css' => '6175808d',
'rsrc/css/phui/phui-form.css' => 'b62c01d8',
'rsrc/css/phui/phui-head-thing.css' => 'fd311e5f',
'rsrc/css/phui/phui-header-view.css' => '9cf828ce',
@@ -530,7 +530,7 @@
'rsrc/js/phuix/PHUIXActionView.js' => 'b3465b9b',
'rsrc/js/phuix/PHUIXAutocomplete.js' => '7c492cd2',
'rsrc/js/phuix/PHUIXDropdownMenu.js' => '8018ee50',
- 'rsrc/js/phuix/PHUIXFormControl.js' => 'bbece68d',
+ 'rsrc/js/phuix/PHUIXFormControl.js' => '83e03671',
'rsrc/js/phuix/PHUIXIconView.js' => 'bff6884b',
),
'symbols' => array(
@@ -847,7 +847,7 @@
'phui-font-icon-base-css' => '870a7360',
'phui-fontkit-css' => '1320ed01',
'phui-form-css' => 'b62c01d8',
- 'phui-form-view-css' => 'cf198e10',
+ 'phui-form-view-css' => '6175808d',
'phui-head-thing-view-css' => 'fd311e5f',
'phui-header-view-css' => '9cf828ce',
'phui-hovercard' => '1bd28176',
@@ -887,7 +887,7 @@
'phuix-action-view' => 'b3465b9b',
'phuix-autocomplete' => '7c492cd2',
'phuix-dropdown-menu' => '8018ee50',
- 'phuix-form-control-view' => 'bbece68d',
+ 'phuix-form-control-view' => '83e03671',
'phuix-icon-view' => 'bff6884b',
'policy-css' => '957ea14c',
'policy-edit-css' => '815c66f7',
@@ -1518,6 +1518,10 @@
'javelin-behavior',
'javelin-scrollbar',
),
+ '83e03671' => array(
+ 'javelin-install',
+ 'javelin-dom',
+ ),
'8499b6ab' => array(
'javelin-behavior',
'javelin-dom',
@@ -1902,10 +1906,6 @@
'javelin-vector',
'javelin-install',
),
- 'bbece68d' => array(
- 'javelin-install',
- 'javelin-dom',
- ),
'bcaccd64' => array(
'javelin-behavior',
'javelin-behavior-device',
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -2602,6 +2602,7 @@
'PhabricatorEdgesDestructionEngineExtension' => 'infrastructure/edges/engineextension/PhabricatorEdgesDestructionEngineExtension.php',
'PhabricatorEditEngine' => 'applications/transactions/editengine/PhabricatorEditEngine.php',
'PhabricatorEditEngineAPIMethod' => 'applications/transactions/editengine/PhabricatorEditEngineAPIMethod.php',
+ 'PhabricatorEditEngineCheckboxesCommentAction' => 'applications/transactions/commentaction/PhabricatorEditEngineCheckboxesCommentAction.php',
'PhabricatorEditEngineColumnsCommentAction' => 'applications/transactions/commentaction/PhabricatorEditEngineColumnsCommentAction.php',
'PhabricatorEditEngineCommentAction' => 'applications/transactions/commentaction/PhabricatorEditEngineCommentAction.php',
'PhabricatorEditEngineCommentActionGroup' => 'applications/transactions/commentaction/PhabricatorEditEngineCommentActionGroup.php',
@@ -7686,6 +7687,7 @@
'PhabricatorPolicyInterface',
),
'PhabricatorEditEngineAPIMethod' => 'ConduitAPIMethod',
+ 'PhabricatorEditEngineCheckboxesCommentAction' => 'PhabricatorEditEngineCommentAction',
'PhabricatorEditEngineColumnsCommentAction' => 'PhabricatorEditEngineCommentAction',
'PhabricatorEditEngineCommentAction' => 'Phobject',
'PhabricatorEditEngineCommentActionGroup' => 'Phobject',
diff --git a/src/applications/differential/storage/DifferentialReviewer.php b/src/applications/differential/storage/DifferentialReviewer.php
--- a/src/applications/differential/storage/DifferentialReviewer.php
+++ b/src/applications/differential/storage/DifferentialReviewer.php
@@ -50,4 +50,35 @@
return ($this->getReviewerStatus() == $status_resigned);
}
+ public function isAccepted($diff_phid) {
+ $status_accepted = DifferentialReviewerStatus::STATUS_ACCEPTED;
+
+ if ($this->getReviewerStatus() != $status_accepted) {
+ return false;
+ }
+
+ if (!$diff_phid) {
+ return true;
+ }
+
+ $action_phid = $this->getLastActionDiffPHID();
+
+ if (!$action_phid) {
+ return true;
+ }
+
+ if ($action_phid == $diff_phid) {
+ return true;
+ }
+
+ $sticky_key = 'differential.sticky-accept';
+ $is_sticky = PhabricatorEnv::getEnvConfig($sticky_key);
+
+ if ($is_sticky) {
+ return true;
+ }
+
+ return false;
+ }
+
}
diff --git a/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php b/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php
--- a/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php
@@ -48,6 +48,72 @@
return pht('Accept a revision.');
}
+ protected function getActionOptions(
+ PhabricatorUser $viewer,
+ DifferentialRevision $revision) {
+
+ $reviewers = $revision->getReviewers();
+
+ $options = array();
+ $value = array();
+
+ // Put the viewer's user reviewer first, if it exists, so that "Accept as
+ // yourself" is always at the top.
+ $head = array();
+ $tail = array();
+ foreach ($reviewers as $key => $reviewer) {
+ if ($reviewer->isUser()) {
+ $head[$key] = $reviewer;
+ } else {
+ $tail[$key] = $reviewer;
+ }
+ }
+ $reviewers = $head + $tail;
+
+ $diff_phid = $this->getActiveDiffPHID($revision);
+ $reviewer_phids = array();
+
+ // If the viewer isn't a reviewer, add them to the list of options first.
+ // This happens when you navigate to some revision you aren't involved in:
+ // you can accept and become a reviewer.
+
+ $viewer_phid = $viewer->getPHID();
+ if ($viewer_phid) {
+ if (!isset($reviewers[$viewer_phid])) {
+ $reviewer_phids[$viewer_phid] = $viewer_phid;
+ }
+ }
+
+ foreach ($reviewers as $reviewer) {
+ if (!$reviewer->hasAuthority($viewer)) {
+ // If the viewer doesn't have authority to act on behalf of a reviewer,
+ // don't include that reviewer as an option.
+ continue;
+ }
+
+ if ($reviewer->isAccepted($diff_phid)) {
+ // If a reviewer is already in a full "accepted" state, don't
+ // include that reviewer as an option.
+ continue;
+ }
+
+ $reviewer_phid = $reviewer->getReviewerPHID();
+ $reviewer_phids[$reviewer_phid] = $reviewer_phid;
+ }
+
+ $handles = $viewer->loadHandles($reviewer_phids);
+
+ foreach ($reviewer_phids as $reviewer_phid) {
+ $options[$reviewer_phid] = pht(
+ 'Accept as %s',
+ $viewer->renderHandle($reviewer_phid));
+
+ $value[] = $reviewer_phid;
+ }
+
+ return array($options, $value);
+ }
+
public function generateOldValue($object) {
$actor = $this->getActor();
return $this->isViewerFullyAccepted($object, $actor);
@@ -87,10 +153,39 @@
}
}
+ protected function validateOptionValue($object, $actor, array $value) {
+ if (!$value) {
+ throw new Exception(
+ pht(
+ 'When accepting a revision, you must accept on behalf of at '.
+ 'least one reviewer.'));
+ }
+
+ list($options) = $this->getActionOptions($actor, $object);
+ foreach ($value as $phid) {
+ if (!isset($options[$phid])) {
+ throw new Exception(
+ pht(
+ 'Reviewer "%s" is not a valid reviewer which you have authority '.
+ 'to accept on behalf of.',
+ $phid));
+ }
+ }
+ }
+
public function getTitle() {
- return pht(
- '%s accepted this revision.',
- $this->renderAuthor());
+ $new = $this->getNewValue();
+ if (is_array($new) && $new) {
+ return pht(
+ '%s accepted this revision as %s reviewer(s): %s.',
+ $this->renderAuthor(),
+ phutil_count($new),
+ $this->renderHandleList($new));
+ } else {
+ return pht(
+ '%s accepted this revision.',
+ $this->renderAuthor());
+ }
}
public function getTitleForFeed() {
diff --git a/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php b/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php
--- a/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php
@@ -19,6 +19,10 @@
abstract protected function validateAction($object, PhabricatorUser $viewer);
abstract protected function getRevisionActionLabel();
+ protected function validateOptionValue($object, $actor, array $value) {
+ return null;
+ }
+
public function getCommandKeyword() {
return null;
}
@@ -70,6 +74,15 @@
return ($viewer->getPHID() === $revision->getAuthorPHID());
}
+ protected function getActionOptions(
+ PhabricatorUser $viewer,
+ DifferentialRevision $revision) {
+ return array(
+ array(),
+ null,
+ );
+ }
+
public function newEditField(
DifferentialRevision $revision,
PhabricatorUser $viewer) {
@@ -107,6 +120,12 @@
// It's not clear that these combinations are actually useful, so just
// keep things simple for now.
$field->setActionConflictKey('revision.action');
+
+ list($options, $value) = $this->getActionOptions($viewer, $revision);
+ if (count($options) > 1) {
+ $field->setOptions($options);
+ $field->setValue($value);
+ }
}
}
@@ -129,6 +148,20 @@
$errors[] = $this->newInvalidError(
$action_exception->getMessage(),
$xaction);
+ continue;
+ }
+
+ $new = $xaction->getNewValue();
+ if (!is_array($new)) {
+ continue;
+ }
+
+ try {
+ $this->validateOptionValue($object, $actor, $new);
+ } catch (Exception $ex) {
+ $errors[] = $this->newInvalidError(
+ $ex->getMessage(),
+ $xaction);
}
}
diff --git a/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php b/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php
--- a/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php
@@ -7,6 +7,26 @@
return DifferentialRevisionEditEngine::ACTIONGROUP_REVIEW;
}
+ public function generateNewValue($object, $value) {
+ if (!is_array($value)) {
+ return true;
+ }
+
+ // If the list of options is the same as the default list, just treat this
+ // as a "take the default action" transaction.
+ $viewer = $this->getActor();
+ list($options, $default) = $this->getActionOptions($viewer, $object);
+
+ sort($default);
+ sort($value);
+
+ if ($default === $value) {
+ return true;
+ }
+
+ return $value;
+ }
+
protected function isViewerAnyReviewer(
DifferentialRevision $revision,
PhabricatorUser $viewer) {
@@ -118,6 +138,12 @@
// In all cases, you affect yourself.
$map[$viewer->getPHID()] = $status;
+ // If the user has submitted a specific list of reviewers to act as (by
+ // unchecking some checkboxes under "Accept"), only affect those reviewers.
+ if (is_array($value)) {
+ $map = array_select_keys($map, $value);
+ }
+
// Convert reviewer statuses into edge data.
foreach ($map as $reviewer_phid => $reviewer_status) {
$map[$reviewer_phid] = array(
diff --git a/src/applications/transactions/commentaction/PhabricatorEditEngineCheckboxesCommentAction.php b/src/applications/transactions/commentaction/PhabricatorEditEngineCheckboxesCommentAction.php
new file mode 100644
--- /dev/null
+++ b/src/applications/transactions/commentaction/PhabricatorEditEngineCheckboxesCommentAction.php
@@ -0,0 +1,36 @@
+<?php
+
+final class PhabricatorEditEngineCheckboxesCommentAction
+ extends PhabricatorEditEngineCommentAction {
+
+ private $options = array();
+
+ public function setOptions(array $options) {
+ $this->options = $options;
+ return $this;
+ }
+
+ public function getOptions() {
+ return $this->options;
+ }
+
+ public function getPHUIXControlType() {
+ return 'checkboxes';
+ }
+
+ public function getPHUIXControlSpecification() {
+ $options = $this->getOptions();
+
+ $labels = array();
+ foreach ($options as $key => $option) {
+ $labels[$key] = hsprintf('%s', $option);
+ }
+
+ return array(
+ 'value' => $this->getValue(),
+ 'keys' => array_keys($options),
+ 'labels' => $labels,
+ );
+ }
+
+}
diff --git a/src/applications/transactions/editfield/PhabricatorApplyEditField.php b/src/applications/transactions/editfield/PhabricatorApplyEditField.php
--- a/src/applications/transactions/editfield/PhabricatorApplyEditField.php
+++ b/src/applications/transactions/editfield/PhabricatorApplyEditField.php
@@ -5,6 +5,7 @@
private $actionDescription;
private $actionConflictKey;
+ private $options;
protected function newControl() {
return null;
@@ -28,8 +29,21 @@
return $this->actionConflictKey;
}
+ public function setOptions(array $options) {
+ $this->options = $options;
+ return $this;
+ }
+
+ public function getOptions() {
+ return $this->options;
+ }
+
protected function newHTTPParameterType() {
- return new AphrontBoolHTTPParameterType();
+ if ($this->getOptions()) {
+ return new AphrontPHIDListHTTPParameterType();
+ } else {
+ return new AphrontBoolHTTPParameterType();
+ }
}
protected function newConduitParameterType() {
@@ -43,9 +57,16 @@
}
protected function newCommentAction() {
- return id(new PhabricatorEditEngineStaticCommentAction())
- ->setDescription($this->getActionDescription())
- ->setConflictKey($this->getActionConflictKey());
+ $options = $this->getOptions();
+ if ($options) {
+ return id(new PhabricatorEditEngineCheckboxesCommentAction())
+ ->setConflictKey($this->getActionConflictKey())
+ ->setOptions($options);
+ } else {
+ return id(new PhabricatorEditEngineStaticCommentAction())
+ ->setConflictKey($this->getActionConflictKey())
+ ->setDescription($this->getActionDescription());
+ }
}
}
diff --git a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php
--- a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php
+++ b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php
@@ -1608,6 +1608,8 @@
),
),
+ '%s accepted this revision as %s reviewer(s): %s.' =>
+ '%s accepted this revision as: %3$s.',
);
}
diff --git a/webroot/rsrc/css/phui/phui-form-view.css b/webroot/rsrc/css/phui/phui-form-view.css
--- a/webroot/rsrc/css/phui/phui-form-view.css
+++ b/webroot/rsrc/css/phui/phui-form-view.css
@@ -548,3 +548,16 @@
padding: 4px;
color: {$bluetext};
}
+
+.phuix-form-checkbox-action {
+ padding: 4px;
+ color: {$bluetext};
+}
+
+.phuix-form-checkbox-action input[type=checkbox] {
+ margin: 4px 0;
+}
+
+.phuix-form-checkbox-label {
+ margin-left: 4px;
+}
diff --git a/webroot/rsrc/js/phuix/PHUIXFormControl.js b/webroot/rsrc/js/phuix/PHUIXFormControl.js
--- a/webroot/rsrc/js/phuix/PHUIXFormControl.js
+++ b/webroot/rsrc/js/phuix/PHUIXFormControl.js
@@ -50,6 +50,9 @@
case 'static':
input = this._newStatic(spec);
break;
+ case 'checkboxes':
+ input = this._newCheckboxes(spec);
+ break;
default:
// TODO: Default or better error?
JX.$E('Bad Input Type');
@@ -194,6 +197,89 @@
};
},
+ _newCheckboxes: function(spec) {
+ var checkboxes = [];
+ var checkbox_list = [];
+ for (var ii = 0; ii < spec.keys.length; ii++) {
+ var key = spec.keys[ii];
+ var checkbox_id = 'checkbox-' + Math.floor(Math.random() * 1000000);
+
+ var checkbox = JX.$N(
+ 'input',
+ {
+ type: 'checkbox',
+ value: key,
+ id: checkbox_id
+ });
+
+ checkboxes.push(checkbox);
+
+ var label = JX.$N(
+ 'label',
+ {
+ className: 'phuix-form-checkbox-label',
+ htmlFor: checkbox_id
+ },
+ JX.$H(spec.labels[key] || ''));
+
+ var display = JX.$N(
+ 'div',
+ {
+ className: 'phuix-form-checkbox-item'
+ },
+ [checkbox, label]);
+
+ checkbox_list.push(display);
+ }
+
+ var node = JX.$N(
+ 'div',
+ {
+ className: 'phuix-form-checkbox-action'
+ },
+ checkbox_list);
+
+ var get_value = function() {
+ var list = [];
+ for (var ii = 0; ii < checkboxes.length; ii++) {
+ if (checkboxes[ii].checked) {
+ list.push(checkboxes[ii].value);
+ }
+ }
+ return list;
+ };
+
+ var set_value = function(value) {
+ value = value || [];
+
+ if (!value.length) {
+ value = [];
+ }
+
+ var map = {};
+ var ii;
+ for (ii = 0; ii < value.length; ii++) {
+ map[value[ii]] = true;
+ }
+
+ for (ii = 0; ii < checkboxes.length; ii++) {
+ if (map.hasOwnProperty(checkboxes[ii].value)) {
+ checkboxes[ii].checked = 'checked';
+ } else {
+ checkboxes[ii].checked = false;
+ }
+ }
+ };
+
+ set_value(spec.value);
+
+ return {
+ node: node,
+ get: get_value,
+ set: set_value
+ };
+ },
+
_newPoints: function(spec) {
var attrs = {
type: 'text',

File Metadata

Mime Type
text/plain
Expires
Sat, Nov 23, 6:30 PM (18 h, 13 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6779207
Default Alt Text
D17533.diff (18 KB)

Event Timeline

Phabricator · Built by Phacility