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

Make the default behavior of getApplicationTransactionCommentObject() "return null" instead of "throw"

Summary:
Depends on D20115. See <https://discourse.phabricator-community.org/t/transaction-search-endpoint-does-not-work-on-differential-diffs/2369/>.

Currently, `getApplicationTransactionCommentObject()` throws by default. Subclasses must override it to `return null` to indicate that they don't support comments.

This is silly, and leads to a bunch of code that does a `try / catch` around it, and at least some code (here, `transaction.search`) which doesn't `try / catch` and gets the wrong behavior as a result.

Just make it `return null` by default, meaning "no support for comments". Then remove the `try / catch` stuff and all the `return null` implementations.

Test Plan:
  - Grepped for `getApplicationTransactionCommentObject()`, fixed each callsite / definition.
  - Called `transaction.search` on a diff with transactions (i.e., not a sourced-from-commit diff).

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: jbrownEP

Differential Revision: https://secure.phabricator.com/D20121
This commit is contained in:
epriestley 2019-02-06 19:36:41 -08:00
parent 4fa5a2421e
commit 9f5e6bee90
30 changed files with 9 additions and 131 deletions

View file

@ -7,8 +7,4 @@ abstract class AlmanacModularTransaction
return 'almanac'; return 'almanac';
} }
public function getApplicationTransactionCommentObject() {
return null;
}
} }

View file

@ -11,10 +11,6 @@ final class PhabricatorAuthPasswordTransaction
return PhabricatorAuthPasswordPHIDType::TYPECONST; return PhabricatorAuthPasswordPHIDType::TYPECONST;
} }
public function getApplicationTransactionCommentObject() {
return null;
}
public function getBaseTransactionClass() { public function getBaseTransactionClass() {
return 'PhabricatorAuthPasswordTransactionType'; return 'PhabricatorAuthPasswordTransactionType';
} }

View file

@ -33,10 +33,6 @@ final class PhabricatorAuthProviderConfigTransaction
return PhabricatorAuthAuthProviderPHIDType::TYPECONST; return PhabricatorAuthAuthProviderPHIDType::TYPECONST;
} }
public function getApplicationTransactionCommentObject() {
return null;
}
public function getIcon() { public function getIcon() {
$old = $this->getOldValue(); $old = $this->getOldValue();
$new = $this->getNewValue(); $new = $this->getNewValue();

View file

@ -15,10 +15,6 @@ final class PhabricatorAuthSSHKeyTransaction
return PhabricatorAuthSSHKeyPHIDType::TYPECONST; return PhabricatorAuthSSHKeyPHIDType::TYPECONST;
} }
public function getApplicationTransactionCommentObject() {
return null;
}
public function getTitle() { public function getTitle() {
$author_phid = $this->getAuthorPHID(); $author_phid = $this->getAuthorPHID();

View file

@ -13,10 +13,6 @@ final class PhabricatorConfigTransaction
return PhabricatorConfigConfigPHIDType::TYPECONST; return PhabricatorConfigConfigPHIDType::TYPECONST;
} }
public function getApplicationTransactionCommentObject() {
return null;
}
public function getTitle() { public function getTitle() {
$author_phid = $this->getAuthorPHID(); $author_phid = $this->getAuthorPHID();

View file

@ -11,8 +11,4 @@ final class DivinerLiveBookTransaction
return DivinerBookPHIDType::TYPECONST; return DivinerBookPHIDType::TYPECONST;
} }
public function getApplicationTransactionCommentObject() {
return null;
}
} }

View file

@ -11,10 +11,6 @@ final class FundBackerTransaction
return FundBackerPHIDType::TYPECONST; return FundBackerPHIDType::TYPECONST;
} }
public function getApplicationTransactionCommentObject() {
return null;
}
public function getBaseTransactionClass() { public function getBaseTransactionClass() {
return 'FundBackerTransactionType'; return 'FundBackerTransactionType';
} }

View file

@ -19,14 +19,11 @@ final class HeraldCommentAction extends HeraldAction {
} }
$xaction = $object->getApplicationTransactionTemplate(); $xaction = $object->getApplicationTransactionTemplate();
try {
$comment = $xaction->getApplicationTransactionCommentObject(); $comment = $xaction->getApplicationTransactionCommentObject();
if (!$comment) { if (!$comment) {
return false; return false;
} }
} catch (PhutilMethodNotImplementedException $ex) {
return false;
}
return true; return true;
} }

View file

@ -11,10 +11,6 @@ final class HeraldWebhookTransaction
return HeraldWebhookPHIDType::TYPECONST; return HeraldWebhookPHIDType::TYPECONST;
} }
public function getApplicationTransactionCommentObject() {
return null;
}
public function getBaseTransactionClass() { public function getBaseTransactionClass() {
return 'HeraldWebhookTransactionType'; return 'HeraldWebhookTransactionType';
} }

View file

