Make Commerce Stock module and submodules Drupal 10 compatible

Create a patch with core_version_requirement modified and rector suggested changes.

CommentFileSizeAuthor
#17 commerce-stock-d10-compat-3330022-17.patch25.25 KBnicklasmf
#8 commerce-stock-d10-compat-3330022-8.patch25.32 KBSilviuChingaru

Issue fork commerce_stock-3330022

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:

About issue forks

Comments

silviuchingaru created an issue. See original summary.

silviuchingaru opened merge request !14

rgnyldz’s picture

rgnyldz commented

Any updates on this? Is there a dev version we can test and give feedback ?

SilviuChingaru’s picture

SilviuChingaru commented
Status: Active » Needs review

@RgnYLDZ you can apply the patch against current dev version and review it. You can even clone the 3330022-drupal-10-compatibility branch.
Thank you for your interest!

guy_schneerson’s picture

guy_schneerson commented

Thanks, @silviuchingaru I was expecting the Event stuff and could not think of anything else that should cause an issue so I suspect you got it spot on :)

SilviuChingaru’s picture

SilviuChingaru commented
Status: Needs review » Needs work

@guy_schneerson you are welcomed but I think that the patch is not ready because it uses event from Symfony 6 and is marked as compatible with Drupal 8 which is not the case because Symfony 6 was implemented in Drupal > 9.0.
I think we should use it like Commerce does to make it also backward compatible.
I'll update merge request soon with this.

SilviuChingaru’s picture

SilviuChingaru commented
Title: Drupal 10 compatibility » Drupal 10 compatibility (Fixed all tests)
Status: Needs work » Needs review

Fixed all tests (error and deprecation)!

After this commit, the output of run-tests.sh is:

[obfuscated-user@localhost d10]$ ./run-tests.sh --verbose --directory modules/contrib/commerce_stock

Drupal test run
---------------

Tests to be run:
  - Drupal\Tests\commerce_stock\Kernel\StockServiceManagerTest
  - Drupal\Tests\commerce_stock\Functional\ProductAdminTest
  - Drupal\Tests\commerce_stock\Functional\StockServiceTest
  - Drupal\Tests\commerce_stock_local\Kernel\Entity\StockLocationTest
  - Drupal\Tests\commerce_stock_local\Kernel\LocalStockServiceTest
  - Drupal\Tests\commerce_stock_local\Kernel\LocalStockUpdaterTest
  - Drupal\Tests\commerce_stock_local\Kernel\OrderEventTransactionsKernelTest
  - Drupal\Tests\commerce_stock_local\Kernel\OrderEventsTransactionsTest
  - Drupal\Tests\commerce_stock_local\Kernel\StockLocationStorageTest
  - Drupal\Tests\commerce_stock_field\Kernel\StockLevelTest
  - Drupal\Tests\commerce_stock_field\Functional\StockLevelFormatterTest
  - Drupal\Tests\commerce_stock_field\Functional\StockLevelWidgetsTest
  - Drupal\Tests\commerce_stock_enforcement\Functional\EnforcementBrowserTest
  - Drupal\Tests\commerce_stock_enforcement\Functional\OutOfStockTest

Test run started:
  Wednesday, January 25, 2023 - 23:58

Test summary
------------

Drupal\Tests\commerce_stock\Kernel\StockServiceManagerTest     1 passes
Drupal\Tests\commerce_stock\Functional\ProductAdminTest        2 passes
Drupal\Tests\commerce_stock\Functional\StockServiceTest        1 passes
Drupal\Tests\commerce_stock_local\Kernel\Entity\StockLocatio   1 passes
Drupal\Tests\commerce_stock_local\Kernel\LocalStockServiceTe   1 passes
Drupal\Tests\commerce_stock_local\Kernel\LocalStockUpdaterTe   1 passes
Drupal\Tests\commerce_stock_local\Kernel\OrderEventTransacti   3 passes
Drupal\Tests\commerce_stock_local\Kernel\OrderEventsTransact   7 passes
Drupal\Tests\commerce_stock_local\Kernel\StockLocationStorag   1 passes
Drupal\Tests\commerce_stock_field\Kernel\StockLevelTest        6 passes
Drupal\Tests\commerce_stock_field\Functional\StockLevelForma   2 passes
Drupal\Tests\commerce_stock_field\Functional\StockLevelWidge   6 passes
Drupal\Tests\commerce_stock_enforcement\Functional\Enforceme   2 passes
Drupal\Tests\commerce_stock_enforcement\Functional\OutOfStoc   2 passes

