From 8247edff98e86515dc7da5f1046eb8743bad6b97 Mon Sep 17 00:00:00 2001 From: Mike Riley Date: Thu, 13 Oct 2016 21:07:02 +0000 Subject: [PATCH] Modularize Owners package transactions Summary: Converts Owners package transactions to modular transactions. Test Plan: - created a new package - edited all simple properties from the web ui - checked that project and user owners were added as reviewers appropriately to new diffs - inspected the change details for various types of path add / remove / update / reorder changes Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin, epriestley Differential Revision: https://secure.phabricator.com/D16651 --- src/__phutil_library_map__.php | 22 +- .../PhabricatorOwnersArchiveController.php | 3 +- .../PhabricatorOwnersPathsController.php | 2 +- .../PhabricatorOwnersPackageEditEngine.php | 21 +- ...bricatorOwnersPackageTransactionEditor.php | 342 ------------------ .../PhabricatorOwnersPackageTransaction.php | 254 +------------ ...icatorOwnersPackageAuditingTransaction.php | 32 ++ ...atorOwnersPackageAutoreviewTransaction.php | 56 +++ ...torOwnersPackageDescriptionTransaction.php | 29 ++ ...icatorOwnersPackageDominionTransaction.php | 56 +++ ...habricatorOwnersPackageNameTransaction.php | 52 +++ ...bricatorOwnersPackageOwnersTransaction.php | 76 ++++ ...abricatorOwnersPackagePathsTransaction.php | 189 ++++++++++ ...ricatorOwnersPackagePrimaryTransaction.php | 15 + ...bricatorOwnersPackageStatusTransaction.php | 29 ++ ...habricatorOwnersPackageTransactionType.php | 4 + 16 files changed, 578 insertions(+), 604 deletions(-) create mode 100644 src/applications/owners/xaction/PhabricatorOwnersPackageAuditingTransaction.php create mode 100644 src/applications/owners/xaction/PhabricatorOwnersPackageAutoreviewTransaction.php create mode 100644 src/applications/owners/xaction/PhabricatorOwnersPackageDescriptionTransaction.php create mode 100644 src/applications/owners/xaction/PhabricatorOwnersPackageDominionTransaction.php create mode 100644 src/applications/owners/xaction/PhabricatorOwnersPackageNameTransaction.php create mode 100644 src/applications/owners/xaction/PhabricatorOwnersPackageOwnersTransaction.php create mode 100644 src/applications/owners/xaction/PhabricatorOwnersPackagePathsTransaction.php create mode 100644 src/applications/owners/xaction/PhabricatorOwnersPackagePrimaryTransaction.php create mode 100644 src/applications/owners/xaction/PhabricatorOwnersPackageStatusTransaction.php create mode 100644 src/applications/owners/xaction/PhabricatorOwnersPackageTransactionType.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 5fee27238f..34d379c376 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3017,20 +3017,30 @@ phutil_register_library_map(array( 'PhabricatorOwnersListController' => 'applications/owners/controller/PhabricatorOwnersListController.php', 'PhabricatorOwnersOwner' => 'applications/owners/storage/PhabricatorOwnersOwner.php', 'PhabricatorOwnersPackage' => 'applications/owners/storage/PhabricatorOwnersPackage.php', + 'PhabricatorOwnersPackageAuditingTransaction' => 'applications/owners/xaction/PhabricatorOwnersPackageAuditingTransaction.php', + 'PhabricatorOwnersPackageAutoreviewTransaction' => 'applications/owners/xaction/PhabricatorOwnersPackageAutoreviewTransaction.php', 'PhabricatorOwnersPackageDatasource' => 'applications/owners/typeahead/PhabricatorOwnersPackageDatasource.php', + 'PhabricatorOwnersPackageDescriptionTransaction' => 'applications/owners/xaction/PhabricatorOwnersPackageDescriptionTransaction.php', + 'PhabricatorOwnersPackageDominionTransaction' => 'applications/owners/xaction/PhabricatorOwnersPackageDominionTransaction.php', 'PhabricatorOwnersPackageEditEngine' => 'applications/owners/editor/PhabricatorOwnersPackageEditEngine.php', 'PhabricatorOwnersPackageFulltextEngine' => 'applications/owners/query/PhabricatorOwnersPackageFulltextEngine.php', 'PhabricatorOwnersPackageFunctionDatasource' => 'applications/owners/typeahead/PhabricatorOwnersPackageFunctionDatasource.php', 'PhabricatorOwnersPackageNameNgrams' => 'applications/owners/storage/PhabricatorOwnersPackageNameNgrams.php', + 'PhabricatorOwnersPackageNameTransaction' => 'applications/owners/xaction/PhabricatorOwnersPackageNameTransaction.php', 'PhabricatorOwnersPackageOwnerDatasource' => 'applications/owners/typeahead/PhabricatorOwnersPackageOwnerDatasource.php', + 'PhabricatorOwnersPackageOwnersTransaction' => 'applications/owners/xaction/PhabricatorOwnersPackageOwnersTransaction.php', 'PhabricatorOwnersPackagePHIDType' => 'applications/owners/phid/PhabricatorOwnersPackagePHIDType.php', + 'PhabricatorOwnersPackagePathsTransaction' => 'applications/owners/xaction/PhabricatorOwnersPackagePathsTransaction.php', + 'PhabricatorOwnersPackagePrimaryTransaction' => 'applications/owners/xaction/PhabricatorOwnersPackagePrimaryTransaction.php', 'PhabricatorOwnersPackageQuery' => 'applications/owners/query/PhabricatorOwnersPackageQuery.php', 'PhabricatorOwnersPackageRemarkupRule' => 'applications/owners/remarkup/PhabricatorOwnersPackageRemarkupRule.php', 'PhabricatorOwnersPackageSearchEngine' => 'applications/owners/query/PhabricatorOwnersPackageSearchEngine.php', + 'PhabricatorOwnersPackageStatusTransaction' => 'applications/owners/xaction/PhabricatorOwnersPackageStatusTransaction.php', 'PhabricatorOwnersPackageTestCase' => 'applications/owners/storage/__tests__/PhabricatorOwnersPackageTestCase.php', 'PhabricatorOwnersPackageTransaction' => 'applications/owners/storage/PhabricatorOwnersPackageTransaction.php', 'PhabricatorOwnersPackageTransactionEditor' => 'applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php', 'PhabricatorOwnersPackageTransactionQuery' => 'applications/owners/query/PhabricatorOwnersPackageTransactionQuery.php', + 'PhabricatorOwnersPackageTransactionType' => 'applications/owners/xaction/PhabricatorOwnersPackageTransactionType.php', 'PhabricatorOwnersPath' => 'applications/owners/storage/PhabricatorOwnersPath.php', 'PhabricatorOwnersPathsController' => 'applications/owners/controller/PhabricatorOwnersPathsController.php', 'PhabricatorOwnersPathsSearchEngineAttachment' => 'applications/owners/engineextension/PhabricatorOwnersPathsSearchEngineAttachment.php', @@ -7949,20 +7959,30 @@ phutil_register_library_map(array( 'PhabricatorFulltextInterface', 'PhabricatorNgramsInterface', ), + 'PhabricatorOwnersPackageAuditingTransaction' => 'PhabricatorOwnersPackageTransactionType', + 'PhabricatorOwnersPackageAutoreviewTransaction' => 'PhabricatorOwnersPackageTransactionType', 'PhabricatorOwnersPackageDatasource' => 'PhabricatorTypeaheadDatasource', + 'PhabricatorOwnersPackageDescriptionTransaction' => 'PhabricatorOwnersPackageTransactionType', + 'PhabricatorOwnersPackageDominionTransaction' => 'PhabricatorOwnersPackageTransactionType', 'PhabricatorOwnersPackageEditEngine' => 'PhabricatorEditEngine', 'PhabricatorOwnersPackageFulltextEngine' => 'PhabricatorFulltextEngine', 'PhabricatorOwnersPackageFunctionDatasource' => 'PhabricatorTypeaheadCompositeDatasource', 'PhabricatorOwnersPackageNameNgrams' => 'PhabricatorSearchNgrams', + 'PhabricatorOwnersPackageNameTransaction' => 'PhabricatorOwnersPackageTransactionType', 'PhabricatorOwnersPackageOwnerDatasource' => 'PhabricatorTypeaheadCompositeDatasource', + 'PhabricatorOwnersPackageOwnersTransaction' => 'PhabricatorOwnersPackageTransactionType', 'PhabricatorOwnersPackagePHIDType' => 'PhabricatorPHIDType', + 'PhabricatorOwnersPackagePathsTransaction' => 'PhabricatorOwnersPackageTransactionType', + 'PhabricatorOwnersPackagePrimaryTransaction' => 'PhabricatorOwnersPackageTransactionType', 'PhabricatorOwnersPackageQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorOwnersPackageRemarkupRule' => 'PhabricatorObjectRemarkupRule', 'PhabricatorOwnersPackageSearchEngine' => 'PhabricatorApplicationSearchEngine', + 'PhabricatorOwnersPackageStatusTransaction' => 'PhabricatorOwnersPackageTransactionType', 'PhabricatorOwnersPackageTestCase' => 'PhabricatorTestCase', - 'PhabricatorOwnersPackageTransaction' => 'PhabricatorApplicationTransaction', + 'PhabricatorOwnersPackageTransaction' => 'PhabricatorModularTransaction', 'PhabricatorOwnersPackageTransactionEditor' => 'PhabricatorApplicationTransactionEditor', 'PhabricatorOwnersPackageTransactionQuery' => 'PhabricatorApplicationTransactionQuery', + 'PhabricatorOwnersPackageTransactionType' => 'PhabricatorModularTransactionType', 'PhabricatorOwnersPath' => 'PhabricatorOwnersDAO', 'PhabricatorOwnersPathsController' => 'PhabricatorOwnersController', 'PhabricatorOwnersPathsSearchEngineAttachment' => 'PhabricatorSearchEngineAttachment', diff --git a/src/applications/owners/controller/PhabricatorOwnersArchiveController.php b/src/applications/owners/controller/PhabricatorOwnersArchiveController.php index 2ef618a2d5..47f9d87d23 100644 --- a/src/applications/owners/controller/PhabricatorOwnersArchiveController.php +++ b/src/applications/owners/controller/PhabricatorOwnersArchiveController.php @@ -31,8 +31,9 @@ final class PhabricatorOwnersArchiveController $xactions = array(); + $type = PhabricatorOwnersPackageStatusTransaction::TRANSACTIONTYPE; $xactions[] = id(new PhabricatorOwnersPackageTransaction()) - ->setTransactionType(PhabricatorOwnersPackageTransaction::TYPE_STATUS) + ->setTransactionType($type) ->setNewValue($new_status); id(new PhabricatorOwnersPackageTransactionEditor()) diff --git a/src/applications/owners/controller/PhabricatorOwnersPathsController.php b/src/applications/owners/controller/PhabricatorOwnersPathsController.php index 7f911d29d9..b69faf5648 100644 --- a/src/applications/owners/controller/PhabricatorOwnersPathsController.php +++ b/src/applications/owners/controller/PhabricatorOwnersPathsController.php @@ -48,7 +48,7 @@ final class PhabricatorOwnersPathsController ); } - $type_paths = PhabricatorOwnersPackageTransaction::TYPE_PATHS; + $type_paths = PhabricatorOwnersPackagePathsTransaction::TRANSACTIONTYPE; $xactions = array(); $xactions[] = id(new PhabricatorOwnersPackageTransaction()) diff --git a/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php b/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php index b20613c92e..4adaebc582 100644 --- a/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php +++ b/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php @@ -95,14 +95,16 @@ EOTEXT ->setKey('name') ->setLabel(pht('Name')) ->setDescription(pht('Name of the package.')) - ->setTransactionType(PhabricatorOwnersPackageTransaction::TYPE_NAME) + ->setTransactionType( + PhabricatorOwnersPackageNameTransaction::TRANSACTIONTYPE) ->setIsRequired(true) ->setValue($object->getName()), id(new PhabricatorDatasourceEditField()) ->setKey('owners') ->setLabel(pht('Owners')) ->setDescription(pht('Users and projects which own the package.')) - ->setTransactionType(PhabricatorOwnersPackageTransaction::TYPE_OWNERS) + ->setTransactionType( + PhabricatorOwnersPackageOwnersTransaction::TRANSACTIONTYPE) ->setDatasource(new PhabricatorProjectOrUserDatasource()) ->setIsCopyable(true) ->setValue($object->getOwnerPHIDs()), @@ -112,7 +114,7 @@ EOTEXT ->setDescription( pht('Change package dominion rules.')) ->setTransactionType( - PhabricatorOwnersPackageTransaction::TYPE_DOMINION) + PhabricatorOwnersPackageDominionTransaction::TRANSACTIONTYPE) ->setIsCopyable(true) ->setValue($object->getDominion()) ->setOptions($dominion_map), @@ -124,7 +126,7 @@ EOTEXT 'Automatically trigger reviews for commits affecting files in '. 'this package.')) ->setTransactionType( - PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW) + PhabricatorOwnersPackageAutoreviewTransaction::TRANSACTIONTYPE) ->setIsCopyable(true) ->setValue($object->getAutoReview()) ->setOptions($autoreview_map), @@ -135,7 +137,8 @@ EOTEXT pht( 'Automatically trigger audits for commits affecting files in '. 'this package.')) - ->setTransactionType(PhabricatorOwnersPackageTransaction::TYPE_AUDITING) + ->setTransactionType( + PhabricatorOwnersPackageAuditingTransaction::TRANSACTIONTYPE) ->setIsCopyable(true) ->setValue($object->getAuditingEnabled()) ->setOptions( @@ -148,13 +151,14 @@ EOTEXT ->setLabel(pht('Description')) ->setDescription(pht('Human-readable description of the package.')) ->setTransactionType( - PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION) + PhabricatorOwnersPackageDescriptionTransaction::TRANSACTIONTYPE) ->setValue($object->getDescription()), id(new PhabricatorSelectEditField()) ->setKey('status') ->setLabel(pht('Status')) ->setDescription(pht('Archive or enable the package.')) - ->setTransactionType(PhabricatorOwnersPackageTransaction::TYPE_STATUS) + ->setTransactionType( + PhabricatorOwnersPackageStatusTransaction::TRANSACTIONTYPE) ->setIsConduitOnly(true) ->setValue($object->getStatus()) ->setOptions($object->getStatusNameMap()), @@ -162,7 +166,8 @@ EOTEXT ->setKey('paths.set') ->setLabel(pht('Paths')) ->setIsConduitOnly(true) - ->setTransactionType(PhabricatorOwnersPackageTransaction::TYPE_PATHS) + ->setTransactionType( + PhabricatorOwnersPackagePathsTransaction::TRANSACTIONTYPE) ->setConduitDescription( pht('Overwrite existing package paths with new paths.')) ->setConduitTypeDescription( diff --git a/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php b/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php index 5fb4a9af8a..40657abd57 100644 --- a/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php +++ b/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php @@ -14,354 +14,12 @@ final class PhabricatorOwnersPackageTransactionEditor public function getTransactionTypes() { $types = parent::getTransactionTypes(); - $types[] = PhabricatorOwnersPackageTransaction::TYPE_NAME; - $types[] = PhabricatorOwnersPackageTransaction::TYPE_OWNERS; - $types[] = PhabricatorOwnersPackageTransaction::TYPE_AUDITING; - $types[] = PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION; - $types[] = PhabricatorOwnersPackageTransaction::TYPE_PATHS; - $types[] = PhabricatorOwnersPackageTransaction::TYPE_STATUS; - $types[] = PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW; - $types[] = PhabricatorOwnersPackageTransaction::TYPE_DOMINION; - $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY; $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY; return $types; } - protected function getCustomTransactionOldValue( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PhabricatorOwnersPackageTransaction::TYPE_NAME: - return $object->getName(); - case PhabricatorOwnersPackageTransaction::TYPE_OWNERS: - $phids = mpull($object->getOwners(), 'getUserPHID'); - $phids = array_values($phids); - return $phids; - case PhabricatorOwnersPackageTransaction::TYPE_AUDITING: - return (int)$object->getAuditingEnabled(); - case PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION: - return $object->getDescription(); - case PhabricatorOwnersPackageTransaction::TYPE_PATHS: - $paths = $object->getPaths(); - return mpull($paths, 'getRef'); - case PhabricatorOwnersPackageTransaction::TYPE_STATUS: - return $object->getStatus(); - case PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW: - return $object->getAutoReview(); - case PhabricatorOwnersPackageTransaction::TYPE_DOMINION: - return $object->getDominion(); - } - } - - protected function getCustomTransactionNewValue( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PhabricatorOwnersPackageTransaction::TYPE_NAME: - case PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION: - case PhabricatorOwnersPackageTransaction::TYPE_STATUS: - case PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW: - case PhabricatorOwnersPackageTransaction::TYPE_DOMINION: - return $xaction->getNewValue(); - case PhabricatorOwnersPackageTransaction::TYPE_PATHS: - $new = $xaction->getNewValue(); - foreach ($new as $key => $info) { - $new[$key]['excluded'] = (int)idx($info, 'excluded'); - } - return $new; - case PhabricatorOwnersPackageTransaction::TYPE_AUDITING: - return (int)$xaction->getNewValue(); - case PhabricatorOwnersPackageTransaction::TYPE_OWNERS: - $phids = $xaction->getNewValue(); - $phids = array_unique($phids); - $phids = array_values($phids); - return $phids; - } - } - - protected function transactionHasEffect( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PhabricatorOwnersPackageTransaction::TYPE_PATHS: - $old = $xaction->getOldValue(); - $new = $xaction->getNewValue(); - - $diffs = PhabricatorOwnersPath::getTransactionValueChanges($old, $new); - list($rem, $add) = $diffs; - - return ($rem || $add); - } - - return parent::transactionHasEffect($object, $xaction); - } - - protected function applyCustomInternalTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PhabricatorOwnersPackageTransaction::TYPE_NAME: - $object->setName($xaction->getNewValue()); - return; - case PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION: - $object->setDescription($xaction->getNewValue()); - return; - case PhabricatorOwnersPackageTransaction::TYPE_AUDITING: - $object->setAuditingEnabled($xaction->getNewValue()); - return; - case PhabricatorOwnersPackageTransaction::TYPE_OWNERS: - case PhabricatorOwnersPackageTransaction::TYPE_PATHS: - return; - case PhabricatorOwnersPackageTransaction::TYPE_STATUS: - $object->setStatus($xaction->getNewValue()); - return; - case PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW: - $object->setAutoReview($xaction->getNewValue()); - return; - case PhabricatorOwnersPackageTransaction::TYPE_DOMINION: - $object->setDominion($xaction->getNewValue()); - return; - } - - return parent::applyCustomInternalTransaction($object, $xaction); - } - - protected function applyCustomExternalTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PhabricatorOwnersPackageTransaction::TYPE_NAME: - case PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION: - case PhabricatorOwnersPackageTransaction::TYPE_AUDITING: - case PhabricatorOwnersPackageTransaction::TYPE_STATUS: - case PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW: - case PhabricatorOwnersPackageTransaction::TYPE_DOMINION: - return; - case PhabricatorOwnersPackageTransaction::TYPE_OWNERS: - $old = $xaction->getOldValue(); - $new = $xaction->getNewValue(); - - $owners = $object->getOwners(); - $owners = mpull($owners, null, 'getUserPHID'); - - $rem = array_diff($old, $new); - foreach ($rem as $phid) { - if (isset($owners[$phid])) { - $owners[$phid]->delete(); - unset($owners[$phid]); - } - } - - $add = array_diff($new, $old); - foreach ($add as $phid) { - $owners[$phid] = id(new PhabricatorOwnersOwner()) - ->setPackageID($object->getID()) - ->setUserPHID($phid) - ->save(); - } - - // TODO: Attach owners here - return; - case PhabricatorOwnersPackageTransaction::TYPE_PATHS: - $old = $xaction->getOldValue(); - $new = $xaction->getNewValue(); - - $paths = $object->getPaths(); - - $diffs = PhabricatorOwnersPath::getTransactionValueChanges($old, $new); - list($rem, $add) = $diffs; - - $set = PhabricatorOwnersPath::getSetFromTransactionValue($rem); - foreach ($paths as $path) { - $ref = $path->getRef(); - if (PhabricatorOwnersPath::isRefInSet($ref, $set)) { - $path->delete(); - } - } - - foreach ($add as $ref) { - $path = PhabricatorOwnersPath::newFromRef($ref) - ->setPackageID($object->getID()) - ->save(); - } - - return; - } - - return parent::applyCustomExternalTransaction($object, $xaction); - } - - protected function validateTransaction( - PhabricatorLiskDAO $object, - $type, - array $xactions) { - - $errors = parent::validateTransaction($object, $type, $xactions); - - switch ($type) { - case PhabricatorOwnersPackageTransaction::TYPE_NAME: - $missing = $this->validateIsEmptyTextField( - $object->getName(), - $xactions); - - if ($missing) { - $error = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Required'), - pht('Package name is required.'), - nonempty(last($xactions), null)); - - $error->setIsMissingFieldError(true); - $errors[] = $error; - } - - foreach ($xactions as $xaction) { - $new = $xaction->getNewValue(); - if (preg_match('([,!])', $new)) { - $errors[] = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Invalid'), - pht( - 'Package names may not contain commas (",") or exclamation '. - 'marks ("!"). These characters are ambiguous when package '. - 'names are parsed from the command line.'), - $xaction); - } - } - - break; - case PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW: - $map = PhabricatorOwnersPackage::getAutoreviewOptionsMap(); - foreach ($xactions as $xaction) { - $new = $xaction->getNewValue(); - - if (empty($map[$new])) { - $valid = array_keys($map); - - $errors[] = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Invalid'), - pht( - 'Autoreview setting "%s" is not valid. '. - 'Valid settings are: %s.', - $new, - implode(', ', $valid)), - $xaction); - } - } - break; - case PhabricatorOwnersPackageTransaction::TYPE_DOMINION: - $map = PhabricatorOwnersPackage::getDominionOptionsMap(); - foreach ($xactions as $xaction) { - $new = $xaction->getNewValue(); - - if (empty($map[$new])) { - $valid = array_keys($map); - - $errors[] = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Invalid'), - pht( - 'Dominion setting "%s" is not valid. '. - 'Valid settings are: %s.', - $new, - implode(', ', $valid)), - $xaction); - } - } - break; - case PhabricatorOwnersPackageTransaction::TYPE_PATHS: - if (!$xactions) { - continue; - } - - $old = mpull($object->getPaths(), 'getRef'); - foreach ($xactions as $xaction) { - $new = $xaction->getNewValue(); - - // Check that we have a list of paths. - if (!is_array($new)) { - $errors[] = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Invalid'), - pht('Path specification must be a list of paths.'), - $xaction); - continue; - } - - // Check that each item in the list is formatted properly. - $type_exception = null; - foreach ($new as $key => $value) { - try { - PhutilTypeSpec::checkMap( - $value, - array( - 'repositoryPHID' => 'string', - 'path' => 'string', - 'excluded' => 'optional wild', - )); - } catch (PhutilTypeCheckException $ex) { - $errors[] = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Invalid'), - pht( - 'Path specification list contains invalid value '. - 'in key "%s": %s.', - $key, - $ex->getMessage()), - $xaction); - $type_exception = $ex; - } - } - - if ($type_exception) { - continue; - } - - // Check that any new paths reference legitimate repositories which - // the viewer has permission to see. - list($rem, $add) = PhabricatorOwnersPath::getTransactionValueChanges( - $old, - $new); - - if ($add) { - $repository_phids = ipull($add, 'repositoryPHID'); - - $repositories = id(new PhabricatorRepositoryQuery()) - ->setViewer($this->getActor()) - ->withPHIDs($repository_phids) - ->execute(); - $repositories = mpull($repositories, null, 'getPHID'); - - foreach ($add as $ref) { - $repository_phid = $ref['repositoryPHID']; - if (isset($repositories[$repository_phid])) { - continue; - } - - $errors[] = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Invalid'), - pht( - 'Path specification list references repository PHID "%s", '. - 'but that is not a valid, visible repository.', - $repository_phid)); - } - } - } - break; - } - - return $errors; - } - protected function shouldSendMail( PhabricatorLiskDAO $object, array $xactions) { diff --git a/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php b/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php index 359a1fcb8a..66e15b634a 100644 --- a/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php +++ b/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php @@ -1,17 +1,7 @@ getOldValue(); - $new = $this->getNewValue(); - - switch ($this->getTransactionType()) { - case self::TYPE_OWNERS: - if (!is_array($old)) { - $old = array(); - } - - if (!is_array($new)) { - $new = array(); - } - - $add = array_diff($new, $old); - foreach ($add as $phid) { - $phids[] = $phid; - } - $rem = array_diff($old, $new); - foreach ($rem as $phid) { - $phids[] = $phid; - } - break; - } - - return $phids; - } - - public function shouldHide() { - $old = $this->getOldValue(); - $new = $this->getNewValue(); - - switch ($this->getTransactionType()) { - case self::TYPE_DESCRIPTION: - if ($old === null) { - return true; - } - break; - case self::TYPE_PRIMARY: - // TODO: Eventually, remove these transactions entirely. - return true; - } - - return parent::shouldHide(); - } - - public function getTitle() { - $old = $this->getOldValue(); - $new = $this->getNewValue(); - $author_phid = $this->getAuthorPHID(); - - switch ($this->getTransactionType()) { - case PhabricatorTransactions::TYPE_CREATE: - return pht( - '%s created this package.', - $this->renderHandleLink($author_phid)); - case self::TYPE_NAME: - if ($old === null) { - return pht( - '%s created this package.', - $this->renderHandleLink($author_phid)); - } else { - return pht( - '%s renamed this package from "%s" to "%s".', - $this->renderHandleLink($author_phid), - $old, - $new); - } - case self::TYPE_OWNERS: - $add = array_diff($new, $old); - $rem = array_diff($old, $new); - if ($add && !$rem) { - return pht( - '%s added %s owner(s): %s.', - $this->renderHandleLink($author_phid), - count($add), - $this->renderHandleList($add)); - } else if ($rem && !$add) { - return pht( - '%s removed %s owner(s): %s.', - $this->renderHandleLink($author_phid), - count($rem), - $this->renderHandleList($rem)); - } else { - return pht( - '%s changed %s package owner(s), added %s: %s; removed %s: %s.', - $this->renderHandleLink($author_phid), - count($add) + count($rem), - count($add), - $this->renderHandleList($add), - count($rem), - $this->renderHandleList($rem)); - } - case self::TYPE_AUDITING: - if ($new) { - return pht( - '%s enabled auditing for this package.', - $this->renderHandleLink($author_phid)); - } else { - return pht( - '%s disabled auditing for this package.', - $this->renderHandleLink($author_phid)); - } - case self::TYPE_DESCRIPTION: - return pht( - '%s updated the description for this package.', - $this->renderHandleLink($author_phid)); - case self::TYPE_PATHS: - // TODO: Flesh this out. - return pht( - '%s updated paths for this package.', - $this->renderHandleLink($author_phid)); - case self::TYPE_STATUS: - if ($new == PhabricatorOwnersPackage::STATUS_ACTIVE) { - return pht( - '%s activated this package.', - $this->renderHandleLink($author_phid)); - } else if ($new == PhabricatorOwnersPackage::STATUS_ARCHIVED) { - return pht( - '%s archived this package.', - $this->renderHandleLink($author_phid)); - } - case self::TYPE_AUTOREVIEW: - $map = PhabricatorOwnersPackage::getAutoreviewOptionsMap(); - $map = ipull($map, 'name'); - - $old = idx($map, $old, $old); - $new = idx($map, $new, $new); - - return pht( - '%s adjusted autoreview from "%s" to "%s".', - $this->renderHandleLink($author_phid), - $old, - $new); - case self::TYPE_DOMINION: - $map = PhabricatorOwnersPackage::getDominionOptionsMap(); - $map = ipull($map, 'short'); - - $old = idx($map, $old, $old); - $new = idx($map, $new, $new); - - return pht( - '%s adjusted package dominion rules from "%s" to "%s".', - $this->renderHandleLink($author_phid), - $old, - $new); - } - - return parent::getTitle(); - } - - public function hasChangeDetails() { - switch ($this->getTransactionType()) { - case self::TYPE_DESCRIPTION: - return ($this->getOldValue() !== null); - case self::TYPE_PATHS: - return true; - } - - return parent::hasChangeDetails(); - } - - public function renderChangeDetails(PhabricatorUser $viewer) { - switch ($this->getTransactionType()) { - case self::TYPE_DESCRIPTION: - $old = $this->getOldValue(); - $new = $this->getNewValue(); - - return $this->renderTextCorpusChangeDetails( - $viewer, - $old, - $new); - case self::TYPE_PATHS: - $old = $this->getOldValue(); - $new = $this->getNewValue(); - - $diffs = PhabricatorOwnersPath::getTransactionValueChanges($old, $new); - list($rem, $add) = $diffs; - - $rows = array(); - foreach ($rem as $ref) { - $rows[] = array( - 'class' => 'diff-removed', - 'change' => '-', - ) + $ref; - } - - foreach ($add as $ref) { - $rows[] = array( - 'class' => 'diff-added', - 'change' => '+', - ) + $ref; - } - - $rowc = array(); - foreach ($rows as $key => $row) { - $rowc[] = $row['class']; - $rows[$key] = array( - $row['change'], - $row['excluded'] ? pht('Exclude') : pht('Include'), - $viewer->renderHandle($row['repositoryPHID']), - $row['path'], - ); - } - - $table = id(new AphrontTableView($rows)) - ->setRowClasses($rowc) - ->setHeaders( - array( - null, - pht('Type'), - pht('Repository'), - pht('Path'), - )) - ->setColumnClasses( - array( - null, - null, - null, - 'wide', - )); - - return $table; - } - - return parent::renderChangeDetails($viewer); - } - - public function getRemarkupBlocks() { - $blocks = parent::getRemarkupBlocks(); - - switch ($this->getTransactionType()) { - case self::TYPE_DESCRIPTION: - $blocks[] = $this->getNewValue(); - break; - } - - return $blocks; + public function getBaseTransactionClass() { + return 'PhabricatorOwnersPackageTransactionType'; } } diff --git a/src/applications/owners/xaction/PhabricatorOwnersPackageAuditingTransaction.php b/src/applications/owners/xaction/PhabricatorOwnersPackageAuditingTransaction.php new file mode 100644 index 0000000000..df4f0feb01 --- /dev/null +++ b/src/applications/owners/xaction/PhabricatorOwnersPackageAuditingTransaction.php @@ -0,0 +1,32 @@ +getAuditingEnabled(); + } + + public function generateNewValue($object, $value) { + return (int)$value; + } + + public function applyInternalEffects($object, $value) { + $object->setAuditingEnabled($value); + } + + public function getTitle() { + if ($this->getNewValue()) { + return pht( + '%s enabled auditing for this package.', + $this->renderAuthor()); + } else { + return pht( + '%s disabled auditing for this package.', + $this->renderAuthor()); + } + } + +} diff --git a/src/applications/owners/xaction/PhabricatorOwnersPackageAutoreviewTransaction.php b/src/applications/owners/xaction/PhabricatorOwnersPackageAutoreviewTransaction.php new file mode 100644 index 0000000000..03ec4f7af1 --- /dev/null +++ b/src/applications/owners/xaction/PhabricatorOwnersPackageAutoreviewTransaction.php @@ -0,0 +1,56 @@ +getAutoReview(); + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + + $map = PhabricatorOwnersPackage::getAutoreviewOptionsMap(); + foreach ($xactions as $xaction) { + $new = $xaction->getNewValue(); + + if (empty($map[$new])) { + $valid = array_keys($map); + + $errors[] = $this->newInvalidError( + pht( + 'Autoreview setting "%s" is not valid. '. + 'Valid settings are: %s.', + $new, + implode(', ', $valid)), + $xaction); + } + } + + return $errors; + } + + public function applyInternalEffects($object, $value) { + $object->setAutoReview($value); + } + + public function getTitle() { + $map = PhabricatorOwnersPackage::getAutoreviewOptionsMap(); + $map = ipull($map, 'name'); + + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + $old = idx($map, $old, $old); + $new = idx($map, $new, $new); + + return pht( + '%s adjusted autoreview from %s to %s.', + $this->renderAuthor(), + $this->renderValue($old), + $this->renderValue($new)); + } + +} diff --git a/src/applications/owners/xaction/PhabricatorOwnersPackageDescriptionTransaction.php b/src/applications/owners/xaction/PhabricatorOwnersPackageDescriptionTransaction.php new file mode 100644 index 0000000000..8b2effe80c --- /dev/null +++ b/src/applications/owners/xaction/PhabricatorOwnersPackageDescriptionTransaction.php @@ -0,0 +1,29 @@ +getDescription(); + } + + public function applyInternalEffects($object, $value) { + $object->setDescription($value); + } + + public function getTitle() { + return pht( + '%s updated the description for this package.', + $this->renderAuthor()); + } + + public function newChangeDetailView() { + return id(new PhabricatorApplicationTransactionTextDiffDetailView()) + ->setViewer($this->getViewer()) + ->setOldText($this->getOldValue()) + ->setNewText($this->getNewValue()); + } + +} diff --git a/src/applications/owners/xaction/PhabricatorOwnersPackageDominionTransaction.php b/src/applications/owners/xaction/PhabricatorOwnersPackageDominionTransaction.php new file mode 100644 index 0000000000..0c1ff0a916 --- /dev/null +++ b/src/applications/owners/xaction/PhabricatorOwnersPackageDominionTransaction.php @@ -0,0 +1,56 @@ +getDominion(); + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + + $map = PhabricatorOwnersPackage::getDominionOptionsMap(); + foreach ($xactions as $xaction) { + $new = $xaction->getNewValue(); + + if (empty($map[$new])) { + $valid = array_keys($map); + + $errors[] = $this->newInvalidError( + pht( + 'Dominion setting "%s" is not valid. '. + 'Valid settings are: %s.', + $new, + implode(', ', $valid)), + $xaction); + } + } + + return $errors; + } + + public function applyInternalEffects($object, $value) { + $object->setDominion($value); + } + + public function getTitle() { + $map = PhabricatorOwnersPackage::getDominionOptionsMap(); + $map = ipull($map, 'short'); + + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + $old = idx($map, $old, $old); + $new = idx($map, $new, $new); + + return pht( + '%s adjusted package dominion rules from %s to %s.', + $this->renderAuthor(), + $this->renderValue($old), + $this->renderValue($new)); + } + +} diff --git a/src/applications/owners/xaction/PhabricatorOwnersPackageNameTransaction.php b/src/applications/owners/xaction/PhabricatorOwnersPackageNameTransaction.php new file mode 100644 index 0000000000..b4e50daec2 --- /dev/null +++ b/src/applications/owners/xaction/PhabricatorOwnersPackageNameTransaction.php @@ -0,0 +1,52 @@ +getName(); + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + + $missing = $this->isEmptyTextTransaction( + $object->getName(), + $xactions); + + if ($missing) { + $errors[] = $this->newRequiredError( + pht('Package name is required.'), + nonempty(last($xactions), null)); + } + + foreach ($xactions as $xaction) { + $new = $xaction->getNewValue(); + if (preg_match('([,!])', $new)) { + $errors[] = $this->newInvalidError( + pht( + 'Package names may not contain commas (",") or exclamation '. + 'marks ("!"). These characters are ambiguous when package '. + 'names are parsed from the command line.'), + $xaction); + } + } + + return $errors; + } + + public function applyInternalEffects($object, $value) { + $object->setName($value); + } + + public function getTitle() { + return pht( + '%s renamed this package from %s to %s.', + $this->renderAuthor(), + $this->renderOldValue(), + $this->renderNewValue()); + } + +} diff --git a/src/applications/owners/xaction/PhabricatorOwnersPackageOwnersTransaction.php b/src/applications/owners/xaction/PhabricatorOwnersPackageOwnersTransaction.php new file mode 100644 index 0000000000..7fef0d5e6c --- /dev/null +++ b/src/applications/owners/xaction/PhabricatorOwnersPackageOwnersTransaction.php @@ -0,0 +1,76 @@ +getOwners(), 'getUserPHID'); + $phids = array_values($phids); + return $phids; + } + + public function generateNewValue($object, $value) { + $phids = array_unique($value); + $phids = array_values($phids); + return $phids; + } + + public function applyExternalEffects($object, $value) { + $old = $this->generateOldValue($object); + $new = $value; + + $owners = $object->getOwners(); + $owners = mpull($owners, null, 'getUserPHID'); + + $rem = array_diff($old, $new); + foreach ($rem as $phid) { + if (isset($owners[$phid])) { + $owners[$phid]->delete(); + unset($owners[$phid]); + } + } + + $add = array_diff($new, $old); + foreach ($add as $phid) { + $owners[$phid] = id(new PhabricatorOwnersOwner()) + ->setPackageID($object->getID()) + ->setUserPHID($phid) + ->save(); + } + + // TODO: Attach owners here + } + + public function getTitle() { + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + $add = array_diff($new, $old); + $rem = array_diff($old, $new); + if ($add && !$rem) { + return pht( + '%s added %s owner(s): %s.', + $this->renderAuthor(), + count($add), + $this->renderHandleList($add)); + } else if ($rem && !$add) { + return pht( + '%s removed %s owner(s): %s.', + $this->renderAuthor(), + count($rem), + $this->renderHandleList($rem)); + } else { + return pht( + '%s changed %s package owner(s), added %s: %s; removed %s: %s.', + $this->renderAuthor(), + count($add) + count($rem), + count($add), + $this->renderHandleList($add), + count($rem), + $this->renderHandleList($rem)); + } + } + +} diff --git a/src/applications/owners/xaction/PhabricatorOwnersPackagePathsTransaction.php b/src/applications/owners/xaction/PhabricatorOwnersPackagePathsTransaction.php new file mode 100644 index 0000000000..5952b78aed --- /dev/null +++ b/src/applications/owners/xaction/PhabricatorOwnersPackagePathsTransaction.php @@ -0,0 +1,189 @@ +getPaths(); + return mpull($paths, 'getRef'); + } + + public function generateNewValue($object, $value) { + $new = $value; + foreach ($new as $key => $info) { + $new[$key]['excluded'] = (int)idx($info, 'excluded'); + } + return $new; + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + + if (!$xactions) { + return $errors; + } + + $old = mpull($object->getPaths(), 'getRef'); + foreach ($xactions as $xaction) { + $new = $xaction->getNewValue(); + + // Check that we have a list of paths. + if (!is_array($new)) { + $errors[] = $this->newInvalidError( + pht('Path specification must be a list of paths.'), + $xaction); + continue; + } + + // Check that each item in the list is formatted properly. + $type_exception = null; + foreach ($new as $key => $value) { + try { + PhutilTypeSpec::checkMap( + $value, + array( + 'repositoryPHID' => 'string', + 'path' => 'string', + 'excluded' => 'optional wild', + )); + } catch (PhutilTypeCheckException $ex) { + $errors[] = $this->newInvalidError( + pht( + 'Path specification list contains invalid value '. + 'in key "%s": %s.', + $key, + $ex->getMessage()), + $xaction); + $type_exception = $ex; + } + } + + if ($type_exception) { + continue; + } + + // Check that any new paths reference legitimate repositories which + // the viewer has permission to see. + list($rem, $add) = PhabricatorOwnersPath::getTransactionValueChanges( + $old, + $new); + + if ($add) { + $repository_phids = ipull($add, 'repositoryPHID'); + + $repositories = id(new PhabricatorRepositoryQuery()) + ->setViewer($this->getActor()) + ->withPHIDs($repository_phids) + ->execute(); + $repositories = mpull($repositories, null, 'getPHID'); + + foreach ($add as $ref) { + $repository_phid = $ref['repositoryPHID']; + if (isset($repositories[$repository_phid])) { + continue; + } + + $errors[] = $this->newInvalidError( + pht( + 'Path specification list references repository PHID "%s", '. + 'but that is not a valid, visible repository.', + $repository_phid)); + } + } + } + + return $errors; + } + + public function applyExternalEffects($object, $value) { + $old = $this->generateOldValue($object); + $new = $value; + + $paths = $object->getPaths(); + + $diffs = PhabricatorOwnersPath::getTransactionValueChanges($old, $new); + list($rem, $add) = $diffs; + + $set = PhabricatorOwnersPath::getSetFromTransactionValue($rem); + foreach ($paths as $path) { + $ref = $path->getRef(); + if (PhabricatorOwnersPath::isRefInSet($ref, $set)) { + $path->delete(); + } + } + + foreach ($add as $ref) { + $path = PhabricatorOwnersPath::newFromRef($ref) + ->setPackageID($object->getID()) + ->save(); + } + } + + public function getTitle() { + // TODO: Flesh this out. + return pht( + '%s updated paths for this package.', + $this->renderAuthor()); + } + + public function hasChangeDetailView() { + return true; + } + + public function newChangeDetailView() { + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + $diffs = PhabricatorOwnersPath::getTransactionValueChanges($old, $new); + list($rem, $add) = $diffs; + + $rows = array(); + foreach ($rem as $ref) { + $rows[] = array( + 'class' => 'diff-removed', + 'change' => '-', + ) + $ref; + } + + foreach ($add as $ref) { + $rows[] = array( + 'class' => 'diff-added', + 'change' => '+', + ) + $ref; + } + + $rowc = array(); + foreach ($rows as $key => $row) { + $rowc[] = $row['class']; + $rows[$key] = array( + $row['change'], + $row['excluded'] ? pht('Exclude') : pht('Include'), + $this->renderHandle($row['repositoryPHID']), + $row['path'], + ); + } + + $table = id(new AphrontTableView($rows)) + ->setViewer($this->getViewer()) + ->setRowClasses($rowc) + ->setHeaders( + array( + null, + pht('Type'), + pht('Repository'), + pht('Path'), + )) + ->setColumnClasses( + array( + null, + null, + null, + 'wide', + )); + + return $table; + } + +} diff --git a/src/applications/owners/xaction/PhabricatorOwnersPackagePrimaryTransaction.php b/src/applications/owners/xaction/PhabricatorOwnersPackagePrimaryTransaction.php new file mode 100644 index 0000000000..76d3c2f10f --- /dev/null +++ b/src/applications/owners/xaction/PhabricatorOwnersPackagePrimaryTransaction.php @@ -0,0 +1,15 @@ +getStatus(); + } + + public function applyInternalEffects($object, $value) { + $object->setStatus($value); + } + + public function getTitle() { + $new = $this->getNewValue(); + if ($new == PhabricatorOwnersPackage::STATUS_ACTIVE) { + return pht( + '%s activated this package.', + $this->renderAuthor()); + } else if ($new == PhabricatorOwnersPackage::STATUS_ARCHIVED) { + return pht( + '%s archived this package.', + $this->renderAuthor()); + } + } + +} diff --git a/src/applications/owners/xaction/PhabricatorOwnersPackageTransactionType.php b/src/applications/owners/xaction/PhabricatorOwnersPackageTransactionType.php new file mode 100644 index 0000000000..5faaa79a17 --- /dev/null +++ b/src/applications/owners/xaction/PhabricatorOwnersPackageTransactionType.php @@ -0,0 +1,4 @@ +