MyBB Community Forums

Hi again.

Short description:

When warning a user with a predefined warning, the points, duration, admin comments etc. are stored correctly. But the warning type isn't. This leads to only e.g. "(+30 Points)" being displayed in user profiles at /warnings.php?uid=1234 instead of "The predefined reason for this warning (+30 Points)".

Details and potential bugfix:

Set up some warning types in ACP in "admin/index.php?module=config-warning". When issuing such a warning to a user directly (at the page "/warnings.php?action=warn&uid=1234") the points, admin comments and duration will be stored correctly. But the type of the warning will never reach the database.

There are two bugs in datahandlers/warnings.php

First: The method "validate_post()" does

if(!isset($warning['tid']))
{
  $warning['tid'] = $post['tid'];
}

This is wrong, because "tid" of warning isn't supposed to be a "thread id", it is supposed to be the warning "type id". This assignment simply shouldn't be made at all, the whole if-block quoted above can and should be removed.

Second: The final assignment of the data for insertion into the database doesn't respect or handle the actual warning "type".

In the function insert_warning() you will find:

$warning = &$this->data;
$this->write_warning_data = array(
"uid" => (int)$warning['uid'],
"tid" => (int)$warning['tid'],
"pid" => (int)$warning['pid'],
"title" => $db->escape_string($warning['title']),
"points" => (int)$warning['points'],
"dateline" => TIME_NOW,
"issuedby" => (int)$mybb->user['uid'],
"expires" => $db->escape_string($warning['expires']),
"expired" => 0,
"revokereason" => '',
"notes" => $db->escape_string($warning['notes'])
);

Here we can see that the database field "tid" (warning "type id") is assigned the value of "$warning['tid']", which (after fixing the first bug) would be empty anyway. But it should store the warning type, which is available as 'type' in the $warning object. Like this:

"tid" => (int)$warning['type'],

At this point 'type' should be either 'custom' or a number (the warning 'type id'). You may want to do instead some additional validation at this point or somewhere earlier (e.g. in validate_type() ) to ensure that, but it should already work out fine anyway thanks to the (int) cast at that line, converting "custom" to 0 and keeping numeric ids the way they are.

Thanks but this has already been reported (and possibly fixed):
http://community.mybb.com/thread-163080.html
https://github.com/mybb/mybb/pull/1658

I didn't see that one before. So I tested it and it does not fix the actual problem, just obscure it.

Here is a test setup I used with the latest version with that fix linked in your thread:

First, two warnings are defined



These warnings have the "tid" 15 and 16 respectively, as seen here in the database:



Now I attempt to give a warning for a post with post id 492 which happens to be in the thread with the thread id 16. So I enter the warning. I choose the smaller warning, which has warning type id 15 as seen before in the database:



The expected result would be, that the user has now been warned with 10 points for 1 month and the reason "This is the intended warning".

The actual outcome is this:





As you can see, the text is wrong, but everything else is correct. Where does the text come from? In the database, the warning looks like this:



As explained before, currently and even after the patch linked above, the field "tid" is filled with the thread id (in case of warnings for posts), not the warning type id, which is wrong. It doesn't even matter that we do set the correct warning text now additionally for every warning (thanks to the patch from above), because if there is a warning type that coincidentially matches the thread id, that warning text will be used instead.

I'll open a pull request with these changes on github for you.

Edit: Here it is: https://github.com/mybb/mybb/pull/1677