Jump to content

Recommended Posts

I created an htlm form using $_SERVER['PHP_SELF']  to update my database and I am calling the form like so

<?php if( $_POST['update']){
		$unit 	= $_POST['unit'];
		$date 	= $_POST['date'];
		$amount = $_POST['amount'];
		$notes 	= $_POST['notes'];
	
		$york->update($unit, $date, $amount, $notes);
} ?>

in my database class, I have created an update function

public function update($unit, $date, $amount, $notes){
		$conn = $this->link;
		$sql = "UPDATE amount SET dues = ' . $amount . ', notes = '. $notes .' WHERE duesID = '. $unit .'";
		
		if (mysqli_query($conn, $sql)) {
			echo "Record updated successfully";
		} else {
			echo "Error updating record: " . mysqli_error($conn);
		}
	}

$this->link connects to the database in my __construct and there are no errors as far as that goes. When I hit the update button in the html form, the page reloads and there is a message saying "Record updated successfully" but when I check the database, there isn't any new record. Looking at it again, I have realized that the query is looking for duesID to match the $unit number which in this case should be 10 but the amount table doesn't have a record for that. so what I need is a query that will select the address table and amount table then update the amount table where the address table unitID = $unit which is passed through my html form. How do I do that?

The Address table does have a 14 entries in it with just to columns. Unit and unitID. Is this where a join comes into play?

Edited by CyberShot

There are several issues with the code you posted.

$_SERVER['PHP_SELF'] is vulnerable to an XSS Attack.

You need to check the REQUEST METHOD, not rely on the name of a button or hidden field in order for the script to work.

Do not create variables for nothing

Your code is vulnerable to an SQL Injection Attack. Never ever put variables in your query. You need to use Prepared Statements.

Do not output internal system errors to the user.

  • Like 1

all of this is done on localhost. I would assume it's not as dangerous?

I have updated the code to use the

if($_SERVER['REQUEST_METHOD'] == 'POST')

How do you perform an update that doesn't have variables? are you saying that they are ok to pass if I use prepared statements but not just in an mysqli->query?

I do remember reading that PHP_SELF was vulnerable attacks. What should I put there then? I don't want to separate my update function out of the db class. What is the best method for this?

why can't you output internal system errors? how do I get my errors if I don't output them?

Quote

How do you perform an update that doesn't have variables?

You already have the POST variables, just use them.

Quote

are you saying that they are ok to pass if I use prepared statements

Yes

Quote

I do remember reading that PHP_SELF was vulnerable attacks. What should I put there then?

Nothing. Just leave the action= out completely.

Quote

why can't you output internal system errors? how do I get my errors if I don't output them?

Because it leaks sensitive info that is only good to hackers and useless to the user. You turn on error reporting in the local dev php.ini and you also log the errors.

Edited by benanamen

Barand. I am trying to get your query working but I am having trouble. It is close though but I am getting an error at the last line. Here is what my database looks like

 

table address
   unit ( varchar(20) )
   unitID ( int, primary key, auto increment )

table amount
   autodate (timestamp)
   dues (int(6) )
   late (varchar(1) )
   paid (varchar(1) )
   notes ( varchar(500) )
   duesID ( int, primary key, auto increment )

Right now, I am just trying to get a test query in so that I can get  a "scaffold" to work off of. so I thought the way this would work is that the query would check the unitID in the address table to see if it matches the "unit" number passed through the post variable from the html form. Benanamen said not to create useless variables, so I took them out and just used the post variables as suggested. So my html form has a text box for

 

unit #, date, amount paid and notes

Right now, I am just trying to get the unit # to match the primary key in the address table and then put the amount, and notes into the amount table. After I get that working, I will work off of that to get the rest of the fields to populate. I have 14 entries in the address table and 1 in the amount table. As the query is now, it's says I have an error in my sql syntax "WHERE ad.unitID = '10'

The ten should match the primary key in the address table and put the record into the amount table to match.

Here is what I have been trying with my query

$sql = "UPDATE 
		amount am
		INNER JOIN 
		  address ad
		ON ad.unitID = '{$unit}'
		SET dues = {$amount},
	    notes = '{$notes}',
		WHERE ad.unitID = {$unit}";

 

