Page MenuHomePhabricator
Files F13996689

D8293.id19721.diff
No OneTemporary
Actions

D8293.id19721.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
@@ -464,7 +464,6 @@
'DifferentialRevisionUpdateHistoryView' => 'applications/differential/view/DifferentialRevisionUpdateHistoryView.php',
'DifferentialRevisionViewController' => 'applications/differential/controller/DifferentialRevisionViewController.php',
'DifferentialSearchIndexer' => 'applications/differential/search/DifferentialSearchIndexer.php',
- 'DifferentialSubscribeController' => 'applications/differential/controller/DifferentialSubscribeController.php',
'DifferentialSubscribersField' => 'applications/differential/customfield/DifferentialSubscribersField.php',
'DifferentialSummaryField' => 'applications/differential/customfield/DifferentialSummaryField.php',
'DifferentialSummaryFieldSpecification' => 'applications/differential/field/specification/DifferentialSummaryFieldSpecification.php',
@@ -3014,7 +3013,6 @@
'DifferentialRevisionUpdateHistoryView' => 'AphrontView',
'DifferentialRevisionViewController' => 'DifferentialController',
'DifferentialSearchIndexer' => 'PhabricatorSearchDocumentIndexer',
- 'DifferentialSubscribeController' => 'DifferentialController',
'DifferentialSubscribersField' => 'DifferentialCoreCustomField',
'DifferentialSummaryField' => 'DifferentialCoreCustomField',
'DifferentialSummaryFieldSpecification' => 'DifferentialFreeformFieldSpecification',
diff --git a/src/applications/differential/application/PhabricatorApplicationDifferential.php b/src/applications/differential/application/PhabricatorApplicationDifferential.php
--- a/src/applications/differential/application/PhabricatorApplicationDifferential.php
+++ b/src/applications/differential/application/PhabricatorApplicationDifferential.php
@@ -63,8 +63,6 @@
=> 'DifferentialInlineCommentEditController',
),
),
- 'subscribe/(?P<action>add|rem)/(?P<id>[1-9]\d*)/'
- => 'DifferentialSubscribeController',
'preview/' => 'PhabricatorMarkupPreviewController',
),
);
diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php
--- a/src/applications/differential/controller/DifferentialRevisionViewController.php
+++ b/src/applications/differential/controller/DifferentialRevisionViewController.php
@@ -459,23 +459,17 @@
}
private function getRevisionActions(DifferentialRevision $revision) {
- $user = $this->getRequest()->getUser();
- $viewer_phid = $user->getPHID();
- $viewer_is_owner = ($revision->getAuthorPHID() == $viewer_phid);
- $viewer_is_reviewer = in_array($viewer_phid, $revision->getReviewers());
- $viewer_is_cc = in_array($viewer_phid, $revision->getCCPHIDs());
- $logged_in = $this->getRequest()->getUser()->isLoggedIn();
- $status = $revision->getStatus();
+ $viewer = $this->getRequest()->getUser();
$revision_id = $revision->getID();
$revision_phid = $revision->getPHID();
- $links = array();
-
$can_edit = PhabricatorPolicyFilter::hasCapability(
- $user,
+ $viewer,
$revision,
PhabricatorPolicyCapability::CAN_EDIT);
+ $links = array();
+
$links[] = array(
'icon' => 'edit',
'href' => "/differential/revision/edit/{$revision_id}/",
@@ -484,24 +478,6 @@
'sigil' => $can_edit ? null : 'workflow',
);
- if (!$viewer_is_owner && !$viewer_is_reviewer) {
- $action = $viewer_is_cc ? 'rem' : 'add';
- $links[] = array(
- 'icon' => $viewer_is_cc ? 'disable' : 'check',
- 'href' => "/differential/subscribe/{$action}/{$revision_id}/",
- 'name' => $viewer_is_cc ? pht('Unsubscribe') : pht('Subscribe'),
- 'instant' => $logged_in,
- 'disabled' => !$logged_in,
- 'sigil' => $can_edit ? null : 'workflow',
- );
- } else {
- $links[] = array(
- 'icon' => 'enable',
- 'name' => pht('Automatically Subscribed'),
- 'disabled' => true,
- );
- }
-
$this->requireResource('phabricator-object-selector-css');
$this->requireResource('javelin-behavior-phabricator-object-selector');
diff --git a/src/applications/differential/controller/DifferentialSubscribeController.php b/src/applications/differential/controller/DifferentialSubscribeController.php
deleted file mode 100644
--- a/src/applications/differential/controller/DifferentialSubscribeController.php
+++ /dev/null
@@ -1,80 +0,0 @@
-<?php
-
-final class DifferentialSubscribeController extends DifferentialController {
-
- private $id;
- private $action;
-
- public function willProcessRequest(array $data) {
- $this->id = $data['id'];
- $this->action = $data['action'];
- }
-
- public function processRequest() {
-
- $request = $this->getRequest();
- $user = $request->getUser();
-
- $revision = id(new DifferentialRevisionQuery())
- ->withIDs(array($this->id))
- ->setViewer($request->getUser())
- ->needRelationships(true)
- ->needReviewerStatus(true)
- ->executeOne();
- if (!$revision) {
- return new Aphront404Response();
- }
-
- if (!$request->isFormPost()) {
- $dialog = new AphrontDialogView();
-
- switch ($this->action) {
- case 'add':
- $button = pht('Subscribe');
- $title = pht('Subscribe to Revision');
- $prompt = pht('Really subscribe to this revision?');
- break;
- case 'rem':
- $button = pht('Unsubscribe');
- $title = pht('Unsubscribe from Revision');
- $prompt = pht('Really unsubscribe from this revision? Herald will '.
- 'not resubscribe you to a revision you unsubscribe '.
- 'from.');
- break;
- default:
- return new Aphront400Response();
- }
-
- $dialog
- ->setUser($user)
- ->setTitle($title)
- ->appendChild(phutil_tag('p', array(), $prompt))
- ->setSubmitURI($request->getRequestURI())
- ->addSubmitButton($button)
- ->addCancelButton('/D'.$revision->getID());
-
- return id(new AphrontDialogResponse())->setDialog($dialog);
- }
-
- $phid = $user->getPHID();
-
- switch ($this->action) {
- case 'add':
- DifferentialRevisionEditor::addCCAndUpdateRevision(
- $revision,
- $phid,
- $user);
- break;
- case 'rem':
- DifferentialRevisionEditor::removeCCAndUpdateRevision(
- $revision,
- $phid,
- $user);
- break;
- default:
- return new Aphront400Response();
- }
-
- return id(new AphrontRedirectResponse())->setURI('/D'.$revision->getID());
- }
-}
diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php
--- a/src/applications/differential/storage/DifferentialRevision.php
+++ b/src/applications/differential/storage/DifferentialRevision.php
@@ -444,20 +444,41 @@
public function isAutomaticallySubscribed($phid) {
- // TODO: Reviewers are also automatically subscribed, but that data may
- // not always be loaded, so be conservative for now. All the editing code
- // has checks around this already.
- return ($phid == $this->getAuthorPHID());
+ if ($phid == $this->getAuthorPHID()) {
+ return true;
+ }
+
+ // TODO: This only happens when adding or removing CCs, and is safe from a
+ // policy perspective, but the subscription pathway should have some
+ // opportunity to load this data properly. For now, this is the only case
+ // where implicit subscription is not an intrinsic property of the object.
+ if ($this->reviewerStatus == self::ATTACHABLE) {
+ $reviewers = id(new DifferentialRevisionQuery())
+ ->setViewer(PhabricatorUser::getOmnipotentUser())
+ ->withPHIDs(array($this->getPHID()))
+ ->needReviewerStatus(true)
+ ->executeOne()
+ ->getReviewerStatus();
+ } else {
+ $reviewers = $this->getReviewerStatus();
+ }
+
+ foreach ($reviewers as $reviewer) {
+ if ($reviewer->getReviewerPHID() == $phid) {
+ return true;
+ }
+ }
+
+ return false;
}
public function shouldShowSubscribersProperty() {
- // TODO: For now, Differential has its own stuff.
+ // TODO: Differential does its own thing for now.
return false;
}
public function shouldAllowSubscription($phid) {
- // TODO: For now, Differential has its own stuff.
- return false;
+ return true;
}

File Metadata

Mime Type
text/plain
Expires
Thu, Oct 24, 11:12 PM (3 w, 4 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6747424
Default Alt Text
D8293.id19721.diff (8 KB)

Event Timeline

Phabricator · Built by Phacility