🇬🇧United Kingdom @aaron.ferris

Not really sure what the intention is re: 1.x/2.x so raised an MR against both.

aaron.ferris → made their first commit to this issue’s fork.

aaron.ferris → made their first commit to this issue’s fork.

Thanks @nexusnovaz - there are some conflicts that need to be resolved here.

aaron.ferris → made their first commit to this issue’s fork.

Thanks for the MR and patch, ive pulled this change down and can see that the change fires Drupal.jstimer.timer_loop as expected, im not seeing anything untoward as far as regressions are concerned. Merged.

aaron.ferris → made their first commit to this issue’s fork.

aaron.ferris → changed the visibility of the branch 2.0.x to hidden.

aaron.ferris → changed the visibility of the branch 3511224-fix-cspell-issues to hidden.

aaron.ferris → made their first commit to this issue’s fork.

Thanks both, merged.

Isn't Country a required field, as in, the address itself can't be added without it?

Could you please provide reproduction steps in getting the country to be empty? Migrated values perhaps?

Not that this change isn't the right way, im just wondering if this being empty could have consequences elsewhere.

Added a permission/entity view hook for this.

Example jsonapi response with an anonymous user, with access to view

{"jsonapi":{"version":"1.0","meta":{"links":{"self":{"href":"http:\/\/jsonapi.org\/format\/1.0\/"}}}},"data":[{"type":"search_api_synonym--search_api_synonym","id":"abb09e71-552d-4ad0-bf75-804b0e969469","links":{"self":{"href":"https:\/\/drupal-11.ddev.site\/jsonapi\/search_api_synonym\/search_api_synonym\/abb09e71-552d-4ad0-bf75-804b0e969469"}},"attributes":{"drupal_internal__sid":4,"langcode":"en","search_api_synonym_type":"spelling_error","word":"marmelade","synonyms":"marmalade,mermelad,marmellade","status":true,"created":"2025-04-27T13:01:40+00:00","changed":"2025-04-27T13:01:40+00:00","field_asddsadasd":null},"relationships":{"uid":{"data":{"type":"user--user","id":"1f83400a-0892-4ed0-99b3-1379d9c831b3","meta":{"drupal_internal__target_id":1}},"links":{"related":{"href":"https:\/\/drupal-11.ddev.site\/jsonapi\/search_api_synonym\/search_api_synonym\/abb09e71-552d-4ad0-bf75-804b0e969469\/uid"},"self":{"href":"https:\/\/drupal-11.ddev.site\/jsonapi\/search_api_synonym\/search_api_synonym\/abb09e71-552d-4ad0-bf75-804b0e969469\/relationships\/uid"}}}}}],"links":{"self":{"href":"https:\/\/drupal-11.ddev.site\/jsonapi\/search_api_synonym\/search_api_synonym"}}}

Tested the change, looks fine. Ill leave this as RTBC for a couple of days to see if there are any arguments against merging.

Code change looks fine, I don't see any inherent reason not to expose fields, just wondering what the use case could be, custom export/import plugins?

Search API solr, at least as far as I can tell, has moved away from managed resources. Indeed, if you try and use ManagedSynonyms the config export will strip the field. This is still technically feasible, with a custom export plugin, but im not convinced we should be incorporating this into the module given the standardised approach of search_api_solr (using a file).

The phpcs pipeline is passing on 3.x, 2.x is considered legacy at this point.

8.x is unsupported. Please create a new ticket for 2.x/3.x.

Agreed this is odd.

MR raised

#
# Synonyms file for Apache Solr generated by Search API Synonym.
# See file https://www.drupal.org/project/search_api_synonym.
#

marmalade,mermelad,marmellade => marmelade
search => replace

Agreed with this, I don't think we need to change the synonym case but the spelling error does need a switch I believe.

Ive reviewed these changes, they're not applicable anymore as far as I can tell.

Made an initial start on this, we will need to compile the synonyms to add to the files post config creation.

Ive decided to take a slightly different approach to this, by adding another option in the interval select.

This should give the best of both worlds. We now have

1. Never - self explanatory
2. List of intervals - the way the module has historically worked
3. Bypass - which leaves the cron configuration open, but will ignore any time based interval, so that it can be used with Ultimate Cron and the likes

Feedback/testing welcome.

aaron.ferris → changed the visibility of the branch 3471724-remove-cron-frequency to hidden.

Updated main page, glad that helps.

aaron.ferris → created an issue.

You should only need to move to 3.x if you are on Drupal 11, the core functionality remains largely the same,

We've had to switch to a D10+ specific branch for Drupal 11 compatibility.

Thanks for this, makes sense, had to resolve some conflicts owing to the phpcs ticket being merged in. Ill merge when the pipeline finishes.

THanks, looks good to me, had to fix some conflicts caused by the phpcs ticket merge, will merge this in when the pipeline finishes.

( https://www.drupal.org/project/search_api_synonym/issues/3519773 📌 Fix pipeline issues - phpcs Active )

The changes seem OK, ive pushed a change to reintroduce the failing method, settings form works correctly, I can add and export synonyms as expected

#
# Synonyms file for Apache Solr generated by Search API Synonym.
# See file https://www.drupal.org/project/search_api_synonym.
#

another synonym, synonym
synonym, test

I think removing the unused 'use' is OK.

Testing this change, im seeing in an issue on the settings form

Fatal error: Class Drupal\search_api_synonym\Plugin\search_api_synonym\export\Solr contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (Drupal\Core\Plugin\PluginFormInterface::submitConfigurationForm) in /var/www/html/web/modules/contrib/search_api_synonym/src/Plugin/search_api_synonym/export/Solr.php on line 17

Thanks @julio_retkwa

This would indeed be nice, probably in a PostConfigFilesGenerationEvent event subscriber.

This will need work, although certain aspects of the patch will still apply others won't - please raise an MR. (Appreciate its old!)

Thanks for this, clean and makes sense. We can leave this in until we make this configurable, in which case we can update the readme,

Configurability of the folder: https://www.drupal.org/project/search_api_synonym/issues/2996538 ✨ Make file folder configurable Active

Raised MR, will merge in when the pipeline completes.

aaron.ferris → made their first commit to this issue’s fork.

Closing as duplicate given https://www.drupal.org/project/search_api_synonym/issues/3180873 →

Needs some thought on how we approach this, given this module is already out in the wild. Perhaps we need this behind some sort of toggle, so that the module will continue to behave as it currently is (cron settings from config) but can be overridden with a checkbox.