4
\$\begingroup\$

I put together some functions that allows a PHP script to send the SQL data obtained from user inputs on a website via an email attachment as a CSV file. It works perfectly and I have no issues with the code. Alternatively with the right SQL query it would run every X amount of days with a cron job and only produce the data within a specific criteria such as every month.

The idea behind the code is for users that cannot extract their data from SQL and have limited experience running their businesses online. I would like some input from what I have put together from scripts and endless amounts of testing to see if the code is up to standard.

Firstly I create the fileName:

  function fileName()
{
    $path     = "csv/";
    $filename = "Subscribers" . date('Ymd') . ".csv";
    $filepath = $path . $filename;
    return $filepath;
}

After which I create the CSV file:

function createCSV()
{
    include_once "conn.php";
    global $conn;
    
    $filepath = fileName();
    $output   = fopen($filepath, "w");
    fputcsv($output, array(
        'Name',
        'Email',
        'Date Joined'
    ));
    $query  = "SELECT name, email, dateTime FROM tableData";
    $result = mysqli_query($conn, $query);
    while ($row = mysqli_fetch_assoc($result)) {
        fputcsv($output, $row);
    }
    return $output;
    fclose($output);
    
}

Followed by the code I cut and edited from a thread online for attaching the CSV using the built in mail function:


function sendCSV()
{
    
    // Recipient 
    $to = 'recipient@email.com';
    
    // Sender 
    $from     = 'sender@example.com';
    $fromName = 'Test';
    
    // Email subject 
    $subject = 'PHP Email with Attachment';
    
    // Attachment file 
    $file  = fileName();
    $files = glob('csv/*.csv');
    unlink($files);
    
    createCSV();
    
    // Email body content 
    $htmlContent = ' 
    <h3>PHP Email with Attachment</h3> 
    <p>This email is sent from the PHP script with attachment.</p> 
    ';
    
    // Header for sender info 
    $headers = "From: $fromName" . " <" . $from . ">";
    
    // Boundary  
    $semi_rand     = md5(time());
    $mime_boundary = "==Multipart_Boundary_x{$semi_rand}x";
    
    // Headers for attachment  
    $headers .= "\nMIME-Version: 1.0\n" . "Content-Type: multipart/mixed;\n" . " boundary=\"{$mime_boundary}\"";
    
    // Multipart boundary  
    $message = "--{$mime_boundary}\n" . "Content-Type: text/html; charset=\"UTF-8\"\n" . "Content-Transfer-Encoding: 7bit\n\n" . $htmlContent . "\n\n";
    
    // Preparing attachment 
    if (!empty($file) > 0) {
        if (is_file($file)) {
            $message .= "--{$mime_boundary}\n";
            $fp   = @fopen($file, "rb");
            $data = @fread($fp, filesize($file));
            
            @fclose($fp);
            $data = chunk_split(base64_encode($data));
            $message .= "Content-Type: application/octet-stream; name=\"" . basename($file) . "\"\n" . "Content-Description: " . basename($file) . "\n" . "Content-Disposition: attachment;\n" . " filename=\"" . basename($file) . "\"; size=" . filesize($file) . ";\n" . "Content-Transfer-Encoding: base64\n\n" . $data . "\n\n";
        }
    }
    $message .= "--{$mime_boundary}--";
    $returnpath = "-f" . $from;
    
    // Send email 
    $mail = @mail($to, $subject, $message, $headers, $returnpath);
    
    // Email sending status 
    echo $mail ? "<h1>Email Sent Successfully!</h1>" : "<h1>Email sending failed.</h1>";
    }

I am quite new to PHP but managed to put something together that actually works. If you can point out my mistakes (which I do know there would be), or guide me to more simple techniques, I would really appreciate it and it will help me progress in my PHP learning path.

\$\endgroup\$
2
\$\begingroup\$

Global variable

It appears the connection string is used as a global variable:

global $conn;

Global variables have more negative aspects than positives. The conn.php file could define a function that would return the connection represented by $conn And the function could be called where needed - e.g. in createCSV(). Then there would be no need to globally reference that variable and the function could be stubbed/mocked for unit testing.

Limit Data

It looks like this query is executed to get all data:

$query  = "SELECT name, email, dateTime FROM tableData";

There is no indication of how muc. Then there would be no data exists but you should consider limiting data in some manner- either by adding WHERE conditions and/or a LIMIT clause. Otherwise selecting all rows has the potential to take a large amount of time.

String interpolation

The line to add the from header:

$headers = "From: $fromName" . " <" . $from . ">";

Uses string interpolation. That could be simplified to:

$headers = "From: $fromName <$from>";

conditionals

I see this:

if (!empty($file) > 0) {
     if (is_file($file)) {

empty() returns a bool, so to compare a negated bool “greater than zero” seems like a convoluted comparison but would function. It would be simpler to just see if it is true so the type does not need to be converted:

if (!empty($file) === true) {

Though with PHP this can be written simply as:

if (!empty($file)) {

Then the next conditional can be combined using logical AND - i.e. &&

if (!empty($file) && is_file($file)) {

This will keep indentation levels minimized.

After inspecting the return value of fileName() and how $file is not modified from the original value returned from that function, the first conditional expression seems pointless. It would make sense to have it if the value could change.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.