Duplicate avatars, wrong total count, trolls not hidden (Vanilla 2.1.11)

ozonorojo

Thanks for this plugin. I have been testing it a bit under Vanilla 2.1.11 and found the following problems:

The total count of discussants shown appears to be based on the number of different avatar picture files, so all discussants sharing the same picture (such as the default avatar) are only counted once. Displaying the same picture only once makes sense, but the count should probably include everyone.

The avatar of the original poster will appear repeated if he makes a comment in his own discussion, provided someone else has commented before him. This repeating is kind of confusing, making it seem as if they are different users.

When using the plugin Troll Manager (from the master branch of Vanilla source on GitHub), trolls are not hidden from the list of avatars (in fact, they are highlighted with the word "Troll". It's buggy but funny ), nor from the total number of discussants.

R_J

Thanks for your feedback! I'll try to take a look at that plugin, but I guess, I will skip testing it for 2.1 and only see if it works with 2.2 (which doesn't mean that it will not work with 2.1, I simply will not test that). But I'm not sure when I find the time for that. It might became mid of July...

ozonorojo

If you find time to fix those problems for Vanilla 2.2 that's good enough. No need to waste time testing old versions. I will update to the newer version once it is stable anyway.

R_J

Well, that has been fun and the result is all shiny and new => http://vanillaforums.org/addon/discussants-plugin

Thanks again for finding the error and for giving me a reason to look at the plugin which has been one of my first works.

ozonorojo

Wow, that was efficient!

I have tested it in Vanilla 2.1 and I can confirm the first two problems I reported above are fixed. Total count of discussants is correct now and avatars don't appear repeated. There is also that nice new sliding effect.

Avatars of users marked as trolls still appear (regardless of roles) in my version.

Thanks for the quick update.

R_J

Before the foreach in line 401 add:
$sessionUserID = Gdn::session()->UserID;

And before line 411

// Don't echo avatar if that user is a troll and it is not he himself.
if ($users[$key]['Troll'] && $key != $sessionUserID) {
    continue;
}

I guess that should do the trick, though I'm not sure. At least it shouldn't break anything

ozonorojo

Can I have a pepperoni pizza too?

I added the second part at the beginning of the foreach, to skip the iteration altogether for trolls.

It's all working nice. Great work.

R_J

That could break the markup for discussions with more than 22 participants. There is a check in the loop: "is this the 20th participant and are there more than 22? if yes add a sub list". After the loop, that check is repeated and the list is closed.
So if the troll is the 20th user in the list, the sublist wouldn't opened, but closed.

I must admit that this might not happen too often, but it could

ozonorojo

Totally missed that point. Thanks again, I just moved it to the correct place as you first suggested.

ozonorojo

When we thought we were done with this...

I just noticed the little fix to hide trolls also hides them from moderators, which is a bit inconsistent, as they can see discussions and comments from trolls.

I fixed it with an extra condition on the last piece of code you wrote:

// Don't echo avatar if that user is a troll and it is not he himself.
if ($users[$key]['Troll'] && $key != $sessionUserID &&
    !Gdn::Session()->CheckPermission('Garden.Moderation.Manage')) {
    continue;
}

R_J

I guess it would even be a tiny bit better to do the check outside the loop and store the result in a variable

ozonorojo

Yes, I just noticed what I did up there. I'm such a brute.

$sessionUserID = Gdn::session()->UserID;
$isModerator = Gdn::Session()->CheckPermission('Garden.Moderation.Manage');

foreach ($discussants[1] as $key => $value) {
    // Add class that reflects the percentaged participation.
    $cssClass = 'Discussants'.intval($value/10);

    $discussantsCounter += 1;
    // Start sub group if more than max participants.
    if (($discussantsCounter == $maxVisible) && $maxVisibleLimit) {
        echo '<li><ul class="HiddenDiscussantsContainer HoverHelp">';
    }
    // Don't echo avatar if that user is a troll and it is not he himself.
    if ($users[$key]['Troll'] && $key != $sessionUserID && !$isModerator) {
        continue;
    }
    // Create list entry with avatar and "username [count]" title.
    echo '<li class="',$cssClass,'">',userPhoto($users[$key], array('Title' => $users[$key]['Name'].' ['.$discussants[0][$key].']')),'</li>';
}