Volunteer Signup Report

Question

I was creating a Volunteer Signup Report report in Microsoft Excel yesterday for my job and I had a moment of inspiration. What if I turned it into a website and made it dynamic? The volunteers sign up, the data is logged in a MySQL database, and then the report is automatically updated for the managers to view.

Obviously a website like this is a big project and requires a lot of pages, so I needed a place to start. So I chose to script the report and get it working.

It's been years since I did serious coding in PHP, so a lot of my practices may be out of date. I'd like to run it by you for a thorough code review.

HTML output:

report.php:

<?

// PAGE PURPOSE: Generates a volunteer report for the specified company. Loads all of their events, and loads the volunteer shifts for each event.

if ( !defined('VIEWER') ) {
    http_response_code(403);
    die("403 Forbidden");
}

// TODO: script a login system, and pull the $company_id from the user's account instead of hard-coding it
$company_id = 1;

///// FUNCTIONS /////
/* TODO: Move some of the formatting stuff over to the templates. Examples from the Smarty manual:
    Name: {$name|capitalize}
    Addr: {$address|escape}
    Date: {$smarty.now|date_format:"%b %e, %Y"} */
function format_date($timestamp = 0) {
    if ( $timestamp == null || $timestamp == 0 )    {
        $timestamp = time();
    }
    return(date("D m/d/y", $timestamp));
}

function get_race_name($race_id) {
    $query = mysql_query("SELECT race_name FROM races WHERE race_id = $race_id;");
    $races = mysql_fetch_array($query);
    return($races['race_name']);
}

function get_days_until_event($race_id) {
    $query = mysql_query("SELECT race_date FROM races WHERE race_id = $race_id;");
    $races = mysql_fetch_array($query);
    return(floor((strtotime($races['race_date']) - time())/60/60/24));
}

function get_weeks_until_event($race_id) {
    $query = mysql_query("SELECT race_date FROM races WHERE race_id = $race_id;");
    $races = mysql_fetch_array($query);
    return(floor((strtotime($races['race_date']) - time())/60/60/24/7));
}

function get_percent_formatting($percent) {
    if ( $percent < 50 ) {
        return("id='red'");
    } else if ( $percent < 90 ) {
        return("id='yellow'");
    } else {
        return("id='green'");
    }
}

///// REPORT HEADER /////
$query = mysql_query("SELECT * FROM companies WHERE company_id = $company_id;");
$companies = mysql_fetch_array($query);
$smarty->assign('company_name', $companies['company_name']);
$smarty->assign('today', format_date());

///// EACH RACE /////
$races_t = array();
$query = mysql_query("SELECT DISTINCT race_id FROM shifts WHERE company_id = $company_id ORDER BY race_id ASC;");
while ( $races = mysql_fetch_array($query) ) {
    //// RACE HEADER /////
    $races['race_name'] = get_race_name($races['race_id']);

    ///// RACE ROWS (VOLUNTEER SHIFTS) /////
    // TODO: change Signed Up and Needed amounts from integer in SQL table to SQL COUNT(*) later on
    $shifts_t = array();
    $race_id = $races['race_id'];
    $query2 = mysql_query("SELECT * FROM shifts WHERE race_id = $race_id ORDER BY shift_date ASC;");
    while ( $shifts = mysql_fetch_array($query2) ) {
        $races['signed_up_total'] += $shifts['shift_enrolled'];
        $races['needed_total'] += $shifts['shift_needed'];

        $shifts['shift_date'] = format_date(strtotime($shifts['shift_date']));
        $shifts['percent'] = ($shifts['shift_needed'] == 0) ? 0 : round($shifts['shift_enrolled'] / $shifts['shift_needed'] * 100);
        $shifts['percent_formatting'] = get_percent_formatting($shifts['percent']);
        $shifts['percent'] .= "%";

        // TODO: Something right here is broken. Smarty prints the same shifts for every race. Currently debugging on smarty.net forums.
        $shifts_t[] = $shifts;
    }
    // TODO: Make a new array for the SMARTY vars and assign everything that is going to be passed to the template explicitly. Should help with code readability. Right now it is hard to tell what vars are going to be passed to the template, which could become an issue with sanitizing them, etc.
    $smarty->assign('shifts_t', $shifts_t);

    ///// RACE FOOTER /////
    // TODO: check the rounding behavior on the weeks_until_event and days_until_event calculations
    $races['weeks_until_event'] = get_weeks_until_event($races['race_id']);
    $races['days_until_event'] = get_days_until_event($races['race_id']);
    $races['percent_average'] = ($races['needed_total'] == 0) ? 0 : round($races['signed_up_total'] / $races['needed_total'] * 100);
    $races['percent_formatting'] = get_percent_formatting($races['percent_average']);
    $races['percent_average'] .= "%";

    $races_t[] = $races;
}
$smarty->assign('races_t', $races_t);

