Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15190710
D14859.id35924.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
12 KB
Referenced Files
None
Subscribers
None
D14859.id35924.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
@@ -2834,6 +2834,7 @@
'PhabricatorProjectConfigOptions' => 'applications/project/config/PhabricatorProjectConfigOptions.php',
'PhabricatorProjectConfiguredCustomField' => 'applications/project/customfield/PhabricatorProjectConfiguredCustomField.php',
'PhabricatorProjectController' => 'applications/project/controller/PhabricatorProjectController.php',
+ 'PhabricatorProjectCoreTestCase' => 'applications/project/__tests__/PhabricatorProjectCoreTestCase.php',
'PhabricatorProjectCustomField' => 'applications/project/customfield/PhabricatorProjectCustomField.php',
'PhabricatorProjectCustomFieldNumericIndex' => 'applications/project/storage/PhabricatorProjectCustomFieldNumericIndex.php',
'PhabricatorProjectCustomFieldStorage' => 'applications/project/storage/PhabricatorProjectCustomFieldStorage.php',
@@ -2843,7 +2844,6 @@
'PhabricatorProjectDescriptionField' => 'applications/project/customfield/PhabricatorProjectDescriptionField.php',
'PhabricatorProjectEditDetailsController' => 'applications/project/controller/PhabricatorProjectEditDetailsController.php',
'PhabricatorProjectEditPictureController' => 'applications/project/controller/PhabricatorProjectEditPictureController.php',
- 'PhabricatorProjectEditorTestCase' => 'applications/project/editor/__tests__/PhabricatorProjectEditorTestCase.php',
'PhabricatorProjectFeedController' => 'applications/project/controller/PhabricatorProjectFeedController.php',
'PhabricatorProjectFulltextEngine' => 'applications/project/search/PhabricatorProjectFulltextEngine.php',
'PhabricatorProjectHeraldAction' => 'applications/project/herald/PhabricatorProjectHeraldAction.php',
@@ -7168,6 +7168,7 @@
'PhabricatorStandardCustomFieldInterface',
),
'PhabricatorProjectController' => 'PhabricatorController',
+ 'PhabricatorProjectCoreTestCase' => 'PhabricatorTestCase',
'PhabricatorProjectCustomField' => 'PhabricatorCustomField',
'PhabricatorProjectCustomFieldNumericIndex' => 'PhabricatorCustomFieldNumericIndexStorage',
'PhabricatorProjectCustomFieldStorage' => 'PhabricatorCustomFieldStorage',
@@ -7177,7 +7178,6 @@
'PhabricatorProjectDescriptionField' => 'PhabricatorProjectStandardCustomField',
'PhabricatorProjectEditDetailsController' => 'PhabricatorProjectController',
'PhabricatorProjectEditPictureController' => 'PhabricatorProjectController',
- 'PhabricatorProjectEditorTestCase' => 'PhabricatorTestCase',
'PhabricatorProjectFeedController' => 'PhabricatorProjectController',
'PhabricatorProjectFulltextEngine' => 'PhabricatorFulltextEngine',
'PhabricatorProjectHeraldAction' => 'HeraldAction',
diff --git a/src/applications/project/editor/__tests__/PhabricatorProjectEditorTestCase.php b/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
rename from src/applications/project/editor/__tests__/PhabricatorProjectEditorTestCase.php
rename to src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
--- a/src/applications/project/editor/__tests__/PhabricatorProjectEditorTestCase.php
+++ b/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
@@ -1,6 +1,6 @@
<?php
-final class PhabricatorProjectEditorTestCase extends PhabricatorTestCase {
+final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase {
protected function getPhabricatorTestCaseConfiguration() {
return array(
@@ -39,6 +39,49 @@
$this->assertFalse((bool)$this->refreshProject($proj, $user2));
}
+ public function testIsViewerMemberOrWatcher() {
+ $user1 = $this->createUser()
+ ->save();
+
+ $user2 = $this->createUser()
+ ->save();
+
+ $user3 = $this->createUser()
+ ->save();
+
+ $proj1 = $this->createProject($user1);
+ $proj1 = $this->refreshProject($proj1, $user1);
+
+ $this->joinProject($proj1, $user1);
+ $this->joinProject($proj1, $user3);
+ $this->watchProject($proj1, $user3);
+
+ $proj1 = $this->refreshProject($proj1, $user1);
+
+ $this->assertTrue($proj1->isUserMember($user1->getPHID()));
+
+ $proj1 = $this->refreshProject($proj1, $user1, false, true);
+
+ $this->assertTrue($proj1->isUserMember($user1->getPHID()));
+ $this->assertFalse($proj1->isUserWatcher($user1->getPHID()));
+
+ $proj1 = $this->refreshProject($proj1, $user1, true, false);
+
+ $this->assertTrue($proj1->isUserMember($user1->getPHID()));
+ $this->assertFalse($proj1->isUserMember($user2->getPHID()));
+ $this->assertTrue($proj1->isUserMember($user3->getPHID()));
+
+ $proj1 = $this->refreshProject($proj1, $user1, true, true);
+
+ $this->assertTrue($proj1->isUserMember($user1->getPHID()));
+ $this->assertFalse($proj1->isUserMember($user2->getPHID()));
+ $this->assertTrue($proj1->isUserMember($user3->getPHID()));
+
+ $this->assertFalse($proj1->isUserWatcher($user1->getPHID()));
+ $this->assertFalse($proj1->isUserWatcher($user2->getPHID()));
+ $this->assertTrue($proj1->isUserWatcher($user3->getPHID()));
+ }
+
public function testEditProject() {
$user = $this->createUser();
$user->save();
@@ -210,11 +253,13 @@
private function refreshProject(
PhabricatorProject $project,
PhabricatorUser $viewer,
- $need_members = false) {
+ $need_members = false,
+ $need_watchers = false) {
$results = id(new PhabricatorProjectQuery())
->setViewer($viewer)
->needMembers($need_members)
+ ->needWatchers($need_watchers)
->withIDs(array($project->getID()))
->execute();
@@ -255,19 +300,54 @@
private function joinProject(
PhabricatorProject $project,
PhabricatorUser $user) {
- $this->joinOrLeaveProject($project, $user, '+');
+ return $this->joinOrLeaveProject($project, $user, '+');
}
private function leaveProject(
PhabricatorProject $project,
PhabricatorUser $user) {
- $this->joinOrLeaveProject($project, $user, '-');
+ return $this->joinOrLeaveProject($project, $user, '-');
+ }
+
+ private function watchProject(
+ PhabricatorProject $project,
+ PhabricatorUser $user) {
+ return $this->watchOrUnwatchProject($project, $user, '+');
+ }
+
+ private function unwatchProject(
+ PhabricatorProject $project,
+ PhabricatorUser $user) {
+ return $this->watchOrUnwatchProject($project, $user, '-');
}
private function joinOrLeaveProject(
PhabricatorProject $project,
PhabricatorUser $user,
$operation) {
+ return $this->applyProjectEdgeTransaction(
+ $project,
+ $user,
+ $operation,
+ PhabricatorProjectProjectHasMemberEdgeType::EDGECONST);
+ }
+
+ private function watchOrUnwatchProject(
+ PhabricatorProject $project,
+ PhabricatorUser $user,
+ $operation) {
+ return $this->applyProjectEdgeTransaction(
+ $project,
+ $user,
+ $operation,
+ PhabricatorObjectHasWatcherEdgeType::EDGECONST);
+ }
+
+ private function applyProjectEdgeTransaction(
+ PhabricatorProject $project,
+ PhabricatorUser $user,
+ $operation,
+ $edge_type) {
$spec = array(
$operation => array($user->getPHID() => $user->getPHID()),
@@ -276,9 +356,7 @@
$xactions = array();
$xactions[] = id(new PhabricatorProjectTransaction())
->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
- ->setMetadataValue(
- 'edge:type',
- PhabricatorProjectProjectHasMemberEdgeType::EDGECONST)
+ ->setMetadataValue('edge:type', $edge_type)
->setNewValue($spec);
$editor = id(new PhabricatorProjectTransactionEditor())
@@ -286,6 +364,9 @@
->setContentSource(PhabricatorContentSource::newConsoleSource())
->setContinueOnNoEffect(true)
->applyTransactions($project, $xactions);
+
+ return $project;
}
+
}
diff --git a/src/applications/project/query/PhabricatorProjectQuery.php b/src/applications/project/query/PhabricatorProjectQuery.php
--- a/src/applications/project/query/PhabricatorProjectQuery.php
+++ b/src/applications/project/query/PhabricatorProjectQuery.php
@@ -126,59 +126,57 @@
}
protected function loadPage() {
- $table = new PhabricatorProject();
- $data = $this->loadStandardPageRows($table);
- $projects = $table->loadAllFromArray($data);
+ return $this->loadStandardPage($this->newResultObject());
+ }
- if ($projects) {
- $viewer_phid = $this->getViewer()->getPHID();
- $project_phids = mpull($projects, 'getPHID');
+ protected function willFilterPage(array $projects) {
+ $viewer_phid = $this->getViewer()->getPHID();
+ $project_phids = mpull($projects, 'getPHID');
- $member_type = PhabricatorProjectProjectHasMemberEdgeType::EDGECONST;
- $watcher_type = PhabricatorObjectHasWatcherEdgeType::EDGECONST;
+ $member_type = PhabricatorProjectProjectHasMemberEdgeType::EDGECONST;
+ $watcher_type = PhabricatorObjectHasWatcherEdgeType::EDGECONST;
- $need_edge_types = array();
- if ($this->needMembers) {
- $need_edge_types[] = $member_type;
- } else {
- foreach ($data as $row) {
- $projects[$row['id']]->setIsUserMember(
- $viewer_phid,
- ($row['viewerIsMember'] !== null));
- }
- }
+ $types = array();
+ $types[] = $member_type;
+ if ($this->needWatchers) {
+ $types[] = $watcher_type;
+ }
- if ($this->needWatchers) {
- $need_edge_types[] = $watcher_type;
- }
+ $edge_query = id(new PhabricatorEdgeQuery())
+ ->withSourcePHIDs($project_phids)
+ ->withEdgeTypes($types);
- if ($need_edge_types) {
- $edges = id(new PhabricatorEdgeQuery())
- ->withSourcePHIDs($project_phids)
- ->withEdgeTypes($need_edge_types)
- ->execute();
+ // If we only need to know if the viewer is a member, we can restrict
+ // the query to just their PHID.
+ if (!$this->needMembers && !$this->needWatchers) {
+ $edge_query->withDestinationPHIDs(array($viewer_phid));
+ }
- if ($this->needMembers) {
- foreach ($projects as $project) {
- $phid = $project->getPHID();
- $project->attachMemberPHIDs(
- array_keys($edges[$phid][$member_type]));
- $project->setIsUserMember(
- $viewer_phid,
- isset($edges[$phid][$member_type][$viewer_phid]));
- }
- }
+ $edge_query->execute();
- if ($this->needWatchers) {
- foreach ($projects as $project) {
- $phid = $project->getPHID();
- $project->attachWatcherPHIDs(
- array_keys($edges[$phid][$watcher_type]));
- $project->setIsUserWatcher(
- $viewer_phid,
- isset($edges[$phid][$watcher_type][$viewer_phid]));
- }
- }
+ foreach ($projects as $project) {
+ $project_phid = $project->getPHID();
+
+ $member_phids = $edge_query->getDestinationPHIDs(
+ array($project_phid),
+ array($member_type));
+
+ $project->setIsUserMember(
+ $viewer_phid,
+ in_array($viewer_phid, $member_phids));
+
+ if ($this->needMembers) {
+ $project->attachMemberPHIDs($member_phids);
+ }
+
+ if ($this->needWatchers) {
+ $watcher_phids = $edge_query->getDestinationPHIDs(
+ array($project_phid),
+ array($watcher_type));
+ $project->attachWatcherPHIDs($watcher_phids);
+ $project->setIsUserWatcher(
+ $viewer_phid,
+ in_array($viewer_phid, $watcher_phids));
}
}
@@ -231,20 +229,6 @@
return $projects;
}
- protected function buildSelectClauseParts(AphrontDatabaseConnection $conn) {
- $select = parent::buildSelectClauseParts($conn);
-
- // NOTE: Because visibility checks for projects depend on whether or not
- // the user is a project member, we always load their membership. If we're
- // loading all members anyway we can piggyback on that; otherwise we
- // do an explicit join.
- if (!$this->needMembers) {
- $select[] = 'vm.dst viewerIsMember';
- }
-
- return $select;
- }
-
protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
$where = parent::buildWhereClauseParts($conn);
@@ -336,15 +320,6 @@
protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) {
$joins = parent::buildJoinClauseParts($conn);
- if (!$this->needMembers !== null) {
- $joins[] = qsprintf(
- $conn,
- 'LEFT JOIN %T vm ON vm.src = p.phid AND vm.type = %d AND vm.dst = %s',
- PhabricatorEdgeConfig::TABLE_NAME_EDGE,
- PhabricatorProjectProjectHasMemberEdgeType::EDGECONST,
- $this->getViewer()->getPHID());
- }
-
if ($this->memberPHIDs !== null) {
$joins[] = qsprintf(
$conn,
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Sat, Feb 22, 9:55 PM (5 h, 6 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7182877
Default Alt Text
D14859.id35924.diff (12 KB)
Attached To
Mode
D14859: Simplify ProjectQuery handling of viewer membership
Attached
Detach File
Event Timeline
Log In to Comment