[buddypress-trac] [BuddyPress Trac] #6005: No-js bulk deletion of messages

buddypress-trac noreply at wordpress.org
Thu Nov 20 02:09:04 UTC 2014


#6005: No-js bulk deletion of messages
----------------------------------------+------------------
 Reporter:  lakrisgubben                |       Owner:
     Type:  enhancement                 |      Status:  new
 Priority:  normal                      |   Milestone:  2.2
Component:  Messaging                   |     Version:
 Severity:  normal                      |  Resolution:
 Keywords:  needs-refresh dev-feedback  |
----------------------------------------+------------------

Comment (by boonebgorges):

 +1 to what hnla said - patch is looking great. I really like the new
 layout. We talked about the changes in this week's dev chat, and there was
 general agreement that this is a big improvement, and that we should go
 ahead with it for 2.2.

 On that note, a couple nitty-gritty comments on the patch:

 * There is some whitespace funkiness, like double-spaces between `!` and
 `bp_is_action_variable()`. Have a second look at this.
 * Our general pattern for checking nonces in "action" functions like
 `bp_messages_action_mark_read()` is: check `wp_verify_nonce()`, and if it
 fails, return false out of the function; if it passes, go ahead and do the
 logic. In other words, don't bother with `bp_core_add_message()` and the
 redirect if the nonce check fails.
 * In `bp_messages_action_bulk_manage()`, there's something kinda funny
 about looping through each thread, and bailing within the loop if
 `messages_check_thread_access()` fails. It means that you could end up
 successfully performing the bulk action on *some* of the threads, but then
 display a message saying that there was a problem and not process the
 rest. It means a little extra looping, but I'd suggest doing a loop up
 front and checking for thread access *first*, and only process the action
 once the entire loop has been validated.
 * Can we take this chance to move some of the table styling into the
 stylesheet? I'm thinking of the `<td>` widths.

--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/6005#comment:29>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac


More information about the buddypress-trac mailing list