Skip to:
Content

BuddyPress.org

Opened 10 years ago

Closed 9 years ago

Last modified 7 years ago

#4222 closed defect (bug) (fixed)

Deleting Comment with @mention Resends Notification

Reported by: SlothLoveChunk Owned by:
Milestone: 1.6 Priority: normal
Severity: normal Version: 1.6
Component: Toolbar & Notifications Keywords: has-patch needs-testing commit
Cc:

Description

Hi All -

In some cases if you delete a comment with an @mention in it a notification email is triggered (as if there is a new mention). This only seems to happen if you delete the comment via the WP edit comment admin, not the main comment admin.

For example:

  • Deleting a comment via, "/wp-admin/comment.php?action=editcomment&c=<comment_ID)" will trigger the notification:
  • If a comment is deleted from, "/wp-admin/edit-comments.php" no notification is sent.

Attachments (3)

debug_backtrace_output.txt (11.3 KB) - added by DJPaul 9 years ago.
4222.01.patch (786 bytes) - added by DJPaul 9 years ago.
4222.02.patch (1.2 KB) - added by DJPaul 9 years ago.

Download all attachments as: .zip

Change History (26)

#1 @DJPaul
9 years ago

  • Milestone changed from Awaiting Review to 1.6

This is probably happening because bp_activity_at_name_filter() is hooked to pre_comment_content.

#2 @boonebgorges
9 years ago

I'm unable to recreate this. I've tested on the latest trunk as well as the 1.5 branch, and I can't get these comment notifications to resend, no matter how I delete the comment. Anyone else?

#3 @SlothLoveChunk
9 years ago

Strange. I just tested this again by deleting both parent and child comments from the WP Edit Comment admin. All users mentioned in the comments received notification emails.

Is there any more info I can provide besides the following?

Running:

  • WP 3.4
  • BP @6109

#4 @boonebgorges
9 years ago

Strange indeed. Are you running a vanilla install (bp-default, no plugins)? Is it a multisite installation? If so, are the comments on the main site? What do you mean by "parent and child comments"?

#5 @SlothLoveChunk
9 years ago

Hey @boonebgorges,

This particular site is not a multisite install and I am testing with all plugins disabled (w/ exception of BP of course). By parent / child I am referring to threaded comments. I have done some additional testing and this is definitely proving tough to be debug. Here is what I have found so far (hopefully something here will help):

  • Contrary to my initial findings I have been able to trigger notifications using both the Edit Comment admin and the inline ajax comment "Trash" function.
  • It seems that not all comments that contain @mentions resend notifications when deleted.
  • If you "undo" a deleted comment a notification is sent again. This could be another bug altogether, but I'm mentioning it here just in case they're related.
  • When there are multiple @mentions notification emails are sent to each user who is mentioned.
  • If I "Undo" a deleted comment and delete it yet again -- assuming a notification was sent upon deleting -- no additional notification is sent (this makes it difficult to recreate this issue). The same is also true for the "undo" notification emails.

I am happy to keep digging if this is proving useful. Thanks for everything you guys do.

#6 @boonebgorges
9 years ago

Weird, I still can't reproduce it, on any setup that I've tested with.

