Project

General

Profile

Actions

Bug #5127

closed

Forum posts to multiple groups -- no notice in attachment

Added by Matt Gold almost 9 years ago. Updated almost 9 years ago.

Status:
Resolved
Priority name:
Normal
Assignee:
Daniel Jones
Category name:
Email Notifications
Target version:
Start date:
2016-01-15
Due date:
% Done:

0%

Estimated time:
Deployment actions:

Description

Per ticket "4244:https://redmine.gc.cuny.edu/issues/4244", notifications of forum posts to multiple groups should include info on all of the groups to which the post was submitted. I just submitted a post to multiple groups and did not see that in the notification email.

Please see the attached email notification, which was from this post - http://commons.gc.cuny.edu/groups/digital-humanities-initiative/forum/topic/positions-available-for-the-cuny-digital-history-archive-project-january-2016/


Files

multiple-groups.pdf (152 KB) multiple-groups.pdf Matt Gold, 2016-01-15 12:36 PM
bbg-mail-debug.php (348 Bytes) bbg-mail-debug.php Boone Gorges, 2016-01-15 05:13 PM

Related issues

Related to CUNY Academic Commons - Design/UX #4244: Posting to multiple groupsResolvedDaniel Jones2015-06-30

Actions
Actions #1

Updated by Boone Gorges almost 9 years ago

  • Assignee changed from Boone Gorges to Daniel Jones
  • Target version set to 1.9.5

Assigning to Dan for review.

Actions #2

Updated by Daniel Jones almost 9 years ago

I've been having some issues with this locally - there was a time when it wasn't being included in emails sent on my local install, but was working on cdev. Do you have any thoughts, Boone? Right now I can't get my local setup to send me emails at all.

Actions #3

Updated by Boone Gorges almost 9 years ago

I don't have specific thoughts about the issue, but please don't let your local environment hang you up - if you can't send email, then write a small handler for wp_mail() that will log outgoing mail. I've attached the one that I sometimes use.

Actions #4

Updated by Daniel Jones almost 9 years ago

Oh wow what a time-saver!

So I think I've figured out the problem here, but I'm not sure what the best solution would be: the issue is that the emails are sent as the activities are saved, and so the first activity, the one created using the create new topic form, sends out its email to the members of the group before any of the duplicates are created. And similarly, if a user was a member of group B but not group A, and a topic was created for groups A, B, and C, then the user would only see that the topic had also been posted in A, because the topic, and so also the activity, for group C hasn't been created when the email is sent out.

I think the way to do this, then, would be to separate the creation of the topics themselves from the creation of the activities. If we create all the topics first, then we'll have the metadata about them that we need to include the info about all the other duplicates all. I think the trickiest part there will be intervening in the creation of the original topic to stop it from generating an activity automatically, and then generate that activity manually the we're already doing it for the duplicates.

Does that sound right to you, Boone?

Actions #5

Updated by Boone Gorges almost 9 years ago

(Adding Ray as a watcher.)

Good find, Dan! Your analysis sounds right, and I think your suggested strategy sounds right.

bp-multiple-forum-post creates forum duplicates and the corresponding activity items at 'bbp_new_topic_post_extras'. I wonder if it would work to simply break out the creation of activity items to a callback at 'bbp_new_topic_post_extras' with a higher priority - say, 20. The ugly part will be passing info - duplicate topic IDs - between the two functions. The cheapest way to do it is to throw them into a global variable, and then to access it in the second function.

Have a go at a first implementation, and let me know if you run into snags. Ray or I will probably have some (more or less lousy) suggestions for you :)

Actions #6

Updated by Raymond Hoh almost 9 years ago

I agree with Boone's insights.

You should be able to create the topics first and then gather your group IDs to do the activity creation on a separate hook.

The global variable stashing technique is ugly, but efficient. If you work with WordPress long enough, you'll find it is necesssary :)

In the bpmfp_create_duplicate_topics() function, after:

$original_activity_id = $original_activity['activities'][0]->id;

You could add this:

// Create our global variable for stashing purposes.
// bbPress already has a global object called 'extend'; let's just re-use it.
bbpress()->extend->temp_topic_ids = array();

During the foreach loop, after:

// Create the duplicate topic for this group
$new_topic_id = bbp_insert_topic( $topic_data, $topic_meta );

We can stash the relevant data:

// Stash our topic ID info for later.
bbpress()->extend->temp_topic_ids[$new_topic_id] = array(
    'group_id' => $group_id,
    'group_forum_id' => $group_forum_id,
);

And on our new hook, we can reference our variable stashing:

// Do your activity stuff here.
function bpmfp_create_duplicate_activity_items() {
    // Sanity check
    if ( empty( bbpress()->extend->temp_topic_ids ) ) {
        return;
    }

    // Add code here for create the activity items from the stashed topic data.
    foreach ( bbpress()->extend->temp_topic_ids as $topic_id => $topic_data ) {
        // Do what you need to here.
    }

    // Clean up after ourselves!
    unset( bbpress()->extend->temp_topic_ids );
}
add_action( 'bbp_new_topic_post_extras', 'bpmfp_create_duplicate_activity_items', 20 );

