fix status check and remove side effects; add confirm page to updating stripe webhooks

CRM/Stripe/Form/UpdateWebhook.php

+<?php
+
+use CRM_Stripe_ExtensionUtil as E;
+
+/**
+ * Form controller class
+ *
+ * @see https://wiki.civicrm.org/confluence/display/CRMDOC/QuickForm+Reference
+ */
+class CRM_Stripe_Form_UpdateWebhook extends CRM_Core_Form {
+
+  public function buildQuickForm() {
+    // Defaults.
+    $this->assign('shouldOfferToFix', 0);
+    $this->assign('isStillBad', 0);
+    $this->assign('isAllOk', 0);
+
+    // Run check.
+    $messages = [];
+    CRM_Stripe_Webhook::check($messages);
+    if (!$messages) {
+      $this->assign('isAllOk', 1);
+    }
+    else {
+      $this->assign('shouldOfferToFix', 1);
+      $this->assignMessages($messages);
+
+      $this->addButtons(array(
+        array(
+          'type' => 'submit',
+          'name' => E::ts('Update / Create webhook'),
+          'isDefault' => TRUE,
+        ),
+      ));
+    }
+
+    // export form elements
+    $this->assign('elementNames', $this->getRenderableElementNames());
+    parent::buildQuickForm();
+  }
+
+  public function postProcess() {
+    $messages = [];
+    $attemptFix = TRUE;
+    CRM_Stripe_Webhook::check($messages, $attemptFix);
+
+    if ($messages) {
+      $this->assign('isStillBad', 1);
+      $this->assign('shouldOfferToFix', 0);
+      $this->assignMessages($messages);
+    }
+    else {
+      $this->assign('isAllOk', 1);
+      $this->assign('shouldOfferToFix', 0);
+      $this->assign('isStillBad', 0);
+      $this->assign('intro', E::ts('All webhooks update successfully.'));
+    }
+
+    parent::postProcess();
+  }
+
+  /**
+   * @param array $messages
+   */
+  private function assignMessages($messages) {
+    $messagesArray = [];
+    foreach ($messages as $message) {
+      $messagesArray[] = [
+        'title' => $message->getTitle(),
+        'message' => $message->getMessage(),
+      ];
+    }
+    $this->assign('messages', $messagesArray);
+  }
+
+  /**
+   * Get the fields/elements defined in this form.
+   *
+   * @return array (string)
+   */
+  public function getRenderableElementNames() {
+    // The _elements list includes some items which should not be
+    // auto-rendered in the loop -- such as "qfKey" and "buttons".  These
+    // items don't have labels.  We'll identify renderable by filtering on
+    // the 'label'.
+    $elementNames = array();
+    foreach ($this->_elements as $element) {
+      /** @var HTML_QuickForm_Element $element */
+      $label = $element->getLabel();
+      if (!empty($label)) {
+        $elementNames[] = $element->getName();
+      }
+    }
+    return $elementNames;
+  }
+
+}

CRM/Stripe/Webhook.php

