1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-20 13:52:40 +01:00

Make Projects a PhabricatorSubscribableInterface, but with restricted defaults

Summary:
Ref T4379. I want project subscriptions to work like this (yell if this seems whacky, since it makes subscriptions mean somethign a little different for projects than they do for other objects):

  - You can only subscribe to a project if you're a project member.
  - When you're added as a member, you're added as a subscriber.
  - When you're removed as a member, you're removed as a subscriber.
  - While you're a member, you can optionally unsubscribe.

From a UI perspective:

  - We don't show the subscriber list, since it's going to be some uninteresting subset of the member list.
  - We don't show CC transactions in history, since they're an uninteresting near-approximation of the membership transactions.
  - You only see the subscription controls if you're a member.

To do this, I've augmented `PhabricatorSubscribableInterface` with two new methods. It would be nice if we were on PHP 5.4+ and could just use traits for this, but we should get data about version usage before we think about this. For now, copy/paste the default implementations into every implementing class.

Then, I implemented the interface in `PhabricatorProject` but with alternate defaults.

Test Plan:
  - Used the normal interaction on existing objects.
  - This has no actual effect on projects, verified no subscription stuff mysteriously appeared.
  - Hit the new error case by fiddling with the UI.

Reviewers: btrahan

Reviewed By: btrahan

CC: chad, aran

Maniphest Tasks: T4379

Differential Revision: https://secure.phabricator.com/D8165
This commit is contained in:
epriestley 2014-02-10 14:29:17 -08:00
parent 3e946140d3
commit 21de2b1a0c
16 changed files with 191 additions and 15 deletions

View file

@ -4555,6 +4555,7 @@ phutil_register_library_map(array(
0 => 'PhabricatorProjectDAO',
1 => 'PhabricatorFlaggableInterface',
2 => 'PhabricatorPolicyInterface',
3 => 'PhabricatorSubscribableInterface',
),
'PhabricatorProjectBoardController' => 'PhabricatorProjectController',
'PhabricatorProjectBoardEditController' => 'PhabricatorProjectController',

View file

@ -933,6 +933,14 @@ final class PhabricatorFile extends PhabricatorFileDAO
return ($this->authorPHID == $phid);
}
public function shouldShowSubscribersProperty() {
return true;
}
public function shouldAllowSubscription($phid) {
return true;
}
/* -( PhabricatorTokenReceiverInterface )---------------------------------- */

View file

@ -64,6 +64,14 @@ final class HarbormasterBuildPlan extends HarbormasterDAO
return false;
}
public function shouldShowSubscribersProperty() {
return true;
}
public function shouldAllowSubscription($phid) {
return true;
}
/* -( PhabricatorPolicyInterface )----------------------------------------- */

View file

@ -91,13 +91,25 @@ final class LegalpadDocument extends LegalpadDAO
return 'L'.$this->getID();
}
/* -( PhabricatorSubscribableInterface Implementation )-------------------- */
/* -( PhabricatorSubscribableInterface )----------------------------------- */
public function isAutomaticallySubscribed($phid) {
return ($this->creatorPHID == $phid);
}
/* -( PhabricatorPolicyInterface Implementation )-------------------------- */
public function shouldShowSubscribersProperty() {
return true;
}
public function shouldAllowSubscription($phid) {
return true;
}
/* -( PhabricatorPolicyInterface )----------------------------------------- */
public function getCapabilities() {
return array(

View file

@ -79,6 +79,14 @@ final class PhabricatorFileImageMacro extends PhabricatorFileDAO
return false;
}
public function shouldShowSubscribersProperty() {
return true;
}
public function shouldAllowSubscription($phid) {
return true;
}
/* -( PhabricatorPolicyInterface )----------------------------------------- */

View file

@ -87,6 +87,14 @@ final class PhabricatorPaste extends PhabricatorPasteDAO
return ($this->authorPHID == $phid);
}
public function shouldShowSubscribersProperty() {
return true;
}
public function shouldAllowSubscription($phid) {
return true;
}
/* -( PhabricatorTokenReceiverInterface )---------------------------------- */

View file

@ -137,6 +137,14 @@ final class PholioMock extends PholioDAO
return ($this->authorPHID == $phid);
}
public function shouldShowSubscribersProperty() {
return true;
}
public function shouldAllowSubscription($phid) {
return true;
}
/* -( PhabricatorPolicyInterface Implementation )-------------------------- */

View file

@ -104,6 +104,10 @@ final class PhrictionDocument extends PhrictionDAO
return $parts[1].'/';
}
/* -( PhabricatorPolicyInterface )----------------------------------------- */
public function getCapabilities() {
return array(
PhabricatorPolicyCapability::CAN_VIEW,
@ -133,12 +137,26 @@ final class PhrictionDocument extends PhrictionDAO
return null;
}
/* -( PhabricatorSubscribableInterface )----------------------------------- */
public function isAutomaticallySubscribed($phid) {
return false;
}
public function shouldShowSubscribersProperty() {
return true;
}
public function shouldAllowSubscription($phid) {
return true;
}
/* -( PhabricatorTokenReceiverInterface )---------------------------------- */
public function getUsersToNotifyOfTokenGiven() {
return PhabricatorSubscribersQuery::loadSubscribersForPHID($this->phid);
}

View file

@ -188,5 +188,12 @@ final class PonderAnswer extends PonderDAO
return ($phid == $this->getAuthorPHID());
}
public function shouldShowSubscribersProperty() {
return true;
}
public function shouldAllowSubscription($phid) {
return true;
}
}

