1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-22 23:02:42 +01:00

Allow unsubscription from projects

Summary:
Fixes T4379. Several changes:

  - Migrate all project members into subscribers.
  - When members are added or removed, subscribe or unsubscribe them.
  - Show sub/unsub in the UI.
  - Determine mailable membership of projects by querying subscribers.

Test Plan:
  - As `duck`, joined a project.
  - Added the project as a reviewer to a revision.
  - Commented on the revision.
  - Observed `duck` receive mail.
  - Unsubscribed as `duck`.
  - Observed no mail.
  - Resubscribed as `duck`.
  - Mail again.
  - Joined/left project, checked sub/unsub status.
  - Ran migration, looked at database.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, asherkin

Maniphest Tasks: T4379

Differential Revision: https://secure.phabricator.com/D8189
This commit is contained in:
epriestley 2014-02-11 07:45:56 -08:00
parent 4e70664ed4
commit b1243e549c
4 changed files with 42 additions and 8 deletions

View file

@ -0,0 +1,8 @@
/* These are here so `grep` will find them if we ever change things: */
/* PhabricatorEdgeConfig::TYPE_PROJ_MEMBER = 13 */
/* PhabricatorEdgeConfig::TYPE_OBJECT_HAS_SUBSCRIBER = 21 */
INSERT IGNORE INTO {$NAMESPACE}_project.edge (src, type, dst, dateCreated)
SELECT src, 21, dst, dateCreated FROM {$NAMESPACE}_project.edge
WHERE type = 13;

View file

@ -37,24 +37,24 @@ final class PhabricatorMetaMTAMemberQuery extends PhabricatorQuery {
foreach ($type_map as $type => $phids) { foreach ($type_map as $type => $phids) {
switch ($type) { switch ($type) {
case PhabricatorProjectPHIDTypeProject::TYPECONST: case PhabricatorProjectPHIDTypeProject::TYPECONST:
// TODO: For now, project members are always on the "mailing list" // NOTE: We're loading the projects here in order to respect policies.
// implied by the project, but we should differentiate members and
// subscribers (i.e., allow you to unsubscribe from mail about
// a project).
$projects = id(new PhabricatorProjectQuery()) $projects = id(new PhabricatorProjectQuery())
->setViewer($this->getViewer()) ->setViewer($this->getViewer())
->needMembers(true)
->withPHIDs($phids) ->withPHIDs($phids)
->execute(); ->execute();
$subscribers = id(new PhabricatorSubscribersQuery())
->withObjectPHIDs($phids)
->execute();
$projects = mpull($projects, null, 'getPHID'); $projects = mpull($projects, null, 'getPHID');
foreach ($phids as $phid) { foreach ($phids as $phid) {
$project = idx($projects, $phid); $project = idx($projects, $phid);
if (!$project) { if (!$project) {
$results[$phid] = array(); $results[$phid] = array();
} else { } else {
$results[$phid] = $project->getMemberPHIDs(); $results[$phid] = idx($subscribers, $phid, array());
} }
} }
break; break;

View file

@ -118,10 +118,36 @@ final class PhabricatorProjectTransactionEditor
case PhabricatorTransactions::TYPE_VIEW_POLICY: case PhabricatorTransactions::TYPE_VIEW_POLICY:
case PhabricatorTransactions::TYPE_EDIT_POLICY: case PhabricatorTransactions::TYPE_EDIT_POLICY:
case PhabricatorTransactions::TYPE_JOIN_POLICY: case PhabricatorTransactions::TYPE_JOIN_POLICY:
case PhabricatorTransactions::TYPE_EDGE:
case PhabricatorProjectTransaction::TYPE_STATUS: case PhabricatorProjectTransaction::TYPE_STATUS:
case PhabricatorProjectTransaction::TYPE_IMAGE: case PhabricatorProjectTransaction::TYPE_IMAGE:
return; return;
case PhabricatorTransactions::TYPE_EDGE:
switch ($xaction->getMetadataValue('edge:type')) {
case PhabricatorEdgeConfig::TYPE_PROJ_MEMBER:
// When project members are added or removed, add or remove their
// subscriptions.
$old = $xaction->getOldValue();
$new = $xaction->getNewValue();
$add = array_keys(array_diff_key($new, $old));
$rem = array_keys(array_diff_key($old, $new));
// NOTE: The subscribe is "explicit" because there's no implicit
// unsubscribe, so Join -> Leave -> Join doesn't resubscribe you
// if we use an implicit subscribe, even though you never willfully
// unsubscribed. Not sure if adding implicit unsubscribe (which
// would not write the unsubscribe row) is justified to deal with
// this, which is a fairly weird edge case and pretty arguable both
// ways.
id(new PhabricatorSubscriptionsEditor())
->setActor($this->requireActor())
->setObject($object)
->subscribeExplicit($add)
->unsubscribe($rem)
->save();
break;
}
return;
} }
return parent::applyCustomExternalTransaction($object, $xaction); return parent::applyCustomExternalTransaction($object, $xaction);

View file

@ -171,7 +171,7 @@ final class PhabricatorProject extends PhabricatorProjectDAO
} }
public function shouldAllowSubscription($phid) { public function shouldAllowSubscription($phid) {
return false; return $this->isUserMember($phid);
} }