Posted by Chi on March 2, 2011 at 2:04pm
10 followers
| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | forum.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | needs backport to D7, Quick fix |
Issue Summary
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;
Comments
#1
introduce in this patch:
http://drupal.org/files/issues/taxonomy_67.patch
wrong patch I think, quick fix
#2
I have applied the patch. It works fine.
#3
The patch will be commited to HEAD?
#4
#5
I'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:
<?php$variables['topics'][$id]->created = theme('forum_submitted', array('topic' => $topic));
$topic->uid = $topic->last_comment_uid ? $topic->last_comment_uid : $topic->uid;
$variables['topics'][$id]->last_reply = theme('forum_submitted', array('topic' => isset($topic->last_reply) ? $topic->last_reply : NULL));
?>
#6
Yes. Also not major.
#7
#8
opening ticket. mistake to close. implementing the fix noted here.
#9
#10
Preface 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.
#11
The last submitted patch, forum-createduser-1078646.patch, failed testing.
#12
Your patch was in a patch and there was an incorrect indentation.
#13
what's the diff by move that line ?
#14
#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:
$variables['topics'][$id]->last_reply = theme('forum_submitted', array('topic' => isset($topic->last_reply) ? $topic->last_reply : NULL));#15
After 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.
#16
The last submitted patch, forum-createduser-1078646.patch, failed testing.
#17
Ignore the last patch. This one is correct.
#18
@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 ?
#19
Yes. This patch is the same #1.
#20
This ticket also exists as a duplicate. Closed http://drupal.org/node/1181240 as duplicate.
#21
Hey,
Please don't change version again. Thanks!
#22
still same as #1 but a -p1 style patch
both D7 & D8
#23
+++ b/modules/forum/forum.module@@ -1123,7 +1123,6 @@ function template_preprocess_forum_topic_list(&$variables) {
- $topic->uid = $topic->last_comment_uid ? $topic->last_comment_uid : $topic->uid;
This 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
$variables['topics'][$id]->created = theme('forum_submitted', array('topic' => $topic));and before
$variables['topics'][$id]->last_reply = theme('forum_submitted', array('topic' => isset($topic->last_reply) ? $topic->last_reply : NULL));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.
#24
Reroll with my comments
#25
@larowlan,
It passed
<?php$topic->last_reply
?>
and theme function access
<?php$variables['topic']->uid
?>
the result it will access:
<?php$topic->last_reply->uid
?>
see my attached image & try to test my patch #22 :)
EDIT:
my debug function is after this line
<?php$topic->uid = $topic->last_comment_uid ? $topic->last_comment_uid : $topic->uid;
dsm($topic);
?>
topic created with UID 1, so the attached image UID change to 2 which is wrong.
#26
Please 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.
#27
can 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 ?
#28
my turn. I don't see any problem
#29
$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...
#30
@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 ??
#31
@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.
#32
@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
<?phpfunction template_preprocess_forum_submitted(&$variables) {
dsm($variables['topic']);
$variables['author'] = isset($variables['topic']->uid) ? theme('username', array('account' => $variables['topic'])) : '';
$variables['time'] = isset($variables['topic']->created) ? format_interval(REQUEST_TIME - $variables['topic']->created) : '';
}
?>
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)
<?php
$variables['topics'][$id]->last_reply = theme('forum_submitted', array('topic' => isset($topic->last_reply) ? $topic->last_reply : NULL));
?>
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
<?phpdsm($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 ??
#33
You'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.
#34
#22: forum_1078646.patch queued for re-testing.
#35
The 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.
#36
Sorry 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?
#37
Hi 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.
#38
Thanks for the clarification Larowlan :)
I'll delete the line from my 7.8, and look forward to seeing the implementation in 7.9
#39
+++ b/modules/forum/forum.module@@ -1127,8 +1127,10 @@ function template_preprocess_forum_topic_list(&$variables) {
- $topic->uid = $topic->last_comment_uid ? $topic->last_comment_uid : $topic->uid;
$variables['topics'][$id]->created = theme('forum_submitted', array('topic' => $topic));
+ // Now we set the uid to correspond to the last comment so that username
+ // is used in the themed output.
+ $topic->uid = $topic->last_comment_uid ? $topic->last_comment_uid : $topic->uid;
For 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.
#40
@sun, you're reviewed a wrong patch.
#22 is the right one.
(im just reupload it once.)
#41
Yeah, the patch at #40 is the right one (same as #22).
#42
Committed to 7.x and 8.x. Thanks.
#43
Automatically closed -- issue fixed for 2 weeks with no activity.