2
\$\begingroup\$

I have removed a lot of the code, but you should get an idea of what I am doing:

<?php

$upOne = realpath(__DIR__ . '/..');
require_once $upOne . '/vendor/autoload.php';

use phpseclib\Net\SFTP;

$errors = array();
$fileData = array();
$inputArray = array();
$config = new CONFIG\Config();


if (empty($_POST['emailAdd'])) {
    $errors['emailAdd'] = '- Please input your Email address';
} else {
    if (!filter_var($_POST['emailAdd'], FILTER_VALIDATE_EMAIL)) {
        $errors['emailAdd'] = '- Please input your Email address';
    } else {
        $email = filter_var($_POST['emailAdd'], FILTER_SANITIZE_EMAIL);
        array_push($inputArray, $email);
    }
}

//Additional validation
...

if (!empty($errors)) {
    echo json_encode("failed");
} else {
    try {
        $dbh = new PDO("mysql:host=".$config::DB_HOST.";dbname=".$config::DB_NAME."", $config::DB_USER, $config::DB_PASSWORD);

        $stmt = $dbh->prepare("INSERT INTO emirates_user("...");

        if($stmt->execute()) {

            $lastId = $dbh->lastInsertId();

            if (isset($_FILES)) {
                foreach($_FILES as $file) {
                    $upload = UPLOAD\Upload::factory($config::TEMP_DIR);
                    $upload->file($file);

                    if(!$upload->get_errors()) {
                        $fileData[$results['full_path']] = $results['mime'];
                    }
                    else {
                        echo json_encode("failed");
                        break;
                    }
                }

                $date = date('_dmY_H:i:s');
                $pdf = new PDF\PdfGenerator($fileData, $inputArray, $date);

                if($pdf) {
                    $command = '/usr/bin/python /srv/www/vhosts/someurl/html/php/somefile.py';
                    exec($command, $retval);

                    if(isset($retval[0])  && $retval[0] === 'success') {
                        $mail = new PHPMailer;

                        .....

                        if(!$mail->send()) {
                            echo json_encode("failed");
                        } else {
                            $lstmt = $dbh->prepare("...");
                            ...

                            if($lstmt->execute()) {
                                echo json_encode("success");
                            }
                        }
                    } else {
                        echo json_encode("failed");
                    }
                }

            } else {
                echo json_encode("failed");
            }
        } else {
            echo json_encode("failed");
        }
    }
    catch(Exception $e) {
        echo json_encode("failed");
    }
}

First of all, I validate the user input. If this fails, I return a failed message back to my Ajax. If all is ok, the process begins. This script inserts into a database, generates a PDF, SFTP's the file using a Python script, sends an email etc. All of these actions could potentially go wrong. I have handled things via conditional statements to make sure the code is executed correctly. If not, then I return a failed message again. If it makes it to the very end, I return a success message.

My Ajax is like so (not the whole function, just the done and failure sections):

.done(function (response) {
    if(response == 'failed'){
        window.location.replace('failure.html');
    } else {
        window.location.replace('success.html');
    }
}).fail(function (jqXHR, textStatus) {
    console.log(textStatus);
});

So if the response is failed, I display a failed page.

Really, I am just seeking advice as to whether this type of logic is ok. Should I maybe return more detail information about the error so I know what it relates too? If so, how would I handle this in Ajax?

\$\endgroup\$
1
\$\begingroup\$

There are a few things that you might consider here, to improve readability, maintainability, and (indirectly through those qualities), reliability.

As a guideline, if a piece of code goes over 10 lines or statements, it is probably about time to break it up into functions.

In this case, I might consider breaking it up into following functions:

public function getEmailInput() {}
public function getFileInput() {}
public function insertIntoDatabase() {}
public function generatePdf() {}
public function sendConfirmationEmail() {}
//Maybe some others, as you see fit

This will allow give you a little more freedom to update the steps as that inevitably becomes necessary without as much risk of breaking other parts or having to remember how the entire program works.

On the JavaScript side, it looks like this is the kind of script that needs to fail safe, so I might consider checking for success rather than failure, like so:

.done(function (response) {
    if(response === 'success'){ //make sure we got the response we needed
        window.location.replace('failure.html');
    } else {
        window.location.replace('success.html');
    }
}).fail(function (jqXHR, textStatus) {
    console.log(textStatus);
});
\$\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.