Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
#1183440: Multilingual Feeds - Make field import language-aware has demonstrated that out mapping configuration API is less than stellar and should be significantly enhanced.
We should, in a backwards compatible way, allow multiple form and summary callbacks to be defined. We should also add either a mechanism for default values, or a process callback so that the configuration values can be prepared before mapping.
Comment | File | Size | Author |
---|---|---|---|
#42 | feeds-extend-mapping-api-2333029-42.patch | 54.12 KB | twistor |
#42 | interdiff.txt | 3.91 KB | twistor |
Comments
Comment #1
twistor CreditAttribution: twistor commentedComment #2
twistor CreditAttribution: twistor commentedLet's see the damage.
Comment #3
MegaChriz CreditAttribution: MegaChriz commentedNot much damage yet! I hope to look at the patch in a few days. Or is the patch just a start/incomplete?
Comment #4
twistor CreditAttribution: twistor commentedIncorporating #1706026: Create a hook_feeds_processor_targets()..
Comment #5
twistor CreditAttribution: twistor commentedThere's plenty more to come.
I still need to:
Comment #6
twistor CreditAttribution: twistor commentedWell that was surprising!
I should have most of the functionality wrapped up today.
Another @todo in this patch: We need a way to ensure
hook_feeds_processor_targets()
is invoked. There will be a gap where other processors won't be callingmodule_invoke_all('feeds_processor_targets')
, but still callingdrupal_alter('feeds_processor_targets')
.I'm thinking we can set a global flag that tracks if
module_invoke_all('feeds_processor_targets')
has been called, and if it hasn't we can call it fromfeeds_feeds_processor_targets_alter()
. It's not very pretty, and we should probably post patches to the relevant processor modules, but we should have a hack for the mean time.Comment #7
twistor CreditAttribution: twistor commentedAlrighty, this is what I was thinking about with the preprocess business.
I'm not sure if it makes sense yet.
Comment #8
twistor CreditAttribution: twistor commentedComment #10
twistor CreditAttribution: twistor commentedComment #12
twistor CreditAttribution: twistor commentedWhew! I would have been worried if I didn't get any failures.
Comment #13
twistor CreditAttribution: twistor commentedThis is just to prove to myself that the backwards compatibility works.
Comment #14
twistor CreditAttribution: twistor commentedI might be getting a little carried away, but I'm feeling better about things.
Comment #15
twistor CreditAttribution: twistor commentedComment #16
twistor CreditAttribution: twistor commentedI think that means I should quit for today.
Comment #18
twistor CreditAttribution: twistor commentedComment #19
twistor CreditAttribution: twistor commentedComment #22
twistor CreditAttribution: twistor commentedRe-roll.
Comment #23
twistor CreditAttribution: twistor commentedComment #26
twistor CreditAttribution: twistor commentedComment #28
twistor CreditAttribution: twistor commentedThis found a bug.
We need to bump our required features version to 2.x.
Comment #29
twistor CreditAttribution: twistor commentedI've added some tests.
I figured out a way to make this perfectly backwards compatible with processors that don't invoke hook_feeds_targets. It's hacky, but it works.
I think this is ready to go. I've been running tests on non-upgraded feeds modules I have lying around. Only one problem in feeds_ex, but that was an existing bug.
Comment #31
twistor CreditAttribution: twistor commentedJust need to add 'preprocess_callbacks' => array('feeds_tests_preprocess_callback'), to test_unique target.
I think it's ready!
Comment #32
MegaChriz CreditAttribution: MegaChriz commentedGreat! I hope to take a look at this patch next Thursday. I remember from last time, back in September last year, I had a hard time to review it.
Comment #33
MegaChriz CreditAttribution: MegaChriz commentedThis patch looks like it brings a lot of improvements and clean-ups. Great work!
Here is what I could find so far after looking at the code.
I think you should leave the
call_user_func_array()
here, because a callback can also be defined as an array if the callback points to a class method, for example:See http://php.net/manual/en/language.types.callable.php
Same story here. A callback can be a class method.
Typos:
hook_feeds_processor_targets()
is missing in feeds.api.php.my_module_preprocess_callback()
and an example target to feeds.api.php.Comment #34
twistor CreditAttribution: twistor commentedA quick test about the callback thing.
On my machince,
$callback = [$obj, 'method']; $callback($arg1, $arg2);
works fine, but I seem to remember there being a PHP version issue. I can't seem to track down anything, so throwing up a quick test to verify.Comment #36
twistor CreditAttribution: twistor commentedDocumentation fixes up next.
Comment #38
twistor CreditAttribution: twistor commentedBahhh
Comment #39
twistor CreditAttribution: twistor commentedComment #40
twistor CreditAttribution: twistor commentedIf this passes, I'm going to commit it. Any problems found can be followups.
Comment #41
twistor CreditAttribution: twistor commentedComment #42
twistor CreditAttribution: twistor commentedopps. Forgot the extra test coverage for the second form/summary callbacks.
Comment #44
twistor CreditAttribution: twistor commented