Add hook hook_feeds_config_defaults() to allow other modules to add extra configuration

This is a nice idea. I have tried your approach before, creating a module that works with all plugins, it's not easy.

+++ b/includes/FeedsConfigurable.incundefined
@@ -166,7 +166,10 @@ abstract class FeedsConfigurable {
-    return array();
+    $hook = 'feeds_' . get_class($this) . '_defaults';

This should use the plugin id, not the class. I know they are usually the same, but not always.

+++ b/feeds.moduleundefined
@@ -928,10 +928,12 @@ function feeds_source($importer_id, $feed_nid = 0) {
-function feeds_plugin($plugin, $id) {
+function feeds_plugin($plugin_key, $id) {
   ctools_include('plugins');
-  if ($class = ctools_plugin_load_class('feeds', 'plugins', $plugin, 'handler')) {
-    return FeedsConfigurable::instance($class, $id);
+  if ($class = ctools_plugin_load_class('feeds', 'plugins', $plugin_key, 'handler')) {
+    $plugin = FeedsConfigurable::instance($class, $id);
+    $plugin->plugin_key = $plugin_key;
+    return $plugin;

Errr... No.

+++ b/includes/FeedsConfigurable.incundefined
@@ -166,7 +166,14 @@ abstract class FeedsConfigurable {
-    return array();
+    $defaults = array();
+    if (isset($this->plugin_key)) {
+      $hook = 'feeds_' . $this->plugin_key . '_defaults';
+      $defaults = module_invoke_all($hook, $this);
+    }
+    $defaults = array_merge($defaults, module_invoke_all('feeds_defaults', $this));
+
+    return $defaults;

This needs to be moved to FeedsPlugin. FeedsConfigurables are not necessarily plugins.

So, you can get the plugin_key in a rather convoluted way without touching feeds_plugin(). Either way, that's the wrong place to do it. I will open up a followup issue about adding a pluginKey() method, which I've been meaning to do already. But I want to do a bit more.

Until then, from FeedsPlugin::configDefaults():
feeds_importer($this->id)->config[$this->pluginType()]['plugin_key']

I don't understand:

this only happens in the feed processing, and not in the admin screens. Which is why I changed feeds_plugin() to record the plugin_key.

The changes in feeds_plugin() are unnecessary and wrong.

Wouldn't it make more sense to have the hook name be hook_feeds_PLUGIN_TYPE_defaults()?

1. +++ b/feeds.api.php
   @@ -363,5 +363,24 @@ function my_module_form_callback($mapping, $target, $form, $form_state) {
   +function hook_feeds_defaults(FeedsConfigurable $configurable) {
   +  if (is_a($configurable, 'FeedsProcessor')) {
   +    return array(
   +      'purge_unseen_items' => 0,
   +      'purge_after_period' => NULL,
   +      'purge_delete' => 0,
   

   The hook name is wrong.
   Use instanceof rather than is_a().
   The array being returned should be something more generic.

2. +++ b/includes/FeedsConfigurable.inc
   @@ -166,7 +166,14 @@ abstract class FeedsConfigurable {
   +    $defaults = array();
   +    if (method_exists($this, 'pluginType')) {
   +      $hook = 'feeds_' . $this->pluginType() . '_defaults';
   +      $defaults = module_invoke_all($hook, $this);
   +    }
   +    $defaults = array_merge($defaults, module_invoke_all('feeds_defaults', $this));
   

   This should be moved to FeedsPlugin.

3. +++ b/plugins/FeedsCSVParser.inc
   @@ -174,7 +174,7 @@ class FeedsCSVParser extends FeedsParser {
   +    )+parent::configDefaults();
   

   Needs spaces.

This also needs a test to ensure it works.

This is also useful for the Feeds Import Preview module to bundle additional configuration along with the importer. See #2735141-3: Extending to Preview All . .

So I took a step at this and did the following:

1. Changed the names of the two hooks to hook_feeds_config_defaults() and hook_feeds_PLUGIN_TYPE_config_defaults()
2. Documented the hooks in feeds.api.php.
3. Moved the plugin specific hook to FeedsPlugin::configDefaults().
4. Added automated tests for both hooks.

Still need to check if this implementation helps Feeds Import Preview enough.

This patch work's great for my usecase :)

#2822831: Allow other modules to add other options for dealing with non-existent items

Thanks for the feedback, I make this a target for 7.x-2.0-beta4.

Reroll.

I'm testing the patch now with Feeds Import Preview module. It appears that the only thing to add is that the hook hook_feeds_config_defaults() may live in modulename.feeds.inc. For the hook hook_feeds_PLUGIN_TYPE_config_defaults() I don't think this is possible, as it is a dynamic hook.

Turns out that hook_feeds_PLUGIN_TYPE_config_defaults() *can* be detected as PLUGIN_TYPE is either 'fetcher', 'parser' or 'processor', so I added these three variations to feeds_hook_info(). Also moved hook implementations for the tests to feeds_tests.feeds.inc.

Committed #14.