Availability checks should use a value object instead of boolean return values

Dejavu: #2710107: Finalize the availability manager service for more background.

The value object from steveoliver is a good start. As discussed in the comments of his PR, we can omit the min qty and get away with one qty value (see here).

For commerce stock it would be fine, if we could return an own message plus the qty that is available. AvailabilityResult is a good name.
I will reach out for guy schneerson to join the discussion.

@mglaman The constraint idea is a really beautiful one. Thanks for getting that done. :)

It makes sense to triage the old issue. The comments on github and the issue are a good source. I think it was a comprehensive discussion back then, that covered a lot of use cases, not only stock. IIRC at that time commerce was in beta, much to do for bojanz and nobody wanted to break BC for commerce stock ... ;)

Hi @mglaman
I did like the idea of the min/max but agree with @olafkarsten that returning an absolute quantity value is good enough for stock.
As far the add to cart functionality no need to worry about stock backward compatibility as it currently does not use the availability manager. As long as the availability manager will support an "allowed quantity" and providing "a reason why not available" message, I think we should be able to easily move over to using it.
One area I don't remember much about is if/how the availability manager behaves on cart and checkout as those have other required behaviors. On the cart page, for example, we may not wish to automatically adjust the quantity, but I suspect this may be using existing stock validation and not down for the availability manager to handle.
Also backing @olafkarsten on the point of issues/2710107 and commerce/pull/593 containing useful information.

I agree with @guy_schneerson about just requiring an allowed quantity. By way of example, I have a commerce_stock_backorderable module that overrides the commerce_stock availabilityChecker as a service. Here is the check function:

public function check(PurchasableEntityInterface $entity, $quantity, Context $context) {

    if ($stock_checker->getIsAlwaysInStock($entity)) {
      return AvailabilityResponse::available(0, $quantity);
    }

    $no_stock_sale_field = $entity->get('allow_no_stock_sale');
    if ($no_stock_sale_field) {
      $allow_no_stock = $no_stock_sale_field->value;
      if (boolval($allow_no_stock)) {
        return AvailabilityResponse::available(0, $quantity);
      }
    }

    $stock_service = $this->stockServiceManager->getService($entity);
    $stock_checker = $stock_service->getStockChecker();
    $stock_config = $stock_service->getConfiguration();
    $locations = $stock_config->getAvailabilityLocations($this->stockServiceManager->getContext($entity), $entity);
    $stock_level = $stock_checker->getTotalStockLevel($entity, $locations);

    if ($stock_level >= $quantity) {
      return AvailabilityResponse::available(0, $stock_level);
    }
    else {
      return AvailabilityResponse::unavailable(0, $stock_level, 'maximum exceeded');
    }
  }

}

As you can see for the available min I am just using 0, so I have no problem with changing min/max to just an allowed quantity.

Regarding #18, the crux of the issue is in AvailabilityOrderProcessor::process

In this function we need to e.g. take a quantity of three for an item and if availabilityManager->check says there is only one available, we need to remove two from the order and tell the customer about it. So this product is "available", just not i the desired quantity. I guess this could be an example of "forbidden" where there is one available, and we do something about that here in ::process

If we look at its current state, https://git.drupalcode.org/project/commerce/-/blob/8.x-2.x/modules/order...

I don't get the feeling that it can do anything like that, which must be why I'm using PR 593

https://github.com/drupalcommerce/commerce/blob/329fabe2da6bab6cee33a903...

So yea, where PR 593 says $availability->getMax() < $order_item->getQuantity() and it triggers a change in quantity, we could instead be responding to AvailableResultForbidden and using AvailableResultForbidden::$available to change the quantity appropriately.

And if its not forbidden, yes, we don't care what the quantity is -- just return from the function.

Note that the patch from #3138952: Make the AvailabilityManager order aware also introduces new value objects for returning an availability result.