1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-19 12:00:55 +01:00

Implement a "Project Members" object policy rule

Summary:
Fixes T9019. Pretty much ripped from D14467. I added the "policy hint" stuff so that you can create a project with this policy immediately.

I really dislike how the "hint" code works, but we //almost// never need to use it and the badness feels fairly well-contained.

Also pick up a quick feedback fix from D14863.

Test Plan:
  - Added test coverage, got it to pass.
  - Created a project with "Visible To: Project Members".

Reviewers: joshuaspence, chad

Reviewed By: chad

Maniphest Tasks: T9019

Differential Revision: https://secure.phabricator.com/D14869
This commit is contained in:
epriestley 2015-12-23 16:34:01 -08:00
parent e2edb1577c
commit 1c572d1da5
6 changed files with 198 additions and 5 deletions

View file

@ -2859,6 +2859,7 @@ phutil_register_library_map(array(
'PhabricatorProjectMemberOfProjectEdgeType' => 'applications/project/edge/PhabricatorProjectMemberOfProjectEdgeType.php', 'PhabricatorProjectMemberOfProjectEdgeType' => 'applications/project/edge/PhabricatorProjectMemberOfProjectEdgeType.php',
'PhabricatorProjectMembersDatasource' => 'applications/project/typeahead/PhabricatorProjectMembersDatasource.php', 'PhabricatorProjectMembersDatasource' => 'applications/project/typeahead/PhabricatorProjectMembersDatasource.php',
'PhabricatorProjectMembersEditController' => 'applications/project/controller/PhabricatorProjectMembersEditController.php', 'PhabricatorProjectMembersEditController' => 'applications/project/controller/PhabricatorProjectMembersEditController.php',
'PhabricatorProjectMembersPolicyRule' => 'applications/project/policyrule/PhabricatorProjectMembersPolicyRule.php',
'PhabricatorProjectMembersRemoveController' => 'applications/project/controller/PhabricatorProjectMembersRemoveController.php', 'PhabricatorProjectMembersRemoveController' => 'applications/project/controller/PhabricatorProjectMembersRemoveController.php',
'PhabricatorProjectMoveController' => 'applications/project/controller/PhabricatorProjectMoveController.php', 'PhabricatorProjectMoveController' => 'applications/project/controller/PhabricatorProjectMoveController.php',
'PhabricatorProjectNoProjectsDatasource' => 'applications/project/typeahead/PhabricatorProjectNoProjectsDatasource.php', 'PhabricatorProjectNoProjectsDatasource' => 'applications/project/typeahead/PhabricatorProjectNoProjectsDatasource.php',
@ -7195,6 +7196,7 @@ phutil_register_library_map(array(
'PhabricatorProjectMemberOfProjectEdgeType' => 'PhabricatorEdgeType', 'PhabricatorProjectMemberOfProjectEdgeType' => 'PhabricatorEdgeType',
'PhabricatorProjectMembersDatasource' => 'PhabricatorTypeaheadCompositeDatasource', 'PhabricatorProjectMembersDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
'PhabricatorProjectMembersEditController' => 'PhabricatorProjectController', 'PhabricatorProjectMembersEditController' => 'PhabricatorProjectController',
'PhabricatorProjectMembersPolicyRule' => 'PhabricatorPolicyRule',
'PhabricatorProjectMembersRemoveController' => 'PhabricatorProjectController', 'PhabricatorProjectMembersRemoveController' => 'PhabricatorProjectController',
'PhabricatorProjectMoveController' => 'PhabricatorProjectController', 'PhabricatorProjectMoveController' => 'PhabricatorProjectController',
'PhabricatorProjectNoProjectsDatasource' => 'PhabricatorTypeaheadDatasource', 'PhabricatorProjectNoProjectsDatasource' => 'PhabricatorTypeaheadDatasource',

View file

@ -608,6 +608,11 @@ final class PhabricatorPolicyFilter extends Phobject {
$rules = PhabricatorPolicyQuery::getObjectPolicyRules(null); $rules = PhabricatorPolicyQuery::getObjectPolicyRules(null);
// Make sure we have clean, empty policy rule objects.
foreach ($rules as $key => $rule) {
$rules[$key] = clone $rule;
}
$results = array(); $results = array();
foreach ($map as $key => $object_list) { foreach ($map as $key => $object_list) {
$rule = idx($rules, $key); $rule = idx($rules, $key);

View file

@ -287,7 +287,6 @@ final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase {
// In this second case, set the name first and then the slugs separately. // In this second case, set the name first and then the slugs separately.
$name2 = 'slugproject2'; $name2 = 'slugproject2';
$xactions = array(); $xactions = array();
$xactions[] = id(new PhabricatorProjectTransaction()) $xactions[] = id(new PhabricatorProjectTransaction())
@ -396,6 +395,51 @@ final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase {
$this->assertTrue((bool)$caught); $this->assertTrue((bool)$caught);
} }
public function testProjectMembersVisibility() {
// This is primarily testing that you can create a project and set the
// visibility or edit policy to "Project Members" immediately.
$user1 = $this->createUser();
$user1->save();
$user2 = $this->createUser();
$user2->save();
$project = PhabricatorProject::initializeNewProject($user1);
$name = pht('Test Project %d', mt_rand());
$xactions = array();
$xactions[] = id(new PhabricatorProjectTransaction())
->setTransactionType(PhabricatorProjectTransaction::TYPE_NAME)
->setNewValue($name);
$xactions[] = id(new PhabricatorProjectTransaction())
->setTransactionType(PhabricatorTransactions::TYPE_VIEW_POLICY)
->setNewValue(
id(new PhabricatorProjectMembersPolicyRule())
->getObjectPolicyFullKey());
$edge_type = PhabricatorProjectProjectHasMemberEdgeType::EDGECONST;
$xactions[] = id(new PhabricatorProjectTransaction())
->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
->setMetadataValue('edge:type', $edge_type)
->setNewValue(
array(
'=' => array($user1->getPHID() => $user1->getPHID()),
));
$this->applyTransactions($project, $user1, $xactions);
$this->assertTrue((bool)$this->refreshProject($project, $user1));
$this->assertFalse((bool)$this->refreshProject($project, $user2));
$this->leaveProject($project, $user1);
$this->assertFalse((bool)$this->refreshProject($project, $user1));
}
public function testParentProject() { public function testParentProject() {
$user = $this->createUser(); $user = $this->createUser();
$user->save(); $user->save();

View file

@ -692,4 +692,53 @@ final class PhabricatorProjectTransactionEditor
return $slugs; return $slugs;
} }
protected function adjustObjectForPolicyChecks(
PhabricatorLiskDAO $object,
array $xactions) {
$copy = parent::adjustObjectForPolicyChecks($object, $xactions);
$type_edge = PhabricatorTransactions::TYPE_EDGE;
$edgetype_member = PhabricatorProjectProjectHasMemberEdgeType::EDGECONST;
$member_xaction = null;
foreach ($xactions as $xaction) {
if ($xaction->getTransactionType() !== $type_edge) {
continue;
}
$edgetype = $xaction->getMetadataValue('edge:type');
if ($edgetype !== $edgetype_member) {
continue;
}
$member_xaction = $xaction;
}
if ($member_xaction) {
$object_phid = $object->getPHID();
if ($object_phid) {
$members = PhabricatorEdgeQuery::loadDestinationPHIDs(
$object_phid,
PhabricatorProjectProjectHasMemberEdgeType::EDGECONST);
} else {
$members = array();
}
$clone_xaction = clone $member_xaction;
$hint = $this->getPHIDTransactionNewValue($clone_xaction, $members);
$rule = new PhabricatorProjectMembersPolicyRule();
$hint = array_fuse($hint);
PhabricatorPolicyRule::passTransactionHintToRule(
$copy,
$rule,
$hint);
}
return $copy;
}
} }

View file

@ -0,0 +1,97 @@
<?php
final class PhabricatorProjectMembersPolicyRule extends PhabricatorPolicyRule {
private $memberships = array();
public function getRuleDescription() {
return pht('members of project');
}
public function willApplyRules(
PhabricatorUser $viewer,
array $values,
array $objects) {
$viewer_phid = $viewer->getPHID();
if (!$viewer_phid) {
return;
}
if (empty($this->memberships[$viewer_phid])) {
$this->memberships[$viewer_phid] = array();
}
foreach ($objects as $key => $object) {
$cache = $this->getTransactionHint($object);
if ($cache === null) {
continue;
}
unset($objects[$key]);
if (isset($cache[$viewer_phid])) {
$this->memberships[$viewer_phid][$object->getPHID()] = true;
}
}
if (!$objects) {
return;
}
$object_phids = mpull($objects, 'getPHID');
$edge_query = id(new PhabricatorEdgeQuery())
->withSourcePHIDs(array($viewer_phid))
->withDestinationPHIDs($object_phids)
->withEdgeTypes(
array(
PhabricatorProjectMemberOfProjectEdgeType::EDGECONST,
));
$edge_query->execute();
$memberships = $edge_query->getDestinationPHIDs();
if (!$memberships) {
return;
}
$this->memberships[$viewer_phid] += array_fill_keys($memberships, true);
}
public function applyRule(
PhabricatorUser $viewer,
$value,
PhabricatorPolicyInterface $object) {
$viewer_phid = $viewer->getPHID();
if (!$viewer_phid) {
return false;
}
$memberships = idx($this->memberships, $viewer_phid);
return isset($memberships[$object->getPHID()]);
}
public function getValueControlType() {
return self::CONTROL_TYPE_NONE;
}
public function canApplyToObject(PhabricatorPolicyInterface $object) {
return ($object instanceof PhabricatorProject);
}
public function getObjectPolicyKey() {
return 'project.members';
}
public function getObjectPolicyName() {
return pht('Project Members');
}
public function getObjectPolicyIcon() {
return 'fa-users';
}
public function getPolicyExplanation() {
return pht('Project members can take this action.');
}
}

View file

@ -95,10 +95,6 @@ final class PhabricatorProjectQuery
return $this; return $this;
} }
public function getProperty() {
return $this->property;
}
public function withDepthBetween($min, $max) { public function withDepthBetween($min, $max) {
$this->minDepth = $min; $this->minDepth = $min;
$this->maxDepth = $max; $this->maxDepth = $max;