🇺🇸United States @lisastreeter

Account created on 6 May 2016, about 8 years ago
#

Recent comments

🇺🇸United States lisastreeter

@liquidcms - Unless you've deleted it, you should see the admin Orders page View here: /admin/structure/views/view/commerce_orders

If you look at the "Page" display for that view, you'll see the path admin/commerce/orders/list defined.

You can configure this Orders page using this View.

🇺🇸United States lisastreeter

Just chiming in here since I did something similar to option "c" with a little bit of custom code. It's been working well in production for many months now (used on a daily basis.) Most of our orders are created administratively, so "resend receipt" for us is actually used to send the "initial" receipt.

Here's a full description of what I did: https://www.susubi.io/sa/order-receipt-resend-copy

The biggest difference between the request here and what I did is that we have an option to copy the address of the administrative user who clicks the button rather than the generic "customer service" email that's copied when receipts are sent automatically.

🇺🇸United States lisastreeter

@ressa Thank you so much for your response and all the helpful suggestions. Really appreciate it! I'll work on this today.

🇺🇸United States lisastreeter

Thanks! And sorry if the point of my comment was unclear. I'm just thinking about existing sites, many of which are highly customized. I only mentioned the order "status" vs. "state" customization we have as an example of something we needed to do to improve the UX for our own site--originally, I left the standard Commerce order "state" labels in place, but it caused confusion. So I just changed the labels. If the word "workflow" suddenly pops up, that will also be problematic. We have older CSRs, in their 50s and 60s, who would wonder, "what is a "workflow??" But this is absolutely a "my site" problem, not a Commerce-project-as-a-whole problem.

I have no objections to your changes as a potential improvement for new sites, potential new sites evaluating Drupal Commerce, and even many existing sites. I just think there are some existing sites, like ours, for which the update will not work. But that's fine, especially since it's easy enough to add a custom template that will leave our site unchanged.

If I hadn't noticed this issue and released the changes as part of a routine update, then our admin users would have certainly wondered why their UI was changed. So then I would have gone searching through the release notes and issues included in the release to try to identify the source of the change. Then I would have found my way to this issue and read through the comments before looking at the specific code changes. It would have been helpful for me to find a comment that just provided the name of the template that was changed and made it clear I could "fix" our site by just reverting it to the previous version. So that was the main point of my comment--just an attempt to help some existing site admin who could want/need to "un-improve" the order workflow UI.

🇺🇸United States lisastreeter

It looks like this is just a change to a template, so if the current workflow UI is confusing for a specific shop, couldn't these changes just be made in a custom template?

If this gets committed, existing websites may need to create a custom commerce-order--admin.html.twig template to remove the changes if they don't make sense to their administrative users. Just wanted to highlight that potential solution in this comment, if this update causes problems for existing sites.

For our site, the label "workflow" doesn't make sense, and even the "Current state" label would be problematic, since we had to change every order "state" label to "status" in our admin UI. It's been called "status" in our order entry system for more than 30 years, long before we started using Drupal, and nobody wanted to switch to "state"!

🇺🇸United States lisastreeter

One more comment, regarding your solution in the "Proposed Resolution." It is possible to provide more options than just Default and Receipt. However, to do so you need to write custom code to implement a new Order Document plugin. In one of my screenshots above, there's an "Invoice" option. That appears because I have custom plugin code that starts like this:

<?php

namespace Drupal\my_module\Plugin\Commerce\OrderDocument;

use Drupal\commerce_order\Entity\OrderInterface;
use Drupal\commerce_order_document\Plugin\Commerce\OrderDocument\OrderDocumentBase;
use Drupal\commerce_order_document\Plugin\Commerce\OrderDocument\OrderDocumentInterface;

/**
 * Provides the Invoice order document.
 *
 * @CommerceOrderDocument(
 *   id = "my_invoice",
 *   label = "Invoice",
 *   display_label = "Invoice",
 * )
 */
