Make Differential views capability-sensitive
Summary: Ref T603. Make Differential behaviors for logged-out and underprivleged users more similar to other apps. I'm going to drop this "anonymous access" thing at some point, but `reviews.fb.net` actually looks like it's running semi-modern code, so leave it alive until we have a more compelling replacement in the upstream. Test Plan: As a logged out user, browsed Differential and clicked things and such. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T603 Differential Revision: https://secure.phabricator.com/D7148
This commit is contained in:
@@ -3,7 +3,15 @@
|
|||||||
final class DifferentialChangesetViewController extends DifferentialController {
|
final class DifferentialChangesetViewController extends DifferentialController {
|
||||||
|
|
||||||
public function shouldRequireLogin() {
|
public function shouldRequireLogin() {
|
||||||
return !$this->allowsAnonymousAccess();
|
if ($this->allowsAnonymousAccess()) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
return parent::shouldRequireLogin();
|
||||||
|
}
|
||||||
|
|
||||||
|
public function shouldAllowPublic() {
|
||||||
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
public function processRequest() {
|
public function processRequest() {
|
||||||
@@ -28,6 +36,17 @@ final class DifferentialChangesetViewController extends DifferentialController {
|
|||||||
return new Aphront404Response();
|
return new Aphront404Response();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TODO: (T603) Make Changeset policy-aware. For now, just fake it
|
||||||
|
// by making sure we can see the diff.
|
||||||
|
$diff = id(new DifferentialDiffQuery())
|
||||||
|
->setViewer($request->getUser())
|
||||||
|
->withIDs(array($changeset->getDiffID()))
|
||||||
|
->executeOne();
|
||||||
|
if (!$diff) {
|
||||||
|
return new Aphront404Response();
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
$view = $request->getStr('view');
|
$view = $request->getStr('view');
|
||||||
if ($view) {
|
if ($view) {
|
||||||
$changeset->attachHunks($changeset->loadHunks());
|
$changeset->attachHunks($changeset->loadHunks());
|
||||||
|
|||||||
@@ -6,7 +6,10 @@ final class DifferentialRevisionListController extends DifferentialController
|
|||||||
private $queryKey;
|
private $queryKey;
|
||||||
|
|
||||||
public function shouldRequireLogin() {
|
public function shouldRequireLogin() {
|
||||||
return !$this->allowsAnonymousAccess();
|
if ($this->allowsAnonymousAccess()) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
return parent::shouldRequireLogin();
|
||||||
}
|
}
|
||||||
|
|
||||||
public function shouldAllowPublic() {
|
public function shouldAllowPublic() {
|
||||||
|
|||||||
@@ -5,7 +5,14 @@ final class DifferentialRevisionViewController extends DifferentialController {
|
|||||||
private $revisionID;
|
private $revisionID;
|
||||||
|
|
||||||
public function shouldRequireLogin() {
|
public function shouldRequireLogin() {
|
||||||
return !$this->allowsAnonymousAccess();
|
if ($this->allowsAnonymousAccess()) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
return parent::shouldRequireLogin();
|
||||||
|
}
|
||||||
|
|
||||||
|
public function shouldAllowPublic() {
|
||||||
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
public function willProcessRequest(array $data) {
|
public function willProcessRequest(array $data) {
|
||||||
@@ -405,8 +412,15 @@ final class DifferentialRevisionViewController extends DifferentialController {
|
|||||||
));
|
));
|
||||||
if ($comment_form) {
|
if ($comment_form) {
|
||||||
$page_pane->appendChild($comment_form->render());
|
$page_pane->appendChild($comment_form->render());
|
||||||
|
} else {
|
||||||
|
// TODO: For now, just use this to get "Login to Comment".
|
||||||
|
$page_pane->appendChild(
|
||||||
|
id(new PhabricatorApplicationTransactionCommentView())
|
||||||
|
->setUser($user)
|
||||||
|
->setRequestURI($request->getRequestURI()));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
$object_id = 'D'.$revision->getID();
|
$object_id = 'D'.$revision->getID();
|
||||||
|
|
||||||
$top_anchor = id(new PhabricatorAnchorView())
|
$top_anchor = id(new PhabricatorAnchorView())
|
||||||
@@ -493,22 +507,25 @@ final class DifferentialRevisionViewController extends DifferentialController {
|
|||||||
$viewer_is_owner = ($revision->getAuthorPHID() == $viewer_phid);
|
$viewer_is_owner = ($revision->getAuthorPHID() == $viewer_phid);
|
||||||
$viewer_is_reviewer = in_array($viewer_phid, $revision->getReviewers());
|
$viewer_is_reviewer = in_array($viewer_phid, $revision->getReviewers());
|
||||||
$viewer_is_cc = in_array($viewer_phid, $revision->getCCPHIDs());
|
$viewer_is_cc = in_array($viewer_phid, $revision->getCCPHIDs());
|
||||||
$viewer_is_anonymous = !$this->getRequest()->getUser()->isLoggedIn();
|
$logged_in = $this->getRequest()->getUser()->isLoggedIn();
|
||||||
$status = $revision->getStatus();
|
$status = $revision->getStatus();
|
||||||
$revision_id = $revision->getID();
|
$revision_id = $revision->getID();
|
||||||
$revision_phid = $revision->getPHID();
|
$revision_phid = $revision->getPHID();
|
||||||
|
|
||||||
$links = array();
|
$links = array();
|
||||||
|
|
||||||
if ($viewer_is_owner) {
|
$can_edit = PhabricatorPolicyFilter::hasCapability(
|
||||||
|
$user,
|
||||||
|
$revision,
|
||||||
|
PhabricatorPolicyCapability::CAN_EDIT);
|
||||||
|
|
||||||
$links[] = array(
|
$links[] = array(
|
||||||
'icon' => 'edit',
|
'icon' => 'edit',
|
||||||
'href' => "/differential/revision/edit/{$revision_id}/",
|
'href' => "/differential/revision/edit/{$revision_id}/",
|
||||||
'name' => pht('Edit Revision'),
|
'name' => pht('Edit Revision'),
|
||||||
|
'disabled' => !$can_edit,
|
||||||
|
'sigil' => $can_edit ? null : 'workflow',
|
||||||
);
|
);
|
||||||
}
|
|
||||||
|
|
||||||
if (!$viewer_is_anonymous) {
|
|
||||||
|
|
||||||
if (!$viewer_is_owner && !$viewer_is_reviewer) {
|
if (!$viewer_is_owner && !$viewer_is_reviewer) {
|
||||||
$action = $viewer_is_cc ? 'rem' : 'add';
|
$action = $viewer_is_cc ? 'rem' : 'add';
|
||||||
@@ -516,7 +533,9 @@ final class DifferentialRevisionViewController extends DifferentialController {
|
|||||||
'icon' => $viewer_is_cc ? 'disable' : 'check',
|
'icon' => $viewer_is_cc ? 'disable' : 'check',
|
||||||
'href' => "/differential/subscribe/{$action}/{$revision_id}/",
|
'href' => "/differential/subscribe/{$action}/{$revision_id}/",
|
||||||
'name' => $viewer_is_cc ? pht('Unsubscribe') : pht('Subscribe'),
|
'name' => $viewer_is_cc ? pht('Unsubscribe') : pht('Subscribe'),
|
||||||
'instant' => true,
|
'instant' => $logged_in,
|
||||||
|
'disabled' => !$logged_in,
|
||||||
|
'sigil' => $can_edit ? null : 'workflow',
|
||||||
);
|
);
|
||||||
} else {
|
} else {
|
||||||
$links[] = array(
|
$links[] = array(
|
||||||
@@ -534,6 +553,7 @@ final class DifferentialRevisionViewController extends DifferentialController {
|
|||||||
'name' => pht('Edit Dependencies'),
|
'name' => pht('Edit Dependencies'),
|
||||||
'href' => "/search/attach/{$revision_phid}/DREV/dependencies/",
|
'href' => "/search/attach/{$revision_phid}/DREV/dependencies/",
|
||||||
'sigil' => 'workflow',
|
'sigil' => 'workflow',
|
||||||
|
'disabled' => !$can_edit,
|
||||||
);
|
);
|
||||||
|
|
||||||
$maniphest = 'PhabricatorApplicationManiphest';
|
$maniphest = 'PhabricatorApplicationManiphest';
|
||||||
@@ -543,9 +563,9 @@ final class DifferentialRevisionViewController extends DifferentialController {
|
|||||||
'name' => pht('Edit Maniphest Tasks'),
|
'name' => pht('Edit Maniphest Tasks'),
|
||||||
'href' => "/search/attach/{$revision_phid}/TASK/",
|
'href' => "/search/attach/{$revision_phid}/TASK/",
|
||||||
'sigil' => 'workflow',
|
'sigil' => 'workflow',
|
||||||
|
'disabled' => !$can_edit,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
||||||
$request_uri = $this->getRequest()->getRequestURI();
|
$request_uri = $this->getRequest()->getRequestURI();
|
||||||
$links[] = array(
|
$links[] = array(
|
||||||
|
|||||||
@@ -44,6 +44,32 @@ final class DifferentialDiffQuery
|
|||||||
}
|
}
|
||||||
|
|
||||||
public function willFilterPage(array $diffs) {
|
public function willFilterPage(array $diffs) {
|
||||||
|
$revision_ids = array_filter(mpull($diffs, 'getRevisionID'));
|
||||||
|
|
||||||
|
$revisions = array();
|
||||||
|
if ($revision_ids) {
|
||||||
|
$revisions = id(new DifferentialRevisionQuery())
|
||||||
|
->setViewer($this->getViewer())
|
||||||
|
->withIDs($revision_ids)
|
||||||
|
->execute();
|
||||||
|
}
|
||||||
|
|
||||||
|
foreach ($diffs as $key => $diff) {
|
||||||
|
if (!$diff->getRevisionID()) {
|
||||||
|
$diff->attachRevision(null);
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
$revision = idx($revisions, $diff->getRevisionID());
|
||||||
|
if ($revision) {
|
||||||
|
$diff->attachRevision($revision);
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
unset($diffs[$key]);
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
if ($this->needChangesets) {
|
if ($this->needChangesets) {
|
||||||
$this->loadChangesets($diffs);
|
$this->loadChangesets($diffs);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -32,6 +32,7 @@ final class DifferentialDiff
|
|||||||
private $unsavedChangesets = array();
|
private $unsavedChangesets = array();
|
||||||
private $changesets = self::ATTACHABLE;
|
private $changesets = self::ATTACHABLE;
|
||||||
private $arcanistProject = self::ATTACHABLE;
|
private $arcanistProject = self::ATTACHABLE;
|
||||||
|
private $revision = self::ATTACHABLE;
|
||||||
|
|
||||||
public function addUnsavedChangeset(DifferentialChangeset $changeset) {
|
public function addUnsavedChangeset(DifferentialChangeset $changeset) {
|
||||||
if ($this->changesets === null) {
|
if ($this->changesets === null) {
|
||||||
@@ -284,6 +285,15 @@ final class DifferentialDiff
|
|||||||
return $changes;
|
return $changes;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function getRevision() {
|
||||||
|
return $this->assertAttached($this->revision);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function attachRevision(DifferentialRevision $revision = null) {
|
||||||
|
$this->revision = $revision;
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
/* -( PhabricatorPolicyInterface )----------------------------------------- */
|
/* -( PhabricatorPolicyInterface )----------------------------------------- */
|
||||||
|
|
||||||
@@ -295,10 +305,18 @@ final class DifferentialDiff
|
|||||||
}
|
}
|
||||||
|
|
||||||
public function getPolicy($capability) {
|
public function getPolicy($capability) {
|
||||||
|
if ($this->getRevision()) {
|
||||||
|
return $this->getRevision()->getPolicy($capability);
|
||||||
|
}
|
||||||
|
|
||||||
return PhabricatorPolicies::POLICY_USER;
|
return PhabricatorPolicies::POLICY_USER;
|
||||||
}
|
}
|
||||||
|
|
||||||
public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
|
public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
|
||||||
|
if ($this->getRevision()) {
|
||||||
|
return $this->getRevision()->hasAutomaticCapability($capability, $viewer);
|
||||||
|
}
|
||||||
|
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -3,6 +3,10 @@
|
|||||||
final class PhabricatorHelpKeyboardShortcutController
|
final class PhabricatorHelpKeyboardShortcutController
|
||||||
extends PhabricatorHelpController {
|
extends PhabricatorHelpController {
|
||||||
|
|
||||||
|
public function shouldAllowPublic() {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
public function processRequest() {
|
public function processRequest() {
|
||||||
$request = $this->getRequest();
|
$request = $this->getRequest();
|
||||||
$user = $request->getUser();
|
$user = $request->getUser();
|
||||||
|
|||||||
Reference in New Issue
Block a user