From 4787069e96fb8b664713d1c2c838139a74394151 Mon Sep 17 00:00:00 2001
From: Bob Trahan <btrahan@phacility.com>
Date: Wed, 20 May 2015 13:55:23 -0700
Subject: [PATCH] Transactions - finish making built-in transaction types
 implementation optional

Summary: Fixes T6403. Remaining built-ins were already built-in effectively so this is a small re-factor plus some docs. I probably wouldn't have written anything if not for the TODO so please feel free to tell me to write something else or what have you.

Test Plan: NA since this didn't actually change anything.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6403

Differential Revision: https://secure.phabricator.com/D12937
---
 ...habricatorApplicationTransactionEditor.php | 29 ++++++++++++-------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
index 8c9c26bfe9..57a3e4bde8 100644
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -385,12 +385,11 @@ abstract class PhabricatorApplicationTransactionEditor
     PhabricatorApplicationTransaction $xaction) {
 
     switch ($xaction->getTransactionType()) {
-      case PhabricatorTransactions::TYPE_BUILDABLE:
-      case PhabricatorTransactions::TYPE_TOKEN:
-        return;
       case PhabricatorTransactions::TYPE_CUSTOMFIELD:
         $field = $this->getCustomFieldForTransaction($object, $xaction);
         return $field->applyApplicationTransactionInternalEffects($xaction);
+      case PhabricatorTransactions::TYPE_BUILDABLE:
+      case PhabricatorTransactions::TYPE_TOKEN:
       case PhabricatorTransactions::TYPE_VIEW_POLICY:
       case PhabricatorTransactions::TYPE_EDIT_POLICY:
       case PhabricatorTransactions::TYPE_JOIN_POLICY:
@@ -408,9 +407,6 @@ abstract class PhabricatorApplicationTransactionEditor
     PhabricatorLiskDAO $object,
     PhabricatorApplicationTransaction $xaction) {
     switch ($xaction->getTransactionType()) {
-      case PhabricatorTransactions::TYPE_BUILDABLE:
-      case PhabricatorTransactions::TYPE_TOKEN:
-        return;
       case PhabricatorTransactions::TYPE_SUBSCRIBERS:
         $subeditor = id(new PhabricatorSubscriptionsEditor())
           ->setObject($object)
@@ -442,6 +438,8 @@ abstract class PhabricatorApplicationTransactionEditor
         $field = $this->getCustomFieldForTransaction($object, $xaction);
         return $field->applyApplicationTransactionExternalEffects($xaction);
       case PhabricatorTransactions::TYPE_EDGE:
+      case PhabricatorTransactions::TYPE_BUILDABLE:
+      case PhabricatorTransactions::TYPE_TOKEN:
       case PhabricatorTransactions::TYPE_VIEW_POLICY:
       case PhabricatorTransactions::TYPE_EDIT_POLICY:
       case PhabricatorTransactions::TYPE_JOIN_POLICY:
@@ -471,11 +469,17 @@ abstract class PhabricatorApplicationTransactionEditor
       "implementation!");
   }
 
-  // TODO: Write proper documentation for these hooks. These are like the
-  // "applyCustom" hooks, except that implementation is optional, so you do
-  // not need to handle all of the builtin transaction types. See T6403. These
-  // are not completely implemented.
-
+  /**
+   * @{class:PhabricatorTransactions} provides many built-in transactions
+   * which should not require much - if any - code in specific applications.
+   *
+   * This method is a hook for the exceedingly-rare cases where you may need
+   * to do **additional** work for built-in transactions. Developers should
+   * extend this method, making sure to return the parent implementation
+   * regardless of handling any transactions.
+   *
+   * See also @{method:applyBuiltinExternalTransaction}.
+   */
   protected function applyBuiltinInternalTransaction(
     PhabricatorLiskDAO $object,
     PhabricatorApplicationTransaction $xaction) {
@@ -493,6 +497,9 @@ abstract class PhabricatorApplicationTransactionEditor
     }
   }
 
+  /**
+   * See @{method::applyBuiltinInternalTransaction}.
+   */
   protected function applyBuiltinExternalTransaction(
     PhabricatorLiskDAO $object,
     PhabricatorApplicationTransaction $xaction) {