Add a "default view" policy to Differential
Summary: Ref T603. Allows the Differential view policy to be configured with a default. I've omitted "edit" because I want to wait and see how comment/comment-action policies work out. I could imagine locking "edit" down to only the owner at some point, and providing a wider "interact" capability, or something like that, which would cover accept/reject/commandeer. Users in this group could still edit indirectly by commandeering first. Test Plan: Created new revisions from the CLI and conduit. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T603 Differential Revision: https://secure.phabricator.com/D7269
This commit is contained in:
@@ -309,6 +309,7 @@ phutil_register_library_map(array(
|
|||||||
'DifferentialBranchFieldSpecification' => 'applications/differential/field/specification/DifferentialBranchFieldSpecification.php',
|
'DifferentialBranchFieldSpecification' => 'applications/differential/field/specification/DifferentialBranchFieldSpecification.php',
|
||||||
'DifferentialCCWelcomeMail' => 'applications/differential/mail/DifferentialCCWelcomeMail.php',
|
'DifferentialCCWelcomeMail' => 'applications/differential/mail/DifferentialCCWelcomeMail.php',
|
||||||
'DifferentialCCsFieldSpecification' => 'applications/differential/field/specification/DifferentialCCsFieldSpecification.php',
|
'DifferentialCCsFieldSpecification' => 'applications/differential/field/specification/DifferentialCCsFieldSpecification.php',
|
||||||
|
'DifferentialCapabilityDefaultView' => 'applications/differential/capability/DifferentialCapabilityDefaultView.php',
|
||||||
'DifferentialChangeType' => 'applications/differential/constants/DifferentialChangeType.php',
|
'DifferentialChangeType' => 'applications/differential/constants/DifferentialChangeType.php',
|
||||||
'DifferentialChangeset' => 'applications/differential/storage/DifferentialChangeset.php',
|
'DifferentialChangeset' => 'applications/differential/storage/DifferentialChangeset.php',
|
||||||
'DifferentialChangesetDetailView' => 'applications/differential/view/DifferentialChangesetDetailView.php',
|
'DifferentialChangesetDetailView' => 'applications/differential/view/DifferentialChangesetDetailView.php',
|
||||||
@@ -2410,6 +2411,7 @@ phutil_register_library_map(array(
|
|||||||
'DifferentialBranchFieldSpecification' => 'DifferentialFieldSpecification',
|
'DifferentialBranchFieldSpecification' => 'DifferentialFieldSpecification',
|
||||||
'DifferentialCCWelcomeMail' => 'DifferentialReviewRequestMail',
|
'DifferentialCCWelcomeMail' => 'DifferentialReviewRequestMail',
|
||||||
'DifferentialCCsFieldSpecification' => 'DifferentialFieldSpecification',
|
'DifferentialCCsFieldSpecification' => 'DifferentialFieldSpecification',
|
||||||
|
'DifferentialCapabilityDefaultView' => 'PhabricatorPolicyCapability',
|
||||||
'DifferentialChangeset' => 'DifferentialDAO',
|
'DifferentialChangeset' => 'DifferentialDAO',
|
||||||
'DifferentialChangesetDetailView' => 'AphrontView',
|
'DifferentialChangesetDetailView' => 'AphrontView',
|
||||||
'DifferentialChangesetHTMLRenderer' => 'DifferentialChangesetRenderer',
|
'DifferentialChangesetHTMLRenderer' => 'DifferentialChangesetRenderer',
|
||||||
|
|||||||
@@ -118,5 +118,14 @@ final class PhabricatorApplicationDifferential extends PhabricatorApplication {
|
|||||||
return $status;
|
return $status;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
protected function getCustomCapabilities() {
|
||||||
|
return array(
|
||||||
|
DifferentialCapabilityDefaultView::CAPABILITY => array(
|
||||||
|
'caption' => pht(
|
||||||
|
'Default view policy for newly created revisions.')
|
||||||
|
),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,16 @@
|
|||||||
|
<?php
|
||||||
|
|
||||||
|
final class DifferentialCapabilityDefaultView
|
||||||
|
extends PhabricatorPolicyCapability {
|
||||||
|
|
||||||
|
const CAPABILITY = 'differential.default.view';
|
||||||
|
|
||||||
|
public function getCapabilityKey() {
|
||||||
|
return self::CAPABILITY;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getCapabilityName() {
|
||||||
|
return pht('Default View Policy');
|
||||||
|
}
|
||||||
|
|
||||||
|
}
|
||||||
@@ -30,11 +30,12 @@ final class ConduitAPI_differential_getcommitmessage_Method
|
|||||||
|
|
||||||
protected function execute(ConduitAPIRequest $request) {
|
protected function execute(ConduitAPIRequest $request) {
|
||||||
$id = $request->getValue('revision_id');
|
$id = $request->getValue('revision_id');
|
||||||
|
$viewer = $request->getUser();
|
||||||
|
|
||||||
if ($id) {
|
if ($id) {
|
||||||
$revision = id(new DifferentialRevisionQuery())
|
$revision = id(new DifferentialRevisionQuery())
|
||||||
->withIDs(array($id))
|
->withIDs(array($id))
|
||||||
->setViewer($request->getUser())
|
->setViewer($viewer)
|
||||||
->needRelationships(true)
|
->needRelationships(true)
|
||||||
->needReviewerStatus(true)
|
->needReviewerStatus(true)
|
||||||
->executeOne();
|
->executeOne();
|
||||||
@@ -43,8 +44,7 @@ final class ConduitAPI_differential_getcommitmessage_Method
|
|||||||
throw new ConduitException('ERR_NOT_FOUND');
|
throw new ConduitException('ERR_NOT_FOUND');
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
$revision = new DifferentialRevision();
|
$revision = DifferentialRevision::initializeNewRevision($viewer);
|
||||||
$revision->attachRelationships(array());
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
@@ -57,7 +57,7 @@ final class ConduitAPI_differential_getcommitmessage_Method
|
|||||||
$pro_tips = array();
|
$pro_tips = array();
|
||||||
|
|
||||||
foreach ($aux_fields as $key => $aux_field) {
|
foreach ($aux_fields as $key => $aux_field) {
|
||||||
$aux_field->setUser($request->getUser());
|
$aux_field->setUser($viewer);
|
||||||
$aux_field->setRevision($revision);
|
$aux_field->setRevision($revision);
|
||||||
$pro_tips[] = $aux_field->getCommitMessageTips();
|
$pro_tips[] = $aux_field->getCommitMessageTips();
|
||||||
if (!$aux_field->shouldAppearOnCommitMessage()) {
|
if (!$aux_field->shouldAppearOnCommitMessage()) {
|
||||||
|
|||||||
@@ -32,8 +32,7 @@ final class DifferentialRevisionEditController extends DifferentialController {
|
|||||||
return new Aphront404Response();
|
return new Aphront404Response();
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
$revision = new DifferentialRevision();
|
$revision = DifferentialRevision::initializeNewRevision($viewer);
|
||||||
$revision->attachRelationships(array());
|
|
||||||
}
|
}
|
||||||
|
|
||||||
$aux_fields = $this->loadAuxiliaryFields($revision);
|
$aux_fields = $this->loadAuxiliaryFields($revision);
|
||||||
|
|||||||
@@ -40,10 +40,8 @@ final class DifferentialRevisionEditor extends PhabricatorEditor {
|
|||||||
DifferentialDiff $diff,
|
DifferentialDiff $diff,
|
||||||
PhabricatorUser $actor) {
|
PhabricatorUser $actor) {
|
||||||
|
|
||||||
$revision = new DifferentialRevision();
|
$revision = DifferentialRevision::initializeNewRevision($actor);
|
||||||
$revision->setPHID($revision->generatePHID());
|
$revision->setPHID($revision->generatePHID());
|
||||||
$revision->setAuthorPHID($actor->getPHID());
|
|
||||||
$revision->setStatus(ArcanistDifferentialRevisionStatus::NEEDS_REVIEW);
|
|
||||||
|
|
||||||
$editor = new DifferentialRevisionEditor($revision);
|
$editor = new DifferentialRevisionEditor($revision);
|
||||||
$editor->setActor($actor);
|
$editor->setActor($actor);
|
||||||
@@ -168,9 +166,6 @@ final class DifferentialRevisionEditor extends PhabricatorEditor {
|
|||||||
$revision = $this->getRevision();
|
$revision = $this->getRevision();
|
||||||
|
|
||||||
$is_new = $this->isNewRevision();
|
$is_new = $this->isNewRevision();
|
||||||
if ($is_new) {
|
|
||||||
$this->initializeNewRevision($revision);
|
|
||||||
}
|
|
||||||
|
|
||||||
$revision->loadRelationships();
|
$revision->loadRelationships();
|
||||||
|
|
||||||
@@ -1101,29 +1096,6 @@ final class DifferentialRevisionEditor extends PhabricatorEditor {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private function initializeNewRevision(DifferentialRevision $revision) {
|
|
||||||
// These fields aren't nullable; set them to sensible defaults if they
|
|
||||||
// haven't been configured. We're just doing this so we can generate an
|
|
||||||
// ID for the revision if we don't have one already.
|
|
||||||
$revision->setLineCount(0);
|
|
||||||
if ($revision->getStatus() === null) {
|
|
||||||
$revision->setStatus(ArcanistDifferentialRevisionStatus::NEEDS_REVIEW);
|
|
||||||
}
|
|
||||||
if ($revision->getTitle() === null) {
|
|
||||||
$revision->setTitle('Untitled Revision');
|
|
||||||
}
|
|
||||||
if ($revision->getAuthorPHID() === null) {
|
|
||||||
$revision->setAuthorPHID($this->getActorPHID());
|
|
||||||
}
|
|
||||||
if ($revision->getSummary() === null) {
|
|
||||||
$revision->setSummary('');
|
|
||||||
}
|
|
||||||
if ($revision->getTestPlan() === null) {
|
|
||||||
$revision->setTestPlan('');
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Try to move a revision to "accepted". We look for:
|
* Try to move a revision to "accepted". We look for:
|
||||||
*
|
*
|
||||||
|
|||||||
@@ -6,7 +6,7 @@ final class PhabricatorDifferentialRevisionTestDataGenerator
|
|||||||
public function generate() {
|
public function generate() {
|
||||||
$author = $this->loadPhabrictorUser();
|
$author = $this->loadPhabrictorUser();
|
||||||
$authorPHID = $author->getPHID();
|
$authorPHID = $author->getPHID();
|
||||||
$revision = new DifferentialRevision();
|
$revision = DifferentialRevision::initializeNewRevision($author);
|
||||||
$revision->setTitle($this->generateTitle());
|
$revision->setTitle($this->generateTitle());
|
||||||
$revision->setSummary($this->generateDescription());
|
$revision->setSummary($this->generateDescription());
|
||||||
$revision->setTestPlan($this->generateDescription());
|
$revision->setTestPlan($this->generateDescription());
|
||||||
|
|||||||
@@ -6,12 +6,12 @@ final class DifferentialRevision extends DifferentialDAO
|
|||||||
PhabricatorPolicyInterface,
|
PhabricatorPolicyInterface,
|
||||||
PhrequentTrackableInterface {
|
PhrequentTrackableInterface {
|
||||||
|
|
||||||
protected $title;
|
protected $title = '';
|
||||||
protected $originalTitle;
|
protected $originalTitle;
|
||||||
protected $status;
|
protected $status;
|
||||||
|
|
||||||
protected $summary;
|
protected $summary = '';
|
||||||
protected $testPlan;
|
protected $testPlan = '';
|
||||||
|
|
||||||
protected $phid;
|
protected $phid;
|
||||||
protected $authorPHID;
|
protected $authorPHID;
|
||||||
@@ -19,7 +19,7 @@ final class DifferentialRevision extends DifferentialDAO
|
|||||||
|
|
||||||
protected $dateCommitted;
|
protected $dateCommitted;
|
||||||
|
|
||||||
protected $lineCount;
|
protected $lineCount = 0;
|
||||||
protected $attached = array();
|
protected $attached = array();
|
||||||
|
|
||||||
protected $mailKey;
|
protected $mailKey;
|
||||||
@@ -44,6 +44,22 @@ final class DifferentialRevision extends DifferentialDAO
|
|||||||
const RELATION_REVIEWER = 'revw';
|
const RELATION_REVIEWER = 'revw';
|
||||||
const RELATION_SUBSCRIBED = 'subd';
|
const RELATION_SUBSCRIBED = 'subd';
|
||||||
|
|
||||||
|
public static function initializeNewRevision(PhabricatorUser $actor) {
|
||||||
|
$app = id(new PhabricatorApplicationQuery())
|
||||||
|
->setViewer($actor)
|
||||||
|
->withClasses(array('PhabricatorApplicationDifferential'))
|
||||||
|
->executeOne();
|
||||||
|
|
||||||
|
$view_policy = $app->getPolicy(
|
||||||
|
DifferentialCapabilityDefaultView::CAPABILITY);
|
||||||
|
|
||||||
|
return id(new DifferentialRevision())
|
||||||
|
->setViewPolicy($view_policy)
|
||||||
|
->setAuthorPHID($actor->getPHID())
|
||||||
|
->attachRelationships(array())
|
||||||
|
->setStatus(ArcanistDifferentialRevisionStatus::NEEDS_REVIEW);
|
||||||
|
}
|
||||||
|
|
||||||
public function getConfiguration() {
|
public function getConfiguration() {
|
||||||
return array(
|
return array(
|
||||||
self::CONFIG_AUX_PHID => true,
|
self::CONFIG_AUX_PHID => true,
|
||||||
|
|||||||
Reference in New Issue
Block a user