class MyInvoice extends OrderDocumentBase implements OrderDocumentInterface {

And then in my buildOrderDocument I explicitly set the theme (template) that should be used to generate documents. That template file can have any name I want. It's not limited to the commerce-order-document--DOCUMENT-ID.html.twig format. For example, the template for this plugin could be named my-order-invoice.html.twig or anything else I want.

🇺🇸United States lisastreeter

Hi! I'm not sure why you're having trouble--it sounds like you want to do precisely what this module is designed to do.

If you select "Default" as the plugin, set the machine name for your document type to "factura", and create template 'commerce-order-document--factura.html.twig' in your custom theme, that theme should be used for your "Factura" documents. You don't need to "select" anything in the configuration. The code is written in a way that first Drupal will attempt to find a template named 'commerce-order-document--factura.html.twig' and then only use the standard 'commerce-order-document.html.twig' template if the document-type-specific template can't be found. Maybe try rebuilding caches?

I'm doing it right now for a order document type named "proforma." Here are screenshots:

🇺🇸United States lisastreeter

The behavior of an existing Commerce order workflow should not be changed. Existing commerce sites already running in production will have custom Event subscribers, Guards, etc. Moving an order to the "Completed" state may trigger business logic like invoicing. (Or affect other business logic related to inventory, accounting, etc., as mentioned above.)

Existing e-commerce sites should not be forced to rework their working/tested custom code so that some new Drupal Commerce websites can have a workflow that allows cancelation from the Completed state.

Especially when it is very simple to create a new workflow. Yes, a custom module is needed. But it's just some configuration yaml, not custom code or anything complex. Just one folder for the module, one yaml file to define the module, and one yaml file for the workflow: https://docs.drupalcommerce.org/commerce2/developer-guide/orders/workflo...
No actual "coding" is needed. Just start with a copy of an existing workflow and modify it as you like, with a new name/label.

(For various logic to work as is in Commerce Core, you should keep the 'draft' 'canceled', 'completed' state IDs as is as well as the 'place' transition.)

🇺🇸United States lisastreeter

I needed to refactor this patch to use one for the Support Google Analytics 4 issue #3190724 Support Google Analytics 4 Needs review . For some reason, "private" methods" were changed to "protected" methods as part of that issue, so patch #3 here does not apply.

Posting an updated patch here for anybody else needing to make the switch to GA4 by the end of the month.

🇺🇸United States lisastreeter

@apaderno Thanks for the example class comment. In that same Core views module, I also see a constructor in class BlockForm extends EntityForm { that does not use the full namespace:

* Constructs a BlockForm object.

So core doesn't seem to be consistent in its phrasing. When working within the Commerce ecosystem, I tend to follow the coding standards used within Commerce Core. I read through this document, but wasn't able to find specific guidance:
https://www.drupal.org/docs/develop/standards/php/api-documentation-and-comment-standards

One possible shorter sentence for:

// First entity should be order document; additional should be orders to be rendered.

// Entities should be order document followed by orders to be rendered.

🇺🇸United States lisastreeter

Looks like the latest patch still has an incorrect change in the .info file.

Also, can somebody point me to the Drupal Coding Standards policy that states we need the full class namespace? I'm not seeing that format in any existing Commerce Core code, and personally, I feel like the original is more readable (and doesn't violate the 80 character line limit.)

For this change:

-    // First entity should be order document; additional should be orders to be rendered.
+    // First entity should be order document;
+    // additional should be orders to be rendered.

Perhaps we can just reword the original comment instead of breaking it into two separate comments? I think that would look cleaner.

Also, here:

-    // Pass the plugin configuration only if the plugin hasn't been changed via #ajax.
+    // Pass the plugin configuration only ,
+    // if the plugin hasn't been changed via #ajax.
🇺🇸United States lisastreeter

The problem was resolved with a composer update.

🇺🇸United States lisastreeter

example code fix

🇺🇸United States lisastreeter

Additional content

🇺🇸United States lisastreeter

Completed document

🇺🇸United States lisastreeter

Document completion.

🇺🇸United States lisastreeter

Document completed.

🇺🇸United States lisastreeter

Additional information

🇺🇸United States lisastreeter

Yes, forbidding deletion with message indicating reason sounds good.

May also want to look at implications for references from products, promotions, shipping methods (commerce shipping module, but also references more general.) Make sure that references to missing store entities don't break anything else.

🇺🇸United States lisastreeter

I don't think we should automatically delete orders on store deletion. A better approach might be similar to Content Type deletion:

Store name is used by 1,579 orders. You may not remove Store name until you have removed all of the Store name content.

And then perhaps set up your Orders view so that you can select by Store and use a "Delete orders" bulk action. I think most sites in production would want to strictly block any order bulk-deletion functionality. Deleting draft orders is generally fine but unless you're exporting all your orders to an external management system, deleting Completed orders (with payments!) could be very bad.

🇺🇸United States lisastreeter

@rhovland so sorry I missed that all this was within the tax module! Just plain dopiness on my part.

We use Commerce Avatax for tax calculations with customizations. And then we use order refresh for updating orders that have been placed--so taxes and everything else that gets processed during order refresh are automatically calculated. Administrative users never need to trigger any recalculations manually. Any updates they're allowed to make to placed orders (item quantity updates, additional, removals, shipping method changes, etc.) trigger order refresh. I just have this line of code before order saves in various places:
$order->setRefreshState(OrderInterface::REFRESH_ON_SAVE);

For overriding tax, we set a "tax_override" value in the order data. There's a custom order operation that links to a custom form, only available for default-type orders that are not-yet-completed. The main reason for the operation vs. a tab is that the Order tabs all correspond to things our admins should be routinely doing for all orders: View, Edit, Items, Shipping, WBC Sync, and Payments. Tax overrides should be rare so better to make it a little more difficult to find.

The custom form makes an api call to Avalara to get the standard tax rate based on shipping address. The form allows admin users to enable/disable the override, enter the desired tax amount and specify the reason for the override from a list of "tax status" options (provided by our accountants.) On submission, if an override is enabled, a new locked adjustment is added to the order. (Pre-existing tax adjustments are removed.) All updates are logged with custom commerce-log templates.

Elsewhere, the order "tax_override" data value is checked to prevent application of avatax values.

I'm always happy to share any custom code I have; I just generally assume that much of what I'm doing is fairly specific to the Drupal Commerce website I'm working on. I am working on putting together a personal website with more generalized versions of custom code solutions relevant to Drupal Commerce sites. "Tax overrides" is one of the topics on my to-do list (assuming it doesn't become part of Core Commerce anytime soon...)

