From dfc8f8bcb465aa509b944eca6f6d87af075ce015 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 18 Feb 2016 05:54:50 -0800 Subject: [PATCH] Make callsigns editable on repository basic information Summary: Ref T4245. This is a prelude to removing them from the "create" screen. Currently, if you try to delete the callsign you get an unceremonious database error, but the next diff (or maybe two) will permit that, so I didn't put any "this is required yada yada" text in. This could also maybe use some big flashing warning lights and a "if you edit this, all your URIs break" but I'll save that for later. Test Plan: Changed the callsign for a repository. Reviewers: chad Reviewed By: chad Maniphest Tasks: T4245 Differential Revision: https://secure.phabricator.com/D15304 --- ...DiffusionRepositoryEditBasicController.php | 19 +++++++ .../editor/PhabricatorRepositoryEditor.php | 50 ++++++++++++++++++- .../storage/PhabricatorRepository.php | 28 ++++++++++- .../PhabricatorRepositoryTransaction.php | 21 ++++++++ 4 files changed, 115 insertions(+), 3 deletions(-) diff --git a/src/applications/diffusion/controller/DiffusionRepositoryEditBasicController.php b/src/applications/diffusion/controller/DiffusionRepositoryEditBasicController.php index 65f7dd0974..e23a57e655 100644 --- a/src/applications/diffusion/controller/DiffusionRepositoryEditBasicController.php +++ b/src/applications/diffusion/controller/DiffusionRepositoryEditBasicController.php @@ -18,11 +18,13 @@ final class DiffusionRepositoryEditBasicController $v_name = $repository->getName(); $v_desc = $repository->getDetail('description'); $v_slug = $repository->getRepositorySlug(); + $v_callsign = $repository->getCallsign(); $v_projects = PhabricatorEdgeQuery::loadDestinationPHIDs( $repository->getPHID(), PhabricatorProjectObjectHasProjectEdgeType::EDGECONST); $e_name = true; $e_slug = null; + $e_callsign = null; $errors = array(); $validation_exception = null; @@ -31,6 +33,7 @@ final class DiffusionRepositoryEditBasicController $v_desc = $request->getStr('description'); $v_projects = $request->getArr('projectPHIDs'); $v_slug = $request->getStr('slug'); + $v_callsign = $request->getStr('callsign'); if (!strlen($v_name)) { $e_name = pht('Required'); @@ -47,6 +50,7 @@ final class DiffusionRepositoryEditBasicController $type_desc = PhabricatorRepositoryTransaction::TYPE_DESCRIPTION; $type_edge = PhabricatorTransactions::TYPE_EDGE; $type_slug = PhabricatorRepositoryTransaction::TYPE_SLUG; + $type_callsign = PhabricatorRepositoryTransaction::TYPE_CALLSIGN; $xactions[] = id(clone $template) ->setTransactionType($type_name) @@ -60,6 +64,10 @@ final class DiffusionRepositoryEditBasicController ->setTransactionType($type_slug) ->setNewValue($v_slug); + $xactions[] = id(clone $template) + ->setTransactionType($type_callsign) + ->setNewValue($v_callsign); + $xactions[] = id(clone $template) ->setTransactionType($type_edge) ->setMetadataValue( @@ -78,11 +86,16 @@ final class DiffusionRepositoryEditBasicController try { $editor->applyTransactions($repository, $xactions); + // The preferred edit URI may have changed if the callsign or slug + // were adjusted, so grab a fresh copy. + $edit_uri = $this->getRepositoryControllerURI($repository, 'edit/'); + return id(new AphrontRedirectResponse())->setURI($edit_uri); } catch (PhabricatorApplicationTransactionValidationException $ex) { $validation_exception = $ex; $e_slug = $ex->getShortMessage($type_slug); + $e_callsign = $ex->getShortMessage($type_callsign); } } } @@ -106,6 +119,12 @@ final class DiffusionRepositoryEditBasicController ->setLabel(pht('Short Name')) ->setValue($v_slug) ->setError($e_slug)) + ->appendChild( + id(new AphrontFormTextControl()) + ->setName('callsign') + ->setLabel(pht('Callsign')) + ->setValue($v_callsign) + ->setError($e_callsign)) ->appendChild( id(new PhabricatorRemarkupControl()) ->setUser($viewer) diff --git a/src/applications/repository/editor/PhabricatorRepositoryEditor.php b/src/applications/repository/editor/PhabricatorRepositoryEditor.php index f6674835fc..fa585a826a 100644 --- a/src/applications/repository/editor/PhabricatorRepositoryEditor.php +++ b/src/applications/repository/editor/PhabricatorRepositoryEditor.php @@ -45,6 +45,7 @@ final class PhabricatorRepositoryEditor $types[] = PhabricatorRepositoryTransaction::TYPE_SYMBOLS_SOURCES; $types[] = PhabricatorRepositoryTransaction::TYPE_STAGING_URI; $types[] = PhabricatorRepositoryTransaction::TYPE_AUTOMATION_BLUEPRINTS; + $types[] = PhabricatorRepositoryTransaction::TYPE_CALLSIGN; $types[] = PhabricatorTransactions::TYPE_EDGE; $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY; @@ -110,6 +111,8 @@ final class PhabricatorRepositoryEditor return $object->getDetail('staging-uri'); case PhabricatorRepositoryTransaction::TYPE_AUTOMATION_BLUEPRINTS: return $object->getDetail('automation.blueprintPHIDs', array()); + case PhabricatorRepositoryTransaction::TYPE_CALLSIGN: + return $object->getCallsign(); } } @@ -148,6 +151,7 @@ final class PhabricatorRepositoryEditor case PhabricatorRepositoryTransaction::TYPE_AUTOMATION_BLUEPRINTS: return $xaction->getNewValue(); case PhabricatorRepositoryTransaction::TYPE_SLUG: + case PhabricatorRepositoryTransaction::TYPE_CALLSIGN: $name = $xaction->getNewValue(); if (strlen($name)) { return $name; @@ -240,6 +244,9 @@ final class PhabricatorRepositoryEditor 'automation.blueprintPHIDs', $xaction->getNewValue()); return; + case PhabricatorRepositoryTransaction::TYPE_CALLSIGN: + $object->setCallsign($xaction->getNewValue()); + return; case PhabricatorRepositoryTransaction::TYPE_ENCODING: // Make sure the encoding is valid by converting to UTF-8. This tests // that the user has mbstring installed, and also that they didn't type @@ -468,7 +475,7 @@ final class PhabricatorRepositoryEditor } try { - PhabricatorRepository::asssertValidRepositorySlug($new); + PhabricatorRepository::assertValidRepositorySlug($new); } catch (Exception $ex) { $errors[] = new PhabricatorApplicationTransactionValidationError( $type, @@ -495,6 +502,47 @@ final class PhabricatorRepositoryEditor } break; + case PhabricatorRepositoryTransaction::TYPE_CALLSIGN: + foreach ($xactions as $xaction) { + $old = $xaction->getOldValue(); + $new = $xaction->getNewValue(); + + if (!strlen($new)) { + continue; + } + + if ($new === $old) { + continue; + } + + try { + PhabricatorRepository::assertValidCallsign($new); + } catch (Exception $ex) { + $errors[] = new PhabricatorApplicationTransactionValidationError( + $type, + pht('Invalid'), + $ex->getMessage(), + $xaction); + continue; + } + + $other = id(new PhabricatorRepositoryQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withCallsigns(array($new)) + ->executeOne(); + if ($other && ($other->getID() !== $object->getID())) { + $errors[] = new PhabricatorApplicationTransactionValidationError( + $type, + pht('Duplicate'), + pht( + 'The selected callsign ("%s") is already in use by another '. + 'repository. Choose a unique callsign.', + $new), + $xaction); + continue; + } + } + break; } return $errors; diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php index 3b1f2c8561..d4977d2b89 100644 --- a/src/applications/repository/storage/PhabricatorRepository.php +++ b/src/applications/repository/storage/PhabricatorRepository.php @@ -317,14 +317,14 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO public static function isValidRepositorySlug($slug) { try { - self::asssertValidRepositorySlug($slug); + self::assertValidRepositorySlug($slug); return true; } catch (Exception $ex) { return false; } } - public static function asssertValidRepositorySlug($slug) { + public static function assertValidRepositorySlug($slug) { if (!strlen($slug)) { throw new Exception( pht( @@ -391,6 +391,30 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO } } + public static function assertValidCallsign($callsign) { + if (!strlen($callsign)) { + throw new Exception( + pht( + 'A repository callsign must be at least one character long.')); + } + + if (strlen($callsign) > 32) { + throw new Exception( + pht( + 'The callsign "%s" is not a valid repository callsign. Callsigns '. + 'must be no more than 32 bytes long.', + $callsign)); + } + + if (!preg_match('/^[A-Z]+\z/', $callsign)) { + throw new Exception( + pht( + 'The callsign "%s" is not a valid repository callsign. Callsigns '. + 'may only contain UPPERCASE letters.', + $callsign)); + } + } + /* -( Remote Command Execution )------------------------------------------- */ diff --git a/src/applications/repository/storage/PhabricatorRepositoryTransaction.php b/src/applications/repository/storage/PhabricatorRepositoryTransaction.php index a10ac0a60b..20c975cb33 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryTransaction.php +++ b/src/applications/repository/storage/PhabricatorRepositoryTransaction.php @@ -29,6 +29,7 @@ final class PhabricatorRepositoryTransaction const TYPE_SYMBOLS_LANGUAGE = 'repo:symbol-language'; const TYPE_STAGING_URI = 'repo:staging-uri'; const TYPE_AUTOMATION_BLUEPRINTS = 'repo:automation-blueprints'; + const TYPE_CALLSIGN = 'repo:callsign'; // TODO: Clean up these legacy transaction types. const TYPE_SSH_LOGIN = 'repo:ssh-login'; @@ -466,6 +467,26 @@ final class PhabricatorRepositoryTransaction new PhutilNumber(count($rem)), $this->renderHandleList($rem)); } + + case self::TYPE_CALLSIGN: + if ($old === null) { + return pht( + '%s set the callsign for this repository to "%s".', + $this->renderHandleLink($author_phid), + $new); + } else if ($new === null) { + return pht( + '%s removed the callsign ("%s") for this repository.', + $this->renderHandleLink($author_phid), + $old); + } else { + return pht( + '%s changed the callsign for this repository from "%s" to "%s".', + $this->renderHandleLink($author_phid), + $old, + $new); + } + } return parent::getTitle();