Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Canceling the orders that haven't been paid. #11257

Closed
3 tasks
Tracked by #11550
korycins opened this issue Nov 21, 2022 · 7 comments · Fixed by #11960
Closed
3 tasks
Tracked by #11550

Canceling the orders that haven't been paid. #11257

korycins opened this issue Nov 21, 2022 · 7 comments · Fixed by #11960
Assignees
Labels
rfc Request For Comments: blueprints of upcoming changes.

Comments

@korycins
Copy link
Member

korycins commented Nov 21, 2022

What I'm trying to achieve

With new payment's transaction flow, there is a possibility to create an order then paid for it. If customer cancels the payment process, we will end up with orders that haven't been paid and allocated stock. We need to have a mechanism to release stock and cancel the orders.

General assumptions

  • The OrderSettings in Channel type should include a new field expireOrdersAfter of type Minute.
  • The channelCreate and channelUpdate mutations should include a new input field expireOrdersAfter of type Minute.
  • The Channel model should have a new field expire_orders_after to store the expiration delta. The default value for this field should be None, which means do not expire any orders.
  • Only users with MANAGE_ORDERS or MANAGE_CHANNELS permissions should be able to update OrderSettings (Keep in mind that MANAGE_ORDERS can be used only to update orderSettings. That means, that user cannot use MANAGE_ORDERS permission on mutation that will update OrderSettings and other Channel field).
  • When creating an order from checkout, Saleor should create an unconfirmed order, and only confirm it when a transaction is attached to the order (but only when auto-confirm is active).
  • When creating a transaction for an order, Saleor should mark the order as confirmed (but only when auto-confirm is active).
  • The expired order should be treated in the same way as a canceled order, that means:
    • any update action on expired order should not be allowed
    • cannot generate invoice for expired_order
    • orders_total from Order should not include expired order
    • expired order cannot be captured or confirmed
    • reporting product sales should not contain expired orders
  • New celery task that will be triggered by celery_beat to convert unconfirmed and not paid orders into expired orders.
  • expired orders will have their stock released and vouchers will be available again.
  • When order is being expired, we will trigger the order_expired and order_updated event.
  • New CeleryTask object should be added as a lock for celery tasks.
  • The Order.status field new value: EXPIRED.

Describe a proposed solution

The proposed solution relays on the changes proposed in #11337.
Staff user will have a possibility to provide the time, that Saleor will use to change the unconfirmed and not paid orders into expired orders. We will release a stock allocation for these orders

API changes

OrderSettings type
type OrderSettings {
	"""
	Expiration time in minutes. Default null - means do not expire any orders
	"""
	expireOrdersAfter: Minute <New scalar to add>
}
Mutation

The channelCreate and channelUpdate mutations will get new input fields:

input OrderSettingsInput {
  ...
  expireOrdersAfter: Minute
}

Permission MANAGE_ORDERS or MANAGE_CHANNELS will be required to update orderSettings.

Note
Keep in mind that MANAGE_ORDERS can be used only to update orderSettings. The rest of the fields will still require MANAGE_CHANNELS permission.

orderCreateFromCheckout - will create unconfirmed order, Saleor will confirm it when transaction will be attached to the order (but only when auto-confirm is active).

transactionCreate - calling for order, should also mark order as confirmed (but only when auto-confirm is active).

Changes in the core logic

Order

The expired order should be treated in the same way as cancel order. Any update action on it should not be allowed. (including invoice generate which is available on cancel for unknown reason).

There are voucher that can be used only once. Such voucher must set to be available after cancelling the order

Celery task

We will have a celery task that will be triggered by celery_beat. The main job of this task will be taking a batch of unconfirmed and not paid orders and converting them to expired orders. The stock allocation will be released.

We want to use batches as we have in delete_expired_checkouts task. We want to be sure that we’re executing only one task at the same time. We will add the model CeleryTask based on https://github.com/saleor/saleor/blob/main/saleor/core/models.py#L115.
We will treat the CeleryTask object as our lock. Something similar to the celery solution described in cookbook: https://docs.celeryq.dev/en/latest/tutorials/task-cookbook.html#ensuring-a-task-is-only-executed-one-at-a-time. (linking only as an example, we should use DB model in our case).
The CeleryTask model should have a field for task name, and we should use it to find an ongoing task.
We attach a soft limit to the task (https://docs.celeryq.dev/en/stable/userguide/workers.html#time-limits), set it to 30 minutes, and remove the CeleryTask object in case of raised exception, same for expectedly finishing the task.
In case having CeleryTask with created_at < time.now() - 1 hour, we will call log.error inside the task.
(Reference: Internal link with discussion: https://saleorcommerce.slack.com/archives/C03E6UHTJ6T/p1669801908944829)

DB changes

The Order.status field will accept new value: EXPIRED.
The Channel model will recieve a new field for storing expiration delta: expire_orders_after. Default set to None - None means do not expire any orders

Webhooks

We will need to have a new event order_expired, that will be triggered when order is marked as expired.

Additional actions:

  • Sync work with design & dashboard teams, to provide UI for this feature
  • Update the docs, to explain the expiration feature
  • Create a GH issue to convert reserveStockDurationAnonymousUser from int to Minute
@korycins korycins added in progress rfc Request For Comments: blueprints of upcoming changes. labels Nov 21, 2022
@korycins korycins assigned korycins and unassigned korycins Nov 21, 2022
@IKarbowiak
Copy link
Member

Regarding this

input ChannelUpdateInput {
  ...
  expireOrdersAfter: Second
}

As you described in #11337 we'll have orderSettings: OrderSettingsInput input in ChannelUpdateInput, so we shouldn't extend ChannelUpdateInput with expireOrdersAfter.

@IKarbowiak
Copy link
Member

IKarbowiak commented Nov 30, 2022

The main job of this task will be taking a batch of unconfirmed and not paid orders and converting them to expired orders.

It seems like it requires adding some new field to Order model, IMO it should be described here too,

@korycins
Copy link
Member Author

Regarding this

input ChannelUpdateInput {
  ...
  expireOrdersAfter: Second
}

As you described in #11337 we'll have orderSettings: OrderSettingsInput input in ChannelUpdateInput, so we shouldn't extend ChannelUpdateInput with expireOrdersAfter.

Thanks. Yes, exactly, it should be in OrderSettingsInput. Thanks for catching this!

@korycins
Copy link
Member Author

The main job of this task will be taking a batch of unconfirmed and not paid orders and converting them to expired orders.

It seems like it requires adding some new field to Order model, IMO it should be described here too,

We will add new status to order.status. I will mention it in the RFC. Thanks

@fowczarek
Copy link
Member

In Shop type we have similar input.

type Shop {
  ...
  """
  Default number of minutes stock will be reserved for anonymous checkout or null when stock reservation is disabled.

  Added in Saleor 3.1.

  Requires one of the following permissions: MANAGE_SETTINGS.
  """
  reserveStockDurationAnonymousUser: Int
}

I think we should keep it in minutes instead of seconds. Also when we introduce Minutes scalar we should create a task to upgrade existing settings for time expiration.

@fowczarek
Copy link
Member

Any Staff users shouldn't be able to perform actions like:

  • Any payment action (mark as paid, capture)
  • Update addresses
  • Fulfillment actions
  • Invoice actions
  • return/replace actions
  • order cancel action
    for expired orders.

@korycins
Copy link
Member Author

korycins commented Dec 2, 2022

Thanks @fowczarek for the feedback. I've updated the rfc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rfc Request For Comments: blueprints of upcoming changes.
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants