Error when confirming delete customer

Working on a CLONE of an existing site with a view to creating a "duplicate" webshop. Version 1.5.5d (cloned, not fresh install) Both dbase and folders/files copied. configure.php "edited" to reflect new server location, etc.

Now we want to DELETE the customers and orders from the database of this "clone", as these records will not be relevant when the cloned site is ready for going live.

So we access the the customer "list" in admin panel. Select a customer, then DELETE button. (Get asked if we wish to confirm), we then confirm... then white-screen with error.

Looking in LOG files we see:

[07-Jan-2017 16:25:22 Europe/London] Request URI: /OUR-ADMIN-FOLDER/customers.php?page=1&action=deleteconfirm, IP address: 000.000.000.000
#1  trigger_error() called at [/home/CLONE-SITE/public_html/includes/classes/db/mysql/query_factory.php:167]
#2  queryFactory->show_error() called at [/home/CLONE-SITE/public_html/includes/classes/db/mysql/query_factory.php:139]
#3  queryFactory->set_error() called at [/home/CLONE-SITE/public_html/includes/classes/db/mysql/query_factory.php:266]
#4  queryFactory->Execute() called at [/home/CLONE-SITE/public_html/twiSt-CJT-brAwn/customers.php:404]

[07-Jan-2017 16:25:22 Europe/London] PHP Fatal error:  1064:You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 1 :: delete from products_notifications where customers_id =  ==> (as called by) /home/CLONE-SITE/public_html/OUR-ADMIN-FOLDER/customers.php on line 404 <== in /home/CLONE-SITE/public_html/includes/classes/db/mysql/query_factory.php on line 167

Have uploaded new files (customers.php AND query_factory.php) to respective locations, and looked at Dbase table for product_notifications. All appears to be OK... but error persists.

Ah, it's the little things in life that can really mess stuff up!

