Problem/Motivation

Make this module compatible with Drupal 11.

Proposed resolution

Remove/update composer.json file with following changes.

"drupal/core": "^10 || ^11",
CommentFileSizeAuthor
#8 warning-examples-2.png64.71 KBabhiyanshu
#8 warning-examples-1.png73.43 KBabhiyanshu
#8 Compatible-with-D11.png8.99 KBabhiyanshu

Issue fork commerce-3462426

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

chandu7929 created an issue. See original summary.

chandu7929 opened merge request !279

chandu7929’s picture

chandu7929 commented
Status: Active » Needs review
jsacksick’s picture

jsacksick commented

There is also the core_version_requirement that has to change, and there is more that's needed, we need to remove the use of deprecated code... Was planning to come to that at some point.

abhiyanshu’s picture

abhiyanshu commented
Assigned: Unassigned » abhiyanshu
jsacksick’s picture

jsacksick commented
Status: Needs review » Needs work
chandu7929’s picture

chandu7929 commented

Thanks @jsacksick if that's the case, we should broaden the tests to see what actually needs to be fixed. When I looked at the module's info.yml, I saw it was updated to ^10 || ^11

abhiyanshu’s picture

abhiyanshu commented
Assigned: abhiyanshu » Unassigned
StatusFileSize
new Compatible-with-D11.png8.99 KB
new warning-examples-1.png73.43 KB
new warning-examples-2.png64.71 KB

@chandu7929 While scanning this module via Upgrade Status, I initially found 54 problems. After addressing many of them, only 40 warnings remain.
I can confirm that apart from these warnings, the module is compatible with Drupal 11.

However, most of the warnings appearing are related to the same issue mentioned below.
i.e..,
Twig template modules/contrib/commerce/modules/payment/templates/commerce-payment-total-summary.html.twig contains a syntax error and cannot be parsed.

The 'commerce_cart/admin' library is not defined because the defining extension is not installed. Cannot decide if it is deprecated or not.

Support from all Views contextual filter settings for the default_argument_skip_url setting is removed from drupal:11.0.0. No replacement is provided. See https://www.drupal.org/node/3382316.

For additional clarity, please refer to the attached screenshot. Thanks!

abhiyanshu’s picture

abhiyanshu commented
Issue tags: +Drupal 10 compatibility fixes

vishalkhode made their first commit to this issue’s fork.

jsacksick’s picture

jsacksick commented
Assigned: Unassigned » jsacksick

There are some changes that seem wrong in the MR...
Also, since this is 3.x, let's require Drupal 10.3 minimum so we can simplify code like the following:

method_exists($renderer, 'renderInIsolation') ? 'renderInIsolation': 'renderPlain';

I'll take this over, thanks for the work done so far.

chandu7929’s picture

chandu7929 commented

Hello @jsacksick, I would request that you keep the temp commit as is (done for pulling incompatible modules that require running the D11 test in CI) until you are about to merge the PR changes. This will help other modules that require this module as dependencies, e.g., google_tag.

Thanks!

jsacksick’s picture

jsacksick commented

This commit? https://git.drupalcode.org/project/commerce/-/merge_requests/279/diffs?c...

Profile is already D11 compatible, so I don't understand why we'd need to pull a specific MR? Will work on D11 compatibility for State machine, as for IEF, that might still be needed yes.

chandu7929’s picture

chandu7929 commented

Yes, I am talking about those commits, also profile is not compatible, listing compatibility on module page doesn't make compatible until its actually gets downloaded for D11 see my comments: https://www.drupal.org/project/profile/issues/3438568

If we allow dependent module to be pulled in using vcs, we can get opportunity to run the integration testing on D11 with other modules.

Following change will be needed to run this module test on D11. I'll take look for State machine & IEF as well, all of the required module of commerce are under my radar.

diff --git a/composer.json b/composer.json
index 6568278a..f19bb8db 100644
--- a/composer.json
+++ b/composer.json
@@ -11,18 +11,44 @@
         "drupal/address": "^2.0",
         "drupal/entity": "^1.0",
         "drupal/entity_reference_revisions": "~1.0",
