Closed (fixed)
Project:
Drupal core
Version:
6.x-dev
Component:
node system
Priority:
Normal
Category:
Feature request
Assigned:
Reporter:
Created:
31 Jan 2007 at 19:23 UTC
Updated:
4 Jun 2007 at 21:23 UTC
Jump to comment: Most recent file
Comments
Comment #1
killes@www.drop.org commentedlooks good.
Comment #2
dwwi'm not sure revision_uid is much help on its own. most of the time, you need both uid and name, and then you just want to hand it to theme('username'), which you still couldn't do...
Comment #3
moshe weitzman commentedit is a big help. the caller has a key and can go get name or go get the whole $account object. i won't do another JOIN just to get the name of revision author
Comment #4
dwwfair enough. i hereby remove my reservations.
tested against 5.x (since i fear setting up a HEAD test site for this). ;) works great. code is (obviously) clean and proper. i tried without diff.module enabled. also tried with diff enabled, and my recent work around for this bug (lack of keys on the node_revision_list() array) commented out... all is well. also, just to be paranoid, i greped core for node_revisions_list(), and only the revisions tab uses the function, and it (obviously) isn't confused by having meaningful keys. RTBC.
i consider the lack of keys on node_revision_list() a bug, but the revision_uid thing a feature, so not sure what to do with the category (and therefore prospects for backporting)... so, i'll leave that to moshe and others to fight over. ;)
Comment #5
dries commentedI'd say that in 99% of the case this information in not required/useful. Why would we bother to load if for every node_load()? In the rare case the information is needed, you could do an additional query? Not sure about this.
Comment #6
moshe weitzman commentedthats true. to answer the question, we would bother because there is virtually zero cost ... if the committers prefer, we can delete the hunk about node_load() and take the node_revision_list() fix.
Comment #7
dwwbump: code freeze approacheth... is this going to make it in?
however, old patch no longer applies. here's a version with *just* the keys on $revisions[], which is the only really important change from this patch IMHO (and if Dries doesn't want revision_uid, he can just commit this one).
new patch for both parts that applies to today's HEAD coming up next...
Comment #8
dwwhere's a patch that includes the revisions keys and the revision_uid in node_load(). i agree, there's virtually zero cost -- we're already joining {node_revisions} in this query. however, in some cases, you really need this info, and it sucks to force modules to redo another entire query just for this other field.
Comment #9
moshe weitzman commentedRTBC. Committers - please choose one of the last two patches as you see fit.
Comment #10
dries commentedCommitted to CVS HEAD. Thanks.
Comment #11
dwwthanks, dries. @moshe, FYI: he committed #7, not #8. oh well... ;)
FYI: short blurb added to upgrade docs: http://drupal.org/node/114774#node-revision-list
Comment #12
drummI don't think I will backport this since it changes the return value of an API function and the key is not essential since the vid is in each object.
Comment #13
(not verified) commented