Here are some screenshots, if you're interested. (The form has some custom "order alert" messages displayed at the top that are visible on every administrative order page.)

🇺🇸United States lisastreeter

This seems like it should be a custom solution. I don't know that every Drupal Commerce website would necessarily want an admin "Taxes" form for Orders. At the very least, you need to check that Commerce Tax is installed, as it's optional.

I'd need to use an alter hook to remove this new tab/page, if committed. We already have custom code to automatically recalculate taxes as needed for administrative updates. And we have an admin form for overriding the automatically calculated tax amount. This extra page would just cause confusion for our admin users.

🇺🇸United States lisastreeter

@asishsajeev I'm not concerned with the added condition, but some sites might have custom code to do things like sort promotions by generosity and apply the best to each order item. The new PromotionEvents::FILTER_PROMOTIONS event makes it possible to do that.

If custom logic depends on the assumption that the percentage amount for a promotion is accurate, then this new option could cause problems. Somebody (a content creator) could create a new promotion with the existing offer type and think it was perfectly fine to use the new optional maximum feature, not knowing anything about the inner workings of the site's custom code.

But maybe I'm worrying unnecessarily--this will not personally affect me as it is... One improvement, though, would be to correctly set the "percentage" value for the adjustment. Though I'm not sure whether it would be correct to just eliminate "percentage" from the adjustment data completely when the threshold is reached. Or whether it should just be calculated based on the amount of the adjustment. (The OrderFixedAmountOff offer plugin does not include the percentage amount. The OrderPercentageOffer uses the percentage from configuration.)

I do think this is a great idea, btw. I just worry about unintended consequences for sites that have done custom things to make promotions work for them.

🇺🇸United States lisastreeter

I think this change could cause problems for existing sites. For one, the "percentage" amount in the adjustments is no longer correct in all cases. It blurs the line between a percentage-off offer and a fixed-price-off offer.

Why not just create a new custom promotion offer plugin, in a custom module?

🇺🇸United States lisastreeter

Simple patch with a fix attached.

🇺🇸United States lisastreeter

