Page MenuHomePhabricator
Files F15422549

D20258.id48362.diff
No OneTemporary
Actions

D20258.id48362.diff
View Options

diff --git a/resources/sql/autopatches/20190307.herald.01.comments.sql b/resources/sql/autopatches/20190307.herald.01.comments.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20190307.herald.01.comments.sql
@@ -0,0 +1 @@
+DROP TABLE {$NAMESPACE}_herald.herald_ruletransaction_comment;
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -1535,10 +1535,13 @@
'HeraldRuleAdapterField' => 'applications/herald/field/rule/HeraldRuleAdapterField.php',
'HeraldRuleController' => 'applications/herald/controller/HeraldRuleController.php',
'HeraldRuleDatasource' => 'applications/herald/typeahead/HeraldRuleDatasource.php',
+ 'HeraldRuleDisableTransaction' => 'applications/herald/xaction/HeraldRuleDisableTransaction.php',
+ 'HeraldRuleEditTransaction' => 'applications/herald/xaction/HeraldRuleEditTransaction.php',
'HeraldRuleEditor' => 'applications/herald/editor/HeraldRuleEditor.php',
'HeraldRuleField' => 'applications/herald/field/rule/HeraldRuleField.php',
'HeraldRuleFieldGroup' => 'applications/herald/field/rule/HeraldRuleFieldGroup.php',
'HeraldRuleListController' => 'applications/herald/controller/HeraldRuleListController.php',
+ 'HeraldRuleNameTransaction' => 'applications/herald/xaction/HeraldRuleNameTransaction.php',
'HeraldRulePHIDType' => 'applications/herald/phid/HeraldRulePHIDType.php',
'HeraldRuleQuery' => 'applications/herald/query/HeraldRuleQuery.php',
'HeraldRuleReplyHandler' => 'applications/herald/mail/HeraldRuleReplyHandler.php',
@@ -1546,7 +1549,7 @@
'HeraldRuleSerializer' => 'applications/herald/editor/HeraldRuleSerializer.php',
'HeraldRuleTestCase' => 'applications/herald/storage/__tests__/HeraldRuleTestCase.php',
'HeraldRuleTransaction' => 'applications/herald/storage/HeraldRuleTransaction.php',
- 'HeraldRuleTransactionComment' => 'applications/herald/storage/HeraldRuleTransactionComment.php',
+ 'HeraldRuleTransactionType' => 'applications/herald/xaction/HeraldRuleTransactionType.php',
'HeraldRuleTranscript' => 'applications/herald/storage/transcript/HeraldRuleTranscript.php',
'HeraldRuleTypeConfig' => 'applications/herald/config/HeraldRuleTypeConfig.php',
'HeraldRuleTypeDatasource' => 'applications/herald/typeahead/HeraldRuleTypeDatasource.php',
@@ -7188,18 +7191,21 @@
'HeraldRuleAdapterField' => 'HeraldRuleField',
'HeraldRuleController' => 'HeraldController',
'HeraldRuleDatasource' => 'PhabricatorTypeaheadDatasource',
+ 'HeraldRuleDisableTransaction' => 'HeraldRuleTransactionType',
+ 'HeraldRuleEditTransaction' => 'HeraldRuleTransactionType',
'HeraldRuleEditor' => 'PhabricatorApplicationTransactionEditor',
'HeraldRuleField' => 'HeraldField',
'HeraldRuleFieldGroup' => 'HeraldFieldGroup',
'HeraldRuleListController' => 'HeraldController',
+ 'HeraldRuleNameTransaction' => 'HeraldRuleTransactionType',
'HeraldRulePHIDType' => 'PhabricatorPHIDType',
'HeraldRuleQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'HeraldRuleReplyHandler' => 'PhabricatorApplicationTransactionReplyHandler',
'HeraldRuleSearchEngine' => 'PhabricatorApplicationSearchEngine',
'HeraldRuleSerializer' => 'Phobject',
'HeraldRuleTestCase' => 'PhabricatorTestCase',
- 'HeraldRuleTransaction' => 'PhabricatorApplicationTransaction',
- 'HeraldRuleTransactionComment' => 'PhabricatorApplicationTransactionComment',
+ 'HeraldRuleTransaction' => 'PhabricatorModularTransaction',
+ 'HeraldRuleTransactionType' => 'PhabricatorModularTransactionType',
'HeraldRuleTranscript' => 'Phobject',
'HeraldRuleTypeConfig' => 'Phobject',
'HeraldRuleTypeDatasource' => 'PhabricatorTypeaheadDatasource',
diff --git a/src/applications/herald/controller/HeraldDisableController.php b/src/applications/herald/controller/HeraldDisableController.php
--- a/src/applications/herald/controller/HeraldDisableController.php
+++ b/src/applications/herald/controller/HeraldDisableController.php
@@ -31,7 +31,7 @@
if ($request->isFormPost()) {
$xaction = id(new HeraldRuleTransaction())
- ->setTransactionType(HeraldRuleTransaction::TYPE_DISABLE)
+ ->setTransactionType(HeraldRuleDisableTransaction::TRANSACTIONTYPE)
->setNewValue($is_disable);
id(new HeraldRuleEditor())
diff --git a/src/applications/herald/controller/HeraldRuleController.php b/src/applications/herald/controller/HeraldRuleController.php
--- a/src/applications/herald/controller/HeraldRuleController.php
+++ b/src/applications/herald/controller/HeraldRuleController.php
@@ -359,11 +359,21 @@
$repetition_policy);
$xactions = array();
+
+ // Until this moves to EditEngine, manually add a "CREATE" transaction
+ // if we're creating a new rule. This improves rendering of the initial
+ // group of transactions.
+ $is_new = (bool)(!$rule->getID());
+ if ($is_new) {
+ $xactions[] = id(new HeraldRuleTransaction())
+ ->setTransactionType(PhabricatorTransactions::TYPE_CREATE);
+ }
+
$xactions[] = id(new HeraldRuleTransaction())
- ->setTransactionType(HeraldRuleTransaction::TYPE_EDIT)
+ ->setTransactionType(HeraldRuleEditTransaction::TRANSACTIONTYPE)
->setNewValue($new_state);
$xactions[] = id(new HeraldRuleTransaction())
- ->setTransactionType(HeraldRuleTransaction::TYPE_NAME)
+ ->setTransactionType(HeraldRuleNameTransaction::TRANSACTIONTYPE)
->setNewValue($new_name);
try {
diff --git a/src/applications/herald/editor/HeraldRuleEditor.php b/src/applications/herald/editor/HeraldRuleEditor.php
--- a/src/applications/herald/editor/HeraldRuleEditor.php
+++ b/src/applications/herald/editor/HeraldRuleEditor.php
@@ -11,82 +11,6 @@
return pht('Herald Rules');
}
- public function getTransactionTypes() {
- $types = parent::getTransactionTypes();
-
- $types[] = PhabricatorTransactions::TYPE_COMMENT;
- $types[] = HeraldRuleTransaction::TYPE_EDIT;
- $types[] = HeraldRuleTransaction::TYPE_NAME;
- $types[] = HeraldRuleTransaction::TYPE_DISABLE;
-
- return $types;
- }
-
- protected function getCustomTransactionOldValue(
- PhabricatorLiskDAO $object,
- PhabricatorApplicationTransaction $xaction) {
-
- switch ($xaction->getTransactionType()) {
- case HeraldRuleTransaction::TYPE_DISABLE:
- return (int)$object->getIsDisabled();
- case HeraldRuleTransaction::TYPE_EDIT:
- return id(new HeraldRuleSerializer())
- ->serializeRule($object);
- case HeraldRuleTransaction::TYPE_NAME:
- return $object->getName();
- }
-
- }
-
- protected function getCustomTransactionNewValue(
- PhabricatorLiskDAO $object,
- PhabricatorApplicationTransaction $xaction) {
-
- switch ($xaction->getTransactionType()) {
- case HeraldRuleTransaction::TYPE_DISABLE:
- return (int)$xaction->getNewValue();
- case HeraldRuleTransaction::TYPE_EDIT:
- case HeraldRuleTransaction::TYPE_NAME:
- return $xaction->getNewValue();
- }
- }
-
- protected function applyCustomInternalTransaction(
- PhabricatorLiskDAO $object,
- PhabricatorApplicationTransaction $xaction) {
-
- switch ($xaction->getTransactionType()) {
- case HeraldRuleTransaction::TYPE_DISABLE:
- return $object->setIsDisabled($xaction->getNewValue());
- case HeraldRuleTransaction::TYPE_NAME:
- return $object->setName($xaction->getNewValue());
- case HeraldRuleTransaction::TYPE_EDIT:
- $new_state = id(new HeraldRuleSerializer())
- ->deserializeRuleComponents($xaction->getNewValue());
- $object->setMustMatchAll((int)$new_state['match_all']);
- $object->attachConditions($new_state['conditions']);
- $object->attachActions($new_state['actions']);
-
- $new_repetition = $new_state['repetition_policy'];
- $object->setRepetitionPolicyStringConstant($new_repetition);
-
- return $object;
- }
-
- }
-
- protected function applyCustomExternalTransaction(
- PhabricatorLiskDAO $object,
- PhabricatorApplicationTransaction $xaction) {
- switch ($xaction->getTransactionType()) {
- case HeraldRuleTransaction::TYPE_EDIT:
- $object->saveConditions($object->getConditions());
- $object->saveActions($object->getActions());
- break;
- }
- return;
- }
-
protected function shouldApplyHeraldRules(
PhabricatorLiskDAO $object,
array $xactions) {
@@ -137,7 +61,6 @@
return pht('[Herald]');
}
-
protected function buildMailBody(
PhabricatorLiskDAO $object,
array $xactions) {
diff --git a/src/applications/herald/storage/HeraldRuleTransaction.php b/src/applications/herald/storage/HeraldRuleTransaction.php
--- a/src/applications/herald/storage/HeraldRuleTransaction.php
+++ b/src/applications/herald/storage/HeraldRuleTransaction.php
@@ -1,11 +1,9 @@
<?php
final class HeraldRuleTransaction
- extends PhabricatorApplicationTransaction {
+ extends PhabricatorModularTransaction {
const TYPE_EDIT = 'herald:edit';
- const TYPE_NAME = 'herald:name';
- const TYPE_DISABLE = 'herald:disable';
public function getApplicationName() {
return 'herald';
@@ -15,121 +13,8 @@
return HeraldRulePHIDType::TYPECONST;
}
- public function getApplicationTransactionCommentObject() {
- return new HeraldRuleTransactionComment();
- }
-
- public function getColor() {
- $old = $this->getOldValue();
- $new = $this->getNewValue();
-
- switch ($this->getTransactionType()) {
- case self::TYPE_DISABLE:
- if ($new) {
- return 'red';
- } else {
- return 'green';
- }
- }
-
- return parent::getColor();
- }
-
- public function getActionName() {
- $old = $this->getOldValue();
- $new = $this->getNewValue();
-
- switch ($this->getTransactionType()) {
- case self::TYPE_DISABLE:
- if ($new) {
- return pht('Disabled');
- } else {
- return pht('Enabled');
- }
- case self::TYPE_NAME:
- return pht('Renamed');
- }
-
- return parent::getActionName();
- }
-
- public function getIcon() {
- $old = $this->getOldValue();
- $new = $this->getNewValue();
-
- switch ($this->getTransactionType()) {
- case self::TYPE_DISABLE:
- if ($new) {
- return 'fa-ban';
- } else {
- return 'fa-check';
- }
- }
-
- return parent::getIcon();
- }
-
-
- public function getTitle() {
- $author_phid = $this->getAuthorPHID();
-
- $old = $this->getOldValue();
- $new = $this->getNewValue();
-
- switch ($this->getTransactionType()) {
- case self::TYPE_DISABLE:
- if ($new) {
- return pht(
- '%s disabled this rule.',
- $this->renderHandleLink($author_phid));
- } else {
- return pht(
- '%s enabled this rule.',
- $this->renderHandleLink($author_phid));
- }
- case self::TYPE_NAME:
- if ($old == null) {
- return pht(
- '%s created this rule.',
- $this->renderHandleLink($author_phid));
- } else {
- return pht(
- '%s renamed this rule from "%s" to "%s".',
- $this->renderHandleLink($author_phid),
- $old,
- $new);
- }
- case self::TYPE_EDIT:
- return pht(
- '%s edited this rule.',
- $this->renderHandleLink($author_phid));
- }
-
- return parent::getTitle();
- }
-
- public function hasChangeDetails() {
- switch ($this->getTransactionType()) {
- case self::TYPE_EDIT:
- return true;
- }
- return parent::hasChangeDetails();
- }
-
- public function renderChangeDetails(PhabricatorUser $viewer) {
- $json = new PhutilJSON();
- switch ($this->getTransactionType()) {
- case self::TYPE_EDIT:
- return $this->renderTextCorpusChangeDetails(
- $viewer,
- $json->encodeFormatted($this->getOldValue()),
- $json->encodeFormatted($this->getNewValue()));
- }
-
- return $this->renderTextCorpusChangeDetails(
- $viewer,
- $this->getOldValue(),
- $this->getNewValue());
+ public function getBaseTransactionClass() {
+ return 'HeraldRuleTransactionType';
}
}
diff --git a/src/applications/herald/storage/HeraldRuleTransactionComment.php b/src/applications/herald/storage/HeraldRuleTransactionComment.php
deleted file mode 100644
--- a/src/applications/herald/storage/HeraldRuleTransactionComment.php
+++ /dev/null
@@ -1,10 +0,0 @@
-<?php
-
-final class HeraldRuleTransactionComment
- extends PhabricatorApplicationTransactionComment {
-
- public function getApplicationTransactionObject() {
- return new HeraldRuleTransaction();
- }
-
-}
diff --git a/src/applications/herald/xaction/HeraldRuleDisableTransaction.php b/src/applications/herald/xaction/HeraldRuleDisableTransaction.php
new file mode 100644
--- /dev/null
+++ b/src/applications/herald/xaction/HeraldRuleDisableTransaction.php
@@ -0,0 +1,32 @@
+<?php
+
+final class HeraldRuleDisableTransaction
+ extends HeraldRuleTransactionType {
+
+ const TRANSACTIONTYPE = 'herald:disable';
+
+ public function generateOldValue($object) {
+ return (bool)$object->getIsDisabled();
+ }
+
+ public function generateNewValue($object, $value) {
+ return (bool)$value;
+ }
+
+ public function applyInternalEffects($object, $value) {
+ $object->setIsDisabled((int)$value);
+ }
+
+ public function getTitle() {
+ if ($this->getNewValue()) {
+ return pht(
+ '%s disabled this rule.',
+ $this->renderAuthor());
+ } else {
+ return pht(
+ '%s enabled this rule.',
+ $this->renderAuthor());
+ }
+ }
+
+}
diff --git a/src/applications/herald/xaction/HeraldRuleEditTransaction.php b/src/applications/herald/xaction/HeraldRuleEditTransaction.php
new file mode 100644
--- /dev/null
+++ b/src/applications/herald/xaction/HeraldRuleEditTransaction.php
@@ -0,0 +1,56 @@
+<?php
+
+final class HeraldRuleEditTransaction
+ extends HeraldRuleTransactionType {
+
+ const TRANSACTIONTYPE = 'herald:edit';
+
+ public function generateOldValue($object) {
+ return id(new HeraldRuleSerializer())
+ ->serializeRule($object);
+ }
+
+ public function applyInternalEffects($object, $value) {
+ $new_state = id(new HeraldRuleSerializer())
+ ->deserializeRuleComponents($value);
+
+ $object->setMustMatchAll((int)$new_state['match_all']);
+ $object->attachConditions($new_state['conditions']);
+ $object->attachActions($new_state['actions']);
+
+ $new_repetition = $new_state['repetition_policy'];
+ $object->setRepetitionPolicyStringConstant($new_repetition);
+ }
+
+ public function applyExternalEffects($object, $value) {
+ $object->saveConditions($object->getConditions());
+ $object->saveActions($object->getActions());
+ }
+
+ public function getTitle() {
+ return pht(
+ '%s edited this rule.',
+ $this->renderAuthor());
+ }
+
+ public function hasChangeDetailView() {
+ return true;
+ }
+
+ public function newChangeDetailView() {
+ $viewer = $this->getViewer();
+
+ $old = $this->getOldValue();
+ $new = $this->getNewValue();
+
+ $json = new PhutilJSON();
+ $old_json = $json->encodeFormatted($old);
+ $new_json = $json->encodeFormatted($new);
+
+ return id(new PhabricatorApplicationTransactionTextDiffDetailView())
+ ->setViewer($viewer)
+ ->setOldText($old_json)
+ ->setNewText($new_json);
+ }
+
+}
diff --git a/src/applications/herald/xaction/HeraldRuleNameTransaction.php b/src/applications/herald/xaction/HeraldRuleNameTransaction.php
new file mode 100644
--- /dev/null
+++ b/src/applications/herald/xaction/HeraldRuleNameTransaction.php
@@ -0,0 +1,48 @@
+<?php
+
+final class HeraldRuleNameTransaction
+ extends HeraldRuleTransactionType {
+
+ const TRANSACTIONTYPE = 'herald:name';
+
+ public function generateOldValue($object) {
+ return $object->getName();
+ }
+
+ public function applyInternalEffects($object, $value) {
+ $object->setName($value);
+ }
+
+ public function getTitle() {
+ return pht(
+ '%s renamed this rule from %s to %s.',
+ $this->renderAuthor(),
+ $this->renderOldValue(),
+ $this->renderNewValue());
+ }
+
+ public function validateTransactions($object, array $xactions) {
+ $errors = array();
+
+ if ($this->isEmptyTextTransaction($object->getName(), $xactions)) {
+ $errors[] = $this->newRequiredError(
+ pht('Rules must have a name.'));
+ }
+
+ $max_length = $object->getColumnMaximumByteLength('name');
+ foreach ($xactions as $xaction) {
+ $new_value = $xaction->getNewValue();
+
+ $new_length = strlen($new_value);
+ if ($new_length > $max_length) {
+ $errors[] = $this->newInvalidError(
+ pht(
+ 'Rule names can be no longer than %s characters.',
+ new PhutilNumber($max_length)));
+ }
+ }
+
+ return $errors;
+ }
+
+}
diff --git a/src/applications/herald/xaction/HeraldRuleTransactionType.php b/src/applications/herald/xaction/HeraldRuleTransactionType.php
new file mode 100644
--- /dev/null
+++ b/src/applications/herald/xaction/HeraldRuleTransactionType.php
@@ -0,0 +1,4 @@
+<?php
+
+abstract class HeraldRuleTransactionType
+ extends PhabricatorModularTransactionType {}

File Metadata

Mime Type
text/plain
Expires
Sun, Mar 23, 9:17 AM (2 w, 3 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7718271
Default Alt Text
D20258.id48362.diff (17 KB)

Event Timeline

Phabricator · Built by Phacility