Add recipe for search backend

a.dmitriiev → made their first commit to this issue’s fork.

It looks like there's an MR to review here.

This looks like a great start. My main concerns are:

• This absolutely needs test coverage; a ComponentValidationTest at the minimum. Ideally we'd get some end-to-end (Cypress) testing as well.
• This feels like too many recipes -- we don't really want to offer a ton of small parts that only do one thing, that's a UNIX philosophy that is friendly to developers, but likely confusing to site builders and end users. We should ship, maybe, two recipes at most. I thought the plan was to offer Basic Search and Advanced Search - what happened with that?
• There are a few questionable practices in use here, like creating entire config entities using actions. This is no longer appropriate now that we have support for lenient comparison with existing config.
• I'd like certain user-facing wording to be a little more robust and descriptive, but I'm happy to help with that.

I tried to address all the comments from the MR. But now as Drupal CMS requires Drupal 11, there are some more problems with recipe dependencies, as Facets are not yet Drupal 11 compatible, though there is an issue https://www.drupal.org/project/facets/issues/3455217 📌 Automated Drupal 11 compatibility fixes for facets Active already with MR (I will try to push this forward as much as I can). The same is with Simple Search Form module. I have already created a co-maintainer request issue https://www.drupal.org/project/simple_search_form/issues/3486501 💬 Co-maintainer request Active and that module also already have Drupal 11 compatibility issue https://www.drupal.org/project/simple_search_form/issues/3434565 📌 Automated Drupal 11 compatibility fixes for simple_search_form Needs review .

Indeed there are also 3 patches to Search API module included in the recipe as well. I worked on one of them https://www.drupal.org/project/search_api/issues/3483366 ✨ Need a way to populate some config values with list of available entity bundles with config actions Active to allow setting global settings for index and search results view. This is needed as the list of content types is not known for the recipe in advance. I am waiting for the feedback of the maintainer of search_api module. The other 2 patches are not critical but also would be nice to have.

There are also 2 Drupal core patches. Both are with needs review status: https://www.drupal.org/project/drupal/issues/3483353 ✨ Consider making the createCopy config action optionally fail if the entity display already exists Active and https://www.drupal.org/project/drupal/issues/3305859 📌 Add config actions for views Active (this one has Needs work because of some automated checks, I will check them now).

I have also added the tests, but at the moment pipeline fails because of Drupal 11 compatibiliy.

@a.dmitriiev, thanks for the reply.

D11 support is non-negotiable from a product/release perspective, so I think the correct answer here is probably rescoping. The current MR is already quite a bit larger than the declared scope of this issue anyway.

Are Facets and Simple Search Form needed for creating a search backend? If they are, then I guess this issue is blocked. If they're not, I would suggest removing them, and the recipes that use them, for now. We can add those features in other issues which can have smaller numbers of blockers. I think it would be better to ship a strong, supported search backend and kludgey UI, then add Facets later when it supports D11, than ship a more complete package much later. Incremental, rapid improvement is the name of the game with Drupal CMS.

So let's dial back the scope here, and in this issue only add a recipe that provides a search backend (even if there's no UI beyond that), and the most basic possible UI for it. What do you think?

Also, can you please add the blockers as related issues?

Adding blockers, other issues with patches are optional and probably would need a clean-up in recipe.

Facets have a lot of open issues and personally I am not sure if we should ever include it unless these issues are fixed:

🌱 [META] Overview of Facets + Ajax issues Active
🐛 Facets with AJAX not working in most of situations Needs work

It is problematic to run Facets even on Drupal 10.3 if you want to enable ajax. Yes, we can probably ship it without ajax enabled, but if someone start to explore these options, it will be very easy to break everything.

So +1 from me to removing Facets for now.

@poker10 the issues mentioned by you are for Facets version 2.x. The recipe includes Facets 3.x that was completely rewritten to "natively" support views exposed filters, there is even a special submodule facets_exposed_form now included in the codebase for that. As it is now just a views filter and not a block as it was before, there are no problems with ajax anymore.

Thank you for the explanation @a.dmitriiev. I haven't noticed the new beta release of the 3.x branch from the last week). When we tried the 3.x branch few months ago, in that time it was not possible to use it as a replacement for 2.x branch, as it still had a lot of issues. We will take a look at the new changes :)

For me, facets and autocomplete are both nice to have so would rather get a recipe in without those while we work on what's needed to get them added later.

Some update:

Module https://www.drupal.org/project/simple_search_form → now has Drupal 11 support. I have a contact with the module maintainer and I am also co-maintainer now, just in case.

