1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 00:42:41 +01:00

Use ApplicationTransactions in ApplicationEmail

Summary:
Ref T8498. I want to add Spaces to these, and the logic for getting Spaces right is a bit tricky, so swap these to ApplicationTransactions.

One new piece of tech: made it easier for Editors to raise DuplicateKeyException as a normal ValidationException, so callers don't have to handle this case specially.

One behavioral change: we no longer require these addresses to be at the `auth.email-domains` domains -- I think this wasn't quite right in the general case. It's OK to require users to have `@mycompany.com` addresses but add `@phabricator.mycompany-infrastructure.com` addresses here if you want.

Test Plan:
  - Tried to create a duplicate email.
  - Tried to create an empty email.
  - Tried to create an invalid email.
  - Created a new email.
  - Deleted an email.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8498

Differential Revision: https://secure.phabricator.com/D13246
This commit is contained in:
epriestley 2015-06-11 10:15:49 -07:00
parent 325f4c863c
commit 6d6211d441
8 changed files with 308 additions and 64 deletions

View file

@ -0,0 +1,19 @@
CREATE TABLE {$NAMESPACE}_metamta.metamta_applicationemailtransaction (
id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
phid VARBINARY(64) NOT NULL,
authorPHID VARBINARY(64) NOT NULL,
objectPHID VARBINARY(64) NOT NULL,
viewPolicy VARBINARY(64) NOT NULL,
editPolicy VARBINARY(64) NOT NULL,
commentPHID VARBINARY(64) DEFAULT NULL,
commentVersion INT UNSIGNED NOT NULL,
transactionType VARCHAR(32) COLLATE {$COLLATE_TEXT} NOT NULL,
oldValue LONGTEXT COLLATE {$COLLATE_TEXT} NOT NULL,
newValue LONGTEXT COLLATE {$COLLATE_TEXT} NOT NULL,
contentSource LONGTEXT COLLATE {$COLLATE_TEXT} NOT NULL,
metadata LONGTEXT COLLATE {$COLLATE_TEXT} NOT NULL,
dateCreated INT UNSIGNED NOT NULL,
dateModified INT UNSIGNED NOT NULL,
UNIQUE KEY `key_phid` (`phid`),
KEY `key_object` (`objectPHID`)
) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT};

View file

