HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Student status form

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
formstudentstatus

Problem

I'm programming a system and I want to know if this is the correct way or if there is a better way to do it.

Container:

I have more fields but I only added one to show you the container.

                     
    
        Status:
        
            " id="studentId">
            
                Activo
                Inactivo
            
        
        Guardar                                    
         
                                                          


When the user clicks on statusSaveButton, I append a confirmation button (statusSaveButtonYes and statusSaveButtonNo). I only add the action if the user clicks on YES.

```
$(document).on('click','#statusSaveButton',function() {
$('#studentStatus').attr('disabled', 'disabled');
$('#statusSaveButton').prop('disabled', true);
var studentStatus = $("#studentStatus").val();
if ($("#studentStatusConfirm").length) {
$("#studentStatusConfirm").html('Are you sure you want to modify the record?  Yes  No');
$("#studentStatusConfirm").show();
} else {
$("#div-studentStatus").append('¿Estas seguro que deseas modificar el status del alumno?  Si  No');
}
});
$(document).on('click','#statusSaveButtonYes',function() {
var studentId = $("#studentId").val();
var studentStatus = $("#studentStatus").val();
$.post("content/studentsAction.php", {action:"editStudentStatus", studentId:studentId, studentStatus:studentStatus}, function(data){
if (data != "Successful") {
$("#studentStatusConfirm").html(''+data+'');
$("#statusSaveButtons").show();
$('#studentStatus').removeAttr('disabled');
$('#statusSaveButton').prop('disabled', false);
} else {
$("#studentStatusConfirm").html('Success.');
$("#statusSaveButtons").show();
$('#studentStatus').removeAttr('disabled');
$('#statusSaveButton').prop('disabled', false);

Solution

Variable definition

You sometimes define variables in camelCase and sometimes with underscores. I recommend to use one spelling type only.

PHP

If function updateStudentStatus is not part of a class consider developing a database class that handles it. Another option is to add a save function to each of your Model classes. This makes your code easier to read. For more details check mvc pattern.

In the updateStudentStatus function you establish a new database connection with each call. You can avoid it by promoting the function to a proper class and define a global attribute holding your database connection.

JS

In your javascript you have html code. I recommend to either load it with rest of the dom and hide it by default or load it using ajax. I personally compare it with inline css.

HTML

In html form you have an attribute onsubmit. It should be in a javascript file. I compare it with inline css as well.

"QUOTES"

Also see CodeX answer.

As tim mentioned $_POST and other variables not provided by yourself have to be validated and parsed by htmlspecialchars e.g. to avoid XSS.

A bit about security

I've added this section due to mentioned XSS.

Security is a very complex topic and must not be underestimated. There are a lot of options to implement and grant security. What kind of options you choose depends on your time/budget but also kind of project and data that are going to be stored in your database. I kindly ask you to think wise about the data that will be saved in the database and make researches about proper security measures.

There are a lot of PHP Frameworks. I recommend to take a look into some of them as those provide security standards.

Context

StackExchange Code Review Q#74602, answer score: 2

Revisions (0)

No revisions yet.