Problem/Motivation

With Feeds now officially supporting field translation using Entity translation, mapping targets provided by contrib modules should be adjusted to follow this change.
For fields that can be multilingual, Feeds added a mapping option "language" which is passed to all target callbacks. To support Feeds 7.x-2.0-beta2 and later, this option should be respected.

Proposed resolution

Follow the instructions in the change record: use $mapping['language'] at places where LANGUAGE_NONE or und is used.

Remaining tasks

Apply the proposed resolution.

User interface changes

When the field is configured as multilingual, on the target configuration an option will become available to configure the language. This module is expected to respect that option.

API changes

None.

Data model changes

None.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

MegaChriz created an issue. See original summary.

joelpittet’s picture

Status: Active » Needs review
FileSize
2.02 KB

Thanks for all the bits @MegaChriz. I put a couple of other fixes and the $mapping was already used but not passed in a parameter.

joelpittet’s picture

@MegaChriz What do you think, does this fix it for you?

MegaChriz’s picture

Status: Needs review » Needs work

@joelpittet
Quick glance over I think it is mostly okay. I do notice that the made changes make Entityreference incompatible with Feeds versions older than 7.x-2.0-beta1 (the $values parameter is always an array since that version, but before that it was not enforced), but I don't think that is a problem. Feeds 7.x-2.0-beta1 was released more than two years ago.

  1. Nitpick:
    +++ b/entityreference.feeds.inc
    @@ -62,17 +62,18 @@ function entityreference_feeds_processor_targets_alter(&$targets, $entity_type,
    + * @param \FeedsSource $source
    

    This the D8 style of documenting classes, not sure if this should be done as well for D7.
    Now we are at it, perhaps the parameters of entityreference_feeds_set_target() don't have to be documented at all as it is implementing a callback. The parameters are already documented in my_module_set_target() in feeds.api.php.

  2. +++ b/entityreference.feeds.inc
    @@ -62,17 +62,18 @@ function entityreference_feeds_processor_targets_alter(&$targets, $entity_type,
    +function entityreference_feeds_set_target(FeedsSource $source, $entity, $target, array $value, array $mapping) {
    

    With typehinting the $value parameter to be an array, the following code lines become redundant and should be removed:

    // Assume that the passed in value could really be any number of values.
    if (is_array($value)) {
      $values = $value;
    }
    else {
      $values = array($value);
    }
    

    Note that $value parameter should then be renamed to $values as well (note the 's' at the end).

  3. If you are ambitious, you could add an automated test for this functionality. There is an example for the email field module in #2672732: Feeds integration: support mapping option "language".
joelpittet’s picture

Status: Needs work » Needs review
FileSize
1.51 KB
2.37 KB

Not that ambitious;) But I tried to address your other points.

Thanks for the review @MegaChriz!

vbard’s picture

Status: Needs review » Reviewed & tested by the community

#5 works with latest dev, thank you!

joelpittet’s picture

Bump

joelpittet’s picture

Bumping again because I saw a new release and this didn't get in with it. (we'll continue to apply the patch)

joseph.olstad’s picture

Triggered tests for PHP 8.0, 8.1, 8.2

joseph.olstad’s picture

Status: Reviewed & tested by the community » Fixed

Fixed in 7.x-1.x-dev

joseph.olstad’s picture

joelpittet’s picture

Thanks @joseph.olstad!

Status: Fixed » Closed (fixed)

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

joseph.olstad’s picture