mds1256 Posted August 14, 2012 Author Share Posted August 14, 2012 Keep playing if you have, if you haven't just play with it, it's really simple if you put the register & login on two pages. Once you see whats happening you can combine them. I removed the register form but still with the same problem, I am misunderstanding what removing this web form is achieving? Quote Link to comment Share on other sites More sharing options...
floridaflatlander Posted August 14, 2012 Share Posted August 14, 2012 What do you have now? PS. You haven't played with it much, I made my last post 55+- minutes ago. Quote Link to comment Share on other sites More sharing options...
mds1256 Posted August 14, 2012 Author Share Posted August 14, 2012 What do you have now? PS. You haven't played with it much, I made my last post 55+- minutes ago. I tried a few ideas which also didnt work by moving things about. I could call the function before any HTML output but how would I get the form to display where I want it rather than the top of the page before the header etc. At the moment I have signin.php <?php require_once('_coreFunctions.php'); // this is where DB functions are e.g. connection etc require_once('_signInFunctions.php'); require_once('header.php'); // this has HTML inside of it e.g. head tag etc ?> <div id="contentBoxWrapper"> <div id="contentBoxTop"> <div id="contentBoxTopInner"><a href="/" class="firstLink">Home</a> <span class="breadcrumbDivider">></span> Sign In<hr /></div> </div> <div id="contentBoxContent"> <div id="signInFormWrapper"> <h1>Sign In</h1> <?php sif_loginForm(); ?> // this is where I call the login form function...... <br /> <a href="resetpassword">Forgotten password?</a> </div> </div> <div id="contentBoxBottom"></div> </div> <?php require_once('footer.php'); ?> _signInFunctions.php <?php require_once('_coreFunctions.php'); function sif_loginForm() { $displayForm = true; $error = ""; $username = ""; if(isset($_POST['signInSubmit'])) { $username = mysqli_real_escape_string(cf_dbConnect(), $_POST['username']); $password = mysqli_real_escape_string(cf_dbConnect(), $_POST['password']); $query = mysqli_query(cf_dbConnect(), "call sp_checkLoginDetails('{$username}', '{$password}')"); if(mysqli_num_rows($query) == 1) { $_SESSION['isLoggedIn'] = true; while($row = mysqli_fetch_object($query)) { $_SESSION['firstName'] = $row->firstName; } mysqli_query(cf_dbConnect(), "call sp_updateLoginDateTime('{$_SESSION['firstName']}')"); header("Location: ."); // this is my redirect part } else { $error = "<div id=\"formMessages\">Username or Password is incorrect. Please try again.</div>"; if($username == "username...") { $username = ""; } } } if($displayForm) { echo " <form name=\"signInForm\" id=\"signInForm\" method=\"post\" action=\"\"> <label for=\"username\">Username:</label><br /> <input type=\"text\" name=\"username\" id=\"signInFormUsername\" class=\"formField\" value=\"{$username}\" /><br /><br /> <label for=\"password\">Password:</label><br /> <input type=\"password\" name=\"password\" id=\"signInFormPassword\" class=\"formField\" /><br /><br /> <input type=\"submit\" id=\"signInSubmit\" name=\"signInSubmit\" value=\"Login\" /> </form> <br /> {$error} "; } } ?> Quote Link to comment Share on other sites More sharing options...
mds1256 Posted August 14, 2012 Author Share Posted August 14, 2012 Ok, I have came up with an idea, using the principle of seperating out the business logic from the layout. Rather than echoing out the form code from within the function, I can call the function at the top of the page before any HTML e.g. before the header.php include. The fuction will still have the if - post so will still work. But.... Using this principle if I use the same idea for a contact form as an example and I then want to hide the form once submitted and show a response message of 'thank you for contacting us', how would I achieve this using this new method as the form code would always be hard coded into the html page? Quote Link to comment Share on other sites More sharing options...
floridaflatlander Posted August 14, 2012 Share Posted August 14, 2012 php processes from top to bottom. I'd do something like this but it probably has some errors <?php require_once('_coreFunctions.php'); // this is where DB functions are e.g. connection etc require_once('_signInFunctions.php'); $displayForm = true; $error = ""; $username = ""; if(isset($_POST['signInSubmit'])) { $username = mysqli_real_escape_string(cf_dbConnect(), $_POST['username']); $password = mysqli_real_escape_string(cf_dbConnect(), $_POST['password']); $query = mysqli_query(cf_dbConnect(), "call sp_checkLoginDetails('{$username}', '{$password}')"); if(mysqli_num_rows($query) == 1) { $_SESSION['isLoggedIn'] = true; while($row = mysqli_fetch_object($query)) { $_SESSION['firstName'] = $row->firstName; } mysqli_query(cf_dbConnect(), "call sp_updateLoginDateTime('{$_SESSION['firstName']}')"); header("Location: ."); // this is my redirect part } else { $error = "<div id=\"formMessages\">Username or Password is incorrect. Please try again.</div>"; if($username == "username...") { $username = ""; } } } require_once('header.php'); // this has HTML inside of it e.g. head tag etc ?> <div id="contentBoxWrapper"> <div id="contentBoxTop"> <div id="contentBoxTopInner"><a href="/" class="firstLink">Home</a> <span class="breadcrumbDivider">></span> Sign In<hr /></div> </div> <div id="contentBoxContent"> <div id="signInFormWrapper"> <h1>Sign In</h1> <?php if (isset($error)) {echo $error;} ?> <form name=\"signInForm\" id=\"signInForm\" method=\"post\" action=\"\"> <label for=\"username\">Username:</label><br /> <input type=\"text\" name=\"username\" id=\"signInFormUsername\" class=\"formField\" value=\"{$username}\" /><br /><br /> <label for=\"password\">Password:</label><br /> <input type=\"password\" name=\"password\" id=\"signInFormPassword\" class=\"formField\" /><br /><br /> <input type=\"submit\" id=\"signInSubmit\" name=\"signInSubmit\" value=\"Login\" /> </form> <br /> <a href="resetpassword">Forgotten password?</a> </div> </div> <div id="contentBoxBottom"></div> </div> <?php require_once('footer.php'); ?> Quote Link to comment Share on other sites More sharing options...
floridaflatlander Posted August 14, 2012 Share Posted August 14, 2012 I then want to hide the form once submitted and show a response message of 'thank you for contacting us', how would I achieve this using this new method as the form code would always be hard coded into the html page? use an if() condition Quote Link to comment Share on other sites More sharing options...
mds1256 Posted August 14, 2012 Author Share Posted August 14, 2012 I then want to hide the form once submitted and show a response message of 'thank you for contacting us', how would I achieve this using this new method as the form code would always be hard coded into the html page? use an if() condition Thanks for the above code, that is what I was thinking towards, when you say use an IF(), i am guessing I would need to put the if condition in the signin.php around the form which is back in normal HTML, so wouldnt that be mixing layout with logic again e.g. <?php require_once('_coreFunctions.php'); // this is where DB functions are e.g. connection etc require_once('_signInFunctions.php'); $displayForm = true; $error = ""; $username = ""; if(isset($_POST['signInSubmit'])) { $username = mysqli_real_escape_string(cf_dbConnect(), $_POST['username']); $password = mysqli_real_escape_string(cf_dbConnect(), $_POST['password']); $query = mysqli_query(cf_dbConnect(), "call sp_checkLoginDetails('{$username}', '{$password}')"); if(mysqli_num_rows($query) == 1) { $_SESSION['isLoggedIn'] = true; while($row = mysqli_fetch_object($query)) { $_SESSION['firstName'] = $row->firstName; } mysqli_query(cf_dbConnect(), "call sp_updateLoginDateTime('{$_SESSION['firstName']}')"); header("Location: ."); // this is my redirect part } else { $error = "<div id=\"formMessages\">Username or Password is incorrect. Please try again.</div>"; if($username == "username...") { $username = ""; } } } require_once('header.php'); // this has HTML inside of it e.g. head tag etc ?> <div id="contentBoxWrapper"> <div id="contentBoxTop"> <div id="contentBoxTopInner"><a href="/" class="firstLink">Home</a> <span class="breadcrumbDivider">></span> Sign In<hr /></div> </div> <div id="contentBoxContent"> <div id="signInFormWrapper"> <h1>Sign In</h1> <?php if (isset($error)) {echo $error;} ?> <?php if(!isset($_POST['submit'])) { echo " <form name=\"signInForm\" id=\"signInForm\" method=\"post\" action=\"\"> <label for=\"username\">Username:</label><br /> <input type=\"text\" name=\"username\" id=\"signInFormUsername\" class=\"formField\" value=\"{$username}\" /><br /><br /> <label for=\"password\">Password:</label><br /> <input type=\"password\" name=\"password\" id=\"signInFormPassword\" class=\"formField\" /><br /><br /> <input type=\"submit\" id=\"signInSubmit\" name=\"signInSubmit\" value=\"Login\" /> </form>"; } ?> <br /> <a href="resetpassword">Forgotten password?</a> </div> </div> <div id="contentBoxBottom"></div> </div> <?php require_once('footer.php'); ?> Quote Link to comment Share on other sites More sharing options...
floridaflatlander Posted August 14, 2012 Share Posted August 14, 2012 Yeah if signed in display the thank you, if not display the form. Quote Link to comment Share on other sites More sharing options...
Christian F. Posted August 14, 2012 Share Posted August 14, 2012 Now we're talking. What you can do, and what I actually recommend to do, is to still use the function to validate input and generate the form. That way you can leverage the return construct to exit the function early, and as such cut down a whole lot on the nesting. Something which will make your code a lot easier to read, and thus more maintainable. Quote Link to comment Share on other sites More sharing options...
mds1256 Posted August 14, 2012 Author Share Posted August 14, 2012 Now we're talking. What you can do, and what I actually recommend to do, is to still use the function to validate input and generate the form. That way you can leverage the return construct to exit the function early, and as such cut down a whole lot on the nesting. Something which will make your code a lot easier to read, and thus more maintainable. So have one function that will check if form is posted (like the one I already have) and then also have another function to echo the form out? Do you have a very basic example on what you mean by the return construct to exit? Quote Link to comment Share on other sites More sharing options...
Christian F. Posted August 14, 2012 Share Posted August 14, 2012 I can give you an basic example of the flow that I'm using, in pseudo-code, perhaps that will give you some pointers. Using a contact form as the base, as it's a very simple and common issue. Do note that I'm using a template engine though, to select and show the proper HTML code. In addition to that I'm running the following code inside a function, so this is just the specific code for the contact form itself. // First off we check if something has been submitted. if (submit) { // If it has, retrieve and validate all user input immediately. validate (username); validate (email); validate (message); // Make sure all fields validated. if (!validated) { // If something failed validate, make sure user knows what and doesn't have to type in any of the details again. template->add_warning (Failed fields); template->repopulate (form); template->add_form (); // Return to the calling function, so that the template data can be parsed and printed. return; } // Make sure input it safe for use in e-mails. escape_output ('mail'); // Try to send the email, and redirect the user to the correct page depending upon the success of the email. if (send_mail ()) { header ('location: success'); die (); } template->add_error ('Email failed'); template->repopulate (form); template->add_form (); } // Nothing has been posted. template->add_form (); Quote Link to comment Share on other sites More sharing options...
mds1256 Posted August 14, 2012 Author Share Posted August 14, 2012 Ok, I have refactored my code just to try and get this working then I will start sorting the functions out to display the form. The problem is now that I am losing variables due to scope, I DONT want to create globals as I hear they are not good. Basically I need to use the $displayForm variable that is defined in the function to be accessible outside of the function and on the signin.php page, the same would go for the $error variable too? any ideas on the best way to do this? Thanks signin.php <?php require_once('_coreFunctions.php'); require_once('_signInFunctions.php'); sif_checkLoginForm(); // calling the check form function where $displayForm is defined require_once('header.php'); ?> <div id="contentBoxWrapper"> <div id="contentBoxTop"> <div id="contentBoxTopInner"><a href="/" class="firstLink">Home</a> <span class="breadcrumbDivider">></span> Sign In<hr /></div> </div> <div id="contentBoxContent"> <div id="signInFormWrapper"> <h1>Sign In</h1> <?php if($displayForm) // this variable isnt available due to scoping problems { echo " <form name=\"signInForm\" id=\"signInForm\" method=\"post\" action=\"\"> <label for=\"username\">Username:</label><br /> <input type=\"text\" name=\"username\" id=\"signInFormUsername\" class=\"formField\" value=\"{$username}\" /> <br /><br /> <label for=\"password\">Password:</label><br /> <input type=\"password\" name=\"password\" id=\"signInFormPassword\" class=\"formField\" /><br /><br /> <input type=\"submit\" id=\"signInSubmit\" name=\"signInSubmit\" value=\"Login\" /> </form> <br /> {$error} "; } ?> <br /> <a href="resetpassword">Forgotten password?</a> </div> </div> <div id="contentBoxBottom"></div> </div> <?php require_once('footer.php'); ?> _signInFunctions.php <?php require_once('_coreFunctions.php'); function sif_checkLoginForm() { $username = ""; $displayForm = true; // defined here, dont want to use GLOBALS if(isset($_POST['signInSubmit'])) { $username = mysqli_real_escape_string(cf_dbConnect(), $_POST['username']); $password = mysqli_real_escape_string(cf_dbConnect(), $_POST['password']); $query = mysqli_query(cf_dbConnect(), "call sp_checkLoginDetails('{$username}', '{$password}')"); if(mysqli_num_rows($query) == 1) { $_SESSION['isLoggedIn'] = true; while($row = mysqli_fetch_object($query)) { $_SESSION['firstName'] = $row->firstName; } mysqli_query(cf_dbConnect(), "call sp_updateLoginDateTime('{$_SESSION['firstName']}')"); header("Location: ."); die(); } else { $error = "<div id=\"formMessages\">Username or Password is incorrect. Please try again.</div>"; if($username == "username...") { $username = ""; } } } } ?> Quote Link to comment Share on other sites More sharing options...
Christian F. Posted August 14, 2012 Share Posted August 14, 2012 Use return, or if you need multiple variables from the function, passing by reference. Quote Link to comment Share on other sites More sharing options...
mds1256 Posted August 14, 2012 Author Share Posted August 14, 2012 Use return, or if you need multiple variables from the function, passing by reference. The passing by reference worked a treat! my amended code signin.php <?php require_once('_coreFunctions.php'); require_once('_signInFunctions.php'); sif_checkLoginForm($displayForm, $username, $error); // I am passing in the variables defined in _signInFuntions.php require_once('header.php'); ?> <div id="contentBoxWrapper"> <div id="contentBoxTop"> <div id="contentBoxTopInner"><a href="/" class="firstLink">Home</a> <span class="breadcrumbDivider">></span> Sign In<hr /></div> </div> <div id="contentBoxContent"> <div id="signInFormWrapper"> <h1>Sign In</h1> <?php if($displayForm) { echo " <form name=\"signInForm\" id=\"signInForm\" method=\"post\" action=\"\"> <label for=\"username\">Username:</label><br /> <input type=\"text\" name=\"username\" id=\"signInFormUsername\" class=\"formField\" value=\"{$username}\" /> <br /><br /> <label for=\"password\">Password:</label><br /> <input type=\"password\" name=\"password\" id=\"signInFormPassword\" class=\"formField\" /><br /><br /> <input type=\"submit\" id=\"signInSubmit\" name=\"signInSubmit\" value=\"Login\" /> </form> <br /> {$error} "; } ?> <br /> <a href="resetpassword">Forgotten password?</a> </div> </div> <div id="contentBoxBottom"></div> </div> <?php require_once('footer.php'); ?> _signInFunctions.php <?php require_once('_coreFunctions.php'); $displayForm = true; // define the variables that are needed on the signin.php page $username = ""; $error = ""; function sif_checkLoginForm(&$displayForm, &$username, &$error) // pass in references to these variables so they can be accessed { if(isset($_POST['signInSubmit'])) { $username = mysqli_real_escape_string(cf_dbConnect(), $_POST['username']); $password = mysqli_real_escape_string(cf_dbConnect(), $_POST['password']); $query = mysqli_query(cf_dbConnect(), "call sp_checkLoginDetails('{$username}', '{$password}')"); if(mysqli_num_rows($query) == 1) { $_SESSION['isLoggedIn'] = true; while($row = mysqli_fetch_object($query)) { $_SESSION['firstName'] = $row->firstName; } mysqli_query(cf_dbConnect(), "call sp_updateLoginDateTime('{$_SESSION['firstName']}')"); header("Location: ."); die(); } else { $error = "<div id=\"formMessages\">Username or Password is incorrect. Please try again.</div>"; if($username == "username...") { $username = ""; } } } } ?> Quote Link to comment Share on other sites More sharing options...
Christian F. Posted August 14, 2012 Share Posted August 14, 2012 Note that passing by reference is something you should use sparingly, as it make the code harder to read. Most of the time it's better to return an array, and use list () to separate it into individual variables if you need that. I've also taken the liberty to clean up your function a bit, to further showcase what I meant by my previous posts: // Pass in references to these variables so they can be accessed function sif_checkLoginForm (&$displayForm, &$username, &$error) { // Make sure the form has been submitted. if (!isset ($_POST['signInSubmit'])) { // If not, exit early. return false; } // I suspect you want to use the database object here, and not (what looks like) // starting a new connection for each call to mysqli_real_escape_string (). $username = mysqli_real_escape_string (cf_dbConnect (), $_POST['username']); $password = mysqli_real_escape_string (cf_dbConnect (), $_POST['password']); // Check for error status first. $query = mysqli_query (cf_dbConnect (), "call sp_checkLoginDetails('{$username}', '{$password}')"); if (mysqli_num_rows ($query) != 1) { $error = "<div id=\"formMessages\">Username or Password is incorrect. Please try again.</div>"; if ($username == "username...") { $username = ""; } // Exit early. return false; } $_SESSION['isLoggedIn'] = true; while ($row = mysqli_fetch_object ($query)) { $_SESSION['firstName'] = $row->firstName; } mysqli_query (cf_dbConnect (), "call sp_updateLoginDateTime('{$_SESSION['firstName']}')"); header ("Location: ."); die (); } Anyway, glad we could help. Quote Link to comment Share on other sites More sharing options...
mds1256 Posted August 14, 2012 Author Share Posted August 14, 2012 Note that passing by reference is something you should use sparingly, as it make the code harder to read. Most of the time it's better to return an array, and use list () to separate it into individual variables if you need that. Anyway, glad we could help. Ok, I have switched to returning an array and using list as suggested and it works too it also like you suggests keeps my code readable. Thank you all who contributed to this, it is now working perfect Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.