here are my two tables. you will see in the below in the amount table, there is one record that has a duesID of 5 which does correspond to the address table unitID of 5. This is the correct record to show for unit 4109. I am trying to get a query that will choose the unitID and put the proper record down below so that when I query the database, the correct record will show for the correct unit. Right now, I have a form with unit #, date, amount paid and notes. My thought was that I could put in the unit number and then have the query select the matching number from the unitID in the address table and then insert the record into the amount table with the matching duesID number so that when I want to select a single record, I could just choose the unit number which will match the unitID and that record will populate.

mysql> select * from address;
+--------+----------------+
| unitID | unit           |
+--------+----------------+
|      1 | 4101           |
|      2 | 4103           |
|      3 | 4105           |
|      4 | 4107           |
|      5 | 4109           |
|      6 | 4111           |
|      7 | 4113           |
|      8 | 4115           |
|     10 | 4119           |
|     11 | 4121           |
|     12 | 4123           |
|     13 | 4125           |
|     14 | 4127           |
|      9 | 4117           |
+--------+----------------+

mysql> select * from amount;
+--------+------+------+------+---------------------+------------+---------------------+
| duesID | dues | late | paid | notes               | date       | autodate            |
+--------+------+------+------+---------------------+------------+---------------------+
|      5 |  210 | 0    | 1    | paid through paypal | 2018-12-04 | 2018-12-10 20:54:07 |
+--------+------+------+------+---------------------+------------+---------------------+

 

Edited by CyberShot

It's unusual to join the primary key of one table to the primary key of another. Do you only ever want one amount record for each address?

Try

UPDATE 
   amount am
     INNER JOIN 
   address ad
        ON am.duesID = ad.unitID
SET dues = ?,
    notes = ?
WHERE ad.unit = ?

 

Edited by Barand

you are going about this in the hardest way possible. even the naming of your tables and columns is not helping.

your address table appears to be a rental unit table. it should be named that - rental_units. the id column in this table defines the unit id, the other column in that table is apparently the unit name. it should be named so - name. i'm assuming it is a varchar column because rental unit names can include alphabetic characters, A, B, C...?

the amount table is some sort of unit dues payment table. it should be named to indicate the meaning of the data in the table - due_payments or similar name. the id column in this table defines the payment id, not the unit id. this table should have a separate unit_id column to relate the payment record back to the unit it corresponds to.

the form to insert a new payment record should have a way of selecting from the existing units, such as a select/option menu. this will reduce typo mistakes and only allow valid units to be chosen. this select/option menu would display the unit name, but the value that is submitted would be the unit_id. the INSERT form processing code would insert a record into the payment table with the submitted unit_id, amount, and other fields you have defined.

if you are editing an existing payment record, such as when correcting a typo in a value, you would reference the specific record using its payment id, not the unit_id. to display the corresponding unit name as part of this process, the SELECT query that retrieves  the existing payment record would JOIN with the units table to get the unit name. the UPDATE form processing code would update the record indicated by the payment id value.

as to displaying/logging the database statement errors. use exceptions to handle all database statement errors (connection, query, prepare, and execute) and in most cases let php catch the exception where it will use its error_reporting, display_errors, and log errors_settings to control what happens with the actual error information. the exception to this rule is if your application needs to detect and handle duplicate values for an insert/update query. your code would catch the exception in this case, detect if the error was for a duplicate index, set up and output an error message to the user telling them what is wrong with the data they submitted, or if error is not for a duplicate index, re-throw the exception and let php handle it. when learning, developing, and debugging code/queries, you would display all errors, which now includes the database statement errors, and when on a live/public server, you would log all errors, just by changing the php display_errors and log_errors settings. this will allow you to remove the existing error handling logic in your code, thereby simplifying it. to enable exceptions for the php mysqli extension, add the following line of code before the point where you are making the database connection -  

mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);

lastly, since you need to change to use prepared queries when supplying external/unknown values to the sql query statement, you need to switch to the much simpler php PDO extension for all the database statements. the amount of code you have to write to accomplish a prepared query using the mysqli extension is an error prone waste of time (and the code needed to write a mysqli general purpose prepared query function/method is complicated and slow.)

Edited by mac_gyver
20 hours ago, Barand said:

It's unusual to join the primary key of one table to the primary key of another. Do you only ever want one amount record for each address

 

I only need one record per month per unit. each unit could actually make multiple payments per month though. My plan is to output each month into a table and each month the data in that table would reset to zero to show the new payments for each unit for the current month. Then the database would hold all payments made over time. I would like to query the database to show units that have not paid, how much they owe and when the last payment was made and what months are missing.

