DiscussionsTable: display error if there is no comment

korelstar

I've set the discussion layout to table layout. Hence, the list of discussions in categories (/categories/XYZ), "my discussions" (/discussions/mine) and tags (/discussions/tagged/plateau) has a column for the newest comment in the respective discussion. However, there is a problem if there is no comment for a discussion.

For categories, the discussion itself is used and hence it's author with avatar and date is shown. Unfortunately, in "my discussions" and tags, there is a display error: While the date is shown as in the first case, the author is missing and the avatar image is broken (see screenshot).

I'm using the bootstrap theme, but as far as I can see, it doesn't override the table layout. So I would assume that there is a bug in vanilla 2.2. Can someone approve this?

hgtonight

What is the src attribute on the img tag that is broken?

Right click and inspect it to figure it out.

korelstar

You're right, I should have investigated this in more detail. I'm sorry for that, because I've found a problem on my side (there was a problem with vanillicon). Now the situation has changed a little bit:

The author's name for the newest comment is still not shown, but an avatar is now shown. However, this avatar is not correct! I've debugged a little bit and found out:

• although the discussion's author has an individual avatar image set, it is not loaded.
• therefore, vanillicon is used
• in the vanillicon plugin, the hash is calculated based on the UserID and not using the Email

It looks like there is some data missing (i.e. the author's name, it's avatar and it's email).

hgtonight

Try running the counts at http://forum.example.com/dba/counts and see if that fixes it.

korelstar

No, that doesn't fix it.

By the way: It's not clear to me, what behavior is intended by vanilla:
a) show the discussion's data as newest comment
b) show nothing but the time of the discussion as newest comment.

Variant a) is reality for category views and b) is reality for tags etc. if vanillicon is disabled. However, the layout looks weird then.

korelstar

I've debugged this issue and came to a solution. Is this the right place to discuss such issues on this coding level or should I write an issue on Github for that?

Debugging the Problem

The table layout is realized in applications/vanilla/views/discussions/table_functions.php in the function writeDiscussionRow(...). In line 49, there is a special case for discussions without comments:
if ($Discussion->LastUserID) $Last = UserBuilder($Discussion, 'Last'); else { $Last = $First; }

However, in some cases $Discussion->LastUserID is set, even there is no comment and even the row LastUserID in the database is set to null. This is due to the function DiscussionModel->Calculate(...) which has also a special case for discussions without comments (applications/vanilla/models/class.discussionmodel.php line 719):
if ($Discussion->LastUserID == null) { $Discussion->LastUserID = $Discussion->InsertUserID; $Discussion->LastDate = $Discussion->DateInserted; }

As you can see, only LastUserID and LastDate is set, and not also required attributes Name, Photo and Email. However, these attributes are set by calling Gdn::userModel()->joinUsers(...).

Strangely, the functions Calculate (typically indirect by calling DiscussionModel->addDiscussionColumns()) and joinUsers is not always called in the same order:

• CategoriesController->index(...) calls DiscussionModel->getWhere(...) which calls Calculate at first and therefore provides correct results.
• DiscussionsController->mine(...) as well as TaggingPlugin->discussionsController_Tagged_create(...) call DiscussionModel->get(...) which calls joinUsers before calling calculate and therefore provide erroneous results.

Suggestions for Bugfixes

There are several ways for fixing this issue. Since I'm relatively new to the code of vanilla, I can't estimate the side effects of a bugfix. Therefore, I'm listing all possibilities I can spot. However, one solution is sufficient.

a) Fixing DiscussionModel->get(...)
applications/vanilla/models/class.discussionmodel.php line 316:
move this line
Gdn::userModel()->joinUsers($Data, array('FirstUserID', 'LastUserID'));
5 lines downwards after the line
$this->AddDiscussionColumns($Data);
Now, the user data is gathered also for the (fake) last user ID. However, this change of function call might have also to be done in other situations, e.g. in DiscussionModel->getID(...).

b) Fixing DiscussionModel->Calculate(...)
applications/vanilla/models/class.discussionmodel.php line 719
Remove the following lines:
if ($Discussion->LastUserID == null) { $Discussion->LastUserID = $Discussion->InsertUserID; $Discussion->LastDate = $Discussion->DateInserted; } Now, there is only one source for the special case for discussions without comments (applications/vanilla/views/discussions/table_functions.php line 49). However, this could be problematic for other layouts than table layout. But I didn't checked this.

I would be happy, if someone integrates a bugfix into the vanilla code

Linc

Thanks!