Test run duration: 45 min 19 sec

Detailed test results
---------------------


---- Drupal\Tests\commerce_stock\Functional\ProductAdminTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      ProductAdminTest.   27 Drupal\Tests\commerce_stock\Functio

Pass      Other      ProductAdminTest.   73 Drupal\Tests\commerce_stock\Functio



---- Drupal\Tests\commerce_stock\Functional\StockServiceTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      StockServiceTest.   40 Drupal\Tests\commerce_stock\Functio



---- Drupal\Tests\commerce_stock\Kernel\StockServiceManagerTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      StockServiceManag   38 Drupal\Tests\commerce_stock\Kernel\



---- Drupal\Tests\commerce_stock_enforcement\Functional\EnforcementBrowserTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      EnforcementBrowse   34 Drupal\Tests\commerce_stock_enforce

Pass      Other      EnforcementBrowse   42 Drupal\Tests\commerce_stock_enforce



---- Drupal\Tests\commerce_stock_enforcement\Functional\OutOfStockTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      OutOfStockTest.ph   34 Drupal\Tests\commerce_stock_enforce

Pass      Other      OutOfStockTest.ph   42 Drupal\Tests\commerce_stock_enforce



---- Drupal\Tests\commerce_stock_field\Functional\StockLevelFormatterTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      StockLevelFormatt   28 Drupal\Tests\commerce_stock_field\F

Pass      Other      StockLevelFormatt   42 Drupal\Tests\commerce_stock_field\F



---- Drupal\Tests\commerce_stock_field\Functional\StockLevelWidgetsTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      StockLevelWidgets   24 Drupal\Tests\commerce_stock_field\F

Pass      Other      StockLevelWidgets  155 Drupal\Tests\commerce_stock_field\F

Pass      Other      StockLevelWidgets  242 Drupal\Tests\commerce_stock_field\F

Pass      Other      StockLevelWidgets  357 Drupal\Tests\commerce_stock_field\F

Pass      Other      StockLevelWidgets  452 Drupal\Tests\commerce_stock_field\F

Pass      Other      StockLevelWidgets  470 Drupal\Tests\commerce_stock_field\F

---- Drupal\Tests\commerce_stock_field\Kernel\StockLevelTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      StockLevelTest.ph  151 Drupal\Tests\commerce_stock_field\K

Pass      Other      StockLevelTest.ph  169 Drupal\Tests\commerce_stock_field\K

Pass      Other      StockLevelTest.ph  181 Drupal\Tests\commerce_stock_field\K

Pass      Other      StockLevelTest.ph  191 Drupal\Tests\commerce_stock_field\K

Pass      Other      StockLevelTest.ph  199 Drupal\Tests\commerce_stock_field\K

Pass      Other      StockLevelTest.ph  230 Drupal\Tests\commerce_stock_field\K



---- Drupal\Tests\commerce_stock_local\Kernel\Entity\StockLocationTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      StockLocationTest   54 Drupal\Tests\commerce_stock_local\K



---- Drupal\Tests\commerce_stock_local\Kernel\LocalStockServiceTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      LocalStockService   47 Drupal\Tests\commerce_stock_local\K



---- Drupal\Tests\commerce_stock_local\Kernel\LocalStockUpdaterTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      LocalStockUpdater  149 Drupal\Tests\commerce_stock_local\K



---- Drupal\Tests\commerce_stock_local\Kernel\OrderEventTransactionsKernelTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      OrderEventTransac  240 Drupal\Tests\commerce_stock_local\K

Pass      Other      OrderEventTransac  325 Drupal\Tests\commerce_stock_local\K

Pass      Other      OrderEventTransac  447 Drupal\Tests\commerce_stock_local\K



---- Drupal\Tests\commerce_stock_local\Kernel\OrderEventsTransactionsTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      OrderEventsTransa  262 Drupal\Tests\commerce_stock_local\K

Pass      Other      OrderEventsTransa  299 Drupal\Tests\commerce_stock_local\K

