Skip to:
Content
Pages
Categories
Search
Top
Bottom

Extending bbPress: bbp_notify_subscribers


  • tharsheblows
    Participant

    @tharsheblows

    Oh, eep. I am working on a one-click unsubscribe link that will need to use the old way of sending the emails individually. Is there any reason bbp_notify_subscribers wasn’t just added to a hook (existing only in my head at the moment) at the very end of function bbp_new_reply_handler after the redirect (ie around 749 in replies/functions.php?) I know it will still take a long time but do you think it will affect anything?

Viewing 11 replies - 1 through 11 (of 11 total)

  • Stephen Edgar
    Keymaster

    @netweb

    @tharsheblows I think your looking at bbp_edit_reply_handler rather than bbp_new_reply_handler, you should be looking here (I think, if I’m following you correctly) scroll up to /includes/replies/functions.php#L426


    tharsheblows
    Participant

    @tharsheblows

    Oh yes, you’re right, that’s what I was looking at initially – I went back to find the line number and got the wrong one. (This sort of thing happens all too frequently.)

    So what do you think? Would it affect anything else if I just put a hook there (ie after the redirect) and hooked in bbp_notify_subscribers?


    Stephen Edgar
    Keymaster

    @netweb

    It already is…

    bbp_new_reply_handler as linked above line #426 -> do_action( 'bbp_new_reply'...

    /includes/core/actions.php#L229

    
    229	add_action( 'bbp_new_reply',    'bbp_notify_subscribers',                 11, 5 );
    230	add_action( 'bbp_new_topic',    'bbp_notify_forum_subscribers',           11, 4 );
    

    tharsheblows
    Participant

    @tharsheblows

    Oh no, I know that – I am doing a somewhat bad thing and suggesting that I add in a hook to line 447 (ish – after line 446 at any rate and before the current line 448) eg — I want the mail send to happen *after* the page redirects:

    do_action( 'bbp_after_new_reply', $reply_id, $topic_id, $forum_id, $anonymous_data, $reply_author, false, $reply_to );

    then in bbp-functions (or something like this)

    remove_action( 'bbp_new_reply',    'bbp_notify_subscribers',                 11, 5 );
    add_action( 'bbp_after_new_reply',    'bbp_notify_subscribers',                 11, 5 );

    Stephen Edgar
    Keymaster

    @netweb

    I suppose that would work, though with the updates of the above code in bbPress 2.6 the performance cost of creating a separate email for each individually subscribed user has been removed and the impact of this before or after the redirect would be minimal.

    Your original post:

    I am working on a one-click unsubscribe link that will need to use the old way of sending the emails individually.

    Essentially by doing this you will reintroduce the bottle neck that we just removed :/

    Why do you need to create the emails individually?

    Creating a single email with the recipients BCC’d gets the email to every subscriber from the ‘list of topic subscribers’ with only a single email and is far more efficient than creating an individual email for every subscriber.


    tharsheblows
    Participant

    @tharsheblows

    What will it bottleneck? What’s going to not happen quickly? That’s the part I’m unsure about. I can’t see anything but thought I’d ask! The send can go slowly, that’s fine – I just want the page to load quickly after they post and figured if that happened before sending all the emails, people wouldn’t notice or care about the slow emails; those don’t have to be instantaneous.

    I am writing something to add a one-click unsubscribe link for the subscription emails, so need them to be unique for each recipient in that respect.


    Stephen Edgar
    Keymaster

    @netweb

    I am writing something to add a one-click unsubscribe link for the subscription emails, so need them to be unique for each recipient in that respect.

    Ok, I follow you now, cool idea 🙂 If you ‘hack’ core it will break when you upgrade bbPress. 🙁

    I presume you want the individual email so each link is unique and the user does not have to be logged in to unsubscribe?

    So maybe create a ticket on Trac with a patch to have bbp_notify_subscribers fire after the redirect.


    tharsheblows
    Participant

    @tharsheblows

    I definitely need to sit down and learn how to submit a patch on Trac. This is in my list of things to do.

    I have a system* for keeping track of core changes on plugins – I’m pretty used to doing upgrades then adding back in my own changes.

    *it involves notes to myself in shouty capital letters and exclamation points in the plugin description. It is not the greatest system.


    tharsheblows
    Participant

    @tharsheblows

    Sorry I just realised I didn’t answer your question. Yes – a link like this:

    …/topic-unsubscribe?user=2&topic=5586&cs=5E82028F8F1866AF401D5C7C7FC16331

    where cs is a md5 hash which involves user id, topic id and a salt. I am not going to use a WP nonce because I don’t want the link to ever expire as that’s frustrating for users and I don’t know a way to change the lifespan of a single nonce. This does mean that if they forward the email to someone, that person can unsubscribe them but I think that will be ok. The most important thing for me is that all one-click signups on my site have one-click unsubscribes.

    This is just in my head at the moment, I haven’t done it yet, so the actual implementation will probably change.


    Stephen Edgar
    Keymaster

    @netweb

    I definitely need to sit down and learn how to submit a patch on Trac. This is in my list of things to do

    Check out my comments on this ticket, they should help get you on your way. Then just create your ticket and and any questions you have along the way I’ll help guide you along. 🙂

    And yes, that was the way I was thinking you were going once I understood what you were saying 😉


    tharsheblows
    Participant

    @tharsheblows

    Cool, thanks. It will take me a while to get to it, but I will!

Viewing 11 replies - 1 through 11 (of 11 total)
  • You must be logged in to reply to this topic.
Skip to toolbar