$smarty->display('report.html');

?>

HTML source:

<!DOCTYPE html>
<html lang='en-US'>

<head>
    <title>Acme Events, Inc. Volunteer Report</title>
    <style>
        body {
            font-family: Calibri;
            font-size: 11pt;
        }

        table {
            width: 600px;
            border-collapse: collapse;
            border: 2px solid black;
            white-space: nowrap;
        }

        td {
            border: 1px solid black;
        }

        #title {
            text-align: center;
            font-weight: bold;
            font-size: 16pt;
        }

        #subtitle {
            text-align: center;
            font-style: italic;
        }

        #firstrow {
            font-weight: bold;
            font-style: italic;
            text-decoration: underline;
            border-top: 2px solid black;
        }

        #shift {
            width: 36%;
        }

        #date {
            width: 16%;
        }

        #time {
            width: 17%;
            border-right: 2px solid black;
        }

        #signedup {
            width: 12%;
        }

        #needed {
            width: 10%;
        }

        #percent {
            width: 10%;
        }

        #bold_upper_border {
            border-top: 2px solid black;
        }

        #bold_right_border {
            border-right: 2px solid black;
        }

        #red {
            color: #9C0006;
            background-color: #FFC7CE;
        }

        #yellow {
            color: #9C6500;
            background-color: #FFEB9C;
        }

        #green {
            color: #006100;
            background-color: #C6EFCE;
        }
    </style>
</head>

