Banner advert rotation

[dualzNZ]

Hi all
I have recently watched a tutorial series from PHPACADEMY called the advert rotation script series.
I have found a small problem, the problem is that if any of the adverts have expired then the rest will not show.

does anyone have any solutions for this issue, that if 1 or more have expired the active adverts will still run.

Code:
ADS main code

<?php 
include 'db.inc.php';

$ads = mysql_query("Select `advert_id`, `image` FROM `ads` WHERE UNIX_TIMESTAMP() < `expires` AND `shown`=0 ORDER BY `advert_id` ASC LIMIT 1");
while ($ads_row = mysql_fetch_assoc($ads)) {
    $advert_id = $ads_row['advert_id'];
    $image = $ads_row['image'];

    echo '<a href="go.php?advert_id='.$advert_id.'" target="_blank"><img src="'.$image.'" /></a>';

    mysql_query("UPDATE `adverts` SET `shown`=1, `imprssions`=`impressions`+1 WHERE `advert_id`=$advert_id");

    $shown = mysql_query("SELECT COUNT(`advert_id`) FROM `ads` WHERE `shown`=0");
    if (mysql_result($shown, 0) == 0) {
        mysql_query("UPDATE `adverts` SET `shown`=0");
    }
}

?>

the db.inc.php is just the database connection file.

DB Structure:
advert_id int(11) primary auto incrament
image varchar 255
url varchar 255
impressions int(11)
clicks int(11)
expires int(11)
shown int(1)

thanks

[kkeith29]

There is a typo in the update query inside the while loop. imprssions should be impressions.

[dualzNZ]

ahh yes i found that, na its still doing it

[kkeith29]

Did you change the table name to 'ads' in that query? You have 'adverts' in the code you posted.

[dualzNZ]

yes i did, its on the top query, for some reason it just stops when any of the adverts expires from UNIX_TIMESTAMP()

[dualzNZ]

i also tried splitting each id into a key, but this did not fix the issue either

[kkeith29]

This this:

<?php

include 'db.inc.php';

$ads_query = mysql_query("SELECT `advert_id`,`image` FROM `ads` WHERE UNIX_TIMESTAMP() < `expires` AND `shown` = 0 ORDER BY `advert_id` ASC LIMIT 1") or die(mysql_error());

while( list( $ad_id,$image ) = mysql_fetch_row( $ads_query ) ) {
    echo '<a href="go.php?advert_id='.$ad_id.'" target="_blank"><img src="'.$image.'" /></a>';
    mysql_query("UPDATE `ads` SET `shown` = 1,`impressions` = `impressions` + 1 WHERE `advert_id` = {$ad_id}") or die(mysql_error());
    $shown = mysql_query("SELECT COUNT(`advert_id`) FROM `ads` WHERE `shown` = 0") or die(mysql_error());
    list( $count ) = mysql_fetch_row( $shown );
    if ( (int) $count === 0 ) {
        mysql_query("UPDATE `ads` SET `shown` = 0") or die(mysql_error());
    }
}

?>

I don't see anything that would cause the problem you are describing.

[kkeith29]

<?php

include 'db.inc.php';

$ads_query = mysql_query("SELECT `advert_id`,`image` FROM `ads` WHERE UNIX_TIMESTAMP() < `expires` AND `shown` = 0 ORDER BY `advert_id` ASC LIMIT 1") or die(mysql_error());
list( $ad_id,$image ) = mysql_fetch_row( $ads_query );
echo '<a href="go.php?advert_id='.$ad_id.'" target="_blank"><img src="'.$image.'" /></a>';
mysql_query("UPDATE `ads` SET `shown` = 1,`impressions` = `impressions` + 1 WHERE `advert_id` = {$ad_id}") or die(mysql_error());
$shown = mysql_query("SELECT COUNT(`advert_id`) FROM `ads` WHERE `shown` = 0") or die(mysql_error());
list( $count ) = mysql_fetch_row( $shown );
if ( (int) $count === 0 ) {
    mysql_query("UPDATE `ads` SET `shown` = 0") or die(mysql_error());
}

?>

This is better. I finally saw that the while loop was unnecessary.

[dualzNZ]

i copied your code and i get
You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 1

but i cant see any error that would cause that effect

[kkeith29]

Debug it and find which sql statement is broken. I am not sure which query that came from.

[dualzNZ]

ok fixed that, just testing it now.

ok once one of the rows have expired the error of
You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 1
comes up.

but when both rows have not expired they appear as normal per refresh

i also think that the count has something to do with it becase when each refrsh happens the shown goes from 0 to 1
1 = been shown on the page so it moves to the next 0 in the table

go.php

<?php
include "adverts/db.inc.php";

if (isset($_GET['advert_id'])) {
    $advert_id = (int)$_GET['advert_id'];

    mysql_query("UPDATE ads SET clicks=clicks+1 WHERE advert_id='$advert_id'"); 

    $url_query = mysql_query("SELECT url FROM ads WHERE advert_id='$advert_id'");
    $url = mysql_result($url_query, 0, 'url');

    header('Location: '.$url);
    die();
}

?>

then at the end of all rows being 1 on shown it resets them to 0 then stats the proceess again!

[kkeith29]

Oh, I feel stupid. On the count query you need to make sure it is only counting records that are not expired. Just add the unix timestamp check to that query and your problem will be fixed.

You also need to add an if statement to check if the first query returns any results. If it returns 0 results you get the sql error.

[dualzNZ]

ok

i have added the unix to the count query where do i add the if ?

$ads_query = mysql_query("SELECT `advert_id`, `image` FROM `ads` WHERE UNIX_TIMESTAMP() < `expires` AND `shown` = 0 ORDER BY `advert_id` ASC LIMIT 1") or die(mysql_error());
list($advert_id,$image) = mysql_fetch_row($ads_query);
// do i add if here ??
echo '<a href="go.php?advert_id='.$advert_id.'" target="_blank"><img src="'.$image.'" /></a>';
mysql_query("UPDATE `ads` SET `shown` = 1,`impressions` = `impressions` + 1 WHERE `advert_id` = {$advert_id}") or die(mysql_error());
$shown = mysql_query("SELECT COUNT(`advert_id`) FROM `ads` WHERE UNIX_TIMESTAMP < `expires` AND `shown` = 0") or die(mysql_error());
list($count) = mysql_fetch_row($shown);
if ((int)$count === 0) {
    mysql_query("UPDATE `ads` SET `shown` = 0") or die(mysql_error());
}

[kkeith29]

You would add it right after the first query:

$ads_query = mysql_query("SELECT `advert_id`, `image` FROM `ads` WHERE UNIX_TIMESTAMP() < `expires` AND `shown` = 0 ORDER BY `advert_id` ASC LIMIT 1") or die(mysql_error());
if ( mysql_num_rows( $ads_query ) == 1 ) { //if a result is returned show image
    //code to display image here
}
else { //no ads found
    //something here maybe
}

[dualzNZ]

that got rid of the error, but no images who on the screen now, i still have 1 of the rows as a expired unix timestamp.

[kkeith29]

Reset all the information in your table to fresh data and try again.

I have setup my own version on my local server and it worked without any problems.

[dualzNZ]

ok so if any of the rows in the tabel expired the others still showed?

[kkeith29]

Yes. I tested that and it worked fine.

[dualzNZ]

you my frend are a bloody legend, it works prefectly now.

thanks for all ya help