Drupal 9 Deprecated Code Report

Had a go at this:

• simplenews_recent_newsletters() looks like it could be removed? Is it just a leftover from D7?

• ------ ---------------------------------------------------------------------------------- 
    Line   src/Plugin/monitoring/SensorPlugin/PendingSensorPlugin.php                        
   ------ ---------------------------------------------------------------------------------- 
           Class Drupal\monitoring\SensorPlugin\SensorPluginBase not found and could not be autoloaded.                                                                       
   ------ ---------------------------------------------------------------------------------- 

  This is a false positive, this is for integrating with https://www.drupal.org/project/monitoring

Addressed all the other things in the description.

Tagging for DrupalCon Seattle Friday contribution sprint.

See https://github.com/mglaman/drupal-check/wiki/Drupal-9-Readiness

It seems that REQUEST_TIME has not been converted in the original patch. Here's the re-rolled patch.

Patch doesn't apply.

First, it's in -p0 format instead of -p1 as required.

Second, the \Drupal:: construct should not appear anywhere in the OO code - that's only appropriate for procedural code. Instead, you should be injecting the service.

Specifically, \Drupal::entityTypeManager() should not be used, \Drupal::time(), \Drupal::service('renderer'), \Drupal::database() etc. should not be used unless it's in procedural code. There are only a few small hunks of this patch that are correct because they apply to procedural code (in simplenews.module, simplenews.install, simplenews.tokens.inc) - all the rest of the patch needs to inject the appropriate service and use that, otherwise it's no better that what's currently there.

While it's easy to run a script and identify deprecated functions, it's a lot more difficult to address them PROPERLY.

Thanks @TR for describing the problems with the current patch.

I hope we can look at this as an opportunity to teach and learn how to do it properly. And I find these tools a great way to bootstrap new contributors.

I hope we can look at this as an opportunity to teach and learn how to do it properly. And I find these tools a great way to bootstrap new contributors.

Unfortunately, while the tool is useful, the way the tool is USED is not very productive IMO. The reason is that new contributors blindly run the script on dozens of projects at the same time, then post similar issues for those dozens of projects, then dozens of project managers need to respond with the same information on how to properly fix things. This is cross-posting writ large (we actively discourage cross-posting in other contexts ...) And to top it off, the new contributors never follow up with new patches, so it seems that the dozens of explanations were lost on an audience that isn't interested in learning.

It would be far more productive for everyone if the new contributor posted one and only one issue, then followed up with that issue learning along the way. Once the contributor has gone through the process for one issue, and has learned how to properly address the problems pointed out by the script, THEN the contributor should feel free to open up the same issue on dozens of other projects

Pardon my cynicism - it's just that I've dealt with too many of these sorts of posts on the projects that I maintain. Although initially I spent a lot of effort describing what was wrong with the patches and why, I found I didn't receive ANY follow-up patches or help to complete the task. Additionally, many of the problems pointed out by the script ALREADY have open issues (with non-trivial fixes) that have been ignored by the person who posts the new issue based on the script output. So I have come to view these posts as a nuisance.

@tatarbj: If you want or someone else want to address this at your DrupalCamp this weekend, I will be happy to review any patches posted and I will try to respond promptly so that the issue can fixed while the camp is still in progress.

Thanks to everyone contributing - your work is useful every time we get closer to a working patch. Any time you have a better patch than the current one please upload it even if it's not perfect. You can use a comment to explain which parts are done and which left to do.

As @TR says, dependency injection will be necessary in some cases. However in my view it isn't a requirement in test code. For any forms there are already many traits available on FormBase to provide common services.

@AdamPS
I added dependency injection almost in the all files that was in the drupal-check list. Exception is:
src/Mail/MailEntity.php
src/Spool/SpoolList.php
There we need to decide what to do - move them to the services or something else...

Regarding to the tests files. I replaced deprecated code but with \Drupal:: calls. In any case we should re-write it to use PHPUnit, am I right?

Great, thanks w@drupal.

@TR I would welcome your review as you seem to be an expert here.

This is a big patch, and it is likely to force other issues with patches to need manual reroll. The fewer lines you change the better please. Please try to avoid adding "extra" changes that are not related to deprecated code. For example please don't fix coding standards. If possible don't change indentation on lines that are otherwise unchanged.

Exception is: ... There we need to decide what to do - move them to the services or something else...

Please can you leave them with \Drupal:: calls. Large refactoring would need to be a separate issue - but there are other issues that might fix them anyway.

In any case we should re-write it to use PHPUnit, am I right?

Yes but not in this issue - see #3055728: Convert from Simpletests to PHPUnit tests