Pass      Other      OrderEventsTransa  318 Drupal\Tests\commerce_stock_local\K

Pass      Other      OrderEventsTransa  340 Drupal\Tests\commerce_stock_local\K

Pass      Other      OrderEventsTransa  376 Drupal\Tests\commerce_stock_local\K

Pass      Other      OrderEventsTransa  453 Drupal\Tests\commerce_stock_local\K

Pass      Other      OrderEventsTransa  557 Drupal\Tests\commerce_stock_local\K



---- Drupal\Tests\commerce_stock_local\Kernel\StockLocationStorageTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      StockLocationStor   49 Drupal\Tests\commerce_stock_local\K
SilviuChingaru’s picture

SilviuChingaru commented
StatusFileSize
new commerce-stock-d10-compat-3330022-8.patch25.32 KB

Patch to add a test.

SilviuChingaru’s picture

SilviuChingaru commented

Against D10 I had tested locally and all pass, after commit, we'll have a dev release for composer to install and test further, D9.x has some deprecated functions that won't pass against PHP 8.1, It's a known issue, but all should pass also from 7.3 to 8.0.
I think this is ready for commit now.

nicklasmf’s picture

nicklasmf commented

I'm getting the following .rej error file when trying to apply the patch to Drupal 9.5.3:

--- modules/field/commerce_stock_field.info.yml
+++ modules/field/commerce_stock_field.info.yml
@@ -7,4 +7,4 @@ dependencies:
   - commerce_stock:commerce_stock
   - commerce_stock:commerce_stock_local
 core: 8.x
-core_version_requirement: ^8 || ^9
+core_version_requirement: ^8 || ^9 || ^10

I am running a D10 site but couldn't install the module and the patch at the same time, so I downgraded to D9.5 and then installed commerce_stock. After this, I tried to apply the patch but got the error above.

Can you confirm that the patch still works for you, and if so, can you elaborate on how I can test the patch?

SilviuChingaru’s picture

SilviuChingaru commented

I queued a test agains Drupal 9.5.2, let's see if it passes.
Meanwhile please post the content of.rej file.

nicklasmf’s picture

nicklasmf commented

Sorry, if it was not clear enough in my post. The code in the grey box is the .rej file.

SilviuChingaru’s picture

SilviuChingaru commented

Can you post exact commands and their output of how you try to patch, please?

guy_schneerson’s picture

guy_schneerson commented

@NicklasMF thank you for your hard work on this.
I Tested on 10.0.3 with commerce 2.33 and all worked so far.
I have not had a chance to test with 9 and I can see how Symfony 6 events may be an issue although it looks like Symfony 5 has an Event class that provides a forward-compatibility layer so maybe it will just work.

nicklasmf’s picture

nicklasmf commented

Sure. I have following in composer.json:

cweagans/composer-patches: 1.7.3
drupal/commerce: 2.33.0
drupal/commerce_stock: 1.0.0

I then add the following:

"patches": {
  "drupal/commerce_stock": {
    "#3330022 D10": "https://www.drupal.org/files/issues/2023-01-26/commerce-stock-d10-compat-3330022-8.patch"
  }
},

And run composer update drupal/commerce_stock in console.

I get following error:

Gathering patches for dependencies. This might take a minute.
  - Installing drupal/commerce_stock (1.0.0): Extracting archive
  - Applying patches for drupal/commerce_stock
    https://www.drupal.org/files/issues/2023-01-26/commerce-stock-d10-compat-3330022-8.patch (#D10)
   Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2023-01-26/commerce-stock-d10-compat-3330022-8.patch

And the file commerce_stock_field.info.yml.rejis created:

--- modules/field/commerce_stock_field.info.yml
+++ modules/field/commerce_stock_field.info.yml
@@ -7,4 +7,4 @@ dependencies:
   - commerce_stock:commerce_stock
   - commerce_stock:commerce_stock_local
 core: 8.x
-core_version_requirement: ^8 || ^9
+core_version_requirement: ^8 || ^9 || ^10
SilviuChingaru’s picture

SilviuChingaru commented
Assigned: SilviuChingaru » Unassigned

Can you try also, please, with:

"patches": {
  "drupal/commerce_stock": {
    "#3330022 D10": "https://git.drupalcode.org/project/commerce_stock/-/merge_requests/14.diff"
  }
},

Thank you!

