Error when confirming delete customer

[schoolboy]

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.

[lat9]

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 $customers_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 = " . $customers_id);

[schoolboy]

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

. $customers_id);

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);

Seems to have been added in for 1.5.5d

Case for a bug report?

[mc12345678]

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

[Website Rob]

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

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');

[mc12345678]

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.

[schoolboy]

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...

[lat9]

... 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.

[schoolboy]

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" 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...

[DrByte]

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.

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