@TR: I do agree with you in parts (I've been pushing to better consider how contrib maintainers want to maintain their projects and address the D9 readiness topic, and people are working on that), but your tone is pretty harsh and I think uncalled for.

The drupal-check approach is *vastly* better than the random one-off deprecation issues that we used to get before, and I definitely see much more collaboration already on those issues than before.

Yes, it is a problem that many people who contribute especially during code sprints at events tend to start issues/write a first patch and then not follow-up. That's not really something we can fix, but this issue already proves you wrong on parts of what you said, because there are now already 3 people collaborating and picking up existing work instead of creating overlapping duplicates.

So, back to the topic.

1. +++ b/simplenews.module
   @@ -259,7 +260,9 @@ function simplenews_form_node_type_submit(array $form, FormStateInterface $form_
    
        // Set the default widget.
   -    entity_get_form_display('node', $node_type->id(), 'default')
   +    \Drupal::entityTypeManager()
   +      ->getStorage('entity_form_display')
   +      ->load('node.' . $node_type->id() . '.default')
   

   I would strongly recommend to not touch these functions and instead use the new replacement that's coming in 8.7: https://www.drupal.org/node/2835616

2. +++ b/simplenews.module
   @@ -643,7 +650,9 @@ function simplenews_subscriber_load_by_uid($uid) {
     */
    function simplenews_subscriber_load_multiple($snids = array()) {
   -  return entity_load_multiple('simplenews_subscriber', $snids);
   

   I'd instead deprecate our own load function too and stop using that, rely on Subscriber::load() in functional code/tests or the entity storage elsewhere.

3. +++ b/simplenews.module
   @@ -834,7 +846,12 @@ function simplenews_newsletter_get_all() {
    function simplenews_newsletter_load_multiple($ids = NULL, $reset = FALSE) {
   -  return entity_load_multiple('simplenews_newsletter', $ids, $reset);
   +  $storage = \Drupal::entityTypeManager()
   +    ->getStorage('simplenews_newsletter');
   +  if ($reset) {
   +    $storage->resetCache($ids);
   +  }
   +  return $storage->loadMultiple($ids);
   

   same.

4. +++ b/src/EventSubscriber/MigrationSubscriber.php
   @@ -28,10 +36,13 @@ class MigrationSubscriber implements EventSubscriberInterface {
       *
   +   * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entityTypeManager
   +   *   The entity type manager service.
       * @param \Drupal\Core\Entity\EntityFieldManagerInterface $entityFieldManager
       *   The entity field manager service.
       */
   -  public function __construct(EntityFieldManagerInterface $entityFieldManager) {
   +  public function __construct(EntityTypeManagerInterface $entityTypeManager, EntityFieldManagerInterface $entityFieldManager) {
   +    $this->entityTypeManager = $entityTypeManager;
        $this->entityFieldManager = $entityFieldManager;
   

   I'd suggest to add new arguments at the end.

   The approach in core is make them optional and add a \Drupal::service() fallback, not sure if we need to that here too.

   That said, this specific example is only needed for the form display thing, which has a new replacement that will require a different service, so skip this class completely.

5. +++ b/src/Form/NodeTabForm.php
   @@ -2,16 +2,91 @@
   +   */
   +  public function __construct(SpoolStorageInterface $spool_storage, RecipientHandlerManager $recipient_handler_manager, AccountInterface $current_user, EmailValidatorInterface $email_validator, MailerInterface $simplenews_mailer) {
   +    $this->spoolStorage = $spool_storage;
   

   there is a BC problem with the email validator in 8.6 => 8.7, so this will likely only work in other version. Again, probably better to skip that and wait for 8.7.

6. +++ b/src/Tests/SimplenewsSendTest.php
   @@ -530,15 +530,15 @@ class SimplenewsSendTest extends SimplenewsTestBase {
        // Verify.
   -    \Drupal::entityManager()->getStorage('node')->resetCache();
   +    \Drupal::entityTypeManager()->getStorage('node')->resetCache();
        $this->assertFalse(Node::load($node->id()));
   -    $spooled = db_query('SELECT COUNT(*) FROM {simplenews_mail_spool} WHERE entity_id = :nid AND entity_type = :type', array(':nid' => $node->id(), ':type' => 'node'))->fetchField();
   +    $spooled = \Drupal::database()->query('SELECT COUNT(*) FROM {simplenews_mail_spool} WHERE entity_id = :nid AND entity_type = :type', array(':nid' => $node->id(), ':type' => 'node'))->fetchField();
   

   Some people disagree but IMHO \Drupal::.. is perfectly fine in tests, doesn't matter if simpletest or phpunit.. converting them is usually pretty trivial (with the exception of tests that require JS).

NB The key goal of this function is to move away from deprecated code without introducing bugs - this is required before D9. We should also try to minimise breaking of patches on other issues.

A secondary goal is to use dependency injection instead of Drupal::. For sure we should do this unless there's a good reason, but Drupal:: will work in D9, so we don't have to remove them all for Drupal 9 compatibility.

We can easily have multiple commits. If in doubt, please leave code unchanged in the next patch. That way hopefully we can commit 90% of the problems quickly and safely.

Clarification of #16

1. The replacement is coming in D8.8. Please leave all calls to entity_get_form_display unchanged.

2/3. I have raised #3055868: Delete entity load/delete wrappers for the deprecation. Please leave these functions unchanged in this issue.

5. This is the CR for D8.7.

@Berdir and @AdamPS thanx for your comments.

I applied changes that you described above. Not sure about simplenews_newsletter_load_multiple() and simplenews_subscriber_delete_multiple(), should I also leave them as-is?

Thanks again w@drupal. It's looking pretty good.

1) Main task remaining is the test fails. It looks like the same one or two errors repeated over and over, so hopefully not too hard to fix.

2) I updated #3055868: Delete entity load/delete wrappers. The full list of functions that you don't need to change because they will be removed is:

• simplenews_subscriber_load()
• simplenews_subscriber_load_by_uid()
• simplenews_subscriber_load_by_uid()
• simplenews_subscriber_load_multiple()
• simplenews_subscriber_delete_multiple()
• simplenews_newsletter_load()
• simplenews_newsletter_load_multiple()

3)

+ $filtered_html_format = \Drupal::entityTypeManager()
+ ->getStorage('filter_format')
+ ->create(array(

Please can you put this all on one line, then don't add extra indent to all the following lines.

I fixed "Object of class Drupal\Core\Url could not be converted to string" error and added @AdamPS remarks

And fix for "Object of class Drupal\Core\Url could not be converted to intstrpos"

Great thanks w@drupal. For SubscriberMassUnsubscribeForm.php, the patch just adds a "use" statement so I didn't commit that file.

Now postposed waiting for D8.8 - I updated the IS. If someone can rerun the deprecated summary report and add it to the IS that would be useful.

Revert and commit again to fix the credit

There are a few deprecation problems active for 8.7 that don't need us to wait until 8.8 is released. I've updated the issue summary with those problems.

@AdamPS imho we should avoid revert commits whenever possible. If you forgot attribution i usually add an empty commit (--allow-empty) with the updated message. Patch rerolls or merges / rebases of forks with conflicts otherwise can cause big pain or at least confusion.

@miro_dietiker ah sorry, yes I didn't think of that, your suggestion is much better and I will do that in future

Update the deprecated usages for the current 8.x-2.x (dev) branch.

@w,drupal you are assigned to this issue but perhaps that is just left over from before. Are you willing to help us out once more? If not please can you assign to none? Thanks.

@AdamPS unassigned myself from this task

@ilchovuchkov Can you run it with just the deprecation messages, and not all possible static analysis messages?

All those warnings about uninitialised variables and usage of dependency injection are not deprecated code. They are simply better programming practices.

Updated the issue summary back to the all the currently deprecated removing the entity_get_form_display() and entity_get_display() deprecations, which must wait another month.

The email.validator service was introduced in 8.6, so we also should wait for 8.8 for that one.. No point in not starting to work on all of them already.

I'm updating the issue summary to include all 8.8 deprecations. We're only a month away, anyway.

@jcnventura I was too late here. Thanks for the summary's update.

Upload a patch for the replacement of D9 deprecated functions/methods.

Changed #41 slightly to use proper dependency injection of the email.validator service (and took the opportunity to DI also the simplenews.subscription_manager service), and to DI the entity_display.repository service in src/EventSubscriber/MigrationSubscriber.php.

I undid the change from User::getUserName() to User::getDisplayName(), using instead User::getAccountName(), as that's the one that keeps the code working as it currently is. It is very likely that User::getDisplayName() is the right method to use, but that should be a new issue, and we should not change functionality in this issue.

Re #43 - While I agree on using dependency injection in general, I don't think we should be doing this here. Changing the signature of these services is a backwards compatibility breaking change, and people making use of these services directly will have their code stop working. I believe that should only be done on when upgrading to a major version.

@Manuel Garcia, we are currently doing precisely that, and still in alpha, where it's very acceptable to do that.

Test passed on rerun, setting back to 'Needs review'.

Added a child issue to address further DI issues, as we really should fix those before the release of 2.0-beta1.

Great thanks @jcnventura. However this patch seems to fail on D8.7 which is the current stable release - D8.8 is only alpha. So I think that we should wait a little longer until D8.8 is stable.

1) Please can you explain the rearrangement? Is there some Drupal coding standard against assignment within if?
--- a/src/Mail/Mailer.php
+++ b/src/Mail/Mailer.php
@@ -402,8 +402,9 @@ class Mailer implements MailerInterface {
if (!empty($mail)) {
$subscriber = Subscriber::loadByMail($mail, 'create', $this->languageManager->getCurrentLanguage());

- if ($account = $subscriber->getUser()) {
- $recipients['user'][] = $account->getUserName() . ' <' . $mail . '>';
+ $account = $subscriber->getUser();
+ if ($account) {
+ $recipients['user'][] = $account->getAccountName() . ' <' . $mail . '>';
}
else {
$recipients['anonymous'][] = $mail;

There's no Drupal rules against it (there should be...), but it's terribly unreadable code. Someone reading that might thing that condition is a comparison, when it is in fact an assignment.

Code should be easy to read, and no one should read that line and think 'if $account is equal to $subscriber->getUser() then do..'. The rearrangement makes the code clearer, and I cannot believe that PHP will be that much faster because of doing the assignment in the if. I changed it because, when I read that, I first thought it was a comparison before noticing it was an assignment.

One coding standard that specifically forbids this is the CERT C Coding Standard: https://wiki.sei.cmu.edu/confluence/display/c/EXP45-C.+Do+not+perform+as.... And yes, I do know that PHP is not C. The logic why CERT decided to make this a coding standard fully applies to PHP as well.

> There's no Drupal rules against it (there should be...), but it's terribly unreadable code.

But as you said, there isn't a rule for that in Drupal. It exists 1000+ times in core alone. IMHO changes that aren't clearly defined in the coding standard should be avoided to avoid discussions like this.

And yes, using 8.8 API means this needs to be postponed until the module decides to rely on that, and I usually wait at least a few months after a release, 8.7 still has security support until next summer. This being the new alpha branch makes this a bit different, so we might require that earlier, but that will depend I guess on when @AdamPS will update his projects and actually can use it there too. I'm not very involved right now in 8.x-2.x.

Also, I would actually suggest to use getDisplayName() there, not account name, as it is displayed to the user. On sites relying on that feature, the account might be the e-mail or some kind of generated string/number.

As to the fact that this code still breaks 8.7, indeed. We should probably split this into stuff that can already be committed once 8.8 is released next month - i.e compatible with >= 8.7 - and stuff that must wait 6 months for 8.9.

Hi, after apply patch comment #42 and running drupal-check.

------ ------------------------------------------------- 
  Line   simplenews.drush.inc                             
 ------ ------------------------------------------------- 
  50     Call to deprecated function drush_get_option().  
  55     Call to deprecated function drush_log().         
  86     Call to deprecated function drush_get_option().  
  92     Call to deprecated function drush_log().         
  93     Call to deprecated function drush_log().         
 ------ ------------------------------------------------- 

Need work about Call to deprecated function drush_get_option().
Thanks,

Great thanks. Please can you clarify what version of Drupal Core this patch is intended for. Right now we would like a patch that is correct for D8.8. Hopefully that's what you made. If so then we need to run the tests against D8.8 - I'll queue that now.

I think that drush_get_option() is fine - that file is for Drush 8 and that function exists in Drush 8. I guess you ran your report against a newer drush version and saw the warning because of that.

1. +++ b/tests/modules/simplenews_test/simplenews_test.info.yml
   @@ -4,4 +4,4 @@ package: Mail
    hidden: true
    core: 8.x
    dependencies:
   -  - simplenews
   +  - simplenews:simplenews
   

   Since the code requires 8.8 now, we should define that by replacing core: 8.x with the new core_version_constraint key: https://www.drupal.org/node/3070687

   So that would be "core_version_requirement: ^8.8 || ^9".

   Also, drupal-check doesn't find all issues. Another way to find problems is to use the deprecations check in PhpStorm if you have that and/or running all tests with phpunit and deprecations enabled.

   That will for example report about the new $defaulTheme property that needs to be set on all (base) test classes.

2. +++ b/tests/src/Functional/SimplenewsRecipientHandlerTest.php
   @@ -22,7 +22,10 @@ class SimplenewsRecipientHandlerTest extends SimplenewsTestBase {
   -    user_delete_multiple(\Drupal::entityQuery('user')->condition('uid', 0, '>')->execute());
   +    $ids = \Drupal::entityQuery('user')->condition('uid', 0, '>')->execute();
   +    $controller = \Drupal::entityTypeManager()->getStorage('user');
   +    $entities = $controller->loadMultiple($ids);
   +    $controller->delete($entities);
        simplenews_cron();
   

   $controller => $storage.

3. +++ b/tests/src/Functional/SimplenewsTestBase.php
   @@ -162,11 +162,11 @@ abstract class SimplenewsTestBase extends BrowserTestBase {
          'bundle' => $bundle,
        ])->save();
   -    entity_get_form_display($entity_type, $bundle, 'default')
   +    \Drupal::service('entity_display.repository')->getFormDisplay($entity_type, $bundle)
          ->setComponent($field_name, [
            'type' => 'string_textfield',
          ])->save();
   -    entity_get_display($entity_type, $bundle, 'default')
   +    \Drupal::service('entity_display.repository')->getViewDisplay($entity_type, $bundle)
          ->setComponent($field_name, [
   

   These are the new replacements in 8.8, so yes, that was run against 8.8.0 (yay for that finally being out).

I can see one outstanding point:

#42:

I undid the change from User::getUserName() to User::getDisplayName(), using instead User::getAccountName(), as that's the one that keeps the code working as it currently is. It is very likely that User::getDisplayName() is the right method to use, but that should be a new issue, and we should not change functionality in this issue.

#50:

Also, I would actually suggest to use getDisplayName() there, not account name, as it is displayed to the user. On sites relying on that feature, the account might be the e-mail or some kind of generated string/number.

I think #50 is a valid point. Isn't the whole point of the original core CR to persuade modules to correct their code?

@AdamPS

Not sure what your plan is when you want to require 8.8, I guess 8.x-2.x being alpha means you can basically switch as early as you want aka you update your own sites. But I don't know yet how quickly our sites will be able to switch the 2.x branch and/or when it will become (officially) more stable than 1.x so I think I'll also make 8.x-1.x D9 compatible.

So, if you commit this, please keep it open and assign to 8.x-1.x and I'll adapt and commit it there once we're starting to test/prepare our sites.

Also, drupal-check doesn't find all issues.

True. But even so I think we should commit a change that fixes everything reported by drupal-check. We can always make more commits later.

Yes I had planned to commit this to 2.x dev right away. I would wait a while before releasing another alpha.

I had originally intended to add more non-BC changes into 2.x but that's no longer realistic. I now plan to make a 2.x Beta as soon as I have a fix for #3059184: View migration for 8.x-1.x to 8.x-2.x. It's not desirable to maintain 2 major branches in the long term. There are lots of high-severity fixes in 2.x missing from 1.x so we should start to get the live sites moving over soon.

So, if you commit this, please keep it open and assign to 8.x-1.x and I'll adapt and commit it there once we're starting to test/prepare our sites.

Sure. But with luck I might get 2.x beta in time so you don't need to.

True. But even so I think we should commit a change that fixes everything reported by drupal-check. We can always make more commits later.

Fine with me, but adding the correct dependency is important or people will install it on 8.7 and it might break (the 8.8-dependency is just in tests now I think, but still). Note that it also needs to be done in all .info.yml files. So I think that and the correct variable name should be done still.

And adding the constraint will also allow to actually test it on D9, although we also need a corresponding composer.json addition at the moment per the change record, forgot to mention that. We're making good progress on removing deprecated stuff in the D9 branch, so once committed, you can start a test run against 9.0.x to see how it goes.

Sure. But with luck I might get 2.x beta in time so you don't need to.

Sounds good, although I might still minimally maintain it for a bit, it does work for us as it is and we support some large sites with heavily customized newsletter, so I'll be wary of updating them without extensive testing.

Btw, I started a local test run with the patch applied, will post the results once that finished.

PhpStorm found a usage each of DRUPAL_ANONYMOUS_RID and DRUPAL_AUTHENTICATED_RID in simplenews.install, that's a trivial fix. And lots of usages of REQUEST_TIME, that is deprecated too but also not yet properly removed in core, so might or might not actually be removed in D9.

And these are the reported deprecations from running the whole test suite:

  46x: Drupal\Tests\BrowserTestBase::$defaultTheme is required in drupal:9.0.0 when using an install profile that does not set a default theme. See https://www.drupal.org/node/3083055, which includes recommendations on which theme to use.
    1x in SimplenewsSourceTest::testSendNoCaching from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testSendFail from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testDelete from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testImpersonation from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testNewsletterTheme from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testHtmlEscaping from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSendMinimalSourceImplementation from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSendCaching from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSendHtml from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSendHidden from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSendMissingNode from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testSendPublishNoCron from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSendMissingSubscriber from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSkip from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSubscribeTest::testSubscribeMultiple from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSubscribeTest::testSubscribeAnonymous from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSubscribeTest::testSubscribeAnonymousSingle from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSubscribeTest::testSubscribeAuthenticated from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSubscribeTest::testSimplenewsSubscriptionBlock from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSubscribeTest::testAdminCreate from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSynchronizeFieldsFormTest::testSubscriberFormFieldSync from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testUpdateNewsletter from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testSendNowCron from Drupal\Tests\simplenews\Functional
    1x in SimplenewsDemoTest::testInstalled from Drupal\Tests\simplenews_demo\Functional
    1x in SimplenewsPersonalizationFormsTest::testSynchronizeRegisterSubscribe from Drupal\Tests\simplenews\Functional
    1x in SimplenewsAdministrationTest::testNewsletterSettings from Drupal\Tests\simplenews\Functional
    1x in SimplenewsAdministrationTest::testSubscriptionManagement from Drupal\Tests\simplenews\Functional
    1x in SimplenewsAdministrationTest::testContentTypes from Drupal\Tests\simplenews\Functional
    1x in SimplenewsAdministrationTest::testSubscriberStatusFilter from Drupal\Tests\simplenews\Functional
    1x in SimplenewsAdministrationTest::testNewsletterIssuesOverview from Drupal\Tests\simplenews\Functional
    1x in SimplenewsAdministrationTest::testAccess from Drupal\Tests\simplenews\Functional
    1x in SimplenewsFieldUiTest::testContentTypeCreation from Drupal\Tests\simplenews\Functional
    1x in SimplenewsI18nTest::testNewsletterIssueTranslation from Drupal\Tests\simplenews\Functional
    1x in SimplenewsPersonalizationFormsTest::testSynchronizeSubscribeRegister from Drupal\Tests\simplenews\Functional
    1x in SimplenewsPersonalizationFormsTest::testSubscribeRequestPassword from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testSendNowCronThrottle from Drupal\Tests\simplenews\Functional
    1x in SimplenewsPersonalizationFormsTest::testDisableAccount from Drupal\Tests\simplenews\Functional
    1x in SimplenewsPersonalizationFormsTest::testDeleteAccount from Drupal\Tests\simplenews\Functional
    1x in SimplenewsPersonalizationFormsTest::testBlockedSubscribe from Drupal\Tests\simplenews\Functional
    1x in SimplenewsRecipientHandlerTest::testSiteMail from Drupal\Tests\simplenews\Functional
    1x in SimplenewsRecipientHandlerTest::testNewUsers from Drupal\Tests\simplenews\Functional
    1x in SimplenewsRecipientHandlerTest::testSubscribersByRole from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testProgrammaticNewsletter from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testSendNowNoCron from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testSendMultipleNoCron from Drupal\Tests\simplenews\Functional
    1x in SimplenewsUninstallTest::testUninstall from Drupal\Tests\simplenews\Functional

  46x: Any entity_reference_autocomplete component of an entity_form_display must have a match_limit setting. The uid field on the node.simplenews_issue.default form display is missing it. This BC layer will be removed before 9.0.0. See https://www.drupal.org/node/2863188
    1x in SimplenewsSourceTest::testSendNoCaching from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testSendFail from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testDelete from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testImpersonation from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testNewsletterTheme from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testHtmlEscaping from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSendMinimalSourceImplementation from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSendCaching from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSendHtml from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSendHidden from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSendMissingNode from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testSendPublishNoCron from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSendMissingSubscriber from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSkip from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSubscribeTest::testSubscribeMultiple from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSubscribeTest::testSubscribeAnonymous from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSubscribeTest::testSubscribeAnonymousSingle from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSubscribeTest::testSubscribeAuthenticated from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSubscribeTest::testSimplenewsSubscriptionBlock from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSubscribeTest::testAdminCreate from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSynchronizeFieldsFormTest::testSubscriberFormFieldSync from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testUpdateNewsletter from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testSendNowCron from Drupal\Tests\simplenews\Functional
    1x in SimplenewsDemoTest::testInstalled from Drupal\Tests\simplenews_demo\Functional
    1x in SimplenewsPersonalizationFormsTest::testSynchronizeRegisterSubscribe from Drupal\Tests\simplenews\Functional
    1x in SimplenewsAdministrationTest::testNewsletterSettings from Drupal\Tests\simplenews\Functional
    1x in SimplenewsAdministrationTest::testSubscriptionManagement from Drupal\Tests\simplenews\Functional
    1x in SimplenewsAdministrationTest::testContentTypes from Drupal\Tests\simplenews\Functional
    1x in SimplenewsAdministrationTest::testSubscriberStatusFilter from Drupal\Tests\simplenews\Functional
    1x in SimplenewsAdministrationTest::testNewsletterIssuesOverview from Drupal\Tests\simplenews\Functional
    1x in SimplenewsAdministrationTest::testAccess from Drupal\Tests\simplenews\Functional
    1x in SimplenewsFieldUiTest::testContentTypeCreation from Drupal\Tests\simplenews\Functional
    1x in SimplenewsI18nTest::testNewsletterIssueTranslation from Drupal\Tests\simplenews\Functional
    1x in SimplenewsPersonalizationFormsTest::testSynchronizeSubscribeRegister from Drupal\Tests\simplenews\Functional
    1x in SimplenewsPersonalizationFormsTest::testSubscribeRequestPassword from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testSendNowCronThrottle from Drupal\Tests\simplenews\Functional
    1x in SimplenewsPersonalizationFormsTest::testDisableAccount from Drupal\Tests\simplenews\Functional
    1x in SimplenewsPersonalizationFormsTest::testDeleteAccount from Drupal\Tests\simplenews\Functional
    1x in SimplenewsPersonalizationFormsTest::testBlockedSubscribe from Drupal\Tests\simplenews\Functional
    1x in SimplenewsRecipientHandlerTest::testSiteMail from Drupal\Tests\simplenews\Functional
    1x in SimplenewsRecipientHandlerTest::testNewUsers from Drupal\Tests\simplenews\Functional
    1x in SimplenewsRecipientHandlerTest::testSubscribersByRole from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testProgrammaticNewsletter from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testSendNowNoCron from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testSendMultipleNoCron from Drupal\Tests\simplenews\Functional
    1x in SimplenewsUninstallTest::testUninstall from Drupal\Tests\simplenews\Functional

  39x: Support for asserting against non-boolean values in ::assertTrue is deprecated in drupal:8.8.0 and is removed from drupal:9.0.0. Use a different assert method, for example, ::assertNotEmpty(). See https://www.drupal.org/node/3082086
    10x in SimplenewsSourceTest::testSendCaching from Drupal\Tests\simplenews\Functional
    4x in SimplenewsSourceTest::testSendHtml from Drupal\Tests\simplenews\Functional
    3x in SimplenewsSendTest::testSendMultipleNoCron from Drupal\Tests\simplenews\Functional
    3x in SimplenewsAdministrationTest::testSubscriptionManagement from Drupal\Tests\simplenews\Functional
    2x in SimplenewsSubscribeTest::testSubscribeAuthenticated from Drupal\Tests\simplenews\Functional
    2x in SimplenewsSubscribeTest::testSubscribeAnonymous from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSubscribeTest::testSubscribeAnonymousSingle from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSendMissingSubscriber from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSendMissingNode from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSendNoCaching from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSendHidden from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testNewsletterTheme from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testDelete from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testUpdateNewsletter from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testSendPublishNoCron from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testSendNowCron from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testSendNowCronThrottle from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testSendNowNoCron from Drupal\Tests\simplenews\Functional
    1x in SimplenewsI18nTest::testNewsletterIssueTranslation from Drupal\Tests\simplenews\Functional
    1x in SimplenewsAdministrationTest::testContentTypes from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSynchronizeFieldsTest::testSynchronizeBaseFields from Drupal\Tests\simplenews\Kernel

  4x: Support for asserting against non-boolean values in ::assertFalse is deprecated in drupal:8.8.0 and is removed from drupal:9.0.0. Use a different assert method, for example, ::assertEmpty(). See https://www.drupal.org/node/3082086
    2x in SimplenewsSynchronizeFieldsTest::testSynchronizeBaseFields from Drupal\Tests\simplenews\Kernel
    1x in SimplenewsPersonalizationFormsTest::testDisableAccount from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testDelete from Drupal\Tests\simplenews\Functional

  2x: \Drupal\Core\Extension\ThemeHandlerInterface::install() is deprecated in drupal:8.0.0 and is removed from drupal:9.0.0. Use \Drupal\Core\Extension\ThemeInstallerInterface::install() instead. See https://www.drupal.org/node/3017233
    1x in SimplenewsDemoTest::testInstalled from Drupal\Tests\simplenews_demo\Functional
    1x in SimplenewsSendTest::testNewsletterTheme from Drupal\Tests\simplenews\Functional

  1x: \Drupal\Core\Session\AccountInterface::getUsername() is deprecated in Drupal 8.0.0, will be removed before Drupal 9.0.0. Use \Drupal\Core\Session\AccountInterface::getAccountName() or \Drupal\user\UserInterface::getDisplayName() instead. See https://www.drupal.org/node/2572493
    1x in SimplenewsSubscribeTest::testSubscribeAnonymous from Drupal\Tests\simplenews\Functional

So we have the defaultTheme, which should be pretty easy to fix, just two tests extend from BrowserTestBase directly, so needs to be added there. The entity_reference_autocomplete one is a bit annoying, default config needs to be updated for that. assertTrue() might be a bit tricky to find as the message doesn't tell you which ones exactly if there's more than one in a test that is the problem. The theme installer again should be easy to fix and getUsername() too.

Hi @AdamPS, I ran the patch over D8.8.

Thank's

So the minimum before I can commit is to fix comments relating to the changes that are already in the patch:

• #55 regarding User::getDisplayName().
• #54.2

Then there are lots of comments (thank you @Berdir) about extra things that need to be changed: #54.1, #59 (mentions an important change), #60. This can be a separate commit. However the important bits need to be done soon, certainly before creating another alpha.

We should get this in before 2.x beta because it is not BC (in places where we add DI). If we can get a working patch in the next 2 weeks that would be great. This doesn't have to include everything @Berdir has mentioned, just enough to make consistent usable code free of deprecated code report errors for D8.8.

New patch fixes most of the remaining problems except for a few listed in the IS.

Some of the problems from #60 will remain - @Berdir it would be useful to have a rerun of your testing if you get a chance.

Only these are left:

  46x: Any entity_reference_autocomplete component of an entity_form_display must have a match_limit setting. The uid field on the node.simplenews_issue.default form display is missing it. This BC layer will be removed before 9.0.0. See https://www.drupal.org/node/2863188
    1x in SimplenewsSourceTest::testSendNoCaching from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testSendFail from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testDelete from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testImpersonation from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testNewsletterTheme from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testHtmlEscaping from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSendMinimalSourceImplementation from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSendCaching from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSendHtml from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSendHidden from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSendMissingNode from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testSendPublishNoCron from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSendMissingSubscriber from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSkip from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSubscribeTest::testSubscribeMultiple from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSubscribeTest::testSubscribeAnonymous from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSubscribeTest::testSubscribeAnonymousSingle from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSubscribeTest::testSubscribeAuthenticated from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSubscribeTest::testSimplenewsSubscriptionBlock from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSubscribeTest::testAdminCreate from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSynchronizeFieldsFormTest::testSubscriberFormFieldSync from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testUpdateNewsletter from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testSendNowCron from Drupal\Tests\simplenews\Functional
    1x in SimplenewsDemoTest::testInstalled from Drupal\Tests\simplenews_demo\Functional
    1x in SimplenewsPersonalizationFormsTest::testSynchronizeRegisterSubscribe from Drupal\Tests\simplenews\Functional
    1x in SimplenewsAdministrationTest::testNewsletterSettings from Drupal\Tests\simplenews\Functional
    1x in SimplenewsAdministrationTest::testSubscriptionManagement from Drupal\Tests\simplenews\Functional
    1x in SimplenewsAdministrationTest::testContentTypes from Drupal\Tests\simplenews\Functional
    1x in SimplenewsAdministrationTest::testSubscriberStatusFilter from Drupal\Tests\simplenews\Functional
    1x in SimplenewsAdministrationTest::testNewsletterIssuesOverview from Drupal\Tests\simplenews\Functional
    1x in SimplenewsAdministrationTest::testAccess from Drupal\Tests\simplenews\Functional
    1x in SimplenewsFieldUiTest::testContentTypeCreation from Drupal\Tests\simplenews\Functional
    1x in SimplenewsI18nTest::testNewsletterIssueTranslation from Drupal\Tests\simplenews\Functional
    1x in SimplenewsPersonalizationFormsTest::testSynchronizeSubscribeRegister from Drupal\Tests\simplenews\Functional
    1x in SimplenewsPersonalizationFormsTest::testSubscribeRequestPassword from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testSendNowCronThrottle from Drupal\Tests\simplenews\Functional
    1x in SimplenewsPersonalizationFormsTest::testDisableAccount from Drupal\Tests\simplenews\Functional
    1x in SimplenewsPersonalizationFormsTest::testDeleteAccount from Drupal\Tests\simplenews\Functional
    1x in SimplenewsPersonalizationFormsTest::testBlockedSubscribe from Drupal\Tests\simplenews\Functional
    1x in SimplenewsRecipientHandlerTest::testSiteMail from Drupal\Tests\simplenews\Functional
    1x in SimplenewsRecipientHandlerTest::testNewUsers from Drupal\Tests\simplenews\Functional
    1x in SimplenewsRecipientHandlerTest::testSubscribersByRole from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testProgrammaticNewsletter from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testSendNowNoCron from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testSendMultipleNoCron from Drupal\Tests\simplenews\Functional
    1x in SimplenewsUninstallTest::testUninstall from Drupal\Tests\simplenews\Functional

  39x: Support for asserting against non-boolean values in ::assertTrue is deprecated in drupal:8.8.0 and is removed from drupal:9.0.0. Use a different assert method, for example, ::assertNotEmpty(). See https://www.drupal.org/node/3082086
    10x in SimplenewsSourceTest::testSendCaching from Drupal\Tests\simplenews\Functional
    4x in SimplenewsSourceTest::testSendHtml from Drupal\Tests\simplenews\Functional
    3x in SimplenewsSendTest::testSendMultipleNoCron from Drupal\Tests\simplenews\Functional
    3x in SimplenewsAdministrationTest::testSubscriptionManagement from Drupal\Tests\simplenews\Functional
    2x in SimplenewsSubscribeTest::testSubscribeAuthenticated from Drupal\Tests\simplenews\Functional
    2x in SimplenewsSubscribeTest::testSubscribeAnonymous from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSubscribeTest::testSubscribeAnonymousSingle from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSendMissingSubscriber from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSendMissingNode from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSendNoCaching from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSourceTest::testSendHidden from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testNewsletterTheme from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testDelete from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testUpdateNewsletter from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testSendPublishNoCron from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testSendNowCron from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testSendNowCronThrottle from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testSendNowNoCron from Drupal\Tests\simplenews\Functional
    1x in SimplenewsI18nTest::testNewsletterIssueTranslation from Drupal\Tests\simplenews\Functional
    1x in SimplenewsAdministrationTest::testContentTypes from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSynchronizeFieldsTest::testSynchronizeBaseFields from Drupal\Tests\simplenews\Kernel

  4x: Support for asserting against non-boolean values in ::assertFalse is deprecated in drupal:8.8.0 and is removed from drupal:9.0.0. Use a different assert method, for example, ::assertEmpty(). See https://www.drupal.org/node/3082086
    2x in SimplenewsSynchronizeFieldsTest::testSynchronizeBaseFields from Drupal\Tests\simplenews\Kernel
    1x in SimplenewsPersonalizationFormsTest::testDisableAccount from Drupal\Tests\simplenews\Functional
    1x in SimplenewsSendTest::testDelete from Drupal\Tests\simplenews\Functional

  2x: \Drupal\Core\Extension\ThemeHandlerInterface::install() is deprecated in drupal:8.0.0 and is removed from drupal:9.0.0. Use \Drupal\Core\Extension\ThemeInstallerInterface::install() instead. See https://www.drupal.org/node/3017233
    1x in SimplenewsDemoTest::testInstalled from Drupal\Tests\simplenews_demo\Functional
    1x in SimplenewsSendTest::testNewsletterTheme from Drupal\Tests\simplenews\Functional

  1x: \Drupal\Core\Session\AccountInterface::getUsername() is deprecated in Drupal 8.0.0, will be removed before Drupal 9.0.0. Use \Drupal\Core\Session\AccountInterface::getAccountName() or \Drupal\user\UserInterface::getDisplayName() instead. See https://www.drupal.org/node/2572493
    1x in SimplenewsSubscribeTest::testSubscribeAnonymous from Drupal\Tests\simplenews\Functional

+++ b/tests/src/Functional/SimplenewsSendTest.php
@@ -287,7 +287,7 @@ class SimplenewsSendTest extends SimplenewsTestBase {
 
-    $this->assertTrue(preg_match('|node/(\d+)$|', $this->getUrl(), $matches), 'Node created');
+    $this->assertEqual(1, preg_match('|node/(\d+)$|', $this->getUrl(), $matches), 'Node created');
     $node = Node::load($matches[1]);
 
     $this->clickLink(t('Newsletter'));

getUrl() might include the base path, so this might be /checkout/node/X on testbot. And then it's not always 1.

Thanks @Berdir. I interpret your remark as a warning that there might be multiple matches and so you are worried that the return value might not be 1. However preg_match() doesn't return the number of matches, see https://www.php.net/manual/en/function.preg-match.php

preg_match() returns 1 if the pattern matches given subject, 0 if it does not, or FALSE if an error occurred.

We have a problem with SubscriberInterface::getStatus() and setStatus(). They are documented as taking bool, which seems intuitive and sensible because it matches UserInterface::isActive() so you can write code like this from Subscriber::fillFromAccount():

    $this->setStatus($account->isActive());

However there are also constants SubscriberInterface::INACTIVE and ACTIVE and this is what the code uses in the majority of cases - hence the warnings for code that seems very reasonable.

    $this->assertFalse($subscriber->getStatus());

I will handle this is a separate issue #3103733: Mix up over subscriber status: boolean or 0/1 to make it more obvious what's happening for people upgrading to 2.x.

That looks like one random fail and the remainder to be fixed in #3103733: Mix up over subscriber status: boolean or 0/1 so I'm going to commit. If it turns out that I've missed one then we can always do another commit later.

OK hopefully that's done for D9. @Berdir if you want to port to 1.x then go ahead. However if you are able to move your sites to the newer 2.x version that would be great - in which case please set this to fixed.

Thanks, will have a closer look at that in a few weeks when we'll start official preparations for one of our install profiles that uses simplenews.

And yeah, forget about the preg_match() thing, somehow I thought it returns the position of the match but that doesn't make sense :)

About the boolean entity methods, that is a common problem with content entities because their data isn't type safe as it's typically passed through 1:1 from whatever is passed in/the database returns, which is usually strings. Plenty of examples of that in core too. id() typically returns strings too for example.

We'd missed a few deprecated constants. Should now be clean against D9

I really don't support this backport idea as it seems to substantially undermine the significant work to develop 2.x. @Berdir is a very influential member of the community and if he pays for development of a backport then it is likely to influence others away from 2.x. However this doesn't seem supported by the facts - the new release has update hooks, it has change records and there are no issues yet about upgrade problems. On the other hand, 1.x has lots of bugs, some serious.

The upgrade to D9 is the ideal time for sites to upgrade to simplenews 2.x. The nature of the changes required is very similar, some simple formulaic "change this function for that one". Both sets of changes can be tested together. If we need to give more documentation or other support of the migration then we can do that as part of #3124097: Deprecate 8.x-1.x. It seems much better for the community to work together to move forward rather than to spend efforts on backporting.