Closed (fixed)
Project:
Commerce Core
Version:
3.0.x-dev
Component:
Developer experience
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
24 Nov 2022 at 16:27 UTC
Updated:
23 Oct 2024 at 11:59 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
facine commentedComment #3
facine commentedWe need to fix commerce checkout module also.
Comment #4
facine commentedMore fixes.
Comment #5
jsacksick commentedThe 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?
Comment #6
facine commentedIf I want to use my own purchasable entity and not use product variations, why do I have to install product?
Comment #7
jsacksick commentedSo 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:
But it probably wouldn't trigger if:
is NULL.
Comment #9
facine commented@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?
Comment #10
jsacksick commentedI'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.
Comment #11
facine commentedComment #12
facine commentedComment #13
facine commented@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"
Comment #14
jsacksick commentedComment #17
jsacksick commented@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 :).