Drupal 10 compatibility

Hi! As part of Drupal 10 porting day, I am testing this patch.

Reviewed the patch:
1 Downloaded and installed the "Project Update Bot" patch - #4.
2. Examined the module based on Upgrade Status
3. Errors were discovered in the Upgrade Status report.
In summary, the #4 patch appears to be a D10 incompatibility module.
The attache patch #6, which corrects the problems to make the D10 module compatible with the D9 environment, is attached.
The attached problems have not yet been fixed .It has to be fixed in a D10 environment.

CONTRIBUTED PROJECTS
--------------------------------------------------------------------------------
Linkit
Scanned on Fri, 08/26/2022 - 11:34.

7 warnings found.

web/modules/contrib/linkit/src/Plugin/Linkit/Matcher/FileMatcher.php:
┌────────┬──────┬────────────────────────────────────────────────────┐
│ STATUS │ LINE │                      MESSAGE                       │
├────────┼──────┼────────────────────────────────────────────────────┤
│ Fix    │ 60   │ Call to deprecated constant FILE_STATUS_PERMANENT: │
│ later  │      │ Deprecated in drupal:9.3.0 and is removed from     │
│        │      │ drupal:10.0.0. Use                                 │
│        │      │ Drupal\file\FileInterface::STATUS_PERMANENT or     │
│        │      │ \Drupal\file\FileInterface::setPermanent().        │
│        │      │                                                    │
└────────┴──────┴────────────────────────────────────────────────────┘

web/modules/contrib/linkit/src/Plugin/Linkit/Matcher/NodeMatcher.php:
┌────────┬──────┬──────────────────────────────────────────────────────────────┐
│ STATUS │ LINE │                           MESSAGE                            │
├────────┼──────┼──────────────────────────────────────────────────────────────┤
│ Fix    │ 91   │ Call to deprecated method getImplementations() of class      │
│ later  │      │ Drupal\Core\Extension\ModuleHandlerInterface. Deprecated in  │
│        │      │ drupal:9.4.0 and is removed from drupal:10.0.0. Instead you  │
│        │      │ should use ModuleHandlerInterface::invokeAllWith() for hook  │
│        │      │ invocations or you should use                                │
│        │      │ ModuleHandlerInterface::hasImplementations() to determine if │
│        │      │ hooks implementations exist.                                 │
│        │      │                                                              │
└────────┴──────┴──────────────────────────────────────────────────────────────┘

web/modules/contrib/linkit/tests/src/Kernel/LinkitFilterEntityTest.php:
┌────────┬──────┬────────────────────────────────────────────────────┐
│ STATUS │ LINE │                      MESSAGE                       │
├────────┼──────┼────────────────────────────────────────────────────┤
│ Fix    │ 125  │ Call to deprecated constant FILE_STATUS_PERMANENT: │
│ later  │      │ Deprecated in drupal:9.3.0 and is removed from     │
│        │      │ drupal:10.0.0. Use                                 │
│        │      │ Drupal\file\FileInterface::STATUS_PERMANENT or     │
│        │      │ \Drupal\file\FileInterface::setPermanent().        │
│        │      │                                                    │
└────────┴──────┴────────────────────────────────────────────────────┘

web/modules/contrib/linkit/tests/src/Kernel/Matchers/FileMatcherTest.php:
┌────────┬──────┬────────────────────────────────────────────────────┐
│ STATUS │ LINE │                      MESSAGE                       │
├────────┼──────┼────────────────────────────────────────────────────┤
│ Fix    │ 48   │ Call to deprecated constant FILE_STATUS_PERMANENT: │
│ later  │      │ Deprecated in drupal:9.3.0 and is removed from     │
│        │      │ drupal:10.0.0. Use                                 │
│        │      │ Drupal\file\FileInterface::STATUS_PERMANENT or     │
│        │      │ \Drupal\file\FileInterface::setPermanent().        │
│        │      │                                                    │
└────────┴──────┴────────────────────────────────────────────────────┘