<body>
    <table id='table'>
        <tr id='title'>
            <td>
                Acme Events, Inc.
            </td>
        </tr>
        <tr id='title'>
            <td>
                Volunteer Report
            </td>
        </tr>
        <tr id='subtitle'>
            <td>
                Generated on Wed 08/12/15
            </td>
        </tr>
    </table>



    <br />

    <table id='table'>
        <tr id='title'>
            <td colspan='6'>
                September Shuffle 5K
            </td>
        </tr>
        <tr id='firstrow'>
            <td id='shift'>
                Shift
            </td>
            <td id='date'>
                Date
            </td>
            <td id='time'>
                Time
            </td>
            <td id='signedup'>
                Signed Up
            </td>
            <td id='needed'>
                Needed
            </td>
            <td id='percent'>
                %
            </td>
        </tr>



        <tr>
            <td>
                Packet Pick Up 10AM
            </td>
            <td>
                Sat 09/12/15
            </td>
            <td id='bold_right_border'>
                10AM-1:30PM
            </td>
            <td>
                40
            </td>
            <td>
                40
            </td>
            <td id='green'>
                100%
            </td>
        </tr>



        <tr>
            <td>
                Packet Pick Up 1PM
            </td>
            <td>
                Sat 09/12/15
            </td>
            <td id='bold_right_border'>
                1PM-5:30PM
            </td>
            <td>
                40
            </td>
            <td>
                40
            </td>
            <td id='green'>
                100%
            </td>
        </tr>



        <tr>
            <td>
                Kid's Fun Run
            </td>
            <td>
                Sun 09/13/15
            </td>
            <td id='bold_right_border'>
                10AM-12PM
            </td>
            <td>
                15
            </td>
            <td>
                15
            </td>
            <td id='green'>
                100%
            </td>
        </tr>



        <tr>
            <td>
                Pre & Late Registration (Race Day)
            </td>
            <td>
                Sun 09/13/15
            </td>
            <td id='bold_right_border'>
                6AM-10:30AM
            </td>
            <td>
                40
            </td>
            <td>
                50
            </td>
            <td id='yellow'>
                80%
            </td>
        </tr>



        <tr>
            <td>
                Start & Finish Line Support
            </td>
            <td>
                Sun 09/13/15
            </td>
            <td id='bold_right_border'>
                6AM-11AM
            </td>
            <td>
                75
            </td>
            <td>
                75
            </td>
            <td id='green'>
                100%
            </td>
        </tr>



        <tr>
            <td>
                Water Station 1 (Inside Stadium)
            </td>
            <td>
                Sun 09/13/15
            </td>
            <td id='bold_right_border'>
                6AM-10AM
            </td>
            <td>
                41
            </td>
            <td>
                40
            </td>
            <td id='green'>
                103%
            </td>
        </tr>



        <tr>
            <td>
                Volunteer Check In (Race Day)
            </td>
            <td>
                Sun 09/13/15
            </td>
            <td id='bold_right_border'>
                6AM-10:30AM
            </td>
            <td>
                5
            </td>
            <td>
                5
            </td>
            <td id='green'>
                100%
            </td>
        </tr>



        <tr>
            <td>
                Water Station 2 (Outside Stadium)
            </td>
            <td>
                Sun 09/13/15
            </td>
            <td id='bold_right_border'>
                6AM-10AM
            </td>
            <td>
                25
            </td>
            <td>
                40
            </td>
            <td id='yellow'>
                63%
            </td>
        </tr>



        <tr id='bold_upper_border'>
            <td align='center'>
                4 Weeks Until Event
            </td>
            <td colspan='2' align='center' id='bold_right_border'>
                31 Days Until Event
            </td>
            <td>
                281
            </td>
            <td>
                305
            </td>
            <td id='green'>
                92%
            </td>
        </tr>
    </table>



    <br />

    <table id='table'>
        <tr id='title'>
            <td colspan='6'>
                November Turkey Trot 5K/10K
            </td>
        </tr>
        <tr id='firstrow'>
            <td id='shift'>
                Shift
            </td>
            <td id='date'>
                Date
            </td>
            <td id='time'>
                Time
            </td>
            <td id='signedup'>
                Signed Up
            </td>
            <td id='needed'>
                Needed
            </td>
            <td id='percent'>
                %
            </td>
        </tr>



        <tr>
            <td>
                Packet Pick Up - Pre/Late Reg
            </td>
            <td>
                Fri 11/06/15
            </td>
            <td id='bold_right_border'>
                3PM - 7:30PM
            </td>
            <td>
                16
            </td>
            <td>
                50
            </td>
            <td id='red'>
                32%
            </td>
        </tr>



        <tr>
            <td>
                Packet Pick Up - Registratio
            </td>
            <td>
                Fri 11/06/15
            </td>
            <td id='bold_right_border'>
                11AM - 3:30PM
            </td>
            <td>
                14
            </td>
            <td>
                50
            </td>
            <td id='red'>
                28%
            </td>
        </tr>



        <tr>
            <td>
                Start/Finish Line Area: Post Race F
            </td>
            <td>
                Sat 11/07/15
            </td>
            <td id='bold_right_border'>
                6AM - 10AM
            </td>
            <td>
                15
            </td>
            <td>
                50
            </td>
            <td id='red'>
                30%
            </td>
        </tr>



        <tr>
            <td>
                Pre/Late- Registration
            </td>
            <td>
                Sat 11/07/15
            </td>
            <td id='bold_right_border'>
                5AM - 8AM
            </td>
            <td>
                7
            </td>
            <td>
                50
            </td>
            <td id='red'>
                14%
            </td>
        </tr>



        <tr>
            <td>
                Volunteer Check-In
            </td>
            <td>
                Sat 11/07/15
            </td>
            <td id='bold_right_border'>
                5AM - 10AM
            </td>
            <td>
                5
            </td>
            <td>
                5
            </td>
            <td id='green'>
                100%
            </td>
        </tr>



        <tr>
            <td>
                Water Station 1
            </td>
            <td>
                Sat 11/07/15
            </td>
            <td id='bold_right_border'>
                6AM - 10AM
            </td>
            <td>
                1
            </td>
            <td>
                40
            </td>
            <td id='red'>
                3%
            </td>
        </tr>



        <tr>
            <td>
                Water Station 2
            </td>
            <td>
                Sat 11/07/15
            </td>
            <td id='bold_right_border'>
                6AM - 10AM
            </td>
            <td>
                1
            </td>
            <td>
                40
            </td>
            <td id='red'>
                3%
            </td>
        </tr>



        <tr id='bold_upper_border'>
            <td align='center'>
                12 Weeks Until Event
            </td>
            <td colspan='2' align='center' id='bold_right_border'>
                86 Days Until Event
            </td>
            <td>
                59
            </td>
            <td>
                285
            </td>
            <td id='red'>
                21%
            </td>
        </tr>
    </table>

