...

View Full Version : Class repeating an action



tomharto
09-13-2011, 11:58 PM
I made a class to handle banning users from my site by IP


class ban
{
public $yourID = "";
public $yourLevel = "";
public $yourHigher = false;
public $theirIP = "";

function __construct($yourID)
{
$this->yourID = $yourID;
$this->yourLevel = $_SESSION['adminLevel'];
}

function yourAboveThem($theirID)
{
$this->theirID = $theirID;

$Sql2 = "SELECT member_level_int, IP FROM members WHERE user_id ='".$theirID."'";
$Res2 = mysql_query($Sql2);
if ((mysql_num_rows($Res2)) > 0)
{
$row2 = mysql_fetch_assoc($Res2);
}
$theirLevel = $row2['member_level_int'];
$this->theirIP = $row2['IP'];

if ($this->yourLevel > $theirLevel)
{
$this->yourHigher = true;
}
}

function banThem($theirID, $time, $reason)
{

if ($this->yourHigher == true)
{
$now = time();
if ($time < $now)
{
if (($this->yourLevel == "4") || ($this->yourLevel == "6") || ($this->yourLevel == "7") || ($this->yourLevel == "8") || ($this->yourLevel == "9") || ($this->yourLevel == "10") || ($this->yourLevel == "11"))
{

die("Sorry, you dont have permission to perminetly ban people ($time - $now)");
}
else
{
$time = "0";
}
}

$reason = mysql_real_escape_string($reason);
$Sql3 = "INSERT INTO phpbb_banlist
(ban_id, ban_userid, ban_ip, ban_email, ban_start, ban_end, ban_exclude, ban_reason, ban_give_reason)
VALUES
(NULL, '$theirID', '".$this->theirIP."', '', '$now', '$time', '', '$reason', '".$this->yourID."')";
$Res3 = mysql_query($Sql3);
die ("User banned");


}
else
{
die ("Their a higher level than you, you cant ban them");
}
}
}
?>

But for some reason if i ban a users e.g. a higher level it says i cant (which is correct) but then if i ban a user a lower level it runs the sql query twice. (Likewise if i get 2 messages about e.g. user level then run a ban i can do it inserts it 3 times)

Can anyone see why?

Also i imagine theres many issues/room for improvment i could do to that script so feel free to suggest anything :)

EDIT: Forgot to put, this is how its called
<?php
session_start();
error_reporting (E_ALL ^ E_NOTICE);
include ("../../include/config.php");
include ("../includes/function_banning.php");
$ban = new ban($_SESSION['adminUserID']);


if ($_POST['action'] == "sb")
{
$theirID = mysql_real_escape_string($_POST['ID']);
$reason = "You have been temporary banned from EG because you are not mature enough to play within our community. To dispute you ban, please contact the site director (email address)";
$time = mktime(0,0,0,date("m"),date("d")+1,date("Y"));
$ban->yourAboveThem($theirID);
$ban->banThem($theirID, $time, $reason);

}
else
{
if ((!isset($_POST['ID'])) || (!isset($_POST['time'])))
{
echo "Error 0142";
die();
}
$theirID = mysql_real_escape_string($_POST['ID']);
$reason = mysql_real_escape_string($_POST['reason']);
$time = strtotime($_POST['time']);
$ban->yourAboveThem($theirID);
$ban->banThem($theirID, $time, $reason);
}

?>

BluePanther
09-14-2011, 12:59 AM
Well, from you class alone there are no repeats or loops causing multiple inserts. So, on that basis it must be the code calling the method that's being repeated. Can we see that code?

Also, like you said, a little critique on your code :). Your admin levels are being stored as strings, but they look like integer values. So, why not have them as integers. Makes permission comparisons much easier.
For example, your current code where you compare admin levels for instant bans goes through several or statements. If they were integers, you could speed it up greatly, and make it easier to read, by saying:


if($this->yourLevel <= 12){
die('Not authorised');
else{
// do stuff
}

Also, it would be a good idea to put in some error handling for your queries. Could look into raising exceptions, if you're bold :P or just having a simple mysql_query() or die() statement.

tomharto
09-14-2011, 01:02 AM
I would have done that if but if you see theres no 5, it goes 4, 6 etc. Ive posted my code calling it, and the page that is on is ran using jQuery AJAX (onclick on a link).

BluePanther
09-14-2011, 01:05 AM
I can't see anything obvious (to me anyway) in the ban script. Try running it without ajax and see what happens. I have a feeling the problem lies there

tomharto
09-14-2011, 01:07 AM
Okay ill try that, this is the jQuery, the links href is just a number and the class is ban


$(".ban").click(function(e){
e.preventDefault();
var ID = $(this).attr("href");
$.ajax({
type: "POST",
url: "_aGadminCp/modules/banMember.php",
data: "action=sb&ID="+ID,
success: function(msg){
popUp(msg);
}
});
});

But ill try it without see what happens

BluePanther
09-14-2011, 01:16 AM
This may sound silly, but are you double clicking on the link? My javascript knowledge is limited, but I think I'm right when I say double clicking on the link with id #ban will run the ban function twice.

tomharto
09-14-2011, 01:17 AM
I thought that too, i made sure i wasnt, and i tried it clicking a link what wouldnt let me ban twice, then ban someone, inserted 3 rows, click someone i couldnt ban 5 times, then banned someone, it inserted 6 rows, it seems to hold the queries until it gets the okay then inserts them all, very strange :S

BluePanther
09-14-2011, 01:27 AM
How did trying your script without ajax go?

Also, can we see the actual SQL stuff? I notice there's no connection in your ban class, that might be the problem. Let me see how the sql is actually connected.

tomharto
09-14-2011, 06:46 PM
Sorry bout the slow reply, only just been able to try it without AJAX, without ajax it works okay and only inserts one no matter how many times it says cant ban because of x reason.

But i think i have an idea why.

Everytime i clikc the link to ban someone it runs this line


$ban = new ban($_SESSION['adminUserID']);

So maybe is it making multiple instances? Is there a way i can make it destroy the intance of $ban if the class throws and error saying cant ban because of x reason.?

BluePanther
09-14-2011, 08:30 PM
once your done, you can always use unset(). I don't think that's needed though, if it's working without AJAX. I would say your ajax is at fault somehow. Dunno, needs a second opinion.

tomharto
09-14-2011, 08:32 PM
Okay, well all i can think is for some reason the AJAX is getting triggered extra times, but i cant figure out why or how it would be, ill go posti n jquery/JS forum see if they see anything wrong there. Thanks for your help :)

BluePanther
09-14-2011, 08:56 PM
No problem :)



EZ Archive Ads Plugin for vBulletin Copyright 2006 Computer Help Forum