Skip to:
Content

BuddyPress.org

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#5266 closed defect (bug) (fixed)

Replace older notification functions with new bp-notifications functions

Reported by: r-a-y's profile r-a-y Owned by:
Milestone: 1.9 Priority: normal
Severity: normal Version:
Component: Toolbar & Notifications Keywords: has-patch
Cc:

Description (last modified by r-a-y)

Stemming from ticket:5259#comment:6, we should use the new bp-notifications functions instead of the older ones in bp-member-notifications.php.

The attached patch also replaces the recently-introduced notification wrapper functions (bp_core_mark_notifications_by_X()) functions for the native ones (bp_notifications_mark_notifications_by_X()) that were specifically written for the new component.

Attachments (7)

5266.01.patch (25.2 KB) - added by r-a-y 10 years ago.
5266.friends.patch (11.5 KB) - added by johnjamesjacoby 10 years ago.
Friends Notifications abstraction
5266.friends.2.patch (20.6 KB) - added by johnjamesjacoby 10 years ago.
5266.friends.3.patch (25.7 KB) - added by johnjamesjacoby 10 years ago.
Commit worthy patch. Includes the best of both friends patches, plus some surrounding clean-up
5266.messages.patch (9.2 KB) - added by johnjamesjacoby 10 years ago.
Clean up Messages Notifications integration
5266.activity.patch (13.3 KB) - added by johnjamesjacoby 10 years ago.
Activity
5266.groups.patch (42.1 KB) - added by johnjamesjacoby 10 years ago.
Groups Notifications

Download all attachments as: .zip

Change History (22)

@r-a-y
10 years ago

#1 @r-a-y
10 years ago

  • Description modified (diff)
  • Summary changed from Replace bp_core_mark_notifications_by_X() functions with native bp-notification functions to Replace older notification functions with new bp-notifications functions

#2 @boonebgorges
10 years ago

The swap-outs look good to me.

Re deprecation: If we're going to deprecate bp_core_delete_notifications_from_user() etc as noted in the docblocks, we should actually move them to a deprecated functions file, and use _deprecated_function(). This should be more effective if we actually want plugin authors to move away from their use.

#3 @johnjamesjacoby
10 years ago

Agree with Boone. Deprecation seems fine, and we should use _deprecated_function() where applicable.

Attaching a patch with how I'd like to see us abstract notifications out for each component, using Friends as an example.

#4 @johnjamesjacoby
10 years ago

Ray, your patch breaks notifications for all components. :)

bp_notifications_add_notification() does not share the same argument pattern as bp_core_add_notification(). Notifications uses an arguments array, Core uses individual arguments.

@johnjamesjacoby
10 years ago

Friends Notifications abstraction

#5 @johnjamesjacoby
10 years ago

New patch cleans up code and also moves friendship-accepted activity into the same action that notifications get moved to.

@johnjamesjacoby
10 years ago

Commit worthy patch. Includes the best of both friends patches, plus some surrounding clean-up

#6 @johnjamesjacoby
10 years ago

In 7619:

Refactor Friends component's approach to Notifications integration:

  • Relocate friends_clear_friendship_notifications() out of bp-friends-cache.php and into bp-friends-notifications.php
  • Introduce helper functions for handling the adding/marking/deleting of notifications. Hook these new functions into their respective actions rather than have them hardcoded and interspersed amongst the first-class code.
  • See #5266. Hat-tip r-a-y.

@johnjamesjacoby
10 years ago

Clean up Messages Notifications integration

#7 @johnjamesjacoby
10 years ago

In 7621:

Refactor Messages component's approach to Notifications integration:

  • Introduce helper functions for handling the adding/marking/deleting of notifications. Hook these new functions into their respective actions rather than have them hardcoded and interspersed amongst the first-class code.
  • See #5266.

@johnjamesjacoby
10 years ago

Activity

#8 @johnjamesjacoby
10 years ago

In 7622:

Refactor Activity component's approach to Notifications integration:

  • Introduce helper functions for handling the adding/marking/deleting of notifications. Hook these new functions into their respective actions rather than have them hardcoded and interspersed amongst the first-class code.
  • See #5266.

@johnjamesjacoby
10 years ago

Groups Notifications

#9 @johnjamesjacoby
10 years ago

In 7624:

Refactor Groups component's approach to Notifications and Activity integration:

  • Introduce helper functions for handling the adding/marking/deleting of notifications and adding/deleting activity stream entries. Hook these new functions into their respective actions rather than have them hardcoded and interspersed amongst the first-class code.
  • Inadvertently fixes a bug where banning or removing a member from a group within 5 minutes of their having joined would not delete the recent activity stream update.
  • See #5266.

#10 @johnjamesjacoby
10 years ago

In 7627:

Add deprecated notices to Core notifications functions. Hat-tip boonebgorges, r-a-y. See #5266.

#11 @johnjamesjacoby
10 years ago

In 7628:

Swap old Core notifications functions for new native Notifications components ones. Props r-a-y. See #5266.

#12 @johnjamesjacoby
10 years ago

In 7629:

Make $friendship an optional parameter to avoid debug notices and maintain backwards compatibility. See #5266.

#13 @johnjamesjacoby
10 years ago

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

All done here; closing as fixed. Thanks gents.

#14 @r-a-y
10 years ago

Thanks for your work on this, JJJ!

#15 @r-a-y
10 years ago

In 7652:

Groups: Fix invalid arguments for new notification function calls.

See #5266, #5282.

Props imath.

Note: See TracTickets for help on using tickets.