web/modules/contrib/linkit/tests/src/Kernel/Matchers/MediaMatcherTest.php:
┌────────┬──────┬────────────────────────────────────────────────────┐
│ STATUS │ LINE │                      MESSAGE                       │
├────────┼──────┼────────────────────────────────────────────────────┤
│ Fix    │ 67   │ Call to deprecated constant FILE_STATUS_PERMANENT: │
│ later  │      │ Deprecated in drupal:9.3.0 and is removed from     │
│        │      │ drupal:10.0.0. Use                                 │
│        │      │ Drupal\file\FileInterface::STATUS_PERMANENT or     │
│        │      │ \Drupal\file\FileInterface::setPermanent().        │
│        │      │                                                    │
└────────┴──────┴────────────────────────────────────────────────────┘

web/modules/contrib/linkit/tests/src/Kernel/SubstitutionPluginTest.php:
┌────────┬──────┬────────────────────────────────────────────────────┐
│ STATUS │ LINE │                      MESSAGE                       │
├────────┼──────┼────────────────────────────────────────────────────┤
│ Fix    │ 109  │ Call to deprecated constant FILE_STATUS_PERMANENT: │
│ later  │      │ Deprecated in drupal:9.3.0 and is removed from     │
│        │      │ drupal:10.0.0. Use                                 │
│        │      │ Drupal\file\FileInterface::STATUS_PERMANENT or     │
│        │      │ \Drupal\file\FileInterface::setPermanent().        │
│        │      │                                                    │
│ Fix    │ 161  │ Call to deprecated constant FILE_STATUS_PERMANENT: │
│ later  │      │ Deprecated in drupal:9.3.0 and is removed from     │
│        │      │ drupal:10.0.0. Use                                 │
│        │      │ Drupal\file\FileInterface::STATUS_PERMANENT or     │
│        │      │ \Drupal\file\FileInterface::setPermanent().        │
│        │      │                                                    │
└────────┴──────┴────────────────────────────────────────────────────┘

@immaculatexavier Thank you for participating in porting day and updating the patch with clear steps on what you did and what's left to do!

Moving to needs work for the rest of the deprecations.

On top of everything in #6, I'm seeing this:

LogicException: system module does not define a schema for table 'key_value_expire'.

→ Change record: https://www.drupal.org/node/3143286.

Next up:

Drupal\Core\Entity\Query\QueryException: Entity queries must explicitly set whether the query should be access checked or not. See Drupal\Core\Entity\Query\QueryInterface::accessCheck().

→ Change record: https://www.drupal.org/node/3201242

Then:

Error: Call to undefined method Drupal\Core\Extension\ModuleHandler::getImplementations()

→ Change record: https://www.drupal.org/node/3000490

Finally:

RuntimeException: Adding non-existent permissions to a role is not allowed. The incorrect permissions are "view test entity translations".

→ that permission was unnecessary!

Finally, linkit_test was not yet marked as compatible with Drupal 10.

All Kernel and Functional tests should now pass.

I made a mistake — #9 does not include #6 🙈

On Drupal 9.3:

1) Drupal\Tests\linkit\Kernel\Matchers\NodeMatcherTest::testNodeMatcherWidthIncludeUnpublished
Error: Call to undefined method Drupal\Core\Extension\ModuleHandler::hasImplementations()

→ because https://www.drupal.org/node/3000490 happened in 9.4.

I think it's reasonable to require Drupal >=9.4 at this time, since the whole point of this issue is Drupal 10 compatibility. Then this module would be compatible with 9.4, 9.5 and 10.0.

While at it, I removed composer.json too, since exactly the same file would be generated by the Drupal.org composer facade — and it was already out of sync with the information in the *.info.yml, which is exactly why it's better to just leave it to the facade.

Missed one FILE_STATUS_PERMANENT spot.

And added a test-only dependency on https://www.drupal.org/project/ckeditor, to allow testing on Drupal 10.

