commerce icon indicating copy to clipboard operation
commerce copied to clipboard

Issue #2710107: Availability responses as value objects with quantity range

Open steveoliver opened this issue 9 years ago • 10 comments

See https://www.drupal.org/node/2710107

steveoliver avatar Dec 31 '16 00:12 steveoliver

@bojanz, Check out the latest fails ... why should ->refresh() change qty from 2 to 1?

steveoliver avatar Jan 23 '17 09:01 steveoliver

Thanks @olafkarsten. Working on getting tests passing now, and after discussion with @BBGuy about his min/max comment, I agree that makes sense, and will make that change as well.

steveoliver avatar Jan 24 '17 17:01 steveoliver

This will no longer apply as a patch due to the change in the AddToCartForm constructor. Rebase please.

ransomweaver avatar May 03 '17 03:05 ransomweaver

Rebased and PHP_INT_MIN and PHP_INT_MAX for min and max.

steveoliver avatar May 03 '17 16:05 steveoliver

This PR no longer applies as a patch. Can it be rebased?

ransomweaver avatar Aug 15 '17 17:08 ransomweaver

This breaks on < PHP 7

    1)
    Drupal\Tests\commerce_cart\Functional\AddToCartFormTest::testProductAddToCartForm
    Exception: Notice: Use of undefined constant PHP_INT_MIN - assumed
    &#039;PHP_INT_MIN&#039;
    Drupal\commerce\AvailabilityManager-&gt;check()() (Line: 35)

mglaman avatar Sep 15 '17 20:09 mglaman

Made changes to stock and works as expected :) A few small ux issues like not using the $availability->getReason() but I think this type of changes can be made later as they will have no impact on the API/function definition. We should get this in ASAP so we can make the stock changes needed.

BBGuy avatar Oct 24 '17 09:10 BBGuy

@BBGuy @steveoliver @mglaman

It looks like this issue and review process is the current bottleneck for getting stock / inventory support into Commerce 2. As the last activity was some time ago, I thought I'd ask to see if there was anything I could do to help here.

bgronek avatar Feb 01 '18 21:02 bgronek

One major problem with the PR as it stands is that it's breaking API.

joachim-n avatar Feb 01 '18 21:02 joachim-n

After seeking to Bojanz and finding it difficult to justify the minimum values of the response. I have spent some time rethinking this aprouch.
The reason is that while products may have a minimum and maximum quantities that they can support, that range is not continues. take for example this Electronic Capacitor that can only be ordered in multiples of 5. I now believe that the solution is to allow the availability checker to provide a number that's valid for purchase alongside an optional message that will give context. This will allow for a module to react on the above product with a request of 17 by returning 15 and a message of "Product CA05668 needs to be ordered in multiples of 5. We have altered your quantity from 17 to 15".

This however leaves one other issue. Commerce needs to know if to disable the add to cart button. If we use the above logic and provide 1 to the checker, then the module may return 0 and the button will be disabled. I think that this is a different type of availability question. If the first question is: "Can the user purchase quantity X of Product Y?" This second question is "Is product Y available for purchase?"

This second question is a simple Yes/No question. I now think that both those questions are needed for a flexible implementation of the availability manager and as far as I can see it leaves no use case were a product is available unsupported. This does not include back order and wait list as I think those should be categorized as unavailable and that's a whole new story.

BBGuy avatar Mar 09 '18 12:03 BBGuy