Page MenuHomePhabricator
Files F13178146

D13259.diff
No OneTemporary
Actions

D13259.diff
View Options

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
@@ -2645,6 +2645,7 @@
'PhabricatorSubscriptionsEditor' => 'applications/subscriptions/editor/PhabricatorSubscriptionsEditor.php',
'PhabricatorSubscriptionsListController' => 'applications/subscriptions/controller/PhabricatorSubscriptionsListController.php',
'PhabricatorSubscriptionsSubscribeEmailCommand' => 'applications/subscriptions/command/PhabricatorSubscriptionsSubscribeEmailCommand.php',
+ 'PhabricatorSubscriptionsSubscribersPolicyRule' => 'applications/subscriptions/policyrule/PhabricatorSubscriptionsSubscribersPolicyRule.php',
'PhabricatorSubscriptionsTransactionController' => 'applications/subscriptions/controller/PhabricatorSubscriptionsTransactionController.php',
'PhabricatorSubscriptionsUIEventListener' => 'applications/subscriptions/events/PhabricatorSubscriptionsUIEventListener.php',
'PhabricatorSubscriptionsUnsubscribeEmailCommand' => 'applications/subscriptions/command/PhabricatorSubscriptionsUnsubscribeEmailCommand.php',
@@ -6166,6 +6167,7 @@
'PhabricatorSubscriptionsEditor' => 'PhabricatorEditor',
'PhabricatorSubscriptionsListController' => 'PhabricatorController',
'PhabricatorSubscriptionsSubscribeEmailCommand' => 'MetaMTAEmailTransactionCommand',
+ 'PhabricatorSubscriptionsSubscribersPolicyRule' => 'PhabricatorPolicyRule',
'PhabricatorSubscriptionsTransactionController' => 'PhabricatorController',
'PhabricatorSubscriptionsUIEventListener' => 'PhabricatorEventListener',
'PhabricatorSubscriptionsUnsubscribeEmailCommand' => 'MetaMTAEmailTransactionCommand',
diff --git a/src/applications/policy/__tests__/PhabricatorPolicyDataTestCase.php b/src/applications/policy/__tests__/PhabricatorPolicyDataTestCase.php
--- a/src/applications/policy/__tests__/PhabricatorPolicyDataTestCase.php
+++ b/src/applications/policy/__tests__/PhabricatorPolicyDataTestCase.php
@@ -208,5 +208,32 @@
PhabricatorPolicyCapability::CAN_VIEW));
}
+ public function testObjectPolicyRuleSubscribers() {
+ $author = $this->generateNewTestUser();
+
+ $rule = new PhabricatorSubscriptionsSubscribersPolicyRule();
+
+ $task = ManiphestTask::initializeNewTask($author);
+ $task->setViewPolicy($rule->getObjectPolicyFullKey());
+ $task->save();
+
+ $this->assertFalse(
+ PhabricatorPolicyFilter::hasCapability(
+ $author,
+ $task,
+ PhabricatorPolicyCapability::CAN_VIEW));
+
+ id(new PhabricatorSubscriptionsEditor())
+ ->setActor($author)
+ ->setObject($task)
+ ->subscribeExplicit(array($author->getPHID()))
+ ->save();
+
+ $this->assertTrue(
+ PhabricatorPolicyFilter::hasCapability(
+ $author,
+ $task,
+ PhabricatorPolicyCapability::CAN_VIEW));
+ }
}
diff --git a/src/applications/policy/rule/PhabricatorPolicyRule.php b/src/applications/policy/rule/PhabricatorPolicyRule.php
--- a/src/applications/policy/rule/PhabricatorPolicyRule.php
+++ b/src/applications/policy/rule/PhabricatorPolicyRule.php
@@ -97,6 +97,56 @@
}
+/* -( Transaction Hints )-------------------------------------------------- */
+
+
+ /**
+ * Tell policy rules about upcoming transaction effects.
+ *
+ * Before transaction effects are applied, we try to stop users from making
+ * edits which will lock them out of objects. We can't do this perfectly,
+ * since they can set a policy to "the moon is full" moments before it wanes,
+ * but we try to prevent as many mistakes as possible.
+ *
+ * Some policy rules depend on complex checks against object state which
+ * we can't set up ahead of time. For example, subscriptions require database
+ * writes.
+ *
+ * In cases like this, instead of doing writes, you can pass a hint about an
+ * object to a policy rule. The rule can then look for hints and use them in
+ * rendering a verdict about whether the user will be able to see the object
+ * or not after applying the policy change.
+ *
+ * @param PhabricatorPolicyInterface Object to pass a hint about.
+ * @param PhabricatorPolicyRule Rule to pass hint to.
+ * @param wild Hint.
+ * @return void
+ */
+ public static function passTransactionHintToRule(
+ PhabricatorPolicyInterface $object,
+ PhabricatorPolicyRule $rule,
+ $hint) {
+
+ $cache = PhabricatorCaches::getRequestCache();
+ $cache->setKey(self::getObjectPolicyCacheKey($object, $rule), $hint);
+ }
+
+ protected function getTransactionHint(
+ PhabricatorPolicyInterface $object) {
+
+ $cache = PhabricatorCaches::getRequestCache();
+ return $cache->getKey(self::getObjectPolicyCacheKey($object, $this));
+ }
+
+ private static function getObjectPolicyCacheKey(
+ PhabricatorPolicyInterface $object,
+ PhabricatorPolicyRule $rule) {
+ $hash = spl_object_hash($object);
+ $rule = get_class($rule);
+ return 'policycache.'.$hash.'.'.$rule;
+ }
+
+
/* -( Implementing Object Policies )--------------------------------------- */
diff --git a/src/applications/subscriptions/policyrule/PhabricatorSubscriptionsSubscribersPolicyRule.php b/src/applications/subscriptions/policyrule/PhabricatorSubscriptionsSubscribersPolicyRule.php
new file mode 100644
--- /dev/null
+++ b/src/applications/subscriptions/policyrule/PhabricatorSubscriptionsSubscribersPolicyRule.php
@@ -0,0 +1,121 @@
+<?php
+
+final class PhabricatorSubscriptionsSubscribersPolicyRule
+ extends PhabricatorPolicyRule {
+
+ private $subscribed = array();
+ private $sourcePHIDs = array();
+
+ public function getObjectPolicyKey() {
+ return 'subscriptions.subscribers';
+ }
+
+ public function getObjectPolicyName() {
+ return pht('Subscribers');
+ }
+
+ public function getPolicyExplanation() {
+ return pht('Subscribers can take this action.');
+ }
+
+ public function getRuleDescription() {
+ return pht('subscribers');
+ }
+
+ public function canApplyToObject(PhabricatorPolicyInterface $object) {
+ return ($object instanceof PhabricatorSubscribableInterface);
+ }
+
+ public function willApplyRules(
+ PhabricatorUser $viewer,
+ array $values,
+ array $objects) {
+
+ // We want to let the user see the object if they're a subscriber or
+ // a member of any project which is a subscriber. Additionally, because
+ // subscriber state is complex, we need to read hints passed from
+ // the TransactionEditor to predict policy state after transactions apply.
+
+ $viewer_phid = $viewer->getPHID();
+ if (!$viewer_phid) {
+ return;
+ }
+
+ if (empty($this->subscribed[$viewer_phid])) {
+ $this->subscribed[$viewer_phid] = array();
+ }
+
+ // Load the project PHIDs the user is a member of.
+ if (!isset($this->sourcePHIDs[$viewer_phid])) {
+ $source_phids = PhabricatorEdgeQuery::loadDestinationPHIDs(
+ $viewer_phid,
+ PhabricatorProjectMemberOfProjectEdgeType::EDGECONST);
+ $source_phids[] = $viewer_phid;
+ $this->sourcePHIDs[$viewer_phid] = $source_phids;
+ }
+
+ // Look for transaction hints.
+ foreach ($objects as $key => $object) {
+ $cache = $this->getTransactionHint($object);
+ if ($cache === null) {
+ // We don't have a hint for this object, so we'll deal with it below.
+ continue;
+ }
+
+ // We have a hint, so use that as the source of truth.
+ unset($objects[$key]);
+
+ foreach ($this->sourcePHIDs[$viewer_phid] as $source_phid) {
+ if (isset($cache[$source_phid])) {
+ $this->subscribed[$viewer_phid][$object->getPHID()] = true;
+ break;
+ }
+ }
+ }
+
+ $phids = mpull($objects, 'getPHID');
+ if (!$phids) {
+ return;
+ }
+
+ $edge_query = id(new PhabricatorEdgeQuery())
+ ->withSourcePHIDs($this->sourcePHIDs[$viewer_phid])
+ ->withEdgeTypes(
+ array(
+ PhabricatorSubscribedToObjectEdgeType::EDGECONST,
+ ))
+ ->withDestinationPHIDs($phids);
+
+ $edge_query->execute();
+
+ $subscribed = $edge_query->getDestinationPHIDs();
+ if (!$subscribed) {
+ return;
+ }
+
+ $this->subscribed[$viewer_phid] += array_fill_keys($subscribed, true);
+ }
+
+ public function applyRule(
+ PhabricatorUser $viewer,
+ $value,
+ PhabricatorPolicyInterface $object) {
+
+ $viewer_phid = $viewer->getPHID();
+ if (!$viewer_phid) {
+ return false;
+ }
+
+ if ($object->isAutomaticallySubscribed($viewer_phid)) {
+ return true;
+ }
+
+ $subscribed = idx($this->subscribed, $viewer_phid);
+ return isset($subscribed[$object->getPHID()]);
+ }
+
+ public function getValueControlType() {
+ return self::CONTROL_TYPE_NONE;
+ }
+
+}
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
@@ -2087,6 +2087,19 @@
foreach ($xactions as $xaction) {
switch ($xaction->getTransactionType()) {
+ case PhabricatorTransactions::TYPE_SUBSCRIBERS:
+ $clone_xaction = clone $xaction;
+ $clone_xaction->setOldValue(array_values($this->subscribers));
+ $clone_xaction->setNewValue(
+ $this->getPHIDTransactionNewValue(
+ $clone_xaction));
+
+ PhabricatorPolicyRule::passTransactionHintToRule(
+ $copy,
+ new PhabricatorSubscriptionsSubscribersPolicyRule(),
+ array_fuse($clone_xaction->getNewValue()));
+
+ break;
case PhabricatorTransactions::TYPE_SPACE:
$space_phid = $this->getTransactionNewValue($object, $xaction);
$copy->setSpacePHID($space_phid);

File Metadata

Mime Type
text/plain
Expires
Thu, May 9, 8:13 PM (1 w, 4 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6276495
Default Alt Text
D13259.diff (9 KB)

Event Timeline

Phabricator · Built by Phacility