Describe your bug or feature request.

Currently, we rely on the Inline entity form module and its "Inline entity form - Complex" widget for the order items widget editing on the order edit form.

This is the last piece of functionality within Commerce relying on IEF.
There is still no stable release of Inline Entity form, and we've got several bug reports to date because of the minimum-stability from composer project templates (See #3197048: Update the Composer constraint for Inline Entity Form, #3313625: can't install commerce, error or #3379772: Commerce module installation fails on fresh drupal website and there are others).

As soon as we build our own widget, we should be able to drop the IEF dependency from the commerce.info.yml and we can update the widget on new installs.
Unfortunately, I don't really think we can remove the IEF dependency from the composer.json as the module is likely used elsewhere on existing installs.
We could either create a change record and drop the IEF dependency completely, or.. Just do this from the 3.x branch...
In any case, here's what needs to happen:

  • Define an "order_item" Inline form plugin.
  • Create an OrderItemsWidget from commerce_order leveraging the "order_item" Inline form plugin

The select list offering the order item type selection should take into account the "order type" setting at the order item type level. In other words, only order items using the order type being edited should be offered.

Perhaps we could take this opportunity to implement some improvements such as displaying the "SKU" in the order items table for instance... In a later phase, we could probably defining an additional widget that performs the add/edit/deletion from within modals, but I'd suggest to just stick with having the exact same feature rebuilt within Commerce.

Thoughts?

CommentFileSizeAuthor
#14 order-items-widget-mockup.jpg159.66 KBrszrama

Issue fork commerce-3403314

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

jsacksick created an issue. See original summary.

jsacksick’s picture

jsacksick commented
Title: Define an OrderItems widget leveraging our Inline Form plugins » Define an OrderItems widget leveraging our Inline Form API
Issue summary: View changes
jsacksick’s picture

jsacksick commented
Version: 8.x-2.x-dev » 3.0.x-dev

tBKoT made their first commit to this issue’s fork.

jsacksick’s picture

jsacksick commented

One cool addition could be to change the UX for creating order items.
For example, instead of offering an order item type select list, we could first require referencing the purchasable entity, and then call the the createFromPurchasableEntity() method (from the order item storage), so that the order item type is determined based off the purchasable entity, rather than requiring a non technical user to figure out the right order item type to use.

Could be great to investigate this alternative UX. This alternate UX might not be possible for merchants using order item types that don't reference purchasable entities.

tbkot opened merge request !315

dalra’s picture

dalra commented

Commerce can not be installed directly from project browser because IEF does not have a stable release.
IEF needs to be installed through composer and only then Commerce can be installed from project browser.

jsacksick’s picture

jsacksick commented
Version: 3.0.x-dev » 3.x-dev
jsacksick’s picture

jsacksick commented
Status: Active » Needs work

We are really close! Good job.
Wanted to include this one in the next release but it looks like it requires a bit more work:

  1. We are currently assuming the order items can only reference product variations, breaking support for referencing other purchasable entity types.
  2. I'm thinking we could update the loop in getListOfBundles() to check the purchasable entity type ID? Like the following:
        foreach ($order_item_types as $order_item_type) {
      // We currently only support adding variations from the admin.
      if ($access_handled->createAccess($order_item_type->id()) &&
        $order_item_type->getPurchasableEntityTypeId() === 'commerce_product_variation') {
        $bundles[] = $order_item_type->id();
      }
    }

    The only problem is, on new installs, the purchasable entity type isn't set... (just tried).

  3. Since commerce_order doesn't depend on commerce_product, I'm suspecting the current code could break... So maybe we need a moduleExists() check in addition?
  4. It would be great if we could implement a fallback to the current behavior in case there are order item types referencing other purchasable entity types? The order item type selector... I'm just afraid not having this fallback means you cannot add order items from the admin? Unless we accept this limitation and in this case people would need to use the IEF widget?
  5. When setting the order item title, there is no code marking the title as overridden, so if it is customized, it'll be reverted to the purchased entity title on refresh. However, this is probably out of scope for this issue and would require a new widget.

Also, not 100% convinced we should force people to use the new widget by writing an update hook... But maybe...?

jsacksick’s picture

jsacksick commented

Ok here is what I think we should do...

If there is only one purchasable entity type referenced (commerce_product_variation, or something else), and it is known, we can probably offer the autocompletion? I don't think we have to hardcode commerce_product_variation right? Probably makes the code a bit more complex, but it should be doable?

In case there are multiple purchasable entity types possible (probably 5% of the usecases), we should offer the current UX as a fallback... Thoughts?

rszrama’s picture

rszrama commented

This needs a reroll to apply on 3.x now. Additionally, can we get screenshots to see the new behavior?