Is there any chance that you have a dev environment where you can do a proper stack trace? Eg, where you have access to xdebug? If I were able to reproduce, I would track the issue by dropping either a formal breakpoint (if you're using an IDE that supports it) or else a statement that'll throw a warning (like echo $undefined_variable;) into the function that sends the @-notification emails (bp_activity_at_message_notification(), in bp-activity/bp-activity-notifications.php). That'd give a full stack trace, which would allow us at least to see the path that's leading to the notification.

#7 @SlothLoveChunk
9 years ago

I run NetBeans 7.1.1 and can use xdebug. That said, I have little experience with it, so I might need a some help to properly set the breakpoints. I'm happy to learn though.

I hate to ask, but are you sure your dev environment is set up to send emails via SMTP? I know mine wasn't, which lead me to a nifty little program called, "Test Mail Server Tool". It simply monitors port 25 on localhost and captures any outgoing email and routes it to your default mail program. It's a great tool when you don't want to bother configuring a local SMTP server.

Anyhow, could this issue possibly manifest itself in bp_activity_at_name_filter()? I ran some tests while outputting $activity_id to the debug.log and when these emails are sent $activity_id is set. When they're not sent it's either not present in the log or is set to '0'.

#8 @boonebgorges
9 years ago

I hate to ask, but are you sure your dev environment is set up to send emails via SMTP?

It's set up to send with standard wp_mail(), which uses PHP's mail(). In any case, other emails are successfully being sent by my system, so that can't be an issue.

could this issue possibly manifest itself in bp_activity_at_name_filter()?

That's part of the chain, but I doubt it's part of the problem. Inside of bp_activity_at_name_filter(), you'll see that notifications are only sent if an $activity_id has been passed to the function. The function is called in a variety of places through BP, but the only place where $activity_id is passed is in bp_activity_at_name_filter_updates(). bp_activity_at_name_filter_updates(), in turn, is only fired at the 'bp_activity_after_save' action. This suggests that when you delete the comment, an activity item is being saved, which should not be happening. This is why a stack track would help us figure out what's going on.

I don't know how to use xdebug or breakpoints in Netbeans. A google search will probably tell you. Something very simple you can do is to put manual breakpoints or error_log() at strategic places right in the codebase. For instance, you might try something like the following, just before the activity item is saved in bp_activity_add():

foreach( (array) $activity as $key => $value ) {
    error_log( $key . ': ' . $value . "
" );
}

Then delete a comment and trail your error log. You should be able to see the item that is being saved, which will give a clue to what's happening.

#9 @SlothLoveChunk
9 years ago

I appreciate all the info.

I put the manual break points into 'bp_activity_add()' and even when the email is triggered nothing appears in the logs. When I add a comment or something else that would trigger an activity entry entries do appear in the log. That makes me think this function isn't the problem.

I moved up the chain a bit and added the same manual breakpoints to 'bp_activity_at_name_filter_updates()' and when a email is triggered I do see entries in the log. When no email is triggered there are no entries. The log entries contain (some info changed for sake of privacy):

[20-Jun-2012 16:58:43]

activity: id: 17414
activity: item_id: 1
activity: secondary_item_id: 175747
activity: user_id: 16864
activity: primary_link: <URL>
activity: component: blogs
activity: type: new_blog_comment

activity: action: <user> commented on the post, <link>

activity: content:

Hi <a href='http://www.domain.com/members/username/' rel="nofollow"></a><a href='http://www.domain.com/members/username/' rel='nofollow'></a><a href='http://www.domain.com/members/username/' rel='nofollow'></a><a href='http://www.domain.com/members/username/' rel='nofollow'><a href='http://www.domain.com/members/username/' rel='nofollow'>@username</a></a>, <rest_of_comment_content> [&hellip;]

activity: date_recorded: 2012-06-02 05:07:27
activity: hide_sitewide: 0
activity: mptt_left: 0
activity: mptt_right: 0
activity: is_spam: 1

The strangest part of this to me is all links in the 'content'. It's hard to tell, but only one user is mentioned in this comment, yet there are five identical @mention links.

I'm more than happy to continue adding breakpoints. Just let me know what you'd like to see. I really hope this is helpful and not a distraction.

Take care!

#10 @boonebgorges
9 years ago

I put the manual break points into 'bp_activity_add()' and even when the email is triggered nothing appears in the logs.

One of two things has to be happening. (1) Something is calling bp_activity_at_name_filter_updates() manually. But given that you are running a clean instance of BP, and BP doesn't call this action from anywhere but 'bp_activity_after_save', this is unlikely. (2) Something is saving an activity item (BP_Activity_Activity::save()) directly. I don't have a clear sense of where this would be happening, from glancing at the code - I don't understand why BP would be trying to save a new activity item on comment deletion.

Have you figured out how to run a full stack trace in Netbeans? It would be invaluable to see the order in which functions are being called here.

#11 @DJPaul
9 years ago

  • Version set to 1.6-beta

#12 @SlothLoveChunk
9 years ago

Sorry for the slow response. I wish I had better news to report, but I haven't made much progress in learning how to debug in Netbeans (got sidetracked with other things). I have xdebug running, but anytime I put in a breakpoint it just opens up the wp-blog-header.php file. Obviously, I'm doing something wrong.

Anyhow, about the only thing I can confirm at this stage is that it is definitely happening with all plugins (sans BP) disabled and the functions.php file empty. I'm honestly surprised I'm the only seeing this issue, but hey, I guess that's a good thing.

#13 @DJPaul
9 years ago

Can you give clear instructions on how to recreate this? I'll give it a go then. Let me know:

  • what user you are posting the comment as (and whether they are an admin or have any other special access level)
  • what user you are editing the comment as (just a normal admin?).
  • you've said it's on multisite. Subdir or subdirectory?
  • confirm that you don't have any plugins in your mu-plugins folder
  • how to recreate it (you've said it's recreatable via the wp-admin/comment.php?action=editcomment screen).
  • are you running an object cache (wp-content/object-cache.php)
  • any server-side page caching like Akamai?

Thanks

#14 @SlothLoveChunk
9 years ago

I will do my very best. As I mentioned above it doesn't happen for every @mention, so I haven't figured out with 100% certainty triggers it. First, to answer your questions:

  • The comments I am trashing are those posted by "Subscribers". I am logged in as an admin.
  • This is not a multisite install, so there is no 'mu-plugins' directory.
  • Nope, I do not run any type of cache, including an object cache or server side caching.
  • Regarding caching, I do run caching on the production site, but I've been able to recreate this both locally and on the live site.

Here is how I've been testing (locally):

* Preface * There are thousands of comments that contain @mentions on this site. Threaded comments are enabled, so some are parent comments, while others are replies (children).

  1. Load the WP comment admin
  2. Now I search for my @username. The results will show comments posted by me and those that contain my @username.
  3. After making sure that I have the, "Test Mail Server Tool" running and I look for comments that have my @username in them (I ignore the ones posted by me). I then just "Trash" one of the comments that contains my @username.
  4. Now I wait a few seconds to see if the test mail server intercepts a new @mention notification email. If it does not, I find another comment that contains my @username and "Trash" it. About half the time I'll get an email to pop-up.

Some interesting things I've found while testing:

  • 'Undo'-ing a comment after trashing it will sometimes trigger an @mention email as well.
  • Trashing a comment after undoing it does not seem to ever trigger an email notification.
  • It doesn't seem to matter whether its a parent or child comment.
  • I haven't found the exact use case that triggers the notification, which is quite frustrating.

If you guys are unable to create this still I would be happy to show share my screen with you via Skype. At this point I've consumed so much of your time that I sure has heck hope that this is not just happening to me. :)

#15 @DJPaul
9 years ago

Ah hah, I got this to recreate. Investigatoring.

#16 @DJPaul
9 years ago

On WP 3.4, BP trunk, non-MS. Subscriber user does some comments with @mentions to the admin user. Position of @mention in comment doesn't seem to matter. Admin user goes to wp-admin comments, searches for "@admin" (or whatever), then clicks Trash on an individual table row. Then this happens:

wp_ajax_delete_comment() -> wp_trash_comment() -> wp_set_comment_status() -> wp_transition_comment_status()

Then the transition_comment_status action fires, calling bp_blogs_transition_activity_status() -> BP_Activity_Activity:save().
Then the bp_activity_after_save action fires, calling bp_activity_at_name_filter_updates().

bp_blogs_transition_activity_status() existed pre-1.6 but I overhauled it significantly when implementing Activity Admin and Akismet integration.

#17 @DJPaul
9 years ago

Added debug_backtrace() output to ticket as attachment.

@DJPaul
9 years ago

#18 @DJPaul
9 years ago

  • Keywords has-patch needs-testing added

I've attached a patch that seems to resolve the issue.

@SlothLoveChunk Can you try this patch on your server and test it? If you're unfamiliar with how to apply a patch, open up bp-blogs/bp-blogs-functions.php in a text editor, and look at the 4222.01.patch. You need to add the block of green text below the lines in white above. A find-in-file search should take you to the right place.

#19 @SlothLoveChunk
9 years ago

Woot! The patch seems to have worked. I've tested in both the main comment admin and the edit comment admin and notification emails are not being triggered when comments are deleted. Just to be safe, I also left a few new comments with @mentions and BP is still sending notification emails properly.

Thanks so much for taking the time to track this one down. I'm stoked that it didn't turn out to be a wild goose chase.

@DJPaul
9 years ago

#20 @DJPaul
9 years ago

The 4222.01.patch has the side affect of the notification counts not being updated. It is recreatable if plugins update an Activity object. The new Activity Management in 1.6 would also have created the same issue as described in this ticket, exposing it to a larger audience. The new 4222.02.patch adds a filter around whether to send BP notifications and emails when the at-mentions are updated, thus capturing all these situations.

#21 @boonebgorges
9 years ago

  • Keywords commit added

DJPaul - 4222.02 is not very elegant, but at least it is generalizable (deleting activity, editing via Activity Management panel, Akismet management, etc). A non-hackish solution would involve moving the notification bits out of the mention-recount function, so that the notification function could be called conditionally, but this would involve a fairly major rethinking of the way @-mentions are detected and counted. This is way more than what we should do for now. And we can always do this refactoring in the future and keep the proposed filter for backpat. So I say let's go with it.

#22 @djpaul
9 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [6167]) Prevent @mention notifications being re-sent when a comment is edited. This also affects
activity items that are modified through the Activity Management screens. Fixes #4222

#23 @johnjamesjacoby
7 years ago

  • Version changed from 1.6-beta to 1.6
Note: See TracTickets for help on using tickets.