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

Remove the "willApplyTransactions()" hook from ApplicationTransactionEditor

Summary:
Depends on D19908. Ref T13222. In D19897, I reordered some transaction code and affected the call order of `willApplyTransactions()`.

It turns out that we use this call for only one thing, and that thing is pretty silly: naming the raw paste data file when editing paste content.

This is only user-visible in the URL when you click "View Raw Paste" and seems exceptionally low-value, so remove the hook and pick a consistent name for the paste datafiles. (We could retain the name behavior in other ways, but it hardly seems worthwhile.)

Test Plan:
  - Created and edited a paste.
  - Grepped for `willApplyTransactions()`.

Note that `EditEngine` (vs `ApplicationTransacitonEditor`) still has a `willApplyTransactions()`, which has one callsite in Phabricator (in Calendar) and a couple in Instances. That's untouched and still works.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13222

Differential Revision: https://secure.phabricator.com/D19909
This commit is contained in:
epriestley 2018-12-18 14:27:39 -08:00
parent 1729e7b467
commit 106e90dcf0
4 changed files with 3 additions and 49 deletions

View file

@ -5,8 +5,6 @@ final class PhabricatorPasteContentTransaction
const TRANSACTIONTYPE = 'paste.create'; const TRANSACTIONTYPE = 'paste.create';
private $fileName;
public function generateOldValue($object) { public function generateOldValue($object) {
return $object->getFilePHID(); return $object->getFilePHID();
} }
@ -32,26 +30,6 @@ final class PhabricatorPasteContentTransaction
return array($error); return array($error);
} }
public function willApplyTransactions($object, array $xactions) {
// Find the most user-friendly filename we can by examining the title of
// the paste and the pending transactions. We'll use this if we create a
// new file to store raw content later.
$name = $object->getTitle();
if (!strlen($name)) {
$name = 'paste.raw';
}
$type_title = PhabricatorPasteTitleTransaction::TRANSACTIONTYPE;
foreach ($xactions as $xaction) {
if ($xaction->getTransactionType() == $type_title) {
$name = $xaction->getNewValue();
}
}
$this->fileName = $name;
}
public function generateNewValue($object, $value) { public function generateNewValue($object, $value) {
// If this transaction does not really change the paste content, return // If this transaction does not really change the paste content, return
// the current file PHID so this transaction no-ops. // the current file PHID so this transaction no-ops.
@ -66,16 +44,16 @@ final class PhabricatorPasteContentTransaction
$editor = $this->getEditor(); $editor = $this->getEditor();
$actor = $editor->getActor(); $actor = $editor->getActor();
$file = $this->newFileForPaste($actor, $this->fileName, $value); $file = $this->newFileForPaste($actor, $value);
return $file->getPHID(); return $file->getPHID();
} }
private function newFileForPaste(PhabricatorUser $actor, $name, $data) { private function newFileForPaste(PhabricatorUser $actor, $data) {
return PhabricatorFile::newFromFileData( return PhabricatorFile::newFromFileData(
$data, $data,
array( array(
'name' => $name, 'name' => 'raw.txt',
'mime-type' => 'text/plain; charset=utf-8', 'mime-type' => 'text/plain; charset=utf-8',
'authorPHID' => $actor->getPHID(), 'authorPHID' => $actor->getPHID(),
'viewPolicy' => PhabricatorPolicies::POLICY_NOONE, 'viewPolicy' => PhabricatorPolicies::POLICY_NOONE,

View file

@ -1026,8 +1026,6 @@ abstract class PhabricatorApplicationTransactionEditor
$xactions = $this->filterTransactions($object, $xactions); $xactions = $this->filterTransactions($object, $xactions);
if (!$is_preview) { if (!$is_preview) {
$this->willApplyTransactions($object, $xactions);
$this->hasRequiredMFA = true; $this->hasRequiredMFA = true;
if ($this->getShouldRequireMFA()) { if ($this->getShouldRequireMFA()) {
$this->requireMFA($object, $xactions); $this->requireMFA($object, $xactions);
@ -4374,19 +4372,6 @@ abstract class PhabricatorApplicationTransactionEditor
return idx($types, $type); return idx($types, $type);
} }
private function willApplyTransactions($object, array $xactions) {
foreach ($xactions as $xaction) {
$type = $xaction->getTransactionType();
$xtype = $this->getModularTransactionType($type);
if (!$xtype) {
continue;
}
$xtype->willApplyTransactions($object, $xactions);
}
}
public function getCreateObjectTitle($author, $object) { public function getCreateObjectTitle($author, $object) {
return pht('%s created this object.', $author); return pht('%s created this object.', $author);
} }

View file

@ -69,11 +69,6 @@ abstract class PhabricatorModularTransaction
->generateNewValue($object, $this->getNewValue()); ->generateNewValue($object, $this->getNewValue());
} }
final public function willApplyTransactions($object, array $xactions) {
return $this->getTransactionImplementation()
->willApplyTransactions($object, $xactions);
}
final public function applyInternalEffects($object) { final public function applyInternalEffects($object) {
return $this->getTransactionImplementation() return $this->getTransactionImplementation()
->applyInternalEffects($object); ->applyInternalEffects($object);

View file

@ -23,10 +23,6 @@ abstract class PhabricatorModularTransactionType
return array(); return array();
} }
public function willApplyTransactions($object, array $xactions) {
return;
}
public function applyInternalEffects($object, $value) { public function applyInternalEffects($object, $value) {
return; return;
} }