For example, I can't tell if the new widget still requires you to select an order item type first before selecting a product variation, which in the current scenario would happily allow you add a product variation to an order with the wrong order item type. In an ideal UX, you'd be selecting the product variation first, and the order item type would be selected from its type settings. (And where other types of purchasable entity are supported for order item referencing, the selection would be the purchasable entity type, not the order item type.)

That said, I'm not sure our current implementation would support alternative purchasable entity types anyways, so it may be a moot point. Our standard should be replacement level functionality, but I'd like to improve the UX here at a minimum.

jsacksick’s picture

jsacksick commented

@tbkot: We've been discussing this internally and decided we don’t want a fallback on the “IEF” widget. What we would like instead is a purchasable entity type selector.

Basically, when multiple purchasable entity types are selected, we should present a select list with a list of purchasable entity types.
Whenever the purchasable entity type is changed, there should be an Ajax refresh refreshing the autocomplete element to autocomplete on the right entity type.

The first available option should be preselected. (Options should be "Product variation", etc...

tbkot’s picture

tbkot commented
Status: Needs work » Needs review
rszrama’s picture

rszrama commented
Status: Needs review » Needs work
StatusFileSize
new order-items-widget-mockup.jpg159.66 KB

Minor suggestion for improvement here. I like the idea of adding the SKU to the input, but not necessarily as a new column. Perhaps we can make it as a sub-line on "Title"? This will also more elegantly degrade in the event an order item doesn't have a SKU; you won't end up with just an empty cell in that column.

Additionally, I think we can make the form more concise by removing the order item type. We don't show that on the view page anyways, and it's an implementation detail a CSR shouldn't care about. This will likely result in more natural inlining of the operations buttons, too.

(That said, it might also just make more sense to get these into a drop-button with "Edit" as the default, particularly once you enable "Duplicate.")

Mocking up all of the above (except the drop-button), I'm imagining:

Other minor changes needed:

  1. We should use the previous labels for checkboxes in the widget settings form: "Allow users to add new order items." and "Allow users to duplicate order items." with that casing and punctuation.
  2. The summaries for these settings should be: "New order items can be added." and "Order items can be duplicated." respectively. If they are disabled, they would be: "New order items cannot be added." and "Order items cannot be duplicated." respectively.
  3. Custom styling on the SKU sub-line in my case was just appending a br tag and wrapping the SKU line in a pre tag. I made this a relative font-size (e.g., .85 rem) and gray (e.g., #999), but we can ask majmunbog for a better recommendation if needed.
  4. Instances of $purchase_entity in the code should be updated to $purchased_entity to match the function name / other users in Commerce Core.
  5. Similarly, anywhere the code references "purchasable entity types," we should be using that language instead of "purchased entity types." This would be the function name getPurchasedEntityTypes() -> getPurchasableEntityTypes() and $purchased_entity_types -> $purchasable_entity_types. Same for other parts of the element and autocomplete.
  6. I think we should tweak the logic in buildAddNewItemAction() when there is no form state input yet. Instead of defaulting to the first item in the array, if product variation is an option, we should always default to that, and only if it isn't should we default to the first option.

Note: I did not have a local instance of a module specifying an alternative purchasable entity type, so I'll trust you that the code there works. 😄

tbkot’s picture

tbkot commented

@rszrama I've added the requested changes.

The table does not contain separate columns for SKU and Order item type.

Operations cannot be put in the "dropbutton" as it supports links only. I've moved "Duplicated" to the first button if it can be used.

Other points:
1. Labels for checkboxes are updated
2. Summary is updated
3. SKU is added under the title with the "inline_template"
4. Cannot find $purchase_entity in the code
5. Method and variables are renamed.
6. Instead of checking if "commerce_product_variation" is in the $purchasable_entity_types, I updated the code to make sure that it is always the first element in the getPurchasableEntityTypes.

tbkot’s picture

tbkot commented
Status: Needs work » Needs review
rszrama’s picture

rszrama commented
Status: Needs review » Needs work

Nice refinement for #6! I like it. 😄

On testing just now, though, I noticed an odd bug. When you add a new order item to an order, draft or placed, and save, the order total does not get recalculated. This typically occurs during order presave, so all I can figure is there's some mismatch between the inline entity form process and the new widget regarding when order item totals are being calculated and / or order items are being saved to the order relative to when the order total recalculation occurs.

I took a video I'll share in Slack as I'm not sure how to embed them in tickets here. 😝

jsacksick’s picture

jsacksick commented
Status: Needs work » Needs review
rszrama’s picture

rszrama commented
Status: Needs review » Reviewed & tested by the community

Looks good to me! Thanks for including the test!

  • jsacksick committed b6f64db6 on 3.x authored by tbkot 
    [#3403314] feat: Define a new widget to simplify adding products to...
jsacksick’s picture

jsacksick commented
Title: Define an OrderItems widget leveraging our Inline Form API » Define a new widget to simplify adding products to orders from the edit page
Status: Reviewed & tested by the community » Fixed

Woohoo! Merged!

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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