Changeset 12749

Timestamp:

07/20/2023 04:01:22 AM (2 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;
+    }
+
 }