Account created on 14 July 2007, over 17 years ago
#

Merge Requests

Recent comments

πŸ‡¬πŸ‡§United Kingdom progga

progga β†’ made their first commit to this issue’s fork.

πŸ‡¬πŸ‡§United Kingdom progga

progga β†’ created an issue.

πŸ‡¬πŸ‡§United Kingdom progga

I am about to test this. I can see two branches listed above. What's the second branch for?

πŸ‡¬πŸ‡§United Kingdom progga

Great to see this merged :)

πŸ“Œ | Geocoder | Add gitlab CI
πŸ‡¬πŸ‡§United Kingdom progga

Thanks a lot. That was real quick :)

πŸ“Œ | Geocoder | Add gitlab CI
πŸ‡¬πŸ‡§United Kingdom progga

Hello,
I have raised a new pull request for this task. It is getting a green although PHPStan checks have returned some errors.

Change summary:
- Drupal Association Gitlab CI template.
- Coding standard fixes for PHP, Javascript, and CSS code.
- I have added some development dependencies to composer.json to make PHPStan happier. Without this change, PHPStan flags up even more errors.

I have avoided making any code changes suggested by PHPStan to avert potential side effects. Perhaps these remaining issues can be dealt with in a separate ticket. Remaining issues include:
- "Unsafe usage of new static()". These could be easily resolved by declaring relevant class constructors as "final". I can't say if such a change will have any side effect.
- "\Drupal calls should be avoided in classes, use dependency injection instead".
- Use of two unknown classes: `Geometry`, and `GeometryCollection`.

πŸ“Œ | Geocoder | Add gitlab CI
πŸ‡¬πŸ‡§United Kingdom progga

progga β†’ made their first commit to this issue’s fork.

πŸ‡¬πŸ‡§United Kingdom progga

> patches don't make much sense in the thread (but only confusion) isn't it?

Yeah, I should have been smarter :(

πŸ‡¬πŸ‡§United Kingdom progga

Thanks for merging. And sorry for causing the confusion. I should have been clearer. The patch was for Eke's fork. So it was more like an interdiff. Please accept Eke's pull request for full functionality.

πŸ‡¬πŸ‡§United Kingdom progga

Hello,
I am proposing an alternate fix where anybody who can edit a Webform would see its "Revisions" tab.

Change summary:
- Permission fix for the "Revisions" tab. Associated functional test method.
- Had to add a Gitlab CI file as DrupalCI isn't running for some reason. Gitlab CI test is passing thankfully.
- Gitlab CI seems to require a composer.json file to grab dependencies. So had to add one.
- Some small fixes to cope with coding standard and static analysis checks.

Please note that, with this change, just because someone has access to the revision history doesn't mean they could revert them. Please let me know if this approach is acceptable. Thanks.

πŸ‡¬πŸ‡§United Kingdom progga

progga β†’ made their first commit to this issue’s fork.

πŸ‡¬πŸ‡§United Kingdom progga

Thanks. Tested with a few Geocoders and worked as expected. No errors or warnings in the Drupal log. RTBC.

πŸ‡¬πŸ‡§United Kingdom progga

Corrected patch for the fork attached.

πŸ‡¬πŸ‡§United Kingdom progga

Thanks Ekes. I have tested the code and it works as expected. ProviderUsingHandlerBase is missing a `use` statement for the `ProviderGeocoderPhpInterface` interface. Otherwise all good.

I was thinking it would be good if the `geocoder` service could accept GeocodeQuery. The attached patch for the fork achieves that.

πŸ‡¬πŸ‡§United Kingdom progga

Hi Andy,
As far as I know, Gin already has the functionality to collapse/uncollapse its sidebar. So perhaps the elbow_room module is not necessary for Gin users?

πŸ‡¬πŸ‡§United Kingdom progga

Change summary:
- Drupal 10 compatibility declaration.
- Replaced `jquery/once` asset library usage with `core/once`.
- Javascript coding standards fixes.
- The Claro theme bundled with Drupal 10 uses CSS grid for layout. The CSS style rules of this module has been adjusted accordingly.

πŸ‡¬πŸ‡§United Kingdom progga

progga β†’ created an issue.

πŸ‡¬πŸ‡§United Kingdom progga

Please find a patch attached to resolve this.

πŸ‡¬πŸ‡§United Kingdom progga

I have thoroughly tested #44 and it works okay. Even stumbled upon a new issue β†’ with search_ap_location in the process but that's unrelated. So I am okay with #44.

πŸ‡¬πŸ‡§United Kingdom progga

Additionally, this new `require` option introduced in #3070519 β†’ is missing a config schema definition. I am not sure if that should be bundled with this fix. Something like this resolves both issues:

diff --git a/modules/search_api_location_views/config/schema/search_api_location_views.schema.yml b/modules/search_api_location_views/config/schema/search_api_location_views.schema.yml
index e110ba7..934ec85 100644
--- a/modules/search_api_location_views/config/schema/search_api_location_views.schema.yml
+++ b/modules/search_api_location_views/config/schema/search_api_location_views.schema.yml
@@ -98,6 +98,9 @@ views.filter.search_api_location:
             to:
               type: string
               label: 'Distance to'
+    require:
+      type: boolean
+      label: 'Does it require successfull location resolving?'
 
 
 views.filter_value.search_api_location:
diff --git a/modules/search_api_location_views/src/Plugin/views/filter/SearchApiFilterLocation.php b/modules/search_api_location_views/src/Plugin/views/filter/SearchApiFilterLocation.php
index f1a7491..ff24b6f 100644
--- a/modules/search_api_location_views/src/Plugin/views/filter/SearchApiFilterLocation.php
+++ b/modules/search_api_location_views/src/Plugin/views/filter/SearchApiFilterLocation.php
@@ -85,7 +85,7 @@ class SearchApiFilterLocation extends FilterPluginBase {
       ],
     ];
 
-    $options['require'] = FALSE;
+    $options['require'] = ['default' => FALSE];
 
     return $options;
   }
πŸ‡¬πŸ‡§United Kingdom progga

I can confirm that the patch from #3 works for me. Thanks a lot for the fix :)

Production build 0.71.5 2024