Skip to:
Content

BuddyPress.org

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#6663 closed enhancement (fixed)

Allow plugins to format profile-related notifications.

Reported by: dcavins Owned by: dcavins
Milestone: 2.4 Priority: normal
Severity: normal Version: 1.9
Component: Extended Profile Keywords: has-patch
Cc: dcavins

Description

The XProfile component doesn't create any notifications of its own. However, a plugin may want to create profile-related notifications. For instance, if you already have 50,000 users and you add a question to the "Base" profile group, wouldn't it be great to create a notification asking your members to update that profile group?

My patch adds the skeleton of a notification callback for the Profile component which allows plugins to hook uncaught actions much like recent changes to groups_format_notifications().

There's one weird bit, and that's that the xprofile component's key in the $bp array is profile. I'm not sure of the best way to handle that, so those cases are clunkily handled via special rules in bp-notifications/bp-notifications-functions.php

Thanks for your feedback.

Attachments (2)

6663.01.patch (3.5 KB) - added by dcavins 5 years ago.
Allow plugins to format profile-related notifications.
6663.02.patch (3.9 KB) - added by dcavins 5 years ago.
Better notes; Yoda logic.

Download all attachments as: .zip

Change History (10)

@dcavins
5 years ago

Allow plugins to format profile-related notifications.

This ticket was mentioned in Slack in #buddypress by dcavins. View the logs.


5 years ago

#2 @r-a-y
5 years ago

dcavins - Are you recording xprofile notifications with the component_name set to xprofile? If so, why not record notifications with the component_name set to profile?

That would probably solve the issue in bp-notification-functions.php.

Edit - Looks like we do the profile / xprofile dance in a few other spots:

  • bp_get_nav_menu_items()
  • bp_attachments_cover_image_is_edit()
  • bp_is_current_component()
  • bp_is_active()
  • bp_set_theme_compat_feature()

Based on the above, the patch looks okay! :)

Last edited 5 years ago by r-a-y (previous) (diff)

#3 @dcavins
5 years ago

  • Owner set to dcavins
  • Status changed from new to accepted

Hi @r-a-y-

Using profile for the component_name in the notification _would_ avoid the first issue at about line 200 (and a note in the format callback would help devs know what to use). The second piece at about line 600 comes from the keys in $bp->active_components. (It's weird that the active_components uses profile for its key, but $bp doesn't, ha ha.)

Thanks for your input!

#4 @boonebgorges
5 years ago

The approach in the patch looks fine to me.

A little background on the xprofile vs profile thing. xprofile is an optional component, and when it's disabled, BuddyPress uses some information from the WordPress user profile to fill in stuff like "display name". So I think profile was chosen as the top-level component name in order to account for these cases, so that you could grab something from $bp->profile without knowing whether xprofile was active. In retrospect, it probably doesn't make much sense for xprofile to share a key with WP profile info #blamepeatling but here we are. In general, I'd say you should be using xprofile to refer to things that are dependent on bp-xprofile, and only use profile when you need compatibility with the global key (or in certain cases, the activity component).

#5 @dcavins
5 years ago

Thanks for your feedback, @r-a-y and @boonebgorges. I'll hold off another day in case John wants to weigh in.

Boone, when you say this:

In general, I'd say you should be using xprofile to refer to things that are dependent on bp-xprofile, and only use profile when >you need compatibility with the global key (or in certain cases, the activity component).

should I take that to mean that you'd prefer that we expect xprofile to be the component_name in a notification record (requiring the first sleight-of-hand/switcheroo in bp-notifications-functions.php)? I'll add a NB to developers in the new hook documentation either way.

Thanks again for chiming in. I appreciate that you both took the time to read over a minor patch this late in the game. :)

#6 @boonebgorges
5 years ago

should I take that to mean that you'd prefer that we expect xprofile to be the component_name in a notification record (requiring the first sleight-of-hand/switcheroo in bp-notifications-functions.php)?

Yes.

@dcavins
5 years ago

Better notes; Yoda logic.

#7 @dcavins
5 years ago

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

In 10284:

Allow plugins to format xprofile-related notifications.

Adds a hook so that plugins may generate
xprofile-related notifications (with the
component_name of xprofile) and
format those notification using a
dynamic filter based on the name
of the component_action.

Fixes #6663.

#8 @r-a-y
5 years ago

  • Milestone changed from Awaiting Review to 2.4
  • Version changed from 2.3.3 to 1.9
Note: See TracTickets for help on using tickets.