If another module defines unique targets using hook_feeds_node_processor_targets_alter(), they should be honored by FeedsNodeProcessor.

I'm not sure if this is a bug report or a feature request, but if we can set a target's uniqueness via the hook, it seems to me that we should also expect it to be honored.

After looking over the code I can see how this might be done for CCK fields, but I'm not sure beyond that - I'd be happy to whip up a patch if it would be useful, otherwise perhaps I ought to just implement my own processor?

CommentFileSizeAuthor
#2 feeds-hook-node-processor-unique-725822-2.patch2.35 KBawjrichards

Comments

awjrichards’s picture

awjrichards commented

On account of my ignorance, it didn't occur to me that this wouldn't really make sense to implement a blanket solution for CCK fields due to the different ways they may store data/mappers might work/etc.

So on second thought, perhaps it would make more sense to expose a hook for determining uniqueness of 3rd party specified targets. I'll work on a patch.

awjrichards’s picture

awjrichards commented
Status: Active » Needs review
StatusFileSize
new feeds-hook-node-processor-unique-725822-2.patch2.35 KB

Patch attached for exposing a hook in FeedNodeProcessor to allow 3rd parties to determine uniqueness of targets.

alex_b’s picture

alex_b commented

I agree that unique support would be only consistent.

I am not happy about the drupal_alter() though as the problem we're trying to solve is better served with a single callback rather than a wholesale alter call to all modules.

If we require implementers of hook_feeds_node_processor_targets_alter() to specify a unique callback we could invoke this callback in existingItemId().

function hook_feeds_node_processor_targets_alter(&$targets, $content_type) {
  $targets['my_node_field'] = array(
    'name' => t('My custom node field'),
    'description' => t('Description of what my custom node field does.'),
    'callback' => 'my_callback',
    'unique' => 'my_unique_callback',
  );
}

protected function existingItemId($source_item, FeedsSource $source) {
  // ...
  if (function_exists($target['unique'])) {
    $function = $target['unique'];
    $nid = $function($this->config['content_type'], $target, $value, $source->feed_nid);
  }
  // ...
}

The return value of uniqueTargets() needs to be modified slightly to obtain the value of 'unique' of course.

Having a 'callback' and a 'unique' key that both take a callback is a little funny, but we could come up with a better key name for 'callback' - for instance 'mapper'.

awjrichards’s picture

awjrichards commented

@alex_b I like the approach and it makes a lot of sense, I think it is more consistent with the Feeds style

alex_b’s picture

alex_b commented
Status: Needs review » Needs work

NW (see #3)

twistor’s picture

twistor commented
Issue summary: View changes
Status: Needs work » Closed (duplicate)

Work is being done here.
#661606: Support unique targets in mappers