[buddypress-trac] [BuddyPress Trac] #8448: Add new table to store nonmember opt-outs.

[buddypress-trac]

#8448: Add new table to store nonmember opt-outs.
------------------------------------+---------------------
 Reporter:  dcavins                 |       Owner:  (none)
     Type:  enhancement             |      Status:  new
 Priority:  normal                  |   Milestone:  8.0.0
Component:  Core                    |     Version:  7.2.0
 Severity:  normal                  |  Resolution:
 Keywords:  has-patch dev-feedback  |
------------------------------------+---------------------

Comment (by imath):

 Hi @dcavins

 I just gave a closer look to your patch and I have some more suggestions
 and advices. I know I could have asked you to "--no-prefix" your patch but
 as I knew I'd suggest you some possible improvements I've thought it was
 faster to do it myself: that's the purpose of
 [https://buddypress.trac.wordpress.org/attachment/ticket/8448/8448.3.patch
 8448.3.patch].

 First, thanks again for iterating on the patch in a great way like you
 did. I've tested it and I've "phpunit" tested it. And it works as
 expected.

 ==== Code Formatting / escaping improvements ====
 In
 [https://buddypress.trac.wordpress.org/attachment/ticket/8448/8448.3-advices.patch
 8448.3-advices.patch] you'll find some multiline functions, array
 parameters & equal signs alignment advices (things we could potentially
 make more automatic using WPCS if #7228 were fixed 😉). There's also some
 recommanded changes about escaping some outputs and about using the `_n()`
 to output the search information at the top of the Nonmember Opt-outs
 tool's page.

 ==== Keeping 1 and 1 only BuddyPress tools submenu ====
 I believe just like we are doing for settings, we should use tabs to
 organize our tools pages, now we have more than one. So
 [https://buddypress.trac.wordpress.org/attachment/ticket/8448/8448.3
 -tools-tabs.patch 8448.3-tools-tabs.patch] is a first attempt (you'll need
 to improve if you agree with my belief) to move this way.

 The first thing I've added is a specific section for the Nonmember opt-
 outs tool into the WP Available tools Admin Screen. Here's a screenshot
 below:

 [[Image(https://cldup.com/ljnAprrzHh.png)]]
 PS: you'll need to improve the text of the card ;)

 Then I've made `bp_core_admin_tabs()` and `bp_core_get_admin_tabs()`
 usable for the 'tools' context, so that this area looks like this:

 [[Image(https://cldup.com/wz173xjx-B.png)]]

 Looking into this I've noticed you were using `network_admin_url()` in a
 way that might not generate the URL you expect it. I've tested it dumping
 `network_admin_url( 'network-tools' );` and it output like a subdirectory
 url eg: `site.url/wp-admin/network-tools/` which does not exist. I
 remember we had to create the Tools menu and available tools / BP Tools
 submenus on multisite admin but I believe the main admin page is
 `admin.php?page=$slug` where slug is the slug of your tool's page. I'd
 advice you to test this part on a Multisite config to check everything
 works as expected.

 Thanks again for your work, it's shaping great 💪

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