+1 for requiring 9.4, if that's what's needed to make this compatible with D10.

Apparently DrupalCI does not like a composer.json existing in HEAD but not in the patch. W/E, restoring it. 🤷‍♀️

#16 Can remove composer.json through a separate issue if that helps.

A >=9.5-only failure occurs in LinkitEditorLinkDialogTest:

1) Drupal\Tests\linkit\Kernel\LinkitEditorLinkDialogTest::testAdd
Undefined index: toolbar

… which does not actually get changed here. It is most likely failing since https://www.drupal.org/node/3293540

🚀 consistent failures on all versions! 🥳

+++ b/linkit.libraries.yml
@@ -23,8 +23,6 @@ linkit.autocomplete:
-    - core/jquery.ui.autocomplete

This causes

There was 1 error:

1) Drupal\Tests\linkit\FunctionalJavascript\LinkitDialogTest::testLinkDialog
Error: Call to a member function isVisible() on null

/var/www/html/modules/contrib/linkit/tests/src/FunctionalJavascript/LinkitDialogTest.php:180
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

→ Change record: https://www.drupal.org/node/3156376

⇒ should now use https://www.drupal.org/project/jquery_ui_autocomplete — let's find out how that works!

Related: #3076171-174: Provide a new library to replace jQuery UI autocomplete.

+++ b/linkit.libraries.yml
@@ -23,8 +23,6 @@ linkit.autocomplete:
-    - core/underscore

… and next up, this will cause problems.
→ Change record: https://www.drupal.org/node/3273118

> While at it, I removed composer.json too, since exactly the same file would be generated by the Drupal.org composer facade — and it was already out of sync with the information in the *.info.yml, which is exactly why it's better to just leave it to the facade.

Beside not being able to remove it, I'd vote to keep it anyway. test_dependencies should not be used anymore, use require-dev in composer.json instead. It works immediately in patches, test_dependencies only works after being committed when it's then essentially translated to require-dev. And only a having a composer.json allows per patch/MR override of core dependencies, allowing to run tests against D10.

> +1 for requiring 9.4, if that's what's needed to make this compatible with D10.

It's fairly simple to wrap it in a method exists check and have a fallback, which I've done in quite a few modules. But yes, the closer we get to 9.3 EOL, the more ok it is to require 9.4, fine by me.

🙈

