Problem/Motivation

Order items that are created and added automatically to an order (i.e. as part of a promotion) need a way to disallow customers from changing their quantity or removing them for the cart.

Proposed resolution

Add a locked field to order items, and use it to disable the 'Edit Quantity' and 'Remove Button' views fields, used in the default cart view.

Remaining tasks

Add test coverage, review.

User interface changes

Some order items will no longer be editable by customers in the default cart view.

API changes

Three new methods on OrderItemInterface: isLocked(), lock() and unlock()

Data model changes

A new locked boolean field added to order items.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

amateescu created an issue. See original summary.

amateescu’s picture

Status: Active » Needs review
Issue tags: +Needs tests
FileSize
5.19 KB

This should do it.

amateescu’s picture

rszrama’s picture

Changes look fine on the surface; only question in my mind would be can this new column be added instantly per https://mysqlserverteam.com/mysql-8-0-innodb-now-supports-instant-add-co...? If not, we'll need to provide instructions or an alternate upgrade path for sites with millions of order items. 😬

jsacksick’s picture

Also, we could still want to allow updating quantities (even for an a auto-added order item, in the case mentioned i.e promotions).

What if you giveaway 1 item for free, but still want to order 2 extra (and pay for those extra quantities?). Granted we could add an extra setting to control that behavior and only lock the order item if it should not be editable.

amateescu’s picture

@rszrama, core doesn't provide a way to modify the query which adds a db column when adding an entity field, so.. unless we really want to write some one-off hook_query_alter() hack, we can only provide instructions..

This reminded me that we should set an initial value for the new field, so all the existing order items have a FALSE value by default when adding the column.

@jsacksick, I agree that the auto-locking behavior might not be always needed, and that we could provide a setting for it (TRUE by default) on the promotion, but this question needs to be handled in #3179312: Add an option to Buy X Get Y to auto-add the "Get" product variation, this patch only adds the possibility to lock order items.

amateescu’s picture

Issue tags: -Needs tests
FileSize
7.43 KB
2.21 KB

Added test coverage.

jsacksick’s picture

Change looks good. The only thing I'm wondering is whether we should check if an order item is locked in other places in the code as well (e.g skip resolving the price from OrderRefresh, where we currently check whether the order item unit price is overridden), or should we assume that a locked order item has its unit price overridden as well?).

Also, what about the order admin? Shouldn't we prevent editing locked order items there as well?

@rszrama, @amateescu: thoughts on that?

jsacksick’s picture

jsacksick’s picture

FileSize
1.74 KB

Also forbidding updating/deleting of locked order items (assuming the order is a draft, to ensure we don't frustrate admins removing order items that are let's say, out of stock).

  • jsacksick committed f35c20f on 8.x-2.x authored by amateescu
    Issue #3184272 by amateescu, jsacksick, rszrama: Provide a way to lock...
jsacksick’s picture

Status: Needs review » Fixed

Committed! Thank you!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

joelpittet’s picture

FileSize
64.12 KB

@jsacksick, I'm not sure if there was a typo or something but I'm getting this after updating to the latest stable release:
error

I'll try to run the update hook manually and see what I get, but seems related to this issue

joelpittet’s picture

Running it manually did the trick, probably an issue with moving away from the dev to a stable release and that last index bug I was working on got committed. Anyways solved by drush ev 'module_load_install("commerce_order");commerce_order_update_8211();'