[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