@jsacksick -- yes, my patch wouldn't affect anything at all for a standard Commerce install. (Which is why I didn't bother creating an issue or suggesting the patch as an update.) But in my custom code, I've got this line in several different places, affecting non-draft orders:

$order->setRefreshState(OrderInterface::REFRESH_ON_SAVE);

I've found that Order Refresh works perfectly fine for non-draft orders. The only thing that was getting messed up for placed orders was the contact email address. That little one-line change fixed it without affecting draft orders at all.

Admin users are allowed to change prices/quantities/shipping methods/items ordered and override taxes for already-placed orders. When they do, I need to run order refresh manually.

My personal opinion for this issue is that we should probably leave Order Refresh unaltered--people with complex sites may already be doing custom things related to order refresh.

🇺🇸United States lisastreeter

This issue caught my attention because we allow admins to change order emails and we continue to perform order refresh after an order is placed. So we essentially leave Drupal Commerce alone to do its (standard) thing for customers shopping on the website but for admin users, we have them "place" a new order right away. Then for them, the experience of editing an order a customer placed vs an order they placed is identical. We control order refresh triggers through custom code. But we're B2B and changes to already-placed orders are not uncommon. All that being said, this is how we modify orderRefresh to keep the email from getting reset by refresh after a change by an admin:

Replace: if ($customer->isAuthenticated())
With: if ($customer->isAuthenticated() && $order->getState()->getId() == 'draft')

We've had our custom patch in production since last summer without any problems/conflicts with updates. Perhaps as @jsacksick suggests, merging this patch would just break too many things; it may be better as a custom patch within your project.

🇺🇸United States lisastreeter

@Thorny - I'm using a Zone field for customer "tax exemption documents" so that I can match them up with customer shipping addresses. For the vast majority, admins are just entering one or more US states for the Zone value.

I was just asked to add "State" as a Views filter for the tax-exemption listing and found this issue when I went searching for a solution.

Creating a proper views filter for a zone also seems extremely difficult (or impossible) to me. So...I've been considering a workaround that I'll describe here in case you could do something similar for your "postal code" filter needs.

* Add a multiple value text field for "States" to tax exemption document entities. (do not display on entity form/display)
* On entity save/update, extract the "administrative area" for each zone territory and save it to the States field.
* Add Views filter for that States field. Admin users will simply type the 2-character state abbreviation. (All exemptions are U.S.)

If you only have "included postal codes", perhaps you could do the same, parsing the values into a multi-value text field on entity save. And then use the "Regular expression" operator for the Views filter.

Then again, perhaps creating a custom field as you suggest would be a better/cleaner solution.

🇺🇸United States lisastreeter

@rhovland I agree that the title of the issue doesn't really match the code in the patch. Though generally I do think it's a good idea to delete the referenced entities that belong to an entity when the entity is deleted.

It may have been easier to identify potential issues in custom code if the title was, "Delete a shipment's profile when shipment is deleted." And perhaps the Release could have included a note about the possibility that shipment deletion within custom code could require rework.

🇺🇸United States lisastreeter

@jacksick. Sorry I was unclear! Totally due to custom code on my site. Not the fault of the patch.

I only posted a message because it took me a little while to figure out why my checkout was broken after making a bunch of module updates. My custom code takes the shipping profile from the shipment and adds it directly to the order. So admins can easily edit both the billing and shipping addresses on the order form, side-by-side. We don't actually need the "shipment" after checkout. So I was just deleting it in custom event subscriber after attaching the shipping profile to the order. Patch did its job and deleted the shipment's shipping profile, but my custom code was assuming shipping profile still existed. Removing the reference before deleting the shipment was a simple one line fix in my custom code once I figured out what was happening.

🇺🇸United States lisastreeter

Took me a while to track down the cause of this error message that appeared after updating my site's modules:
Drupal\Core\Entity\EntityStorageException: Update existing 'profile' entity while changing the ID is not supported.

Realized it was this issue affecting custom code in which I delete an order's shipment after it's placed:
$shipment->delete();

Leaving this comment here in case anybody else gets the EntityStorageException error message and is looking for a fix. Adding this to my custom code, right before the delete, eliminated my bug:
$shipment->set('shipping_profile', NULL);

🇺🇸United States lisastreeter