View file

@ -170,10 +170,6 @@ final class PonderQuestion extends PonderDAO
return $this->getPHID();
}
public function isAutomaticallySubscribed($phid) {
return ($phid == $this->getAuthorPHID());
}
public function save() {
if (!$this->getMailKey()) {
$this->setMailKey(Filesystem::readRandomCharacters(20));
@ -181,6 +177,20 @@ final class PonderQuestion extends PonderDAO
return parent::save();
}
public function getOriginalTitle() {
// TODO: Make this actually save/return the original title.
return $this->getTitle();
}
public function getFullTitle() {
$id = $this->getID();
$title = $this->getTitle();
return "Q{$id}: {$title}";
}
/* -( PhabricatorPolicyInterface )----------------------------------------- */
public function getCapabilities() {
return array(
PhabricatorPolicyCapability::CAN_VIEW,
@ -210,15 +220,20 @@ final class PonderQuestion extends PonderDAO
'The user who asked a question can always view and edit it.');
}
public function getOriginalTitle() {
// TODO: Make this actually save/return the original title.
return $this->getTitle();
/* -( PhabricatorSubscribableInterface )----------------------------------- */
public function isAutomaticallySubscribed($phid) {
return ($phid == $this->getAuthorPHID());
}
public function getFullTitle() {
$id = $this->getID();
$title = $this->getTitle();
return "Q{$id}: {$title}";
public function shouldShowSubscribersProperty() {
return true;
}
public function shouldAllowSubscription($phid) {
return true;
}

View file

@ -214,6 +214,7 @@ final class PhabricatorProjectProfileController
}
$view->addAction($action);
$view->addAction(
id(new PhabricatorActionView())
->setName(pht('View History'))

View file

@ -3,7 +3,8 @@
final class PhabricatorProject extends PhabricatorProjectDAO
implements
PhabricatorFlaggableInterface,
PhabricatorPolicyInterface {
PhabricatorPolicyInterface,
PhabricatorSubscribableInterface {
protected $name;
protected $status = PhabricatorProjectStatus::STATUS_ACTIVE;
@ -15,7 +16,6 @@ final class PhabricatorProject extends PhabricatorProjectDAO
protected $editPolicy;
protected $joinPolicy;
private $subprojectsNeedUpdate;
private $memberPHIDs = self::ATTACHABLE;
private $sparseMembers = self::ATTACHABLE;
private $profile = self::ATTACHABLE;
@ -134,4 +134,21 @@ final class PhabricatorProject extends PhabricatorProjectDAO
return 'projects/'.$slug;
}
/* -( PhabricatorSubscribableInterface )----------------------------------- */
public function isAutomaticallySubscribed($phid) {
return false;
}
public function shouldShowSubscribersProperty() {
return false;
}
public function shouldAllowSubscription($phid) {
return false;
}
}

View file

@ -125,6 +125,14 @@ final class PhabricatorSlowvotePoll extends PhabricatorSlowvoteDAO
return ($phid == $this->getAuthorPHID());
}
public function shouldShowSubscribersProperty() {
return true;
}
public function shouldAllowSubscription($phid) {
return true;
}
/* -( PhabricatorTokenReceiverInterface )---------------------------------- */

View file

@ -56,6 +56,13 @@ final class PhabricatorSubscriptionsEditController
$handle->getURI());
}
if (!$object->shouldAllowSubscription($user->getPHID())) {
return $this->buildErrorResponse(
pht('You Can Not Subscribe'),
pht('You can not subscribe to this object.'),
$handle->getURI());
}
if ($object instanceof PhabricatorApplicationTransactionInterface) {
if ($is_add) {
$xaction_value = array(

View file

@ -33,6 +33,11 @@ final class PhabricatorSubscriptionsUIEventListener
return;
}
if (!$object->shouldAllowSubscription($user->getPHID())) {
// This object doesn't allow the viewer to subscribe.
return;
}
if ($object->isAutomaticallySubscribed($user->getPHID())) {
$sub_action = id(new PhabricatorActionView())
->setWorkflow(true)
@ -95,6 +100,12 @@ final class PhabricatorSubscriptionsUIEventListener
// This object isn't subscribable.
return;
}
if (!$object->shouldShowSubscribersProperty()) {
// This object doesn't render subscribers in its property list.
return;
}
$subscribers = PhabricatorSubscribersQuery::loadSubscribersForPHID(
$object->getPHID());
if ($subscribers) {

View file

@ -13,4 +13,43 @@ interface PhabricatorSubscribableInterface {
*/
public function isAutomaticallySubscribed($phid);
/**
* Return `true` to indicate that "Subscribers:" should be shown when
* rendering property lists for this object, or `false` to omit the property.
*
* @return bool True to show the "Subscribers:" property.
*/
public function shouldShowSubscribersProperty();
/**
* Return `true` to indicate that the "Subscribe" action should be shown and
* enabled when rendering action lists for this object, or `false` to omit
* the action.
*
* @param phid Viewing or acting user PHID.
* @return bool True to allow the user to subscribe.
*/
public function shouldAllowSubscription($phid);
}
// TEMPLATE IMPLEMENTATION /////////////////////////////////////////////////////
/* -( PhabricatorSubscribableInterface )----------------------------------- */
/*
public function isAutomaticallySubscribed($phid) {
return false;
}
public function shouldShowSubscribersProperty() {
return true;
}
public function shouldAllowSubscription($phid) {
return true;
}
*/