See your /admin/customers.php, starting at line 401:

        $db->Execute("delete from " . TABLE_WHOS_ONLINE . "
                      where customer_id = '" . (int)$customers_id . "'");

    	$db->Execute("delete from " . TABLE_PRODUCTS_NOTIFICATIONS . " where customers_id = " . $customer_id);

See how the operation on TABLE_WHOS_ONLINE uses the variable $customer**s**_id? See how the operation on TABLE_PRODUCTS_NOTIFICATIONS doesn't? That's the issue, a stupid missing s.
To correct:

        $db->Execute("delete from " . TABLE_WHOS_ONLINE . "
                      where customer_id = '" . (int)$customers_id . "'");

    	$db->Execute("delete from " . TABLE_PRODUCTS_NOTIFICATIONS . " where customers_id = " . $customer[B]s[/B]_id);

lat9:

See your /admin/customers.php, starting at line 401:

. $customer[B]s[/B]_id);

:cool:

Thanks... My head is spinning with all this code stuff. This is off a clean download of ZC 1.5.5d

The corresponding lines (from 401) in 1.5.5b are:

$db->Execute("delete from " . TABLE_CUSTOMERS_BASKET_ATTRIBUTES . "
where customers_id = '" . (int)$customers_id . "'");

    $db->Execute("delete from " . TABLE_WHOS_ONLINE . "
                  where customer_id = '" . (int)$customers_id . "'");

    zen_record_admin_activity('Customer with customer ID ' . (int)$customers_id . ' deleted.', 'warning');

And in 1.5.5d

$db->Execute("delete from " . TABLE_CUSTOMERS_BASKET_ATTRIBUTES . "
where customers_id = '" . (int)$customers_id . "'");

    $db->Execute("delete from " . TABLE_WHOS_ONLINE . "
                  where customer_id = '" . (int)$customers_id . "'");

	$db->Execute("delete from " . TABLE_PRODUCTS_NOTIFICATIONS . " where customers_id = " . $customer_id);

    zen_record_admin_activity('Customer with customer ID ' . (int)$customers_id . ' deleted.', 'warning');

So the line:-

	$db->Execute("delete from " . TABLE_PRODUCTS_NOTIFICATIONS . " where customers_id = " . $customer_id);

Case for a bug report?

Actually it was added in 1.5.5c and has remained the same since then.

Not sure if the error is still present but I did notice the following.

schoolboy:

:cool:

And in 1.5.5d

$db->Execute("delete from " . TABLE_CUSTOMERS_BASKET_ATTRIBUTES . "
where customers_id = '" . (int)$customers_id . "'");

    $db->Execute("delete from " . TABLE_WHOS_ONLINE . "

where customer_id = '" . (int)$customers_id . "'"); <- doesn't where customer_id also require an 's'

    $db->Execute("delete from " . TABLE_PRODUCTS_NOTIFICATIONS . " where customers_id = " . $customer_id);

    zen_record_admin_activity('Customer with customer ID ' . (int)$customers_id . ' deleted.', 'warning');

Website Rob:

Not sure if the error is still present but I did notice the following.

The line identified above referring to the whos_online data table field of customer_id is correct as one can see in the install sql for the table whos_online:

CREATE TABLE whos_online (
  customer_id int(11) default NULL,
  full_name varchar(64) NOT NULL default '',
  session_id varchar(255) NOT NULL default '',
  ip_address varchar(45) NOT NULL default '',

I mean, maybe someone would have preferred that the field customer_id had the s in it, but what schoolboy has identified is that the result on the right side of the added admin/customers.php query is more than likely to evaluate to 0 and therefore is not expected to perform the deletion that was intended.

Website Rob:

Not sure if the error is still present but I did notice the following.

In some tables, the field is named customer_id and in others it is customers_id . I also thought that it needed "s" - but the field in that table is without the "s". Maybe there's a reason for the difference...

schoolboy:

... Case for a bug report?
Yes, that's why I noted my post with that little bug icon!

The issue has absolutely nothing to do with whether a database table uses customer_id or customers_id as its customer-index; the issue is that the line that was added to also remove the customer's records in the products_notifications table ... is using the wrong ***PHP ***variable.

The variable $customers_id (with the s) exists and identifies which customer is currently being removed, while the variable $customer_id (s-less) doesn't exist. That's where the SQL issue comes from.

lat9:

The issue has absolutely nothing to do with whether a database table uses customer_id or customers_id as its customer-index;
Yes... that's correct, of course... what matters is that an execute command in any PHP needs to reference the exact table and field labels. They could be called anything - even "fried_fish_cakes" :D will do. As long as it all matches up.

My point is more about field-labelling "consistency". Given that in many instances, in the tables, there needs to be relational cross-referencing, and indeed cross-writing of data - so where this relational functionality is evident, field labels ought to be consistently named (surely?)

I'm just inquisitive about why a common field in the tables (customers_id) is now labelled without the "s" in a particular table? Is there a logical (functional) reason for a slightly different label?

I don't know if there is any relational activity in writing to the products_notifications table for this particular field... perhaps I'm just splitting hairs...:lookaroun

schoolboy:

My point is more about field-labelling "consistency". Given that in many instances, in the tables, there needs to be relational cross-referencing, and indeed cross-writing of data - so where this relational functionality is evident, field labels ought to be consistently named (surely?)

I'm just inquisitive about why a common field in the tables (customers_id) is now labelled without the "s" in a particular table? Is there a logical (functional) reason for a slightly different label?I couldn't agree more.
But the pain of renaming all the fields in almost a hundred tables is an upgraders nightmare! We debate it during every other upgrade release.
We've considered that for v2. But in reality, if we provide proper functions to allow all those things to happen thru function calls, then (almost) nobody should be writing their own queries anyway, thus minimizing the risk of the pain impacting many people other than perhaps mod-developers until they switch over to using the (some yet to be written) built-in functions.

schoolboy:

I don't know if there is any relational activity in writing to the products_notifications table for this particular field
When deleting a customer, it's prudent to also delete all the notifications they requested to be alerted to ... unless you want to keep that orphaned info for posterity?

Apologies for the typo, and corresponding bug. Thanks for reporting the bugfix. We'll build in

DrByte:

I couldn't agree more.
But the pain of renaming all the fields in almost a hundred tables is an upgraders nightmare! We debate it during every other upgrade release.
We've considered that for v2. But in reality, if we provide proper functions to allow all those things to happen thru function calls, then (almost) nobody should be writing their own queries anyway, thus minimizing the risk of the pain impacting many people other than perhaps mod-developers until they switch over to using the (some yet to be written) built-in functions.

Best to bite the bullet and "geter done!" This also has to happen with the CSS code and whether everything happens quickly or at least as fast as a turtle can run, many hands make the work load lighter. :hug:

With hindsight for guidance and advancements in modular code, shouldn't take as long as one might think. Especially if we all bite the bullet and realize at some point, legacy code needs to be left behind and cannot be used for the newer, more easier to work with coding which will be better for all.

Website Rob:

Best to bite the bullet and "geter done!" This also has to happen with the CSS code and whether everything happens quickly or at least as fast as a turtle can run, many hands make the work load lighter. :hug:

With hindsight for guidance and advancements in modular code, shouldn't take as long as one might think. Especially if we all bite the bullet and realize at some point, legacy code needs to be left behind and cannot be used for the newer, more easier to work with coding which will be better for all.

having dealt with (and continuing to deal with) my share of legacy code, it is much easier to be on the outside looking in, and telling the dev team what to do, then to actually do it.

once named, changing variables down the line presents all kinds of problems; lets say the dev team makes the change. what then happens to all of the plugins modifications one has done to their implementation of ZC? what about those plugins that do not have any person(s) maintaining the code? and frankly is it worth the dev team to focus on this as opposed to other features?

        $db->Execute("delete from " . TABLE_WHOS_ONLINE . "
                      where customer_id = '" . (int)$customers_id . "'");

    	$db->Execute("delete from " . TABLE_PRODUCTS_NOTIFICATIONS . " where customers_id = " . $customer[B]s[/B]_id);
```\[/QUOTE]

This one > where customers_id = '" . (int)$customers_id . "'"); is working for me too. I saw this tread afterwards. Which one is better?

stefanb:

    $db->Execute("delete from " . TABLE_WHOS_ONLINE . "
                  where customer_id = '" . (int)$customers_id . "'");

	$db->Execute("delete from " . TABLE_PRODUCTS_NOTIFICATIONS . " where customers_id = " . $customer[B]s[/B]_id);

This one  is working for me too. I saw this tread afterwards. Which one is better?\[/QUOTE]
Casting the value to int is better (more secure) though the single quotes around that value are unnecessary and requires the sql database to convert the now string into an integer (in a sense double casting by variable to integer inserted into sql as a string that is then converted to a number/integer from there), so this would be the better of the two:

where customers_id = " . (int)$customers_id);