patternjavascriptMinor
Student status form
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.
When the user clicks on
```
$(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);
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
In the
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
"QUOTES"
Also see CodeX answer.
As tim mentioned
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.
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.