Should not create blocks for every CCK field

Agree. Checkboxes on the admin page. Tagged for inclusion in 1.0.

Hoping that I understand the idea here ...

1) Apachesolr settings has new variable "CCK fields to display as facets". It presents multiple select checkboxes, one for each cck field as returned from apachesolr_cck_fields().

2) apachesolr_block checks against this and only creates a block if the checkbox was checked.

This looks good. Here are the nits =)

+      '#description' => t('Select the CCK fields that you want to use as facets.  These will appear as blocks in admin/build/block.'),
+      );

you can make a link to admin/build/block and say something like

t("These will be available as blocks on the !block_administration_page.", array('!block_administration_page', l(t('block administration page'), 'admin/build/block')));

Also, the second line has too much indentation.

Please solicit another review from someone else.

heh heh .. this is very cool guys. thanks. rampin' up, as they say. will not repeat same mistake twice. will not repeat same mistake twice. file new info. will not repeat same mistake twice.

Also been thinking about default values. Should new cck fields default to "checked" ? Should we have an new option for setting "new fields checked"? Am I beating a dead horse while making even more convoluted inline docs?

(as I recently discovered with cck_nodereference nothing shows up in the selector if you don't check off the node types in settings at the bottom of the page)

I'll post a patch after hearing your thoughts on default.

+1 for having all new fields default to checked (preserves current behavior). -1 to adding an option to modify that behavior.

I second that motion - bidding closes at end of day. Right now I've got to drive down to San Leandro to fight a bogus $250 ticket for stopping in a bus stop at the Bart station.. :-[

hehe! Kieran Lal told me about guys he know in the LA area who set their laptops up with voice recognition, and they responded to emails while driving.

A little scary when in incapable "hands". better than an in-depth, live, phone conversation. I do love voice recognition.

byw: I had to look the address up on the web, and when I got there in the nick of time, all I saw was a monster juvenial detention center with massive barbed-wire scary fences that keep kids inside and a huge empty parking lot with a small section that had cars all fenced in to keep kids out. So I missed my hearing, and hit rush hour traffic on the way home... bummer all the way around.

meanwhile... back to Funland

Patch includes:

• groovy link in description
• proper default value data type (array)
• new cck fields now default to "checked"

Style question - should:

      $label = db_result(db_query("SELECT label FROM {node_field_instance} WHERE field_name = '%s'", $name));
      $options[$name] = $label;

be written:

      $options[$name] = db_result(db_query("SELECT label FROM {node_field_instance} WHERE field_name = '%s'", $name));

since the variable is not used again?

Patch is failing for current checkout of DRUPAL-5 branch.

% patch -b <apachesolr_281810_limit_cck_facets.patch 
patching file apachesolr.module
Hunk #1 FAILED at 62.
Hunk #2 succeeded at 483 (offset 42 lines).
1 out of 2 hunks FAILED -- saving rejects to file apachesolr.module.rej

Let's try that again... new patch created against current dev:

// $Id: apachesolr.module,v 1.1.2.38 2008/09/24 18:07:54 robertDouglass Exp $

thanks

subscribing (and will test...).

@Jacob - yes, I think we should postpone/incorporate this iossue into the larger issue of centralizing facet block handling. With that done, at least the performance aspects of this will no longer be a concern. We will still have to figure out the best UI.

Has this been handled in #339467: centralize/register node facets blocks ?