I'm running into a problem in integrating Search API with Facet API (#1182614: Integrate with Facet API), having to do with the fact that Search API tries to create a facet for each field in an index, but some of these are already being provided as default facets. Because module_invoke_all uses array_merge_recursive, instead of overriding the default 'created' facet, for example, search_api's implementation causes the facet name to be an array.

It seems to me that Facet API is assuming that the fields it provides default facets for will be indexed, but it is up to the search backend to decide (or provide configuration for) what gets indexed. Should there really be default facets at all?

Anyway, for now I want to provide patch that at least prevents the array_merge_recursive problem when the same facet key is returned by two modules.

CommentFileSizeAuthor
#5 Screen shot 2011-06-29 at 9.22.58 PM.png40.37 KBcpliakas
#1 1202022-override-default-facets.patch3.03 KBkatbailey

Comments

katbailey’s picture

katbailey commented
Status: Active » Needs review
StatusFileSize
new 1202022-override-default-facets.patch3.03 KB

Here's the patch...

cpliakas’s picture

cpliakas commented

Kat,

Thanks for the patch. The original thinking was that Facet API would provide the field that could be faceted on but that is starting to look like bad idea. I think the goal was to have a couple of facet definitions that every module would conceivably have to implement, however it might not be the best idea now that Search API is being introduced into the mix. I think Peter would agree with your concerns about Facet API providing any default facets. I am starting to come around to this idea :-).

Thanks,
Chris

cpliakas’s picture

cpliakas commented
Status: Needs review » Reviewed & tested by the community

Patch looks good to me.

cpliakas’s picture

cpliakas commented
Status: Reviewed & tested by the community » Fixed

Committed at #d2ecc62.

cpliakas’s picture

cpliakas commented
StatusFileSize
new Screen shot 2011-06-29 at 9.22.58 PM.png40.37 KB

Woo-hoo! (see screenshot)

katbailey’s picture

katbailey commented

Yay - thanks! :-)
So it really is possible to make the patch author show up as the committer! I really need to figure that out for my own projects...

cpliakas’s picture

cpliakas commented

I couldn't git apply your patch for some reason, otherwise it would have worked automatically. However, when I ported your changes it was really easy to say "this is Kat's work".

git commit --author='Katherine Bailey <your-email-address-here>' -m "Issue #1202022 by katbailey: Don't use module_invoke_all in facetapi_get_facet_info()" ..

Status: Fixed » Closed (fixed)

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