I spent a couple of hours with the Subscriptions module last night and it appears, after some testing, that notifications are sent to users even if the comment is not published. (On our site, comments are moderated and must be approved before their release.)

Please let me know if this is a valid feature request or if I'm missing something in the configuration options.

Thanks,
Kevin

CommentFileSizeAuthor
#3 subscriptions.zip37.81 KBMr.Sollis

Comments

dziemecki’s picture

dziemecki commented

It's a valid request. Unfortunately, I've attempted to implement it before, but was unable to find a reliable indicator to drive the send/no send decision. If I recall correctly, the applicable hooks exposed the published status too late in the decision process to useful. That might change, or I might get smarter, so I'll keep this on the to do list. Just be prepared for it to take a little while.

kmillecam’s picture

kmillecam commented

Maybe I can get this functionality by using your "Test held posts prior to sending" feature.

In the help, you explain: Test held posts prior to sending" tells Subscriptions to test if a node or comment
is still active\published prior toi sending a notification. This is mainly to avoid sending
notifications for for posts that have been deleted. This will result in a small performance
hit, and only makes sense if you are delaying the notifications with "Use cron for notifications".

Does this really check to see if a comment is published? or just the parent node? If it does check the comment status, problem solved.

Kevin

Mr.Sollis’s picture

Mr.Sollis commented
Assigned: Unassigned » Mr.Sollis
Category: feature » task
Status: Active » Needs review
StatusFileSize
new subscriptions.zip37.81 KB

I updated the module per the problem illustrated below, feel free to use this if you wish. The update does the following:

  • Fixed bug in check of post status. (test option)
  • Removed code that deletes messages that were skipped (from test option)
  • Added 2 columns to the holdings table. skip_date_first, skip_date_last for logging skipped posts
  • Added Setting for Number of Days before purging messages
  • Added piece to cron job that removes skipped messages older then setting created above (uses DAYOFYEAR function on the two columns)
  • Added watchdog event to log count of removed messages

Hope that helps. I've test a handful of scenarios and it appears to be working. This is a great module!

kmillecam’s picture

kmillecam commented

Thanks solshizzle,

Works like a charm!

two2the8’s picture

two2the8 commented

Hi all,

This is crucial functionality for the community site I administer, so thanks for all your efforts to make it work!

Two problems cropped up for me, though, while testing this module.

  1. I installed the patched module and setting it up to run everything through cron. Then every time I triggered cron, I received this error: warning: Missing argument 2 for variable_get() in /var/www/html/includes/bootstrap.inc on line 266. I reinstalled and (so far) the error has disappeared (yay!)
  2. Subscription emails aren't getting sent for subscriptions to users' blogs. When a user whose blog I'm subscribed to adds a post, I don't get an email notification. (My RSS feed for subscriptions does get updated, though.)

The first problem seems like it's pretty much resolved, but the latter one is not, and I can't-for-the-life-of-me figure out how to fix it. The 'subscribe to blog' feature works perfectly in the unpatched version of the module, so I'm pretty sure it's not a settings issue on my end. If anyone smarter than me could take a look under the hood & see if they could fix this problem, I'd be very, very appreciative.

Thanks so much!

dziemecki’s picture

dziemecki commented
Status: Needs review » Fixed

I believe this is the same as "86818". There's a fix there.

dziemecki’s picture

dziemecki commented
Assigned: Mr.Sollis » dziemecki

Corrected code committed.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)