As for Facets, I am also in contact with Jimmy Henderickx (strykaizer), who is in charge of version 3 and the Drupal 11 support is now the highest prio, so I believe this will be added very soon.

The blocking issue for search_api https://www.drupal.org/project/search_api/issues/3483366 ✨ Need a way to populate some config values with list of available entity bundles with config actions Active now only needs tests and can be completed soon, here I am also in contact with module maintainer.

So I reviewed this and I actually think it looks pretty good! Here are a few major suggestions:

• Your patches are not being applied because you need to add drupal/drupal_cms_search to project_template/composer.json as a dependency. Do that, and then run ddev rebuild, and you should be fully patched.
• We are most likely going to remove the drupal_cms_search_filters recipe due to Facets not being D11 compatible yet. It's not in scope for this issue anyway, so it should be removed and put into another issue/MR. Can you do that?
• "drupal_cms" is not a descriptive or helpful machine name for the search server or index.
• The test coverage is admirable, but it also is not really testing anything valuable beyond the recipes' idempotency. What would be best, IMHO, is to add a single end-to-end Cypress test to drupal_cms_search which does something like:
• log in as a content editor and add some nodes: one included in the index and published, one indexed but not published, one published and excluded, one unpublished and excluded
• run cron
• log out
• confirm that the published, non-excluded nodes is found when you search for them

To me, that would prove that the recipe is actually delivering what is intended. We could add this test later in a follow-up so as not to block commit; the idempotency tests are enough to get this merged.

• @breidert explained to me that drupal_cms_search_base is split out from drupal_cms_search in order to facilitate decoupled searching later. I think that's something we can consider for the future, but we should not do it now; we should just have a single drupal_cms_search recipe with all of this stuff. We can split it out later as demand dictates.

I have addressed all the comments in MR and also now there is only 1 recipe for search that combines all the functionality. Facet filters will be added in a separate issue.

From a code review perspective, I think this looks excellent. Tests are failing, though - chances are there is an idempotency bug. Kicking back for that.

The tests were failing due to a core bug - I've filed 🐛 The PlaceBlock config action doesn't account for the possibility of there being no other blocks in the region Active to deal with that separately. In the meantime, it's unlikely to affect real-world sites, and can be worked around in the test. So that's what I did.

This is ready for final review by @pameeela!

The search is missing styling. This will be handled in the subtheme of Olivero 📌 [Meta] Plan for Olivero subtheme Active and organised in separate issues.

So I think the core bug does affect us, but I would be OK to merge this as WIP since it's not applied by the starter and isn't exposed in the UI. And will unblock other things.

But here's what happened when I tested it:

• Since the recipe is not applied by the starter, I applied it via cli.
• Testing the actual search, I landed on a 404 page at /search. I could see this was an existing views page, so I cleared the cache, and then the page worked as expected.
• Figured I would make sure the same didn't happen when it is applied by the starter recipe, but the installer failed on (I am assuming this is the core bug!):
  

  An AJAX HTTP error occurred.
  HTTP Result Code: 200
  Debugging information follows.
  Path: /core/install.php?profile=drupal_cms_installer&langcode=en&recipes%5B0%5D=drupal_cms_starter&site_name=My%20awesome%20site&id=1&op=do_nojs&op=do
  StatusText: parsererror
  ResponseText: Error: Call to a member function getWeight() on false in Drupal\block\Plugin\ConfigAction\PlaceBlock->apply() (line 82 of /var/www/html/web/core/modules/block/src/Plugin/ConfigAction/PlaceBlock.php).

I'll leave it with Adam to decide how to proceed.

That is, in fact, a core bug with an open issue. The fact that it is a bug in an experimental subsystem means we may be able to land it during core's RC, but I'll get a patch up tomorrow and mark it as a stable blocker for Drupal CMS.

That also explains why tests don't break - we test the installer every way it can be run (CLI -- both core and Drush -- and interactive), but so long as that recipe is not applied by the starter, this problem will be skipped.

I, too, am therefore okay committing it as a WIP. Can't promise it will land in the stable release of Drupal CMS (both because of the bugs here and the fact that it relies on Search API rolling a new release in time for our RC), but at least we can aim for the stars.

Back to you for the RTBC, @pameeela.

Boom! Merged into 0.x. Congrats, everyone! As a next step, let's move the facets recipe and the end-to-end test to their own respective issues. See you there!

Automatically closed - issue fixed for 2 weeks with no activity.