Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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.
Comment | File | Size | Author |
---|---|---|---|
#14 | Status_report.png | 64.12 KB | joelpittet |
#10 | interdiff_7-9.txt | 1.74 KB | jsacksick |
#9 | 3184272-9.patch | 9.42 KB | jsacksick |
#7 | interdiff-7.txt | 2.21 KB | amateescu |
#7 | 3184272-7.patch | 7.43 KB | amateescu |
|
Comments
Comment #2
amateescu CreditAttribution: amateescu at Centarro commentedThis should do it.
Comment #3
amateescu CreditAttribution: amateescu at Centarro commentedComment #4
rszrama CreditAttribution: rszrama at Centarro commentedChanges 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. 😬
Comment #5
jsacksick CreditAttribution: jsacksick at Centarro commentedAlso, 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.
Comment #6
amateescu CreditAttribution: amateescu at Centarro commented@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.
Comment #7
amateescu CreditAttribution: amateescu at Centarro commentedAdded test coverage.
Comment #8
jsacksick CreditAttribution: jsacksick at Centarro commentedChange 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?
Comment #9
jsacksick CreditAttribution: jsacksick at Centarro commentedComment #10
jsacksick CreditAttribution: jsacksick at Centarro commentedAlso 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).
Comment #12
jsacksick CreditAttribution: jsacksick at Centarro commentedCommitted! Thank you!
Comment #14
joelpittet@jsacksick, I'm not sure if there was a typo or something but I'm getting this after updating to the latest stable release:
I'll try to run the update hook manually and see what I get, but seems related to this issue
Comment #15
joelpittetRunning 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();'