@ -16,8 +16,4 @@ final class PhabricatorMetaMTAApplicationEmailTransaction
return PhabricatorMetaMTAApplicationEmailPHIDType::TYPECONST; return PhabricatorMetaMTAApplicationEmailPHIDType::TYPECONST;
} }
public function getApplicationTransactionCommentObject() {
return null;
}
} }

View file

@ -19,10 +19,6 @@ final class PhabricatorOAuthServerTransaction
return PhabricatorOAuthServerClientPHIDType::TYPECONST; return PhabricatorOAuthServerClientPHIDType::TYPECONST;
} }
public function getApplicationTransactionCommentObject() {
return null;
}
public function getTitle() { public function getTitle() {
$author_phid = $this->getAuthorPHID(); $author_phid = $this->getAuthorPHID();
$old = $this->getOldValue(); $old = $this->getOldValue();

View file

@ -15,8 +15,4 @@ final class PhabricatorOwnersPackageTransaction
return 'PhabricatorOwnersPackageTransactionType'; return 'PhabricatorOwnersPackageTransactionType';
} }
public function getApplicationTransactionCommentObject() {
return null;
}
} }

View file

@ -11,10 +11,6 @@ final class PassphraseCredentialTransaction
return PassphraseCredentialPHIDType::TYPECONST; return PassphraseCredentialPHIDType::TYPECONST;
} }
public function getApplicationTransactionCommentObject() {
return null;
}
public function getBaseTransactionClass() { public function getBaseTransactionClass() {
return 'PassphraseCredentialTransactionType'; return 'PassphraseCredentialTransactionType';
} }

View file

@ -11,10 +11,6 @@ final class PhabricatorUserTransaction
return PhabricatorPeopleUserPHIDType::TYPECONST; return PhabricatorPeopleUserPHIDType::TYPECONST;
} }
public function getApplicationTransactionCommentObject() {
return null;
}
public function getBaseTransactionClass() { public function getBaseTransactionClass() {
return 'PhabricatorUserTransactionType'; return 'PhabricatorUserTransactionType';
} }

View file

@ -15,10 +15,6 @@ final class PhameBlogTransaction
return PhabricatorPhameBlogPHIDType::TYPECONST; return PhabricatorPhameBlogPHIDType::TYPECONST;
} }
public function getApplicationTransactionCommentObject() {
return null;
}
public function getBaseTransactionClass() { public function getBaseTransactionClass() {
return 'PhameBlogTransactionType'; return 'PhameBlogTransactionType';
} }

View file

@ -13,10 +13,6 @@ final class PhluxTransaction extends PhabricatorApplicationTransaction {
return PhluxVariablePHIDType::TYPECONST; return PhluxVariablePHIDType::TYPECONST;
} }
public function getApplicationTransactionCommentObject() {
return null;
}
public function getTitle() { public function getTitle() {
$author_phid = $this->getAuthorPHID(); $author_phid = $this->getAuthorPHID();

View file

@ -11,10 +11,6 @@ final class PhortuneAccountTransaction
return PhortuneAccountPHIDType::TYPECONST; return PhortuneAccountPHIDType::TYPECONST;
} }
public function getApplicationTransactionCommentObject() {
return null;
}
public function getBaseTransactionClass() { public function getBaseTransactionClass() {
return 'PhortuneAccountTransactionType'; return 'PhortuneAccountTransactionType';
} }

View file

@ -19,10 +19,6 @@ final class PhortuneCartTransaction
return PhortuneCartPHIDType::TYPECONST; return PhortuneCartPHIDType::TYPECONST;
} }
public function getApplicationTransactionCommentObject() {
return null;
}
public function shouldHideForMail(array $xactions) { public function shouldHideForMail(array $xactions) {
switch ($this->getTransactionType()) { switch ($this->getTransactionType()) {
case self::TYPE_CREATED: case self::TYPE_CREATED:

View file

@ -11,10 +11,6 @@ final class PhortuneMerchantTransaction
return PhortuneMerchantPHIDType::TYPECONST; return PhortuneMerchantPHIDType::TYPECONST;
} }
public function getApplicationTransactionCommentObject() {
return null;
}
public function getBaseTransactionClass() { public function getBaseTransactionClass() {
return 'PhortuneMerchantTransactionType'; return 'PhortuneMerchantTransactionType';
} }

View file

@ -17,10 +17,6 @@ final class PhortunePaymentProviderConfigTransaction
return PhortunePaymentProviderPHIDType::TYPECONST; return PhortunePaymentProviderPHIDType::TYPECONST;
} }
public function getApplicationTransactionCommentObject() {
return null;
}
public function getTitle() { public function getTitle() {
$author_phid = $this->getAuthorPHID(); $author_phid = $this->getAuthorPHID();

View file

@ -19,10 +19,6 @@ final class PhabricatorProjectTransaction
return PhabricatorProjectProjectPHIDType::TYPECONST; return PhabricatorProjectProjectPHIDType::TYPECONST;
} }
public function getApplicationTransactionCommentObject() {
return null;
}
public function getBaseTransactionClass() { public function getBaseTransactionClass() {
return 'PhabricatorProjectTransactionType'; return 'PhabricatorProjectTransactionType';
} }

