Simplenews Drush 9 support

Suggested patch. Please review!

Reviewed and looks fine.

Seems to have the same bug as #2931645: Is Drush simplenews-spool-send command broken item 2.

Based off of jkardum's patch, here's one with a SQL sanitize hook to get rid of user data in the module

@Coops_ Thanks for creating a patch.

I think it makes more sense to keep the two issues separate. Please use this issue to create a patch that adds drush 9 support identical to the current drush 8 support, taking account of comment #4.

Then the other issue can add support for SQL sanitize, but that needs to be done for both drush 8 and drush 9.

Here's an update patch, based on #4 without the additions. It:

• Fixes the SpoolStorageInterface::countMails() call
• Switches to injecting config/spool/mailer so we don't retrieve them multiple times
• Fixes the dockblock and arguments for spoolSend where $limit was optional but $options (including adding a default for pipe

Thanks @andrewbelcher I did a review and that looks good. Minor comments:

1. Comment lines 12-20 - this looks like leftover from some template you started from. I think the intention is that you delete this comment and replace it with your own e.g. "Drush command file for newsletter."
2. Please check for coding standards - e.g. looks like missing trailing new line; line 65: wrong indent.

I didn't test it as I still use drush 8. We still need confirmation from someone else that they have used this and it works.

It doesn't work because of the following error:

An option named "pipe" already exists.

This one works.

Thanks @jcnventura. All commits/patches now go to the new branch 2.x please.

The patch is still the same, as the last change to simplenews.drush.inc is from February 10 (#2931645: Is Drush simplenews-spool-send command broken).

In any case, since I changed some of the documentation to stop referring to simplenews_throttle variable, might as well submit the same changes (plus coding standard fixes) to the Drush 8 file.

Great, thanks for the new patch - it looks good to me although I'm not using Drush 9 so didn't test. Please can you upload an interdiff with the previous patch? If it's a fairly small change since the previous patch then I think I can commit without waiting for someone else to test

Please no coding standards fixes mixed in with other issues - we have a separate issue for them [3037140]. Patches welcome there although right now I have given precedence to #3055728: Convert from Simpletests to PHPUnit tests so it would need to wait a few weeks.

The effective interdiff without coding standard changes between #9 and #13 is:

--- a/src/Commands/SimplenewsCommands.php
+++ b/src/Commands/SimplenewsCommands.php
@@ -60,11 +60,12 @@ class SimplenewsCommands extends DrushCommands {
    * Print the current simplenews mail spool count
    *
    * @param array $options An associative array of options whose values come from cli, aliases, config, etc.
-   * @option pipe
-   *   Just print the count value to allow parsing
    * @validate-module-enabled simplenews
    *
    * @command simplenews:spool-count
@@ -85,27 +86,31 @@ class SimplenewsCommands extends DrushCommands {
    * Send the defined amount of mail spool entries.
    *
    * @param bool  $limit
-   * @option                  pipe
-   *   Just print the sent and remaining count on separate lines to allow
-   *   parsing
    * @usage                   drush sn-ss
    *   Send the default amount of mails, as defined by the simplenews_throttle
    *   variable.

I'll do a new patch that doesn't fix the coding standards in the Drush 8 file, but keeps the documentation fix.

Great thanks @jcnventura that seems simple enough. Please can you explain why the comment for the pipe option isn't needed? Is it a global option in Drush 9?

Yes, it is a global option and an exception is thrown if you try to redefine it.

This is an updated #17 patch without the coding standards changes to the Drush 8 file, but fixing the documentation errors fixed in the Drush 9 documentation.

Thanks looks good to me, I will commit after giving other maintainers a chance to review.

Sorry my first commit missed one of the files - now fixed.