Port FeedAPI Mappers: link

I need this for my project so I started working on it.

I need to write some tests and create a patch file but here what i have at the moment

function link_feeds_node_processor_targets_alter($targets, $content_type) {
  $info = content_types($content_type);
  $fields = array();
  if (isset($info['fields']) && count($info['fields'])) {
    foreach ($info['fields'] as $field_name => $field) {
      if ($field['type'] == array('link')) {
        $name = isset($field['widget']['label']) ? $field['widget']['label'] : $field_name;
        $targets[$field_name.'#url'] = array(
          'name' => $name . ' (' . t('url').')',
          'callback' => 'link_feeds_set_target',
          'description' => t('The URL for the CCK !name field of the node.', array('!name' => $name)),
        );
        if(in_array($field['title'], array('optional', 'required'))) {
          $targets[$field_name.'#title'] = array(
            'name' => $name . ' (' . t('title').')',
            'callback' => 'link_feeds_set_target',
            'description' => t('The title for the CCK !name field of the node.', array('!name' => $name)),
          );
        }
      }
    }
  }
}

function link_feeds_set_target($node, $target, $value) {
  static $defaults = array();
  list($field_name, $sub_field) = split('#', $target);

  if(!isset($defaults[$node->type][$field_name])) {
    $field = content_fields($field_name, $node->type);
    $defaults[$node->type][$field_name]['attributes'] = $field['attributes'];
    if(!in_array($field['title'], array('optional', 'required', 'none'))) {
      $defaults[$node->type][$field_name]['title'] = $field['title'];
    }
  }
	
  $field_data = isset($node->$field_name) ? $node->$field_name : array();

  if(!is_array($value)) {
  	$value = array($value);
  }

  $i = 0;
  foreach ($value as $v) {
    if(!isset($field_data[$i])) {
      $field_data[$i] = $defaults[$node->type][$field_name];
    }
    if ($sub_field != 'url' || (($v = link_cleanup_url($v)) && valid_url($v, true))) {
      $field_data[$i][$sub_field] = $v;
    }
    $i++;
  }
  $node->$field_name = $field_data;
}

I don't know if using hash in the targets for subfields is the right way of doing it but it works.

Here is a patch file against HEAD with the mapper in #3 and its test case.

Sorry for the tabs, here is a fixed patch.

The patch in #6 worked beautifully for me.

Yes also for me #6 is working ok. I import some CSV data with affiliate link and used this patch to map it to CCK link field.
You can see example here:
http://www.pine-chest-of-drawers.com/

I just want to confirm that this patch worked flawlessly for me to!

I tested the patch from #11 with a CSV import feed with 3 different URL sources/targets. All of them worked for me. Just wanted to provide some feedback.

Subscribing, greetings, Martijn

In link_feeds_set_target()
- $defaults is used to store the default title and attributes for the field
- Entries in $defaults are indexed by node type and field name ($defaults[$node->type][$field_name])
- For each value, the defaults are assigned to the field if not already set
- Each URL value is cleaned and validated by (($v = link_cleanup_url($v)) && valid_url($v, true))
- Title values are not validated and directly assigned to the field

subscribing with interest, this makes me consider switching for next version of my site

Hi,

the patch from #11 applies cleanly against feeds 6.x-1.0-alpha9.