Problem 1
    - drupal/jquery_ui_autocomplete[1.1.0, ..., 1.x-dev] require drupal/core ^8 || ^9 -> satisfiable by drupal/core[8.0.0-beta6, ..., 8.9.x-dev, 9.0.0-alpha1, ..., 9.5.x-dev] from composer repo (https://repo.packagist.org) but drupal/core[10.0.x-dev] from path repo (core) has higher repository priority. The packages from the higher priority repository do not match your constraint and are therefore not installable. That repository is canonical so the lower priority repo's packages are not installable. See https://getcomposer.org/repoprio for details and assistance.

We cannot continue here until #3288098: Automated Drupal 10 compatibility fixes is fixed… 😬

Well, jquery_ui stuff is quite a painful dependency with #3277748: Drupal 10 compatibility. That said, I don't think this should be set to postponed, at least that's not how I've worked those kind of issues. I've committed patches that weren't yet testable on D10, although that mostly was about test-only dependencies. And usually there were at least patches that could be tested locally.

Also, #22 hasn't been addressed yet, you really want to use require-dev, not test_dependencies.

That said, per #3277748: Drupal 10 compatibility, this might go in a direction where there is only jquery_ui and all other modules are deprecated, which honestly would make a lot of sense to me and would require that this patch changes, so probably waiting indeed makes sense.

Actually, I do see a way forward to allow this to not be blocked on #3277748: Drupal 10 compatibility 🤓🥳

This uses core/drupal.autocomplete to avoid a dependency on a contrib module with no D10-compatible release. It updates the code for compliance with https://www.drupal.org/node/3158256 and https://www.drupal.org/node/3273118.

+++ b/tests/src/FunctionalJavascript/LinkitDialogTest.php
@@ -160,6 +160,9 @@ class LinkitDialogTest extends WebDriverTestBase {
+    // The dialog is not tall enough to allow the autocomplete to be visible.
+    $this->getSession()->executeScript("document.getElementById('drupal-modal').style.height = '200px';");

FYI this fixes the test failures in #24.

All other changes in #31 are for D10 compatibility.

Should be green now!

LinkitUpdateTest::testLinkitUpdate8500 was marked as being only for Drupal 8 more than a year ago, and Drupal 8 is unsupported at this point, so … no more need for that test!

Tested MR#9 with Drupal 10.0.x and php 8.1

- Was able to install the Linkit module without any issues
- The default profile appears and was able to save default profile
- Created a new profile and added matchers
- On /admin/config/content/formats basic formats I don't see any option to select profile like that of older versions

Moving to NW for this but I might be wrong on this one

#37: that's because you're using CKEditor 5. This issue is not dealing with CKEditor 5 support, just to make Linkit + CKEditor 4 (which requires https://www.drupal.org/project/ckeditor on Drupal 10) work as it did before.

For CKEditor 5 support, see #3232190: CKEditor 5 readiness.

Could you test again with CKEditor 4? 😊🙏

Tested #35 in Drupal 10.1.x using gitpod.io. This is working for me. Here are the tests I took.

1. Installed the Linkit module without any issues
2. Installed ckeditor 4
3. Created a new profile and add Matchers
4. Updated /admin/config/content/formats/manage/full_html. Selected CKeditor (not CKEditor 5)
5. I was able to select the Profile
6. Created 2 Basic nodes. On the second node, I linked to the first page

Thanks Wim, tested again with the steps in #39 and it LGTM.

Thanks!

Yay, thanks @johnwebdev! See you next in #3232190: CKEditor 5 readiness 🤓

@johnwebdev Could you please set up automated tests for 6.0.x-dev at https://www.drupal.org/node/719864/qa? 🙏

@Wim Leers

I added automated tests agains D10 now.

@anon Thanks for creating that, but https://www.drupal.org/pift-ci-job/2517134 is configured to use PHP 5.3, whereas Drupal 10 requires PHP 8.1 or newer 🤓 Could you tweak it? 🙏

Changed to PHP 8.1 and MySQL 8

What's the plan for releasing D10 compatibility?
It seems to be landed on 6.0.x branch, but a release hasn't been created yet.

Indeed, a beta4 would be nice 🤓

Thanks!

Bump, a beta4 would be much appreciated! Looking at the diff it looks like mostly D10 compat changes

Agreed, we would really appreciate an D10 compatibility update as well. Thanks to those that have contributed so far!

@johnwebdev could we please have a tagged release? :) This blocks D10 upgrades.

Thank you, John, Emil, for having time to maintain Linkit. The important editorial module.

Hoping for a soft tag release ( pre-release ), to ease testing with physical Drupal 10 sites.
Testing now with the 6.0.x-dev development branch.

Looking good in our site as well. Thanks. Looking forward to a non-dev release.

Note @mark_fullmer has offered to co-maintain in #3338953: Offering to co-maintain LinkIt. Hopefully that'll help get out a new beta4 release.

Sadly still no release... @Maintainers any plans?

Sadly still no release... @Maintainers any plans?

Pending the resolution of #3338953: Offering to co-maintain LinkIt, I plan to make a D10-compatible release, 6.0.0-alpha1. That will be followed shortly with a subsequent release that includes #3232190: CKEditor 5 readiness.

Module is currently at 6.0.0-beta3, so 6.0.0-alpha1 would be a backwards version.

Or do you mean 7.0.0-alpha1?

Ha. Thanks for keeping me honest! The D10-compatible release should be 6.0.0-beta4. There should be no reason to do a 7.x branch/release for D10 compatibility, given that it doesn't introduce backwards-compatibility-breaking changes.