Backport Facet API to Drupal 6

Initial patch that makes most of the things at least accessible

Installing / uninstalling
integration with apachesolr shows already the realms and some of the exposed facets

Required so far for Drupal 6:
dbtng
autoload
ctools

Oh, I did not even know about the existence of ctools_static! Cool

With ctools_static as proposed

Forgot to save some files...

Small update. Can't seem to get the config facet links working. I suppose there's a change between Drupal 6 and 7 that I don't see

Hi Chris!

I think the patch already incorporates admin/settings but still I am having some small problems. Nothing that cannot be resolved.

Well, I agree that the DBTNG should disappear but to make the backport easier it was a quick step forward. I did the same for Apache Solr backport but removal of these DBTNG queries will go away :-)
The autoload can go away as well, if we manage to make everything stable +-

I could also use some help with CTools since I am still not very familiar with all that it offers!

Committed this patch as a initial start then :-)

Menu adjustments. All menu items should be accessible and at least show some configuration form now.
Still some errors and having problems with drupal_render_children. Should find some alternative for this

Committed this patch to the branch, so if you want to help. Feel free!

Facet blocks are working!!

Some unfortunate backport magic removed the drupal_html_class function because it does not exists in Drupal 6. I don't think we should support this problem for Drupal 6. There are still some notices but screenshot should make clear that at least the basics are working

Less notices and errors. Pages start to look better :-)

Patch 13 + some small changes added on

Small path update

More small path fixes + some protection for a mapping function. Not sure if this should be ported to 7.x (not committed yet to the branch)

The function gave me a SQL error. Which seems like an empty array to me. It happened when I added a CCK field, configured the facet. Added the block to the region and executed the search page. There was no value indexed for this facet and there was no content in the database for this facet. Somehow it broke. More about this later on

I haven't backported that yet. Next big thing is to convert all queries to normal queries starting from now. Suggestions on the #states are welcome?

Added patch to convert some of the D7 paths to D6

Converted most of the dbtng queries to regular queries. I'd like to hear your comment on autoload dependency. It does make class loading way easier and in sync with the D7 version.

I am not able to let simpletest run because some of the classes are not loaded properly. I expect this has something to do with ctools but I have yet to figure out why and how it fails in simpletest

Updated issue summary.

just an issue from dev today while clicking the facet link in the apachesolr (also dev 6x.3x from today):

Fatal error: Call to a member function getFacetSettings() on a non-object in /usr/share/drupal6/sites/all/modules/facetapi/facetapi.admin.inc on line 68

i will try to fix it

Perhaps a bit late, but couldn't perhaps this module be utilized to keep the dbtng-queries from the 7.x-version, making the version easier to keep in sync: http://drupal.org/project/dbtng ? Also, is there anything I can do to help? Perhaps have a look at #states? Need to use apache solr intregration 6.x-3.x and right now, without a d6 port of facetapi, it seems quite broken.

Ah, yes in that case it sounds like the best decision porting dbtng to the D6 DAL. I will clone the repo tomorrow and start looking at what I can to to contribute.

@xeconsulting: Installing http://drupal.org/project/autoload seems to resolve this problem (and probably all others related to d7-autoloadning). Perhaps that should be added as a dependency? Haven't looked at it much, but perhaps it's preferable to using ctools for this purpose?

Replaced call to "drupal_get_query_parameters" and fixed a minor comment inaccuracy(?)

EDIT: Hmm, saw this issue was addressed in 1387164-25.patch, sort of a bug in that code thougth, calling drupal_get_query_parameters within the added fetchQueryParams method. Currently no execution-path goes there, but should still be fixed. I though it a bit overkill to copy all of drupal_get_query_parameters since we don't need to filter out sub elements of query parameter arrays.

Thank you very much gnucifer
The FaceAPI works now and i can active the facets under my search enviroment.
Unfortunately i cannot configure display or export it (actions related to facet field) -> i get always an:

