How is uid set in biblio_save_node() ? [#357298]

I have some difficulty understanding the function biblio_save_node() which appears to use undefined variables $userid and $user. The code is:

function biblio_save_node($node, $batch = FALSE, $session_id = NULL) {
  $options = variable_get('node_options_biblio', array ('status'));
  if (module_exists('i18n') && variable_get('i18n_node_biblio', 0)) {
    $node['language'] = module_invoke('i18n', 'default_language');
  }
  $account = user_load($userid);
  $node_template = array (
      'type' => 'biblio',
      'comment' => variable_get('comment_biblio', 0),
      'promote' => in_array('promote', $options),
      'moderate' => in_array('moderate', $options),
      'sticky' => in_array('sticky', $options),
      'format' => 0,
      'status' => in_array('status', $options),
      'uid' => user_access('administer nodes') ? $account->uid : $user->uid,
      'name' => user_access('administer nodes') ? $account->name : $user->name
  );

  $node = (object) array_merge($node, $node_template);
  if ($batch && $session_id){
    $node = serialize($node);
    db_query("INSERT INTO {biblio_import_cache} (session_id, data) VALUES ('%s', %b)", $session_id, $node);
  }else{
    node_save($node);
  }

  return (isset($node->nid)) ? $node->nid : 0;
}

My question is: how do $userid and $user get set?

Comments

Comment #1

rjerome commented 11 January 2009 at 18:58

Hi Gustav,

What you found there was a bit of "dead code", I realized after putting that in, that the node module overwrites the incoming "uid" value so setting it here makes no difference. In order to set the imported nodes to a different userID, I do it with this code right near the end of biblio_import()...

  if (!empty($node_ids) && !$batch ) {
    if (count($node_ids)) {
      db_query('UPDATE {node} SET uid = %d WHERE nid IN(%s)', $userid, implode(',', $node_ids));
      db_query('UPDATE {node_revisions} SET uid = %d WHERE nid IN(%s)', $userid, implode(',', $node_ids));
    }

    return $node_ids;
  }

I'll remove that useless code from biblio_save_node()

Ron.

Comment #2

gustav commented 12 January 2009 at 07:45

Ron, thanks for that prompt and helpful reply. It clarifies things very well.

I am not sure I totally agree that setting $node->uid is useless because other modules may well make use of that information in their hook_nodeapi. So it is probably slightly safer to set it, even though node_save() will not write it to the database.

Personally, I find this whole idea of claiming that some user created certain nodes even though they did not a bit disturbing. What is the purpose? If the admin created the nodes, why not say so? I suspect this came from a time when you tried to use the uid field of the biblio node to indicate who the author of the publication was. This may have been useful before there was a way to link users to authors. Now that you have the nice new mechanism of linking users to authors, could this idea of messing with the uid field perhaps be dropped?

Comment #3

rjerome commented 12 January 2009 at 13:51

You have a couple of good points there which I had not considered.

Your right, the setting of UID came from the need list user publications on their profile pages and you are also right that specific authors can now be associated with Drupal userID's so perhaps this is now a redundant feature.

If anyone else is following this thread I would welcome comments regarding the removal of this feature.

Ron.

Comment #4

pearson.gn commented 22 January 2009 at 23:38

I'm in agreement with Gustav.

I've got a group site where we'd like to share publication listings. However, anyone who imports a bibtex reference finds the uid altered and subsequently can't edit the entry unless they've got admin privileges. Altering the uid doesn't seem desirable, particularly if there's an alternative way to link a uid to an author (which sounds useful... any pointer to how that's done would be appreciated). Linking the user and author should make it simple enough for a user to list their own publications in a profile shouldn't it?

Works like a charm otherwise. Nice stuff.

Graham

Comment #5

rjerome commented 23 January 2009 at 01:15

Hi Graham,

You seem to be describing exactly the opposite of what Gustav was saying. What he was saying was that if the ADMIN imports the files, they should retain the ADMIN userID, however what you seem to be saying is that your users are importing entries and they are assuming the userID of the ADMIN user (which shouldn't happen, they should retain the ID of the person who imported them). I suspect that what you are seeing may be a privilege issue. Have you checked the biblio settings on "admin/user/permissions"?

As for linking userID's with biblio authors, if you go to the biblio/authors page (there should be a menu item called "Authors") and then click the "edit" link beside one of the authors, you will find a select box which will list all the current Drupal users, just select the user who matches the Author that you are editing.

Ron.

Comment #6

pearson.gn commented 23 January 2009 at 18:46

Hi Ron,

Sorry, I botched that last comment as I was using rc2 rather than dev at the time. Suffice to say, with rc2 my users found that all imports came up as admin-owned nodes and it didn't seem to be a permissions problem. I've switched to the dev version though and I see that now it sets the userID as expected on the import page and it greys out the option to change the value. Ideal.

Thanks for the clarification of Gustav's concern. That feature does seem a bit redundant.

Again, apologies for the version mix-up.
Graham

Comment #7

rjerome commented 23 January 2009 at 20:49

Not a problem, yes that was a little bug in RC2 and previous versions.

You have in fact raised another valid point though (which I had forgotten about) and it sort of answers Gustav's question as to why the admin would want to set the ownership to some other user, and the answer is... as you have discovered, so you can edit entries which you may be the author of, but did not actually enter into biblio in the first place.

I'm sure every site is going to have different requirements for this, so it's hard to please everyone. Given that it's only the admin user (UID == 1) who will have the ability to set the ownership to some other UID, I think we have to trust that he/she knows what they are doing and why they are doing it.