How does SMF know when not to send another notification?

Started by Biology Forums, September 27, 2017, 12:23:47 PM

Previous topic - Next topic

Biology Forums

I've created a mod that uses sendmail() as a means to notify the member. However, every comment that's made prompts the server to send another email out. How does SMF prevent multiple emails being sent out when replies are made to a topic?

What I'd like specifically is for the sendmail function to check when the last email was sent and compare it to the persons last login time stamp. Does this mean I have to create a new table that records the last email?

Arantor

Yes, and it would also trigger for literally every single email, including newsletters.

Normal notifications are batched long before it even gets to sendmail, by the code that runs when a post is actually made, and in 2.0 compares it to the user's preference too to do daily/weekly digests.

Instead of focusing on how you think you should implement it, what are you trying to achieve exactly?

Biology Forums

Thanks for the reply.

Using ultimate profile, when members send each other comments, each comment sent triggers the function sendpm. Of course, whether an email actually sends depends on the recipients notification preferences. Anyway, I found that if I send 1 or more comments in a row, it triggers sendpm as many times as I click send -- this is not good and could hurt the domain's reputation.

I want to develop a system where, when sendpm is triggered, a row is added to a new table called smf_notifications, for example, which records the member ID and the time stamp. The sendpm function also checks against the member's last login (found in the member's table) to see if they've had a chance to login in. If the last login time is less than that recorded, it doesn't send out another email.

Does this sound like a viable plan?

Arantor

How about fixing the actual problem: the fact that Ultimate Profile doesn't have a call out to checkSubmitOnce to literally prevent the spamming of the kind you're talking about. That's what it's there for. It's also a sign of bad design that you can even press the button multiple times - most parts of SMF wouldn't even let you mash the refresh button to achieve that by having a redirect call immediately after the PM was sent so any refresh cannot trigger a new notification.

The other problem is that what you're talking about doing, even in sendpm, still means an extra overhead of at least one query, and often two, per PM. On the kind of scale you're talking about, assuming it's a real threat and that it happens enough to require a fix, that would imply a non trivial server hit.

Biology Forums

I didn't mean clicking the submit button twice, although I see how it could be interpreted that way. I meant to suggest, say you commented, and then 1 minute later commented again.

I didn't get a solid answer from you though. Do you think my plan is a good way to implement what I'm trying to achieve or do you suggest another way? I'm aware that it will slow down sendpm due to the extra queries, but I feel it's worth it. I know it seems trivial, but a lot of modifications that have a commenting system use sendpm, such as SMF gallery, etc.

I just want to reduce the amount of emails my server sends, given it can hurt my server's reputation.

Arantor

For the problem as I understood it, no it wasn't a good explanation. For the problem that you're describing now... still not convinced it's necessarily the best solution.

