From 07423211e98ea6cacae3985a3304837a660f6e60 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 17 Jun 2013 10:53:29 -0700 Subject: [PATCH] Show edit transactions for AuthProviders Summary: Ref T1536. When auth providers are edited, show the edit history. Test Plan: {F46400} Reviewers: btrahan Reviewed By: btrahan CC: aran, chad Maniphest Tasks: T1536 Differential Revision: https://secure.phabricator.com/D6203 --- .../config/PhabricatorAuthEditController.php | 24 +++-- .../PhabricatorAuthProviderConfigEditor.php | 13 ++- .../auth/provider/PhabricatorAuthProvider.php | 15 +++ .../storage/PhabricatorAuthProviderConfig.php | 21 +++- ...abricatorAuthProviderConfigTransaction.php | 98 +++++++++++++++++++ 5 files changed, 158 insertions(+), 13 deletions(-) diff --git a/src/applications/auth/controller/config/PhabricatorAuthEditController.php b/src/applications/auth/controller/config/PhabricatorAuthEditController.php index 0adfcc8bdc..f1c84c6e91 100644 --- a/src/applications/auth/controller/config/PhabricatorAuthEditController.php +++ b/src/applications/auth/controller/config/PhabricatorAuthEditController.php @@ -15,7 +15,6 @@ final class PhabricatorAuthEditController $request = $this->getRequest(); $viewer = $request->getUser(); - $provider = null; if ($this->configID) { $config = id(new PhabricatorAuthProviderConfigQuery()) ->setViewer($viewer) @@ -30,6 +29,11 @@ final class PhabricatorAuthEditController return new Aphront404Response(); } + $provider = $config->getProvider(); + if (!$provider) { + return new Aphront404Response(); + } + $is_new = false; } else { $providers = PhabricatorAuthProvider::getAllBaseProviders(); @@ -88,17 +92,17 @@ final class PhabricatorAuthEditController $xactions[] = id(new PhabricatorAuthProviderConfigTransaction()) ->setTransactionType( PhabricatorAuthProviderConfigTransaction::TYPE_REGISTRATION) - ->setNewValue($request->getInt('allowRegistration')); + ->setNewValue($request->getInt('allowRegistration', 0)); $xactions[] = id(new PhabricatorAuthProviderConfigTransaction()) ->setTransactionType( PhabricatorAuthProviderConfigTransaction::TYPE_LINK) - ->setNewValue($request->getInt('allowLink')); + ->setNewValue($request->getInt('allowLink', 0)); $xactions[] = id(new PhabricatorAuthProviderConfigTransaction()) ->setTransactionType( PhabricatorAuthProviderConfigTransaction::TYPE_UNLINK) - ->setNewValue($request->getInt('allowUnlink')); + ->setNewValue($request->getInt('allowUnlink', 0)); if (!$errors) { $editor = id(new PhabricatorAuthProviderConfigEditor()) @@ -199,8 +203,14 @@ final class PhabricatorAuthEditController $xaction_view = null; if (!$is_new) { - $xactions = id(new PhabricatorAuthProviderConfigTransactionQuery()); - // TOOD: ... + $xactions = id(new PhabricatorAuthProviderConfigTransactionQuery()) + ->withObjectPHIDs(array($config->getPHID())) + ->setViewer($viewer) + ->execute(); + + $xaction_view = id(new PhabricatorApplicationTransactionView()) + ->setUser($viewer) + ->setTransactions($xactions); } return $this->buildApplicationPage( @@ -208,7 +218,7 @@ final class PhabricatorAuthEditController $crumbs, $errors, $form, - $xaction_view + $xaction_view, ), array( 'title' => $title, diff --git a/src/applications/auth/editor/PhabricatorAuthProviderConfigEditor.php b/src/applications/auth/editor/PhabricatorAuthProviderConfigEditor.php index a32d23f04d..d6694609a3 100644 --- a/src/applications/auth/editor/PhabricatorAuthProviderConfigEditor.php +++ b/src/applications/auth/editor/PhabricatorAuthProviderConfigEditor.php @@ -21,13 +21,17 @@ final class PhabricatorAuthProviderConfigEditor switch ($xaction->getTransactionType()) { case PhabricatorAuthProviderConfigTransaction::TYPE_ENABLE: - return $object->getIsEnabled(); + if ($object->getIsEnabled() === null) { + return null; + } else { + return (int)$object->getIsEnabled(); + } case PhabricatorAuthProviderConfigTransaction::TYPE_REGISTRATION: - return $object->getShouldAllowRegistration(); + return (int)$object->getShouldAllowRegistration(); case PhabricatorAuthProviderConfigTransaction::TYPE_LINK: - return $object->getShouldAllowLink(); + return (int)$object->getShouldAllowLink(); case PhabricatorAuthProviderConfigTransaction::TYPE_UNLINK: - return $object->getShouldAllowUnlink(); + return (int)$object->getShouldAllowUnlink(); case PhabricatorAuthProviderConfigTransaction::TYPE_PROPERTY: // TODO throw new Exception("TODO"); @@ -51,7 +55,6 @@ final class PhabricatorAuthProviderConfigEditor protected function applyCustomInternalTransaction( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { - $v = $xaction->getNewValue(); switch ($xaction->getTransactionType()) { case PhabricatorAuthProviderConfigTransaction::TYPE_ENABLE: diff --git a/src/applications/auth/provider/PhabricatorAuthProvider.php b/src/applications/auth/provider/PhabricatorAuthProvider.php index 6db66a08a6..3d05da8a76 100644 --- a/src/applications/auth/provider/PhabricatorAuthProvider.php +++ b/src/applications/auth/provider/PhabricatorAuthProvider.php @@ -2,6 +2,21 @@ abstract class PhabricatorAuthProvider { + private $providerConfig; + + public function attachProviderConfig(PhabricatorAuthProviderConfig $config) { + $this->providerConfig = $config; + return $this; + } + + public function getProviderConfig() { + if ($this->config === null) { + throw new Exception( + "Call attachProviderConfig() before getProviderConfig()!"); + } + return $this->config; + } + public function getNameForCreate() { return $this->getProviderName(); } diff --git a/src/applications/auth/storage/PhabricatorAuthProviderConfig.php b/src/applications/auth/storage/PhabricatorAuthProviderConfig.php index 75c72c89c0..b4595d96d0 100644 --- a/src/applications/auth/storage/PhabricatorAuthProviderConfig.php +++ b/src/applications/auth/storage/PhabricatorAuthProviderConfig.php @@ -7,7 +7,7 @@ final class PhabricatorAuthProviderConfig extends PhabricatorAuthDAO protected $providerType; protected $providerDomain; - protected $isEnabled = 0; + protected $isEnabled; protected $shouldAllowLogin = 0; protected $shouldAllowRegistration = 0; protected $shouldAllowLink = 0; @@ -15,6 +15,8 @@ final class PhabricatorAuthProviderConfig extends PhabricatorAuthDAO protected $properties = array(); + private $provider; + public function generatePHID() { return PhabricatorPHID::generateNewPHID( PhabricatorPHIDConstants::PHID_TYPE_AUTH); @@ -38,6 +40,23 @@ final class PhabricatorAuthProviderConfig extends PhabricatorAuthDAO return $this; } + public function getProvider() { + if (!$this->provider) { + $base = PhabricatorAuthProvider::getAllBaseProviders(); + $found = null; + foreach ($base as $provider) { + if (get_class($provider) == $this->providerClass) { + $found = $provider; + break; + } + } + if ($found) { + $this->provider = id(clone $found)->attachProviderConfig($this); + } + } + return $this->provider; + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/applications/auth/storage/PhabricatorAuthProviderConfigTransaction.php b/src/applications/auth/storage/PhabricatorAuthProviderConfigTransaction.php index ed36b8394a..def9791a60 100644 --- a/src/applications/auth/storage/PhabricatorAuthProviderConfigTransaction.php +++ b/src/applications/auth/storage/PhabricatorAuthProviderConfigTransaction.php @@ -25,5 +25,103 @@ final class PhabricatorAuthProviderConfigTransaction return pht('authentication provider'); } + public function getIcon() { + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + switch ($this->getTransactionType()) { + case self::TYPE_ENABLE: + if ($new) { + return 'new'; + } else { + return 'delete'; + } + } + + return parent::getIcon(); + } + + public function getColor() { + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + switch ($this->getTransactionType()) { + case self::TYPE_ENABLE: + if ($new) { + return 'green'; + } else { + return 'red'; + } + } + + return parent::getColor(); + } + + public function getTitle() { + $author_phid = $this->getAuthorPHID(); + + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + switch ($this->getTransactionType()) { + case self::TYPE_ENABLE: + if ($old === null) { + return pht( + '%s created this provider.', + $this->renderHandleLink($author_phid)); + } else if ($new) { + return pht( + '%s enabled this provider.', + $this->renderHandleLink($author_phid)); + } else { + return pht( + '%s disabled this provider.', + $this->renderHandleLink($author_phid)); + } + break; + case self::TYPE_REGISTRATION: + if ($new) { + return pht( + '%s enabled registration.', + $this->renderHandleLink($author_phid)); + } else { + return pht( + '%s disabled registration.', + $this->renderHandleLink($author_phid)); + } + break; + case self::TYPE_LINK: + if ($new) { + return pht( + '%s enabled accont linking.', + $this->renderHandleLink($author_phid)); + } else { + return pht( + '%s disabled account linking.', + $this->renderHandleLink($author_phid)); + } + break; + case self::TYPE_UNLINK: + if ($new) { + return pht( + '%s enabled account unlinking.', + $this->renderHandleLink($author_phid)); + } else { + return pht( + '%s disabled account unlinking.', + $this->renderHandleLink($author_phid)); + } + break; + case self::TYPE_PROPERTY: + // TODO + return pht( + '%s edited a property of this provider.', + $this->renderHandleLink($author_phid)); + break; + } + + return parent::getTitle(); + } + }