Mapper for user id and user name

Which parser are you referring to?

Author and language of feed source was not a mapping source in FeedAPI/FeedAPI Mapper either. Author and mapping of feed item perhaps?

I'm not sure what chris001 had in mind, but I have a use case where I'd like to inherit characteristics like node author and taxonomy from the feed node. I have some prototype code that allows adding the feed node author via the Node Processor mapping feature. Should be easy to extend so feed node taxonomy can be inherited.

I'm not sure if this fits into the Feeds design philosophy. By strict definition it's not source information, though it is an attribute of how the source information is retrieved.

Here's the abbreviated code:

class FeedsSyndicationParser extends FeedsParser {

  public function parse(FeedsImportBatch $batch, FeedsSource $source) {
    $result = common_syndication_parser_parse($batch->getRaw());

    // Grab feed node information and add to item list
    $feed_node = node_load($source->feed_nid);
    $feed_node_author_data = array('uid' => $feed_node->uid, 'name' => $feed_node->name);
    foreach($result['items'] as $index => $item) {
      $result['items'][$index]['feed_node_author'] = $feed_node_author_data;
    }

    $batch->setTitle($result['title']);
    $batch->setDescription($result['description']);
    $batch->setLink($result['link']);
    $batch->setItems($result['items']);
  }

  public function getMappingSources() {
    return array(
      'feed_node_author' => array(
         'name' => t('Feed Node Author'),
         'description' => t('Node author of feed node.'),
      ),
    );
  }
}

On the target side:

function modulename_feeds_node_processor_targets_alter(&$targets, $content_type) {
  $targets['node_author'] = array(
    'name' => 'Node author',
    'callback' => 'modulename_feeds_set_node_author',
    'description' => t('The node author.'),
  );
}

function modulename_feeds_set_node_author($node, $target, $value) {
  $node->uid = $value['uid'];
  $node->name = $value['name'];
}

It would be nice if this, or an equivalent, could be incorporated into the FeedsParser class so it was standard with all parsers.

chris001: is #2 what you have been asking for? Please clarify request.

Hi Alex,

I am looking for this functionality. I just need to get the author's name for each feed article. What do you suggest?

Awesome work btw!

Thanks,
Tim

I'd like to take a crack at adding the following functionality:
1) A source that provides the author of the feed node (if it exists)
2) A target that allows the feed item node author to be set

Questions:

• The node author consists of uid and name, should these be presented individually?
  I'm thinking that they should, especially on the target side. It gives maximum flexibility if you're importing. I believe the example I gave in #2, which uses an array, would preclude importing from a file where only text fields are possible.
• What if uid and name conflict?
  If both uid and name are given, and they conflict, what action should be taken? I'm thinking either the record can be thrown out and an error logged, or one value can take precedence over the other (probably uid over name) and a warning logged.
• Security Implications?
  What security issues need to be dealt with? In my use case where the node author is being set from an existing node, many security issues go away. When the Drupal account id can be set from an arbitrary imported field originating from somewhere on the Internet, is there some due diligence required in code? A warning to be careful when using? For example, could creating a feed item owned by account 1 (or other privileged account) allow malicious content to run because an input filter is configured more permissive.

And I agree with twooten, awesome work on the module.

#5: Great.

Let's focus this issue on a mapper for user.uid and user.name - actually getting a uid or a name from a feed belongs on a separate issue, a mapper for language belongs on a separate issue, too.

The node author consists of uid and name, should these be presented individually?
I'm thinking that they should,

Agreed.

What if uid and name conflict?

You'd have to map both which doesn't make much sense in the first place. If they do overlap, the second mapping will override the first one. Let's not worry about this.

Security Implications?

A note similar to the one on the Node Processor settings may be warranted.

This patch points the direction how this should be implemented. FeedsNodeProcessor exposes uid and user_name, uid doesn't need special handling, user_name needs to look up a uid by user name when mapping.

torelad: It's all yours now.

For your consideration, mappers and tests for UID and username.

I do a lookup for both UID and username mappings. As far as I can determine, node_save uses both node->uid and node->name (username), so in each case I need to look up the other. It's also a good way to test for account existence.

With both mappers, if a specified account does not exist the node is left owned by anonymous.

Since the user administration page (admin/user/user) uses "Username", I've used this in the text strings (as opposed to "user name" or "user_name").

#7 - as a rule, there is no need to implement simple core fields as an external mapper.

So in the case of uid or user name for node authors, these would be core fields that are simple and common enough to implement within FeedsNodeProcessor like I've suggested in #6 (compare to node title or node body). Using #6 as a guidance would greatly simplify your patch while keeping the tests the same.

Am I missing something?

Ah, when I reviewed the provided patch the full significance didn't sink in.

Implementation redone as part of FeedsNodeProcessor.

#10 Awesome. Nice work.

Now for the nitpicks:

1. As we're not using a separate mapper now, the test can go into feeds.test (I know the feeds.test is large already, breaking it out + cleaning it up is a separate issue).
2. Let's not look up a uid for a uid: It feels like a validation of the input but it isn't really one: it merely checks if a uid exists in the system, if there are many many users on a site this check isn't worth much.
3. If you agree w/ 2. let's use "uid" as key for user.uid. This will allow us to handle uid in the same pass with title, status and created in setTargetElement().
4. Don't use periods in key names, but underscores: user.name becomes user_name. I need to document this but I'd love to keep mapping keys to alphanumerics+underscore with the exception of colons for sub targets (see taxonomy mapper).
5. It would be trivial and very useful to add a target 'user_mail' that looks up a user by their email in analogy to 'name'.

Again, nice work. Can't wait to get this committed.

Heh, I knew it couldn't be that easy :-)

#1: Ok.

#2: After looking more closely, you're right, adding the username to node object serves no useful purpose. Only the UID is saved.

I just did some quick testing, there are problems if you save a node with a UID that doesn't exist. It doesn't show up in admin/content/node and you can't use node_load() to retrieve it. node_load uses an inner join on UID, so if the UID doesn't exist no result is returned even though there's a node record. This also causes the feed "Delete items" option to loop (I think the code uses node_load). If my testing is correct (it might not be) we need to do some kind of check to insure the UID is valid.

#3/4: Ok.

#5: I can do that.

I just did some quick testing, there are problems if you save a node with a UID that doesn't exist. It doesn't show up in admin/content/node and you can't use node_load() to retrieve it. node_load uses an inner join on UID, so if the UID doesn't exist no result is returned even though there's a node record. This also causes the feed "Delete items" option to loop (I think the code uses node_load). If my testing is correct (it might not be) we need to do some kind of check to insure the UID is valid.

Drats. I'd love to do direct hits to the user table instead of using user_load() then in order to keep the import performing well - look at user_load() - it's a very busy function.

#11-5 is btw just an idea - I'd commit the patch without it.

I believe this patch addresses all the issues we discussed.

Wasn't sure how to describe the email->uid mapping option, so I took a shot. Let me know if you want different text. Actually, that goes for any of the descriptions. We've been calling it user mapping, but I wonder if node author isn't a better description.

Looking great. I need to review.

I rerolled after recent commits broke #14. I slightly simplified logic in FeedsNodeProcessor.

Nice work.

- testUserUIDMapper() testUserUsernameMapper() and testUserEmailMapper() should be called from a single test() method - Drupal SimpleTest sets up and breaks down full Drupal installs for each one of them and our test suite is already slooowwww...
- Not sure why FeedsUserMappersTestCase is derived from FeedsMapperTestCase - what are you reusing from that class?

subscribing

File to patch: plugins/FeedsNodeProcessor.inc
patching file plugins/FeedsNodeProcessor.inc
Hunk #1 succeeded at 195 (offset -15 lines).
...
File to patch: tests/feeds.test
patching file tests/feeds.test
Hunk #2 FAILED at 823.
1 out of 2 hunks FAILED -- saving rejects to file tests/feeds.test.rej
patching file user_mapper.csv

Using this patch with beta 4 and it works great with the Feeds XML Parser. What needs to be done to get it in a feeds release?