First objection: doing this at sendpm is the wrong level. Don't tag it against the method doing the work, tag it against the part that decides whether to do it. The problem is the decision logic in Ultimate Profile. I'd possibly consider checking in there (and not on all PMs, as you're suggesting) before sending a notification... possibly do it based on time of last message to that person and if the last message to that person was in the last hour via PM, don't even notify them.

Second objection: is this based on a theoretical risk of sending too many emails or do you have actual evidence that this behaviour is actually occurring? Regarding emails, the reality is that if people get too many emails they'll likely turn them off, then still get PMs without the email notifications.

Thirdly: the proposed solution will not only hurt performance, but will likely confuse users on top because the getting of emails will now be inconsistent. Best overall suggestion: don't use sendpm, have an alerts system instead that ultimately makes the problem go away but this is a larger piece of work to fix.

Biology Forums

Good answer and excellent suggestions

That being said, could you elaborate on:

QuoteNormal notifications are batched long before it even gets to sendmail, by the code that runs when a post is actually made

In other words, how does SMF know when not to send more notifications for unviewed topic replies? Could you boil it down to the function?

Arantor

It's literally done as part of Post2() to work it out. But it's massively more complex than the kind of thing you're hoping to find because it also factors in the list of people who could conceivably see the post, plus the people who've opted into board or topic notifications, and do it based on their preference.

SMF 1.1 isn't especially interesting, the preferences are quite straightforward, and the reality is that most people aren't even opted in most of the time.

SMF 2.0 is massively more complex because it also includes a daily or weekly digest, which amounts to shoving a row into a table to say 'there is a pending email for given topic' and clean that out once a day/week.

SMF 2.1 is even more complex because you can also get actual alerts without using emails.

Biology Forums

I did some more research into the code and found the following:

Post2 activates sendNotifications (found in Subs-Posts.php).

This function considers the topic ID and the type of notification to make, and compares it to what's found in smf_log_notify. Correct me if I'm wrong here, but if the row found in log_notify has the sent set to 1, it will not resend the notification unless the person views the topic again. When the member finally views the topic, the log_notify row changes to sent = 0 by Display (found in Display.php).

Very interesting.

Therefore, I'll need to create a table call smf_alerts and make a function that works similarly to sendNotifications rather than targeting sendmail itself (I liked your suggestion) OR create a table call smf_alerts but modify sendNotifications instead of creating a new function.

Which of these ideas would you use?

Arantor

Correct, that's how it works.

How I'd do it, ultimately, is how SMF 2.1 does it: with an alert system that doesn't send emails unless the user explicitly opts in (and for something like profile updates a la Ultimate Profile, I probably wouldn't even make email an option, alerts or nothing)... but I'm biased as I'm the one who actually wrote the alerts system in 2.1 ;)

Biology Forums

It's actually a very important feature all forum software should include by default, and it helps prevent the forum from using up server resources. It's not a big deal if it's a small forum, but larger forums take the hit.

Would you suggest I modify sendNotifications and build on that or start with a new function?

Quoteand for something like profile updates a la Ultimate Profile, I probably wouldn't even make email an option, alerts or nothing

But how would member's know they've received a profile comment? A little extreme I think not to give that option

Arantor

I wouldn't touch sendNotifications, it's totally geared to topics/posts, I'd go from scratch to suit your needs.

Also, the reality is that people who actually care about profile comments tend to go check in spite of notifications, at least based on the last few years of observing behavioural patterns in platforms like XenForo.

Biology Forums

I'm working on the function as we speak, but I've noticed SMF never designed a way to prevent ALL notifications from sending. Like an option found in the one's profile that would short-circuit sendmail no matter what.

Did the original designers ever consider the poor implementation of mods that don't have a notification turn-off switches? Or perhaps they weren't checked properly when the mods were submitted.

Suki

Althought an SMF dev always has to have in mind people modifiying SMF I don't think it applies for this specific case. The ability to receive emails from the forum solely relies on the user actually wanting to receive those emails. It has been like that since the very beggining and thats a good thing. Just do a search for "always notify users" and you will see a consistent reply from SMF teamers about it.

Having said that, why don't you implemment a small check in Ultimate Profile instead of messing with sending emails?  a flood control combined with disabling the button and checksubmitonce()  will do just fine for your needs.

You don't need to mess up with sendpm() or whatever other function you are dealing with, in fact, applying specific logic to a function shared by many isn't a good thing.

What I did was simply setting a session var with a timestamp and a counter, when an user post something check that session var, if the counter is greater than a hardlimit then reject that post. Check thye timestamp to clear the session once a certain time has passed.

Problem solved, now you can call sendpm AFTER that chek has been applied as you now know it won't be fired that often.
Disclaimer: unless otherwise stated, all my posts are personal and does not represent any views or opinions held by Simple Machines.

Biology Forums

I agree with you about not changing sendPM, but it's best to create a flood control system that's universal.

For instance, SMF gallery has tons of functions where sendmail is used -- i.e. when a comment is made, when someone favorites an image.

And there are tons of mods that are similar that never developed a flood control mechanism. Maybe before accepting mods into the mods site, this should be a prerequisite.

Suki

That just borderlines on babysitting.  Besides, each mod has different needs and a universal flood control won't fit everyone's, for some it will come up as short, for others it will be too robust, etc. etc.

And again, even if such thing would exists, it won't be added to sendpm() or sendnotification or any other low level function.
Disclaimer: unless otherwise stated, all my posts are personal and does not represent any views or opinions held by Simple Machines.

Biology Forums

Surely it wouldn't, but I think 2.1's alert system will do this and all mod authors will use it similar to the way I've proposed.

Suki

Not really.

I use SMF alert system and I still implement a flood control way before any alert/notification is been fired.

I will argue any low level function such as sendpm or sendnotification shouldn't have to deal with any checks or other kinds of logics, you know, Law of Demeter and all that stuff.

In this specific context, a mod should handle their own busness and not rely on others to do their stuff for them.  Use tools? absolutely but passing your burdens to others to handle isn't a nice thing to do.

Do note that I'm talking about big mods with their own logic.
Disclaimer: unless otherwise stated, all my posts are personal and does not represent any views or opinions held by Simple Machines.

Arantor

Quote from: Study Force on September 29, 2017, 11:32:22 AM
Surely it wouldn't, but I think 2.1's alert system will do this and all mod authors will use it similar to the way I've proposed.

I don't think you've actually used it or understand anything about what it does, or indeed anything I've been saying.

Have fun reimplementing the wheel for theoretical problems.

Biology Forums

QuoteI don't think you've actually used it or understand anything about what it does, or indeed anything I've been saying.

I admit, haven't looked at the latest code at all. I was just making assumptions.

Anyway, I created a function that works for my forum. Of course, I had to create a table: [ID, ID_MEMBER, TYPE, and TIMESTAMP] -- smf_alerts. It could be expanded to meet all needs, just would require more coding.


<?php 
function sendAlert($recipient_id$type$misc null$misc_2 null)
{
global $txt$scripturl$db_prefix$language$user_info$ID_MEMBER$modSettings$sourcedir;

/*
// WHAT THIS FUNCTION DOES:
Part 1) Checks to see if anything was recorded for that particular member within the last hour.
Part 2) Records and sends emails.

// $misc can represent ID's if required.
*/

$txt['notification_gallery_comment_subject'] = 'New Comment: %s';
$txt['notification_gallery_comment'] = 'A picture you posted has received a comment from '.un_htmlspecialchars($user_info['name']).'. To view the comment, visit: '.$scripturl.'?action=gallery;sa=view;id=%d#c%d';

$txt['notification_gallery_subscribe'] = '';
$txt['notification_subscribe'] = '';

$notification_types = array(
'gallery_comment' => array('subject' => $txt['notification_gallery_comment_subject'], 'message' => $txt['notification_gallery_comment']),
'gallery_subscribe' => array('subject' => $txt['notification_gallery_subscribe'], 'message' => $txt['notification_subscribe'])
);

$current_type $notification_types[$type];

// If something is missing, return;
if (empty($type) || empty($recipient_id))
return;

// Latest time
$latest_time time();

// Current time minus an hour
$current_time_minus_an_hour time() - 3600;

$found 1;
$pass false;
$pass_1 false;

// Get the last timestamp regardless of the notification type...
$result db_query("
SELECT 
ID_MEMBER, 
TYPE, 
TIMESTAMP
FROM 
{$db_prefix}alerts
WHERE 
ID_MEMBER = 
$recipient_id
ORDER BY 
TIMESTAMP DESC
LIMIT 1"
__FILE____LINE__);

list ($TIMESTAMP) = mysql_fetch_row($result);
mysql_free_result($result);

// If no records are found, a timestamp for this member doesn't exist. This means they've never been sent a notification.
if(empty($TIMESTAMP))
$pass true;
// Records were found!
else
{
// If found, and the last records row has a timestamp less than an hour. This means they were sent something too recently.
if($TIMESTAMP <= $current_time_minus_an_hour)
return;

// If found, and the last recorded row was more than an hour ago. More checks required.
else
$pass_1 true;
}

// Set up the subject line and message specific to what's being sent
if($pass || $pass_1)
{
$pm_from = array(
'id' => 217807//Notifier
'name' => '',
'username' => ''
);
$recipients = array(
'to' => array($recipient_id),
'bcc' => array(),
);

if($type 'gallery_comment')
{
// $misc represents the comment ID
$result db_query("
SELECT 
title
FROM 
{$db_prefix}gallery_pic
WHERE 
ID_PICTURE = '
$misc'
Limit 1"
__FILE____LINE__);

list ($title) = mysql_fetch_row($result);
mysql_free_result($result);

$subject sprintf($current_type['subject'], strlen($title) > 50 substr($title,0,50)."..." $title);
$message sprintf($current_type['message'], $misc$misc_2);
}
else
{
$subject $txt[$current_type['subject']];
$message $txt[$current_type['message']];
}
}

if($pass_1)
{
// Check to see if the $type matches a type found in smf_alerts
$result db_query("
SELECT *
FROM 
{$db_prefix}alerts
WHERE 
ID_MEMBER = '
$recipient_id' AND 
TYPE = '
$type'"__FILE____LINE__);

$found db_affected_rows();
mysql_free_result($result);

// If there's a match, $found == true; This means that the member has not re-checked the forum.
// Send them a PM without an email (sendPM has been modified slightly) -- will serve as a soft notification.
if($found == true)
{
// If true, don't send an email along with the PM
$pm_without_email true;

sendpm($recipientsaddslashes($subject), $messagefalse$pm_from$pm_without_email);
}
else
$found == false;
}

if($found == false || $pass)
{
// Will need to add in the future, an option in the member's profile that would prevent ALL mail from sending.
// Insert the record and send a mail

db_query("
INSERT INTO 
{$db_prefix}alerts (ID_MEMBER, TYPE, TIMESTAMP)
VALUES
('
$recipient_id', '$type', '$latest_time')"__FILE____LINE__);

//Need to find the recipients email address
$result db_query("
SELECT emailAddress
FROM 
{$db_prefix}members
WHERE 
ID_MEMBER = '
$recipient_id'
Limit 1"
__FILE____LINE__);

list ($emailAddress) = mysql_fetch_row($result);
mysql_free_result($result);

sendmail($emailAddress$subject$message$send_html false);
}
}
?>

Advertisement: