Experimental auditing features #79
Conversation
adekbadek
force-pushed
the
feat/experimental-member-discrepancy-detection
branch
from
4947894
to
67569b5
Compare
There was a problem hiding this comment.
Works as expected @adekbadek
There are two very minor issues and a handful of nitpicks, and then this is okay. Nitpicks are not blocking and can be ignored.
| @@ -191,4 +191,16 @@ public static function admin_init() { | |||
| */ | |||
| public static function enqueue_scripts() { | |||
| } | |||
|
|
|||
| /** | |||
| * Has experimental auditing features? | |||
There was a problem hiding this comment.
nit: A return tag is missing and would be helpful here and in a handful of other methods in this PR
| @@ -29,10 +31,14 @@ protected function get_processed_item_output( $event ) { | |||
| * @return \Newspack_Network\Incoming_Events\Abstract_Incoming_Event[] $events An array of events. | |||
| */ | |||
| public function get_events() { | |||
| $roles_to_sync = \Newspack_Network\Utils\Users::get_synced_user_roles(); | |||
| if ( $roles_to_sync === [] ) { | |||
There was a problem hiding this comment.
Nit: Maybe we can use something like PHP empty here
| @@ -49,6 +55,8 @@ public function get_events() { | |||
| ] | |||
| ); | |||
|
|
|||
| WP_CLI::line( sprintf( 'Found %s users eligible for sync.', count( $users ) ) ); | |||
There was a problem hiding this comment.
We need to account for single user (Found 1 user eligible for sync)
| $edit_url = get_edit_post_link( $item['id'] ); | ||
| if ( isset( $item['node_url'] ) ) { | ||
| $edit_url = sprintf( '%s/wp-admin/post.php?post=%d&action=edit', $item['node_url'], $item['id'] ); | ||
| if ( $column_name === 'network_pass_discrepancies' && isset( $item['network_pass_discrepancies'] ) && $item['network_pass_id'] ) { |
There was a problem hiding this comment.
Should we be checking $item['network_pass_id'] with isset as well? Will this always be present at this stage?
There was a problem hiding this comment.
No need for that, network_pass_id is not optional. network_pass_discrepancies is, because it's only included if the auditing features are enabled.
| @@ -123,14 +132,44 @@ public static function get_membershp_plans_from_nodes() { | |||
| $network_pass_id = $meta->value; | |||
| } | |||
| } | |||
| if ( $network_pass_id && \Newspack_Network\Admin::use_experimental_auditing_features() ) { | |||
There was a problem hiding this comment.
Nit: Might be worth namespacing this since we call use_experimental_auditing_features multiple times in this file
adekbadek
force-pushed
the
feat/experimental-member-discrepancy-detection
branch
from
f18f18e
to
121cc68
Compare
# [1.6.0-alpha.1](v1.5.0...v1.6.0-alpha.1) (2024-04-11) ### Features * experimental auditing features ([#79](#79)) ([0c31fd6](0c31fd6))
|
🎉 This PR is included in version 1.6.0-alpha.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
# [1.6.0](v1.5.0...v1.6.0) (2024-04-24) ### Features * experimental auditing features ([#79](#79)) ([0c31fd6](0c31fd6))
|
🎉 This PR is included in version 1.6.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
All Submissions:
Changes proposed in this Pull Request:
A few auditing features, for now restricted to a specific WP user.
How to test the changes in this Pull Request:
NEWSPACK_NETWORK_EXPERIMENTAL_AUDITING_USERwp-config variable, with the value of the admin user you're using (e.g.admin)/wp-admin?users.php) on the Hub and Node sites, verify the new column features correct data and links* to create some discrepancies, add plan members and then quickly remove the corresponding event (
wp_newspack_hub_event_logtable) or webhook (np_webhook_requestpost type), if on hub or node, respectivelyOther information: