Make how to send a newsletter more obvious

"Newsletter issues" is not the easiest way to send the newsletter.

After creating a newsletter node, then normally you are redirected to the page for viewing the node. You should see "tabs" for actions: View, Edit, Delete, and Newsletter. That's the easiest way to send.

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 add a help text after creating a newsletter? I have updated the Issue Summary with my suggestion.

Also, what do you think about this suggestion?

... maybe the README could get updated with the fewest basic steps required to create and send a newsletter?

I created a MR, adding a "Quick Start" section at the very top. The README needs a thorough rewrite after all the changes, 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"?

The README needs a thorough rewrite after all the changes, and also a Markdown massage. But that will take too long

Actually it takes too long (of my time) to keep making small changes, so let's do the job properly or not at all. There is already another issue for README updates (including markdown) so please can you add your changes to that?

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?

Yes I would like to, but unfortunately I don't have permissions. It would need one of the other maintainers to do it.

I totally understand and agree, let's be efficient: I have updated the README 📌 Replace README.txt with README.md RTBC , 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.

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.

Yes it's a good idea. The link should only be shown if the user has permission to send.

ressa → closed merge request !43

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

Great thanks I made some comments in the MR to tidy up a little

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!

Great it's nearly there now.

So adding four words

This issue is a balance as the experienced users might find the message annoying when it's already obvious to them - the tab isn't really so invisible maybe that's just your theme😃. However I have sympathy with the new users, and after creating a newsletter it's very natural to send it, and so I agree to your request, as it says in the issue title.

However the admin link is different. Admin doesn't follow naturally from creating a newsletter - the site owner will likely send 100 newsletters but admin only once. The person who creates a newsletter is often not even the same person as the admin.

We have the help system, and the configure link in the info file - these are the standard places that people should look when getting started with the module.

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.?

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

Gitlab CI test fails are not related, they are currently failing. DrupalCI tests are showing as green.

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.

Great thanks it looks good. I added one comment.

I would love to have the tests working in GitLab. I spent as much time as I could afford without success. You are right we could disable them, however it would be a backward step, and I was hoping that someone might be able to fix them quite soon.

I believe the failure about monitoring is a different case and it's OK on GitLab. The code currently uses "@dependencies monitoring" which I guess ddev doesn't recognises.

OK I'm about to run the coding standards fixer so I'll catch the unused line as part of that. Thanks everyone.

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

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