@Slowlearneruk opened this Issue on June 24th 2016

Originally posted here: https://forum.piwik.org/t/addecommerceitem-not-added-to-subsequent-order/19922

Hi there, I have some JavaScript code that is looping through an order and calling the addEcommerceItem method. Having run through three times it is calling the trackEcommerceOrder method however in subsequent call I am not seeing all of the items in the order. Worse still, the items are getting added to the next order whose items are cascading again into follow orders.

Is there something to bind items to orders as in Google Analytics where the order number is used to group items? Is this a bug in the tracker code?

Edit:
I've amended the code this morning to switch from using a tracker object to just pushing the requests onto the _paq object and I am seeing the same behaviour, my code has pushed three items and then an order but in the data sent to the piwik server there are only two of the items. Placing a second order for one item sends data containing the first two items plus the third item:

First order:
ec_items:[["item3id","item3","item3cat",3,1],["item1id","item1","item1cat",1,1]]

Second order:

ec_items:[["item3id","item3","item3cat",3,1],["item1id","item1","item1cat",1,1],["item4id","item4","item4cat",0.5,1]]

This seems to be thoroughly broken.

Edit

OK, I have tracked down the problem - the items added are keyed on the product SKU which means that if you add two items with the same SKU then the second overwrites the first. This means that we are obliged to first count all of the items and add them with a single entry and updated count. In my view this is poor design. The API should say that these need to be unique for the order - in our case items of a specific type can have variable pricing and we are accumulating them from different lists making the interface cumbersome and dependent on a preprocessing step to build the entries.

I would recommend replacing the code that keys on sku with an array of entries that records all items added rather than silently replacing earlier added items. As a minimum I would suggest that you at least generate some console output when you overwrite an earlier entry.

Edit

The explanation for seeing the items on subsequent orders is simple enough - the ecommerceItems object is never cleared down after the order is placed - I find it hard to see how this was missed in even rudimentary testing.

I've implemented a fix in my code and I'm not clear how you fold in changes from the community, maybe someone could let me know. Here are the changes:

diff --git a/js/piwik.js b/js/piwik.js
index bed97aa..c2a5ef8 100644
--- a/js/piwik.js
+++ b/js/piwik.js
@@ -2882,7 +2882,7 @@ if (typeof window.Piwik !== 'object') {
                 customVariableMaximumLength = 200,

                 // Ecommerce items
-                ecommerceItems = {},
+                ecommerceItems = [],

                 // Browser features via client-side data collection
                 browserFeatures = {},
@@ -3996,36 +3996,19 @@ if (typeof window.Piwik !== 'object') {
                     request += '&ec_dt=' + discount;
                 }

-                if (ecommerceItems) {
-                    // Removing the SKU index in the array before JSON encoding
-                    for (sku in ecommerceItems) {
-                        if (Object.prototype.hasOwnProperty.call(ecommerceItems, sku)) {
-                            // Ensure name and category default to healthy value
-                            if (!isDefined(ecommerceItems[sku][1])) {
-                                ecommerceItems[sku][1] = "";
+                if (ecommerceItems.length) {
+                    ecommerceItems.forEach(function(ecommerceItem) {
+                        // Ensure healthy default values where undefined
+                        [undefined, "", "", 0, 1].forEach(function(sDefault, nIndex) {
+                            if (sDefault !== undefined && ecommerceItem[nIndex] === undefined) {
+                                ecommerceItem[nIndex] = sDefault;
                             }
-
-                            if (!isDefined(ecommerceItems[sku][2])) {
-                                ecommerceItems[sku][2] = "";
-                            }
-
-                            // Set price to zero
-                            if (!isDefined(ecommerceItems[sku][3])
-                                    || String(ecommerceItems[sku][3]).length === 0) {
-                                ecommerceItems[sku][3] = 0;
-                            }
-
-                            // Set quantity to 1
-                            if (!isDefined(ecommerceItems[sku][4])
-                                    || String(ecommerceItems[sku][4]).length === 0) {
-                                ecommerceItems[sku][4] = 1;
-                            }
-
-                            items.push(ecommerceItems[sku]);
-                        }
-                    }
-                    request += '&ec_items=' + encodeWrapper(JSON2.stringify(items));
+                        });
+                    });
+                    request += '&ec_items=' + encodeWrapper(JSON2.stringify(ecommerceItems));
                 }
+                // Clear down items after logging
+                ecommerceItems = [];
                 request = getRequest(request, configCustomData, 'ecommerce', lastEcommerceOrderTs);
                 sendRequest(request, configTrackerPause);
             }
@@ -6329,7 +6312,7 @@ if (typeof window.Piwik !== 'object') {
                  */
                 addEcommerceItem: function (sku, name, category, price, quantity) {
                     if (sku.length) {
-                        ecommerceItems[sku] = [ sku, name, category, price, quantity ];
+                        ecommerceItems.push([ sku, name, category, price, quantity ]);
                     }
                 },
@tsteur commented on June 25th 2016 Owner

@mattab can you maybe have a look? I'm not really sure how it is supposed to work

@mattab commented on July 8th 2016 Owner

Hi @Slowlearneruk - please check this Pull request and patch which should fix the issue. #10279

could you confirm issue is fixed with this?

@mattab commented on July 12th 2016 Owner

Hi @Slowlearneruk looking forward to your feedback.

@Slowlearneruk commented on July 12th 2016

Hi there, sorry to dally - paid work gets in the way sometimes. I have the forked implementation in place and being tested at the moment. It is only different in that it uses an array to store the items rather than a map keyed with the SKU. As far as I am aware the Piwik server does not have a problem with multiple items with the same SKU in an order - it is certainly not complaining however I haven't yet checked the data thoroughly.

Can I just present the case for the change before we write it off completely: The application I have written is a gambling game and the transactions that a user might make are individual bets. Consequently the price that they pay will vary according to what they want to gamble and there is nothing to stop them making the same bet twice over (with different stakes) if they wish. It therefore makes sense to record the individual bets as separate transactions otherwise I would lose information.

Consider as an alternative a site where the price of an item might vary (auction sites for instance) but it's SKU remains the same, one would want to represent separate purchases at different prices individually right?

I'll try and make a closer examination of the data this afternoon and confirm that it is being recorded correctly.

@mattab commented on July 12th 2016 Owner

Definitely you are right about your use case being common, thanks for the use case. I've created another issue to it: Ecommerce: allow to track the same product SKU several times in an ecommerce order #10290

Otherwise the initial smaller issue addEcommerceItem not added to subsequent order will be fixed with https://github.com/piwik/piwik/pull/10279

This Issue was closed on July 12th 2016
Powered by GitHub Issue Mirror