Support from Acquia helps fund testing for Drupal Acquia logo

Comments

johnbarclay’s picture

Title: Phone field not support » Feeds Mapper for Phone field needed
Project: Feeds » Phone
Version: 7.x-2.0-alpha4 » 7.x-1.x-dev

This is a duplicate of #959106: Feeds mapper for phone CCK fields, which was for 6.x.

Mapper support in feeds is only for drupal core fields. This request should be in the phone module. Perhaps a second feature request for 7.x mapper is in order also.

Here is a list of mappers and mapper requests: [#856780]

berenddeboer’s picture

Status: Active » Needs review
FileSize
2.02 KB

Here's the patch.

Ben Coleman’s picture

Status: Needs review » Reviewed & tested by the community

Applied this here, and it seems to work fine. The field shows up in the mapper, and appears to import correctly.

kthull’s picture

#2 worked great...thanks!

berdyshev’s picture

Dev version of the feeds 2.x module allows to simply add MODULE_NAME.feeds.inc file in the root of the module folder and it will load it automatically.
So I have refactored the patch from comment #2 to use this feature. Please review it.

jeffschuler’s picture

Patch in #5 works for me.
Thanks!

TimelessDomain’s picture

i can confirm that #5 works. thanks!

joshmiller’s picture

Patch applied and feeds brought in data.

My only complaint is to do with the fact that the phone field requires a default country. If I'm importing mexican, candadian, and US phone numbers, two of the three will fail validation. Sad, but fixable.

Josh

star-szr’s picture

The patch in #5 works, I'm not sure why the @file refers to link.module though.

deggertsen’s picture

Project: Phone » Feeds

#5 works for me as well. I had to save the file, name it as "phone.inc" and put it in modules/feeds/mappers before it would work. I am reassigning it to the feeds project, as it would need to be applied there I believe. If I'm wrong, please switch it back to "Phone".

Thanks!

star-szr’s picture

Project: Feeds » Phone

@deggertsen - You'll need to use 7.x-2.x-dev of the Feeds module for now.

MrPeanut’s picture

Just confirming that #5 works for me as well.

JSCSJSCS’s picture

It worked for me too, but there is one little side effect. When using Views, and adding fields to a view, there are now TWO phone fields to choose from:

Content: Phone Number
Appears in: node:dealers.

Content: Phone Number (field_dealer_phone_number:delta)
Delta - Appears in: node:dealers.

I had to pick the first one to get anything to show.

jeffschuler’s picture

JSCSJSCS: That's not from this patch. Multiple-value fields provide that to views so you have access to the index of the value within the field.

Rob_Feature’s picture

Yeah, confirmed that #5 works. Let's get this baby committed!

TimelessDomain’s picture

You can add the following code to make sure that empty values are not imported. More info here #1107522: Framework for expected behavior when importing empty/blank values + text field fix

  list($field_name, $sub_field) = explode(':', $target);
  foreach ($value as $v) {
+    // Check if phone value is empty.
+    if(empty($v[0])){
+      continue;
+    }
cweagans’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/phone.feeds.incundefined
@@ -0,0 +1,56 @@
+ * @file
+ * On behalf implementation of Feeds mapping API for link.module.

Copy/paste error?

+++ b/phone.feeds.incundefined
@@ -0,0 +1,56 @@
+/**
+ * Implements hook_feeds_processor_targets_alter().
+ *
+ * @see FeedsNodeProcessor::getMappingTargets().
+ */
+function phone_feeds_processor_targets_alter(&$targets, $entity_type, $bundle_name) {
+  foreach (field_info_instances($entity_type, $bundle_name) as $name => $instance) {
+    $info = field_info_field($name);
+    if ($info['type'] == 'phone') {
+      $targets[$name . ':url'] = array(
+        'name' => check_plain($instance['label']),
+        'callback' => 'phone_feeds_set_target',
+        'description' => t('The @label field of the node.', array('@label' => $instance['label'])),
+      );
+    }
+  }
+}

Why is this implementing hook_feeds_processor_targets_alter() instead of just hook_feeds_processor_targets()? If there's a reason for it, can we add a comment about it? If not, let's change it so that other modules can alter phone's feed processor targets without having to mess with the module weights.

dddbbb’s picture

#5 worked fine for me. Thanks!

barraponto’s picture

Status: Needs work » Needs review
FileSize
1.73 KB

Implemented the check mentioned in #16 and gave the file a proper description, as mentioned by @cweagans. However, as for the second issue @cweagans mentions, I found no mention of phone_feeds_processor_targets() in feeds.api.php. Thus, I left it implemented as phone_feeds_processor_targets_alter()

star-szr’s picture

Indeed, hook_feeds_processor_targets_alter() is the hook to use. If other modules need to change the Phone module's Feeds processor targets, couldn't they just implement hook_module_implements_alter()?

Patch looks good to me now that the @file comment has been fixed up.

cweagans’s picture

Assigned: Unassigned » rfsbsb
Status: Needs review » Reviewed & tested by the community

Ugh, I hate it when modules have alter hooks without info hooks. That's just poor design. I'm okay with this patch, then. I'll leave this as RTBC and let rfsbsb commit it.

rfsbsb’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 7.x-1.x

Status: Fixed » Closed (fixed)

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

tgeller’s picture

I still can't get it to show up in my Feeds mappers. I'm running:

  • Feeds 7.x-2.0-alpha7+29-dev
  • CCK Phone 7.x-1.x-dev

I downloaded #19 and applied the patch. I then tried:

  • moving it into /sites/all/modules/feeds (with the name phone.feeds.inc)
  • moving it into /sites/all/modules/feeds/mappers (with the name phone.inc)

But when I go to edit the Feed Mapper, I don't see my phone field in the popup menu. What am I missing?

tgeller’s picture

Uh-oh. I have cck_phone installed, not phone.

*headdesk*

alex.skrypnyk’s picture

Status: Closed (fixed) » Needs work

Code in #16 and commit in #19 contains error:
Replace

  if(empty($v[0])){

with

  if(empty($v)){

How did it get committed if the feeds import does not work?

alex.skrypnyk’s picture

Re-rolled patch for #19 attached

milos.kroulik’s picture

Issue summary: View changes

Patch at #27 doesn't apply anymore for Phone 7.x-1.0-beta1.

joelpittet’s picture

Assigned: rfsbsb » Unassigned
Status: Needs work » Needs review
FileSize
447 bytes

#26 seems to be right, here's the patch.

szy’s picture

There is still the error with the latest patch:

"Notice: Undefined variable: field w phone_feeds_set_target() (linia 58 z /var/www/my.drup.al/modules/phone/phone.feeds.inc)."

But the phone numbers were imported successfully - with Phone 7.x-1.0-beta1.

Szy.

joelpittet’s picture

$field may need to be set as an empty array() or NULL to initialize it @szy? That or we ignore it if it's not isset()?

Also 'und' should really be the constant LANGUAGE_NONE.

joelpittet’s picture

Version: 7.x-1.x-dev » 7.x-1.0-beta1

I guess my patch above was for beta1.

joelpittet’s picture

Version: 7.x-1.0-beta1 » 7.x-1.x-dev

And I'm crazy it applies to both.