report.html template file (Smarty):

<!DOCTYPE html>
<html lang="en-US">
    <head>
        <title>{$company_name} Volunteer Report</title>
        <style>
            body {
                font-family: Calibri;
                font-size: 11pt;
            } table {
                width: 600px;
                border-collapse: collapse;
                border: 2px solid black;
                white-space: nowrap;
            } td {
                border: 1px solid black;
            } #title {
                text-align: center;
                font-weight: bold;
                font-size: 16pt;
            } #subtitle {
                text-align: center;
                font-style: italic;
            } #firstrow {
                font-weight: bold;
                font-style: italic;
                text-decoration: underline;
                border-top: 2px solid black;
            } #shift {
                width: 36%;
            } #date {
                width: 16%;
            } #time {
                width: 17%;
                border-right: 2px solid black;
            } #signedup {
                width: 12%;
            } #needed {
                width: 10%;
            } #percent {
                width: 10%;
            } #bold_upper_border {
                border-top: 2px solid black;
            } #bold_right_border {
                border-right: 2px solid black;
            } #red {
                color: #9C0006;
                background-color: #FFC7CE;
            } #yellow {
                color: #9C6500;
                background-color: #FFEB9C;
            } #green {
                color: #006100;
                background-color: #C6EFCE;
            }
        </style>
    </head>

    <body>
        <table id="table">
            <tr id="title">
                <td>
                    {$company_name}
                </td>
            </tr>
            <tr id="title">
                <td>
                    Volunteer Report
                </td>
            </tr>
            <tr id="subtitle">
                <td>
                    Generated on {$today}
                </td>
            </tr>
        </table>

{foreach item=race from=$races_t}
        <br />

        <table id="table">
            <tr id="title">
                <td colspan="6">
                    {$race.race_name}
                </td>
            </tr>
            <tr id="firstrow">
                <td id="shift">
                    Shift
                </td>
                <td id="date">
                    Date
                </td>
                <td id="time">
                    Time
                </td>
                <td id="signedup">
                    Signed Up
                </td>
                <td id="needed">
                    Needed
                </td>
                <td id="percent">
                    %
                </td>
            </tr>

{foreach item=shift from=$shifts_t}
            <tr>
                <td>
                    {$shift.shift_name}
                </td>
                <td>
                    {$shift.shift_date}
                </td>
                <td id="bold_right_border">
                    {$shift.shift_time}
                </td>
                <td>
                    {$shift.shift_enrolled}
                </td>
                <td>
                    {$shift.shift_needed}
                </td>
                <td {$shift.percent_formatting}>
                    {$shift.percent}
                </td>
            </tr>
{/foreach}

            <tr id="bold_upper_border">
                <td align="center">
                    {$race.weeks_until_event} Weeks Until Event
                </td>
                <td colspan="2" align="center" id="bold_right_border">
                    {$race.days_until_event} Days Until Event
                </td>
                <td>
                    {$race.signed_up_total}
                </td>
                <td>
                    {$race.needed_total}
                </td>
                <td {$race.percent_formatting}>
                    {$race.percent_average}
                </td>
            </tr>
        </table>
{/foreach}
    </body>
</html>

index.php:

<?php

define('VIEWER', TRUE);

include('system/config.php');

