Page MenuHomePhabricator
Files F13987708

D8167.id18478.diff
No OneTemporary
Actions

D8167.id18478.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
@@ -1852,6 +1852,7 @@
'PhabricatorProjectStatus' => 'applications/project/constants/PhabricatorProjectStatus.php',
'PhabricatorProjectTestDataGenerator' => 'applications/project/lipsum/PhabricatorProjectTestDataGenerator.php',
'PhabricatorProjectTransaction' => 'applications/project/storage/PhabricatorProjectTransaction.php',
+ 'PhabricatorProjectTransactionEditor' => 'applications/project/editor/PhabricatorProjectTransactionEditor.php',
'PhabricatorProjectTransactionQuery' => 'applications/project/query/PhabricatorProjectTransactionQuery.php',
'PhabricatorProjectUpdateController' => 'applications/project/controller/PhabricatorProjectUpdateController.php',
'PhabricatorQuery' => 'infrastructure/query/PhabricatorQuery.php',
@@ -4549,6 +4550,7 @@
0 => 'PhabricatorProjectDAO',
1 => 'PhabricatorFlaggableInterface',
2 => 'PhabricatorPolicyInterface',
+ 3 => 'PhabricatorSubscribableInterface',
),
'PhabricatorProjectBoardController' => 'PhabricatorProjectController',
'PhabricatorProjectBoardEditController' => 'PhabricatorProjectController',
@@ -4586,6 +4588,7 @@
'PhabricatorProjectSearchIndexer' => 'PhabricatorSearchDocumentIndexer',
'PhabricatorProjectTestDataGenerator' => 'PhabricatorTestDataGenerator',
'PhabricatorProjectTransaction' => 'PhabricatorApplicationTransaction',
+ 'PhabricatorProjectTransactionEditor' => 'PhabricatorApplicationTransactionEditor',
'PhabricatorProjectTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
'PhabricatorProjectUpdateController' => 'PhabricatorProjectController',
'PhabricatorRecaptchaConfigOptions' => 'PhabricatorApplicationConfigOptions',
diff --git a/src/applications/project/controller/PhabricatorProjectMembersEditController.php b/src/applications/project/controller/PhabricatorProjectMembersEditController.php
--- a/src/applications/project/controller/PhabricatorProjectMembersEditController.php
+++ b/src/applications/project/controller/PhabricatorProjectMembersEditController.php
@@ -29,41 +29,34 @@
$member_phids = $project->getMemberPHIDs();
- $errors = array();
if ($request->isFormPost()) {
- $changed_something = false;
- $member_map = array_fill_keys($member_phids, true);
+ $member_spec = array();
$remove = $request->getStr('remove');
if ($remove) {
- if (isset($member_map[$remove])) {
- unset($member_map[$remove]);
- $changed_something = true;
- }
- } else {
- $new_members = $request->getArr('phids');
- foreach ($new_members as $member) {
- if (empty($member_map[$member])) {
- $member_map[$member] = true;
- $changed_something = true;
- }
- }
+ $member_spec['-'] = array_fuse(array($remove));
}
- $xactions = array();
- if ($changed_something) {
- $xaction = new PhabricatorProjectTransaction();
- $xaction->setTransactionType(
- PhabricatorProjectTransaction::TYPE_MEMBERS);
- $xaction->setNewValue(array_keys($member_map));
- $xactions[] = $xaction;
+ $add_members = $request->getArr('phids');
+ if ($add_members) {
+ $member_spec['+'] = array_fuse($add_members);
}
- if ($xactions) {
- $editor = new PhabricatorProjectEditor($project);
- $editor->setActor($user);
- $editor->applyTransactions($xactions);
- }
+ $type_member = PhabricatorEdgeConfig::TYPE_PROJ_MEMBER;
+
+ $xactions = array();
+
+ $xactions[] = id(new PhabricatorProjectTransaction())
+ ->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
+ ->setMetadataValue('edge:type', $type_member)
+ ->setNewValue($member_spec);
+
+ $editor = id(new PhabricatorProjectTransactionEditor($project))
+ ->setActor($user)
+ ->setContentSourceFromRequest($request)
+ ->setContinueOnNoEffect(true)
+ ->setContinueOnMissingFields(true)
+ ->applyTransactions($project, $xactions);
return id(new AphrontRedirectResponse())
->setURI($request->getRequestURI());
diff --git a/src/applications/project/controller/PhabricatorProjectUpdateController.php b/src/applications/project/controller/PhabricatorProjectUpdateController.php
--- a/src/applications/project/controller/PhabricatorProjectUpdateController.php
+++ b/src/applications/project/controller/PhabricatorProjectUpdateController.php
@@ -45,14 +45,35 @@
$project_uri = '/project/view/'.$project->getID().'/';
if ($process_action) {
+
+ $edge_action = null;
switch ($this->action) {
case 'join':
- PhabricatorProjectEditor::applyJoinProject($project, $user);
+ $edge_action = '+';
break;
case 'leave':
- PhabricatorProjectEditor::applyLeaveProject($project, $user);
+ $edge_action = '-';
break;
}
+
+ $type_member = PhabricatorEdgeConfig::TYPE_PROJ_MEMBER;
+ $member_spec = array(
+ $edge_action => array($user->getPHID() => $user->getPHID()),
+ );
+
+ $xactions = array();
+ $xactions[] = id(new PhabricatorProjectTransaction())
+ ->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
+ ->setMetadataValue('edge:type', $type_member)
+ ->setNewValue($member_spec);
+
+ $editor = id(new PhabricatorProjectTransactionEditor($project))
+ ->setActor($user)
+ ->setContentSourceFromRequest($request)
+ ->setContinueOnNoEffect(true)
+ ->setContinueOnMissingFields(true)
+ ->applyTransactions($project, $xactions);
+
return id(new AphrontRedirectResponse())->setURI($project_uri);
}
diff --git a/src/applications/project/editor/PhabricatorProjectEditor.php b/src/applications/project/editor/PhabricatorProjectEditor.php
--- a/src/applications/project/editor/PhabricatorProjectEditor.php
+++ b/src/applications/project/editor/PhabricatorProjectEditor.php
@@ -18,51 +18,6 @@
return $this->shouldArchive;
}
- public static function applyJoinProject(
- PhabricatorProject $project,
- PhabricatorUser $user) {
-
- $members = $project->getMemberPHIDs();
- $members[] = $user->getPHID();
-
- self::applyOneTransaction(
- $project,
- $user,
- PhabricatorProjectTransaction::TYPE_MEMBERS,
- $members);
- }
-
- public static function applyLeaveProject(
- PhabricatorProject $project,
- PhabricatorUser $user) {
-
- $members = array_fill_keys($project->getMemberPHIDs(), true);
- unset($members[$user->getPHID()]);
- $members = array_keys($members);
-
- self::applyOneTransaction(
- $project,
- $user,
- PhabricatorProjectTransaction::TYPE_MEMBERS,
- $members);
- }
-
- private static function applyOneTransaction(
- PhabricatorProject $project,
- PhabricatorUser $user,
- $type,
- $new_value) {
-
- $xaction = new PhabricatorProjectTransaction();
- $xaction->setTransactionType($type);
- $xaction->setNewValue($new_value);
-
- $editor = new PhabricatorProjectEditor($project);
- $editor->setActor($user);
- $editor->applyTransactions(array($xaction));
- }
-
-
public function __construct(PhabricatorProject $project) {
$this->project = $project;
}
@@ -94,30 +49,10 @@
$project,
PhabricatorPolicyCapability::CAN_VIEW);
- $need_edit = false;
- $need_join = false;
- foreach ($transactions as $key => $xaction) {
- if ($this->getTransactionRequiresEditCapability($xaction)) {
- $need_edit = true;
- }
- if ($this->getTransactionRequiresJoinCapability($xaction)) {
- $need_join = true;
- }
- }
-
- if ($need_edit) {
- PhabricatorPolicyFilter::requireCapability(
- $actor,
- $project,
- PhabricatorPolicyCapability::CAN_EDIT);
- }
-
- if ($need_join) {
- PhabricatorPolicyFilter::requireCapability(
- $actor,
- $project,
- PhabricatorPolicyCapability::CAN_JOIN);
- }
+ PhabricatorPolicyFilter::requireCapability(
+ $actor,
+ $project,
+ PhabricatorPolicyCapability::CAN_EDIT);
}
if (!$transactions) {
@@ -317,75 +252,4 @@
return ($xaction->getOldValue() !== $xaction->getNewValue());
}
-
- /**
- * All transactions except joining or leaving a project require edit
- * capability.
- */
- private function getTransactionRequiresEditCapability(
- PhabricatorProjectTransaction $xaction) {
- return ($this->isJoinOrLeaveTransaction($xaction) === null);
- }
-
-
- /**
- * Joining a project requires the join capability. Anyone leave a project.
- */
- private function getTransactionRequiresJoinCapability(
- PhabricatorProjectTransaction $xaction) {
- $type = $this->isJoinOrLeaveTransaction($xaction);
- return ($type == 'join');
- }
-
-
- /**
- * Returns 'join' if this transaction causes the acting user ONLY to join the
- * project.
- *
- * Returns 'leave' if this transaction causes the acting user ONLY to leave
- * the project.
- *
- * Returns null in all other cases.
- */
- private function isJoinOrLeaveTransaction(
- PhabricatorProjectTransaction $xaction) {
-
- $type = $xaction->getTransactionType();
- if ($type != PhabricatorProjectTransaction::TYPE_MEMBERS) {
- return null;
- }
-
- switch ($type) {
- case PhabricatorProjectTransaction::TYPE_MEMBERS:
- $old = $xaction->getOldValue();
- $new = $xaction->getNewValue();
-
- $add = array_diff($new, $old);
- $rem = array_diff($old, $new);
-
- if (count($add) > 1) {
- return null;
- } else if (count($add) == 1) {
- if (reset($add) != $this->getActor()->getPHID()) {
- return null;
- } else {
- return 'join';
- }
- }
-
- if (count($rem) > 1) {
- return null;
- } else if (count($rem) == 1) {
- if (reset($rem) != $this->getActor()->getPHID()) {
- return null;
- } else {
- return 'leave';
- }
- }
- break;
- }
-
- return true;
- }
-
}
diff --git a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php
new file mode 100644
--- /dev/null
+++ b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php
@@ -0,0 +1,113 @@
+<?php
+
+final class PhabricatorProjectTransactionEditor
+ extends PhabricatorApplicationTransactionEditor {
+
+ public function getTransactionTypes() {
+ $types = parent::getTransactionTypes();
+
+ $types[] = PhabricatorTransactions::TYPE_EDGE;
+
+ return $types;
+ }
+
+ protected function getCustomTransactionOldValue(
+ PhabricatorLiskDAO $object,
+ PhabricatorApplicationTransaction $xaction) {
+
+ switch ($xaction->getTransactionType()) {
+ }
+
+ return parent::getCustomTransactionOldValue($object, $xaction);
+ }
+
+ protected function getCustomTransactionNewValue(
+ PhabricatorLiskDAO $object,
+ PhabricatorApplicationTransaction $xaction) {
+
+ switch ($xaction->getTransactionType()) {
+ }
+
+ return parent::getCustomTransactionNewValue($object, $xaction);
+ }
+
+ protected function applyCustomInternalTransaction(
+ PhabricatorLiskDAO $object,
+ PhabricatorApplicationTransaction $xaction) {
+
+ switch ($xaction->getTransactionType()) {
+ case PhabricatorTransactions::TYPE_EDGE:
+ return;
+ }
+
+ return parent::applyCustomInternalTransaction($object, $xaction);
+ }
+
+ protected function applyCustomExternalTransaction(
+ PhabricatorLiskDAO $object,
+ PhabricatorApplicationTransaction $xaction) {
+
+ switch ($xaction->getTransactionType()) {
+ case PhabricatorTransactions::TYPE_EDGE:
+ return;
+ }
+
+ return parent::applyCustomExternalTransaction($object, $xaction);
+ }
+
+ protected function validateTransaction(
+ PhabricatorLiskDAO $object,
+ $type,
+ array $xactions) {
+
+ $errors = parent::validateTransaction($object, $type, $xactions);
+
+ switch ($type) {
+ }
+
+ return $errors;
+ }
+
+ protected function requireCapabilities(
+ PhabricatorLiskDAO $object,
+ PhabricatorApplicationTransaction $xaction) {
+
+ switch ($xaction->getTransactionType()) {
+ case PhabricatorTransactions::TYPE_EDGE:
+ switch ($xaction->getMetadataValue('edge:type')) {
+ case PhabricatorEdgeConfig::TYPE_PROJ_MEMBER:
+ $old = $xaction->getOldValue();
+ $new = $xaction->getNewValue();
+
+ $add = array_keys(array_diff_key($new, $old));
+ $rem = array_keys(array_diff_key($old, $new));
+
+ $actor_phid = $this->requireActor()->getPHID();
+
+ $is_join = (($add === array($actor_phid)) && !$rem);
+ $is_leave = (($rem === array($actor_phid)) && !$add);
+
+ if ($is_join) {
+ // You need CAN_JOIN to join a project.
+ PhabricatorPolicyFilter::requireCapability(
+ $this->requireActor(),
+ $object,
+ PhabricatorPolicyCapability::CAN_JOIN);
+ } else if ($is_leave) {
+ // You don't need any capabilities to leave a project.
+ } else {
+ // You need CAN_EDIT to change members other than yourself.
+ PhabricatorPolicyFilter::requireCapability(
+ $this->requireActor(),
+ $object,
+ PhabricatorPolicyCapability::CAN_EDIT);
+ }
+ return;
+ }
+ break;
+ }
+
+ return parent::requireCapabilities();
+ }
+
+}
diff --git a/src/applications/project/editor/__tests__/PhabricatorProjectEditorTestCase.php b/src/applications/project/editor/__tests__/PhabricatorProjectEditorTestCase.php
--- a/src/applications/project/editor/__tests__/PhabricatorProjectEditorTestCase.php
+++ b/src/applications/project/editor/__tests__/PhabricatorProjectEditorTestCase.php
@@ -21,7 +21,7 @@
$proj = $this->refreshProject($proj, $user, true);
- PhabricatorProjectEditor::applyJoinProject($proj, $user);
+ $this->joinProject($proj, $user);
$proj->setViewPolicy(PhabricatorPolicies::POLICY_USER);
$proj->save();
@@ -123,7 +123,7 @@
'Arbitrary user not member of project.');
// Join the project.
- PhabricatorProjectEditor::applyJoinProject($proj, $user);
+ $this->joinProject($proj, $user);
$proj = $this->refreshProject($proj, $user, true);
$this->assertEqual(true, (bool)$proj);
@@ -135,7 +135,7 @@
// Join the project again.
- PhabricatorProjectEditor::applyJoinProject($proj, $user);
+ $this->joinProject($proj, $user);
$proj = $this->refreshProject($proj, $user, true);
$this->assertEqual(true, (bool)$proj);
@@ -147,7 +147,7 @@
// Leave the project.
- PhabricatorProjectEditor::applyLeaveProject($proj, $user);
+ $this->leaveProject($proj, $user);
$proj = $this->refreshProject($proj, $user, true);
$this->assertEqual(true, (bool)$proj);
@@ -159,7 +159,7 @@
// Leave the project again.
- PhabricatorProjectEditor::applyLeaveProject($proj, $user);
+ $this->leaveProject($proj, $user);
$proj = $this->refreshProject($proj, $user, true);
$this->assertEqual(true, (bool)$proj);
@@ -178,7 +178,7 @@
$proj = $this->refreshProject($proj, $user, true);
$caught = null;
try {
- PhabricatorProjectEditor::applyJoinProject($proj, $user);
+ $this->joinProject($proj, $user);
} catch (Exception $ex) {
$caught = $ex;
}
@@ -191,13 +191,13 @@
$proj->save();
$proj = $this->refreshProject($proj, $user, true);
- PhabricatorProjectEditor::applyJoinProject($proj, $user);
+ $this->joinProject($proj, $user);
$proj = $this->refreshProject($proj, $user, true);
$this->assertEqual(
true,
$proj->isUserMember($user->getPHID()),
'Join allowed with edit permission.');
- PhabricatorProjectEditor::applyLeaveProject($proj, $user);
+ $this->leaveProject($proj, $user);
// If a user can join a project, they can join, even if they can't edit.
@@ -206,7 +206,7 @@
$proj->save();
$proj = $this->refreshProject($proj, $user, true);
- PhabricatorProjectEditor::applyJoinProject($proj, $user);
+ $this->joinProject($proj, $user);
$proj = $this->refreshProject($proj, $user, true);
$this->assertEqual(
true,
@@ -220,7 +220,7 @@
$proj->save();
$proj = $this->refreshProject($proj, $user, true);
- PhabricatorProjectEditor::applyLeaveProject($proj, $user);
+ $this->leaveProject($proj, $user);
$proj = $this->refreshProject($proj, $user, true);
$this->assertEqual(
false,
@@ -273,4 +273,38 @@
return $user;
}
+ private function joinProject(
+ PhabricatorProject $project,
+ PhabricatorUser $user) {
+ $this->joinOrLeaveProject($project, $user, '+');
+ }
+
+ private function leaveProject(
+ PhabricatorProject $project,
+ PhabricatorUser $user) {
+ $this->joinOrLeaveProject($project, $user, '-');
+ }
+
+ private function joinOrLeaveProject(
+ PhabricatorProject $project,
+ PhabricatorUser $user,
+ $operation) {
+
+ $spec = array(
+ $operation => array($user->getPHID() => $user->getPHID()),
+ );
+
+ $xactions = array();
+ $xactions[] = id(new PhabricatorProjectTransaction())
+ ->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
+ ->setMetadataValue('edge:type', PhabricatorEdgeConfig::TYPE_PROJ_MEMBER)
+ ->setNewValue($spec);
+
+ $editor = id(new PhabricatorProjectTransactionEditor())
+ ->setActor($user)
+ ->setContentSource(PhabricatorContentSource::newConsoleSource())
+ ->setContinueOnNoEffect(true)
+ ->applyTransactions($project, $xactions);
+ }
+
}

File Metadata

Mime Type
text/plain
Expires
Tue, Oct 22, 10:53 AM (4 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6728729
Default Alt Text
D8167.id18478.diff (17 KB)

Event Timeline

Phabricator · Built by Phacility