Skip to:

Opened 10 years ago

Closed 10 years ago

#5419 closed enhancement (fixed)

Object cache active notice

Reported by: r-a-y's profile r-a-y Owned by: r-a-y's profile r-a-y
Milestone: 2.0 Priority: normal
Severity: normal Version: 1.0
Component: Messages Keywords: has-patch


When a user is logged in, we always fetch the active notice.

Let's object cache this unnecessary query as notices rarely change.

Attachments (1)

5419.01.patch (3.8 KB) - added by r-a-y 10 years ago.

Download all attachments as: .zip

Change History (4)

10 years ago

#1 @r-a-y
10 years ago

  • Keywords has-patch added

Attached patch:

  • Introduces a new hook - 'messages_notice_before_delete' to do something before a notice is deleted
  • Adds object caching to the get_active() method
  • Adds an invalidation function to bp-messages-filters.php
  • Adds a unit test

I guess what I need feedback on is the unit test. Should I be putting all the asserts in my one method? Or should I split them up into a different file? I kind of lumped them altogether since the asserts are related to one another.

Version 0, edited 10 years ago by r-a-y (next)

#2 @boonebgorges
10 years ago

Unit test looks fine to me. Theoretically you could split it into two tests, but it's fine to put them together. (Only practical difference is that when you do multiple assertions in a single test, and one fails, the rest of the assertions in that test are not run. So if you're going to put them together, make sure they're closely linked and would likely be broken by the same bug.)

I'm pretty sure do_action_ref_array() is not necessary here. do_action( 'messages_notice_before_delete', $this ) will pass the object by reference automatically, as of PHP 5.0.

#3 @r-a-y
10 years ago

  • Owner set to r-a-y
  • Resolution set to fixed
  • Status changed from new to closed

In 7994:

Add object caching when fetching the active notice.

This commit:

  • Introduces a new hook - 'messages_notice_before_delete' to do something before a notice is deleted
  • Adds object caching to the BP_Messages_Notice::get_active() method
  • Adds a cache invalidation function to bp-messages-cache.php
  • Adds a unit test

Fixes #5419

Note: See TracTickets for help on using tickets.