Make how to send a newsletter more obvious

Thanks for clarifying, I totally missed that "Newsletter" tab ... It's greyed out, and just a faint element among many other elements ...

View | Edit | Newsletter | Delete | Revisions | Devel

I am probably not the only one to miss it, so maybe we can clarify how to send it, by adding a help text, after creating a newsletter?

Also, what do you think about my suggestion about adding the fewest basic steps required to create and send a newsletter in the README?

I have updated the Issue Summary with my suggested solutions for both issues.

I created a MR, adding a "Quick Start" section at the very top. The README needs a thorough rewrite after the changes in 4.x, and also a Markdown massage. But that will take too long, so I'll keep the focus on getting new users on-boarded faster in this issue.

Even though 3.x is the currently recommended version, perhaps the default Gitlab branch could be considered to get updated to 4.x, for easier MR creation, since that's where the current development is taking place?

PS. It currently says "Add content" > "Newsletter Issue" ... could that get trimmed down to just "Newsletter"?

I totally understand and agree, let's be efficient: I have updated the README, please review.

What do you think about "Proposed resolution" #1, about adding link to the "Send newsletter" tab, after creation? I think that would make the most difference by far, in terms of helping new users get started.

Here's an attempt at adding the link above a newsletter, and only show if the user has administer or send newsletter permission.

Thanks for a fast review, I have attempted to update the MR.

I have kept the admin link, since I think it's a killer feature, which may easily get overlooked. So adding four words ("administer under Newsletter issues") in the big scheme of things is all right, I would think.

The test is failing, I am not sure why ...

At least I discovered DDEV with Selenium Standalone Chrome in the process :)

The problem is that the function gets declared again after a Preview:

Fatal error: Cannot redeclare simplenews_form_node_submit() (previously declared in /var/www/html/web/modules/contrib/simplenews/simplenews.module:314) in /var/www/html/web/modules/contrib/simplenews/simplenews.module on line 314

It's these steps of the test, from /simplenews/tests/src/Functional/SimplenewsSendTest.php:

    // Verify that the newsletter settings are shown.
    $this->drupalGet('node/add/simplenews_issue');
    $this->assertSession()->pageTextContains('Create Newsletter Issue');

    $edit = [
      'title[0][value]' => $this->randomString(10),
      'simplenews_issue[target_id]' => 'default',
    ];
    // Try preview first.
    $this->submitForm($edit, 'Preview');

    $this->clickLink(t('Back to content editing'));
[...]

I tried different solutions, which either didn't work, or caused many other tests to fail ... so a little assistance is probably needed to make this work. Thanks!

Thanks for the constructive feedback, it's great to be able to trim down the code.
And phew, this is taxing for me -- new territory :)

Yes, you're right that only some users can administer, that's a good argument -- so let's lose the admin link.

About visibility, I am using Olivero, heh. But I honestly think that if you sat down with a handful of new Simplenews users, that a lot of them would stall at the final "send the newsletter" part.

I see now that the steps are described on the "Create Newsletter Issue" page, at the top, but I usually skip long lines of text, and just crack on with the task ...

> The problem is that the function gets declared again after a Preview:

That's very strange! How can that function be redeclared but the rest of the file not? I would just try to experiment, for example, what if you change the function name, move it to a different place in the file or a different file etc.?

Yes, and I did try that and pushed it, but then quickly ran the local PHPUnit test in DDEV, which gave lots of errors, so I reverted it. But now I see the test completed with no errors on Gitlab (18 December 2023 at 14:21). Strange.

I'll try again with the re-located function, let's see how it goes.

Also, I almost got the link working with the placeholder, except it's shown as text. If I remove the ->toString() from $newsletter_tab->toString() it complains:

TypeError: strpos(): Argument #1 ($haystack) must be of type string, Drupal\Core\Link [...]

Do you know how to resolve this?

Ok, so moving the function down fixes the test failure in #19, but causes many other test failures ... I am not sure how to proceed.

Remaining tasks:

• Find a great place for the function, which does not break any tests
• Make the link render as actual HTML

Thanks for the comments @berdir, they were really helpful, just like the ones I got from @AdamPS.

I don't mean to turn this into an issue about tests, but I got to say that they have been a small blessing, but mostly a hurdle. The one I have found useful is this specific test, run like this, using only the https://github.com/ddev/ddev-selenium-standalone-chrome:

ddev exec -d /var/www/html/web "../vendor/bin/phpunit -v -c ./core/phpunit.xml.dist ./modules/contrib/simplenews/tests/src/Functional/SimplenewsSendTest.php"

This test caught that my new code broke the "Preview" test, and I could update the code, and verify that the test then passed, which was great

But if I run all tests, I get a lot of failures:

$ ddev exec -d /var/www/html/web "../vendor/bin/phpunit -v -c ./core/phpunit.xml.dist --group simplenews"
PHPUnit 9.6.15 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.13
Configuration: ./core/phpunit.xml.dist

Testing 
.E.........E...EE.E...EEE.E...E....E.........................     61 / 61 (100%)

Time: 10:11.034, Memory: 150.00 MB

There were 11 errors:

1) Drupal\Tests\simplenews\Kernel\SimplenewsMonitoringTest::testSensors
PHPUnit\Framework\Exception: Unavailable module: 'monitoring'. If this module needs to be downloaded separately, annotate the test class with '@requires module monitoring'.

/var/www/html/web/core/tests/Drupal/KernelTests/KernelTestBase.php:541
/var/www/html/web/core/tests/Drupal/KernelTests/KernelTestBase.php:384
/var/www/html/web/core/tests/Drupal/KernelTests/KernelTestBase.php:258
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

2) Drupal\Tests\simplenews\Functional\SimplenewsPersonalizationFormsTest::testSynchronizeRegisterSubscribe
Behat\Mink\Exception\ElementNotFoundException: Button with id|name|label|value "Confirm" not found.
[...]

I also tried with https://github.com/ddev/ddev-drupal-contrib, but running a single test like this fails:

$ ddev phpunit SimplenewsSendTest.php
PHPUnit 9.6.15 by Sebastian Bergmann and contributors.

Simplenews Demo (Drupal\Tests\simplenews_demo\Functional\SimplenewsDemo)
 ✔ Installed

Simplenews Administration (Drupal\Tests\simplenews\Functional\SimplenewsAdministration)
 ✘ Newsletter settings
   │
   │ Behat\Mink\Exception\ResponseTextException: The text "Simplenews adds elements to the newsletter node add/edit" was not found anywhere in the text of the current page.
   │
   │ /var/www/html/vendor/behat/mink/src/WebAssert.php:907
   │ /var/www/html/vendor/behat/mink/src/WebAssert.php:293
   │ /var/www/html/tests/src/Functional/SimplenewsAdministrationTest.php:271
   │ /var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

If Gitlab CI tests are known to not work, shouldn't they be turned off? From my point of view, this false positive was also not helping me.

I originally got involved with Lando + Drupal Contributions because it offered a turnkey solution for PHPUnit + Nightwatch test. I feel DDEV is very close to that with https://github.com/ddev/ddev-drupal-contrib and/or https://github.com/ddev/ddev-selenium-standalone-chrome, but that a simple piece of the puzzle is missing.

Sorry about the test rant, I created Simplenews tests are failing #27 in the DDEV Drupal Contrib issue queue to maybe clear this up.

Thanks @AdamPS, I appreciate all the help I got here, great to get this in the module. Have a nice day.