[buddypress-trac] [BuddyPress] #5148: Make notifications a separate component

buddypress-trac noreply at wordpress.org
Thu Nov 7 03:01:31 UTC 2013


#5148: Make notifications a separate component
----------------------------------+------------------------------
 Reporter:  johnjamesjacoby       |       Owner:  johnjamesjacoby
     Type:  defect (bug)          |      Status:  new
 Priority:  normal                |   Milestone:  1.9
Component:  Notifications         |     Version:
 Severity:  normal                |  Resolution:
 Keywords:  needs-patch needs-ui  |
----------------------------------+------------------------------

Comment (by boonebgorges):

 johnjamesjacoby - Thanks for the initial patch. You've done much of the
 hard work.

 As I mentioned to you earlier, I think that you were too conservative in
 porting from the current implementation. This is particularly true in the
 database class, where you've kept almost exactly the same logic. This
 loses all the elegance, flexibility, robustness, testability,
 maintainability, etc of what r-a-y had been exploring with his earlier
 patches.

 In 5148.02.patch, I implement most of what r-a-y originally suggested on
 top of 5148.02.patch. All update, insert, delete, and get queries are now
 being piped through single, all-purpose methods. Unit tests have been
 provided for most major database functionality. Since `$wpdb` provides
 `update()`, `insert()`, and `delete()`, but nothing for SELECT, I knew I'd
 need my own `get()` with my own SQL; since I was going to do it anyway, I
 added support so that all params for `get()` can be either single values
 or arrays of values for an `IN` clause. I think this technique will give
 us much more flexibility down the road, and will be far easier to maintain
 (in addition to being far more DRY).

 I've kept most of the original database methods suggested by 5148.patch,
 but they are all internally using the primary CRUD methods of the class.
 This includes everything under the header "Convienience Methods" in bp-
 notifications-classes.php. IMHO, these could all be removed, with their
 logic put instead into procedural functions with similar names. That'd
 keep the database class lean-n-mean, and promote a proper separation of
 duties.

 Other miscellaneous cleanup in 5148.02.patch:

 - Added/improved docblocks
 - Better coding standards compliance (commas after final array values,
 stuff like that)
 - Fixed some of the half-implemented logic for pagination (you had 'max'
 but hadn't fully implemented 'page' and 'per_page')

 I haven't really touched any of the front-end stuff, as I'm going a little
 nutso staring at this all day :) Pagination obviously needs to be cleaned
 up in the display, in addition to the concatenation of strings like you
 mention above.

 Another todo: activate the Notifications (and Settings) components for
 upgrading systems.

--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/5148#comment:14>
BuddyPress <http://buddypress.org/>
BuddyPress


More information about the buddypress-trac mailing list