#16 needs to be resolved.

Back to where I can spend some more time on this.

I have testUserUIDMapper() testUserUsernameMapper() and testUserEmailMapper() as separate tests because I'm creating the test data from a single file. I use node_load by title to retrieve the node from the database and verify it was created correctly. Without separate tests there's a risk of old data intefering with the new test.

To insure unique title names/free data for each test without separate tests I'd need to either delete the nodes between each run (UID/Username/Email) or use individual test files. Either solution is probably "cheaper" than separate tests, though I think separate tests is the most "correct". Which solution would you prefer, deleting nodes between each test, or 3 different .csv import files, one for each test?

FeedsUserMappersTestCase is derived from FeedsMapperTestCase so I can create a test parser:
$importer_id = 'importer_'. mt_rand();
$this->createFeedConfiguration($importer_id, $importer_id);
$this->setSettings($importer_id, NULL, array('content_type' => '', 'import_period' => FEEDS_SCHEDULE_NEVER));
$this->setPlugin($importer_id, 'FeedsFileFetcher');
$this->setPlugin($importer_id, 'FeedsCSVParser');
$this->setSettings($importer_id, 'FeedsNodeProcessor', array('content_type' => $typename));

If there is a better way of doing this, please let me know.

I was incorrect in #21, I was using FeedsMapperTestCase because I was using $this->createContentType() function.

After thinking about it, using 3 individual test input files seemed like the best way to go. I've redone the code as follows:
- Test class now derives from FeedsWebTestCase like the other test classes
- There is a single test, not 3
- There are 3 test input files: user_mapper_email.csv, user_mapper_uid.csv, and user_mapper_username.csv

I tried #22 against beta 9 but was unable to get it apply with an error on line 11. Is this thread/patch the recomended way to importing and author from a feed? If not what are people using, if so what do you need from me to get this commited.

Thanks.

Is there no way to map to a user id. Under mapping I select a target of User Id and have a feed that includes the users ID number but no matter what number is present in the feed every created node is listed as being created by user 1. Is this something that is going to be included in feeds, has it been abandoned or am I missing a step. I have tried it using the Common syndication parser and the XPath XML parser but neither seem to work. Hopefull it is just me doing something stupid.

Thank you.

#22 does not apply to 6.x branch.

So then what is the correct way to map a user id, is this not possible? My feeds have the correct user id for the author but I see no way to use the value. If this is incorrect can you please point me to some info?

In beta 10 this just started working for me, mapped just like any other field. Unfortunately I do not know what I changed to make it work but it does work now.

I recreated the patch provided in #22 with Git. That allows it to apply, mostly. However, since that patch was created the single big file containing tests has been deleted and a conflicting change has been added to plugins/FeedsNodeProcessor.inc. In short the patch needs a lot of work to actually function.

I cleaned up the patch enough that it applies and split it into two. The functionality and the tests. I'm pretty sure the tests are completely broken at the moment. I'm afraid that I cannot be the one to fix them however. Hope that helps someone.

FYI: patch is working for me

I'm just trying to sort out what needs to happen for this patch to get in. It looks like the User ID was put in here: #853194: Mapping: don't reset all targets and this got into beta3.

I'll try re-creating the patch with the tests, without User ID.

My tests are failing but this patch is cleanest so far. I'll try to fix up the tests.

Which field were you trying to map? Username or email? Did either work?

The patch in #32 worked for me, at least for username - haven't tested email.

subscribing

I've submitted a D7 patch that will allow you to map via username, user ID, or inherit from the current user all within the same field (ie. what the patch in this thread does plus some bonuses): http://drupal.org/node/1189000.

Shiraz

sub

Any Chance for getting a patch that works with Drupal 7 please!!

I really enjoy this module but can't work with out the submitting author to be assigned to the imported nodes.

Hi Richard_1618, please don't hijack issue statuses. If you want this for Drupal 7 please open a new feature request (or, in this case, see #1189000: submitting patch to map to username OR user ID OR current user in same field as somebody has already done this.)

Fixed in #1241754: Add targets for author name and email in node processor..