CCK fields can be shared across content types but the cck mappings mechanism doesn't respect that. this patch adds an array element called content_types to the mappings array and merges the array over all instances of a field (last one will win on things like label, whereas the first one was winning before). Now we can accurately track which content types a cck facet applies to.

CommentFileSizeAuthor
#9 551278-content-types-9.patch2.46 KBpwolanin
#7 551278_7.patch2.24 KBrjbrown99
apachesolr_fix_cck_1.patch2.28 KBrobertdouglass

Comments

robertdouglass’s picture

robertdouglass commented
Status: Needs review » Fixed

committing to 6.2.

Status: Fixed » Closed (fixed)

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

robertdouglass’s picture

robertdouglass commented
Version: 6.x-2.x-dev » 6.x-1.x-dev
Status: Closed (fixed) » Patch (to be ported)
jpmckinney’s picture

jpmckinney commented
Assigned: Unassigned » jpmckinney
jpmckinney’s picture

jpmckinney commented

Related: #551620: Type dependent facet blocks for introducing "content types", #552152: OR facet counts are confusing for modifying some "content types" code, #642602: API Consistency - s/content types/content_types/ for renaming "content types" to "content_types"

5-2 already seems to have this patch due to #664994: Cache fields info (and small API improvement).

jpmckinney’s picture

jpmckinney commented
Assigned: jpmckinney » Unassigned

Should commit parts of #592722-3: PHP notices after this.

rjbrown99’s picture

rjbrown99 commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new 551278_7.patch2.24 KB

Enclosed is the original patch from #1, ported to the current DRUPAL-6--1 tree as of this morning. I actually need this functionality: #751004: Flag node for re-indexing if the 'exclude' setting is changed for one of its fields, but it appears that this issue is the prerequisite.

I will be testing this and reporting back, and I'll also roll a patch for the other issue.

pwolanin’s picture

pwolanin commented

generally looks ok, but I'm not really fond of casting an unset value to array.

better would be to add something like :

if (!isset($fields[$row->field_name]))) {
  $fields[$row->field_name] = array();
}
pwolanin’s picture

pwolanin commented
StatusFileSize
new 551278-content-types-9.patch2.46 KB

seems like this is a minor API change. But I see why it may really be a bug we have to fix.

Also, patch above does not match 6.x-2.x:

          $row->multiple          = (bool) $row->multiple;

          if (isset($fields[$row->field_name])) {
            $fields[$row->field_name] = array_merge((array) $fields[$row->field_name], (array) $row);
          }
          else {
            $fields[$row->field_name] = (array) $row;
          }
          $fields[$row->field_name]['content_types'][] = $row->content_type;
          unset($fields[$row->field_name]['content_type']);
        }

I think this patch is better.

pwolanin’s picture

pwolanin commented
Status: Needs review » Closed (fixed)

setting back to closed, let's fix 6.x-1.x at #751004: Flag node for re-indexing if the 'exclude' setting is changed for one of its fields