PHP contact form code review

The way you have it set up

if (isset($error_message))

should be

if (!isset($error_message))

What Casey said is the only error I see, the other one, just for cleaner code is this.

$name = ($_POST['name']);
$email = ($_POST['email']);
$phone = ($_POST['phone']);
$shipment = ($_POST['shipment']);

those don't need to be in parantheses. Imo it would look cleaner and be easier to read without it. I think it would also be best practice to put

    $mail = new PHPMailer();

inside of your !isset($error_message) loop so that a new mail instance isn't being created every time even when it's not going to use it.

Another note, on your home page, the request a quote button anchor tag padding seems to be off. You can only click the text and not the rest of the orange. You need to expand the padding on that a tag so it will let you click it. I didn't seem to have this problem on the actual quote page

Jake and Casey,

Thanks a lot guys thing seem to be working a lot better. I will fix that padding issue on the home page as well.

However I know have another issue and I think I have found the culprit.

$mail->AddAddress($address, "Power Only Transit");

that piece of code seems to be throwing it off. When I have that line written like that it sends the email correctly but it will not display my "thank you message" it just reloads leaving the values as if there was an error and displays this:

Invalid address: "whatever is typed in the name field here"

BUT if I remove this piece of code it displays all of the correct messages and acts perfect, except then it doesn't send an email. Can someone walk me through exactly what that line of code does/means?

Thanks everyone for the help, Kris

What the line of code does is basically add the address (by calling the AddAddress METHOD of the $mail OBJECT) you're giving it as an address to send the email TO. You can probably get rid of the $address variable and change to:

$mail->AddAddress('quotes@poweronlytransit.com', 'Power Only Transit');

I would take out the capital Q as this might be causing PHPMailer not to trust it.

Casey

Casey,

Tried it and I seem to be getting the same result.

It seems that my form is actually trying to verify the "name" field as an email address. because if you put an email address in both the name and email slot on the form it works perfect.

meaning that the problem might be with:

    if (!filter_var($email, FILTER_VALIDATE_EMAIL)) {
    //invalid emailAddress
    $error_message = "<div class='alert alert-error'>
    <p style='text-align:center'>The email you entered doesn't seem to be valid</p>
    </div>";
}

or the actual form

<form action="get-a-quote.php" method="post">
            <fieldset>

                <label for="name">Name</label>
                <input type="text" name="name" placeholder="first and last name" value="<?php if (isset($name)) { echo htmlspecialchars($name); } ?>">

                <label for="email">Email</label>
                <input type="text" name="email" placeholder="yourname@domain.com" value="<?php if (isset($email)) { echo htmlspecialchars($email); } ?>">

                <label for="phone">Phone Number</label>
                <input type="text" name="phone" placeholder="555-555-5555" maxlength="12" value="<?php if (isset($phone)) { echo htmlspecialchars($phone); } ?>">

                <label for="shipment">Please describe your shipment to help us get you the best possible price</label>
                <textarea rows="6" name="shipment" class="span4"><?php if (isset($shipment)) { echo htmlspecialchars($shipment); } ?></textarea><br>


                <button type="submit" class="btnQuote">Request a Quote</button>
            </fieldset>
        </form>

notice anything wrong here?

Thanks for all the help Casey!

This looks like what is wrong

$mail->SetFrom($name, $email);

should be in this form

SetFrom($address, $name = "")

I got that from the method documentation here. So just switch the order of the variables and that should resolve that.

Also something I noticed in your code, you are using " " and ' ' weirdly. I want to say incorrectly but that's the wrong word. It's just the standards use them differently and that's because they work differently. Inside of " " you can use variable names and call them directly. so you could say

"Hello there $name"

and if $name was set to "Jake" that would output "Hello there Jake". Inside of ' ' you CANNOT put variable names in there directly. For an example, your code :

$email_body = "";
$email_body = $email_body . "Name: " . $name . "<br>";
$email_body = $email_body . "Email: " . $email . "<br>";
$email_body = $email_body . "Phone: " . $phone . "<br>";
$email_body = $email_body . "Shipment: " . $shipment . "<br>";

Can be written as

$email_body = "";
$email_body = "$email_body Name: $name <br>";
$email_body = "$email_body Email: $email <br>";
$email_body = "$email_body Phone: $phone <br>";
$email_body = "$email_body Shipment: $shipment <br>";

Though, an even better way to write that, in my opinion, would be like this:

$email_body =   "Name: $name <br>
                 Email: $email <br>
                 Phone: $phone <br>
                 Shipment: $shipment";

Which would do the same thing.

Also, instead of assigning the variable to itself over and over, Take a look at the "concatenating assignment operator ('.=')"

In this situation isn't the best time to use it, but in other places it comes in handy so it's good to know.

Also on this line your ! operator is being used incorrectly.

if(!$mail->Send()) {

You need to remove that. The method reference says that returns false when the message is not sent. So currently your code is saying to redirect them to the "Thanks" page when it's NOT sent. you want the opposite, only redirect them when it is sent.

In your else statement you would then want to echo an error saying something like "Message was not send, contact administrator to get this resolved", because that would be something outside of just email validation causing it to not be sent. Or just delete that else statment and not say anything, though it's good practice to include it and output an error if there is one, so the customer doesn't think it was sent.

Jake,

Thanks man your on fire! I don't know if i would have been able to solve that without your help. I looked at all the links which seriously helped out a lot. The form now works as intended!!

I think I am clear on everything I need to do a little bit more research on quotes when it comes to single and double to make sure I have it down pat.

$email_body =   "Name: $name <br>
             Email: $email <br>
             Phone: $phone <br>
             Shipment: $shipment";

This makes perfect sense and I have decided to go with it in my code.

Also based on Casey's recommendation I took out the $address variable.

Here is a link to the whole code, I'm hoping It's pretty clean/good now.

http://pastie.org/6607910#58

I think my next step is to build my errors into an array and have it loop through and display all errors that apply rather than just popping errors via top down priority.

Again, Thank you for all the help!!

Kris

To explain the difference between single quotes and double quotes. Single quotes can only contain plain text and no variables. So

$name = 'jake'; 
echo 'Hello $name!'; //Outputs: Hello $name!
echo "Hello $name!"; //Outputs: Hello jake!

To clean up your code, you close and reopen the php tags from line 57 and 59. No reason to do that, probably just left over from something else :) Doesn't hurt the code, just unnecessary.

For the error message array, I think that's a great idea! I almost typed out the answer for you but I figured I'd let you give it a shot and let me know if you have any questions! Feel free to contact me on any of the links on my profile if you have any specific ones.

Jake,

Boom! cut and dry single vs. double. thanks man! fixed that close/open. I'm gonna start working on the array and i'll definitely take you up on that offer about asking questions.

Kris