Page MenuHomePhabricator
Files F13185646

D8202.diff
No OneTemporary
Actions

D8202.diff
View Options

Index: resources/sql/autopatches/20140211.dx.3.migsubscriptions.sql
===================================================================
--- /dev/null
+++ resources/sql/autopatches/20140211.dx.3.migsubscriptions.sql
@@ -0,0 +1,10 @@
+/* For `grep`: */
+
+/* PhabricatorEdgeConfig::TYPE_OBJECT_HAS_SUBSCRIBER = 21 */
+
+INSERT IGNORE INTO {$NAMESPACE}_differential.edge (src, type, dst, seq)
+ SELECT rev.phid, 21, rel.objectPHID, rel.sequence
+ FROM {$NAMESPACE}_differential.differential_revision rev
+ JOIN {$NAMESPACE}_differential.differential_relationship rel
+ ON rev.id = rel.revisionID
+ WHERE relation = 'subd';
Index: src/__phutil_library_map__.php
===================================================================
--- src/__phutil_library_map__.php
+++ src/__phutil_library_map__.php
@@ -2953,6 +2953,7 @@
3 => 'PhabricatorFlaggableInterface',
4 => 'PhrequentTrackableInterface',
5 => 'HarbormasterBuildableInterface',
+ 6 => 'PhabricatorSubscribableInterface',
),
'DifferentialRevisionCommentListView' => 'AphrontView',
'DifferentialRevisionCommentView' => 'AphrontView',
Index: src/applications/differential/conduit/ConduitAPI_differential_query_Method.php
===================================================================
--- src/applications/differential/conduit/ConduitAPI_differential_query_Method.php
+++ src/applications/differential/conduit/ConduitAPI_differential_query_Method.php
@@ -164,7 +164,7 @@
$query->withResponsibleUsers($responsible_users);
}
if ($subscribers) {
- $query->withSubscribers($subscribers);
+ $query->withCCs($subscribers);
}
if ($branches) {
$query->withBranches($branches);
Index: src/applications/differential/editor/DifferentialRevisionEditor.php
===================================================================
--- src/applications/differential/editor/DifferentialRevisionEditor.php
+++ src/applications/differential/editor/DifferentialRevisionEditor.php
@@ -250,12 +250,16 @@
$rem_ccs = array();
$xscript_phid = null;
if ($diff) {
+ $unsubscribed_phids = PhabricatorEdgeQuery::loadDestinationPHIDs(
+ $revision->getPHID(),
+ PhabricatorEdgeConfig::TYPE_OBJECT_HAS_UNSUBSCRIBER);
+
$adapter = HeraldDifferentialRevisionAdapter::newLegacyAdapter(
$revision,
$diff);
$adapter->setExplicitCCs($new['ccs']);
$adapter->setExplicitReviewers($new['rev']);
- $adapter->setForbiddenCCs($revision->loadUnsubscribedPHIDs());
+ $adapter->setForbiddenCCs($unsubscribed_phids);
$adapter->setIsNewObject($is_new);
$xscript = HeraldEngine::loadAndApplyRules($adapter);
@@ -597,13 +601,12 @@
$dont_add = self::getImpliedCCs($revision);
$add_phids = array_diff($add_phids, $dont_add);
- return self::alterRelationships(
- $revision,
- $stable_phids,
- $rem_phids,
- $add_phids,
- $reason_phid,
- DifferentialRevision::RELATION_SUBSCRIBED);
+ id(new PhabricatorSubscriptionsEditor())
+ ->setActor(PhabricatorUser::getOmnipotentUser())
+ ->setObject($revision)
+ ->subscribeExplicit($add_phids)
+ ->unsubscribe($rem_phids)
+ ->save();
}
private static function getImpliedCCs(DifferentialRevision $revision) {
@@ -694,97 +697,6 @@
->save();
}
- private static function alterRelationships(
- DifferentialRevision $revision,
- array $stable_phids,
- array $rem_phids,
- array $add_phids,
- $reason_phid,
- $relation_type) {
-
- $rem_map = array_fill_keys($rem_phids, true);
- $add_map = array_fill_keys($add_phids, true);
-
- $seq_map = array_values($stable_phids);
- $seq_map = array_flip($seq_map);
- foreach ($rem_map as $phid => $ignored) {
- if (!isset($seq_map[$phid])) {
- $seq_map[$phid] = count($seq_map);
- }
- }
- foreach ($add_map as $phid => $ignored) {
- if (!isset($seq_map[$phid])) {
- $seq_map[$phid] = count($seq_map);
- }
- }
-
- $raw = $revision->getRawRelations($relation_type);
- $raw = ipull($raw, null, 'objectPHID');
-
- $sequence = count($seq_map);
- foreach ($raw as $phid => $ignored) {
- if (isset($seq_map[$phid])) {
- $raw[$phid]['sequence'] = $seq_map[$phid];
- } else {
- $raw[$phid]['sequence'] = $sequence++;
- }
- }
- $raw = isort($raw, 'sequence');
-
- foreach ($raw as $phid => $ignored) {
- if (isset($rem_map[$phid])) {
- unset($raw[$phid]);
- }
- }
-
- foreach ($add_phids as $add) {
- $reason = is_array($reason_phid)
- ? idx($reason_phid, $add)
- : $reason_phid;
-
- $raw[$add] = array(
- 'objectPHID' => $add,
- 'sequence' => idx($seq_map, $add, $sequence++),
- 'reasonPHID' => $reason,
- );
- }
-
- $conn_w = $revision->establishConnection('w');
-
- $sql = array();
- foreach ($raw as $relation) {
- $sql[] = qsprintf(
- $conn_w,
- '(%d, %s, %s, %d, %s)',
- $revision->getID(),
- $relation_type,
- $relation['objectPHID'],
- $relation['sequence'],
- $relation['reasonPHID']);
- }
-
- $conn_w->openTransaction();
- queryfx(
- $conn_w,
- 'DELETE FROM %T WHERE revisionID = %d AND relation = %s',
- DifferentialRevision::RELATIONSHIP_TABLE,
- $revision->getID(),
- $relation_type);
- if ($sql) {
- queryfx(
- $conn_w,
- 'INSERT INTO %T
- (revisionID, relation, objectPHID, sequence, reasonPHID)
- VALUES %Q',
- DifferentialRevision::RELATIONSHIP_TABLE,
- implode(', ', $sql));
- }
- $conn_w->saveTransaction();
-
- $revision->loadRelationships();
- }
-
-
private function createComment() {
$template = id(new DifferentialComment())
->setAuthorPHID($this->getActorPHID())
Index: src/applications/differential/query/DifferentialRevisionQuery.php
===================================================================
--- src/applications/differential/query/DifferentialRevisionQuery.php
+++ src/applications/differential/query/DifferentialRevisionQuery.php
@@ -33,7 +33,6 @@
private $revIDs = array();
private $commitHashes = array();
private $phids = array();
- private $subscribers = array();
private $responsibles = array();
private $branches = array();
private $arcanistProjectPHIDs = array();
@@ -114,7 +113,7 @@
* Filter results to revisions which CC one of the listed people. Calling this
* function will clear anything set by previous calls to @{method:withCCs}.
*
- * @param array List of PHIDs of subscribers
+ * @param array List of PHIDs of subscribers.
* @return this
* @task config
*/
@@ -221,20 +220,6 @@
/**
- * Filter results to only return revisions with a given set of subscribers
- * (i.e., they are authors, reviewers or CC'd).
- *
- * @param array List of user PHIDs.
- * @return this
- * @task config
- */
- public function withSubscribers(array $subscriber_phids) {
- $this->subscribers = $subscriber_phids;
- return $this;
- }
-
-
- /**
* Filter results to only return revisions with a given set of arcanist
* projects.
*
@@ -628,11 +613,11 @@
if ($this->ccs) {
$joins[] = qsprintf(
$conn_r,
- 'JOIN %T cc_rel ON cc_rel.revisionID = r.id '.
- 'AND cc_rel.relation = %s '.
- 'AND cc_rel.objectPHID in (%Ls)',
- DifferentialRevision::RELATIONSHIP_TABLE,
- DifferentialRevision::RELATION_SUBSCRIBED,
+ 'JOIN %T e_ccs ON e_ccs.src = r.phid '.
+ 'AND e_ccs.type = %s '.
+ 'AND e_ccs.dst in (%Ls)',
+ PhabricatorEdgeConfig::TABLE_NAME_EDGE,
+ PhabricatorEdgeConfig::TYPE_OBJECT_HAS_SUBSCRIBER,
$this->ccs);
}
@@ -647,27 +632,6 @@
$this->reviewers);
}
- if ($this->subscribers) {
- // TODO: These can be expressed as a JOIN again (and the corresponding
- // WHERE clause removed) once subscribers move to edges.
- $joins[] = qsprintf(
- $conn_r,
- 'LEFT JOIN %T sub_rel_cc ON sub_rel_cc.revisionID = r.id '.
- 'AND sub_rel_cc.relation = %s '.
- 'AND sub_rel_cc.objectPHID in (%Ls)',
- DifferentialRevision::RELATIONSHIP_TABLE,
- DifferentialRevision::RELATION_SUBSCRIBED,
- $this->subscribers);
- $joins[] = qsprintf(
- $conn_r,
- 'LEFT JOIN %T sub_rel_reviewer ON sub_rel_reviewer.src = r.phid '.
- 'AND sub_rel_reviewer.type = %s '.
- 'AND sub_rel_reviewer.dst in (%Ls)',
- PhabricatorEdgeConfig::TABLE_NAME_EDGE,
- PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER,
- $this->subscribers);
- }
-
$joins = implode(' ', $joins);
return $joins;
@@ -757,13 +721,6 @@
$this->arcanistProjectPHIDs);
}
- if ($this->subscribers) {
- $where[] = qsprintf(
- $conn_r,
- '(sub_rel_cc.objectPHID IS NOT NULL)
- OR (sub_rel_reviewer.dst IS NOT NULL)');
- }
-
switch ($this->status) {
case self::STATUS_ANY:
break;
@@ -828,8 +785,7 @@
$join_triggers = array_merge(
$this->pathIDs,
$this->ccs,
- $this->reviewers,
- $this->subscribers);
+ $this->reviewers);
$needs_distinct = (count($join_triggers) > 1);
@@ -929,31 +885,32 @@
private function loadRelationships($conn_r, array $revisions) {
assert_instances_of($revisions, 'DifferentialRevision');
- $relationships = queryfx_all(
- $conn_r,
- 'SELECT * FROM %T WHERE revisionID in (%Ld)
- AND relation != %s ORDER BY sequence',
- DifferentialRevision::RELATIONSHIP_TABLE,
- mpull($revisions, 'getID'),
- DifferentialRevision::RELATION_REVIEWER);
- $relationships = igroup($relationships, 'revisionID');
$type_reviewer = PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER;
+ $type_subscriber = PhabricatorEdgeConfig::TYPE_OBJECT_HAS_SUBSCRIBER;
+
$edges = id(new PhabricatorEdgeQuery())
->withSourcePHIDs(mpull($revisions, 'getPHID'))
- ->withEdgeTypes(array($type_reviewer))
+ ->withEdgeTypes(array($type_reviewer, $type_subscriber))
->setOrder(PhabricatorEdgeQuery::ORDER_OLDEST_FIRST)
->execute();
+ $type_map = array(
+ DifferentialRevision::RELATION_REVIEWER => $type_reviewer,
+ DifferentialRevision::RELATION_SUBSCRIBED => $type_subscriber,
+ );
+
foreach ($revisions as $revision) {
- $data = idx($relationships, $revision->getID(), array());
- $revision_edges = $edges[$revision->getPHID()][$type_reviewer];
- foreach ($revision_edges as $dst_phid => $edge_data) {
- $data[] = array(
- 'relation' => DifferentialRevision::RELATION_REVIEWER,
- 'objectPHID' => $dst_phid,
- 'reasonPHID' => null,
- );
+ $data = array();
+ foreach ($type_map as $rel_type => $edge_type) {
+ $revision_edges = $edges[$revision->getPHID()][$edge_type];
+ foreach ($revision_edges as $dst_phid => $edge_data) {
+ $data[] = array(
+ 'relation' => $rel_type,
+ 'objectPHID' => $dst_phid,
+ 'reasonPHID' => null,
+ );
+ }
}
$revision->attachRelationships($data);
Index: src/applications/differential/storage/DifferentialRevision.php
===================================================================
--- src/applications/differential/storage/DifferentialRevision.php
+++ src/applications/differential/storage/DifferentialRevision.php
@@ -6,7 +6,8 @@
PhabricatorPolicyInterface,
PhabricatorFlaggableInterface,
PhrequentTrackableInterface,
- HarbormasterBuildableInterface {
+ HarbormasterBuildableInterface,
+ PhabricatorSubscribableInterface {
protected $title = '';
protected $originalTitle;
@@ -39,7 +40,6 @@
private $reviewerStatus = self::ATTACHABLE;
- const RELATIONSHIP_TABLE = 'differential_relationship';
const TABLE_COMMIT = 'differential_commit';
const RELATION_REVIEWER = 'revw';
@@ -190,12 +190,6 @@
queryfx(
$conn_w,
'DELETE FROM %T WHERE revisionID = %d',
- self::RELATIONSHIP_TABLE,
- $this->getID());
-
- queryfx(
- $conn_w,
- 'DELETE FROM %T WHERE revisionID = %d',
self::TABLE_COMMIT,
$this->getID());
@@ -240,22 +234,24 @@
return;
}
- // Read "subscribed" and "unsubscribed" data out of the old relationship
- // table.
- $data = queryfx_all(
- $this->establishConnection('r'),
- 'SELECT * FROM %T WHERE revisionID = %d
- AND relation != %s ORDER BY sequence',
- self::RELATIONSHIP_TABLE,
- $this->getID(),
- self::RELATION_REVIEWER);
+ $data = array();
+
+ $subscriber_phids = PhabricatorEdgeQuery::loadDestinationPHIDs(
+ $this->getPHID(),
+ PhabricatorEdgeConfig::TYPE_OBJECT_HAS_SUBSCRIBER);
+ $subscriber_phids = array_reverse($subscriber_phids);
+ foreach ($subscriber_phids as $phid) {
+ $data[] = array(
+ 'relation' => self::RELATION_SUBSCRIBED,
+ 'objectPHID' => $phid,
+ 'reasonPHID' => null,
+ );
+ }
- // Read "reviewer" data out of the new table.
$reviewer_phids = PhabricatorEdgeQuery::loadDestinationPHIDs(
$this->getPHID(),
PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER);
$reviewer_phids = array_reverse($reviewer_phids);
-
foreach ($reviewer_phids as $phid) {
$data[] = array(
'relation' => self::RELATION_REVIEWER,
@@ -290,12 +286,6 @@
return idx($this->relationships, $relation, array());
}
- public function loadUnsubscribedPHIDs() {
- return PhabricatorEdgeQuery::loadDestinationPHIDs(
- $this->phid,
- PhabricatorEdgeConfig::TYPE_OBJECT_HAS_UNSUBSCRIBER);
- }
-
public function getPrimaryReviewer() {
$reviewers = $this->getReviewers();
$last = $this->lastReviewerPHID;
@@ -425,4 +415,25 @@
return $this->getPHID();
}
+
+/* -( PhabricatorSubscribableInterface )----------------------------------- */
+
+
+ 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());
+ }
+
+ public function shouldShowSubscribersProperty() {
+ // TODO: For now, Differential has its own stuff.
+ return false;
+ }
+
+ public function shouldAllowSubscription($phid) {
+ // TODO: For now, Differential has its own stuff.
+ return false;
+ }
+
}

File Metadata

Mime Type
text/plain
Expires
Sun, May 12, 3:06 AM (1 w, 4 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6286153
Default Alt Text
D8202.diff (14 KB)

Event Timeline

Phabricator · Built by Phacility