Improve Higlight-MFA Module notice to specify the affected roles#28
Improve Higlight-MFA Module notice to specify the affected roles#28andrea-sdl merged 3 commits intoproductionfrom
Conversation
…is are admins/editors
There was a problem hiding this comment.
Pull Request Overview
This PR refines the Highlight MFA module's notice messaging to explicitly reference affected high-privilege roles, either “Administrator or Editor” for the default configuration or a customizable set of roles. The changes include updating translation strings in the PHPUnit tests and refactoring the module's logic by introducing a default role constant and helper methods for generating the notice messages.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/phpunit/test-highlight-mfa-users.php | Updates to translation strings and tests to support role-specific notices. |
| modules/highlight-mfa-users/class-highlight-mfa-users.php | Refactored default role configuration and message generation logic. |
andrea-sdl
changed the title
| $notice_message_text = ''; | ||
| if ( $is_default_config ) { | ||
| $notice_message_text = sprintf( | ||
| /* Translators: %d is the number of users with Administrator or Editor roles and 2FA disabled. */ |
There was a problem hiding this comment.
Minor: Indentation looks off here
There was a problem hiding this comment.
@brunobasto Agree. I wonder why it wasn't caught by the linting.
brunobasto
left a comment
brunobasto
left a comment
There was a problem hiding this comment.
Tested and looks good! Thank you for the quick fix!
Description
When using the default config, the Highlight MFA module will create a notice saying that there are users without MFA.
That was considered too broad and would not represent the actual config which is based on the administrators and editors who do not have MFA active.
In this PR we're introducing a change in the messaging so that whenever we're using the default config, we're exposing the fact that administrators and editors are the one affected by this notice.
At the same time if the module is set up to use a different set of roles, we will also support those by having a more generic yet less broad definition which uses the term "high privileges" to indicate that the affected users are relevant when managing the site.
Pre-review checklist
Please make sure the items below have been covered before requesting a review:
Pre-deploy checklist
Steps to Test
wp-admin>Userslist and check the two new messages, click on the filter link and check that the message is consistent.