Changeset 12749

Timestamp:

07/20/2023 04:01:22 AM (3 years ago)

Author:

dd32

Message:

Support Forums: Improve the user notes for role changes to include the previous non-participant role, capture moderator/spectator role sets, and generally have cleaner code.

See #5707.

Location:

sites/trunk/wordpress.org/public_html/wp-content/plugins/support-forums/inc

Files:

• class-audit-log.php (modified) (5 diffs)
• class-hooks.php (modified) (6 diffs)

sites/trunk/wordpress.org/public_html/wp-content/plugins/support-forums/inc/class-audit-log.php

@@ -2 +2 @@
 
 namespace WordPressdotorg\Forums;
+use WP_User;
 
 class Audit_Log {
@@ -11 +12 @@
          */
         var $edited_notes = [];
+
+        /**
+         * Keeps track of the role changes for each user.
+         *
+         * @var array
+         */
+        var $last_role_change = [];
 
         function __construct() {
                 // Audit-log entries for Term subscriptions (add/remove)
-                add_action( 'wporg_bbp_add_user_term_subscription',    array( $this, 'term_subscriptions' ), 10, 2 );
-                add_action( 'wporg_bbp_remove_user_term_subscription', array( $this, 'term_subscriptions' ), 10, 2 );
+                add_action( 'wporg_bbp_add_user_term_subscription',    [ $this, 'term_subscriptions' ], 10, 2 );
+                add_action( 'wporg_bbp_remove_user_term_subscription', [ $this, 'term_subscriptions' ], 10, 2 );
+
+                // Add a user note for forum role changes.
+                add_action( 'add_user_role',     [ $this, 'monitor_role_changes' ], 10, 2 );
+                add_action( 'remove_user_role',  [ $this, 'monitor_role_changes' ], 10, 2 );
+                add_filter( 'bbp_set_user_role', [ $this, 'bbp_set_user_role' ],    10, 3 );
+
+                // Add a user note when flagging/unflagging a user.
+                add_action( 'wporg_bbp_flag_user',   [ $this, 'log_user_flag_changes' ], 10, 1 );
+                add_action( 'wporg_bbp_unflag_user', [ $this, 'log_user_flag_changes' ], 10, 1 );
 
                 // Slack logs for Moderation notes.
                 add_action( 'wporg_bbp_note_added', [ $this, 'forums_notes_added' ], 10, 2 );
+                add_action( 'shutdown',             [ $this, 'forums_notes_added_shutdown' ] );
         }
 
@@ -51 +69 @@
         }
 
+
+        /**
+         * Keep track of the role changes that happen via bbp_set_user_role.
+         *
+         * TODO: Check if this is still needed, bbPress might pass context via bbp_set_user_role.
+         */
+        function monitor_role_changes( $user_id, $role ) {
+                $this->last_role_change[ $user_id ] ??= [];
+                $this->last_role_change[ $user_id ][ current_filter() ] = $role;
+        }
+
+        /**
+         * Monitor for role changes, and log as appropriate.
+         *
+         * NOTE: This is also triggered on Locale forums. We may need to revisit that at some point.
+         */
+        function bbp_set_user_role( $new_role, $user_id, WP_User $user ) {
+                $previous_role = $this->last_role_change[ $user_id ][ 'remove_user_role' ] ?? false;
+
+                // For the purposes of this function, a previous participant role is irrelevant.
+                if ( bbp_get_participant_role() == $previous_role ) {
+                        $previous_role = false;
+                }
+
+                $monitored_roles = [
+                        bbp_get_keymaster_role(),
+                        bbp_get_moderator_role(),
+                        // bbp_get_participant_role(), // Not monitoring for changes involving only this role.
+                        bbp_get_spectator_role(),
+                        bbp_get_blocked_role()
+                ];
+
+                if (
+                        // If the role change is not one we're monitoring, bail.
+                        ! array_intersect( $monitored_roles, [ $new_role, $previous_role ] ) ||
+                        // If we can't detect any change, bail.
+                        $new_role === $previous_role
+                ) {
+                        return $new_role; // We're on a filter.
+                }
+
+                // Determine what triggered this change.
+                $where_from = ! ms_is_switched() ? home_url( '/' ) : $_SERVER['HTTP_HOST'] . $_SERVER['REQUEST_URI'];
+                $where_from = explode( '?', $where_from )[0];
+                $where_from = preg_replace( '!^(?:https?://)?(.+?)/?[^/]*$!i', '$1', $where_from );
+
+                // Add a user note about this action.
+                $note_text = sprintf(
+                        'Forum role changed to %s%s%s.',
+                        get_role( $new_role )->name,
+                        $previous_role ? sprintf( ' from %s', get_role( $previous_role )->name ) : '',
+                        $where_from ?    sprintf( ' via %s', $where_from ) : ''
+                );
+
+                // Used in wporg-login to add context.
+                $note_text = apply_filters( 'wporg_bbp_forum_role_changed_note_text', $note_text, $user );
+
+                // Add a user note about this action.
+                Plugin::get_instance()->user_notes->add_user_note_or_update_previous(
+                        $user->ID,
+                        $note_text
+                );
+
+                // It's a filter, so we need to return the value.
+                return $new_role;
+        }
+
+        /**
+         * Add a user note when a user is flagged / unflagged.
+         *
+         * @param int $user_id
+         */
+        function log_user_flag_changes( $user_id ) {
+                $flag_action = ( 'wporg_bbp_flag_user' === current_action() ) ? 'flagged' : 'unflagged';
+
+                $note_text = "User {$flag_action}.";
+
+                Plugin::get_instance()->user_notes->add_user_note_or_update_previous(
+                        $user_id,
+                        $note_text
+                );
+        }
+
         /**
          * Record Note changes to Slack.
@@ -60 +161 @@
                 $this->edited_notes[ $user_id ] ??= [];
                 $this->edited_notes[ $user_id ][] = $note_id;
-
-                if ( ! has_action( 'shutdown', [ $this, 'forums_notes_added_shutdown' ] ) ) {
-                        add_action( 'shutdown', [ $this, 'forums_notes_added_shutdown' ] );
-                }
         }
 
@@ -70 +167 @@
          */
         function forums_notes_added_shutdown() {
-                if ( ! function_exists( 'notify_slack' ) || ! defined( 'FORUMS_MODACTIONS_SLACK_CHANNEL' ) ) {
+                if ( ! $this->edited_notes || ! function_exists( 'notify_slack' ) || ! defined( 'FORUMS_MODACTIONS_SLACK_CHANNEL' ) ) {
                         return;
                 }

sites/trunk/wordpress.org/public_html/wp-content/plugins/support-forums/inc/class-hooks.php

@@ -136 +136 @@
                 add_filter( 'pre_oembed_result', array( $this, 'pre_oembed_result_dont_embed_wordpress_org_anchors' ), 20, 2 );
 
-                // Add a user note when flagging/unflagging a user.
-                add_filter( 'wporg_bbp_flag_user', array( $this, 'log_user_flag_changes' ) );
-                add_filter( 'wporg_bbp_unflag_user', array( $this, 'log_user_flag_changes' ) );
-
                 // Break users sessions / passwords when they get blocked, on the main forums only.
                 if ( 'wordpress.org' === get_blog_details()->domain ) {
-                        add_action( 'bbp_set_user_role', array( $this, 'user_blocked_password_handler' ), 10, 3 );
+                        add_filter( 'bbp_set_user_role', array( $this, 'user_blocked_password_handler' ), 10, 3 );
                 }
         }
@@ -1360 +1356 @@
          *
          * Note: This method is called even when the users role is not changed.
+         *
+         * See Audit_Log class for where the note is set/updated.
          */
         public function user_blocked_password_handler( $new_role, $user_id, \WP_User $user ) {
@@ -1366 +1364 @@
                 // ~~~ is a reset password on WordPress.org. Let's ignore those.
                 if ( '~~~' === $user->user_pass ) {
-                        return;
+                        return $new_role;
                 }
 
@@ -1374 +1372 @@
                 $blocked_role    = bbp_get_blocked_role();
                 $password_broken = ( 0 === strpos( $user->user_pass, $blocked_prefix ) );
-                $note_text       = false;
 
                 // WP_User::has_role() does not exist, and WP_User::has_cap( 'bbp_blocked' ) will be truthful for super admins.
                 $user_has_blocked_role = ! empty( $user->roles ) && in_array( $blocked_role, $user->roles, true );
-
-                // Define what has blocked the user.
-                if ( ! ms_is_switched() ) {
-                        $where_from = preg_replace( '!^https?://!i', '', home_url( is_admin() ? '/wp-admin' : '' ) );
-                } else {
-                        // When we're switched, we can't determine the source of the switch, so we use a bit of URL parsing magic.
-                        $where_from = $_SERVER['HTTP_HOST'] . $_SERVER['REQUEST_URI'];
-                        if ( str_contains( $where_from, '?' ) ) {
-                                list( $where_from, ) = explode( '?', $where_from );
-                        }
-                        // Trim actual filename off, just the major path component.
-                        $where_from = preg_replace( '!/[^/?]+\.[a-z]{3}$!i', '', $where_from  );
-                }
 
                 if (
@@ -1413 +1397 @@
                         $manager = \WP_Session_Tokens::get_instance( $user->ID );
                         $manager->destroy_all();
-
-                        // Add a user note about this action.
-                        $note_text = sprintf(
-                                $where_from ? 'Forum role changed to %s via %s.' : 'Forum role changed to %s.',
-                                get_role( $new_role )->name,
-                                $where_from
-                        );
-
-                        // Used in wporg-login to add context.
-                        $note_text = apply_filters( 'wporg_bbp_forum_role_changed_note_text', $note_text, $user );
                 } else if (
                         $password_broken &&
@@ -1440 +1414 @@
 
                         clean_user_cache( $user );
-
-                        // Add a user note about this action.
-                        $note_text = sprintf(
-                                $where_from ? 'Forum role changed to %s via %s.' : 'Forum role changed to %s.',
-                                get_role( $new_role )->name,
-                                $where_from
-                        );
-
-                        // Unused, here for consistency with above.
-                        $note_text = apply_filters( 'wporg_bbp_forum_role_changed_note_text', $note_text, $user );
-                }
-
-                if ( $note_text ) {
-                        // Add a user note about this action.
-                        Plugin::get_instance()->user_notes->add_user_note_or_update_previous(
-                                $user->ID,
-                                $note_text
-                        );
-                }
-
-        }
-
-        /**
-         * Add a user note when a user is flagged / unflagged.
-         */
-        function log_user_flag_changes( $user_id ) {
-                $flag_action = ( 'wporg_bbp_flag_user' === current_filter () ) ? 'flagged' : 'unflagged';
-
-                $note_text = "User {$flag_action}.";
-
-                Plugin::get_instance()->user_notes->add_user_note_or_update_previous(
-                        $user_id,
-                        $note_text
-                );
-        }
+                }
+
+                // It's a filter, return the value.
+                return $new_role;
+        }
+
 }