From 50868e8df1dd501961d7298f59f8b83c08e2a068 Mon Sep 17 00:00:00 2001
From: Coleman Watts <coleman@civicrm.org>
Date: Tue, 26 Nov 2019 13:33:53 -0500
Subject: [PATCH] Api improvements and test fix

Improve efficiency by not fetching layout if not needed in api.get
Fix test failure by correctly merging existing metadata during update
---
 .../core/Civi/Api4/Action/Afform/Get.php      |  2 +-
 .../core/Civi/Api4/Utils/AfformSaveTrait.php  | 27 ++++++++++---------
 ext/afform/core/afform.php                    | 17 +++++-------
 3 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/ext/afform/core/Civi/Api4/Action/Afform/Get.php b/ext/afform/core/Civi/Api4/Action/Afform/Get.php
index cb059391fe..92d5df4564 100644
--- a/ext/afform/core/Civi/Api4/Action/Afform/Get.php
+++ b/ext/afform/core/Civi/Api4/Action/Afform/Get.php
@@ -19,7 +19,7 @@ class Get extends \Civi\Api4\Generic\BasicGetAction {
     $values = [];
     foreach ($names as $name) {
       $record = $scanner->getMeta($name);
-      $layout = $scanner->getLayout($name);
+      $layout = $this->_isFieldSelected('layout') ? $scanner->getLayout($name) : NULL;
       if ($layout !== NULL) {
         // FIXME check for validity?
         $record['layout'] = $this->convertHtmlToOutput($layout);
diff --git a/ext/afform/core/Civi/Api4/Utils/AfformSaveTrait.php b/ext/afform/core/Civi/Api4/Utils/AfformSaveTrait.php
index ff0ada980e..406f99ea4b 100644
--- a/ext/afform/core/Civi/Api4/Utils/AfformSaveTrait.php
+++ b/ext/afform/core/Civi/Api4/Utils/AfformSaveTrait.php
@@ -25,31 +25,34 @@ trait AfformSaveTrait {
         $suffix++;
       }
       $item['name'] .= $suffix;
+      $orig = NULL;
     }
     elseif (!preg_match('/^[a-zA-Z][a-zA-Z0-9\-]*$/', $item['name'])) {
       throw new \API_Exception("Afform.{$this->getActionName()}: name should use alphanumerics and dashes.");
     }
+    else {
+      // Fetch existing metadata
+      $fields = \Civi\Api4\Afform::getfields()->setCheckPermissions(FALSE)->addSelect('name')->execute()->column('name');
+      unset($fields[array_search('layout', $fields)]);
+      unset($fields[array_search('name', $fields)]);
+      $orig = \Civi\Api4\Afform::get()->setCheckPermissions(FALSE)->addWhere('name', '=', $item['name'])->setSelect($fields)->execute()->first();
+    }
 
     // FIXME validate all field data.
-    $updates = _afform_fields_filter($item);
+    $item = _afform_fields_filter($item);
 
     // Create or update aff.html.
-    if (isset($updates['layout'])) {
+    if (isset($item['layout'])) {
       $layoutPath = $scanner->createSiteLocalPath($item['name'], 'aff.html');
       \CRM_Utils_File::createDir(dirname($layoutPath));
-      file_put_contents($layoutPath, $this->convertInputToHtml($updates['layout']));
+      file_put_contents($layoutPath, $this->convertInputToHtml($item['layout']));
       // FIXME check for writability then success. Report errors.
     }
 
-    $orig = NULL;
-    $meta = $updates;
-    unset($meta['layout']);
-    unset($meta['name']);
+    $meta = $item + (array) $orig;
+    unset($meta['layout'], $meta['name']);
     if (!empty($meta)) {
       $metaPath = $scanner->createSiteLocalPath($item['name'], \CRM_Afform_AfformScanner::METADATA_FILE);
-      if (file_exists($metaPath)) {
-        $orig = $scanner->getMeta($item['name']);
-      }
       \CRM_Utils_File::createDir(dirname($metaPath));
       file_put_contents($metaPath, json_encode($meta, JSON_PRETTY_PRINT));
       // FIXME check for writability then success. Report errors.
@@ -58,13 +61,13 @@ trait AfformSaveTrait {
     // We may have changed list of files covered by the cache.
     _afform_clear();
 
-    if (($updates['server_route'] ?? NULL) !== ($orig['server_route'] ?? NULL)) {
+    if (($item['server_route'] ?? NULL) !== ($orig['server_route'] ?? NULL)) {
       \CRM_Core_Menu::store();
       \CRM_Core_BAO_Navigation::resetNavigation();
     }
     // FIXME if asset-caching is enabled, then flush the asset cache.
 
-    return $updates;
+    return $meta + $item;
   }
 
 }
diff --git a/ext/afform/core/afform.php b/ext/afform/core/afform.php
index eee6d1d025..273ed37c83 100644
--- a/ext/afform/core/afform.php
+++ b/ext/afform/core/afform.php
@@ -4,10 +4,6 @@ require_once 'afform.civix.php';
 use CRM_Afform_ExtensionUtil as E;
 use Civi\Api4\Action\Afform\Submit;
 
-function _afform_fields() {
-  return ['name', 'title', 'description', 'requires', 'layout', 'server_route', 'is_public'];
-}
-
 /**
  * Filter the content of $params to only have supported afform fields.
  *
@@ -16,15 +12,16 @@ function _afform_fields() {
  */
 function _afform_fields_filter($params) {
   $result = [];
-  foreach (_afform_fields() as $field) {
-    if (isset($params[$field])) {
-      $result[$field] = $params[$field];
+  $fields = \Civi\Api4\Afform::getfields()->setCheckPermissions(FALSE)->execute()->indexBy('name');
+  foreach ($fields as $fieldName => $field) {
+    if (isset($params[$fieldName])) {
+      $result[$fieldName] = $params[$fieldName];
     }
 
-    if (isset($result[$field])) {
-      switch ($field) {
+    if (isset($result[$fieldName])) {
+      switch ($fieldName) {
         case 'is_public':
-          $result[$field] = CRM_Utils_String::strtobool($result[$field]);
+          $result[$fieldName] = CRM_Utils_String::strtobool($result[$fieldName]);
           break;
 
       }
-- 
2.25.1