[buddypress-trac] [BuddyPress Trac] #8292: Multiple member types users table issue

[buddypress-trac]

#8292: Multiple member types users table issue
-------------------------------------+---------------------
 Reporter:  etatus                   |       Owner:  (none)
     Type:  defect (bug)             |      Status:  new
 Priority:  normal                   |   Milestone:  7.0.0
Component:  Members                  |     Version:  5.2.0
 Severity:  major                    |  Resolution:
 Keywords:  has-patch needs-refresh  |
-------------------------------------+---------------------
Changes (by imath):

 * keywords:  has-patch => has-patch needs-refresh


Comment:

 Hi @vapvarun

 Thanks a lot for you great work on this. The video looks very promising
 🤩. I must say I have a very strong concern about changing the 2nd
 argument type of the `bp_set_member_type()` function from a string to an
 array because it will break BuddyPress plugins using this function. It
 will also break the BP REST API and if you run the PHPUnit test suite, you
 will probably see a lot unit tests fail.

 I believe we possibly have 2 options to deal with this :
 1. create a new function to set member type**s** like
 `bp_set_member_types()`
 2. accept array as well as string for the `bp_set_member_type()` function
 and put the string into an array instead of returning false.

 The 2nd option seems to be what was chosen for the Group Type feature. So
 I guess we should use this way to keep some consistency. Have a look at
 the `bp_groups_set_group_type()` function.

 The 2 very important things we **absolutely need to preserve** about the
 `bp_set_member_type()` function are :
 - `bp_set_member_type( $user_id, 'foo' )` must add the 'foo' member type
 to the user.
 - `bp_set_member_type( $user_id, '' )` must remove all member types from a
 user as it's used by the `bp_remove_member_type_on_user_delete()` function
 to do so.

 About the rest of your patch in `src/bp-members/classes/class-bp-members-
 admin.php`:
 1. Lines 1301 to 1306 should look like this:

 {{{
         1301    <option value="">
         1302        <?php // should have its own line
         1303        /* translators: no option picked in select box */
         1304        esc_attr_e( '----', 'buddypress' );
         1305        ?> // should be on a new line.
         1306    </option>
 }}}

 Same thing for lines 1308 to 1312, they should look like this:

 {{{

         1308    <?php // should have its own line
         1309    if ( ! empty( $current_types ) && in_array( $type->name,
 $current_types ) ) { // add a space between 'if' and '(' and before '{'
         1310        $selected = 'selected';
         1311    } else { // add a space before and after 'else'
         1312        $selected = '';
         1313    }
         1314    ?> // should be on a new line.
 }}}

 Let's strictly follow [https://developer.wordpress.org/coding-standards
 /wordpress-coding-standards/php/ WordPress PHP code standards] 😉 for the
 new code we're adding (we'll need to review existing code later)

 2. What's the goal of line 2340 : ```$counter = 0;```
 3. It looks like there's an indentation problem from line 2351 to 2364.
 4. At line 2510 : ```if ( count( $types ) > 5 )``` why slicing the array
 when there are more than 5 member types ?
 5. lines 2522 to 2523 should look like this:
 {{{
 2522    $url           = add_query_arg( array( 'bp-member-type' =>
 urlencode( $type ) ) );
 2523    $member_types .= '<a href="' . esc_url( $url ) . '">' . esc_html(
 $type_obj->labels['singular_name'] ) . '</a>';
 // consecutive equal signs must be aligned.
 }}}

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