You said it's unusual to join the primary key of one table to another. I guess I don't understand because that is how I thought it was done. What is the right way to do it as that is the way I would like it to be.

12 hours ago, mac_gyver said:

as to displaying/logging the database statement errors. use exceptions to handle all database statement errors (connection, query, prepare, and execute) and in most cases let php catch the exception where it will use its error_reporting, display_errors, and log errors_settings to control what happens with the actual error information. the exception to this rule is if your application needs to detect and handle duplicate values for an insert/update query. your code would catch the exception in this case, detect if the error was for a duplicate index, set up and output an error message to the user telling them what is wrong with the data they submitted, or if error is not for a duplicate index, re-throw the exception and let php handle it. when learning, developing, and debugging code/queries, you would display all errors, which now includes the database statement errors, and when on a live/public server, you would log all errors, just by changing the php display_errors and log_errors settings. this will allow you to remove the existing error handling logic in your code, thereby simplifying it. to enable exceptions for the php mysqli extension, add the following line of code before the point where you are making the database connection -  


mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);

lastly, since you need to change to use prepared queries when supplying external/unknown values to the sql query statement, you need to switch to the much simpler php PDO extension for all the database statements. the amount of code you have to write to accomplish a prepared query using the mysqli extension is an error prone waste of time (and the code needed to write a mysqli general purpose prepared query function/method is complicated and slow.)

I agree with you 100%. I have managed to create an html select that does display each unit from the database where the value of the select is the address table primary key. This is a home owners association and what I am trying to keep tack of is the HOA dues. Those dues pay the water, sewer, garbage, landscaping and electrical for the complex. There are 14 units. Each unit pays monthly. There is no good cheap software out there that does what I want so I was trying to make something myself but as you can tell, I don't know what I am doing. I made the database, inserted some records and have managed to display them in a table. The html form was at this point was just to get a working query that I could work off of and then make more correct with prepared statements at a later time. Once the basic structure was up and running. The update queries are not working. I can get it to tell me that the database has been updated but the new record doesn't actually show up. Maybe I should just scrap the project and go back to watching TV and playing games.

Store payments as individual transactions. When a payment is made, insert a new record in a payment table

Table: unit
+---------+--------------------+
| unit_id |   address          |
+---------+--------------------+
|     1   | 2 Effin Close      |
|     2   | 9 Letsby Avenue    |
+---------+--------------------+
      |
      |
      +---------------------------------+
                                        |
                    Table: payment      |
                    +------------+------------+------------+----------+
                    | payment_id |  unit_id   |  date_paid |   amount |
                    +------------+------------+------------+----------+
                    |       1    |      1     |  218-08-15 |   250.00 |
                    |       2    |      2     |  218-08-17 |   250.00 |
                    |       3    |      1     |  218-09-15 |   250.00 |
                    |       5    |      1     |  218-10-15 |   250.00 |
                    |       4    |      2     |  218-10-28 |   150.00 |
                    |       6    |      1     |  218-11-15 |   250.00 |
                    +------------+------------+------------+----------+

Now you can see what they have paid, and that Unit 2 missed payments in September and November and didn't pay the full amount in October.

Edited by Barand

Ok, I have remade my table to match what you have posted.I had to alter the tables in order to set up the foreign keys. Was that necessary?

ALTER TABLE unit ENGINE=InnoDB;

ALTER TABLE payment ENGINE=InnoDB;

I was then able to set up the relationships. I set the payment_id a the primary_key and the unit_id as the foreign key in the payment table. The unit_id is the unit table should be the primary key set to auto_increment. Is this good?

Capture.PNG.1e8089605a67b8b2424e740328605126.PNG

Edited by CyberShot

I would change the amount type

unit
+----------------+---------------------+-----------------+
| column         |       type          |     keys        |
+----------------+---------------------+-----------------+
|  unit_id       |  int                |  PK AI          |
|  address       |  varchar(20)        |                 |
+----------------+---------------------+-----------------+

payment
+----------------+---------------------+-----------------+
| column         |       type          |     keys        |
+----------------+---------------------+-----------------+
|  payment_id    |  int                |  PK AI          |
|  unit_id       |  int                |  FK             |
|  date_paid     |  date               |                 |
|  amount        |  decimal(10,2)      |                 |  monetary amounts are better with fixed precision instead of float
+----------------+---------------------+-----------------+

 

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.