FosterCommerce / klaviyoconnect

A plugin for Craft CMS. Grow your ecommerce business with smarter email automations.
https://klaviyoconnect.fostercommerce.com/
Other
10 stars 10 forks source link

Undefined array key "Slug" #104

Closed totov closed 1 year ago

totov commented 1 year ago

Hi, a customer of mine is getting this error for some "Syncing orders to Klaviyo" jobs in their queue:

Screenshot 2023-02-08 at 16 32 34

I believe it references this line:

https://github.com/FosterCommerce/klaviyoconnect/blob/8a862ae577d2da1e2731452c68acdfd548b1e542/src/services/Track.php#L317

Can you let me know if this is a bug or something I need to update somewhere?

peteeveleigh commented 1 year ago

Hi @totov I've got this in my schedule this week. Will get it fixed asap.

peteeveleigh commented 1 year ago

@totov Would it be possible to share the contents of some of the orders where this error occurs?

totov commented 1 year ago

@fantasticmachine after reviewing the order mentioned in the error above, I can see that one of the product variants (but not the product itself) has been deleted from the CMS, which would perhaps explain the missing array key for Slug? Happy to send you any more info, just let me know what you need.

peteeveleigh commented 1 year ago

@totov Yes that would probably do it. Although, strictly speaking, the plugin should be working from the order snapshot rather than the "live" items. Then it wouldn't be trying to update with a missing slug.

I think I may need to just put a sticking plaster in place and check for the presence of the item before trying to do something with it. At least that will suppress the error you are seeing. I can then dig deeper and figure out what's really going on.

peteeveleigh commented 1 year ago

@totov Can you tell me which version of the plugin you are using?

totov commented 1 year ago

@fantasticmachine apologies for the delayed response - it's version 4.0.15

peteeveleigh commented 1 year ago

@totov thanks... I'll be looking at this again either tomorrow or on Monday

peteeveleigh commented 1 year ago

@totov I've had another go with this using 4.0.15 and still cannot reproduce the problem. But that may be because I don't exactly know the steps you're going through when you see it.

Would you be able to write out a series of instructions to reproduce the error or record a video?

totov commented 1 year ago

@fantasticmachine I think here:

https://github.com/FosterCommerce/klaviyoconnect/blob/8a862ae577d2da1e2731452c68acdfd548b1e542/src/services/Track.php#L358

You're accessing the purchasable directly on the $lineItem rather than on $lineItem->snapshot? Then on the following line, because $product has defaulted to [] it fails the if statement and therefore has no properties, such as Slug, when accessed later on in line 317.

https://docs.craftcms.com/commerce/api/v2/craft-commerce-models-lineitem.html#snapshot

This is how it looks in the order (note the first product/variant has been deleted, so no link to it):

Screenshot 2023-03-14 at 12 37 35

I've checked the snapshots for both line items on the order and they seem to have the information the plugin is looking for (in the case of this error, the product's slug), so I think it may just be the plugin accessing the live purchasable/product rather than the snapshot?

I don't have a fresh install to attempt this with, but I think you can replicate this by doing the following:

  1. Add a product with multiple variants
  2. Complete an order including a line item for one of the variants
  3. Delete the variant from Craft Commerce
  4. Try to use the Send Orders to Klaviyo function under Utilities for a period covering the order

Hope that helps, but let me know if you need more details. Happy to share the database or snapshots of the line items over email if necessary.

peteeveleigh commented 1 year ago

@totov hmmm. I am still unable to reproduce any kind of problem here.

I created an order with 2 line items. Completed the order Deleted one of the variants that populated one of the line items Used the Send Orders to Klaviyo function

Nothing changed with regards to the order in question. No queue tasks failed.

I performed another test, keeping an eye on the queue. It seems that the order updates are sent immediately the change is made. I didn't see any errors appear in the queue.

I even tried deleting the product entirely. Still no queue errors. Although at this point I noticed something odd which seems to be more of a Commerce issue than one with the plugin or Klaviyo - I don't believe it is related to the issue you are having.

I'm not sure where to go from here.

To confirm, I am testing with: Craft 3.8.1 Commerce 3.4.20 Klaviyo Connect 4.0.15

totov commented 1 year ago

@fantasticmachine when you deleted the product, did you delete it from the Trashed section too? Other than that I'm a bit stumped too!

peteeveleigh commented 1 year ago

@totov Ahh! No I didn't. I will test that again

peteeveleigh commented 1 year ago

@totov Even after deleting the product from Trashed it still doesn't show any issues in the queue and the data is sent to Klaviyo.

What I am noticing is a problem in Commerce. If you have 2 different items in your order and you delete the products they relate to, then they get combined into a single product with a quantity of 2. If the prices of those line items was different then the order total is no longer correct. That seems like a Commerce bug though. And the information is still sent to Klaviyo, it's just now showing the wrong price. This only seems to be a problem in Commerce 3.

totov commented 1 year ago

@fantasticmachine Ah well, think this one has truly stumped both of us...

If there's anything I can send you over email to help you debug then let me know, but other than that I'm happy if you want to close this ticket off (this issue only happens for one order from the middle of last year so isn't stopping my customer from using the plugin going forward).

peteeveleigh commented 1 year ago

I'm feeling it's something peculiar to that one order so would be good to figure out exactly what is causing it. We haven't had any reports of this from anyone else and I've so far failed to reproduce it.

@totov It might be useful to have a copy of the database so we can figure out what's happening but there are privacy issues around that. Are you able to reproduce it at will with another order?

totov commented 1 year ago

@fantasticmachine I hadn't tried until today but I can replicate the issue by following these steps:

  1. Navigate to any completed order where a purchased product has variants enabled
  2. Delete the specific variant used within a lineitem (and in my case, because it was the only variant present, I created another variant and marked it as the default)
  3. Try to use the "Send Orders to Klaviyo" function from within Utilities for a date covering the order (one thing worth noting is that if I only use the specific date of the order then it doesn't pick up the order – I need to select the day before to the day after in the calendar popup)

When following the above, I added this line to the Track.php file mentioned above after line 358:

Craft::debug($product, 'klaviyo');

And it spits out [], which means it then skips the next code block wrapped in the if statement, which is why Slug is not set later on.

PHP 8.0 if that helps too!

peteeveleigh commented 1 year ago

@totov that is helpful. I'll have another go at replicating and put some kind of mitigation in place

peteeveleigh commented 1 year ago

@totov OK I have managed to reproduce the issue!

peteeveleigh commented 1 year ago

It appears that the underlying issue is the way that the plugin is retrieving the line item details from the order. It's not just the slug that is missing, the entire "item" is missing so when it's building the data to send to Klaviyo it is using an empty Item. The failure is occuring when it tries to build an Event Id to pass to Klaviyo

peteeveleigh commented 1 year ago

@totov I've just release 4.0.16 which should fix the problem (albeit somewhat messily). Give that a whirl and see if it now works for you.

Looking at the code, I can see it could use a refactor but I'll probably save that for the Craft 4 version.

totov commented 1 year ago

Thanks @fantasticmachine, will do

totov commented 1 year ago

Just gave it a spin and it works, thank you 🙌

totov commented 1 year ago

@fantasticmachine I'm getting a new error related to the snapshot that's now being used from these changes:

TypeError: Cannot access offset of type string on string
#20 craft/vendor/fostercommerce/klaviyoconnect/src/services/Track.php(368): fostercommerce\klaviyoconnect\services\Track::getOrderDetails
#19 craft/vendor/fostercommerce/klaviyoconnect/src/services/Track.php(268): fostercommerce\klaviyoconnect\services\Track::trackOrder
#18 craft/vendor/fostercommerce/klaviyoconnect/src/services/Track.php(130): fostercommerce\klaviyoconnect\services\Track::onCartUpdated
#17 craft/vendor/fostercommerce/klaviyoconnect/src/Plugin.php(81): fostercommerce\klaviyoconnect\Plugin::fostercommerce\klaviyoconnect\{closure}
#16 [internal](0): call_user_func
#15 craft/vendor/yiisoft/yii2/base/Event.php(312): yii\base\Event::trigger
#14 craft/vendor/yiisoft/yii2/base/Component.php(642): yii\base\Component::trigger
#13 craft/vendor/craftcms/cms/src/base/Element.php(4203): craft\base\Element::afterSave
#12 craft/vendor/craftcms/commerce/src/elements/Order.php(2099): craft\commerce\elements\Order::afterSave
#11 craft/vendor/craftcms/cms/src/services/Elements.php(2732): craft\services\Elements::_saveElementInternal
#10 craft/vendor/craftcms/cms/src/services/Elements.php(785): craft\services\Elements::saveElement
#9 craft/vendor/craftcms/commerce/src/controllers/OrdersController.php(211): craft\commerce\controllers\OrdersController::actionSave

This is only when the order is updated via the CMS by the admin (guessing the snapshot is just a JSON string instead of an array for some reason during this?).

peteeveleigh commented 1 year ago

@totov Yes the lineitem snapshot is a json object.

Can you tell me what was being done when you say the admin was updating the order?

totov commented 1 year ago

It was my customer whose actions resulted in the error - I only got notified of the error via Sentry, but I'll try to replicate it tomorrow. I'm guessing maybe they updated the status of the order via the CMS or similar. Not sure why the snapshot wouldn't be unserialized at that point though 🤷‍♂️

peteeveleigh commented 1 year ago

@totov hmm yeah, let me know what happens. I'm pretty sure I tried updating an order after making my changes. I can't think why it would cause a problem either way.

totov commented 1 year ago

@fantasticmachine Here's the explanation from the customer:

We got an order on the website and when I try to change its status from processing to shipped an "Internal Server Error" message comes up.

peteeveleigh commented 1 year ago

@totov I just cut another release that prevents this error being thrown. I'm looking at getting this part of the plugin refactored as I suspect it's not quite right.