diff --git a/resources/sql/autopatches/20141119.differential.diff.policy.sql b/resources/sql/autopatches/20141119.differential.diff.policy.sql new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20141119.differential.diff.policy.sql @@ -0,0 +1,5 @@ +ALTER TABLE {$NAMESPACE}_differential.differential_diff + ADD viewPolicy VARBINARY(64) NOT NULL; + +UPDATE {$NAMESPACE}_differential.differential_diff + SET viewPolicy = 'users' WHERE viewPolicy = ''; diff --git a/src/applications/differential/__tests__/DifferentialParseRenderTestCase.php b/src/applications/differential/__tests__/DifferentialParseRenderTestCase.php --- a/src/applications/differential/__tests__/DifferentialParseRenderTestCase.php +++ b/src/applications/differential/__tests__/DifferentialParseRenderTestCase.php @@ -35,7 +35,9 @@ $parser = new ArcanistDiffParser(); $changes = $parser->parseDiff($data); - $diff = DifferentialDiff::newFromRawChanges($changes); + $diff = DifferentialDiff::newFromRawChanges( + PhabricatorUser::getOmnipotentUser(), + $changes); if (count($diff->getChangesets()) !== 1) { throw new Exception("Expected one changeset: {$file}"); } diff --git a/src/applications/differential/conduit/DifferentialCreateDiffConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialCreateDiffConduitAPIMethod.php --- a/src/applications/differential/conduit/DifferentialCreateDiffConduitAPIMethod.php +++ b/src/applications/differential/conduit/DifferentialCreateDiffConduitAPIMethod.php @@ -69,7 +69,7 @@ $changes[] = ArcanistDiffChange::newFromDictionary($dict); } - $diff = DifferentialDiff::newFromRawChanges($changes); + $diff = DifferentialDiff::newFromRawChanges($viewer, $changes); // TODO: Remove repository UUID eventually; for now continue writing // the UUID. Note that we'll overwrite it below if we identify a diff --git a/src/applications/differential/conduit/DifferentialCreateRawDiffConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialCreateRawDiffConduitAPIMethod.php --- a/src/applications/differential/conduit/DifferentialCreateRawDiffConduitAPIMethod.php +++ b/src/applications/differential/conduit/DifferentialCreateRawDiffConduitAPIMethod.php @@ -15,6 +15,7 @@ return array( 'diff' => 'required string', 'repositoryPHID' => 'optional string', + 'viewPolicy' => 'optional string', ); } @@ -45,7 +46,7 @@ $parser = new ArcanistDiffParser(); $changes = $parser->parseDiff($raw_diff); - $diff = DifferentialDiff::newFromRawChanges($changes); + $diff = DifferentialDiff::newFromRawChanges($viewer, $changes); $diff_data_dict = array( 'creationMethod' => 'web', @@ -58,6 +59,12 @@ ->setTransactionType(DifferentialDiffTransaction::TYPE_DIFF_CREATE) ->setNewValue($diff_data_dict),); + if ($request->getValue('viewPolicy')) { + $xactions[] = id(new DifferentialTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_VIEW_POLICY) + ->setNewValue($request->getValue('viewPolicy')); + } + id(new DifferentialDiffEditor()) ->setActor($viewer) ->setContentSourceFromConduitRequest($request) diff --git a/src/applications/differential/controller/DifferentialDiffCreateController.php b/src/applications/differential/controller/DifferentialDiffCreateController.php --- a/src/applications/differential/controller/DifferentialDiffCreateController.php +++ b/src/applications/differential/controller/DifferentialDiffCreateController.php @@ -5,8 +5,11 @@ public function processRequest() { $request = $this->getRequest(); + $viewer = $request->getUser(); $diff = null; + // This object is just for policy stuff + $diff_object = DifferentialDiff::initializeNewDiff($viewer); $repository_phid = null; $repository_value = array(); $errors = array(); @@ -41,8 +44,9 @@ 'differential.createrawdiff', array( 'diff' => $diff, - 'repositoryPHID' => $repository_phid,)); - $call->setUser($request->getUser()); + 'repositoryPHID' => $repository_phid, + 'viewPolicy' => $request->getStr('viewPolicy'),)); + $call->setUser($viewer); $result = $call->execute(); $path = id(new PhutilURI($result['uri']))->getPath(); return id(new AphrontRedirectResponse())->setURI($path); @@ -68,10 +72,15 @@ $repository_value = $this->loadViewerHandles(array($repository_phid)); } + $policies = id(new PhabricatorPolicyQuery()) + ->setViewer($viewer) + ->setObject($diff_object) + ->execute(); + $form ->setAction('/differential/diff/create/') ->setEncType('multipart/form-data') - ->setUser($request->getUser()) + ->setUser($viewer) ->appendInstructions( pht( 'The best way to create a Differential diff is by using %s, but you '. @@ -101,6 +110,13 @@ ->setValue($repository_value) ->setLimit(1)) ->appendChild( + id(new AphrontFormPolicyControl()) + ->setUser($viewer) + ->setName('viewPolicy') + ->setPolicyObject($diff_object) + ->setPolicies($policies) + ->setCapability(PhabricatorPolicyCapability::CAN_VIEW)) + ->appendChild( id(new AphrontFormSubmitControl()) ->addCancelButton($cancel_uri) ->setValue(pht('Create Diff'))); diff --git a/src/applications/differential/controller/DifferentialRevisionEditController.php b/src/applications/differential/controller/DifferentialRevisionEditController.php --- a/src/applications/differential/controller/DifferentialRevisionEditController.php +++ b/src/applications/differential/controller/DifferentialRevisionEditController.php @@ -79,6 +79,13 @@ if ($repository_field) { $repository_field->setValue($request->getStr($repo_key)); } + $view_policy_key = id(new DifferentialViewPolicyField())->getFieldKey(); + $view_policy_field = idx( + $field_list->getFields(), + $view_policy_key); + if ($view_policy_field) { + $view_policy_field->setValue($diff->getViewPolicy()); + } } $validation_exception = null; diff --git a/src/applications/differential/editor/DifferentialDiffEditor.php b/src/applications/differential/editor/DifferentialDiffEditor.php --- a/src/applications/differential/editor/DifferentialDiffEditor.php +++ b/src/applications/differential/editor/DifferentialDiffEditor.php @@ -22,6 +22,7 @@ public function getTransactionTypes() { $types = parent::getTransactionTypes(); + $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY; $types[] = DifferentialDiffTransaction::TYPE_DIFF_CREATE; return $types; @@ -61,6 +62,9 @@ $dict = $this->diffDataDict; $this->updateDiffFromDict($object, $dict); return; + case PhabricatorTransactions::TYPE_VIEW_POLICY: + $object->setViewPolicy($xaction->getNewValue()); + return; } return parent::applyCustomInternalTransaction($object, $xaction); @@ -72,6 +76,7 @@ switch ($xaction->getTransactionType()) { case DifferentialDiffTransaction::TYPE_DIFF_CREATE: + case PhabricatorTransactions::TYPE_VIEW_POLICY: return; } diff --git a/src/applications/differential/parser/__tests__/DifferentialHunkParserTestCase.php b/src/applications/differential/parser/__tests__/DifferentialHunkParserTestCase.php --- a/src/applications/differential/parser/__tests__/DifferentialHunkParserTestCase.php +++ b/src/applications/differential/parser/__tests__/DifferentialHunkParserTestCase.php @@ -40,7 +40,9 @@ throw new Exception("Expected 1 changeset for '{$name}'!"); } - $diff = DifferentialDiff::newFromRawChanges($changes); + $diff = DifferentialDiff::newFromRawChanges( + PhabricatorUser::getOmnipotentUser(), + $changes); return head($diff->getChangesets())->getHunks(); } diff --git a/src/applications/differential/query/DifferentialDiffQuery.php b/src/applications/differential/query/DifferentialDiffQuery.php --- a/src/applications/differential/query/DifferentialDiffQuery.php +++ b/src/applications/differential/query/DifferentialDiffQuery.php @@ -62,7 +62,6 @@ foreach ($diffs as $key => $diff) { if (!$diff->getRevisionID()) { - $diff->attachRevision(null); continue; } diff --git a/src/applications/differential/storage/DifferentialDiff.php b/src/applications/differential/storage/DifferentialDiff.php --- a/src/applications/differential/storage/DifferentialDiff.php +++ b/src/applications/differential/storage/DifferentialDiff.php @@ -33,6 +33,8 @@ protected $description; + protected $viewPolicy; + private $unsavedChangesets = array(); private $changesets = self::ATTACHABLE; private $arcanistProject = self::ATTACHABLE; @@ -136,10 +138,27 @@ return $ret; } - public static function newFromRawChanges(array $changes) { + public static function initializeNewDiff(PhabricatorUser $actor) { + $app = id(new PhabricatorApplicationQuery()) + ->setViewer($actor) + ->withClasses(array('PhabricatorDifferentialApplication')) + ->executeOne(); + $view_policy = $app->getPolicy( + DifferentialDefaultViewCapability::CAPABILITY); + + $diff = id(new DifferentialDiff()) + ->setViewPolicy($view_policy); + + return $diff; + } + + public static function newFromRawChanges( + PhabricatorUser $actor, + array $changes) { + assert_instances_of($changes, 'ArcanistDiffChange'); - $diff = new DifferentialDiff(); + $diff = self::initializeNewDiff($actor); // There may not be any changes; initialize the changesets list so that // we don't throw later when accessing it. $diff->attachChangesets(array()); @@ -289,6 +308,10 @@ return $changes; } + public function hasRevision() { + return $this->revision !== self::ATTACHABLE; + } + public function getRevision() { return $this->assertAttached($this->revision); } @@ -318,27 +341,27 @@ } public function getPolicy($capability) { - if ($this->getRevision()) { + if ($this->hasRevision()) { return $this->getRevision()->getPolicy($capability); } - return PhabricatorPolicies::POLICY_USER; + return $this->viewPolicy; } public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { - if ($this->getRevision()) { + if ($this->hasRevision()) { return $this->getRevision()->hasAutomaticCapability($capability, $viewer); } - return false; + return ($this->getAuthorPHID() == $viewer->getPhid()); } public function describeAutomaticCapability($capability) { - if ($this->getRevision()) { + if ($this->hasRevision()) { return pht( 'This diff is attached to a revision, and inherits its policies.'); } - return null; + return pht('The author of a diff can see it.'); } diff --git a/src/applications/differential/storage/__tests__/DifferentialDiffTestCase.php b/src/applications/differential/storage/__tests__/DifferentialDiffTestCase.php --- a/src/applications/differential/storage/__tests__/DifferentialDiffTestCase.php +++ b/src/applications/differential/storage/__tests__/DifferentialDiffTestCase.php @@ -7,6 +7,7 @@ $parser = new ArcanistDiffParser(); $diff = DifferentialDiff::newFromRawChanges( + PhabricatorUser::getOmnipotentUser(), $parser->parseDiff(Filesystem::readFile($root.'lint_engine.diff'))); $copies = idx(head($diff->getChangesets())->getMetadata(), 'copy:lines'); @@ -46,7 +47,9 @@ {$oblock} EODIFF; - $diff = DifferentialDiff::newFromRawChanges($parser->parseDiff($raw_diff)); + $diff = DifferentialDiff::newFromRawChanges( + PhabricatorUser::getOmnipotentUser(), + $parser->parseDiff($raw_diff)); $this->assertTrue(true); } diff --git a/src/applications/diffusion/controller/DiffusionChangeController.php b/src/applications/diffusion/controller/DiffusionChangeController.php --- a/src/applications/diffusion/controller/DiffusionChangeController.php +++ b/src/applications/diffusion/controller/DiffusionChangeController.php @@ -22,7 +22,9 @@ $drequest->updateSymbolicCommit($data['effectiveCommit']); $raw_changes = ArcanistDiffChange::newFromConduit($data['changes']); - $diff = DifferentialDiff::newFromRawChanges($raw_changes); + $diff = DifferentialDiff::newFromRawChanges( + $viewer, + $raw_changes); $changesets = $diff->getChangesets(); $changeset = reset($changesets); diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -276,6 +276,7 @@ $content[] = $change_panel; $changesets = DiffusionPathChange::convertToDifferentialChangesets( + $user, $changes); $vcs = $repository->getVersionControlSystem(); diff --git a/src/applications/diffusion/controller/DiffusionDiffController.php b/src/applications/diffusion/controller/DiffusionDiffController.php --- a/src/applications/diffusion/controller/DiffusionDiffController.php +++ b/src/applications/diffusion/controller/DiffusionDiffController.php @@ -54,7 +54,9 @@ )); $drequest->updateSymbolicCommit($data['effectiveCommit']); $raw_changes = ArcanistDiffChange::newFromConduit($data['changes']); - $diff = DifferentialDiff::newFromRawChanges($raw_changes); + $diff = DifferentialDiff::newFromRawChanges( + $user, + $raw_changes); $changesets = $diff->getChangesets(); $changeset = reset($changesets); diff --git a/src/applications/diffusion/data/DiffusionPathChange.php b/src/applications/diffusion/data/DiffusionPathChange.php --- a/src/applications/diffusion/data/DiffusionPathChange.php +++ b/src/applications/diffusion/data/DiffusionPathChange.php @@ -142,10 +142,12 @@ return array_select_keys($result, $direct); } - final public static function convertToDifferentialChangesets(array $changes) { + final public static function convertToDifferentialChangesets( + PhabricatorUser $user, + array $changes) { assert_instances_of($changes, 'DiffusionPathChange'); $arcanist_changes = self::convertToArcanistChanges($changes); - $diff = DifferentialDiff::newFromRawChanges($arcanist_changes); + $diff = DifferentialDiff::newFromRawChanges($user, $arcanist_changes); return $diff->getChangesets(); } diff --git a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php --- a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php +++ b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php @@ -1115,7 +1115,9 @@ $parser = new ArcanistDiffParser(); $changes = $parser->parseDiff($raw_diff); - $diff = DifferentialDiff::newFromRawChanges($changes); + $diff = DifferentialDiff::newFromRawChanges( + $this->getViewer(), + $changes); return $diff->getChangesets(); } diff --git a/src/applications/herald/adapter/HeraldCommitAdapter.php b/src/applications/herald/adapter/HeraldCommitAdapter.php --- a/src/applications/herald/adapter/HeraldCommitAdapter.php +++ b/src/applications/herald/adapter/HeraldCommitAdapter.php @@ -346,7 +346,9 @@ $parser = new ArcanistDiffParser(); $changes = $parser->parseDiff($raw); - $diff = DifferentialDiff::newFromRawChanges($changes); + $diff = DifferentialDiff::newFromRawChanges( + PhabricatorUser::getOmnipotentUser(), + $changes); return $diff; } diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php --- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php +++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php @@ -265,7 +265,7 @@ $changes = array(); } - $diff = DifferentialDiff::newFromRawChanges($changes) + $diff = DifferentialDiff::newFromRawChanges($viewer, $changes) ->setRepositoryPHID($this->repository->getPHID()) ->setAuthorPHID($actor_phid) ->setCreationMethod('commit') diff --git a/src/infrastructure/diff/PhabricatorDifferenceEngine.php b/src/infrastructure/diff/PhabricatorDifferenceEngine.php --- a/src/infrastructure/diff/PhabricatorDifferenceEngine.php +++ b/src/infrastructure/diff/PhabricatorDifferenceEngine.php @@ -163,7 +163,9 @@ $diff = $this->generateRawDiffFromFileContent($old, $new); $changes = id(new ArcanistDiffParser())->parseDiff($diff); - $diff = DifferentialDiff::newFromRawChanges($changes); + $diff = DifferentialDiff::newFromRawChanges( + PhabricatorUser::getOmnipotentUser(), + $changes); return head($diff->getChangesets()); }