The code in user_node_load() function assumes the user having uid = $node->uid exists. Warnings are generated if such user is missing in 'users' table. I suggest to check user existence:
function user_node_load($nodes, $types) {
// Build an array of all uids for node authors, keyed by nid.
$uids = array();
foreach ($nodes as $nid => $node) {
$uids[$nid] = $node->uid;
}
// Fetch name, picture, and data for these users.
$user_fields = db_query("SELECT uid, name, picture, data FROM {users} WHERE uid IN (:uids)", array(':uids' => $uids))->fetchAllAssoc('uid');
// Add these values back into the node objects.
foreach ($uids as $nid => $uid) {
$user_exists = isset($user_fields[$uid]); // <-
$nodes[$nid]->name = $user_exists ? $user_fields[$uid]->name : ''; // <-
$nodes[$nid]->picture = $user_exists ? $user_fields[$uid]->picture : 0; // <-
$nodes[$nid]->data = $user_exists ? $user_fields[$uid]->data : NULL; // <-
}
}
| Comment | File | Size | Author |
|---|---|---|---|
| #18 | user-node-load-patch-2653342-18.patch | 760 bytes | collinhaines |
Comments
Comment #2
maximpodorov commentedComment #4
maximpodorov commentedComment #5
maximpodorov commentedSo the patch passes tests now.
Comment #6
arun.kumar09 commentedHi,
Tested the patch at my end and works fine.
Comment #7
Revathi Manohar commentedhi, i have tested and it is working fine
Comment #8
mstrelan commentedAlternative patch with only one conditional per loop instead of four. Should we be setting the name to
variable_get('anonymous', t('Anonymous'))instead of''?Comment #9
bkosborneWorks for me and makes sense.
Comment #10
David_Rothstein commentedWhy would the users be missing given that https://api.drupal.org/api/drupal/modules%21node%21node.module/function/... removes a user's content when it is deleted?
If the issue is older sites (e.g. Drupal 6 or earlier) that already had this problem when they upgraded to Drupal 7, maybe we should think about fixing this via an update function instead - i.e. explicitly assign the orphaned nodes to the anonymous user or something like that.
Comment #11
bkosborneIt's a valid point. Somehow things got into a weird state where the node author was not updated for whatever reason. In my particular case it was not due to a migration path.
Comment #12
tamarpe commentedHad those warnings as well , #8 works +1
Comment #13
collinhaines commentedRe-rolling for line number changes in
modules/user/user.modulewith Drupal 7.71.Comment #14
marcelovaniI found the same issue because of deleted users. This patch works.
Comment #15
marcelovaniComment #16
collinhaines commentedRe-rolling for line number changes in
modules/user/user.modulewith Drupal 7.76. Also reverted coalesce operator from previous patch since Drupal 7.x still officially supports PHP 5.x.Comment #17
marcelovaniAlternatively
Comment #18
collinhaines commentedRe-rolling for line number changes in
modules/user/user.modulewith Drupal 7.90.Comment #20
avpadernoComment #21
klausiLatest patch is still good, back to RTBC.
Comment #22
poker10 commentedI think there are few important comments here, like #8 and #10, so let's discuss this a bit more. This seems to be caused by inconsistent database as normally it should not be possible to delete a user while the UID is left in the node table.
This patch seems like an easy workaround, but it can probably hide more problems - there can be other parts in code where the existence of that user is expected. I think the cleaner solution will be something like David proposed in #10, for example to update all nodes where UID is not in users table to 0 (anonymous user). However I can imagine this can have a pretty big impact on some sites with lot of content referencing non-existing users. On the other hand, I would not expect to have an empty username in any case (I would expect either Anonymous, or user name). So moving this to Needs Review for some additional discussion. Thanks.