Please upgrade here. These earlier versions are no longer being updated and have security issues.
HackerOne users: Testing against this community violates our program's Terms of Service and will result in your bounty being denied.
Options

DiscussionsTable: display error if there is no comment

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?

Comments

  • Options
    hgtonighthgtonight ∞ · New MVP

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

    Right click and inspect it to figure it out.

    Search first

    Check out the Documentation! We are always looking for new content and pull requests.

    Click on insightful, awesome, and funny reactions to thank community volunteers for their valuable posts.

  • Options

    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).

  • Options
    hgtonighthgtonight ∞ · New MVP

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

    Search first

    Check out the Documentation! We are always looking for new content and pull requests.

    Click on insightful, awesome, and funny reactions to thank community volunteers for their valuable posts.

  • Options

    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.

  • Options

    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 :awesome:

  • Options
    LincLinc Detroit Admin

    Thanks!

Sign In or Register to comment.