nicklasmf’s picture

nicklasmf commented
StatusFileSize
new commerce-stock-d10-compat-3330022-17.patch25.25 KB

I don't know why that didn't work neither.
I tried some things back and forth and finally got it to work. I removed one line in the start and added a new line after the specific file.

From line 76:

diff --git a/modules/field/commerce_stock_field.info.yml b/modules/field/commerce_stock_field.info.yml
index 820088b68788c6e770fdb4432ab963d908f75429..d4994864e6db85472d9eca5fc554b74fde16f97f 100644
--- a/modules/field/commerce_stock_field.info.yml
+++ b/modules/field/commerce_stock_field.info.yml
@@ -8,4 +8,4 @@ dependencies:
   - commerce_stock:commerce_stock_local
 core: 8.x
-core_version_requirement: ^8 || ^9
+core_version_requirement: ^8 || ^9 || ^10

diff --git a/modules/field/tests/src/Functional/StockLevelFieldTestBase.php b/modules/field/tests/src/Functional/StockLevelFieldTestBase.php

I'll just add the patch in case it is useful.
And then I'll go test the patch now.

alfthecat’s picture

alfthecat commented

Latest patch applied cleanly, so far it all seems to work.

vipin.j made their first commit to this issue’s fork.

erichhaemmerle’s picture

erichhaemmerle commented

So how do we install this with Drupal 10 then?

I added this to my patches section in composer.json:

"drupal/commerce_stock": {
    "3330022 - Drupal 10 compatibility (Fixed all tests)": "https://www.drupal.org/files/issues/2023-02-26/commerce-stock-d10-compat-3330022-17.patch"
}

but if I do composer require drupal/commerce_stock, I get

Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - drupal/commerce_stock[1.0.0-alpha1, ..., 1.0.0-alpha4] require drupal/core ~8.0 -> found drupal/core[8.0.0-beta6, ..., 8.9.x-dev] but the package is fixed to 10.0.7 (lock file version) by a partial update and that version does not match. Make sure you list it as an argument for the update command.
    - drupal/commerce_stock[1.0.0-alpha5, ..., 1.x-dev] require drupal/core ^8 || ^9 -> found drupal/core[8.0.0-beta6, ..., 8.9.x-dev, 9.0.0-alpha1, ..., 9.5.x-dev] but the package is fixed to 10.0.7 (lock file version) by a partial update and that version does not match. Make sure you list it as an argument for the update command.
    - Root composer.json requires drupal/commerce_stock ^1.0 -> satisfiable by drupal/commerce_stock[1.0.0-alpha1, ..., 1.x-dev].

Use the option --with-all-dependencies (-W) to allow upgrades, downgrades and removals for packages currently locked to specific versions.
You can also try re-running composer require with an explicit version constraint, e.g. "composer require drupal/commerce_stock:*" to figure out if any version is installable, or "composer require drupal/commerce_stock:^2.1" if you know which you need.

Installation failed, reverting ./composer.json and ./composer.lock to their original content.

Doesn't the patch get run AFTER composer downloading the files?

guy_schneerson’s picture

guy_schneerson commented

After testing on both 9 and 10 I committed SilviuChingaru patch commerce-stock-d10-compat-3330022-8.patch
I assume we will still fail tests and we will need a second commit but thought it best to have a dev version with 10 support to work and test on.
Will look at the other proposed changes wen I can.

agoradesign’s picture

agoradesign commented

is there anything left do do? this is the last important blocker for our commerce sites to upgrade to 10.x

guy_schneerson’s picture

guy_schneerson commented

@agoradesign should be good to go. Dev version should be all fine if you can give that a go and confirm that will help.
I am away until Thursday and can push a version then.

agoradesign’s picture

agoradesign commented

Actually I do have the dev installed on 3 10.x sites. No problems so far, but I have to mention that two of them are stell under development and the third one is live though, but not under heavy traffic :D
Anyway, we had one or two orders there since then, as well as saving stock changes via migrate import did not cause any errors as well

guy_schneerson’s picture

guy_schneerson commented
Status: Needs review » Fixed

A big thank you to everyone that helped with this. I just pushed 8.x-1.1 and marked the issue as fixed. If anyone gets any issues specific to D10 then please reopen.

Status: Fixed » Closed (fixed)

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