@@ -7,24 +7,23 @@ class CRM_Stripe_Webhook {
   use CRM_Stripe_WebhookTrait;
 
   /**
-   * Checks whether the payment processors have a correctly configured
-   * webhook (we may want to check the test processors too, at some point, but
-   * for now, avoid having false alerts that will annoy people).
+   * Checks whether the payment processors have a correctly configured webhook
    *
    * @see stripe_civicrm_check()
    *
    * @param array $messages
+   * @param bool $attemptFix If TRUE, try to fix the webhook.
    *
-   * @return array
    * @throws \CiviCRM_API3_Exception
    */
-  public static function check($messages) {
+  public static function check(&$messages, $attemptFix = FALSE) {
     $result = civicrm_api3('PaymentProcessor', 'get', [
       'class_name' => 'Payment_Stripe',
       'is_active' => 1,
     ]);
 
     foreach ($result['values'] as $paymentProcessor) {
+      $messageTexts = [];
       $webhook_path = self::getWebhookPath($paymentProcessor['id']);
 
       \Stripe\Stripe::setApiKey(CRM_Core_Payment_Stripe::getSecretKey($paymentProcessor));
@@ -35,12 +34,11 @@ class CRM_Stripe_Webhook {
         $error = $e->getMessage();
         $messages[] = new CRM_Utils_Check_Message(
           'stripe_webhook',
-          E::ts('The %1 (%2) Payment Processor has an error: %3', [
+          $error,
+          E::ts('Stripe Payment Processor: %1 (%2)', [
             1 => $paymentProcessor['name'],
             2 => $paymentProcessor['id'],
-            3 => $error,
           ]),
-          E::ts('Stripe - API Key'),
           \Psr\Log\LogLevel::ERROR,
           'fa-money'
         );
@@ -54,50 +52,63 @@ class CRM_Stripe_Webhook {
           $found_wh = TRUE;
           // Check and update webhook
           try {
-            self::checkAndUpdateWebhook($wh);
+            $updates = self::checkWebhook($wh);
+            if ($updates) {
+              if ($attemptFix) {
+                // We should try to update the webhook.
+                $messageTexts[] = E::ts('Unable to update the webhook %1. To correct this please delete the webhook at Stripe and then revisit this page which will recreate it correctly.',
+                  [1 => urldecode($webhook_path)]
+                );
+                \Stripe\WebhookEndpoint::update($wh['id'], $updates);
+              }
+              else {
+                $messageTexts[] = E::ts('Problems detected with Stripe webhook %1. Please visit <a href="%2">Fix Stripe Webhook</a> to fix.', [
+                  1 => urldecode($webhook_path),
+                  2 => CRM_Utils_System::url('civicrm/stripe/fix-webhook'),
+                ]);
+              }
+            }
           }
           catch (Exception $e) {
-            $messages[] = new CRM_Utils_Check_Message(
-              'stripe_webhook',
-              E::ts('Could not update existing webhook - to fix this delete the existing webhook from your stripe account and re-run this check.<br/>The webhook URL is: %3', [
-                1 => $paymentProcessor['name'],
-                2 => $paymentProcessor['id'],
-                3 => urldecode($webhook_path),
-              ]) . '.<br/>Error from Stripe: <em>' . $e->getMessage() . '</em>',
-              E::ts('Stripe Webhook: %1 (%2)', [
-                  1 => $paymentProcessor['name'],
-                  2 => $paymentProcessor['id'],
-                ]
-              ),
-              \Psr\Log\LogLevel::WARNING,
-              'fa-money'
+            $messageTexts[] = E::ts('Could not check/update existing webhooks, got error from stripe <em>%1</em>', [
+                1 => htmlspecialchars($e->getMessage())
+              ]
             );
           }
         }
       }
 
       if (!$found_wh) {
-        try {
-          self::createWebhook($paymentProcessor['id']);
+        if ($attemptFix) {
+          try {
+            // Try to create one.
+            self::createWebhook($paymentProcessor['id']);
+          }
+          catch (Exception $e) {
+            $messageTexts[] = E::ts('Could not create webhook, got error from stripe <em>%1</em>', [
+              1 => htmlspecialchars($e->getMessage())
+            ]);
+          }
         }
-        catch (Exception $e) {
-          $messages[] = new CRM_Utils_Check_Message(
-            'stripe_webhook',
-            E::ts('Could not create webhook. You can review from your Stripe account, under Developers > Webhooks.<br/>The webhook URL is: %3', [
-              1 => $paymentProcessor['name'],
-              2 => $paymentProcessor['id'],
-              3 => urldecode($webhook_path),
-            ]) . '.<br/>Error from Stripe: <em>' . $e->getMessage() . '</em>',
-            E::ts('Stripe Webhook: %1 (%2)', [
-                1 => $paymentProcessor['name'],
-                2 => $paymentProcessor['id'],
-              ]
-            ),
-            \Psr\Log\LogLevel::WARNING,
-            'fa-money'
-          );
+        else {
+          $messageTexts[] = E::ts('Stripe Webhook missing! Please visit <a href="%1">Fix Stripe Webhook</a> to fix.', [
+            1 => CRM_Utils_System::url('civicrm/stripe/fix-webhook'),
+          ]);
         }
       }
+
+      foreach ($messageTexts as $messageText) {
+        $messages[] = new CRM_Utils_Check_Message(
+          'stripe_webhook',
+          $messageText,
+          E::ts('Stripe Payment Processor Webhook: %1 (%2)', [
+            1 => $paymentProcessor['name'],
+            2 => $paymentProcessor['id'],
+          ]),
+          \Psr\Log\LogLevel::WARNING,
+          'fa-money'
+        );
+      }
     }
   }
 
@@ -118,24 +129,25 @@ class CRM_Stripe_Webhook {
     \Stripe\WebhookEndpoint::create($params);
   }
 
+
   /**
    * Check and update existing webhook
    *
    * @param array $webhook
+   * @return array of correction params. Empty array if it's OK.
    */
-  public static function checkAndUpdateWebhook($webhook) {
-    $update = FALSE;
+  public static function checkWebhook($webhook) {
+    $params = [];
+
     if (empty($webhook['api_version']) || ($webhook['api_version'] !== CRM_Core_Payment_Stripe::API_VERSION)) {
-      $update = TRUE;
       $params['api_version'] = CRM_Core_Payment_Stripe::API_VERSION;
     }
+
     if (array_diff(self::getDefaultEnabledEvents(), $webhook['enabled_events'])) {
-      $update = TRUE;
       $params['enabled_events'] = self::getDefaultEnabledEvents();
     }
-    if ($update) {
-      \Stripe\WebhookEndpoint::update($webhook['id'], $params);
-    }
+
+    return $params;
   }
 
   /**

docs/release/release_notes.md

+## Release 5.4.1
+* Don't overwrite system messages when performing webhook checks.
+* Add form to handle creating/updating webhooks instead of automatically during system check (Thanks @artfulrobot)
+
 ## Release 5.4
 This release fixes multiple bugs and introduces a few small features.
 

info.xml

@@ -12,8 +12,8 @@
     <author>Matthew Wire (MJW Consulting)</author>
     <email>mjw@mjwconsult.co.uk</email>
   </maintainer>
-  <releaseDate>2019-07-10</releaseDate>
-  <version>5.4</version>
+  <releaseDate>2019-07-21</releaseDate>
+  <version>5.4.1</version>
   <develStage>stable</develStage>
   <compatibility>
     <ver>5.13</ver>

stripe.civix.php

@@ -263,16 +263,17 @@ function _stripe_civix_find_files($dir, $pattern) {
  */
 function _stripe_civix_civicrm_managed(&$entities) {
   $mgdFiles = _stripe_civix_find_files(__DIR__, '*.mgd.php');
+  sort($mgdFiles);
   foreach ($mgdFiles as $file) {
     $es = include $file;
     foreach ($es as $e) {
       if (empty($e['module'])) {
         $e['module'] = E::LONG_NAME;
       }
-      $entities[] = $e;
       if (empty($e['params']['version'])) {
         $e['params']['version'] = '3';
       }
+      $entities[] = $e;
     }
   }
 }
@@ -352,8 +353,10 @@ function _stripe_civix_glob($pattern) {
  * Inserts a navigation menu item at a given place in the hierarchy.
  *
  * @param array $menu - menu hierarchy
- * @param string $path - path where insertion should happen (ie. Administer/System Settings)
- * @param array $item - menu you need to insert (parent/child attributes will be filled for you)
+ * @param string $path - path to parent of this item, e.g. 'my_extension/submenu'
+ *    'Mailing', or 'Administer/System Settings'
+ * @param array $item - the item to insert (parent/child attributes will be
+ *    filled for you)
  */
 function _stripe_civix_insert_navigation_menu(&$menu, $path, $item) {
   // If we are done going down the path, insert menu
@@ -443,3 +446,16 @@ function _stripe_civix_civicrm_alterSettingsFolders(&$metaDataFolders = NULL) {
     $metaDataFolders[] = $settingsDir;
   }
 }
+
+/**
+ * (Delegated) Implements hook_civicrm_entityTypes().
+ *
+ * Find any *.entityType.php files, merge their content, and return.
+ *
+ * @link http://wiki.civicrm.org/confluence/display/CRMDOC/hook_civicrm_entityTypes
+ */
+
+function _stripe_civix_civicrm_entityTypes(&$entityTypes) {
+  $entityTypes = array_merge($entityTypes, array (
+  ));
+}

stripe.mgd.php

@@ -16,7 +16,7 @@ return [
       'description' => 'Stripe Payment Processor',
       'class_name' => 'Payment_Stripe',
       'user_name_label' => 'Secret Key',
-      'password_label' => 'Public Key',
+      'password_label' => 'Publishable key',
       'url_site_default' => 'https://api.stripe.com/v2',
       'url_recur_default' => 'https://api.stripe.com/v2',
       'url_site_test_default' => 'https://api.stripe.com/v2',

templates/CRM/Stripe/Form/UpdateWebhook.tpl

+{if $isAllOk}
+  <div class="alert alert-success">{$intro}</div>
+{/if}
+{if $shouldOfferToFix}
+    <h3>Problems discovered:</h3>
+  <div class="alert alert-warning">
+    <ul>
+    {foreach from=$messages item=message}
+      <li>{$message.title}: {$message.message}</li>
+    {/foreach}
+    </ul>
+  </div>
+  <div class="alert alert-info">
+    <p>Please click the Update / Create webhook button to
+      attempt to fix.</p>
+    <p>This will attempt to check and correct your Stripe webhooks. Note: do not
+    run this in a development environment unless you want a webhook set up that
+    points to your development domain(!).</p>
+  </div>
+{/if}
+
+{if $isStillBad}
+  <div class="alert alert-danger">
+    <h3>There were errors updating the webhook(s):</h3>
+    <ul>
+        {foreach from=$messages item=message}
+          <li>{$message.title}: {$message.message}</li>
+        {/foreach}
+    </ul>
+  </div>
+  <div class="alert alert-info">
+    The easiest way to fix this, is to
+    delete your webhooks from your Stripe account(s) and then revisit this page
+    to recreate them correctly.
+  </div>
+{/if}
+{* FOOTER *}
+{if $shouldOfferToFix || $isStillBad}
+  <div class="crm-submit-buttons">
+  {include file="CRM/common/formButtons.tpl" location="bottom"}
+  </div>
+{/if}

xml/Menu/stripe.xml

+<?xml version="1.0"?>
+<menu>
+  <item>
+    <path>civicrm/stripe/fix-webhook</path>
+    <page_callback>CRM_Stripe_Form_UpdateWebhook</page_callback>
+    <title>Check and Fix Stripe Webhooks</title>
+    <access_arguments>access CiviCRM</access_arguments>
+  </item>
+</menu>