Problem with form and case/break loop for email

[deputy963]

Hello everyone,

 

I'm a PHP noob. I haven't written a lick of code in over 25 years and then it was basic/pascal, so please be patient with me.

 

I had an idea to eliminate paperwork and streamline some tasks at work. It seemed simple at the time and I wanted to prove it could be done. I've worked my way through most of it, but I've reached a point where I'm lost.

 

So, I have a form. User fills out the form and it gets written to a MySQL database. That information is then emailed to a specific person using a function. A separate email is also sent to a variety of people based on the Station field. Problem is I've screwed up the PHP code and the page won't load.

 

I've attached the php template, rather than insert 300 lines of code.

Thank you in advance for any help you can provide this PHP noob who is up to his eyeballs in a language he doesn't fully understand.

Discrep_Entry_test.php

[BagoZonde]

Every line like that:

 

<p>DO NOT reply to this email! For further assistance please contact Dave Sturgeon at <a href="mailto:dave.sturgeon@wishtv.com">dave.sturgeon@wishtv.com</a> or Gerry Inks at <a href="mailto:gerry.inks@linmedia.com">gerry.inks@linmedia.com</a>.</p>

 

change to:

 

<p>DO NOT reply to this email! For further assistance please contact Dave Sturgeon at <a href=\"mailto:dave.sturgeon@wishtv.com\">dave.sturgeon@wishtv.com</a> or Gerry Inks at <a href=\"mailto:gerry.inks@linmedia.com\">gerry.inks@linmedia.com</a>.</p>

 

as you don't escape quotes.

I'm not looking for code as it's baaaad , I'm just telling you what you must fix to run that script properly .

 

Also if PHP don't display errors, try change ini on the very beginning and maybe it can help you in future to check where are problems:

 

 

ini_set('error_reporting', E_ALL^E_NOTICE);
ini_set('display_errors', 1);

Edited January 25, 2013 by BagoZonde

[deputy963]

Are you making fun of this noob's awesome coding skills?

 

Rookie mistake. I'll give it a shot after I run a few errands. Is there a better way to accomplish this, or general advise (other than I should never code again)?

[BagoZonde]

I'm not a guru but one advice: you should use some arrays to keep your data more organised then it would be more readable for yourself and others. Also repeating some parts isn't a good practise as you're forced to make fixes in different parts of code. So, use arrays and keep rollin' :].

[deputy963]

That may be above my skill level, which is "guru-1,000,000,000".

[PFMaBiSmAd]

Your shouldn't be repeating blocks of code that only differs in the data it operates on. A switch/case statement is for running different code in each case, i.e. insert code, update code, delete code... Your switch/case statement should be replaced with code that validates that the input data is one of the permitted values, then use the (one) variable holding that value in the the message you are building. The code to build the message would only appear once.

 

Also, the global keyword only has meaning inside of a function definition (those outside of a function definition just wasted your time typing them), and even inside of a function it should not be used. You should be passing the values into a function as call-time parameters. Another way of looking at this, if a function requires a long list of variables that are present in your main application, it's likely that the code in your function is part of your application and shouldn't be in a function.

[PFMaBiSmAd]

Here's an example of what has been suggested. Using a data driven design, where you define a data structure someplace (array, database table), and use general purpose code, that you don't have to modify to add/remove/change the data, that takes care of outputting or processing the information -

 

// define a data structure that the code will use
$stations['EANE'] = array('city'=>'Ft. Wayne','contact'=>'xx@xxxx.com');
$stations['WANE'] = array('city'=>'Ft. Wayne','contact'=>'xx@xxxx.com');
$stations['WISH'] = array('city'=>'Indianapolis','contact'=>'xx@xxxx.com');
$stations['WISH LWS'] = array('city'=>'Indianapolis','contact'=>'xx@xxxx.com');
$stations['WLFI'] = array('city'=>'Lafayette','contact'=>'xx@xxxx.com');
$stations['WNDY'] = array('city'=>'Marion','contact'=>'xx@xxxx.com');

// at some point, you have validated the submitted data and stored it in a named variable
$var_station = $_POST['station'];

// Begin section to send station dependent emails
if(!isset($stations[$var_station])){
// the submitted station doesn't exist, handle that condition here...
// should only occur due to a programing error or someone submitting their own data
echo 'The submitted station does not exist.';
} else {
// $var_station is one of the defined stations
$to = $stations[$var_station]['contact'];
$subject = "New Discrepancy for $var_station";
$message = "<html>
<head>
<title>New Record Added</title>
</head>
<body>
<p>A new discrepancy has been added for:<br />Station: <strong>$var_station</strong></p><p>Log Date: <strong>$var_date</strong><br />Log Time: <strong>$var_time</strong><br />Trouble Source: <strong>$var_source</strong><br />Description: <strong>$var_description</strong><br />Content: <strong>$var_titleID</strong><br />Loss: <strong>$var_loss</strong><br />Operator: <strong>$var_operator</strong></p>
<p>DO NOT reply to this email! For further assistance please contact Dave Sturgeon at <a href='mailto:dave.sturgeon@wishtv.com'>dave.sturgeon@wishtv.com</a> or Gerry Inks at <a href='mailto:gerry.inks@linmedia.com'>gerry.inks@linmedia.com</a>.</p>
</body>
</html>";
$headers = "From: LIN Discrepancy System <>";
$headers .= 'MIME-Version: 1.0' . "\r\n";
$headers .= 'Content-type: text/html; charset=iso-8859-1' . "\r\n";
$sent = mail($to,$subject,$message,$headers);
}

 

Once you have a defining data structure, you would also use it everywhere that data is referenced -

 

// produce the option list for the form
$station_options = '';
foreach($stations as $station=>$arr){
$legend = "$station - {$arr['city']}";
$station_options .= "<option value='$station'>$legend</option>\n";
}

// echo the above $station_options variable inside the <select></select> tags to build the complete select drop-down

Edited January 25, 2013 by PFMaBiSmAd

[deputy963]

Sorry for taking so long to get back to this...

 

I see where you're going PFMaBiSmAd, and I appreciate the reply/advice. The more I thought about it the more sense it makes to put the variables into a table - at least from a maintenance standpoint. So I have the original table "discrepancy" and a new table "discrepancy_contact", which contains a column for Station, Contact, and admin contact.

 

Knowing that the station is chosen by the form user. I would like to compare that choice $var_station to the station column in the new table and use the associated information in the email.

[deputy963]

I guess I should have added... What is the best way to accomplish that?