Skip to content

[Highlight MFA] Use roles instead of capabilities#20

Merged
andrea-sdl merged 4 commits intoproductionfrom
add/PLTFRM-763-use-roles
May 28, 2025
Merged

[Highlight MFA] Use roles instead of capabilities#20
andrea-sdl merged 4 commits intoproductionfrom
add/PLTFRM-763-use-roles

Conversation

@andrea-sdl
Copy link
Copy Markdown
Contributor

@andrea-sdl andrea-sdl commented May 26, 2025

Description

In this PR, we're moving away from capabilities, and then using roles instead to highlight the users who need MFA:

CleanShot 2025-05-26 at 15 10 06@2x

Pre-review checklist

Please make sure the items below have been covered before requesting a review:

  • This change works and has been tested locally or in Codespaces (or has an appropriate fallback).
  • This change has relevant unit tests (if applicable).
  • This change has relevant documentation additions / updates (if applicable).
  • I've created a changelog description that aligns with the provided examples.

Pre-deploy checklist

  • VIP staff: Ensure any alerts added/updated conform to internal standards (see internal documentation).

Steps to Test

  • Check out PR & ensure the module is active on your site
  • vip dev-env create && vip dev-env start
  • Generate users
vip dev-env exec --slug=vip-security-boost -- wp user generate --count=1 --role=administrator
vip dev-env exec --slug=vip-security-boost -- wp user generate --count=1 --role=editor
vip dev-env exec --slug=vip-security-boost -- wp user generate --count=1 --role=author
vip dev-env exec --slug=vip-security-boost -- wp user generate --count=1 --role=contributor
vip dev-env exec --slug=vip-security-boost -- wp user generate --count=1 --role=subscriber
  • play with the config of the module, using
"highlight-mfa-users": {
        "roles": ["administrator","editor","author"]
      }
  • Navigate to wp-admin/users and verify that the count matches the module config. Click the filter link in the notice and ensure the filtered users are the one with the selected roles.

Copilot AI review requested due to automatic review settings May 26, 2025 13:17
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR replaces the use of capabilities with roles for highlighting users who need MFA. The key changes include updating configuration defaults, modifying query parameters in both the unit tests and main class, and adjusting corresponding expected outcomes.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tests/phpunit/test-highlight-mfa-users.php Updated test assertions and expected user count for roles filter
modules/highlight-mfa-users/class-highlight-mfa-users.php Renamed and replaced capabilities with roles in filtering logic
Comments suppressed due to low confidence (2)

modules/highlight-mfa-users/class-highlight-mfa-users.php:11

  • Update the PHPDoc to reflect that the variable now holds roles instead of capabilities.
 * @var array An array of capability slugs.

tests/phpunit/test-highlight-mfa-users.php:127

  • Double-check that the test expectations for roles filtering correctly cover all necessary role scenarios. Ensure additional tests exist if additional roles might be used.
$this->assertEquals( [ 'administrator' ], $roles_in_query );

@andrea-sdl andrea-sdl requested a review from a team May 26, 2025 13:18
Copy link
Copy Markdown
Contributor

@brunobasto brunobasto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@andrea-sdl andrea-sdl added this pull request to the merge queue May 28, 2025
Merged via the queue into production with commit fee4cae May 28, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants