Describe your bug or feature request.

Since we can sell any Purchasable Entity using Commerce Core, I think the cart should not require Commerce Product and add its functionalities that require Commerce Product only if the module is active.

Issue fork commerce-3323623

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:

Comments

facine created an issue. See original summary.

facine’s picture

Status: Active » Needs review
StatusFileSize
new2.3 KB
facine’s picture

Title: Remove Commerce Product dependency from Commerce Cart » Remove Commerce Product dependency from Commerce Cart and Commerce Checkout
StatusFileSize
new3.65 KB

We need to fix commerce checkout module also.

facine’s picture

StatusFileSize
new4.82 KB

More fixes.

jsacksick’s picture

The changes look weird to me, besides, I don't want to risk introducing regressions at this point... Why are we now conditionally swapping the add to cart form only if the product module is enabled?

facine’s picture

If I want to use my own purchasable entity and not use product variations, why do I have to install product?

jsacksick’s picture

+  if (\Drupal::moduleHandler()->moduleExists('commerce_product')) {
+    $entity_types['commerce_order_item']->setFormClass('add_to_cart', '\Drupal\commerce_cart\Form\AddToCartForm');
+  }

So no add to cart if the product module isn't enabled? That is the change that looks weird to me.

I guess this is due to this logic:

if ($selected_variation && $order_item->getPurchasedEntityId() != $selected_variation) {
        /** @var \Drupal\commerce_product\Entity\ProductVariationInterface $selected_variation */
        $selected_variation = $this->entityTypeManager->getStorage('commerce_product_variation')->load($selected_variation);
        if ($selected_variation->getOrderItemTypeId() !== $order_item->bundle()) {
          /** @var \Drupal\commerce_order\OrderItemStorageInterface $order_item_storage */
          $order_item_storage = $this->entityTypeManager->getStorage('commerce_order_item');
          $order_item = $order_item_storage->createFromPurchasableEntity($selected_variation);
          $this->prepareOrderItem($order_item);
          $this->setEntity($order_item);
          $form_display = EntityFormDisplay::collectRenderDisplay($order_item, $this->operation);
          $this->setFormDisplay($form_display, $form_state);
          $form = $original_form;
          $form = parent::buildForm($form, $form_state);
        }
      }

But it probably wouldn't trigger if:

$selected_variation = $form_state->get('selected_variation');

is NULL.

Status: Needs review » Needs work

The last submitted patch, 4: 3323623-4.patch, failed testing. View results

facine’s picture

Status: Needs work » Needs review

@jsacksick there are cross-dependencies that make it impossible not to install product.

The idea is to be able to use a custom purchasable entity and an add-to-cart different from the one provided by core.

The alternative would be to create an alternative cart contrib without these dependencies.

Do you think it would be possible to introduce these changes or would you prefer to create a contrib module?

jsacksick’s picture

Status: Needs review » Needs work

I'm not sure why you're setting this task to "needs review"... As far as I can see, the patch submitted has 42 failing tests in case you haven't noticed.

facine’s picture

StatusFileSize
new10.72 KB
facine’s picture

StatusFileSize
new11.18 KB
facine’s picture

Status: Needs work » Needs review

@jsacksick, I've fixed the tests, the errors in D10 are not related to these changes: https://www.drupal.org/pift-ci-job/2746156.

"3.0.x-dev test with PHP 8.1 & MySQL 5.7, Drupal 10.1.x" is failing since "2 Jan 2023 at 16:32 CET"

jsacksick’s picture

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

  • jsacksick committed 228d8573 on 3.0.x
    Issue #3323623 by facine, jsacksick: Remove Commerce Product dependency...
jsacksick’s picture

Status: Needs review » Fixed

@facine: Thank you! For some reason when applying the patch, the config files weren't properly moved to the optional directory.
Had to spend time fixing other test failures to make sure this didn't introduce a regression, but tests are green! Good job :).

Status: Fixed » Closed (fixed)

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