9c5543bf80956

Problem/Motivation

When I try to install the HTMLMail module via composer on Drupal 9.0.2 installation obtain an error of compatibility and none installation has made.

Steps to reproduce

Create a new project via composer for Drupal 9

$ composer create-project drupal/recommended-project my_site_name_dir
$ composer require drupal/htmlmail

Proposed resolution

I made the requested update to the sources to make the module compliant with Drupal 9.x in the patch attached to this issue

CommentFileSizeAuthor
#6 3163507-6-file-scan-directory.patch906 bytestr
compatibility_for_drupal_9.patch1.47 KBumpire274

Comments

umpire274 created an issue. See original summary.

salvis’s picture

salvis
he/his
commented
Status: Active » Needs review

Thank you for the report and patch, umpire274!

salvis’s picture

salvis
he/his
commented
Status: Needs review » Needs work

The patch contains tabs and other non-standard white space and indenting.

Also, it doesn't follow the advice given in https://api.drupal.org/api/drupal/core%21includes%21file.inc/function/fi...

tr’s picture

tr commented
Title: Compatibility for Drupal 9 » Compatibility for Drupal 9y to install the HTMLMail module via compos

There are a lot more things that need to be fixed to make this module compatible with D9.

For example, the Simpletest needs to be ported to PHPUnit, as Simpletest is no longer supported in D9. See #2990651: Move tests from Simpletest/WebTestBase to PHPUnit/BrowserTestBase

If this is to be done, it would be best IMO to make a meta issue and address each problem individually BEFORE changing the core_version_requirement. It is best to keep the core dependency completely accurate so that people don't try to install this on D9.

tr’s picture

tr commented
Title: Compatibility for Drupal 9y to install the HTMLMail module via compos » file_scan_directory() is deprecated

@salvis: I don't know exactly what you meant in #3 when you said "it doesn't follow the advice given in..."

Aside from the patch format and the coding standards, what there something you thought was wrong about using scanDirectory() here?

tr’s picture

tr commented
Category: Feature request » Task
Status: Needs work » Needs review
StatusFileSize
new 3163507-6-file-scan-directory.patch906 bytes

Here's a re-roll of #1 with only the file_scan_directory() change.

salvis’s picture

salvis
he/his
commented

@salvis: I don't know exactly what you meant in #3 when you said "it doesn't follow the advice given in..."

The documentation says to

Use \Drupal\Core\File\FileSystemInterface::scanDirectory() instead.

tr’s picture

tr commented

Unfortunately most deprecation messages are pretty vague and lacking specifics about exactly how to replace the deprecated code. These messages contain a lot of assumptions.

In this case, we can't call FileSystemInterface::scanDirectory() directly - FileSystemInterface is an interface. So we need find and use an object that implements FileSystemInterface in order to call scanDirectory(). Specifically, we have to get the file_system service class returned from \Drupal::service('file_system'), which is the object we need.

So I think the patch in #6 is the right way to do this. We have to use \Drupal::service() instead of injecting the service because we're doing this in procedural code in the htmlmail.module file.

tr’s picture

tr commented
Issue summary: View changes
Status: Needs review » Fixed

Committed #6 with the wrong commit message, which is why the commit doesn't show up here in this issue. That can't be fixed, as the commit is already pushed.

The commit is: 636563534897c1759

Status: Fixed » Closed (fixed)

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