@ -2051,9 +2051,12 @@ phutil_register_library_map(array(
'PhabricatorMetaMTAApplication' => 'applications/metamta/application/PhabricatorMetaMTAApplication.php',
'PhabricatorMetaMTAApplicationEmail' => 'applications/metamta/storage/PhabricatorMetaMTAApplicationEmail.php',
'PhabricatorMetaMTAApplicationEmailDatasource' => 'applications/metamta/typeahead/PhabricatorMetaMTAApplicationEmailDatasource.php',
'PhabricatorMetaMTAApplicationEmailEditor' => 'applications/metamta/editor/PhabricatorMetaMTAApplicationEmailEditor.php',
'PhabricatorMetaMTAApplicationEmailPHIDType' => 'applications/phid/PhabricatorMetaMTAApplicationEmailPHIDType.php',
'PhabricatorMetaMTAApplicationEmailPanel' => 'applications/metamta/applicationpanel/PhabricatorMetaMTAApplicationEmailPanel.php',
'PhabricatorMetaMTAApplicationEmailQuery' => 'applications/metamta/query/PhabricatorMetaMTAApplicationEmailQuery.php',
'PhabricatorMetaMTAApplicationEmailTransaction' => 'applications/metamta/storage/PhabricatorMetaMTAApplicationEmailTransaction.php',
'PhabricatorMetaMTAApplicationEmailTransactionQuery' => 'applications/metamta/query/PhabricatorMetaMTAApplicationEmailTransactionQuery.php',
'PhabricatorMetaMTAAttachment' => 'applications/metamta/storage/PhabricatorMetaMTAAttachment.php',
'PhabricatorMetaMTAConfigOptions' => 'applications/config/option/PhabricatorMetaMTAConfigOptions.php',
'PhabricatorMetaMTAController' => 'applications/metamta/controller/PhabricatorMetaMTAController.php',
@ -5493,11 +5496,16 @@ phutil_register_library_map(array(
'PhabricatorMetaMTAApplicationEmail' => array(
'PhabricatorMetaMTADAO',
'PhabricatorPolicyInterface',
'PhabricatorApplicationTransactionInterface',
'PhabricatorDestructibleInterface',
),
'PhabricatorMetaMTAApplicationEmailDatasource' => 'PhabricatorTypeaheadDatasource',
'PhabricatorMetaMTAApplicationEmailEditor' => 'PhabricatorApplicationTransactionEditor',
'PhabricatorMetaMTAApplicationEmailPHIDType' => 'PhabricatorPHIDType',
'PhabricatorMetaMTAApplicationEmailPanel' => 'PhabricatorApplicationConfigurationPanel',
'PhabricatorMetaMTAApplicationEmailQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'PhabricatorMetaMTAApplicationEmailTransaction' => 'PhabricatorApplicationTransaction',
'PhabricatorMetaMTAApplicationEmailTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
'PhabricatorMetaMTAConfigOptions' => 'PhabricatorApplicationConfigOptions',
'PhabricatorMetaMTAController' => 'PhabricatorController',
'PhabricatorMetaMTADAO' => 'PhabricatorLiskDAO',

View file

@ -190,30 +190,6 @@ final class PhabricatorMetaMTAApplicationEmailPanel
));
}
private function validateApplicationEmail($email) {
$errors = array();
$e_email = true;
if (!strlen($email)) {
$e_email = pht('Required');
$errors[] = pht('Email is required.');
} else if (!PhabricatorUserEmail::isValidAddress($email)) {
$e_email = pht('Invalid');
$errors[] = PhabricatorUserEmail::describeValidAddresses();
} else if (!PhabricatorUserEmail::isAllowedAddress($email)) {
$e_email = pht('Disallowed');
$errors[] = PhabricatorUserEmail::describeAllowedAddresses();
}
$user_emails = id(new PhabricatorUserEmail())
->loadAllWhere('address = %s', $email);
if ($user_emails) {
$e_email = pht('Duplicate');
$errors[] = pht('A user already has this email.');
}
return array($e_email, $errors);
}
private function returnNewAddressResponse(
AphrontRequest $request,
PhutilURI $uri,
@ -265,45 +241,59 @@ final class PhabricatorMetaMTAApplicationEmailPanel
$viewer = $request->getUser();
$e_email = true;
$email = null;
$errors = array();
$default_user_key =
$config_default =
PhabricatorMetaMTAApplicationEmail::CONFIG_DEFAULT_AUTHOR;
$e_email = true;
$v_email = $email_object->getAddress();
$v_default = $email_object->getConfigValue($config_default);
$validation_exception = null;
if ($request->isDialogFormPost()) {
$email = trim($request->getStr('email'));
list($e_email, $errors) = $this->validateApplicationEmail($email);
$email_object->setAddress($email);
$default_user = $request->getArr($default_user_key);
$default_user = reset($default_user);
if ($default_user) {
$email_object->setConfigValue($default_user_key, $default_user);
}
$e_email = null;
if (!$errors) {
try {
$email_object->save();
return id(new AphrontRedirectResponse())->setURI(
$uri->alter('highlight', $email_object->getID()));
} catch (AphrontDuplicateKeyQueryException $ex) {
$e_email = pht('Duplicate');
$errors[] = pht(
'Another application is already configured to use this email '.
'address.');
}
$v_email = trim($request->getStr('email'));
$v_default = $request->getArr($config_default);
$v_default = nonempty(head($v_default), null);
$type_address =
PhabricatorMetaMTAApplicationEmailTransaction::TYPE_ADDRESS;
$type_config =
PhabricatorMetaMTAApplicationEmailTransaction::TYPE_CONFIG;
$key_config = PhabricatorMetaMTAApplicationEmailTransaction::KEY_CONFIG;
$xactions = array();
$xactions[] = id(new PhabricatorMetaMTAApplicationEmailTransaction())
->setTransactionType($type_address)
->setNewValue($v_email);
$xactions[] = id(new PhabricatorMetaMTAApplicationEmailTransaction())
->setTransactionType($type_config)
->setMetadataValue($key_config, $config_default)
->setNewValue($v_default);
$editor = id(new PhabricatorMetaMTAApplicationEmailEditor())
->setActor($viewer)
->setContentSourceFromRequest($request)
->setContinueOnNoEffect(true);
try {
$editor->applyTransactions($email_object, $xactions);
return id(new AphrontRedirectResponse())->setURI(
$uri->alter('highlight', $email_object->getID()));
} catch (PhabricatorApplicationTransactionValidationException $ex) {
$validation_exception = $ex;
$e_email = $ex->getShortMessage($type_address);
}
}
if ($errors) {
$errors = id(new PHUIInfoView())
->setErrors($errors);
}
$default_user = $email_object->getConfigValue($default_user_key);
if ($default_user) {
$default_user_value = array($default_user);
if ($v_default) {
$v_default = array($v_default);
} else {
$default_user_value = array();
$v_default = array();
}
$form = id(new AphrontFormView())
@ -312,28 +302,29 @@ final class PhabricatorMetaMTAApplicationEmailPanel
id(new AphrontFormTextControl())
->setLabel(pht('Email'))
->setName('email')
->setValue($email_object->getAddress())
->setCaption(PhabricatorUserEmail::describeAllowedAddresses())
->setValue($v_email)
->setError($e_email))
->appendControl(
id(new AphrontFormTokenizerControl())
->setDatasource(new PhabricatorPeopleDatasource())
->setLabel(pht('Default Author'))
->setName($default_user_key)
->setName($config_default)
->setLimit(1)
->setValue($default_user_value)
->setValue($v_default)
->setCaption(pht(
'Used if the "From:" address does not map to a known account.')));
if ($is_new) {
$title = pht('New Address');
} else {
$title = pht('Edit Address');
}
$dialog = id(new AphrontDialogView())
->setUser($viewer)
->setWidth(AphrontDialogView::WIDTH_FORM)
->setTitle($title)
->appendChild($errors)
->setValidationException($validation_exception)
->appendForm($form)
->addSubmitButton(pht('Save'))
->addCancelButton($uri);
@ -350,7 +341,8 @@ final class PhabricatorMetaMTAApplicationEmailPanel
PhutilURI $uri,
$email_object_id) {
$viewer = $request->getUser();
$viewer = $this->getViewer();
$email_object = id(new PhabricatorMetaMTAApplicationEmailQuery())
->setViewer($viewer)
->withIDs(array($email_object_id))
@ -365,7 +357,8 @@ final class PhabricatorMetaMTAApplicationEmailPanel
}
if ($request->isDialogFormPost()) {
$email_object->delete();
$engine = new PhabricatorDestructionEngine();
$engine->destroyObject($email_object);
return id(new AphrontRedirectResponse())->setURI($uri);
}

View file

@ -0,0 +1,145 @@
<?php
final class PhabricatorMetaMTAApplicationEmailEditor
extends PhabricatorApplicationTransactionEditor {
public function getEditorApplicationClass() {
return pht('PhabricatorMetaMTAApplication');
}
public function getEditorObjectsDescription() {
return pht('Application Emails');
}
public function getTransactionTypes() {
$types = parent::getTransactionTypes();
$types[] = PhabricatorMetaMTAApplicationEmailTransaction::TYPE_ADDRESS;
$types[] = PhabricatorMetaMTAApplicationEmailTransaction::TYPE_CONFIG;
return $types;
}
protected function getCustomTransactionOldValue(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
switch ($xaction->getTransactionType()) {
case PhabricatorMetaMTAApplicationEmailTransaction::TYPE_ADDRESS:
return $object->getAddress();
case PhabricatorMetaMTAApplicationEmailTransaction::TYPE_CONFIG:
$key = $xaction->getMetadataValue(
PhabricatorMetaMTAApplicationEmailTransaction::KEY_CONFIG);
return $object->getConfigValue($key);
}
return parent::getCustomTransactionOldValue($object, $xaction);
}
protected function getCustomTransactionNewValue(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
switch ($xaction->getTransactionType()) {
case PhabricatorMetaMTAApplicationEmailTransaction::TYPE_ADDRESS:
case PhabricatorMetaMTAApplicationEmailTransaction::TYPE_CONFIG:
return $xaction->getNewValue();
}
return parent::getCustomTransactionNewValue($object, $xaction);
}
protected function applyCustomInternalTransaction(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
$new = $xaction->getNewValue();
switch ($xaction->getTransactionType()) {
case PhabricatorMetaMTAApplicationEmailTransaction::TYPE_ADDRESS:
$object->setAddress($new);
return;
case PhabricatorMetaMTAApplicationEmailTransaction::TYPE_CONFIG:
$key = $xaction->getMetadataValue(
PhabricatorMetaMTAApplicationEmailTransaction::KEY_CONFIG);
$object->setConfigValue($key, $new);
return;
}
return parent::applyCustomInternalTransaction($object, $xaction);
}
protected function applyCustomExternalTransaction(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
switch ($xaction->getTransactionType()) {
case PhabricatorMetaMTAApplicationEmailTransaction::TYPE_ADDRESS:
case PhabricatorMetaMTAApplicationEmailTransaction::TYPE_CONFIG:
return;
}
return parent::applyCustomExternalTransaction($object, $xaction);
}
protected function validateTransaction(
PhabricatorLiskDAO $object,
$type,
array $xactions) {
$errors = parent::validateTransaction($object, $type, $xactions);
switch ($type) {
case PhabricatorMetaMTAApplicationEmailTransaction::TYPE_ADDRESS:
foreach ($xactions as $xaction) {
$email = $xaction->getNewValue();
if (!strlen($email)) {
// We'll deal with this below.
continue;
}
if (!PhabricatorUserEmail::isValidAddress($email)) {
$errors[] = new PhabricatorApplicationTransactionValidationError(
$type,
pht('Invalid'),
pht('Email address is not formatted properly.'));
}
}
$missing = $this->validateIsEmptyTextField(
$object->getAddress(),
$xactions);
if ($missing) {
$error = new PhabricatorApplicationTransactionValidationError(
$type,
pht('Required'),
pht('You must provide an email address.'),
nonempty(last($xactions), null));
$error->setIsMissingFieldError(true);
$errors[] = $error;
}
break;
}
return $errors;
}
protected function didCatchDuplicateKeyException(
PhabricatorLiskDAO $object,
array $xactions,
Exception $ex) {
$errors = array();
$errors[] = new PhabricatorApplicationTransactionValidationError(
PhabricatorMetaMTAApplicationEmailTransaction::TYPE_ADDRESS,
pht('Duplicate'),
pht('This email address is already in use.'),
null);
throw new PhabricatorApplicationTransactionValidationException($errors);
}
}

View file

@ -0,0 +1,10 @@
<?php
final class PhabricatorMetaMTAApplicationEmailTransactionQuery
extends PhabricatorApplicationTransactionQuery {
public function getTemplateApplicationTransaction() {
return new PhabricatorMetaMTAApplicationEmailTransaction();
}
}

View file

@ -2,7 +2,10 @@
final class PhabricatorMetaMTAApplicationEmail
extends PhabricatorMetaMTADAO
implements PhabricatorPolicyInterface {
implements
PhabricatorPolicyInterface,
PhabricatorApplicationTransactionInterface,
PhabricatorDestructibleInterface {
protected $applicationPHID;
protected $address;
@ -109,4 +112,35 @@ final class PhabricatorMetaMTAApplicationEmail
return $this->getApplication()->describeAutomaticCapability($capability);
}
/* -( PhabricatorApplicationTransactionInterface )------------------------- */
public function getApplicationTransactionEditor() {
return new PhabricatorMetaMTAApplicationEmailEditor();
}
public function getApplicationTransactionObject() {
return $this;
}
public function getApplicationTransactionTemplate() {
return new PhabricatorMetaMTAApplicationEmailTransaction();
}
public function willRenderTimeline(
PhabricatorApplicationTransactionView $timeline,
AphrontRequest $request) {
return $timeline;
}
/* -( PhabricatorDestructibleInterface )----------------------------------- */
public function destroyObjectPermanently(
PhabricatorDestructionEngine $engine) {
$this->delete();
}
}

View file

@ -0,0 +1,23 @@
<?php
final class PhabricatorMetaMTAApplicationEmailTransaction
extends PhabricatorApplicationTransaction {
const KEY_CONFIG = 'appemail.config.key';
const TYPE_ADDRESS = 'appemail.address';
const TYPE_CONFIG = 'appemail.config';
public function getApplicationName() {
return 'metamta';
}
public function getApplicationTransactionType() {
return PhabricatorMetaMTAApplicationEmailPHIDType::TYPECONST;
}
public function getApplicationTransactionCommentObject() {
return null;
}
}

View file

@ -844,6 +844,11 @@ abstract class PhabricatorApplicationTransactionEditor
$object->save();
} catch (AphrontDuplicateKeyQueryException $ex) {
$object->killTransaction();
// This callback has an opportunity to throw a better exception,
// so execution may end here.
$this->didCatchDuplicateKeyException($object, $xactions, $ex);
throw $ex;
}
@ -1021,6 +1026,13 @@ abstract class PhabricatorApplicationTransactionEditor
return $xactions;
}
protected function didCatchDuplicateKeyException(
PhabricatorLiskDAO $object,
array $xactions,
Exception $ex) {
return;
}
public function publishTransactions(
PhabricatorLiskDAO $object,
array $xactions) {