From 292054ac6d60acdd133edc7fa92db0f2aa99d3a5 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Mon, 1 Mar 2021 13:54:30 -0500 Subject: [PATCH 1/2] Afform - Remove ngRoute from afformStandalone page Afforms are meant to work as standalone directives so routing on the standalone page was meaningless --- Civi/Angular/AngularLoader.php | 2 +- .../core/CRM/Afform/Page/AfformBase.php | 19 ++++++------------- ext/afform/core/afform.php | 2 ++ ext/afform/core/ang/afformStandalone.ang.php | 2 +- ext/afform/core/ang/afformStandalone.js | 15 ++------------- .../templates/CRM/Afform/Page/AfformBase.tpl | 10 +++++----- .../phpunit/api/v4/AfformRoutingTest.php | 13 +++++++------ 7 files changed, 24 insertions(+), 39 deletions(-) diff --git a/Civi/Angular/AngularLoader.php b/Civi/Angular/AngularLoader.php index 4d92fa81f37b..2920663898cd 100644 --- a/Civi/Angular/AngularLoader.php +++ b/Civi/Angular/AngularLoader.php @@ -73,7 +73,7 @@ public function __construct() { $this->res = \CRM_Core_Resources::singleton(); $this->angular = \Civi::service('angular'); $this->region = \CRM_Utils_Request::retrieve('snippet', 'String') ? 'ajax-snippet' : 'html-header'; - $this->pageName = $_GET['q'] ?? NULL; + $this->pageName = \CRM_Utils_System::currentPath(); $this->modules = []; } diff --git a/ext/afform/core/CRM/Afform/Page/AfformBase.php b/ext/afform/core/CRM/Afform/Page/AfformBase.php index a2981b63aaa8..fa8325d5a58c 100644 --- a/ext/afform/core/CRM/Afform/Page/AfformBase.php +++ b/ext/afform/core/CRM/Afform/Page/AfformBase.php @@ -7,25 +7,18 @@ public function run() { // To avoid php complaints about the number of args passed to this function vs the base function [$pagePath, $pageArgs] = func_get_args(); + // The api will throw an exception if afform is not found (because of the index 0 param) $afform = civicrm_api4('Afform', 'get', [ 'checkPermissions' => FALSE, 'where' => [['name', '=', $pageArgs['afform']]], 'select' => ['title', 'module_name', 'directive_name'], ], 0); - $this->set('afModule', $afform['module_name']); - - $loader = new \Civi\Angular\AngularLoader(); - $loader->setModules([$afform['module_name'], 'afformStandalone']); - $loader->setPageName(implode('/', $pagePath)); - $loader->getRes()->addSetting([ - 'afform' => [ - 'open' => $afform['directive_name'], - ], - ]) - // TODO: Allow afforms to declare their own theming requirements - ->addBundle('bootstrap3'); - $loader->load(); + $this->assign('directive', $afform['directive_name']); + + (new \Civi\Angular\AngularLoader()) + ->setModules([$afform['module_name'], 'afformStandalone']) + ->load(); if (!empty($afform['title'])) { $title = strip_tags($afform['title']); diff --git a/ext/afform/core/afform.php b/ext/afform/core/afform.php index 87c4e2433756..e34c55892265 100644 --- a/ext/afform/core/afform.php +++ b/ext/afform/core/afform.php @@ -197,6 +197,8 @@ function afform_civicrm_angularModules(&$angularModules) { 'basePages' => [], 'partialsCallback' => '_afform_get_partials', '_afform' => $afform['name'], + // TODO: Allow afforms to declare their own theming requirements + 'bundles' => ['bootstrap3'], 'exports' => [ $afform['directive_name'] => 'E', ], diff --git a/ext/afform/core/ang/afformStandalone.ang.php b/ext/afform/core/ang/afformStandalone.ang.php index 3ec3762b42d1..f8b852a5d3d2 100644 --- a/ext/afform/core/ang/afformStandalone.ang.php +++ b/ext/afform/core/ang/afformStandalone.ang.php @@ -6,5 +6,5 @@ ], 'css' => [], 'settings' => [], - 'requires' => ['ngRoute'], + 'requires' => [], ]; diff --git a/ext/afform/core/ang/afformStandalone.js b/ext/afform/core/ang/afformStandalone.js index 6de4b39192b9..626551e40ca7 100644 --- a/ext/afform/core/ang/afformStandalone.js +++ b/ext/afform/core/ang/afformStandalone.js @@ -1,16 +1,5 @@ (function(angular, $, _) { - // Declare a list of dependencies. - angular.module('afformStandalone', CRM.angRequires('afformStandalone')); - - angular.module('afformStandalone', CRM.angular.modules) - .config(function($routeProvider) { - $routeProvider.when('/', { - controller: 'AfformStandalonePageCtrl', - template: function() { - return '
<' + CRM.afform.open + '>
'; - } - }); - }) - .controller('AfformStandalonePageCtrl', function($scope) {}); + // Empty module just loads all available modules. + angular.module('afformStandalone', CRM.angular.modules); })(angular, CRM.$, CRM._); diff --git a/ext/afform/core/templates/CRM/Afform/Page/AfformBase.tpl b/ext/afform/core/templates/CRM/Afform/Page/AfformBase.tpl index 0637db770d1e..9fd1c1b6298d 100644 --- a/ext/afform/core/templates/CRM/Afform/Page/AfformBase.tpl +++ b/ext/afform/core/templates/CRM/Afform/Page/AfformBase.tpl @@ -1,5 +1,5 @@ -{literal} -
-
-
-{/literal} +
+
+ <{$directive}> +
+
diff --git a/ext/afform/mock/tests/phpunit/api/v4/AfformRoutingTest.php b/ext/afform/mock/tests/phpunit/api/v4/AfformRoutingTest.php index 3fd74d34e1f8..d7406c754d9e 100644 --- a/ext/afform/mock/tests/phpunit/api/v4/AfformRoutingTest.php +++ b/ext/afform/mock/tests/phpunit/api/v4/AfformRoutingTest.php @@ -37,7 +37,7 @@ public function testChangingPermissions() { }; $result = $http->get($url('civicrm/mock-page')); - $this->assertNotAuthorized($result); + $this->assertNotAuthorized($result, 'mock-page'); Civi\Api4\Afform::update() ->setCheckPermissions(FALSE) @@ -62,7 +62,7 @@ public function testChangingPath() { ->execute(); $this->assertOpensPage($http->get($url('civicrm/mock-page')), 'mock-page'); - $this->assertNotAuthorized($http->get($url('civicrm/mock-page-renamed'))); + $this->assertNotAuthorized($http->get($url('civicrm/mock-page-renamed')), 'mock-page'); Civi\Api4\Afform::update() ->setCheckPermissions(FALSE) @@ -70,18 +70,19 @@ public function testChangingPath() { ->addValue('server_route', 'civicrm/mock-page-renamed') ->execute(); - $this->assertNotAuthorized($http->get($url('civicrm/mock-page'))); + $this->assertNotAuthorized($http->get($url('civicrm/mock-page')), 'mock-page'); $this->assertOpensPage($http->get($url('civicrm/mock-page-renamed')), 'mock-page'); } /** * @param $result + * @param string $directive */ - private function assertNotAuthorized(Psr\Http\Message\ResponseInterface $result) { + private function assertNotAuthorized(Psr\Http\Message\ResponseInterface $result, $directive) { $contents = $result->getBody()->getContents(); $this->assertEquals(403, $result->getStatusCode()); $this->assertRegExp(';You are not authorized to access;', $contents); - $this->assertNotRegExp(';afform":\{"open":".*"\};', $contents); + $this->assertNotRegExp(';' . preg_quote("<$directive>", ';') . ';', $contents); } /** @@ -93,7 +94,7 @@ private function assertOpensPage(Psr\Http\Message\ResponseInterface $result, $di $contents = $result->getBody()->getContents(); $this->assertEquals(200, $result->getStatusCode()); $this->assertNotRegExp(';You are not authorized to access;', $contents); - $this->assertRegExp(';afform":\{"open":"' . preg_quote($directive, ';') . '"\};', $contents); + $this->assertRegExp(';' . preg_quote("<$directive>", ';') . ';', $contents); } } From 0a7837ca9e0662b294488ee80db76bddf6f5ae4e Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Tue, 2 Mar 2021 08:52:43 -0500 Subject: [PATCH 2/2] Afform - use search params instead of route params --- ext/afform/core/ang/afCore.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ext/afform/core/ang/afCore.js b/ext/afform/core/ang/afCore.js index 12836dde8214..cef661948e73 100644 --- a/ext/afform/core/ang/afCore.js +++ b/ext/afform/core/ang/afCore.js @@ -3,7 +3,7 @@ angular.module('afCore', CRM.angRequires('afCore')); // Use `afCoreDirective(string name)` to generate an AngularJS directive. - angular.module('afCore').service('afCoreDirective', function($routeParams, crmApi4, crmStatus, crmUiAlert) { + angular.module('afCore').service('afCoreDirective', function($location, crmApi4, crmStatus, crmUiAlert) { return function(camelName, meta, d) { d.restrict = 'E'; d.scope = {}; @@ -11,12 +11,16 @@ d.link = { pre: function($scope, $el, $attr) { $scope.ts = CRM.ts(camelName); - $scope.routeParams = $routeParams; $scope.meta = meta; $scope.crmApi4 = crmApi4; $scope.crmStatus = crmStatus; $scope.crmUiAlert = crmUiAlert; $scope.crmUrl = CRM.url; + + // Afforms do not use routing, but some forms get input from search params + $scope.$watch(function() {return $location.search();}, function(params) { + $scope.routeParams = params; + }); } }; return d;