Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
User name is correct, but the link points to the profile of the last reply author.
I can't understand why needs this line in template_preprocess_forum_topic_list()
$topic->uid = $topic->last_comment_uid ? $topic->last_comment_uid : $topic->uid;
Comment | File | Size | Author |
---|---|---|---|
#40 | forum_1078646_0.patch | 721 bytes | droplet |
#28 | forum-topic.jpg | 282.17 KB | droplet |
#25 | topic_user.jpg | 166.48 KB | droplet |
#24 | wrong-link-in-forum-author-1078646-24.patch | 979 bytes | larowlan |
#22 | forum_1078646.patch | 721 bytes | droplet |
Comments
Comment #1
droplet CreditAttribution: droplet commentedintroduce in this patch:
http://drupal.org/files/issues/taxonomy_67.patch
wrong patch I think, quick fix
Comment #2
Chi CreditAttribution: Chi commentedI have applied the patch. It works fine.
Comment #3
Chi CreditAttribution: Chi commentedThe patch will be commited to HEAD?
Comment #4
catchComment #5
Dries CreditAttribution: Dries commentedI'd like another pair of eyes on this patch because I don't think it is correct.
Removing that line is likely to break the 'last reply' field, not?
Shouldn't it be:
Comment #6
sunYes. Also not major.
Comment #7
NnekaComment #8
Nnekaopening ticket. mistake to close. implementing the fix noted here.
Comment #9
NnekaComment #10
NnekaPreface this by saying a newbie to contributing and git.
Made the code change suggested in post #5, tested and everything seems to be working without fail. This attached patch was made for 7.x.
Comment #12
Vincent Giersch CreditAttribution: Vincent Giersch commentedYour patch was in a patch and there was an incorrect indentation.
Comment #13
droplet CreditAttribution: droplet commentedwhat's the diff by move that line ?
Comment #14
droplet CreditAttribution: droplet commented#5, #12 moving that lines would result a wrong topic->uid.
topic->uid should be same as TOPIC author, not the comment user.
#1 my patch seems more right.
@Dires,
last reply handled in other parts:
Comment #15
NnekaAfter looking a little deeper into the code, I would agree with #14.
I believe the only reason that line of code was added ($topic->uid = $topic->last_comment_uid ? $topic->last_comment_uid : $topic->uid;), was to override the Commented by (its being used when called the form_submitted theme to display the user submission info.).
I've tested with this line of code commented out and it works fine.
Including the fix for Drupal 7 with the code above removed.
Comment #17
NnekaIgnore the last patch. This one is correct.
Comment #18
droplet CreditAttribution: droplet commented@Nneka,
Please leave this at 8.x as we fix bugs first in 8.x and backport them as fast as possible.
Git patch:
http://drupal.org/node/707484
Sounds like you are going to do something like #1 patch ?
Comment #19
NnekaYes. This patch is the same #1.
Comment #20
NnekaThis ticket also exists as a duplicate. Closed http://drupal.org/node/1181240 as duplicate.
Comment #21
droplet CreditAttribution: droplet commentedHey,
Please don't change version again. Thanks!
Comment #22
droplet CreditAttribution: droplet commentedstill same as #1 but a -p1 style patch
both D7 & D8
Comment #23
larowlanThis line is needed when constructing the last reply theme output.
The problem is it needs to be set after the created variable is set.
Ie it doesn't need to be removed, it just needs to go after
and before
see http://api.drupal.org/api/drupal/modules--forum--forum-submitted.tpl.php... and http://api.drupal.org/api/drupal/modules--forum--forum.module/function/t... in the last reply variable ($variables['topics'][$id]->last_reply)) we need the 'by' link to point to the last_comment_uid.
Powered by Dreditor.
Comment #24
larowlanReroll with my comments
Comment #25
droplet CreditAttribution: droplet commented@larowlan,
It passed
$topic->last_reply
into theme functionand theme function access
$variables['topic']->uid
the result it will access:
see my attached image & try to test my patch #22 :)
EDIT:
my debug function is after this line
topic created with UID 1, so the attached image UID change to 2 which is wrong.
Comment #26
larowlanPlease see http://api.drupal.org/api/drupal/modules--forum--forum-topic-list.tpl.ph...
We need to set the $topic->uid to that of the last commenter to make the 'last reply' column work.
So there are two calls to theme('forum_submitted'
The first one to setup the 'created by' column, the second to setup the 'last reply' column.
Without your patch, the 'created by' column does not work.
With your patch, the 'last reply' column does not work.
I think my patch is the common ground where they both work.
Please advise.
Comment #27
droplet CreditAttribution: droplet commentedcan you capture a screenshot to show how it doesn't work & point out the line that used $topic->uid on
http://api.drupal.org/api/drupal/modules--forum--forum-topic-list.tpl.ph...
what's your drupal version ?
Comment #28
droplet CreditAttribution: droplet commentedmy turn. I don't see any problem
Comment #29
larowlan$variables['author'] in forum_submitted.tpl.php is set here using $topic->uid:
http://api.drupal.org/api/drupal/modules--forum--forum.module/function/t... and then output here
http://api.drupal.org/api/drupal/modules--forum--forum-submitted.tpl.php...
Comment #30
droplet CreditAttribution: droplet commented@larowlan,
are you tested with fresh installation ?? and on same issue ? I know you are the new forum maintainer, maybe working on a lot of forum issues and make comment on wrong issue ??
What here is fixing template_preprocess_forum_topic_list on forum topic list
but you are taking about forum_submitted on forum thread (node) page.
Anything I missed ??
Comment #31
larowlan@droplet
template_preprocess_forum_topic_list calls theme('forum_submitted' which in turn calls template_preprocess_forum_topic_list which in turn uses $topic->uid to set the author.
template_preprocess_forum_topic_lic calls theme('forum_submitted' twice, once for the creator and again for the last reply - in the first instance the author is the original $topic->uid, in the second instance it's the commenter hence before the second call needs $topic->uid set to that of the commenter.
Comment #32
droplet CreditAttribution: droplet commented@larowlan,
Listen, give me a chance :)
- First reinstall your Drupal-8-dev
- add new user
- create a forum topic & reply is with different user
then edit this function and add debug function
Compare what's the difference.
Because whatever you changed, it never pass into theme function (last_reply). It's like my patch (removed that line)
Notice: Patch #24 will get it work but if you try to theme your fourm list on forum-topic-list.tpl, you never know who is the topic creator.
try add following on forum-topic-list.tpl after you patched with #24
dsm($topics);
You will see UID is always equal to last reply user. (this UID should be the topic creator)
Hope you understand it, I have no idea to explain more in details.
Hope i'm not wrong so. :)
any other guys have chance to review it ??
Comment #33
larowlanYou're right, there is no difference between your patch and mind, the second call to theme('forum_submitted' passes $topic->last_reply (not $topic as I thought).
So yours is the better patch.
Comment #34
larowlan#22: forum_1078646.patch queued for re-testing.
Comment #35
larowlanThe patch in #22 is good to go, I had changed it at #24 based on Dries comments at #5 but as rightly pointed out by @droplet, there is no difference between the two. The last reply columns works without the offending line.
Comment #36
luke.oakes CreditAttribution: luke.oakes commentedSorry for my ignorance and if I've misunderstood... so this patch has been applied to drupal 8... but isn't backported to drupal 7 yet? What's the timeline for this happening?
Comment #37
larowlanHi Luke,
No the patch in #22 has not been applied anywhere yet.
You can apply it to Drupal 7.8 if you wish.
The patch should apply cleanly against both Drupal 7 and 8.
The patch should hopefully be committed before the next Drupal 7 point release (Drupal 7.9) which is due to come out at the end of the month.
Comment #38
luke.oakes CreditAttribution: luke.oakes commentedThanks for the clarification Larowlan :)
I'll delete the line from my 7.8, and look forward to seeing the implementation in 7.9
Comment #39
sunFor the world after us dinosaurs, can we clarify this comment on what "now" refers to and actually means?
Overall, I've the impression that we're relying on quite some bit of magic in that (overridable) theme function. If it gets overridden, the magic might be lost. Or not. Anyway, in the end, it looks like lots of non-obvious assumptions are being made here (unless I'm missing something obvious).
14 days to next Drupal core point release.
Comment #40
droplet CreditAttribution: droplet commented@sun, you're reviewed a wrong patch.
#22 is the right one.
(im just reupload it once.)
Comment #41
larowlanYeah, the patch at #40 is the right one (same as #22).
Comment #42
Dries CreditAttribution: Dries commentedCommitted to 7.x and 8.x. Thanks.