-        "drupal/inline_entity_form": "^1.0@RC || ^3.0@RC",
-        "drupal/profile": "^1.2",
-        "drupal/state_machine": "^1.5",
+        "drupal/inline_entity_form": "^1.0@RC || ^3.0@RC || dev-3438428-automated-drupal-11",
+        "drupal/profile": "^1.2 || dev-3438568-automated-drupal-11",
+        "drupal/state_machine": "^1.5 || dev-project-update-bot-only",
         "drupal/token": "^1.7",
         "commerceguys/intl": "^2.0.6"
     },
     "require-dev": {
-        "drupal/entity_print": "^2.2",
-        "drupal/panelizer": "^5.0",
-        "drupal/physical": "^1.3",
+        "drupal/entity_print": "^2.2 || dev-3430201-automated-drupal-11",
+        "drupal/panelizer": "^5.0 || dev-3433802-automated-drupal-11",
+        "drupal/physical": "^1.3 | dev-project-update-bot-only",
         "drupal/mailsystem": "^4.3"
     },
+    "repositories": {
+        "state_machine": {
+            "type": "vcs",
+            "url": "https://git.drupalcode.org/issue/state_machine-3434761.git"
+        },
+        "profile": {
+            "type": "vcs",
+            "url": "https://git.drupalcode.org/issue/profile-3438568.git"
+        },
+        "inline_entity_form": {
+            "type": "vcs",
+            "url": "https://git.drupalcode.org/issue/inline_entity_form-3438428.git"
+        },
+        "entity_print": {
+            "type": "vcs",
+            "url": "https://git.drupalcode.org/issue/entity_print-3430201.git"
+        },
+        "panelizer": {
+            "type": "vcs",
+            "url": "https://git.drupalcode.org/issue/panelizer-3433802.git"
+        },
+        "physical": {
+            "type": "vcs",
+            "url": "https://git.drupalcode.org/issue/physical-3433920.git"
+        }
+    },
jsacksick’s picture

jsacksick commented

Profile & State machine both have D11 compatible releases...

chandu7929’s picture

chandu7929 commented

Great!, Now I can see test running against D11 :https://git.drupalcode.org/issue/commerce-3462426/-/pipelines/232146

chandu7929’s picture

chandu7929 commented

The failing test against D11 are here which need action: https://git.drupalcode.org/issue/commerce-3462426/-/jobs/2206124

rajeshreeputra’s picture

rajeshreeputra
he/him
Location Pune
commented

The changes look good. However, we cannot merge them until the `composer.json` file is updated 🙂.

jsacksick’s picture

jsacksick commented

No this can't be merged as is and yes, we're going to need to remove the custom repositories and custom branches.
But in order for Commerce to be fully compatible with D11, we need to make sure our dependencies have D11 compatible releases.

jsacksick’s picture

jsacksick commented

Tests are now finally green! However wondering if we shouldn't wait for the dependencies to have D11 compatible releases before merging this? Or I guess we can merge the changes and tests will pass once the dependencies will have D11 releases?

chandu7929’s picture

chandu7929 commented

The only thing blocking this MR to merge and release is "drupal/inline_entity_form": "dev-3438428-automated-drupal-11", once we have IEF merge and release this should good to merge.

We can ignore dev dependencies for now.

jsacksick’s picture

jsacksick commented

I think I can still go ahead and merge those changes since the required changes for D11 to work are non dependent on Commerce anymore, and I wouldn't want the changes to become stale and have to handle conflicts later...

chandu7929’s picture

chandu7929 commented

Make sense, but need to keep this issue open for other changes needed for D11 compatible release.

jsacksick’s picture

jsacksick commented

Make sense, but need to keep this issue open for other changes needed for D11 compatible release.

What are these?

chandu7929’s picture

chandu7929 commented

What are these?

Update latest release of IEF version in composer.json so that any module requiring commerce module should be able to download it. Later on for dev dependencies as well in order to make D11 CI passing

jsacksick’s picture

jsacksick commented

Update latest release of IEF version in composer.json so that any module requiring commerce module should be able to download it. Later on for dev dependencies as well in order to make D11 CI passing

That isn't needed, as soon as a D11 compatible release is going to be tagged, it is going to be picked up... Unless D11 support is added in a new major branch, but I'd expect the IEF 3.x branch to support D11, need to coordinate with the other maintainers on that.

jsacksick’s picture

jsacksick commented
Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

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