View file

@ -11,10 +11,6 @@ final class PhabricatorRepositoryTransaction
return PhabricatorRepositoryRepositoryPHIDType::TYPECONST; return PhabricatorRepositoryRepositoryPHIDType::TYPECONST;
} }
public function getApplicationTransactionCommentObject() {
return null;
}
public function getBaseTransactionClass() { public function getBaseTransactionClass() {
return 'PhabricatorRepositoryTransactionType'; return 'PhabricatorRepositoryTransactionType';
} }

View file

@ -70,14 +70,10 @@ final class PhabricatorFulltextIndexEngineExtension
private function getCommentVersion($object) { private function getCommentVersion($object) {
$xaction = $object->getApplicationTransactionTemplate(); $xaction = $object->getApplicationTransactionTemplate();
try {
$comment = $xaction->getApplicationTransactionCommentObject(); $comment = $xaction->getApplicationTransactionCommentObject();
if (!$comment) { if (!$comment) {
return 'none'; return 'none';
} }
} catch (Exception $ex) {
return 'none';
}
$comment_row = queryfx_one( $comment_row = queryfx_one(
$comment->establishConnection('r'), $comment->establishConnection('r'),

View file

@ -20,8 +20,4 @@ final class PhabricatorProfileMenuItemConfigurationTransaction
return PhabricatorProfileMenuItemPHIDType::TYPECONST; return PhabricatorProfileMenuItemPHIDType::TYPECONST;
} }
public function getApplicationTransactionCommentObject() {
return null;
}
} }

View file

@ -11,10 +11,6 @@ final class PhabricatorUserPreferencesTransaction
return 'user'; return 'user';
} }
public function getApplicationTransactionCommentObject() {
return null;
}
public function getApplicationTransactionType() { public function getApplicationTransactionType() {
return PhabricatorUserPreferencesPHIDType::TYPECONST; return PhabricatorUserPreferencesPHIDType::TYPECONST;
} }

View file

@ -11,10 +11,6 @@ final class PhabricatorSpacesNamespaceTransaction
return PhabricatorSpacesNamespacePHIDType::TYPECONST; return PhabricatorSpacesNamespacePHIDType::TYPECONST;
} }
public function getApplicationTransactionCommentObject() {
return null;
}
public function getBaseTransactionClass() { public function getBaseTransactionClass() {
return 'PhabricatorSpacesNamespaceTransactionType'; return 'PhabricatorSpacesNamespaceTransactionType';
} }

View file

@ -360,12 +360,7 @@ abstract class PhabricatorApplicationTransactionEditor
} }
if ($template) { if ($template) {
try {
$comment = $template->getApplicationTransactionCommentObject(); $comment = $template->getApplicationTransactionCommentObject();
} catch (PhutilMethodNotImplementedException $ex) {
$comment = null;
}
if ($comment) { if ($comment) {
$types[] = PhabricatorTransactions::TYPE_COMMENT; $types[] = PhabricatorTransactions::TYPE_COMMENT;
} }

View file

@ -23,12 +23,7 @@ final class PhabricatorCommentEditEngineExtension
PhabricatorApplicationTransactionInterface $object) { PhabricatorApplicationTransactionInterface $object) {
$xaction = $object->getApplicationTransactionTemplate(); $xaction = $object->getApplicationTransactionTemplate();
try {
$comment = $xaction->getApplicationTransactionCommentObject(); $comment = $xaction->getApplicationTransactionCommentObject();
} catch (PhutilMethodNotImplementedException $ex) {
$comment = null;
}
return (bool)$comment; return (bool)$comment;
} }

View file

@ -76,7 +76,7 @@ abstract class PhabricatorApplicationTransaction
} }
public function getApplicationTransactionCommentObject() { public function getApplicationTransactionCommentObject() {
throw new PhutilMethodNotImplementedException(); return null;
} }
public function getMetadataValue($key, $default = null) { public function getMetadataValue($key, $default = null) {
@ -1731,12 +1731,7 @@ abstract class PhabricatorApplicationTransaction
PhabricatorDestructionEngine $engine) { PhabricatorDestructionEngine $engine) {
$this->openTransaction(); $this->openTransaction();
$comment_template = null;
try {
$comment_template = $this->getApplicationTransactionCommentObject(); $comment_template = $this->getApplicationTransactionCommentObject();
} catch (Exception $ex) {
// Continue; no comments for these transactions.
}
if ($comment_template) { if ($comment_template) {
$comments = $comment_template->loadAllWhere( $comments = $comment_template->loadAllWhere(

View file

@ -23,10 +23,6 @@ final class PhabricatorEditEngineConfigurationTransaction
return PhabricatorEditEngineConfigurationPHIDType::TYPECONST; return PhabricatorEditEngineConfigurationPHIDType::TYPECONST;
} }
public function getApplicationTransactionCommentObject() {
return null;
}
public function getTitle() { public function getTitle() {
$author_phid = $this->getAuthorPHID(); $author_phid = $this->getAuthorPHID();