reverbdotcom / reverb-magento

Magento 1.x plugin for syncing with Reverb
Other
7 stars 10 forks source link

Orders are created on update even though we set the setting to sync only awaiting_shipment #266

Closed skwp closed 7 years ago

skwp commented 7 years ago

I believe we may have introduced a bug in https://github.com/reverbdotcom/reverb-magento/pull/256 because we are now creating orders when they come in on an update.

Here's the bug

  1. User has set his orders to sync only the awaiting_shipment endpoint. Order creation syncs are going against that endpoint. However order update syncs are going against the orders/all endpoint.
  2. The intention is that if an order is not yet paid, it should not sync, but the update is now creating the order.
  3. What we really want is if the user has the setting to sync only awaiting_shipment orders that we don't ever create an order during an update unless it's paid. This is a little complicated because we have to know which statuses should not create orders: https://dev.reverb.com/docs/syncing-orders#list-of-order-statuses

These statuses: unpaid/pending_review/blocked should not cause order creation if the user has set his sync to be only for orders Awaiting Shipment

How to fix

I believe the correct algorithm would be:

IF the user has "Paid Orders Awaiting Shipment" setting

  1. on update, poll the endpoint (still using the "all" endpoint as we do want all updates)
  2. if the status on an order is unpaid/pending_review/blocked, do NOT create the order
  3. otherwise create the order

IF the user has the All Orders setting,

  1. on update, create the order regardless of status
skwp commented 7 years ago

I believe the correct algorithm would be:

IF the user has "Paid Orders Awaiting Shipment" setting

  1. on update, poll the endpoint
  2. if the status on an order is unpaid/pending_review/blocked, do NOT create the order
  3. otherwise create the order

IF the user has the All Orders setting,

  1. on update, create the order regardless of status
proaudio commented 7 years ago

Yes we have this same issue after most recent update. Set to "Paid Orders Awaiting Shipment" still downloading unpaid pending orders. Do you have a fix for this?

skwp commented 7 years ago

We are working on a fix, when we have one we'll close this issue and make a release. Thanks.

dunagan5887 commented 7 years ago

@skwp For this update, I am assuming that if the user has "Paid Orders Awaiting Shipment" setting, and an order is "unpaid", the task should be set to "Complete" or "Aborted", and should not be executed again, correct? Once the order becomes paid, a new Order update will be created in the Reverb system which the Magento system will retrieve via the API, correct?

skwp commented 7 years ago

Yes once the order is paid it will be processed during update. The task should be set to Complete unless we have a way to mark it ignored which is a bit more clear.

dunagan5887 commented 7 years ago

@skwp In your first algorithm above, you noted

on update, poll the endpoint (still using the "all" endpoint as we do want all updates)

However, one of the specifications of the system is that if the user has set the "Which orders should sync from Reverb?" setting to "Paid Orders Awaiting Shipment", then the endpoint is NOT the all endpoint, but is rather the endpoint given below

/api/my/orders/selling/awaiting_shipment?updated_start_date=%s

Should this still be the case? Or should we refactor the system to poll the "All" endpoint regardless of that setting? The "All" endpoint is given below:

/api/my/orders/selling/all?created_start_date=%s

skwp commented 7 years ago

So the problem is:

  1. We want all order updates meaning all updates should come from /all. If we set updates to come from /awaiting_shipment we will never have any state updates like cancelled/refunded
  2. When an order is being updated from the /all endpoint and the user has the "Paid Orders Awaiting Shipment" setting, we should only create the order when the state ==paid
dunagan5887 commented 7 years ago

Got it

skwp commented 7 years ago

Fixed in #269