mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-22 21:40:55 +01:00
When accepting revisions, allow users to accept on behalf of a subset of reviewers
Summary: Ref T12271. Currenty, when you "Accept" a revision, you always accept it for all reviewers you have authority over. There are some situations where communication can be more clear if users can accept as only themselves, or for only some packages, etc. T12271 discusses some of these use cases in more depth. Instead of making "Accept" a blanket action, default it to doing what it does now but let the user uncheck reviewers. In cases where project/package reviewers aren't in use, this doesn't change anything. For now, "reject" still acts the old way (reject everything). We could make that use checkboxes too, but I'm not sure there's as much of a use case for it, and I generally want users who are blocking stuff to have more direct accountability in a product sense. Test Plan: - Accepted normally. - Accepted a subset. - Tried to accept none. - Tried to accept bogus reviewers. - Accepted with myself not a reviewer - Accepted with only one reviewer (just got normal "this will be accepted" text). {F4251255} Reviewers: chad Reviewed By: chad Maniphest Tasks: T12271 Differential Revision: https://secure.phabricator.com/D17533
This commit is contained in:
parent
e1ee8ba428
commit
fab37aa4e3
11 changed files with 361 additions and 16 deletions
|
@ -9,7 +9,7 @@ return array(
|
||||||
'names' => array(
|
'names' => array(
|
||||||
'conpherence.pkg.css' => '82aca405',
|
'conpherence.pkg.css' => '82aca405',
|
||||||
'conpherence.pkg.js' => '6249a1cf',
|
'conpherence.pkg.js' => '6249a1cf',
|
||||||
'core.pkg.css' => 'c012c648',
|
'core.pkg.css' => 'dc689e29',
|
||||||
'core.pkg.js' => '1fa7c0c5',
|
'core.pkg.js' => '1fa7c0c5',
|
||||||
'darkconsole.pkg.js' => 'e7393ebb',
|
'darkconsole.pkg.js' => 'e7393ebb',
|
||||||
'differential.pkg.css' => '90b30783',
|
'differential.pkg.css' => '90b30783',
|
||||||
|
@ -145,7 +145,7 @@ return array(
|
||||||
'rsrc/css/phui/phui-document.css' => 'c32e8dec',
|
'rsrc/css/phui/phui-document.css' => 'c32e8dec',
|
||||||
'rsrc/css/phui/phui-feed-story.css' => '44a9c8e9',
|
'rsrc/css/phui/phui-feed-story.css' => '44a9c8e9',
|
||||||
'rsrc/css/phui/phui-fontkit.css' => '1320ed01',
|
'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-form.css' => 'b62c01d8',
|
||||||
'rsrc/css/phui/phui-head-thing.css' => 'fd311e5f',
|
'rsrc/css/phui/phui-head-thing.css' => 'fd311e5f',
|
||||||
'rsrc/css/phui/phui-header-view.css' => '9cf828ce',
|
'rsrc/css/phui/phui-header-view.css' => '9cf828ce',
|
||||||
|
@ -530,7 +530,7 @@ return array(
|
||||||
'rsrc/js/phuix/PHUIXActionView.js' => 'b3465b9b',
|
'rsrc/js/phuix/PHUIXActionView.js' => 'b3465b9b',
|
||||||
'rsrc/js/phuix/PHUIXAutocomplete.js' => '7c492cd2',
|
'rsrc/js/phuix/PHUIXAutocomplete.js' => '7c492cd2',
|
||||||
'rsrc/js/phuix/PHUIXDropdownMenu.js' => '8018ee50',
|
'rsrc/js/phuix/PHUIXDropdownMenu.js' => '8018ee50',
|
||||||
'rsrc/js/phuix/PHUIXFormControl.js' => 'bbece68d',
|
'rsrc/js/phuix/PHUIXFormControl.js' => '83e03671',
|
||||||
'rsrc/js/phuix/PHUIXIconView.js' => 'bff6884b',
|
'rsrc/js/phuix/PHUIXIconView.js' => 'bff6884b',
|
||||||
),
|
),
|
||||||
'symbols' => array(
|
'symbols' => array(
|
||||||
|
@ -847,7 +847,7 @@ return array(
|
||||||
'phui-font-icon-base-css' => '870a7360',
|
'phui-font-icon-base-css' => '870a7360',
|
||||||
'phui-fontkit-css' => '1320ed01',
|
'phui-fontkit-css' => '1320ed01',
|
||||||
'phui-form-css' => 'b62c01d8',
|
'phui-form-css' => 'b62c01d8',
|
||||||
'phui-form-view-css' => 'cf198e10',
|
'phui-form-view-css' => '6175808d',
|
||||||
'phui-head-thing-view-css' => 'fd311e5f',
|
'phui-head-thing-view-css' => 'fd311e5f',
|
||||||
'phui-header-view-css' => '9cf828ce',
|
'phui-header-view-css' => '9cf828ce',
|
||||||
'phui-hovercard' => '1bd28176',
|
'phui-hovercard' => '1bd28176',
|
||||||
|
@ -887,7 +887,7 @@ return array(
|
||||||
'phuix-action-view' => 'b3465b9b',
|
'phuix-action-view' => 'b3465b9b',
|
||||||
'phuix-autocomplete' => '7c492cd2',
|
'phuix-autocomplete' => '7c492cd2',
|
||||||
'phuix-dropdown-menu' => '8018ee50',
|
'phuix-dropdown-menu' => '8018ee50',
|
||||||
'phuix-form-control-view' => 'bbece68d',
|
'phuix-form-control-view' => '83e03671',
|
||||||
'phuix-icon-view' => 'bff6884b',
|
'phuix-icon-view' => 'bff6884b',
|
||||||
'policy-css' => '957ea14c',
|
'policy-css' => '957ea14c',
|
||||||
'policy-edit-css' => '815c66f7',
|
'policy-edit-css' => '815c66f7',
|
||||||
|
@ -1518,6 +1518,10 @@ return array(
|
||||||
'javelin-behavior',
|
'javelin-behavior',
|
||||||
'javelin-scrollbar',
|
'javelin-scrollbar',
|
||||||
),
|
),
|
||||||
|
'83e03671' => array(
|
||||||
|
'javelin-install',
|
||||||
|
'javelin-dom',
|
||||||
|
),
|
||||||
'8499b6ab' => array(
|
'8499b6ab' => array(
|
||||||
'javelin-behavior',
|
'javelin-behavior',
|
||||||
'javelin-dom',
|
'javelin-dom',
|
||||||
|
@ -1902,10 +1906,6 @@ return array(
|
||||||
'javelin-vector',
|
'javelin-vector',
|
||||||
'javelin-install',
|
'javelin-install',
|
||||||
),
|
),
|
||||||
'bbece68d' => array(
|
|
||||||
'javelin-install',
|
|
||||||
'javelin-dom',
|
|
||||||
),
|
|
||||||
'bcaccd64' => array(
|
'bcaccd64' => array(
|
||||||
'javelin-behavior',
|
'javelin-behavior',
|
||||||
'javelin-behavior-device',
|
'javelin-behavior-device',
|
||||||
|
|
|
@ -2602,6 +2602,7 @@ phutil_register_library_map(array(
|
||||||
'PhabricatorEdgesDestructionEngineExtension' => 'infrastructure/edges/engineextension/PhabricatorEdgesDestructionEngineExtension.php',
|
'PhabricatorEdgesDestructionEngineExtension' => 'infrastructure/edges/engineextension/PhabricatorEdgesDestructionEngineExtension.php',
|
||||||
'PhabricatorEditEngine' => 'applications/transactions/editengine/PhabricatorEditEngine.php',
|
'PhabricatorEditEngine' => 'applications/transactions/editengine/PhabricatorEditEngine.php',
|
||||||
'PhabricatorEditEngineAPIMethod' => 'applications/transactions/editengine/PhabricatorEditEngineAPIMethod.php',
|
'PhabricatorEditEngineAPIMethod' => 'applications/transactions/editengine/PhabricatorEditEngineAPIMethod.php',
|
||||||
|
'PhabricatorEditEngineCheckboxesCommentAction' => 'applications/transactions/commentaction/PhabricatorEditEngineCheckboxesCommentAction.php',
|
||||||
'PhabricatorEditEngineColumnsCommentAction' => 'applications/transactions/commentaction/PhabricatorEditEngineColumnsCommentAction.php',
|
'PhabricatorEditEngineColumnsCommentAction' => 'applications/transactions/commentaction/PhabricatorEditEngineColumnsCommentAction.php',
|
||||||
'PhabricatorEditEngineCommentAction' => 'applications/transactions/commentaction/PhabricatorEditEngineCommentAction.php',
|
'PhabricatorEditEngineCommentAction' => 'applications/transactions/commentaction/PhabricatorEditEngineCommentAction.php',
|
||||||
'PhabricatorEditEngineCommentActionGroup' => 'applications/transactions/commentaction/PhabricatorEditEngineCommentActionGroup.php',
|
'PhabricatorEditEngineCommentActionGroup' => 'applications/transactions/commentaction/PhabricatorEditEngineCommentActionGroup.php',
|
||||||
|
@ -7686,6 +7687,7 @@ phutil_register_library_map(array(
|
||||||
'PhabricatorPolicyInterface',
|
'PhabricatorPolicyInterface',
|
||||||
),
|
),
|
||||||
'PhabricatorEditEngineAPIMethod' => 'ConduitAPIMethod',
|
'PhabricatorEditEngineAPIMethod' => 'ConduitAPIMethod',
|
||||||
|
'PhabricatorEditEngineCheckboxesCommentAction' => 'PhabricatorEditEngineCommentAction',
|
||||||
'PhabricatorEditEngineColumnsCommentAction' => 'PhabricatorEditEngineCommentAction',
|
'PhabricatorEditEngineColumnsCommentAction' => 'PhabricatorEditEngineCommentAction',
|
||||||
'PhabricatorEditEngineCommentAction' => 'Phobject',
|
'PhabricatorEditEngineCommentAction' => 'Phobject',
|
||||||
'PhabricatorEditEngineCommentActionGroup' => 'Phobject',
|
'PhabricatorEditEngineCommentActionGroup' => 'Phobject',
|
||||||
|
|
|
@ -50,4 +50,35 @@ final class DifferentialReviewer
|
||||||
return ($this->getReviewerStatus() == $status_resigned);
|
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;
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -48,6 +48,72 @@ final class DifferentialRevisionAcceptTransaction
|
||||||
return pht('Accept a revision.');
|
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) {
|
public function generateOldValue($object) {
|
||||||
$actor = $this->getActor();
|
$actor = $this->getActor();
|
||||||
return $this->isViewerFullyAccepted($object, $actor);
|
return $this->isViewerFullyAccepted($object, $actor);
|
||||||
|
@ -87,11 +153,40 @@ final class DifferentialRevisionAcceptTransaction
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
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() {
|
public function getTitle() {
|
||||||
|
$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(
|
return pht(
|
||||||
'%s accepted this revision.',
|
'%s accepted this revision.',
|
||||||
$this->renderAuthor());
|
$this->renderAuthor());
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
public function getTitleForFeed() {
|
public function getTitleForFeed() {
|
||||||
return pht(
|
return pht(
|
||||||
|
|
|
@ -19,6 +19,10 @@ abstract class DifferentialRevisionActionTransaction
|
||||||
abstract protected function validateAction($object, PhabricatorUser $viewer);
|
abstract protected function validateAction($object, PhabricatorUser $viewer);
|
||||||
abstract protected function getRevisionActionLabel();
|
abstract protected function getRevisionActionLabel();
|
||||||
|
|
||||||
|
protected function validateOptionValue($object, $actor, array $value) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
public function getCommandKeyword() {
|
public function getCommandKeyword() {
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
@ -70,6 +74,15 @@ abstract class DifferentialRevisionActionTransaction
|
||||||
return ($viewer->getPHID() === $revision->getAuthorPHID());
|
return ($viewer->getPHID() === $revision->getAuthorPHID());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
protected function getActionOptions(
|
||||||
|
PhabricatorUser $viewer,
|
||||||
|
DifferentialRevision $revision) {
|
||||||
|
return array(
|
||||||
|
array(),
|
||||||
|
null,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
public function newEditField(
|
public function newEditField(
|
||||||
DifferentialRevision $revision,
|
DifferentialRevision $revision,
|
||||||
PhabricatorUser $viewer) {
|
PhabricatorUser $viewer) {
|
||||||
|
@ -107,6 +120,12 @@ abstract class DifferentialRevisionActionTransaction
|
||||||
// It's not clear that these combinations are actually useful, so just
|
// It's not clear that these combinations are actually useful, so just
|
||||||
// keep things simple for now.
|
// keep things simple for now.
|
||||||
$field->setActionConflictKey('revision.action');
|
$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 @@ abstract class DifferentialRevisionActionTransaction
|
||||||
$errors[] = $this->newInvalidError(
|
$errors[] = $this->newInvalidError(
|
||||||
$action_exception->getMessage(),
|
$action_exception->getMessage(),
|
||||||
$xaction);
|
$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);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -7,6 +7,26 @@ abstract class DifferentialRevisionReviewTransaction
|
||||||
return DifferentialRevisionEditEngine::ACTIONGROUP_REVIEW;
|
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(
|
protected function isViewerAnyReviewer(
|
||||||
DifferentialRevision $revision,
|
DifferentialRevision $revision,
|
||||||
PhabricatorUser $viewer) {
|
PhabricatorUser $viewer) {
|
||||||
|
@ -118,6 +138,12 @@ abstract class DifferentialRevisionReviewTransaction
|
||||||
// In all cases, you affect yourself.
|
// In all cases, you affect yourself.
|
||||||
$map[$viewer->getPHID()] = $status;
|
$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.
|
// Convert reviewer statuses into edge data.
|
||||||
foreach ($map as $reviewer_phid => $reviewer_status) {
|
foreach ($map as $reviewer_phid => $reviewer_status) {
|
||||||
$map[$reviewer_phid] = array(
|
$map[$reviewer_phid] = array(
|
||||||
|
|
|
@ -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,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
}
|
|
@ -5,6 +5,7 @@ final class PhabricatorApplyEditField
|
||||||
|
|
||||||
private $actionDescription;
|
private $actionDescription;
|
||||||
private $actionConflictKey;
|
private $actionConflictKey;
|
||||||
|
private $options;
|
||||||
|
|
||||||
protected function newControl() {
|
protected function newControl() {
|
||||||
return null;
|
return null;
|
||||||
|
@ -28,9 +29,22 @@ final class PhabricatorApplyEditField
|
||||||
return $this->actionConflictKey;
|
return $this->actionConflictKey;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function setOptions(array $options) {
|
||||||
|
$this->options = $options;
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getOptions() {
|
||||||
|
return $this->options;
|
||||||
|
}
|
||||||
|
|
||||||
protected function newHTTPParameterType() {
|
protected function newHTTPParameterType() {
|
||||||
|
if ($this->getOptions()) {
|
||||||
|
return new AphrontPHIDListHTTPParameterType();
|
||||||
|
} else {
|
||||||
return new AphrontBoolHTTPParameterType();
|
return new AphrontBoolHTTPParameterType();
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
protected function newConduitParameterType() {
|
protected function newConduitParameterType() {
|
||||||
return new ConduitBoolParameterType();
|
return new ConduitBoolParameterType();
|
||||||
|
@ -43,9 +57,16 @@ final class PhabricatorApplyEditField
|
||||||
}
|
}
|
||||||
|
|
||||||
protected function newCommentAction() {
|
protected function newCommentAction() {
|
||||||
|
$options = $this->getOptions();
|
||||||
|
if ($options) {
|
||||||
|
return id(new PhabricatorEditEngineCheckboxesCommentAction())
|
||||||
|
->setConflictKey($this->getActionConflictKey())
|
||||||
|
->setOptions($options);
|
||||||
|
} else {
|
||||||
return id(new PhabricatorEditEngineStaticCommentAction())
|
return id(new PhabricatorEditEngineStaticCommentAction())
|
||||||
->setDescription($this->getActionDescription())
|
->setConflictKey($this->getActionConflictKey())
|
||||||
->setConflictKey($this->getActionConflictKey());
|
->setDescription($this->getActionDescription());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -1608,6 +1608,8 @@ final class PhabricatorUSEnglishTranslation
|
||||||
),
|
),
|
||||||
),
|
),
|
||||||
|
|
||||||
|
'%s accepted this revision as %s reviewer(s): %s.' =>
|
||||||
|
'%s accepted this revision as: %3$s.',
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -548,3 +548,16 @@ properly, and submit values. */
|
||||||
padding: 4px;
|
padding: 4px;
|
||||||
color: {$bluetext};
|
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;
|
||||||
|
}
|
||||||
|
|
|
@ -50,6 +50,9 @@ JX.install('PHUIXFormControl', {
|
||||||
case 'static':
|
case 'static':
|
||||||
input = this._newStatic(spec);
|
input = this._newStatic(spec);
|
||||||
break;
|
break;
|
||||||
|
case 'checkboxes':
|
||||||
|
input = this._newCheckboxes(spec);
|
||||||
|
break;
|
||||||
default:
|
default:
|
||||||
// TODO: Default or better error?
|
// TODO: Default or better error?
|
||||||
JX.$E('Bad Input Type');
|
JX.$E('Bad Input Type');
|
||||||
|
@ -194,6 +197,89 @@ JX.install('PHUIXFormControl', {
|
||||||
};
|
};
|
||||||
},
|
},
|
||||||
|
|
||||||
|
_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) {
|
_newPoints: function(spec) {
|
||||||
var attrs = {
|
var attrs = {
|
||||||
type: 'text',
|
type: 'text',
|
||||||
|
|
Loading…
Reference in a new issue