However (I'm not sure if I am in the right issue queue), I was expecting to become able to map an item from a feed (= "source") to a CCK 'link' field (= "target") at "Mapping for Node processor"; that still doesn't work.

I'm absolutely new to this module and simply want to put the "item URL" from an RSS feed into a CCK field - forgive me if I got something wrong...

Greetings, -asb

Subscribing and will test....

Alex how are we feeling about this in terms of committing it? It has been working well for me now for over a month.

subscribing

I've applied the patch in comment #11 (623444-11_mapper_link.patch) to the 6.x-2.x Feeds branch. SimpleTest reports no fails or exceptions and I'm happy with the mapping results from the iCal parser module (parser_ical).

Another happy customer here. Just tested it against alpha11 and it's working well.

Actually I took this for a spin and noticed that it works fine with purely textual fields but chokes if an enclosure is mapped to the link field. Adding this chunk:

    if ($v instanceof FeedsEnclosure) {
      $v = $v->getValue(); 
    }

inside of the main link_feeds_set_target() loop gets everything working nicely -- and allows me to create linkfields that point to the enclosures from a feed item. I can roll a patch, but wanted to do a quick sanity check with others more familiar with the system: is that solution the right approach, or a mistake?

#24: I'm actually surprised that this is necessary. FeedsEnclosure extends FeedsElement and FeedsElement implements a __toString() method - doesn't the conversion happen automatically or is an explicit conversion required?

I was a bit confused by it as well: both of those test lines print out the same 'test' string. But when the FeedsEnclosure object is passed into Link module and Drupal's helper functions (link_cleanup_url() and valid_url() specifically) things seem to get unhappy. link_cleanup_url() for example tries to call trim() on the FeedsEnclosure object directly, resulting in an empty string.

With the explicit conversion, everything works fine.

Added suggestion of #24 to patch of #11. Ran tests and all works fine. Ready to be committed?

I have been using this since December and have not found any problems or issues with it.

Working for me- thanks!

The patch works nicely and I am currently using it. But there are some problems with the coding standards and some thoughts about the test:

If you run coder there will be a lot of messages.

feeds_mapper_link.test
Line -1: @file block missing (Drupal Docs)

severity: normalLine 25: Use an indent of 2 spaces, with no tabs
'devel', 'feeds', 'feeds_ui', 'ctools', 'content', 'link'

severity: normalLine 35: Use an indent of 2 spaces, with no tabs
'access devel information',

severity: normalLine 143: Control statements should have one space between the control keyword and opening parenthesis
if(in_array($field_name, array('alpha', 'beta', 'gamma', 'omega'))) {

severity: normalLine 145: Control statements should have one space between the control keyword and opening parenthesis
if(in_array($field_name, array('alpha', 'gamma'))) {

severity: normalLine 157: Control statements should have one space between the control keyword and opening parenthesis
if(in_array($field_name, array('alpha', 'beta', 'gamma', 'omega'))) {

severity: normalLine 158: Control statements should have one space between the control keyword and opening parenthesis
if(!is_array($value)) $value = array('url' => $value);

severity: normalLine 160: Control statements should have one space between the control keyword and opening parenthesis
if(in_array($field_name, array('alpha', 'gamma'))) {

link.inc
severity: normalLine 30: string concatenation should be formatted with a space separating the operators (dot .) and non-quote terms
'name' => $name .' (' . t('title').')',

severity: normalLine 64: Use an indent of 2 spaces, with no tabs
$value = array($value);

severity: normalLine 75: Use uppercase for PHP constants, e.g. NULL, TRUE, FALSE
if ($sub_field != 'url' || (($v = link_cleanup_url($v)) && valid_url($v, true))) {

Dreditor

+++ mappers/link.inc	11 Mar 2010 10:53:09 -0000
@@ -0,0 +1,82 @@
+  	$value = array($value);

There is a tab.

+++ mappers/link.inc	11 Mar 2010 10:53:09 -0000
@@ -0,0 +1,82 @@
+      $v = $v->getValue(); ¶

Space after line end.

+++ tests/feeds_mapper_link.test	11 Mar 2010 10:53:10 -0000
@@ -0,0 +1,169 @@
+    	'devel', 'feeds', 'feeds_ui', 'ctools', 'content', 'link'

Tab again.

+++ tests/feeds_mapper_link.test	11 Mar 2010 10:53:10 -0000
@@ -0,0 +1,169 @@
+        	'access devel information'

Another tab.

+++ tests/feeds_mapper_link.test	11 Mar 2010 10:53:10 -0000
@@ -0,0 +1,169 @@
+        'target' => 'title'

Arrays should have a "," after their last item as well. This happens several times.

Powered by Dreditor.

And about the test. You use

+ // Edit the imported node.
+ $this->drupalGet('node/2/edit');

which expects the existens of node id 2. Imho it would be better to select the nid by something else like the title. So you can use a random string.

And the URL from develompement seed. Again I believe it would be better to use something abstract. By this we should have some more reliability as the test constantly changes and it will exists forever.

There is a slight problem with the patch in #27 in that if you have define the field with a static title and are importing an empty URL field, it assigns the default values before it checks if there is a value being imported. This leaves you with a field with a static title defined and no URL. The expected behavior for records without a URL would be not to set any value, no? Not sure if I solved it the proper way but you can see what I did in the attached patch.

can someone tell me... I dont have the link file to patch in my mappers directory........ ?

@AntiNSA- the patch in #31 contains the link.inc file.

+++ mappers/link.inc	11 Mar 2010 10:53:09 -0000
@@ -0,0 +1,82 @@
+  	$value = array($value);
...
+      $v = $v->getValue(); ¶

+++ tests/feeds_mapper_link.test	11 Mar 2010 10:53:10 -0000
@@ -0,0 +1,169 @@
+    	'devel', 'feeds', 'feeds_ui', 'ctools', 'content', 'link'
...
+        	'access devel information'

Still some minor codingstyle issues.

Powered by Dreditor.

And I got many errors running the simpletest:

Failed to set field _add_new_field[label] to alpha_link_label	Other	feeds_mapper_test.inc	133	FeedsMapperTestCase->createContentType()
...
"Added field alpha_link_label" found	Other	feeds_mapper_test.inc	138	FeedsMapperTestCase->createContentType()
...
Failed to set field _add_new_field[label] to beta_link_label	Other	feeds_mapper_test.inc	133	FeedsMapperTestCase->createContentType()
...
Failed to set field _add_new_field[label] to gamma_link_label	Other	feeds_mapper_test.inc	133	FeedsMapperTestCase->createContentType()
...
Found the requested form fields at admin/build/feeds/edit/syndication/mapping	Other	feeds.test.inc	305	FeedsWebTestCase->addMappings()
...
Found the requested form fields at admin/build/feeds/edit/syndication/mapping	Other	feeds.test.inc	305	FeedsWebTestCase->addMappings()

There are many errors like this. But the real error is here:

Call to undefined function content_fields()	PHP Fatal error	feeds_mapper_link.test	156	Unknown	

I run it on a clean D6 stage. Sorry seems to be more work about this :-/

[EDIT]
The last error is basically my fault. I didn't enable CCK on my stage. The lables are still missing but that part works as expected. So this should never happen as no one can select a link CCK field if its not active. But the question comes up if than the mapper plugin should get some wrapper or dependencie to cck? Maybe the test script should double check if the module is really enabled during the test?

I have a CCK Link field type in my target and I applied patch #31 (first to 6.x-1.0-alpha14 and then to 6.x-1.x-dev).
In both instances clicking on Mapping results in a WSOD and the following error is logged

PHP Parse error: syntax error, unexpected $end in /var/www/drupal/sites/all/modules/feeds/mappers/link.inc on line 83

Also getting WSOD error.

+1 on the error on #31. It's just missing two close braces at the bottom. Add this -

  }
}

I have no idea why they don't get applied as they seem to be in the patch itself. Mine cut off after the $node->$field_name = $field_data; line.

subscribing

Applied #31 and fixed manually as in #38 and it worked. I am not sure why the patch is not working right.

Fixed the patch in 31. I think some lines got added after the patch was made. Also fixed code style issues in 35. Seems to be working (fixed all the WSOD stuff) but I have not looked at test results yet.

@servantleader
If you do a patch please set it to "Needs review" :)

Didn't real test it but there is minor coding convention thing here:

+++ mappers/link.inc	7 May 2010 16:28:38 -0000
@@ -0,0 +1,84 @@
+  	$value = array($value);

Don't use tabs.

Powered by Dreditor.

Subscribing

I'm getting the following warning when I import feed items:

user warning: Field 'url' doesn't have a default value query:
INSERT INTO feeds_node_item (nid, id, feed_nid, imported, hash)
VALUES (147, 'my_test_feed_importer', 130, 1275400767,
'509e52a5402134355afe8dd9017ac239') in
C:\Program Files\Apache Software Foundation\Apache2.2\htdocs\drupal-6.16\includes\common.inc
on line 3523.

I'm not sure if this is a "links" module problem, a CCK problem, a Feeds problem or a SQL problem.

I'm running Feeds15Alpha, the latest "links" module, the patch from #44 above. MySQL is ver 5.1.44 and PHP is 5.3.2.

@Spark_man Is your MySQL runnung in strict mode?
http://dev.mysql.com/doc/refman/5.0/en/server-sql-mode.html#sqlmode_stri...

Normally that shouldn't be a problem.

Yes ... I removed the startup variable declaration and it eliminated the problem, but the question is, is this a code problem? I mean, if MySQL 5.x provides this feature as a safeguard against invalid data, shouldn't the code also ensure that it is properly populating the data fields or at least defining them in properly?

I cant get any of my custom cck fields mapped