Any way this could be optional? I'm already going to have to implement a hook to get rid of the "Add Payment" button that will be included in the next Commerce release: https://www.drupal.org/project/commerce/issues/3348311 Add Payment link on order view page Closed: won't fix
For us, it's important that admins review existing Payments before adding a new one. Adding a button to bypass is a bad idea.

This will be one more thing we need to remove. We customized the admin UI so that we don't have a Shipments tab for orders that have already been placed. We only ever have one shipment per order, so "Shipments" doesn't really make sense. If this issue is committed, we'll need a hook to get rid of this.

🇺🇸United States lisastreeter

Oops--just found this issue after I created a new one with a patch: https://www.drupal.org/project/commerce_pricelist/issues/3343492 📌 Revamp the price list admin list using a view Needs review

So this is now a duplicate. Sorry about that!

🇺🇸United States lisastreeter

Looks like the tests don't like the Views configuration. Not sure how to fix but can try to look into it.

🇺🇸United States lisastreeter

While my personal preference would be masks-optional for everybody, I understand that this may not be possible yet.

I do have a concern about requiring masks for speakers. I find it difficult to hear masked speakers clearly, and this may also be an issue for the hearing impaired.

I wonder whether an exception could be made for speakers who prefer to not be masked during their talks. If so, perhaps attendees could be encouraged to sit in the first couple rows if they are comfortable around unmasked speakers, reserving the back couple rows for attendees who would prefer the safety provided by the additional distance.

🇺🇸United States lisastreeter

Oops! Previous patch had a misnamed file. Fixed version attached.

🇺🇸United States lisastreeter

Rerolled flood_control-support_external_flood-2928007-52.patch for 2.3.0

🇺🇸United States lisastreeter

This is great! Thank you. We are using a custom PromotionStorage class to override the loadAvailable method to sort promotions by value to the customer.

The approach here will work perfectly for us--we can use the same code in an event subscriber instead of the custom class.

🇺🇸United States lisastreeter

you can still reproduce the curent behavior to allow user to select between multiple order item types, if desired, by creating multiple order item types that all apply to same order type.

So if this gets released, and I want my N order types to still allow my M order item types, I could needs as many as N * M order item types? And how would that work if a particular product variation type is configured for a specific order item type? It can't be configured for N different order item types, can it?

As for custom order workflows, I find it works well to add a State field at the order item level, to match its specific fulfillment steps. Then the workflow at the order level is more uniform for CSRs. I use the custom Guard functionality to prevent order level transitions until all order items have reached a certain stage.

For me, flexibility is preferable because only about 10% of our orders are placed through the website by customers. The rest are placed by CSRs administratively. As a result, we have a good deal of custom code to make much the process of placing an order as easy as possible for CSRs. The last thing we need is additional limitations on the order/order item architecture.

One thing we do is allow a single product variation to be added to orders as different "order item types," (bypassing the product variation's order item type setting.) A CSR can add a product variation to an order as a free "product sample" or as a "credit item" (when goods are returned.) Each of those is a separate order item type with its own fields. (Credit items need to reference back to original order items and can have a credit amount not equal to qty*price. Sample items also store a "reserve" amount of inventory to be set aside for the customer for future orders.) For orders entered administratively, we don't use the standard Order edit form and inline-entity-form widget for items; instead we have a custom add-item widget on an order items form similar to a customer's "cart" page.

Through the website, when a customer adds to cart, the variation is just added as a standard/default order item. So the existence of the "order item type" setting is needed for our 10%, customer-facing, use case, but it would be very bad if all of a sudden it was impossible to create other order item types for our product variations. I realize that your patch does not directly relate to this at all. But... when I first saw this issue I thought about how for us, it's not a "bug" that we can add a product variation to orders as different types of order items. And for other websites, it's not a "bug" that they can add a single order item type to different types of orders (perhaps only administratively, like we do.)

Personally, I don't think this change would actually affect my site as it is now. I'm just concerned generally about an architectural change that could potentially break existing sites. And so, I agree with Jonathan that the best thing to do is to keep this issue open for a while. Perhaps create a patch. See whether there is widespread interest. If the majority agrees this is a bug that needs fixing, then the minority that requires the current behavior will just need to patch the code if it's released to keep their sites operational.

Production build 0.69.0 2024