Class name must be a valid object or a string in /usr/share/drupal6/sites/all/modules/facetapi/plugins/facetapi/adapter.inc on line 137

I will try to analyze it but i'm not familar with the code right now..

We'd love to keep the amount of dependencies to a minimum. This means that if possible, autoload will not beome a dependency but we will implement some smart loading scheme if possible, Same reasoning is valid here as for the dbtng module :-)

Yeah, I'm using it for now though. Seems like a high quality module, and with 15204 and only a few minor issues it seems relatively safe when it comes to possible bugs. Would be great if the last patch (#25) could be committed to 6.x-3.x so I can get started working on the current issues. I guess I could merge them into my working copy, but I guess it's not ideal since there will probably be more possible merge later on conflicts if I do so.

Commited comment #25

@gnucifer, even though I like your patch I am a bit surprised to see that this is a D6 only problem? Can we track this in a separate thread?
I'm thinking about closing this one and opening separate issues per problem from this point on

Thanks!

And for the record, we'll keep autoload in the loop since it is only giving us advantages and the high amount of users seem to indicate it won't give us lots of troubles in the near future

Ok, thanks for the commit! I will start working on the tasks for the d6-port and post updates in this thread.

Adding the patches to get simpletest working also in this queue

Commited patch #39, 1 more fail left! Has something to do with ctools_static

Latest one fixes all simpletests - we can now continue to work resolving other issues

What's left?

• Reconcile FAPI differences, mainly use of the #states property
• Backport theme hook definitions and implementations
• Explore whether there is a Contextual Links alternative that would maintain the site builder experience with D7
• Remove or re-implement the CTools dropbutton widget

Committed #42

Small SQL issue from the dbtng backport

Dependency UI was not working because a non existing get_entity_info function

Committed 44 and 45

Sorry about my inactivity, work has kept me busy. Thanks @Nick_vh for the recent commits! Now when I finally have the time I will try to make myself useful.

I'm currently working on the ctools-class autoloading. Clicking on "Configure display" gives me the error "Class name must be a valid object or a string" in adapter.inc on line 137 because ctools_plugin_load_class fails. Have sorted most of it out, but will not have time to get a patch ready until tomorrow.

Can't wait to see progress on that! It's a hard nut to crack :-)

Yeah, it was a bit more hairy than expected. I'm testing against the apache solr integration module and seems like there are some issues with ctools-autoloading there as well. Got it working with autoload module enabled (the issue in my above post traced down to facetapi.facetapi.inc being included to late in hook_init), but tomorrow I will hopefully upload a patch mostly getting rid of the autoload dependency.

Now working without autoload. :) Had to patch apache solr search as well, as the ctools plugin declaration were incomplete. Will post that patch in the apache solr issue queue in a moment.

Oops, uploaded the wrong patch. :/ This is the correct one:

Is the first patch the patch for apachesolr? Care to explain why?

Patch for apachesolr: http://drupal.org/node/1485404

double post

@Nick_vh No I accidently uploaded the wrong patch in #51. Just ignore that file. The patch for facetapi is in #52 and apachesolr in #54.

@gnucifer, are you able to run the simpletests with this patch? That would be awesome!

How does the simple-testing on drupal.org work? I ran the test suite on my setup and all tests passed. I saw autoload module is loaded in the test setup though, so problems with ctools autoloading will probably not be caught.

Sorry about the spam, just not sure how drupal.org triggers the automated testing. This obviously didn't work.

What does this patch add? :-)
You can remove autoload from the simpletests to run all of them and hopefully they still work

Edit :
Automated testing won't work for patches until the module itself has been validated. So we should stick with local testing until then.

Without the patch I can't get facetapi to work with apachesolr-module. Even with autoload-module turned on I got the error reported in #32. What this patch does is fix that issue, plus adds more meta-data for ctools so it can figure out how to load the classes, and possible parent classes (the file and path-info on handlers). With all these changes in place, the autoload-module is no longer needed. I will have to recheck the tests without autoload enabled though.

EDIT: Short summary of changes.

