Add recipe for search facets

breidert → credited a.dmitriiev → .

Waiting for https://www.drupal.org/project/drupal_cms/issues/3468271 📌 Add recipe for search backend Active to be merged

I have created MR https://git.drupalcode.org/issue/drupal_cms-3468271/-/merge_requests/1 in the same issue fork as https://www.drupal.org/project/drupal_cms/issues/3468271 📌 Add recipe for search backend Active

@a.dmitriiev since 📌 Add recipe for search backend Active is committed, could you please add the branch here and create an MR?

I have created new MR with 0.x as target branch.

Facets is in beta. My head hurts after discovering I need to do quite some work for upgrading a site from beta1 to beta4.

Do we expect a stable release for January? Is this something we discussed with their maintainers?

This recipe is adding a core patch, is that correct?

re Facets release: https://drupal.slack.com/archives/C3E9QDZ5M/p1733317648598519

I hate to rain on this parade but we cannot merge this with a core patch. I would be open to polyfilling the functionality in a glue code module, though. Either way this is not likely to land in time for our RC so we should probably consider this a 1.1 feature.

Adding the patched issue as related.

Marking postponed since we are blocked on the core issue.

I don't think this need be blocked anymore.

It won't land in 1.0.0. that's true, but given that we have 1.x as our new development branch, we can start working on this in that branch, including any core patches necessary (although really we should try to minimize that). It just won't be committed to 1.0.x.

The existing MR should be changed to target 1.x.

The MR was rebased on 1.x branch.

No real objections to the MR, just a couple of very minor things.

Not a big deal but why is BEF here? Does it have anything specifically to do with Facets?

Since facets 3.x, the rendering of facets is done by views filters (instead of writing our own rendering/ajax handling), BEF makes those filters more capable.

And also submodule facets_exposed_filters (that is needed for facets to be used in exposed filters) depends on BEF https://git.drupalcode.org/project/facets/-/blob/3.0.x/modules/facets_ex...

The comments from MR were addressed.

I like it but I'm not entirely convinced it needs to be its own recipe. Are facets enough of a common use case where >=80% of sites would want them? If so, we should probably just merge this directly into the existing Search recipe unless there's a compelling reason not to.

There is a problem with the input for the recipe to enable facets exposed filters based on it. There is no config action yet to target the specific setting of a filter in the view. I have updated this issue https://www.drupal.org/project/drupal/issues/3305859 → but still it sets the settings, but doesn't merge the existing ones with partially provided settings from the recipe. I am not sure about merging settings approach, should it be like this?

I have updated the MR, now facet filters are part of the main recipe and not a separate on anymore. This also allows to skip patching of the core, as no config actions are needed for views, but facet filters are also enabled by default without any confirmation.

Re #24:

Are facets enough of a common use case where >=80% of sites would want them?

Personally, I am not convinced about this. I see facets as a more complex feature for commerce and bigger sites, but maybe there is something which can be beneficial also for smaller sites?

Yeah, maybe -- but in that case, I think it would be sufficient to simply have the facets disabled (that is, not exposed) by default unless they are enabled by an input.

@a.dmitriiev: That looks good to me. Now, as much as I hate it, I suspect that this might be a case where we need to use the simpleConfigUpdate hack to change the facet's exposed setting using an input. Can you give that a try and see what happens?

Done. Now the recipe requires user input.

Minor phrasing suggestion but that looks good to me from a code perspective. Final sign-off belongs to @pameeela.

Oh, one more thing -- since this adds modules, can you rerun the following:

ddev list-modules > MODULES.md

...and commit the resulting changes to MODULES.md? (Be sure you're synced up with 1.x HEAD before you do this.)

Also, let's file a follow-up for functional test coverage!

I have updated MR with the new modules. After running the command there were more changes to MODULES.md file than I have expected, so I left the changes that were for Search recipe only (others were reverted).

Sending to Pam for final review.

Follow up issue for tests was also created https://www.drupal.org/project/drupal_cms/issues/3493492 📌 Add end to end test for search results page with filters by content type Active

Sorry for the lack of reply here, I discussed it briefly with @phenaproxima last week.

I'm super conscious that a lot of work has gone into this including adapting quickly to changing requirements so I'm really sorry about this late feedback. But initially when this was un-postponed in #17 it was assumed to be post-1.0 and that was my assumption too. And it only became possible for 1.0 when it was merged into the main search recipe, which unfortunately happened without product input. I am not totally sure about that decision, but regardless I don't want to merge this before the search styling is done ( 📌 Provide default styles for search Active ) because this will only add more work and we are extremely short on time. So I would rather aim to get this in v1.1 with some proper attention to design and styling.

Separately, I think there was also a miscommunication somewhere because the facets were supposed to be on tags, not content type. I know that tags was not really ready for this until very recently so I understand if content type was used as a stand-in, but we'll definitely want this on tags instead. I'm not sure if this is a significant change in the actual recipe?