CUNY Academic Commons

This problem arose from the following changeset in BuddyPress Group Email Subscription: https://github.com/boonebgorges/buddypress-group-email-subscription/commit/526b80c617fe9058a859ac4eb4cfb1d42d333aa0 In brief, the second param passed to the 'bp_ass_forum_reply_content' and related filters used to be a BP activity item. But since these functions were refactored to fire on a bbPress action rather than a BP activity action, the activity item does not exist. As a result, any function that attempts to filter these values in a way that uses this second parameter (and operates on bbPress content, since other activity content is processed in a different way) doesn't work anymore. For CAC, that means the following functions:

cac_full_forum_notification()
cac_attachment_message_notification()

Ray, I've added you as a watcher here so you can give advice. On one hand, it's straightforward enough to rewrite these CAC functions to do things properly (though we'll probably need to pass at least the post/topic IDs to the filters in BPGES). On the other hand, if this is broken for CAC, it's possible that there are other installations that have hooked to the filters in a similar way, in which case their filters would be broken as well. We could be more backward compatible if we assembled a faux BP activity object and passed it along to the filter, for the purposes of these older plugins. In my backpat-loving-heart, I think this second option is the right one, but I may be going overboard here, as it certainly won't be very elegant looking. What do you think?

Hi Boone, big facepalm on my part :(

I love your idea of creating the faux activity object! I'm just about finished a commit for GES.

One quick thing, do you mind if I omit the 'action' and 'primary_link' columns in the faux activitiy object? I believe no one will be looking at those columns during filter checks. We can't add the 'id' column for obvious reasons!

Check your pull requests! :)

Btw, I did add in 'action' and 'primary_link' for the hell of it! 'action' won't be 100% identical because it strips the hyperlinks, but that could be considered a good thing?

FYI, this change has been put into the distribution version of BPGES. I'll double check that it's solved the problem next time I upgrade the Commons.

Boone,

I've been reading through 1.4.6 issues to see if there's anything I can help with (in addition to my own assigned issues).

I think I may have noticed a connection between this issue and issue #2121 (Email notification of forum post omits hyperlinked word). In your original comment you link to the commit that has changed the behavior described in this issue. On lines 104 and 127 of bp-activity-subscription-functions.php, $the_content is being filtered through a call to html_entity_decode( strip_tags( stripslashes( $post->post_text ) ), ENT_QUOTES ). Could this be removing words that've been "hyperlinked in the post" as Matt describes in #2121?

Also, in comment-4 Ray says:

'action' won't be 100% identical because it strips the hyperlinks

While I don't fully understand the use of the "faux activity" object, could this addition be affecting the post's hyperlinked content inadvertently?

Additionally, this issue is 2 months old.

Issue #2121 (Email notification of forum post omits hyperlinked word) is 10 days old and issue #2129 ([& hellip ;]) is eight days old. It would seem plausible that both of those issue could be related to this one. If Matt didn't see missing hyperlinks or the [& hellip ;] until the updates mentioned in this issue, they could be caused by the html_entity_decode( strip_tags( stripslashes( $post->post_text ) ), ENT_QUOTES ) on lines 104 and 127 of bp-activity-subscription-functions.php. yes/no?

Dom - The line you reference is probably related to #2121 (strip_tags()) and #2129 (html_entity_decode()), but probably not this ticket.

The link between GES and forum attachments is totally custom. It's handled by the function cac_attachment_message_notification() in bp-custom.php (around 690). Basically, that function does the following:

- Catches the content of the email notification before it's sent
- Uses the $activity information passed by the filter (that's the $content param of the function) to look up the corresponding forum post
- Uses the forum post info to look up whether the post has any attachments
- If attachments are found, fetches the information about them, and appends a message to the end of the forum notification content before sending it back to the GES plugin.

If it's not working, I'm guessing it comes down to one of three things:
1) cac_attachment_message_notification() is not running at all (the hook isn't firing or something)
2) The second parameter being passed to the function, the "faux activity item" created by GES, is incorrectly structured
3) There's a deeper bug in cac_attachment_message_notification() that's preventing it from finding the attachments

My money is on 2, since the problem recently cropped up, and the faux activity item is new as well.

Dom, see what you can find. If you find yourself running in circles, let me know and I'll do a couple strategic debugs that might help you in the right direction.

Since I'm the one that caused this kerfuffle in the first place, it makes sense for me to clean this mess up! :)

I took a look at the codebase and the filters and parameters look like they should work. I'm going to do a round of debugging right now to figure out what's happening.

Okay, found out the problem was not due to the faux activity object.

It was one of those "cart before horse" situations because of the dreaded GES commit by me as outlined here.

Fixed in commit 2e70c39.

Before I mark this as resolved, give it a test on cdev and let me know if the GES email is supposed to look the way it does.