- Added missing 'file', 'path' and 'parent' to all ctools plugin handlers.

- Added ctools class loading where missing in some place or two.

- Added ctools plugin declaration also for abstract plugin handlers (for autoloading of parents to work).

- Removed function facetapi_ctools_plugin_type because as I understand it they should be declared as facetapi_ctools_plugin_adapters,
facetapi_ctools_plugin_dependencies and so on. The 'type' is just a placeholder for plugin type, not in the name of the hook which is pretty confusing. I had to read the ctools source-code to get this (as for most ctools-functionallity).

- Don't know if because hook-execution order, but had to replace:

function facetapi_init() {
  module_load_include('inc', 'facetapi', 'facetapi.facetapi');
}

with

require_once dirname(__FILE__) . '/facetapi.facetapi.inc';

or the hook declarations are not picked up by ctools. Pretty weird actually.

Still seems that most of the tests fail without autoload :

22 passes, 16 fails, and 1 exception

Yes, and mostly it is because the plugin-declartions in facteapi_test.module have the same problems as the ones fixed in facetapi.facetapi.inc. Adding path, file and parent for them resolves all issues for me besides loading of the query_type plugin. Having a look at that now.

Yeah!

28 passes, 0 fails, and 3 exceptions
Thanks gnucifer for building the first brick. Working on those exceptions now

Re-uploading the patch. Working with some new files

222 passes, 0 fails, and 0 exceptions
I would like to get rid of the include_once though. Will try to work on that now

This moves the code to hook_boot, little bit better. Also removed debug calls

Going to commit this patch. Please open up a new one if you find other issues.

Ah but of course! The error I was getting was struggling with in the test case was caused by the test adapter and query-type residing in the same file, so when the first was loaded PHP complaines about a missing parent class for the other that failes to be included. Splitting them up in two files as you have done in the latest patch resolves this. :)

Yes, I will continue working on testing the module together with apachesolr.

Committed to D6 branch

Personally I think its better to include the code with require_once, hook_boot is problematic because I believe it registers the module as having to be loaded by drupal also on cached requests, and as incompatible with aggressive caching.

gnucifer, you have some documentation to proof that? If so, make a patch and we'll revert that

It's not very well documented, but in http://api.drupal.org/api/drupal/includes%21module.inc/function/module_r... this code sets the boostrap-flag for modules implementing hook_boot (before inserting into system table):

$bootstrap = 0;
foreach (bootstrap_hooks() as $hook) {
  if (module_hook($file->name, $hook)) {
    $bootstrap = 1;
     break;
   }
}

Patch reverting to include files the old way:

Committed! :)

Some more fixes in facetapi.callbacks.inc:

oops, wrong thread :) ignore this comment

I'd admit I haven't read all of this thread, but what of the 6.x-3.x-dev version does work? I've got Apache Solr installed and showing its basic facets, but I don't see any settings pages that expose the Facet API blocks. Does more support have to be put into Apache Solr 6.x module or something? Thanks for all the great help and useful code.

Ohh, I'm figuring this out. Latest update: I should be using the 6.x-3.x version of the Apache Solr for the Facet API to work. That makes sense. I'll have to see if moving to Solr 6.x-3.x from 6.x-1.x is trouble or not.

New patch fixing settings for empty behaviour text:

@dandaman: Yes it might work with latest dev 3.x-branch of solr and facetapi, it does for me. For taxonomy-facets to work some uncommited patches are needed though.

Gnucifer, could you move that to another thread and close this one? I'd like to keep track of different issues in different pages

Ah, thought we where doing all the backporting in this thread, but I should post the patches in new issues instead?

Originally yes, but it is becoming a bit unclear for me what patches are a follow-up for what. Hope you don't mind creating new ones. I'll keep this thread for todo's :-)

Yes, np. Will create new issues as soon as I'm done with some other stuff.

Committed to 6.x-3.x dev.

Thanks for your contributions! For new stuff I'd like you to open a new issue.