verbb / snipcart

A Craft CMS plugin to integrate with Snipcart.
Other
21 stars 9 forks source link

Updates to "Shippable" and "Taxable" options in Product Details field don't save properly #43

Closed brettburwell closed 1 year ago

brettburwell commented 2 years ago

Describe the bug

When we update the value of the "Taxable" or "Shippable" lightswitch field on an existing product, the updated value isn't saved. Updating other values of the product details field or other fields on the entry save just fine.

Oddly, if I toggle the "Taxable" or "Shippable" value again, immediately after the first save, and quickly (within a few seconds) resave, sometimes the value is updated. But not always.

It seems to be successful if I re-save while Craft is still busy performing various queue tasks triggered by the initial re-save. But I'm not quite sure why that might be the case and can't find any clues in the logs that might provide an insight to what's happening.

To Reproduce

Steps to reproduce the behavior:

  1. Go to an existing product entry
  2. Toggle the "Taxable" or "Shippable" value
  3. Save the entry
  4. The entry saves and the page reloads, but the value isn't updated

Expected behavior

The "Taxable" or "Shippable" value should be updated.

Software

Additional context

This issue seemed to start popping up when we upgraded to Craft 3.7.20 and Snipcart 1.5.5 a few months back.

I've tried toggling the "Cache API response" config value off with no luck. Same with the "Log Custom Rates" and "Log Webhook Requests."

The site happens to be running Blitz and Async Queue. I've tried temporarily disabling those plugins to see if they might be the source of the underlying issue. There isn't anything else I'm aware of that might be conflicting with the plugin's behavior.

If there's any other information I can pass along please let me know.

brettburwell commented 2 years ago

Hi y'all. Just following up on this issue to see if you have any thoughts.

After letting this sit for a few weeks I came back with fresh eyes this morning to see if I might catch something I was previously missing, but unfortunately had no luck.

I disabled all of the other plugins on the site to make sure there wasn't an odd conflict happening and also upgraded to the latest version of Craft (3.7.35) and Snipcart (1.5.7), but am still seeing the same issue.

Here's a video of exactly what I'm seeing in case it's helpful. You can see that when I try to update the value of the taxable field, it just reverts as soon as the entry is saved.

https://user-images.githubusercontent.com/1228671/156396871-cc2625ff-8d38-4283-b35b-b5bc5f93dc3c.mp4

I realize odd issues like this can be a pain to track down, but we're not seeing this issue occur anywhere else in the CP.

If there's any other info I can share and/or if you have tips on anything I might try on my side please let me know.

sjcallender commented 2 years ago

Hey @brettburwell. I apologize, we'll get to this as soon as possible. Full transparency, our main two devs who worked on this plugin are no longer with us and we're in the middle of onboarding new team members. It's taken a bit longer than expected, but we're moving forward now. As soon as we have everyone settled in, we'll be able to jump back into active support.

brettburwell commented 2 years ago

Hi @sjcallender. Thanks for the update. It's much appreciated.

Best of luck with onboarding the new team members. We'll sit tight and look forward to hearing from y'all.

bleepsandblops commented 2 years ago

Hello there, currently having the same issue

brettburwell commented 2 years ago

Howdy @sjcallender. I hope the new devs are settling into place. Just checking in to see if there were any updates on this. The client is starting to get a little antsy.

And @bleepsandblops, thanks for chiming in. It's helpful to know I'm just not just losing my head over here.

brettburwell commented 2 years ago

@sjcallender Checking in again on this to see when the team might have a chance to take a closer look. This is currently blocking our client from being able to manage their products effectively, and we'd love to get this resolved in the near future if possible.

If there's any additional information I can provide from my end that'd be helpful please let me know.

sjcallender commented 2 years ago

Hey @brettburwell. It's on the schedule now for next week. Thanks for your patience. Onboarding takes time!

brettburwell commented 2 years ago

Thanks for the update @sjcallender. That's great news. If the team has any questions once they dig in please give me a shout.

peteeveleigh commented 2 years ago

Hey @brettburwell Firstly, apologies for the delay. I have got a temporary fix for this issue. It's not elegant but in my testing it seems to work reliably.

You can try it by changing your composer requirement to

"fostercommerce/craft-snipcart": "dev-develop as 1.5.7"

Then run composer update

That should get you using the fixed version.

brettburwell commented 2 years ago

Hi @fantasticmachine. Welcome to the Foster Commerce team and thanks for the followup and hotfix. I appreciate it!

peteeveleigh commented 2 years ago

No problem. Can you let me know how that works out for you?

I'm just starting on some other work which should also provide robust solution but I think the dirty fix should get you functional again.

brettburwell commented 2 years ago

Hi there @fantasticmachine. I had a chance to install the update and unfortunately am still seeing the same issue on our end in production. Just to be sure, I upgraded to the latest and greatest version of Craft too, but still no luck.

If there's any other info I can pass along let me know. But if you're actively working on a more robust fix we can also hang tight until that's ready. Thanks again for your help with this one.

peteeveleigh commented 2 years ago

@brettburwell So the problem I uncovered was that when saving an entry (or when the auto save kicks in) the data for the product details field is being saved. But it is also being retrieved faster than it is saved. The retrieval is faster than the save so you get the old data back in the form.

My dirty fix was just to make the retrieval process wait for a few seconds. Three to be precise but I guess this may not be enough if you have particularly busy entry that takes longer to save.

You could try adjusting the wait time and see if that gets it working for you.

In /src/services/Fields.php on line 64 change that sleep value so there's a longer wait.

I do appreciate this is not a proper fix. I'm currently still finding my way around the plugin and working on refactoring the process by which data is saved (as well as getting it compatible with Craft 4) - which should prevent this issue. I can't give an exact time frame at the moment but rest assured it is actively being dealt with.

brettburwell commented 2 years ago

@fantasticmachine Thanks for the explanation and pointing me to that line! Bumping that sleep value up a bit did the trick for now.

Best of luck getting up and running with the plugin. We'll be excited for the Craft 4 release whenever it's ready.

brettburwell commented 2 years ago

Just in case anyone else stumbles on this thread in the future.

Above, it's suggested to update your composer.json file with: "fostercommerce/craft-snipcart": "dev-develop as 1.5.7"

But as of this post, the hotfix hasn't yet been merged into the develop branch, so for now here's what your updated composer file should look like: "fostercommerce/craft-snipcart": "dev-dev-bugifx/issue-43-tax-and-shippable-settings-not-sticking as 1.5.7"

Then run composer update and you should be ready to rock!

peteeveleigh commented 1 year ago

An update on this. There isn't really much else that we can do with regards this "race condition" problem other than the hacky fix provided here.

However, we have a new version where the data is stored differently and this shouldn't have the same problems. We need to finish writing the migrations to get the old format data into the new format for anyone who upgrades the plugin. This is taking a bit of time to figure out but is currently in progress.

The update will be for the Craft 4 version of the plugin. If possible, it will be rolled back into the Craft 3 version too.