From 7baa0941b95640fcb3c11da697d6938f55c572dc Mon Sep 17 00:00:00 2001
From: James Rhodes <jrhodes@redpointsoftware.com.au>
Date: Thu, 3 Jul 2014 13:49:57 +1000
Subject: [PATCH] Inlines for custom herald actions

Summary: Ref D8784.  Didn't see all of the inlines before hitting `arc land`.  This fixes up the issues raised (and makes all the code nicer).

Test Plan: Made sure custom actions only appear for appropriate adapters and checked to ensure that they triggered correctly.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: edutibau, ite-klass, epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9796
---
 .../herald/adapter/HeraldAdapter.php          | 63 ++++++++++---------
 1 file changed, 35 insertions(+), 28 deletions(-)

diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php
index 65749eb004..b12ffebdf9 100644
--- a/src/applications/herald/adapter/HeraldAdapter.php
+++ b/src/applications/herald/adapter/HeraldAdapter.php
@@ -106,15 +106,28 @@ abstract class HeraldAdapter {
 
   public function getCustomActions() {
     if ($this->customActions === null) {
-      $this->customActions = id(new PhutilSymbolLoader())
+      $custom_actions = id(new PhutilSymbolLoader())
         ->setAncestorClass('HeraldCustomAction')
         ->loadObjects();
 
-      foreach ($this->customActions as $key => $object) {
+      foreach ($custom_actions as $key => $object) {
         if (!$object->appliesToAdapter($this)) {
-          unset($this->customActions[$key]);
+          unset($custom_actions[$key]);
         }
       }
+
+      $this->customActions = array();
+      foreach ($custom_actions as $action) {
+        $key = $action->getActionKey();
+
+        if (array_key_exists($key, $this->customActions)) {
+          throw new Exception(
+            'More than one Herald custom action implementation '.
+            'handles the action key: \''.$key.'\'.');
+        }
+
+        $this->customActions[$key] = $action;
+      }
     }
 
     return $this->customActions;
@@ -163,20 +176,16 @@ abstract class HeraldAdapter {
     }
   }
 
-  public abstract function applyHeraldEffects(array $effects);
+  abstract public function applyHeraldEffects(array $effects);
 
   protected function handleCustomHeraldEffect(HeraldEffect $effect) {
-    foreach ($this->getCustomActions() as $custom_action) {
-      if ($effect->getAction() == $custom_action->getActionKey()) {
-        $result = $custom_action->applyEffect(
-          $this,
-          $this->getObject(),
-          $effect);
+    $custom_action = idx($this->getCustomActions(), $effect->getAction());
 
-        if ($result !== null) {
-          return $result;
-        }
-      }
+    if ($custom_action !== null) {
+      return $custom_action->applyEffect(
+        $this,
+        $this->getObject(),
+        $effect);
     }
 
     return null;
@@ -688,16 +697,21 @@ abstract class HeraldAdapter {
 
 /* -(  Actions  )------------------------------------------------------------ */
 
-  public function getActions($rule_type) {
+  public function getCustomActionsForRuleType($rule_type) {
     $results = array();
     foreach ($this->getCustomActions() as $custom_action) {
       if ($custom_action->appliesToRuleType($rule_type)) {
-        $results[] = $custom_action->getActionKey();
+        $results[] = $custom_action;
       }
     }
     return $results;
   }
 
+  public function getActions($rule_type) {
+    $custom_actions = $this->getCustomActionsForRuleType($rule_type);
+    return mpull($custom_actions, 'getActionKey');
+  }
+
   public function getActionNameMap($rule_type) {
     switch ($rule_type) {
       case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL:
@@ -737,12 +751,8 @@ abstract class HeraldAdapter {
         throw new Exception("Unknown rule type '{$rule_type}'!");
     }
 
-    foreach ($this->getCustomActions() as $custom_action) {
-      if ($custom_action->appliesToRuleType($rule_type)) {
-        $standard[$custom_action->getActionKey()] =
-          $custom_action->getActionName();
-      }
-    }
+    $custom_actions = $this->getCustomActionsForRuleType($rule_type);
+    $standard += mpull($custom_actions, 'getActionName', 'getActionKey');
 
     return $standard;
   }
@@ -922,12 +932,9 @@ abstract class HeraldAdapter {
       }
     }
 
-    foreach ($this->getCustomActions() as $custom_action) {
-      if ($custom_action->appliesToRuleType($rule_type)) {
-        if ($action === $custom_action->getActionKey()) {
-          return $custom_action->getActionType();
-        }
-      }
+    $custom_action = idx($this->getCustomActions(), $action);
+    if ($custom_action !== null) {
+      return $custom_action->getActionType();
     }
 
     throw new Exception("Unknown or invalid action '".$action."'.");