Add an availability checker which checks if the entity is accessible or not

I think this works... The only concern probably here I guess is "BC", but it's probably fine... Does this actually mean that unpublished products can be purchased by default?

Hello everyone, the patch is still needed on commerce 8.x-2.28 ?

Yes, and it's using the "old" API which is now deprecated, there's an availability checker defined in Commerce API that can be copied.

Attaching the availability checker, copied from Commerce API. Some tests could fail, didn't copy the tests from the patch from @mglaman.

Attached patch should fix the existing tests and copied/adapted the test from @mglaman to work with the non legacy availability manager/checker.

Just wondering if we won't get bug reports about products disappearing from the cart with no explanation...

Also, once this goes in, we need to remove the same checker from Commerce API and bump the minimum required Commerce version.

Committed! @mglaman: Thanks for the initial patch!

Just wondering if we won't get bug reports about products disappearing from the cart with no explanation...

Allowing to present messages, when the user visits the cart page next time, would be a nice feature in the future :) like Amazon does for things like "The price for the product in your cart has changed to xxx"

@agoradesign I added a followup issue for this messaging #3276343: Display a message when a cart item is removed due to new availability manager checks

great! thanks :)

Just noting that this availability checker here made selling inaccessible products impossible.

Admittedly I have an odd use case for this: product isn't available in the default situation which is online (i.e. hook_entity_access for the product returns AccessResult::forbidden()), but is available for admin to sell via retail/POS (i.e. for a different store where hook_entity_access returns AccessResult::allowed()). So in this case this new EntityAccessibleAvailabilityChecker always returns AvailabilityResult::unavailable() as $purchasable_entity->access('view') === FALSE. There doesn't seem to be any way to override that as once one checker returns unavailable any other checker cannot override that. Appreciate that is the way this was designed, but it is a limitation.

you can implmenent a ServiceProvider that removes this service

@agoradesign of course, nice idea, thanks.

Very simple alter in your ServiceProvider in case anyone comes across this in future:

  public function alter(ContainerBuilder $container) {
    $container->removeDefinition('commerce_order.entity_accessible_availability_checker');
  }

Decided to revert this for now, considering the amount of issues that were opened since this got committed.

Shouldn't we keep it, but add a config whether to enable it? Defaulting it to true, only updating existing sites to false? Because in my eyes, the old behaviour (w/o the checker) was simply wrong and unexpected to 99% of the store owners. I was a little bit surprised, when I first realized that I have to implement a custom availability checker to prevent disabled product variations (in existing carts) to be purchased. I guess no one really expects that.

So if this is getting some more work it needs to log order item removal. It also needs to notify the user that the item was removed from their order and why.

@agoradesign: Well, this is a needed functionality IMO, but perhaps too late to include in 2.x, and probably require a more polish in order to be considered "fully functional" (logging deletion, displaying messages)...

agree :)

Here is a quick patch to temporarily disable this feature for people who are using 8.x-2.30 which shipped with this feature.

I'm also going to need to remove with ServiceProvider or the quick patch in #36.

This is problematic:

    if (!$purchasable_entity->access('view', $context->getCustomer())) {
      return AvailabilityResult::unavailable();
    }

If an admin user is placing an order in the backend, the customer in $context is not the person placing the order. The admin user should be able to add a product to the order even if it's not visible to the customer on the website. (For admin-only products, we leave the products unpublished but publish the variation, so the $purchasable_entity->isPublished() check is ok.)

So my feedback is that we shouldn't assume that the user adding an item to an order is doing so as a customer with a cart.

perhaps it should be $order->getCustomer(); instead indeed, but wondering if we shouldn't be more explicit, and just check that both the variation and the product are published.

$order->getCustomer() would still be problematic for my site, for backend order creation/management. Also, isn't $context using $order->getCustomer() aleady?

The variation+product published check would also be problematic for us. But perhaps in the more general case, these conditions make sense. We'd just disable this code for ourselves.

Essentially, we want to be able to hide products/variations from the customer that are not available in our "product catalog." They should be neither viewable on product pages nor purchasable by the typical, anonymous/authenticated user. But in the backend, administrative users should be able to add the products to an order. For adding products to an order administratively, we use a custom form with an entity_autocomplete element for the variation selection and a view providing the options. That works fine without this availability checker.

Some products like this are always in this "internal only" state; others may switch from public to private because of certain conditions. For instance, we could discontinue a product but continue to allow certain customers to purchase remaining inventory. So we'd want to unpublish it on the website. And then at some point in the future, the product could come back. Or we could make some product by special request (since we're a manufacturing company) which we want to sell but not merchandise on our website.

I have another use case which i'd like to add. I have a multi-domain commerce site using the domain module. The payment provider only provides one notify url option. So the notify url will always be on the 'default' domain. On that domain the domain access module will say (for a lot of products that don't have the default domain selected) that there is no view access on the products in the cart.

Re-rolled the patch from #19 to the latest 2.x-dev, because it failed to apply on commerce_order.services.yml.

@jsacksick shouldn't this target 3.0.x then based on #34 and get finished to be merged into that before release? (Just read through the comments and you wrote "Well, this is a needed functionality IMO")

I'll leave changing the target version to you.

This is a valid addition. However, with 3.x already released, we missed the opportunity to introduce it in a new major version—especially since it might not be backward compatible. That said, this is actually a good thing. We should ideally avoid introducing backward-incompatible changes even between major versions, when possible.

I believe this new checker can still be added, as long as it only affects new sites and existing sites retain their current behavior.

To make it easier for our core audience—ambitious site builders—to enable or disable availability rules and checkers as needed, I’ve proposed #3520439: Expose Availability Checkers in the Admin UI.