Skip to:
Content

bbPress.org

Opened 5 years ago

Closed 4 years ago

#2629 closed enhancement (maybelater)

Rename bbp_pre_notify_subscribers and bbp_post_notify_subscribers hooks

Reported by: netweb Owned by:
Milestone: Priority: normal
Severity: normal Version: trunk
Component: API - Subscriptions Keywords: has-patch
Cc: jjjay@…

Description

In r5413 - "Deprecate bbp_notify_subscribers() for new bbp_notify_topic_subscribers() function, to better match bbp_notify_forum_subscribers() introduced in 2.5."

We should do the same for bbp_pre_notify_subscribers and bbp_post_notify_subscribers hooks.

Renaming them bbp_pre_notify_topic_subscribers and bbp_post_notify_topic_subscribers respectively.

Better matching bbp_notify_forum_subscribers() bbp_pre_notify_forum_subscribers and bbp_post_notify_forum_subscribers hooks.

Attachments (1)

2629.diff (1.8 KB) - added by netweb 5 years ago.

Download all attachments as: .zip

Change History (5)

@netweb
5 years ago

#1 @netweb
5 years ago

  • Keywords 2nd-opinion added

#2 @tharsheblows
5 years ago

I wouldn't change it. Just googling, it looks like there are definitely some plugins using it, eg https://wordpress.org/support/plugin/bbpressmoderation and https://github.com/rmccue/bbPress-Reply-by-Email and maybe other people too like her: https://bbpress.org/forums/topic/capability-needed-for-subscription-notification/

I guess on the other hand, changing it would make it obvious what it does. Although it really is pretty well commented and not unobvious at all.

The big thing is, subscriptions were just changed and some people had to change what they did using these hooks. I've just done that and if I updated without carefully checking, my users would be getting two subscription emails for every new reply (depending on how 'remove_action' fails, I suppose, I don't know).

So I'd leave it for now at least because the best case is that it looks better and makes it slightly easier in the future and the worst case is that you seriously annoy people who have just had to deal with it in the last update.

#3 @tharsheblows
5 years ago

  • Cc jjjay@… added

#4 @netweb
4 years ago

  • Keywords 2nd-opinion removed
  • Milestone 2.6 deleted
  • Resolution set to maybelater
  • Status changed from new to closed

Agreed, maybelater, maybe never thanks :)

Note: See TracTickets for help on using tickets.