Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F13176058
D15915.id38331.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
14 KB
Referenced Files
None
Subscribers
None
D15915.id38331.diff
View Options
diff --git a/resources/sql/autopatches/20160513.owners.01.autoreview.sql b/resources/sql/autopatches/20160513.owners.01.autoreview.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20160513.owners.01.autoreview.sql
@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_owners.owners_package
+ ADD autoReview VARCHAR(32) NOT NULL COLLATE {$COLLATE_TEXT};
diff --git a/resources/sql/autopatches/20160513.owners.02.autoreviewnone.sql b/resources/sql/autopatches/20160513.owners.02.autoreviewnone.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20160513.owners.02.autoreviewnone.sql
@@ -0,0 +1,2 @@
+UPDATE {$NAMESPACE}_owners.owners_package
+ SET autoReview = 'none' WHERE autoReview = '';
diff --git a/src/applications/differential/customfield/DifferentialProjectReviewersField.php b/src/applications/differential/customfield/DifferentialProjectReviewersField.php
--- a/src/applications/differential/customfield/DifferentialProjectReviewersField.php
+++ b/src/applications/differential/customfield/DifferentialProjectReviewersField.php
@@ -8,7 +8,7 @@
}
public function getFieldName() {
- return pht('Coalition Reviewers');
+ return pht('Group Reviewers');
}
public function getFieldDescription() {
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
@@ -7,6 +7,7 @@
private $isCloseByCommit;
private $repositoryPHIDOverride = false;
private $didExpandInlineState = false;
+ private $affectedPaths;
public function getEditorApplicationClass() {
return 'PhabricatorDifferentialApplication';
@@ -1481,6 +1482,75 @@
return parent::shouldApplyHeraldRules($object, $xactions);
}
+ protected function didApplyHeraldRules(
+ PhabricatorLiskDAO $object,
+ HeraldAdapter $adapter,
+ HeraldTranscript $transcript) {
+
+ $repository = $object->getRepository();
+ if (!$repository) {
+ return array();
+ }
+
+ if (!$this->affectedPaths) {
+ return array();
+ }
+
+ $packages = PhabricatorOwnersPackage::loadAffectedPackages(
+ $repository,
+ $this->affectedPaths);
+
+ foreach ($packages as $key => $package) {
+ if ($package->isArchived()) {
+ unset($packages[$key]);
+ }
+ }
+
+ if (!$packages) {
+ return array();
+ }
+
+ $auto_subscribe = array();
+ $auto_review = array();
+ $auto_block = array();
+
+ foreach ($packages as $package) {
+ switch ($package->getAutoReview()) {
+ case PhabricatorOwnersPackage::AUTOREVIEW_SUBSCRIBE:
+ $auto_subscribe[] = $package;
+ break;
+ case PhabricatorOwnersPackage::AUTOREVIEW_REVIEW:
+ $auto_review[] = $package;
+ break;
+ case PhabricatorOwnersPackage::AUTOREVIEW_BLOCK:
+ $auto_block[] = $package;
+ break;
+ case PhabricatorOwnersPackage::AUTOREVIEW_NONE:
+ default:
+ break;
+ }
+ }
+
+ $owners_phid = id(new PhabricatorOwnersApplication())
+ ->getPHID();
+
+ $xactions = array();
+ if ($auto_subscribe) {
+
+ $xactions[] = $object->getApplicationTransactionTemplate()
+ ->setAuthorPHID($owners_phid)
+ ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS)
+ ->setNewValue(
+ array(
+ '+' => mpull($auto_subscribe, 'getPHID'),
+ ));
+ }
+
+ // TODO: Implement autoreview and autoblock, but these are more invovled.
+
+ return $xactions;
+ }
+
protected function buildHeraldAdapter(
PhabricatorLiskDAO $object,
array $xactions) {
@@ -1557,6 +1627,9 @@
}
$all_paths = array_keys($all_paths);
+ // Save the affected paths; we'll use them later to query Owners.
+ $this->affectedPaths = $all_paths;
+
$path_ids =
PhabricatorRepositoryCommitChangeParserWorker::lookupOrCreatePaths(
$all_paths);
diff --git a/src/applications/owners/controller/PhabricatorOwnersDetailController.php b/src/applications/owners/controller/PhabricatorOwnersDetailController.php
--- a/src/applications/owners/controller/PhabricatorOwnersDetailController.php
+++ b/src/applications/owners/controller/PhabricatorOwnersDetailController.php
@@ -184,6 +184,12 @@
}
$view->addProperty(pht('Owners'), $owner_list);
+ $auto = $package->getAutoReview();
+ $autoreview_map = PhabricatorOwnersPackage::getAutoreviewOptionsMap();
+ $spec = idx($autoreview_map, $auto, array());
+ $name = idx($spec, 'name', $auto);
+ $view->addProperty(pht('Auto Review'), $name);
+
if ($package->getAuditingEnabled()) {
$auditing = pht('Enabled');
} else {
diff --git a/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php b/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php
--- a/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php
+++ b/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php
@@ -84,6 +84,9 @@
EOTEXT
);
+ $autoreview_map = PhabricatorOwnersPackage::getAutoreviewOptionsMap();
+ $autoreview_map = ipull($autoreview_map, 'name');
+
return array(
id(new PhabricatorTextEditField())
->setKey('name')
@@ -101,6 +104,18 @@
->setIsCopyable(true)
->setValue($object->getOwnerPHIDs()),
id(new PhabricatorSelectEditField())
+ ->setKey('autoReview')
+ ->setLabel(pht('Auto Review'))
+ ->setDescription(
+ pht(
+ 'Automatically trigger reviews for commits affecting files in '.
+ 'this package.'))
+ ->setTransactionType(
+ PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW)
+ ->setIsCopyable(true)
+ ->setValue($object->getAutoReview())
+ ->setOptions($autoreview_map),
+ id(new PhabricatorSelectEditField())
->setKey('auditing')
->setLabel(pht('Auditing'))
->setDescription(
diff --git a/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php b/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php
--- a/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php
+++ b/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php
@@ -20,6 +20,7 @@
$types[] = PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION;
$types[] = PhabricatorOwnersPackageTransaction::TYPE_PATHS;
$types[] = PhabricatorOwnersPackageTransaction::TYPE_STATUS;
+ $types[] = PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW;
$types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
$types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
@@ -47,6 +48,8 @@
return mpull($paths, 'getRef');
case PhabricatorOwnersPackageTransaction::TYPE_STATUS:
return $object->getStatus();
+ case PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW:
+ return $object->getAutoReview();
}
}
@@ -58,6 +61,7 @@
case PhabricatorOwnersPackageTransaction::TYPE_NAME:
case PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION:
case PhabricatorOwnersPackageTransaction::TYPE_STATUS:
+ case PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW:
return $xaction->getNewValue();
case PhabricatorOwnersPackageTransaction::TYPE_PATHS:
$new = $xaction->getNewValue();
@@ -113,6 +117,9 @@
case PhabricatorOwnersPackageTransaction::TYPE_STATUS:
$object->setStatus($xaction->getNewValue());
return;
+ case PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW:
+ $object->setAutoReview($xaction->getNewValue());
+ return;
}
return parent::applyCustomInternalTransaction($object, $xaction);
@@ -127,6 +134,7 @@
case PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION:
case PhabricatorOwnersPackageTransaction::TYPE_AUDITING:
case PhabricatorOwnersPackageTransaction::TYPE_STATUS:
+ case PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW:
return;
case PhabricatorOwnersPackageTransaction::TYPE_OWNERS:
$old = $xaction->getOldValue();
@@ -221,6 +229,26 @@
}
break;
+ case PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW:
+ $map = PhabricatorOwnersPackage::getAutoreviewOptionsMap();
+ foreach ($xactions as $xaction) {
+ $new = $xaction->getNewValue();
+
+ if (empty($map[$new])) {
+ $valid = array_keys($map);
+
+ $errors[] = new PhabricatorApplicationTransactionValidationError(
+ $type,
+ pht('Invalid'),
+ pht(
+ 'Autoreview setting "%s" is not valid. '.
+ 'Valid settings are: %s.',
+ $new,
+ implode(', ', $valid)),
+ $xaction);
+ }
+ }
+ break;
case PhabricatorOwnersPackageTransaction::TYPE_PATHS:
if (!$xactions) {
continue;
diff --git a/src/applications/owners/storage/PhabricatorOwnersPackage.php b/src/applications/owners/storage/PhabricatorOwnersPackage.php
--- a/src/applications/owners/storage/PhabricatorOwnersPackage.php
+++ b/src/applications/owners/storage/PhabricatorOwnersPackage.php
@@ -14,6 +14,7 @@
protected $name;
protected $originalName;
protected $auditingEnabled;
+ protected $autoReview;
protected $description;
protected $primaryOwnerPHID;
protected $mailKey;
@@ -28,6 +29,11 @@
const STATUS_ACTIVE = 'active';
const STATUS_ARCHIVED = 'archived';
+ const AUTOREVIEW_NONE = 'none';
+ const AUTOREVIEW_SUBSCRIBE = 'subscribe';
+ const AUTOREVIEW_REVIEW = 'review';
+ const AUTOREVIEW_BLOCK = 'block';
+
public static function initializeNewPackage(PhabricatorUser $actor) {
$app = id(new PhabricatorApplicationQuery())
->setViewer($actor)
@@ -41,6 +47,7 @@
return id(new PhabricatorOwnersPackage())
->setAuditingEnabled(0)
+ ->setAutoReview(self::AUTOREVIEW_NONE)
->setViewPolicy($view_policy)
->setEditPolicy($edit_policy)
->attachPaths(array())
@@ -56,6 +63,24 @@
);
}
+ public static function getAutoreviewOptionsMap() {
+ return array(
+ self::AUTOREVIEW_NONE => array(
+ 'name' => pht('No Autoreview'),
+ ),
+ self::AUTOREVIEW_SUBSCRIBE => array(
+ 'name' => pht('Subscribe to Changes'),
+ ),
+ // TODO: Implement these.
+ // self::AUTOREVIEW_REVIEW => array(
+ // 'name' => pht('Review Changes'),
+ // ),
+ // self::AUTOREVIEW_BLOCK => array(
+ // 'name' => pht('Review Changes (Blocking)'),
+ // ),
+ );
+ }
+
protected function getConfiguration() {
return array(
// This information is better available from the history table.
@@ -69,6 +94,7 @@
'auditingEnabled' => 'bool',
'mailKey' => 'bytes20',
'status' => 'text32',
+ 'autoReview' => 'text32',
),
) + parent::getConfiguration();
}
diff --git a/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php b/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php
--- a/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php
+++ b/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php
@@ -10,6 +10,7 @@
const TYPE_DESCRIPTION = 'owners.description';
const TYPE_PATHS = 'owners.paths';
const TYPE_STATUS = 'owners.status';
+ const TYPE_AUTOREVIEW = 'owners.autoreview';
public function getApplicationName() {
return 'owners';
@@ -143,6 +144,18 @@
'%s archived this package.',
$this->renderHandleLink($author_phid));
}
+ case self::TYPE_AUTOREVIEW:
+ $map = PhabricatorOwnersPackage::getAutoreviewOptionsMap();
+ $map = ipull($map, 'name');
+
+ $old = idx($map, $old, $old);
+ $new = idx($map, $new, $new);
+
+ return pht(
+ '%s adjusted autoreview from "%s" to "%s".',
+ $this->renderHandleLink($author_phid),
+ $old,
+ $new);
}
return parent::getTitle();
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
@@ -703,7 +703,13 @@
$xaction->setEditPolicy($this->getActingAsPHID());
}
- $xaction->setAuthorPHID($this->getActingAsPHID());
+ // If the transaction already has an explicit author PHID, allow it to
+ // stand. This is used by applications like Owners that hook into the
+ // post-apply change pipeline.
+ if (!$xaction->getAuthorPHID()) {
+ $xaction->setAuthorPHID($this->getActingAsPHID());
+ }
+
$xaction->setContentSource($this->getContentSource());
$xaction->attachViewer($actor);
$xaction->attachObject($object);
@@ -957,6 +963,12 @@
if ($herald_xactions) {
$xscript_id = $this->getHeraldTranscript()->getID();
foreach ($herald_xactions as $herald_xaction) {
+ // Don't set a transcript ID if this is a transaction from another
+ // application or source, like Owners.
+ if ($herald_xaction->getAuthorPHID()) {
+ continue;
+ }
+
$herald_xaction->setMetadataValue('herald:transcriptID', $xscript_id);
}
@@ -1217,6 +1229,7 @@
$xaction,
pht('You can not apply transactions which already have IDs/PHIDs!'));
}
+
if ($xaction->getObjectPHID()) {
throw new PhabricatorApplicationTransactionStructureException(
$xaction,
@@ -1224,13 +1237,7 @@
'You can not apply transactions which already have %s!',
'objectPHIDs'));
}
- if ($xaction->getAuthorPHID()) {
- throw new PhabricatorApplicationTransactionStructureException(
- $xaction,
- pht(
- 'You can not apply transactions which already have %s!',
- 'authorPHIDs'));
- }
+
if ($xaction->getCommentPHID()) {
throw new PhabricatorApplicationTransactionStructureException(
$xaction,
@@ -1238,6 +1245,7 @@
'You can not apply transactions which already have %s!',
'commentPHIDs'));
}
+
if ($xaction->getCommentVersion() !== 0) {
throw new PhabricatorApplicationTransactionStructureException(
$xaction,
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Thu, May 9, 10:01 AM (3 w, 3 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6274599
Default Alt Text
D15915.id38331.diff (14 KB)
Attached To
Mode
D15915: Implement "Auto Review" in packages with a "Subscribe" option
Attached
Detach File
Event Timeline
Log In to Comment