// Temporary code. Tries to load a page specified by ?p= in the URL. If no ?p= is specified, it displays a clickable list of the pages. Haven't made an error handler for ?p='s that don't exist yet. Maybe I'll create an array and cross-reference to that later so I can throw a proper error message.
if(isset($_GET['p'])) {
    include('./pages/' . $_GET['p'] . '.php');
} else {
    $directory = './pages';
    $pages = scandir($directory);

    foreach($pages as $pagename) {
        if ( $pagename == "." || $pagename == ".." ) {
            continue;
        } else {
            $pagename = rtrim($pagename, ".php");
            $pagename_pretty = str_replace("_", " ", $pagename);
            $pagename_pretty = ucwords($pagename_pretty);
            print("

                <a href='index.php?p=$pagename'>$pagename_pretty</a><br />

            ");
        }
    }
}

?>

config.php:

<?php

if ( !defined('VIEWER') ) {
    http_response_code(403);
    die("403 Forbidden");
}

///// MYSQL /////
$database_name = "voluntee_volunteers";
$database_username = "voluntee_volunte";
$database_password = "sekretpasswordd"; // changed for security ;-)
mysql_connect("localhost", $database_username, $database_password);
mysql_select_db($database_name);

///// SMARTY /////
include('./smarty/Smarty.class.php');
$smarty = new Smarty();

$smarty->setTemplateDir('./skin');
$smarty->setCompileDir('./smarty/templates_c');
$smarty->setCacheDir('./smarty/cache');
$smarty->setConfigDir('./smarty/configs');

?>

In addition to whatever you recommend in your code reviews, I have the following questions:

1. Is it worth converting this to OOP? For example, making report.php a Report class? It could improve code readability, in exchange for needing to create the class and call a display method somewhere else when I want to load the page.

2. Is it worth converting from MySQL to MySQLi or PDO? Somebody on another forum suggested that. Is there a big learning curve to switch to MySQLi or PDO?

3. This is my first time coding with CSS / StyleSheets in a long time. How'd I do? Any suggestions for that? Perhaps splitting my <style></style> data into a global.css and report.css files?

4. I added a template engine, Smarty, yesterday, which helped to clean up my code since I was able to extract the HTML out of the PHP file. However, other problems have cropped up with that, including difficulties nesting loops correctly, and it being hard to tell exactly what variables the template needs from the PHP file. I am open to any suggestions you guys have related to Smarty, or even suggestions to use a different template engine. A template engine where you can send loop data row by row instead of building a giant array would be ideal, I think.

Answer 1

IDs must be unique

Your document will not validate because you've reused certain ids multiple times. If you need to reuse selectors, use a class instead.

Overusing the id selector

All of your selectors are ids and they all have the same specificity (deja vu?). The C in CSS is what makes it beautiful, but you're not letting it happen by using nothing but ids (seriously, look at all of that repetition).

Semantic names

Classes and ids should be named after the element's purpose, not what it looks like. Why would an element have an id of "red"? Is it an error? Is it a danger level? Is it nearly empty? Is it underpopulated?

Misusing the table element

Tables should only be used for tabular data. If your content doesn't make sense in a spreadsheet then it doesn't belong in a table. Your first table does not contain tabular data.

You're also using the td element incorrectly. Cells that are intended to be used as headings for a particular row or column should be marked up with the th element.

There's an element for that...

HTML5 added a time element that's appropriate for marking up dates/times.

Learn to love shorthand

body {
    font-family: Calibri;
    font-size: 11pt;
}

Becomes:

body {
    font: 11pt Calibri;
}

Also, you really should provide a complete font stack that contains suitable fallback fonts for systems that don't have Calibri installed.

Answer 2

Security

SQL Injection

Is it worth converting from MySQL to MySQLi or PDO? Somebody on another forum suggested that. Is there a big learning curve to switch to MySQLi or PDO?

Yes, you really should move away from mysql_*. It's deprecated for quite a while and not secure. And there isn't that big a learning curve.

Once you actually parse user input instead of having hardcoded values for testing, your code will be open to SQL injection. You shouldn't put variable data directly into queries, but use prepared statements instead, either with PDO or with mysqli.

Includes

include('./pages/' . $_GET['p'] . '.php');

I would prefer to use a whitelist when including stuff. This isn't an immediate thread anymore, as null byte injection was fixed a long time ago (although there might possibly be alternatives, depending on the system).

Still, code like this gives up some control over the control flow of the website, as an attacker can include any PHP file on the system, even files you might not want to have included. An example: You store uploaded files outside the web root, as an additional defense against users uploading PHP shells. An attacker manages to upload a PHP shell, because your upload code is vulnerable. Now because of this include, the attacker can actually gain code execution, which would otherwise not have been possible.

OOP and reusability

Is it worth converting this to OOP? For example, making report.php a Report class? It could improve code readability, in exchange for needing to create the class and call a display method somewhere else when I want to load the page.

I would; if you do, take a look at the MVC pattern. You don't have to follow it exactly, but some separation of model, view, and controller can go a long way.

But even if you stay with your approach, it would be a good idea to either have functions/classes in a file or code that has side-effects, instead of having both in the same file.

Right now, you couldn't eg use the format_date or get_weeks_until_event functions anywhere else, because as soon as you import that file, the additional code will be executed.

Misc

• move your CSS code to it's own file, which you then include. This increases readability, maintainability, and performance.
• create more functions. Your db access functions like get_days_until_event are great, why not create functions like that for the other three queries as well?