fix wrong container parameter determination, see #150 (#157)

dachcom-digital · Jun 4, 2021 · 53219e1 · 53219e1
1 parent 5ac9242
commit 53219e1
Show file tree

Hide file tree

Showing 7 changed files with 43 additions and 21 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -11,6 +11,7 @@ Just click the "update" button or execute the migration command to finish the bu
 - **[IMPROVEMENT]**: use no-cookie domain for youtube videos [@ghettopro](https://github.com/dachcom-digital/pimcore-toolbox/pull/153)
 - **[BUG FIX]**: Fix invalid asset video markup [@gpalmisano](https://github.com/dachcom-digital/pimcore-toolbox/pull/154)
 - **[BUG FIX]**: Fix google maps locations with single quotes [#151](https://github.com/dachcom-digital/pimcore-toolbox/issues/151)
+- **[BUG FIX]**: Fix wrong container parameter concatenation [#150](https://github.com/dachcom-digital/pimcore-toolbox/issues/150)
 
 #### Update from Version 3.2.4 to Version 3.2.5
 - **[BUG FIX]**: Fix column adjuster column_store availability check

diff --git a/docs/11_ElementsOverview.md b/docs/11_ElementsOverview.md
@@ -209,16 +209,16 @@ parameters:
 ```
 
 #### III. Direct Element Configuration
-If you don't want do define them via parameters, you need to override the default parameters
+If you don't want to define them via parameters, you need to override the default parameters
 on configuration layer:
 
 ```yaml
 toolbox:
     areas:
         googleMap:
             config_parameter:
-                map_api_key: '%toolbox.google_maps.browser_api_key%' # replace here
-                simple_api_key: '%toolbox.google_maps.simple_api_key%' # replace here
+                map_api_key: null # replace here
+                simple_api_key: null # replace here
 ```
 
 ### Script Integration

diff --git a/src/ToolboxBundle/DependencyInjection/ToolboxExtension.php b/src/ToolboxBundle/DependencyInjection/ToolboxExtension.php
@@ -27,9 +27,6 @@ class ToolboxExtension extends Extension implements PrependExtensionInterface
      */
     public function prepend(ContainerBuilder $container)
     {
-        $container->setParameter('toolbox.google_maps.browser_api_key', null);
-        $container->setParameter('toolbox.google_maps.simple_api_key', null);
-
         $selfConfigs = $container->getExtensionConfig($this->getAlias());
 
         $rootConfigs = [];

diff --git a/src/ToolboxBundle/Model/Document/Tag/GoogleMap.php b/src/ToolboxBundle/Model/Document/Tag/GoogleMap.php
@@ -220,28 +220,39 @@ public function getId()
      */
     protected function geocodeLocation($location)
     {
+        if ($this->googleLookUpIsDisabled()) {
+            return $location;
+        }
+
         /** @var ConfigManager $configManager */
         $configManager = \Pimcore::getContainer()->get(ConfigManager::class);
         $configNode = $configManager->setAreaNameSpace(ConfigManagerInterface::AREABRICK_NAMESPACE_INTERNAL)->getAreaParameterConfig('googleMap');
 
         $address = $location['street'] . '+' . $location['zip'] . '+' . $location['city'] . '+' . $location['country'];
         $address = urlencode($address);
 
-        $key = '';
+        $key = null;
+        $keyParam = '';
+
+        $fallbackSimpleKey = \Pimcore::getContainer()->getParameter('toolbox.google_maps.simple_api_key');
+        $fallbackBrowserKey = \Pimcore::getContainer()->getParameter('toolbox.google_maps.browser_api_key');
+
         // first try to get server-api-key
         if (!empty($configNode) && isset($configNode['simple_api_key']) && !empty($configNode['simple_api_key'])) {
-            $key = '&key=' . $configNode['simple_api_key'];
-        }
-        // if not set, get browser-api-key
-        if ($key === '' && !empty($configNode) && isset($configNode['map_api_key']) && !empty($configNode['map_api_key'])) {
-            $key = '&key=' . $configNode['map_api_key'];
+            $key = $configNode['simple_api_key'];
+        } elseif (!empty($fallbackSimpleKey)) {
+            $key = $fallbackSimpleKey;
+        } elseif (!empty($configNode) && isset($configNode['map_api_key']) && !empty($configNode['map_api_key'])) {
+            $key = $configNode['map_api_key'];
+        } elseif (!empty($fallbackBrowserKey)) {
+            $key = $fallbackBrowserKey;
         }
 
-        if ($this->googleLookUpIsDisabled()) {
-            return $location;
+        if ($key !== null) {
+            $keyParam = sprintf('&key=%s', $key);
         }
 
-        $url = 'https://maps.google.com/maps/api/geocode/json?address=' . $address . $key;
+        $url = sprintf('https://maps.google.com/maps/api/geocode/json?address=%s%s', $address, $keyParam);
 
         $c = curl_init();
         curl_setopt($c, CURLOPT_RETURNTRANSFER, 1);

diff --git a/src/ToolboxBundle/Resources/config/pimcore/areas/googleMap.yml b/src/ToolboxBundle/Resources/config/pimcore/areas/googleMap.yml
@@ -50,5 +50,5 @@ toolbox:
                     scrollwheel: false
                 map_style_url: false
                 marker_icon: false
-                map_api_key: '%toolbox.google_maps.browser_api_key%'
-                simple_api_key: '%toolbox.google_maps.simple_api_key%'
+                map_api_key: null
+                simple_api_key: null
diff --git a/src/ToolboxBundle/Resources/config/services/twig.yml b/src/ToolboxBundle/Resources/config/services/twig.yml
@@ -45,6 +45,8 @@ services:
             - { name: twig.extension }
 
     ToolboxBundle\Twig\Extension\GoogleAPIKeysExtension:
+        arguments:
+            $fallbackBrowserKey: '%toolbox.google_maps.browser_api_key%'
         public: false
         autowire: true
         tags:

diff --git a/src/ToolboxBundle/Twig/Extension/GoogleAPIKeysExtension.php b/src/ToolboxBundle/Twig/Extension/GoogleAPIKeysExtension.php
@@ -8,16 +8,25 @@
 
 class GoogleAPIKeysExtension extends AbstractExtension
 {
+    /**
+     * @var string
+     */
+    protected $fallbackBrowserKey;
+
     /**
      * @var ConfigManagerInterface
      */
     protected $configManager;
 
     /**
+     * @param string|null            $fallbackBrowserKey
      * @param ConfigManagerInterface $configManager
      */
-    public function __construct(ConfigManagerInterface $configManager)
-    {
+    public function __construct(
+        ?string $fallbackBrowserKey,
+        ConfigManagerInterface $configManager
+    ) {
+        $this->fallbackBrowserKey = $fallbackBrowserKey;
         $this->configManager = $configManager;
     }
 
@@ -38,11 +47,13 @@ public function getFunctions()
      */
     public function getGoogleMapAPIKey()
     {
+        $browserKey = 'please_configure_key_in_systemsettings';
         $configNode = $this->configManager->setAreaNameSpace(ConfigManagerInterface::AREABRICK_NAMESPACE_INTERNAL)->getAreaParameterConfig('googleMap');
 
-        $browserKey = 'please_configure_key_in_systemsettings';
         if (!empty($configNode) && isset($configNode['map_api_key']) && !empty($configNode['map_api_key'])) {
-            $browserKey = $configNode['map_api_key'];
+            return $configNode['map_api_key'];
+        } elseif (!empty($this->fallbackBrowserKey)) {
+            return $this->fallbackBrowserKey;
         }
 
         return $browserKey;