Let me know if you have any questions, Dan.

Actions #7

Updated by Daniel Jones almost 9 years ago

Thanks, you two!

I think I might be able to avoid the global variable technique by just running another foreach loop after the one I use to create the topics themselves, which would eliminate the need for another hook and function. Does that seem right? Instead I can just use a variable scoped to the whole function, and collect the needed ID numbers similar to how Ray is suggesting here.

My biggest obstacle right now is how to interrupt the activity creation or the email-sending for the original topic and wait until we know what all the duplicates are before sending. The way I'm starting to try right now is to tap into the function that adds in the "This topic was also posted in..." message (

bpmfp_duplicate_post_message_notification()
) and collect the content for the original activity email into a global variable (there it is!):

if ( empty( $all_topic_ids ) && !empty( $_POST['groups-to-post-to'] ) ) {
    $bpmfp_original_activity_content = $content;
     return $content;
}

Then, I use the function you already wrote, Boone, to avoid sending multiple emails for one person (

bpmfp_send_bpges_notification_for_user()
) to prevent the original activity email from being sent:
$all_topic_ids = bpmfp_get_duplicate_topics_list( $topic_id );
if ( empty( $all_topic_ids ) && !empty( $_POST['groups-to-post-to'] ) ) {
    $send_it = false;
    return $send_it;
}

I checked the bp group email plugin and that works in terms of the order that the hooks are fired, first it creates the content and then it decides it it'll send it or not.

I think the next thing would be to break out the topic-creation from the activity-creation, and then add in a call to a function at the end of

bpmfp_create_duplicate_topics()
that would manually send an email with the original content (stored in the global variable), borrowing some code from the bp group email plugin or maybe even just manually calling one of its functions.

Does that seem feasible to you two? I don't like that it's taking those two functions and having them do two different things, but it seems like the most straightforward way. Would it be worth it to break out the new behavior into their own functions, hooked to the same filters?

Actions #8

Updated by Raymond Hoh almost 9 years ago

I think I might be able to avoid the global variable technique by just running another foreach loop after the one I use to create the topics themselves, which would eliminate the need for another hook and function. Does that seem right?

Ahh, yes! You are absolutely right! Go ahead and give it a shot.

My biggest obstacle right now is how to interrupt the activity creation or the email-sending for the original topic and wait until we know what all the duplicates are before sending.

To prevent bbPress from creating activity items, we will need write a new filter to do this. Something like the following:

function bpmfp_stop_bbp_activity_creation( $retval ) {
    // Wipe out the activity component; this should stop activity creation.
    // We're checking if the component is 'groups' to ensure this is indeed a group forum entry.
    if ( 'groups' === $retval['component'] ) {
        $retval['component'] = '';
    }

    return $retval;
}

Then, when we want to stop bbPress activity creation somewhere during bpmfp_create_duplicate_topics(), we would add the following lines:

// Stop activity creation by bbPress; run at priority 99 to ensure this runs last.
add_filter( 'bbp_before_record_activity_parse_args', 'bpmfp_stop_bbp_activity_creation', 99 );

We will probably want to bring back activity creation after adding the above snippet. To do so, add the following lines:

// Add this when you want to bring back activity creation.
remove_filter( 'bbp_before_record_activity_parse_args', 'bpmfp_stop_bbp_activity_creation', 99 );

Untested, but hopefully, that helps!

Actions #9

Updated by Boone Gorges almost 9 years ago

  • Target version changed from 1.9.5 to 1.9.6
Actions #10

Updated by Boone Gorges almost 9 years ago

  • Target version changed from 1.9.6 to 1.9.7
Actions #11

Updated by Daniel Jones almost 9 years ago

Okay I think I've got this done here: https://github.com/cuny-academic-commons/cac/commit/7c9be3b898df77ce4b6b7744860d666824120771 with a small change here too: https://github.com/cuny-academic-commons/cac/commit/e7de15b50b24cf5dead2c869b6ecc2d4c170f71a

I basically did what we've talked about on this thread, except instead of interrupting the activity creation itself for the original topic, I just delay sending the email, store the activity in a global variable, wait until after all the duplicate topics are created, and manually call the function to send the email, passing along the activity. Seems to be working for me locally!

Actions #12

Updated by Boone Gorges almost 9 years ago

Thanks for working on this, Dan. It looks like attachment notifications are coming through on all notifications now.

I found in my testing that I was only getting a single email notification for a given cross-posted item, even if I was a member of more than one of the posted groups. Did we do this on purpose? (It's a great feature! I just don't remember building it, and I don't see at a glance how it's accomplished.)

Actions #13

Updated by Daniel Jones almost 9 years ago

Yes that was on purpose - I think you actually wrote that function ;-). It's "bpmfp_send_bpges_notification_for_user()"

Actions #14

Updated by Boone Gorges almost 9 years ago

  • Status changed from Assigned to Resolved

Ha ha. Thanks for refreshing my memory :) Marking resolved.

Actions

Also available in: Atom PDF