Skip to:
Content

Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#3158 closed defect (bug) (fixed)

bp_core -> bp_members has erased a bunch of hooks

Reported by: boonebgorges Owned by: johnjamesjacoby
Milestone: 1.5 Priority: critical
Severity: Version: 1.5
Component: Core Keywords: has-patch
Cc: johnjamesjacoby, Dennissmolek

Description

I was poking through the new bp-members files today, and noted that a bunch of hooks of the form bp_core_ (like bp_core_signup_user) have been changed to bp_members_signup_user. This will break many plugins and customizations, including some that I have written.

IMO, we should include both the old style hooks and the new hooks, with an inline note that the new hooks are to be preferred.

Attachments (1)

backpat.patch (2.4 KB) - added by DennisSmolek 8 years ago.
Patch of the ones I could find

Download all attachments as: .zip

Change History (19)

#1 @boonebgorges
8 years ago

  • Cc johnjamesjacoby added

#2 @boonebgorges
8 years ago

jjj, I'd like your view on this, as you made the changes. Was it just a case of overgenerous search-and-replace?

#3 @boonebgorges
8 years ago

As I think about this, I have similar issues with the changed function names themselves. Can we consider changing them back to bp_core_ (which was, I thought, the original plan)? I see that there are wrapper functions in the deprecated file for 1.3, but this has the potential to overload error logs, and to what end?

#4 @DJPaul
8 years ago

Sort of similar to #3100

#5 @johnjamesjacoby
8 years ago

Overzealous search/replace. I'd rather not rename the function names back to bp_core_ since the goal is to get them out of the bp-core component completely.

The deprecated notices should only happen when WP_DEBUG is set which shouldn't be in a live environment anyhow.

The action and filter names, that's a casualty I/we will need to manage. Can we hook the old actions/filters into the new ones, and pass the variables through it. We can incept them. You be Leonardo Decaprio, Paul will be Joseph Gordon-Levitt, and I will be Ken Watanabe since I'm the bad guy in this instance. :)

#6 @boonebgorges
8 years ago

  • Priority changed from major to critical

I don't feel all that strongly about the function names, though in the future I would prefer to avoid changing more function names unless really necessary. Backpat is more important than internal consistency with something as meaningless as function names.

As for the actions/filters, I would think that it will maximize readability to just stack the old ones on top of the new ones right in the core code.

I have not seen Inception, but I see how it would make sense for me to be DiCaprio, since both he and I are very handsome.

Going to bump the issue up in priority because it really must be done for release.

#7 @modemlooper
8 years ago

I'm a spinning top. :D

#8 @r-a-y
8 years ago

Is Ken Watanabe (JJJ) really the bad guy? He does force Leo (Boone) into the job though ;)

Yes this analogy works on both fronts!

@DennisSmolek
8 years ago

Patch of the ones I could find

#10 @Dennissmolek
8 years ago

  • Cc Dennissmolek added
  • Keywords has-patch added

#11 @DJPaul
8 years ago

  • Owner set to johnjamesjacoby
  • Status changed from new to assigned

#12 @johnjamesjacoby
8 years ago

(In [4605]) First pass at reverting members/core function & filter refactor. See #3158.

#13 @johnjamesjacoby
8 years ago

(In [4606]) Second pass at reverting members/core function & filter refactor. See #3158.

#14 @boonebgorges
8 years ago

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

I'm going to mark this as fixed, as I've not seen any problems after r4605 and r4606. If you find specific issues related to these function/hook names, please open a ticket.

#15 @hnla
8 years ago

Edit bit of a non issue after all? understand or remember now that I had used bp_core_get previously changed it to 'member' for 1.3 and have now reverted it back to 'core' slightly dizzy now :)

Just updated 1.3 checkout and these reversion to func names appear to have broken:

bp_members_get_notifications_for_user() - undefined function

had been using this since approx 1.2.6 or thereabouts

However just checked and changing to:

bp_core_get_notifications_for_user()

is working ok

Last edited 8 years ago by hnla (previous) (diff)

#16 @boonebgorges
8 years ago

I'm quite sure you're incorrect that bp_members_get_notifications_for_user() was in use in 1.2.x. Would you mind looking through some previous tags and pointing to where it was defined? https://buddypress.trac.wordpress.org/browser/tags

FWIW, this function was defined in the BP trunk for a month or two recently. But never in a stable tagged version.

#17 @hnla
8 years ago

Yes sorry I was trying to impart that. I was wrong bp_members_get... wasn't in 1.2, I had lost track of the fact I had had to change it to 'member' from 'core'( or thought I had to?) for 1.3. if I had thought to first look back at an older test install to see what function was used it would have become clearer.

Tis a little confusing though chaps :)

#18 @johnjamesjacoby
8 years ago

Hopefully the worst of that is behind us now. :)

Note: See TracTickets for help on using tickets.