New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Digest notifications do not include all new comments #173

Closed
raamdev opened this Issue Dec 10, 2015 · 55 comments

Comments

Projects
None yet
4 participants
@raamdev
Contributor

raamdev commented Dec 10, 2015

This was tested using the Lite version from the dev branch as of 2015-12-10 16:16:05 EST.

Steps to reproduce this bug

  • Install, activate, and enable Comment Mail Lite
  • Logout and leave a comment on a post selecting "All Comments/Replies" and "Hourly Digest"
  • Leave several additional comments on the same post with different email addresses (i.e., as different commenters)
  • Wait for the Hourly Digest email that should be sent to the first commenter who selected "Hourly Digest"

Expected Behavior

The subscriber who selected "Hourly Digest" should get a digest email that includes a list of all the comments left on the post after the digest subscription was created.

Observed Behavior

The subscriber who selected the "Hourly Digest" receives an email that only contains the very last new comment that was posted, as opposed to all of the comments. Additionally, this notification makes no mention of "digest" at all and looks just like the email they would receive if they simply selected asap. (The title of the email that is received even says "New Comment on...", as opposed to "New Comments on...".)

@raamdev raamdev added the bug label Dec 10, 2015

@raamdev raamdev added this to the Next Release milestone Dec 10, 2015

@raamdev raamdev modified the milestones: Next Release, Future Release Dec 13, 2015

@raamdev

This comment has been minimized.

Contributor

raamdev commented Jan 2, 2016

@kristineds @renzms @Reedyseth I could use some help attempting to reproduce the issue described above. 😄

@kristineds

This comment has been minimized.

Contributor

kristineds commented Jan 15, 2016

@raamdev I was able to reproduce this issue.

  • Installed and enabled Comment Mail Lite v151224
  • Posted as first commenter and selected "All Comments/Replies" and "Hourly Digest"
  • First commented received an email (right after the confirmation email) that contains one comment that was posted right after. i.e. should be "digest" but received it as "asap".
  • Waited for an hour or two to see if I'll still get a "digest" of the other comments, but didn't received anything only the first comment
  • Title of the email that is received says "New Comment on.."

Hourly Digest bug report

@raamdev

This comment has been minimized.

Contributor

raamdev commented Jan 19, 2016

@kristineds writes...

should be "digest" but received it as "asap".

Can you confirm what the "Deliver" option is set to when you edit that subscription in Comment Mail → Subscriptions? It should say "Hourly"...

2016-01-19_10-43-54

@kristineds

This comment has been minimized.

Contributor

kristineds commented Jan 19, 2016

Can you confirm what the "Deliver" option is set to when you edit that subscription in Comment Mail → Subscriptions? It should say "Hourly"...

It says "Hourly" on my end for that subscription.

screen shot 2016-01-19 at 11 57 30 pm

@raamdev

This comment has been minimized.

Contributor

raamdev commented Jan 19, 2016

@kristineds So it sounds like the problem is that the "Hourly" digest option isn't being passed properly to the routine that creates the subscription when someone selects "Hourly" on comment form? Would you mind running a few more tests to confirm that's the case? That should help us narrow this bug down quite a bit.

@raamdev

This comment has been minimized.

Contributor

raamdev commented Jan 19, 2016

@kristineds Oh, sorry, I totally misunderstood what you said the first time. :-) I realize now that it should be hourly.

Hmm, so it sounds like there might actually be a problem with the routine that processes the subscriptions to determine which ones require digests...

@jaswrks

This comment has been minimized.

Member

jaswrks commented Jan 19, 2016

I haven't looked over this issue with any great detail yet, but it might help to know that Comment Mail doesn't have any hard-coded handler for digest notification templates. Comment Mail simply adapts the emails that it sends based on the number of comments it contains.

So for instance, if you subscribe to a digest, you should be getting (at most) one notification every period for which you subscribed. This can lead to each notification that you receive containing multiple comment notifications (in the form of a digest), but if it so happens that there was only one additional comment since your last notification came through, the email will only contain just the one, and the notification format reverts automatically back to what a standard one-off notification would look like in that scenario.

This is mostly handled by the email templates, where we have $is_digest.
https://github.com/websharks/comment-mail-pro/blob/151224/comment-mail-pro/includes/templates/type-a/email/comment-notification/subject.php#L40

@raamdev

This comment has been minimized.

Contributor

raamdev commented Jan 19, 2016

if it so happens that there was only one additional comment since your last notification came through, the email will only contain just the one, and the notification format reverts automatically back to what a standard one-off notification would look like in that scenario.

Ah, that's helpful to know. Thank you. Yes, it sounds like what we need to test is multiple comment notifications within an hour, so that we can trigger the digest email template (which only appears when more than one comment notification is available).

@kristineds Could you try testing that? You'll just need to run a test where one email address is subscribed to a post, and then generate several new replies (with a different email address) so that the digest subscriber receives more than one comment notification and then after 1 hour the digest email with a list of all the notifications should be shown.


Once this has been confirmed, we should also publish a new KB Article for this that explains what Jason just clarified, i.e., that if there's only 1 notification, you'll see the regular asap notification template even if you selected one of the digest options. KB Article title: "Why don't I receive the digest notification?"

@raamdev

This comment has been minimized.

Contributor

raamdev commented Feb 6, 2016

@kristineds Did you get a chance to confirm the above?

@raamdev raamdev modified the milestones: Next Release, Future Release Feb 6, 2016

@kristineds

This comment has been minimized.

Contributor

kristineds commented Feb 6, 2016

@raamdev I tested this but wasn't able to reproduce it. I didn't get any digest email after 1 hour.

  • Installed and enabled Comment Mail Lite v151224

  • Subscribed and commented to a post as kristine_test1, selected "All Comments/Replies" and "Hourly Digest".

    screen shot 2016-02-07 at 2 39 38 am

  • Received an email notification for the subscriber kristine_test1 to confirm subscription.

    screen shot 2016-02-07 at 2 45 35 am

  • Confirmed subscription and started posting several replies to that comment from kristine_test1

  • Received an email notification (again) for the first reply on the comment:

    screen shot 2016-02-07 at 2 48 16 am

  • Waited for an hour to see if I'll still get a "digest email" of the other replies, but didn't receive anything

@jaswrks

This comment has been minimized.

Member

jaswrks commented Feb 6, 2016

@raamdev That sounds a lot like what you experienced, no?

@raamdev

This comment has been minimized.

Contributor

raamdev commented Feb 8, 2016

That sounds a lot like what you experienced, no?

@jaswsinc Yes, that sounds exactly like what I experienced. My hourly digest subscriber received a comment notification for the first new comment reply (instantly), but no more after that.

@jaswrks jaswrks referenced this issue Feb 8, 2016

Closed

PSR 2/4 Compliance #150

15 of 16 tasks complete
@jaswrks

This comment has been minimized.

Member

jaswrks commented Feb 8, 2016

Thanks. I will keep my eyes open for this whenever I reformat this file on Friday.
#150 (comment)

@jaswrks

This comment has been minimized.

Member

jaswrks commented Feb 14, 2016

I found a bug in this class, and it looks like a great suspect for this case.

Referencing: https://github.com/websharks/comment-mail-pro/blob/160213/comment-mail-pro/includes/classes/queue-processor.php#L647-L657

Problem: These case handlers are falling through on each other inadvertently in some scenarios.


Will be fixed in #150

jaswrks referenced this issue in websharks/comment-mail-pro Feb 15, 2016

jaswrks pushed a commit to websharks/comment-mail-pro that referenced this issue Feb 15, 2016

@jaswrks

This comment has been minimized.

Member

jaswrks commented Feb 15, 2016

  • This issue has been fixed by #150 (confirmed).
  • I believe the fix resolves #192 as well, but further testing will be needed to confirm.
@raamdev

This comment has been minimized.

Contributor

raamdev commented May 27, 2016

@kristineds Can you please install WP Crontrol and try manually running the Comment Mail cron job to test the hourly digest notifications using the latest Lite version from the Dev branch?

@kristineds

This comment has been minimized.

Contributor

kristineds commented May 27, 2016

Can you please install WP Crontrol and try manually running the Comment Mail cron job to test the hourly digest notifications using the latest Lite version from the Dev branch?

@raamdev Tested this using the Comment Mail Lite version from the dev branch as of 2015-05-27 with WP Crontrol installed. Below are my observations with the Hourly Digest notifications for all All Comments/Replies enabled.

  • Installed, activated, and enabled WP Crontrol
  • Installed, activated, and enabled Comment Mail Lite v2015-05-27
  • Logged out and left several comments (as different commenters) on a post selecting "All Comments/Replies" and "Hourly Digest"
  • Didn't receive any confirmation emails at all for any of the commenters
  • Manually confirmed subscription for all commenters
  • Waited for an hour to for the "hourly digest" for the first commenter or the other commenters, but didn't receive anything.
@jaswrks

This comment has been minimized.

Member

jaswrks commented Jun 4, 2016

@kristineds @raamdev I did a video to show how I walked through some extensive tests against the dev branch for both Lite and Pro and I have been unable to reproduce any of the problems being reported here. This seems to be working perfectly with moderation on and also with moderation off, in both the lite and pro repos. Maybe the video will offer some insight or maybe you can spot something that I'm not doing that would help reproduce the bug.

📹 Video posted in Slack as Comment Mail Mysteries.

@raamdev

This comment has been minimized.

Contributor

raamdev commented Jun 10, 2016

@jaswsinc writes in #173 (comment)...

I found a bug in this class, and it looks like a great suspect for this case.

Referencing: https://github.com/websharks/comment-mail-pro/blob/160213/comment-mail-pro/includes/classes/queue-processor.php#L647-L657

Problem: These case handlers are falling through on each other inadvertently in some scenarios.

I was just reviewing that code and trying to better understand what scenario (if any) would result in a bug there. I could not come up with any scenario, so I'm not seeing any bug at all. I agree that there should be break; statements there, however since each case statement includes a return statement, a break statement would never be reached anyway (because the return statement would exit the switch first).

To verify this, I created the following test with the missing break statements:

<?php
function tester($v)
{
    switch($v) // Check for digests.
    {
        case 'hourly': // Delivery option = hourly digest.
            if($v == 'hourly')
                return 'returning from hourly';
        case 'daily': // Delivery option = daily digest.
            if($v == 'daily')
                return 'returning from daily';
        case 'weekly': // Delivery option = weekly digest.
            if($v == 'weekly')
                return 'returning from weekly';
    }
    return 'returning from tester() + $v = ' . $v;
}

echo tester('hourly') . "\n";
echo tester('daily') . "\n";
echo tester('weekly') . "\n";
echo tester('n/a') . "\n";

As expected, the results are as follows:


returning from hourly
returning from daily
returning from weekly
returning from tester + $v = n/a

And even if one of those if conditionals returned false and we did fall through the case statement due to a lack of break statements, none of the other case statements would be true (e.g., if we entered case "hourly", case "daily" and case "weekly" would not be true and we'd never enter those). The only way I'm seeing potential for a bug in this case would be if we also had a default: case (because then if an if statement did fail, the default case would always be run despite our having entered one of the other case statements).

So in what scenario might that code cause a problem?

The reason I'm trying to better understand how that (bad) code might have introduced a bug is that you said that fixing it potentially solves two issues: this one, and #192. However, I'm not seeing a bug there at all.

The other reason I'm trying to understand how that's a bug is that I tried reproducing the original issue outlined at the top of this issue, with digest emails not going out, and I'm once again able to reproduce it—the queued notifications get to Holding Until "(less than a minute)" and then just sit there indefinitely (testing Comment Mail Lite, built from 000000-dev branch as of 2016-06-09 21:58:25 EDT).

Here's a video demonstrating the issue: http://quick.as/0Gj3C76PA

@jaswrks

This comment has been minimized.

Member

jaswrks commented Jun 10, 2016

@raamdev writes...

So in what scenario might that code cause a problem?

In the example you're using it would not be an issue I agree. But that's different from what I fixed in Comment Mail. See: these lines where falling through in each case is a definite possibility.

Given the original code, if a subscriber has never been notified before, and they have a setting of hourly, that will result in three separate DB queries and perhaps (although unlikely) be treated as weekly, which would be incorrect. This could occur given three separate queries that might (however unlikely) return something different if the user is still active on the site and doing other things.

And even if one of those if conditionals returned false and we did fall through the case statement due to a lack of break statements, none of the other case statements would be true (e.g., if we entered case "hourly", case "daily" and case "weekly" would not be true and we'd never enter those).

A case only needs to match a starting point, and then when it falls through there is no case check again. It simply falls through to the next case. A neat way to experiment with this is using byte notation. Change $modifier to something else and see how this works.

$value = 1; $modifier = 'tb';
// Convert 1 TB to bytes.

switch ($modifier) {
    case 't':
    case 'tb':
        $value *= 1024;
        // Fall through.

    case 'g':
    case 'gb':
        $value *= 1024;
        // Fall through.

    case 'm':
    case 'mb':
        $value *= 1024;
        // Fall through.

    case 'k':
    case 'kb':
    case 'kbs':
        $value *= 1024;
}
echo $value; // 1099511627776

@raamdev writes...

The reason I'm trying to better understand how that (bad) code might have introduced a bug is that you said that fixing it potentially solves two issues: this one, and #192. However, I'm not seeing a bug there at all.

The Bigger Problem ...

I see. I was trying to avoid adding confusion here, but it looks like I failed, because I left something out that would help you. As I mentioned in the video I did recently, this was actually a two-part fix.

  1. The issue with case falling through in certain circumstances was fix 1.

  2. The next fix. See these lines. Now, look just above into the main loop. See how an entry could be 'processed' but held, and yet it was being deleted in all cases? i.e., NOT held as it should be.

    That was the bigger problem and I discovered this after doing some of the same digging that you are, given how rare it would be for that case issue to occur. So like you, I kept digging and eventually found that additional problem that was far more likely to be the culprit in most of the bug reports we were getting. That was also fixed at the same time, even though I failed to mention that originally.

@jaswrks

This comment has been minimized.

Member

jaswrks commented Jun 10, 2016

—the queued notifications get to Holding Until "(less than a minute)" and then just sit there indefinitely (testing Comment Mail Lite, built from 000000-dev branch as of 2016-06-09 21:58:25 EDT).

Here's a video demonstrating the issue: http://quick.as/0Gj3C76PA

I just tried to reproduce this again locally without success; i.e., it's working as expected on my end. However, given your video and the reports from others it makes me wonder if there is something going on with wp-cron.php on some of your collective test sites.

My suggestion would be to start monitoring activity in this file by adding lines that log incoming requests processed by that file and see what you can find out.

In short, if you find they are just sitting in the queue, it's almost certainly because the CRON is not running. So I'd start by trying to prove that CRON is working (or not) and go from there.

e.g., after this line add the following and then work your way down through this file to see if you can figure out where the CRON is failing at, or if it's never actually running at all?

file_put_contents(WP_CONTENT_DIR.'/cm-debug-cron.log', print_r($_REQUEST, true)."\n\n", FILE_APPEND);

In addition to that, you could also add a similar logging routine here

file_put_contents(WP_CONTENT_DIR.'/cm-debug-cron-hook.log', print_r($_REQUEST, true)."\n\n", FILE_APPEND);

If you find that it's not running at all, then you could look at the HTTP communication that occurs. Is that failing on the test site for some reason? e.g., a firewall or network connectivity issue maybe?

wp-cron.php is triggered by WordPress using a non-blocking HTTP request behind-the-scenes. https://core.trac.wordpress.org/browser/tags/4.5.2/src/wp-includes/cron.php#L318

@raamdev

This comment has been minimized.

Contributor

raamdev commented Jun 10, 2016

@jaswsinc writes...

A case only needs to match a starting point, and then when it falls through there is no case check again. It simply falls through to the next case.

Not true if the case statement includes a return statement, as all of the case statements in the original code did.

<?php
$value = 1; $modifier = 'tb';
// Convert 1 TB to bytes.

function converter($value, $modifier) {
    switch ($modifier) {
        case 't':
        case 'tb':
            $value *= 1024;
            return $value;
            // DOES NOT fall through.

        case 'g':
        case 'gb':
            $value *= 1024;
            return $value;
            // DOES NOT fall through.

        case 'm':
        case 'mb':
            $value *= 1024;
            return $value;
            // DOES NOT fall through.

        case 'k':
        case 'kb':
        case 'kbs':
            $value *= 1024;
            return $value;
            // DOES NOT fall through.
    }
    return $value;
}

echo converter($value, $modifier); // 1024
@raamdev

This comment has been minimized.

Contributor

raamdev commented Jun 10, 2016

if you find they are just sitting in the queue, it's almost certainly because the CRON is not running. So I'd start by trying to prove that CRON is working (or not) and go from there.

You know what, last night on my bike ride home I was thinking about this problem and came to the exact same conclusion, which reminded me of #194 (_Cron event disappears in some scenarios: cron_comment_mail_queue_processor), which we already know is an issue that we proved (and fixed) in Comet Cache. I'm willing to bet that what we're seeing here is THAT bug, which is leading to all of this confusion.

I'll run some more tests today and report back.

@raamdev

This comment has been minimized.

Contributor

raamdev commented Jun 10, 2016

#194 (Cron event disappears in some scenarios: _cron_comment_mail_queue_processor), which we already know is an issue that we proved (and fixed) in Comet Cache. I'm willing to bet that what we're seeing here is THAT bug, which is leading to all of this confusion.

Confirmed!! The _cron_comment_mail_queue_processor is missing:

2016-06-10_10-04-18

@jaswrks

This comment has been minimized.

Member

jaswrks commented Jun 10, 2016

Not true if the case statement includes a return statement, as all of the case statements in the original code did.

Right. But that return was conditional; i.e., it would not always occur.

@jaswrks

This comment has been minimized.

Member

jaswrks commented Jun 10, 2016

Confirmed!! The _cron_comment_mail_queue_processor is missing:

Aha! Very cool. I saw your commits a moment ago.

@raamdev

This comment has been minimized.

Contributor

raamdev commented Jun 10, 2016

Right. But that return was conditional; i.e., it would not always occur.

Try adding in a conditional; it still does not fall through:

        case 't':
        case 'tb':
            if (false) {
                $value *= 1024;
                return $value;
            }
            // DOES NOT fall through.
@raamdev

This comment has been minimized.

Contributor

raamdev commented Jun 10, 2016

Try adding in a conditional; it still does not fall through:

Whoops, user error. 😄 I see it does fall through in the above scenario. I concede defeat. 😆

@raamdev

This comment has been minimized.

Contributor

raamdev commented Jun 10, 2016

@jaswsinc writes...

this was actually a two-part fix.

The issue with case falling through in certain circumstances was fix 1.

The next fix. See these lines. Now, look just above into the main loop. See how an entry could be 'processed' but held, and yet it was being deleted in all cases? i.e., NOT held as it should be.

That was the bigger problem and I discovered this after doing some of the same digging that you are, given how rare it would be for that case issue to occur. So like you, I kept digging and eventually found that additional problem that was far more likely to be the culprit in most of the bug reports we were getting. That was also fixed at the same time, even though I failed to mention that originally.

Ah, thank you for explaining that. Those sorts of bug fixes are important to document so that we can link them to existing bug reports.

So I'm seeing a few bug fixes here:

  • Bug Fix: If a subscriber had never been notified before, and they chose a Digest Delivery option of hourly, there was a possibility, in some scenarios, for that subscription to instead be treated as a weekly digest.
  • Bug Fix: In some scenarios, a notification could be 'processed' but held (e.g., because it belongs to a digest and should go out later), and yet that notification was being deleted in all cases and not being sent out at all.

Do I have that right? Also, that last bug fix: I'm wondering what that would look like symptom-wise. Would it look like #192?

@raamdev

This comment has been minimized.

Contributor

raamdev commented Jun 10, 2016

↑ Noting that the fix I just pushed to the dev branch for a bug related to the Queue Processor cron event was likely the cause for many of the reports in this GitHub issue about never receiving the hourly digest notifications. See #194 (comment)

@jaswrks

This comment has been minimized.

Member

jaswrks commented Jun 10, 2016

Also, that last bug fix: I'm wondering what that would look like symptom-wise. Would it look like #192?

This was causing queue entries that one would expect to be held, to not be held (or sent). They would magically be processed and silently go poof, and never even be logged so they wouldn't show up in the queue event log either.

@raamdev

This comment has been minimized.

Contributor

raamdev commented Jun 10, 2016

and silently go poof

So they would just disappear? I.e., a subscriber simply wouldn't receive the notifications they were expecting when new comments were posted?

@raamdev

This comment has been minimized.

Contributor

raamdev commented Jun 10, 2016

I'm adding two changelog entries to the next release for the two bugs described above that were fixed while restructuring the codebase (#150):

  • Bug Fix: Fixed a bug where a subscriber who selected Hourly Digest and who had never been notified before could, in some scenarios, have their subscription treated instead as a Weekly Digest. This bug was found and fixed during the codebase restructuring. Props @jaswsinc. See Issue #150 and additional discussion in Issue #173.
  • Bug Fix: Fixed a bug where in some scenarios Mail Queue entries for Digest Notifications that should have been held for sending later were not being held and were also not being sent. They also would not have shown up in the Mail Queue Event Log. This bug was found and fixed during the codebase restructuring. Props @jaswsinc. See Issue #150 and additional discussion in Issue #173.

Those two, plus the bug fixed in #194, makes me feel comfortable closing this GitHub issue. I also ran a few more tests on the dev branch (after fixing #194) and I haven't had any trouble with digest notifications.

Thank you to every for all the help testing and debugging this!

@raamdev raamdev closed this Jun 10, 2016

@jaswrks

This comment has been minimized.

Member

jaswrks commented Jun 16, 2016

@raamdev writes...

So they would just disappear? I.e., a subscriber simply wouldn't receive the notifications they were expecting when new comments were posted?

That's correct, yes.

raamdev added a commit that referenced this issue Jun 18, 2016

Phing release of v160618 with the following changes:
- **Restructured Codebase**: The codebase has been completely restructured to improve performance, enhance flexibility, and make it easier to build in new features! Props @jaswsinc. See [Issue #150](#150).
- **Comment Mail Pro Upgrade Notice: Incompatible Advanced Templates.** This version of Comment Mail includes a rewritten and improved codebase. This rewrite, however, came with the unfortunate side effect of breaking backwards compatibility with Advanced Templates that were customized in a previous version of Comment Mail Pro.

     If you are currently using Comment Mail Pro and you've customized your Advanced Templates, all of your customized Advanced Templates will be backed up and the templates will then be reset to their new defaults. You will find the backup of your old customized template appended to the bottom of the new template, separated with a  <code>Legacy Template Backup</code> PHP comment. See [example screenshots](#238 (comment)).

     Note: This change has no effect on Simple templates—only Advanced Templates are affected. Advanced Templates are a Pro-only feature, so this notice only applies to Comment Mail Pro. See [Issue #238](#238).
- **Bug Fix**: Fixed a bug where `esc_html()` was being used where `esc_sql()` should've been used. Props @jaswsinc @kristineds. See [Issue #268](#268).
- **Bug Fix**: Fixed a bug that in some scenarios resulted in a "DB table creation failure" error when activating the plugin. Props @thienhaxanh2405, @PanNovak, @kristineds, and @jaswsinc. See [Issue #260](#260).
- **Bug Fix**: Fixed a bug where "New reply" notification emails were not being parsed properly by some Hotmail accounts and were showing up as blank. Props @kristineds. See [Issue #259](#259).
- **Bug Fix**: Fixed a bug that allowed spam comments to create subscriptions in Comment Mail when using Akismet. Props @IvanRF. See [Issue #250](#250).
- **Bug Fix** (Pro): When Chrome or Firefox Autofill Username/Password was enabled, the Comment Mail Pro Updater fields would incorrectly be autofilled by the browser with invalid credentials. This has been fixed. Props @renzms. [Issue #274](#274).
- **Bug Fix**: Fixed a bug where the cron job for the Queue Processor could get deleted and never recreated, which would result in notifications getting stuck in the Mail Queue and not being sent out. If you ever installed Comment Mail and then deleted it (without first disabling Data Safeguards), and then installed Comment Mail again, you were probably affected by this issue. This release fixes the issue and makes the cron setup more robust. Props @kristineds, @renzms, @jaswsinc, and @IvanRF for help testing. See [Issue #194](#194) and [Issue #173](#173).
- **Bug Fix:** Fixed a bug where a subscriber who selected Hourly Digest and who had never been notified before could, in some scenarios, have their subscription treated instead as a Weekly Digest. This bug was found and fixed during the codebase restructuring. Props @jaswsinc. See [Issue #150](#150) and additional discussion in [Issue #173](#173 (comment)).
- **Bug Fix:** Fixed a bug where in some scenarios Mail Queue entries for Digest Notifications that should have been held for sending later were not being held and were also not being sent. They also would not have shown up in the Mail Queue Event Log. This bug was found and fixed during the codebase restructuring. Props @jaswsinc. See [Issue #150](#150) and additional discussion in [Issue #173](#173 (comment)).
- **Enhancement**: Minor improvements to the Options Page menu links and positioning of the Pro Preview link. Props @renzms. See [Issue #227](#227).
- **Enhancement**: It's now possible to use the following shortcodes in the Email Footer Tag for Email Footer Templates: `[home_url]`, `[blog_name_clip]`, and `[current_host_path]`. Props @kristineds and @IvanRF. See [Issue #246](#246).
- **Enhancement**: Improved the Subscriptions meta box that appears on the Post Edit screen. For each subscription, the meta box now lists the full name and email address, the date the subscription was created, and a view link that allows you to view/edit the subscription. Props @kristineds. See [Issue #231](#231).
- **UX Enhancement (Pro)**: Improved the Dashboard notice that appears when you try to enable the Pro version of Comment Mail when the Lite version is currently enabled. Props @kristineds @jaswsinc. See [Issue #230](#230).
- **UX Enhancement**: When Subscribing Without Commenting, the Add New Subscription form now pre-populates the Name and Email address fields whenever possible. Props @kristineds. See [Issue #204](#204).
- **UI Enhancement**: Dashboard notices generated by Comment Mail now use the WordPress-style dismiss button to keep things consistent. Props @kristineds. See [Issue #193](#193).
@raamdev

This comment has been minimized.

Contributor

raamdev commented Jun 19, 2016

Comment Mail v160618 has been released and includes changes from this GitHub Issue. See the v160618 announcement for further details.


This issue will now be locked to further updates. If you have something to add related to this GitHub Issue, please open a new GitHub Issue and reference this one (#173).

@websharks websharks locked and limited conversation to collaborators Jun 19, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.