Email-notifications

Hello, I've create a patch to implement this functionality

I made a couple of changes to the previous patch to address errors being thrown when the $node object isn't available.

A more general solution covering other entity types might be better, though the current approach works for our use case.

Any answer here?

Hi, I implemented a more general solution, here's the patch. Unfortunately I did it before even discovering the work that had already been done here! That does mean it's reasonably different, e.g. the emails are sent immediately on receiving the post (via an event subscriber) rather than on cron, and I only added a single new configuration key.
Note that it does rely on having the patch from #2931745-2: #callbacks in disqus element doesn't work., since I didn't want to add yet more that would conflict with that good change.

A colleague has just implied to me that there ought to be a way for individual users to opt out of getting the notifications, which I think is a good thought. Perhaps notifications should only be sent if a user has explicitly opted in (or a site could have a setting for whether to use opt in or out behaviour by default).
In my imagination, the current checkbox for enabling email notifications to content authors could be swapped for a dropdown with the something like the following options (welcome to be reworded!):

* Never notify content authors
* Always notify content authors
* Notify content authors unless they have opted out
* Only notify content authors that have explicitly opted in

Anyway, I suggest that be addressed in a follow-up ticket rather than delay notifications of any kind?

So I think the first thing really should be to get the patch in #2931745: #callbacks in disqus element doesn't work. committed please, to allow anyone to implement things like this themselves, regardless of whether it should be part of the disqus module or not.

Then review which of of the general approaches of the last two patches here to use (and incorporate ideas from the other if necessary, e.g. covering any entity type, notifying a configurable address other than the content's author, including flood control or similar). I recognise that I missed out on some good ideas in the last patch (e.g. notifying a configurable address, not just the content author, and making the email content configurable) when producing mine, sorry!
Finally, to follow up with (configurably) opting in or out.

My patch in comment 7 doesn't work well when a comment is made against a post that changes its URL. This one should resolve that :-)

I should add, this latest patch doesn't change the actual notification email content though - so the link would be to the old URL. A well configured site should have the old URL redirected anyway. I figure that if it doesn't, then as it's the post's author that is taken to a 404 when they click the link, they're probably the right person to notice and fix the URL (or add a redirect). So if anything, that might be considered a helpful part of the notification?!

The email subject will also reflect the original title of the post though.

Someone else is welcome to pick those things up and change them, I don't think I'll be doing so any time soon.

The patch in #9 to Disqus.php does not apply to the latest disqus 8.x-1.x-dev and needs a reroll.

error: while searching for:
      // Attach the js with the callback implementation.
      $element['#attached']['library'][] = 'disqus/ga';
    }
    // Add the disqus.js and all the settings to process the JavaScript and load
    // Disqus.
    $element['#attached']['library'][] = 'disqus/disqus';

error: patch failed: src/Element/Disqus.php:121
error: src/Element/Disqus.php: patch does not apply

Sorry, that was made explicit when I posted my first version of a patch back in comment 7:

Note that it does rely on having the patch from #2931745-2: #callbacks in disqus element doesn't work.: #callbacks in disqus element doesn't work., since I didn't want to add yet more that would conflict with that good change.

Thank you so much for working on this feature!!!. The patch would require some changes as well as a reroll. Marking it as needs work for now.

A re-rolled patch against the latest 8.x-1.x.

We will have to add an update hook as there are changes in the configuration schema, taking a hint from https://www.drupal.org/docs/drupal-apis/update-api/updating-configuratio....

Also, there are instances of using services directly without using DI. Those should also be replaced.

We will also have to mention it in the documentation as it is a major change.

Ah yes, you're right about a few services that should have gone through DI.

However, I don't think an update hook is needed, as the only change to the config schema adds a new property, which is not activated by default anyway. So the behaviour is the same with or without the property, until the property is manually enabled.

But anyway ... an update hook is easy to add, so here it is, also with added DI.

I have pushed the patch to the latest 2.0.x branch. Tested the functionality and it was working for me. After, enabling notifications on comments, I did 2-3 test comments and emails are being sent on that!!!.

Fixed a few minor CS issues in the patch and also added a line about configurations in the README file.

Thanks, everyone for working on this.