1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-09-20 09:18:48 +02:00

Migate more of Pholio to Modular Transaction

Summary: Another Franken-transaction. Adds transactions for ImageName and MockName. Adds transaction-level validation for presence of mock name; removed same from edit controller. Removes `PholioTransaction::getRemarkupBodyForFeed()`, which appears to be dead code since that method isn't defined on any other types.

Test Plan: made a bunch of changes to pholio mocks and images and observed expected results

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, chad, epriestley

Differential Revision: https://secure.phabricator.com/D17865
This commit is contained in:
Austin McKinley 2017-05-10 15:11:28 -07:00
parent b32a00dbac
commit fcf64953f0
10 changed files with 226 additions and 117 deletions

View file

@ -4359,10 +4359,11 @@ phutil_register_library_map(array(
'PholioDAO' => 'applications/pholio/storage/PholioDAO.php',
'PholioDefaultEditCapability' => 'applications/pholio/capability/PholioDefaultEditCapability.php',
'PholioDefaultViewCapability' => 'applications/pholio/capability/PholioDefaultViewCapability.php',
'PholioDescriptionTransaction' => 'applications/pholio/xaction/PholioDescriptionTransaction.php',
'PholioImage' => 'applications/pholio/storage/PholioImage.php',
'PholioImageNameTransaction' => 'applications/pholio/xaction/PholioImageNameTransaction.php',
'PholioImagePHIDType' => 'applications/pholio/phid/PholioImagePHIDType.php',
'PholioImageQuery' => 'applications/pholio/query/PholioImageQuery.php',
'PholioImageTransactionType' => 'applications/pholio/xaction/PholioImageTransactionType.php',
'PholioImageUploadController' => 'applications/pholio/controller/PholioImageUploadController.php',
'PholioInlineController' => 'applications/pholio/controller/PholioInlineController.php',
'PholioInlineListController' => 'applications/pholio/controller/PholioInlineListController.php',
@ -4371,6 +4372,7 @@ phutil_register_library_map(array(
'PholioMockAuthorHeraldField' => 'applications/pholio/herald/PholioMockAuthorHeraldField.php',
'PholioMockCommentController' => 'applications/pholio/controller/PholioMockCommentController.php',
'PholioMockDescriptionHeraldField' => 'applications/pholio/herald/PholioMockDescriptionHeraldField.php',
'PholioMockDescriptionTransaction' => 'applications/pholio/xaction/PholioMockDescriptionTransaction.php',
'PholioMockEditController' => 'applications/pholio/controller/PholioMockEditController.php',
'PholioMockEditor' => 'applications/pholio/editor/PholioMockEditor.php',
'PholioMockEmbedView' => 'applications/pholio/view/PholioMockEmbedView.php',
@ -4383,12 +4385,14 @@ phutil_register_library_map(array(
'PholioMockListController' => 'applications/pholio/controller/PholioMockListController.php',
'PholioMockMailReceiver' => 'applications/pholio/mail/PholioMockMailReceiver.php',
'PholioMockNameHeraldField' => 'applications/pholio/herald/PholioMockNameHeraldField.php',
'PholioMockNameTransaction' => 'applications/pholio/xaction/PholioMockNameTransaction.php',
'PholioMockPHIDType' => 'applications/pholio/phid/PholioMockPHIDType.php',
'PholioMockQuery' => 'applications/pholio/query/PholioMockQuery.php',
'PholioMockRelationship' => 'applications/pholio/relationships/PholioMockRelationship.php',
'PholioMockRelationshipSource' => 'applications/search/relationship/PholioMockRelationshipSource.php',
'PholioMockSearchEngine' => 'applications/pholio/query/PholioMockSearchEngine.php',
'PholioMockThumbGridView' => 'applications/pholio/view/PholioMockThumbGridView.php',
'PholioMockTransactionType' => 'applications/pholio/xaction/PholioMockTransactionType.php',
'PholioMockViewController' => 'applications/pholio/controller/PholioMockViewController.php',
'PholioRemarkupRule' => 'applications/pholio/remarkup/PholioRemarkupRule.php',
'PholioReplyHandler' => 'applications/pholio/mail/PholioReplyHandler.php',
@ -9896,14 +9900,15 @@ phutil_register_library_map(array(
'PholioDAO' => 'PhabricatorLiskDAO',
'PholioDefaultEditCapability' => 'PhabricatorPolicyCapability',
'PholioDefaultViewCapability' => 'PhabricatorPolicyCapability',
'PholioDescriptionTransaction' => 'PholioTransactionType',
'PholioImage' => array(
'PholioDAO',
'PhabricatorMarkupInterface',
'PhabricatorPolicyInterface',
),
'PholioImageNameTransaction' => 'PholioImageTransactionType',
'PholioImagePHIDType' => 'PhabricatorPHIDType',
'PholioImageQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'PholioImageTransactionType' => 'PholioTransactionType',
'PholioImageUploadController' => 'PholioController',
'PholioInlineController' => 'PholioController',
'PholioInlineListController' => 'PholioController',
@ -9925,6 +9930,7 @@ phutil_register_library_map(array(
'PholioMockAuthorHeraldField' => 'PholioMockHeraldField',
'PholioMockCommentController' => 'PholioController',
'PholioMockDescriptionHeraldField' => 'PholioMockHeraldField',
'PholioMockDescriptionTransaction' => 'PholioMockTransactionType',
'PholioMockEditController' => 'PholioController',
'PholioMockEditor' => 'PhabricatorApplicationTransactionEditor',
'PholioMockEmbedView' => 'AphrontView',
@ -9937,12 +9943,14 @@ phutil_register_library_map(array(
'PholioMockListController' => 'PholioController',
'PholioMockMailReceiver' => 'PhabricatorObjectMailReceiver',
'PholioMockNameHeraldField' => 'PholioMockHeraldField',
'PholioMockNameTransaction' => 'PholioMockTransactionType',
'PholioMockPHIDType' => 'PhabricatorPHIDType',
'PholioMockQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'PholioMockRelationship' => 'PhabricatorObjectRelationship',
'PholioMockRelationshipSource' => 'PhabricatorObjectRelationshipSource',
'PholioMockSearchEngine' => 'PhabricatorApplicationSearchEngine',
'PholioMockThumbGridView' => 'AphrontView',
'PholioMockTransactionType' => 'PholioTransactionType',
'PholioMockViewController' => 'PholioController',
'PholioRemarkupRule' => 'PhabricatorObjectRemarkupRule',
'PholioReplyHandler' => 'PhabricatorApplicationTransactionReplyHandler',

View file

@ -65,8 +65,8 @@ final class PholioMockEditController extends PholioController {
if ($request->isFormPost()) {
$xactions = array();
$type_name = PholioTransaction::TYPE_NAME;
$type_desc = PholioDescriptionTransaction::TRANSACTIONTYPE;
$type_name = PholioMockNameTransaction::TRANSACTIONTYPE;
$type_desc = PholioMockDescriptionTransaction::TRANSACTIONTYPE;
$type_view = PhabricatorTransactions::TYPE_VIEW_POLICY;
$type_edit = PhabricatorTransactions::TYPE_EDIT_POLICY;
$type_cc = PhabricatorTransactions::TYPE_SUBSCRIBERS;
@ -88,11 +88,6 @@ final class PholioMockEditController extends PholioController {
$mock_xactions[$type_cc] = array('=' => $v_cc);
$mock_xactions[$type_space] = $v_space;
if (!strlen($request->getStr('name'))) {
$e_name = pht('Required');
$errors[] = pht('You must give the mock a name.');
}
$file_phids = $request->getArr('file_phids');
if ($file_phids) {
$files = id(new PhabricatorFileQuery())
@ -173,7 +168,7 @@ final class PholioMockEditController extends PholioController {
$posted_mock_images[] = $add_image;
} else {
$xactions[] = id(new PholioTransaction())
->setTransactionType(PholioTransaction::TYPE_IMAGE_NAME)
->setTransactionType(PholioImageNameTransaction::TRANSACTIONTYPE)
->setNewValue(
array($existing_image->getPHID() => $title));
$xactions[] = id(new PholioTransaction())

View file

@ -29,12 +29,10 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor {
$types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
$types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
$types[] = PholioTransaction::TYPE_NAME;
$types[] = PholioTransaction::TYPE_STATUS;
$types[] = PholioTransaction::TYPE_INLINE;
$types[] = PholioTransaction::TYPE_IMAGE_FILE;
$types[] = PholioTransaction::TYPE_IMAGE_NAME;
$types[] = PholioTransaction::TYPE_IMAGE_DESCRIPTION;
$types[] = PholioTransaction::TYPE_IMAGE_REPLACE;
$types[] = PholioTransaction::TYPE_IMAGE_SEQUENCE;
@ -47,22 +45,11 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor {
PhabricatorApplicationTransaction $xaction) {
switch ($xaction->getTransactionType()) {
case PholioTransaction::TYPE_NAME:
return $object->getName();
case PholioTransaction::TYPE_STATUS:
return $object->getStatus();
case PholioTransaction::TYPE_IMAGE_FILE:
$images = $object->getImages();
return mpull($images, 'getPHID');
case PholioTransaction::TYPE_IMAGE_NAME:
$name = null;
$phid = null;
$image = $this->getImageForXaction($object, $xaction);
if ($image) {
$name = $image->getName();
$phid = $image->getPHID();
}
return array($phid => $name);
case PholioTransaction::TYPE_IMAGE_DESCRIPTION:
$description = null;
$phid = null;
@ -92,9 +79,7 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor {
PhabricatorApplicationTransaction $xaction) {
switch ($xaction->getTransactionType()) {
case PholioTransaction::TYPE_NAME:
case PholioTransaction::TYPE_STATUS:
case PholioTransaction::TYPE_IMAGE_NAME:
case PholioTransaction::TYPE_IMAGE_DESCRIPTION:
case PholioTransaction::TYPE_IMAGE_SEQUENCE:
return $xaction->getNewValue();
@ -205,12 +190,6 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor {
PhabricatorApplicationTransaction $xaction) {
switch ($xaction->getTransactionType()) {
case PholioTransaction::TYPE_NAME:
$object->setName($xaction->getNewValue());
if ($object->getOriginalName() === null) {
$object->setOriginalName($xaction->getNewValue());
}
break;
case PholioTransaction::TYPE_STATUS:
$object->setStatus($xaction->getNewValue());
break;
@ -263,12 +242,6 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor {
}
$object->attachImages($images);
break;
case PholioTransaction::TYPE_IMAGE_NAME:
$image = $this->getImageForXaction($object, $xaction);
$value = (string)head($xaction->getNewValue());
$image->setName($value);
$image->save();
break;
case PholioTransaction::TYPE_IMAGE_DESCRIPTION:
$image = $this->getImageForXaction($object, $xaction);
$value = (string)head($xaction->getNewValue());
@ -303,7 +276,6 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor {
$type = $u->getTransactionType();
switch ($type) {
case PholioTransaction::TYPE_NAME:
case PholioTransaction::TYPE_STATUS:
return $v;
case PholioTransaction::TYPE_IMAGE_REPLACE:
@ -315,7 +287,6 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor {
break;
case PholioTransaction::TYPE_IMAGE_FILE:
return $this->mergePHIDOrEdgeTransactions($u, $v);
case PholioTransaction::TYPE_IMAGE_NAME:
case PholioTransaction::TYPE_IMAGE_DESCRIPTION:
case PholioTransaction::TYPE_IMAGE_SEQUENCE:
$raw_new_value_u = $u->getNewValue();

View file

@ -22,9 +22,9 @@ final class PhabricatorPholioMockTestDataGenerator
// Accumulate Transactions
$changes = array();
$changes[PholioTransaction::TYPE_NAME] =
$changes[PholioMockNameTransaction::TRANSACTIONTYPE] =
$this->generateTitle();
$changes[PholioDescriptionTransaction::TRANSACTIONTYPE] =
$changes[PholioMockDescriptionTransaction::TRANSACTIONTYPE] =
$this->generateDescription();
$changes[PhabricatorTransactions::TYPE_VIEW_POLICY] =
PhabricatorPolicies::POLICY_PUBLIC;

View file

@ -3,12 +3,10 @@
final class PholioTransaction extends PhabricatorModularTransaction {
// Edits to the high level mock
const TYPE_NAME = 'name';
const TYPE_STATUS = 'status';
// Edits to images within the mock
const TYPE_IMAGE_FILE = 'image-file';
const TYPE_IMAGE_NAME= 'image-name';
const TYPE_IMAGE_DESCRIPTION = 'image-description';
const TYPE_IMAGE_REPLACE = 'image-replace';
const TYPE_IMAGE_SEQUENCE = 'image-sequence';
@ -57,7 +55,7 @@ final class PholioTransaction extends PhabricatorModularTransaction {
$phids[] = $old;
break;
case self::TYPE_IMAGE_DESCRIPTION:
case self::TYPE_IMAGE_NAME:
case PholioImageNameTransaction::TRANSACTIONTYPE:
case self::TYPE_IMAGE_SEQUENCE:
$phids[] = key($new);
break;
@ -70,7 +68,6 @@ final class PholioTransaction extends PhabricatorModularTransaction {
$old = $this->getOldValue();
switch ($this->getTransactionType()) {
case self::TYPE_IMAGE_NAME:
case self::TYPE_IMAGE_DESCRIPTION:
return ($old === array(null => null));
// this is boring / silly to surface; changing sequence is NBD
@ -89,14 +86,12 @@ final class PholioTransaction extends PhabricatorModularTransaction {
switch ($this->getTransactionType()) {
case self::TYPE_INLINE:
return 'fa-comment';
case self::TYPE_NAME:
case self::TYPE_STATUS:
if ($new == PholioMock::STATUS_CLOSED) {
return 'fa-ban';
} else {
return 'fa-check';
}
case self::TYPE_IMAGE_NAME:
case self::TYPE_IMAGE_DESCRIPTION:
case self::TYPE_IMAGE_SEQUENCE:
return 'fa-pencil';
@ -118,9 +113,9 @@ final class PholioTransaction extends PhabricatorModularTransaction {
case self::TYPE_STATUS:
$tags[] = self::MAILTAG_STATUS;
break;
case self::TYPE_NAME:
case PholioDescriptionTransaction::TRANSACTIONTYPE:
case self::TYPE_IMAGE_NAME:
case PholioMockNameTransaction::TRANSACTIONTYPE:
case PholioMockDescriptionTransaction::TRANSACTIONTYPE:
case PholioImageNameTransaction::TRANSACTIONTYPE:
case self::TYPE_IMAGE_DESCRIPTION:
case self::TYPE_IMAGE_SEQUENCE:
case self::TYPE_IMAGE_FILE:
@ -142,20 +137,6 @@ final class PholioTransaction extends PhabricatorModularTransaction {
$type = $this->getTransactionType();
switch ($type) {
case self::TYPE_NAME:
if ($old === null) {
return pht(
'%s created "%s".',
$this->renderHandleLink($author_phid),
$new);
} else {
return pht(
'%s renamed this mock from "%s" to "%s".',
$this->renderHandleLink($author_phid),
$old,
$new);
}
break;
case self::TYPE_STATUS:
if ($new == PholioMock::STATUS_CLOSED) {
return pht(
@ -213,15 +194,6 @@ final class PholioTransaction extends PhabricatorModularTransaction {
$this->renderHandleList($rem));
}
break;
case self::TYPE_IMAGE_NAME:
return pht(
'%s renamed an image (%s) from "%s" to "%s".',
$this->renderHandleLink($author_phid),
$this->renderHandleLink(key($new)),
reset($old),
reset($new));
break;
case self::TYPE_IMAGE_DESCRIPTION:
return pht(
'%s updated an image\'s (%s) description.',
@ -248,21 +220,6 @@ final class PholioTransaction extends PhabricatorModularTransaction {
$type = $this->getTransactionType();
switch ($type) {
case self::TYPE_NAME:
if ($old === null) {
return pht(
'%s created %s.',
$this->renderHandleLink($author_phid),
$this->renderHandleLink($object_phid));
} else {
return pht(
'%s renamed %s from "%s" to "%s".',
$this->renderHandleLink($author_phid),
$this->renderHandleLink($object_phid),
$old,
$new);
}
break;
case self::TYPE_STATUS:
if ($new == PholioMock::STATUS_CLOSED) {
return pht(
@ -289,12 +246,6 @@ final class PholioTransaction extends PhabricatorModularTransaction {
$this->renderHandleLink($author_phid),
$this->renderHandleLink($object_phid));
break;
case self::TYPE_IMAGE_NAME:
return pht(
'%s updated the image names of %s.',
$this->renderHandleLink($author_phid),
$this->renderHandleLink($object_phid));
break;
case self::TYPE_IMAGE_DESCRIPTION:
return pht(
'%s updated image descriptions of %s.',
@ -312,23 +263,6 @@ final class PholioTransaction extends PhabricatorModularTransaction {
return parent::getTitleForFeed();
}
public function getRemarkupBodyForFeed(PhabricatorFeedStory $story) {
$text = null;
switch ($this->getTransactionType()) {
case self::TYPE_NAME:
if ($this->getOldValue() === null) {
$mock = $story->getPrimaryObject();
$text = $mock->getDescription();
}
break;
case self::TYPE_INLINE:
$text = $this->getComment()->getContent();
break;
}
return $text;
}
public function getColor() {
$old = $this->getOldValue();
$new = $this->getNewValue();
@ -340,10 +274,6 @@ final class PholioTransaction extends PhabricatorModularTransaction {
} else {
return PhabricatorTransactions::COLOR_GREEN;
}
case self::TYPE_NAME:
if ($old === null) {
return PhabricatorTransactions::COLOR_GREEN;
}
case self::TYPE_IMAGE_REPLACE:
return PhabricatorTransactions::COLOR_YELLOW;
case self::TYPE_IMAGE_FILE:

View file

@ -0,0 +1,95 @@
<?php
final class PholioImageNameTransaction
extends PholioImageTransactionType {
const TRANSACTIONTYPE = 'image-name';
public function generateOldValue($object) {
$name = null;
$phid = null;
$image = $this->getImageForXaction($object);
if ($image) {
$name = $image->getName();
$phid = $image->getPHID();
}
return array($phid => $name);
}
public function applyInternalEffects($object, $value) {
$image = $this->getImageForXaction($object);
$value = (string)head($this->getNewValue());
$image->setName($value);
$image->save();
}
public function getTitle() {
$old = $this->getOldValue();
$new = $this->getNewValue();
return pht(
'%s renamed an image (%s) from %s to %s.',
$this->renderAuthor(),
$this->renderHandle(key($new)),
$this->renderValue($old),
$this->renderValue($new));
}
public function getTitleForFeed() {
return pht(
'%s updated the image names of %s.',
$this->renderAuthor(),
$this->renderObject());
}
public function getIcon() {
$new = $this->getNewValue();
if ($new == PholioMock::STATUS_CLOSED) {
return 'fa-ban';
} else {
return 'fa-check';
}
}
public function mergeTransactions(
$object,
PhabricatorApplicationTransaction $u,
PhabricatorApplicationTransaction $v) {
$raw_new_value_u = $u->getNewValue();
$raw_new_value_v = $v->getNewValue();
$phid_u = head_key($raw_new_value_u);
$phid_v = head_key($raw_new_value_v);
if ($phid_u == $phid_v) {
return $v;
}
return null;
}
public function shouldHide() {
$old = $this->getOldValue();
return ($old === array(null => null));
}
public function validateTransactions($object, array $xactions) {
$errors = array();
$max_length = $object->getColumnMaximumByteLength('name');
foreach ($xactions as $xaction) {
$new_value = head(array_values($xaction->getNewValue()));
$new_length = strlen($new_value);
if ($new_length > $max_length) {
$errors[] = $this->newInvalidError(
pht(
'Mock image names must not be longer than %s character(s).',
new PhutilNumber($max_length)));
}
}
return $errors;
}
}

View file

@ -0,0 +1,18 @@
<?php
abstract class PholioImageTransactionType
extends PholioTransactionType {
protected function getImageForXaction(PholioMock $mock) {
$raw_new_value = $this->getNewValue();
$image_phid = head_key($raw_new_value);
$images = $mock->getImages();
foreach ($images as $image) {
if ($image->getPHID() == $image_phid) {
return $image;
}
}
return null;
}
}

View file

@ -1,7 +1,7 @@
<?php
final class PholioDescriptionTransaction
extends PholioTransactionType {
final class PholioMockDescriptionTransaction
extends PholioMockTransactionType {
const TRANSACTIONTYPE = 'description';

View file

@ -0,0 +1,88 @@
<?php
final class PholioMockNameTransaction
extends PholioMockTransactionType {
const TRANSACTIONTYPE = 'name';
public function generateOldValue($object) {
return $object->getName();
}
public function applyInternalEffects($object, $value) {
$object->setName($value);
if ($object->getOriginalName() === null) {
$object->setOriginalName($this->getNewValue());
}
}
public function getTitle() {
$old = $this->getOldValue();
$new = $this->getNewValue();
if ($old === null) {
return pht(
'%s created %s.',
$this->renderAuthor(),
$this->renderValue($new));
} else {
return pht(
'%s renamed this mock from %s to %s.',
$this->renderAuthor(),
$this->renderValue($old),
$this->renderValue($new));
}
}
public function getTitleForFeed() {
$old = $this->getOldValue();
$new = $this->getNewValue();
if ($old === null) {
return pht(
'%s created %s.',
$this->renderAuthor(),
$this->renderObject());
} else {
return pht(
'%s renamed %s from %s to %s.',
$this->renderAuthor(),
$this->renderObject(),
$this->renderValue($old),
$this->renderValue($new));
}
}
public function getColor() {
$old = $this->getOldValue();
if ($old === null) {
return PhabricatorTransactions::COLOR_GREEN;
}
return parent::getColor();
}
public function validateTransactions($object, array $xactions) {
$errors = array();
if ($this->isEmptyTextTransaction($object->getName(), $xactions)) {
$errors[] = $this->newRequiredError(pht('Mocks 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(
'Mock names must not be longer than %s character(s).',
new PhutilNumber($max_length)));
}
}
return $errors;
}
}

View file

@ -0,0 +1,4 @@
<?php
abstract class PholioMockTransactionType
extends PholioTransactionType {}