backdrop-contrib / search_api

Provides a generic API for modules offering search capabilities
GNU General Public License v2.0
0 stars 6 forks source link

Transliteration search index processor not supported #20

Closed ghost closed 2 years ago

ghost commented 3 years ago

Hello,

The transliteration index processor does not appear as an option at the admin screen here:

/admin/config/search/search_api/index/MY_INDEX_NAME/workflow

This appears to be caused by the expectation that transliteration is a module. I am able to get it working by:

  1. removing the module_exists() all at search_api.module line 1185 which checks to see if transliteration is installed (it's in core now)
  2. Adding include_once BACKDROP_ROOT . '/core/includes/transliteration.inc'; to processor_transliteration.inc and changing the last argument of the line that reads transliteration_get($value, '', language_default('language')); to transliteration_get($value, '', language_default('language')->langcode);

I'm not certain this is the best way to do this, but I am then able to select the transliteration option as filter for my index and rebuild the index successfully. The transliterated search then works as expected for an EN / ES translated node set.

earlyburg commented 3 years ago

Feel free to submit a pull request for this issue. If it works, I'll merge it into the branch after I test it. Thanks for letting me know.

ghost commented 3 years ago

Ok I think I added a pull request, not super familiar with github. Let me know if there is some specific tagging / naming convention or something I am supposed to use.

earlyburg commented 3 years ago

I reviewed the pull request and discovered something interesting. The functions differ between backdrop and d7. In D7 we can pass language_default() a var, but in backdrop we do not. In addition, your pull request calls language_default('language')->langcode) but langcode is not a method or property of this function in backdrop, although this would work in D7. Please have a look at the official documentation here: https://docs.backdropcms.org/api/backdrop/core%21includes%21bootstrap.inc/function/language_default/1 vs the drupal 7 function found here: https://api.drupal.org/api/drupal/includes%21bootstrap.inc/function/language_default/7.x here is an example of how to use the function in a backdrop context: https://docs.backdropcms.org/api/backdrop/core%21modules%21simpletest%21backdrop_web_test_case.php/function/BackdropWebTestCase%3A%3AsetUp/1 That being said - you are correct that /core/includes/transliteration.inc needs work because it uses the old function - I'm fixing that to hopefully improve function. In addition I have changed line 1185 in search.api..module from "if (module_exists('transliteration'))" to
"if (function_exists('transliteration_get'))" to be more consistant with instructions from the documentation found here: https://github.com/backdrop-contrib/transliteration. I have to scrap the pull request but I am pushing these changes in hopes of improving the function of this module. Please let me know how things work out and thanks for pointing this out, I appreciate it

earlyburg commented 3 years ago

Closing this - fixed by latest release: https://github.com/backdrop-contrib/search_api/releases/tag/1.x-1.0.04

ghost commented 3 years ago

@earlyburg this release did not fix the issue for me, were you suggesting I would need to update transliteration.inc to make this work? I ran into the same chain of errors that I did in the previous release, which inspired the "for now" fix I suggested in my first comment and the pull request. Perhaps I am missing something, but here are the chain of issues:

  1. function_exists(transliteration_get()) returns false, but all of the internationalization modules are on (see iamges)
  2. if I comment out the if statement, of course I then get function not defined
  3. If I include transliteration.inc in processor_transliteration.inc, I get a stdClass passed but string expected

Hence my three changes of commenting out the if statement, including transliteration.inc, and adding ->langcode to return "en" instead of stdClass.

Is there some other change - like to core transliteration - that needs to happen? I am on backdrop 1.18.1 Screen Shot 2021-03-01 at 8 04 38 AM Screen Shot 2021-03-01 at 8 04 53 AM Screen Shot 2021-03-01 at 8 05 40 AM Screen Shot 2021-03-01 at 8 05 59 AM Screen Shot 2021-03-01 at 8 08 52 AM Screen Shot 2021-03-01 at 8 09 31 AM

ghost commented 3 years ago

Some context for the images: One of them shows that the transliteration processor is not available before commenting out the if statement. The others are the chain of errors I get when attempting to build the search index.

earlyburg commented 3 years ago

You did not mention there was an ajax error I appreciate the more informative documentation. I am going to try and replicate with the info that you forwarded. More on this soon

earlyburg commented 3 years ago

I liked dpm(language_default()); The documentation is incomplete it seems :) I wquld also like to point out that none of the modules that you have listed are in backdrop core. All the ones in the picture are contrib (all 12 of them) . I spent like 2 months working with the people who maintain entity plus module just to stand this (search api ) set of modules up. Why? Because Drupal 7 was lovely but it was also a huge bloated inefficient mess. A lot of code was removed from core and in our case the code that we had to reconstitute was entity wrapper and the whole idea of portable entities. I would guestimate that a good 30 to 40% of D7 core is just gone. I would not be supprised if we will have to do with translation what we had to do with Entity just to get these modules to work. Slimming down core was done on purpose because over the years people got to add rediculous things to D7 core. Please give me a complete list of the modules you are using - a rough copy paste will do. Please and thank you.

earlyburg commented 3 years ago

@erictoupin The function transliteration_get() does not seem to be available, although it is currently in /core/includes/transliteration.inc I used a vanilla install of Backdrop - added devel module and enabled all the core translation modules - then tried to run:

// include_once('/var/www/rvg-web.net/backdrop/core/includes/transliteration.inc'); if( function_exists('transliteration_get') ) { backdrop_set_message('exists'); }else { backdrop_set_message('nope'); }

return was "exists" when the include_once() was uncommented so there is that. Going to kick this upstairs.

earlyburg commented 3 years ago

Submitted this bug report for core: https://github.com/backdrop/backdrop-issues/issues/4983

earlyburg commented 3 years ago

https://github.com/backdrop-contrib/search_api/releases/tag/1.x-1.0.05 Recent release includes transliteration.inc and associated functions for use with modules and sub-modules in search api

earlyburg commented 2 years ago

Closing this due to lack of activity.