Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Admin user data to JetPack connected event #108

Merged
merged 28 commits into from
Nov 18, 2024
Merged
Changes from 5 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
8b17c7c
Add Admin user data to JetPack connected event
mr-vara Nov 11, 2024
5b792ee
Fix lint issues
mr-vara Nov 11, 2024
5a08584
Move the get admin user to Plugin Helper
mr-vara Nov 11, 2024
2401455
Fix lint issues
mr-vara Nov 11, 2024
96afb63
Keep important fields only
mr-vara Nov 13, 2024
80a3960
Add `jetpack` plugin
BrianHenryIE Nov 15, 2024
7143ee3
Change `static` to instance methods
BrianHenryIE Nov 15, 2024
74d5a28
Add types
BrianHenryIE Nov 15, 2024
de93bed
Create PluginWPUnitTest.php
BrianHenryIE Nov 15, 2024
1b79ec0
Add `WP_CONTENT_DIR` to use Composer installed plugins in tests
BrianHenryIE Nov 15, 2024
38d0ee8
Use `isset()`
BrianHenryIE Nov 15, 2024
0d2b3b6
Strict compare the value, otherwise '"false"' would evaluate to 'true'
BrianHenryIE Nov 15, 2024
0363f52
Add tests for `NewfoldLabs\WP\Module\Data\Helpers\Plugin`
BrianHenryIE Nov 15, 2024
dd41adb
Update patchwork.json
BrianHenryIE Nov 15, 2024
db8d438
Update clients to use instance of class rather than static methods
BrianHenryIE Nov 15, 2024
263222f
Update tests after `auto_update_plugin` fix
BrianHenryIE Nov 15, 2024
038b60f
Add default paths so it runs without parameters
BrianHenryIE Nov 15, 2024
67f252a
Phive install `brianhenryie/php-diff-test`
BrianHenryIE Nov 15, 2024
0f89d2e
Revert "Phive install `brianhenryie/php-diff-test`"
BrianHenryIE Nov 15, 2024
1af8eb2
Allow injecting `Helpers/Plugin`
BrianHenryIE Nov 18, 2024
3626318
Use Mockery instead of Patchwork.
BrianHenryIE Nov 18, 2024
d55aea7
Merge remote-tracking branch 'origin' into PRESS11-17-v2
BrianHenryIE Nov 18, 2024
d10b59b
Update deprecated diff filter config
BrianHenryIE Nov 18, 2024
6436c2b
Use diff in `compose cs-changes` script
BrianHenryIE Nov 18, 2024
dda58c8
Show progress and sniff codes in all reports
BrianHenryIE Nov 18, 2024
85c4630
Remove TODO – covered by a broad Jira ticket
BrianHenryIE Nov 18, 2024
0db7084
`phpcs:ignore` the verify token, which is a nonce
BrianHenryIE Nov 18, 2024
5ec6f29
Rename `codecoverage-main` to `unit-tests`
BrianHenryIE Nov 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions includes/Helpers/Plugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ public static function get_data( $basename, $data, $mu = false ) {
$plugin['mu'] = $mu;
$plugin['auto_updates'] = ( ! $mu && self::does_it_autoupdate( $basename ) );

if ( strpos( $basename, 'jetpack' ) !== false ) {
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't this string match on other Jetpack plugins? Given that Jetpack has the main plugin and other standalone plugins, I'd imagine that maybe we should do an exact match.

$plugin['users'] = self::get_admin_users();
}

return $plugin;
}

Expand All @@ -87,4 +91,29 @@ public static function does_it_autoupdate( $slug ) {

return in_array( $slug, $wp_auto_updates, true );
}

/**
* Get Admin and SuperAdmin user accounts
*
* @return $users Array of Admin & Super Admin users
Copy link
Contributor

Choose a reason for hiding this comment

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

To correctly add the type here (so IDEs can understand it, and so people reading find it in the place it's typically expected), the format is <@var|@param|@return> <type> <$variable> <comment>, and in the case of arrays, please include the array shape, i.e. in this case it's actually an array of arrays, but that is not communicated so far.

@return array<array{id:int, email:string}> $users Array of Admin & Super Admin users

*/
private static function get_admin_users() {
Copy link
Contributor

@BrianHenryIE BrianHenryIE Nov 15, 2024

Choose a reason for hiding this comment

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

  • We should generally prefer non-static functions. A couple of reasons (although not relevant here): they're easier to work with when testing and easier to work with when extending classes.
  • Please include the return type : array

Edit: I see now that all the other functions in this class are static, so I guess you had to use static anyway!

// Get all admin users
$admin_users = get_users(
array(
'role' => 'administrator',
)
);
$users = array();

// Add administrators to the $users and check for super admin
foreach ( $admin_users as $user ) {
$users[] = array(
'id' => $user->ID,
'email' => $user->user_email,
);
}

return $users;
}
}
Loading