I have a table full of input text they look like this:
<input type='text' value='{Value}' id='{Model.ID}' class='ChckNumber' />
the class name is different based off what column it is, the Model.ID is different based off the row and the column is different based off the column and row.
When the input text becomes unfocused I call an api to update the value with what the user entered like so:
$("input[type=text]").on("focusout", function () {
var id = $(this).attr("id");
var column = $(this).attr("class");
var value = $(this).val();
if (typeof id !== "undefined" && typeof column !== "undefined" && typeof value !== "undefined") {
$.ajax({
url: "/api/Action/UpdateTable?id=" + id + "&column=" + column + "&value=" + value,
type: "GET",
error: function (request, status, error) {
InsertLogEntry(request.responseText + " From API Call /api/Action/UpdateTable");
},
success: function (data) {
}
});
}
});
Here is the API Controller its calling:
[HttpGet]
public void UpdateTable(int id, string column, string value)
{
MethodClass method = new MethodClass();
if(column != null && column != "" && id != 0)
{
method.UpdateTable(id, column, value);
}
}
and here is the method call I am making:
public void UpdateTable(int id, string column, string value)
{
connection = new SqlConnection(connectionString);
if(column.Contains("Date"))
{
DateTime dt = Convert.ToDateTime(value);
command = new SqlCommand("UPDATE tblCheckTrackerRegister SET " + column + " = @Date WHERE ID = " + id);
command.Parameters.Add("@Date", System.Data.SqlDbType.DateTime);
command.Parameters["@Date"].Value = dt.Equals(DateTime.MinValue) ? (object)DBNull.Value : dt;
}
else
{
int result;
if (int.TryParse(value, out result))
{
command = new SqlCommand("UPDATE table SET " + column + " = " + value + " WHERE ID = " + id);
}
else
{
command = new SqlCommand("UPDATE table SET " + column + " = '" + value + "' WHERE ID = " + id);
}
}
command.Connection = connection;
connection.Open();
command.ExecuteNonQuery();
connection.Close();
}
This works well, but I am looking to improve it. I honestly believe there must be a better way to go about this as my code seems messy to me, My question is, can anyone suggest another way to go about what I am trying to accomplish?
Your code is prone for SQL Injection attack. It is a red flag for security.
command = new SqlCommand("UPDATE tblCheckTrackerRegister SET " + column + " = @Date WHERE ID = " + id);
command = new SqlCommand("UPDATE table SET " + column + " = " + value + " WHERE ID = " + id);
command = new SqlCommand("UPDATE table SET " + column + " = '" + value + "' WHERE ID = " + id);
Suggestion: Use parameterize query.
You are updating a record, so you should not use GET method. It violates basic REST principle - GET action is said to be safe, and it should not change anything in the system.
Suggestion: Use Ajax POST.
$.ajax({
...
type: "POST",
...
});
I do not know the reason behind why each textbox is updated as soon as out of focus. It is not a common approach. From the user perspective, it is not friendly either because they are not notified about changes being saved.
We normally update a group of control at a time or one row at a time in Grid.
Suggestion : Not really a suggestion; it is just a personal preference, so don't get me wrong. I personally like to update a chunk of control at a time like attached StackOver screen shot -
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With