Add Tracks integration #21

hanifn · 2024-08-21T16:02:47Z

Description

Add Tracks integration into the plugin. This is dependent on the following MU-plugins PR: Automattic/vip-go-mu-plugins#5770

Steps to Test

Check out the MU-plugin PR above.
Check out this PR into the plugins directory of a local VIP Go Skeleton repo
Create a new VIP dev-env with vip dev-env --slug workflow-test --app-code={path to vip-go-skeleton repo} --mu-plugins={path to MU-plugins repo}
Start the dev-env with vip dev-env --slug workflow-test start
Test the plugin

class-workflow.php

modules/telemetry/class-vip-workflow-tracker.php

modules/telemetry/events/notification-events.php

modules/telemetry/events/settings-events.php

…rom a custom status to 'publish'

…ook notification was sent, and track which

alecgeatches · 2024-10-10T19:32:05Z

modules/notifications/notifications.php

@@ -147,13 +147,36 @@ public static function notification_status_change( string $new_status, string $o

 			$action = 'status-change';

-			self::schedule_emails( $action, $post, $subject, $body );
+			$notification_email = OptionsUtilities::get_options_by_key( 'email_address' );


@ingeniumed The notification code has been modified to check for email/webhook presence in this upper function. We use this data ($is_email_scheduled and $is_webhook_scheduled) to determine if we should trigger vw_notification_status_change telemetry at all, and also to include which notification types were sent in that datapoint.

alecgeatches · 2024-10-10T19:33:05Z

modules/telemetry/telemetry.php

+
+		$status_slugs = wp_list_pluck( CustomStatus::get_custom_statuses(), 'slug' );
+
+		if ( ! in_array( $new_status, $status_slugs, true ) && ! in_array( $old_status, $status_slugs, true ) ) {


The logic in this function has been modified so that we'll still track if a post moves from some-custom-status to publish. If the prior or current status is custom, we track.

alecgeatches · 2024-10-10T19:34:03Z

modules/telemetry/telemetry.php

+	 * @param array $update_args The arguments used to update the status.
+	 */
+	public static function record_update_custom_status( WP_Term $updated_status, array $update_args ): void {
+		$is_position_update = 1 === count( $update_args ) && isset( $update_args['position'] );


Here we use the passed-in $update_args to determine if there's just a position switch happening. Before this code was added, any reorder caused an event to be tracked for every status.

alecgeatches · 2024-10-10T19:34:41Z

modules/telemetry/telemetry.php

+
+			foreach ( $custom_statuses as $status ) {
+				if ( isset( $posts_count->{ $status->slug } ) ) {
+					$custom_status_posts += (int) $posts_count->{ $status->slug };


This event has been modified to only count posts in a custom status. Previously we were also tracking published post counts, but I didn't see the need for that datapoint.

I think we want "published" as well for a few reasons:

Since each of these events has a timestamp, it might show associations in workflow shapes, complications, and publishing velocity.

If we have a site with zero published posts, we know something is wonky.

If we started to record Post IDs we could track the time it takes for a post to go from start to finish.

Maybe this is low value/far out, but I'm interested in others thoughts.

I think that's a good reason, I'll add it back in.

Agreed on that. This would also help us better support complicated workflows, and would be beneficial down the line to target new customers who haven't used WordPress before as they are fond of Adobe, etc instead.

alecgeatches · 2024-10-10T19:35:21Z

modules/telemetry/telemetry.php

+		}
+
+		if ( $new_options['email_address'] !== $old_options['email_address'] ) {
+			self::record_send_to_email_toggle( $new_options['email_address'] );


Added this datapoint for tracking when a custom email address is added/deleted/changed, which works the same as the webhook URL toggle tracking.

ingeniumed

Couple of minor changes that are about shifting the tracking calls around and adding more data to be tracked. The core logic is good imo and I'm super pumped to see this go in.

ingeniumed · 2024-10-10T23:44:09Z

modules/custom-status/custom-status.php

@@ -32,11 +32,11 @@ class CustomStatus {
 	const SETTINGS_SLUG = 'vw-custom-status';

 	// The metadata keys for the custom status term
-	const METADATA_POSITION_KEY = 'position';
+	const METADATA_POSITION_KEY          = 'position';


The spacing is interesting as I would have thought running composer run cbf should catch that. Anything that you think would help with this? Prettier maybe on php files as well?

Great question! I've been curious about this but hadn't tracked it down yet. After a quick investigation, it appears to be because this is a PHPCS warning, and we ignore those in our phpcs/phpcbf runs:

vip-workflow-plugin/composer.json

Lines 23 to 24 in e7e6d82

"cs": "@php ./vendor/bin/phpcs -p -s -v -n . --standard=\"phpcs.xml.dist\" --extensions=php --ignore=\"/vendor/*,/node_modules/*\"",

"cbf": "@php ./vendor/bin/phpcbf -p -s -v -n . --standard=\"phpcs.xml.dist\" --extensions=php --ignore=\"/vendor/*,/node_modules/*\"",

From running ./vendor/bin/phpcs --help, we can see the -n parameter is doing this:

-n Do not include warnings. Shortcut for "--warning-severity=0".

What's happening here is my local PHPCS extensions is auto-fixing this warning, but it's being ignored by our tests, even though it's a warning in our rules. If you're curious, here's the result of running phpcs without -n:

./vendor/bin/phpcs -p -s -v --standard="phpcs.xml.dist" --extensions=php --ignore="/vendor/*,/node_modules/*" # ... FILE: modules/shared/php/core-hacks.php ---------------------------------------------------------------------------------------------------------------------------------- FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE ---------------------------------------------------------------------------------------------------------------------------------- 85 | WARNING | The method parameter $post_parent is never used (Generic.CodeAnalysis.UnusedFunctionParameter.FoundAfterLastUsed) ---------------------------------------------------------------------------------------------------------------------------------- FILE: modules/custom-status/rest/custom-status-endpoint.php --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- FOUND 0 ERRORS AND 25 WARNINGS AFFECTING 25 LINES --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- 35 | WARNING | [x] Array double arrow not aligned correctly; expected 18 space(s) between "'name'" and double arrow, but found 14. (WordPress.Arrays.MultipleStatementAlignment.DoubleArrowNotAligned) 46 | WARNING | [x] Array double arrow not aligned correctly; expected 11 space(s) between "'description'" and double arrow, but found 7. (WordPress.Arrays.MultipleStatementAlignment.DoubleArrowNotAligned) 62 | WARNING | [x] Array double arrow not aligned correctly; expected 5 space(s) between "'required_user_ids'" and double arrow, but found 1. | | (WordPress.Arrays.MultipleStatementAlignment.DoubleArrowNotAligned) 81 | WARNING | [x] Array double arrow not aligned correctly; expected 18 space(s) between "'name'" and double arrow, but found 14. (WordPress.Arrays.MultipleStatementAlignment.DoubleArrowNotAligned) 90 | WARNING | [x] Array double arrow not aligned correctly; expected 20 space(s) between "'id'" and double arrow, but found 16. (WordPress.Arrays.MultipleStatementAlignment.DoubleArrowNotAligned) 103 | WARNING | [x] Array double arrow not aligned correctly; expected 11 space(s) between "'description'" and double arrow, but found 7. (WordPress.Arrays.MultipleStatementAlignment.DoubleArrowNotAligned) 119 | WARNING | [x] Array double arrow not aligned correctly; expected 5 space(s) between "'required_user_ids'" and double arrow, but found 1. | | (WordPress.Arrays.MultipleStatementAlignment.DoubleArrowNotAligned) 201 | WARNING | [x] Equals sign not aligned with surrounding assignments; expected 18 spaces but found 15 spaces (Generic.Formatting.MultipleStatementAlignment.NotSameWarning) 202 | WARNING | [x] Equals sign not aligned with surrounding assignments; expected 18 spaces but found 15 spaces (Generic.Formatting.MultipleStatementAlignment.NotSameWarning) 203 | WARNING | [x] Equals sign not aligned with surrounding assignments; expected 11 spaces but found 8 spaces (Generic.Formatting.MultipleStatementAlignment.NotSameWarning) 205 | WARNING | [x] Equals sign not aligned with surrounding assignments; expected 5 spaces but found 1 space (Generic.Formatting.MultipleStatementAlignment.NotSameWarning) 234 | WARNING | [x] Array double arrow not aligned correctly; expected 18 space(s) between "'name'" and double arrow, but found 15. (WordPress.Arrays.MultipleStatementAlignment.DoubleArrowNotAligned) 235 | WARNING | [x] Array double arrow not aligned correctly; expected 11 space(s) between "'description'" and double arrow, but found 8. (WordPress.Arrays.MultipleStatementAlignment.DoubleArrowNotAligned) 236 | WARNING | [x] Array double arrow not aligned correctly; expected 18 space(s) between "'slug'" and double arrow, but found 15. (WordPress.Arrays.MultipleStatementAlignment.DoubleArrowNotAligned) 238 | WARNING | [x] Array double arrow not aligned correctly; expected 5 space(s) between "'required_user_ids'" and double arrow, but found 1. | | (WordPress.Arrays.MultipleStatementAlignment.DoubleArrowNotAligned) 253 | WARNING | [x] Equals sign not aligned with surrounding assignments; expected 22 spaces but found 19 spaces (Generic.Formatting.MultipleStatementAlignment.NotSameWarning) 254 | WARNING | [x] Equals sign not aligned with surrounding assignments; expected 18 spaces but found 15 spaces (Generic.Formatting.MultipleStatementAlignment.NotSameWarning) 255 | WARNING | [x] Equals sign not aligned with surrounding assignments; expected 18 spaces but found 15 spaces (Generic.Formatting.MultipleStatementAlignment.NotSameWarning) 256 | WARNING | [x] Equals sign not aligned with surrounding assignments; expected 11 spaces but found 8 spaces (Generic.Formatting.MultipleStatementAlignment.NotSameWarning) 258 | WARNING | [x] Equals sign not aligned with surrounding assignments; expected 5 spaces but found 1 space (Generic.Formatting.MultipleStatementAlignment.NotSameWarning) 275 | WARNING | [x] Equals sign not aligned correctly; expected 1 space but found 3 spaces (Generic.Formatting.MultipleStatementAlignment.IncorrectWarning) 300 | WARNING | [x] Array double arrow not aligned correctly; expected 18 space(s) between "'name'" and double arrow, but found 15. (WordPress.Arrays.MultipleStatementAlignment.DoubleArrowNotAligned) 301 | WARNING | [x] Array double arrow not aligned correctly; expected 11 space(s) between "'description'" and double arrow, but found 8. (WordPress.Arrays.MultipleStatementAlignment.DoubleArrowNotAligned) 302 | WARNING | [x] Array double arrow not aligned correctly; expected 18 space(s) between "'slug'" and double arrow, but found 15. (WordPress.Arrays.MultipleStatementAlignment.DoubleArrowNotAligned) 304 | WARNING | [x] Array double arrow not aligned correctly; expected 5 space(s) between "'required_user_ids'" and double arrow, but found 1. | | (WordPress.Arrays.MultipleStatementAlignment.DoubleArrowNotAligned) --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- PHPCBF CAN FIX THE 25 MARKED SNIFF VIOLATIONS AUTOMATICALLY --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- FILE: modules/shared/php/util.php -------------------------------------------------------------------------------------------------------------------------------------------------------- FOUND 0 ERRORS AND 6 WARNINGS AFFECTING 3 LINES -------------------------------------------------------------------------------------------------------------------------------------------------------- 60 | WARNING | Processing form data without nonce verification. (WordPress.Security.NonceVerification.Recommended) 60 | WARNING | Processing form data without nonce verification. (WordPress.Security.NonceVerification.Recommended) 61 | WARNING | Processing form data without nonce verification. (WordPress.Security.NonceVerification.Recommended) 61 | WARNING | Detected usage of a non-sanitized input variable: $_GET['preview_id'] (WordPress.Security.ValidatedSanitizedInput.InputNotSanitized) 62 | WARNING | Processing form data without nonce verification. (WordPress.Security.NonceVerification.Recommended) 62 | WARNING | Detected usage of a non-sanitized input variable: $_GET['preview_nonce'] (WordPress.Security.ValidatedSanitizedInput.InputNotSanitized) -------------------------------------------------------------------------------------------------------------------------------------------------------- FILE: modules/shared/php/options-utilities.php ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ FOUND 0 ERRORS AND 4 WARNINGS AFFECTING 4 LINES ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ 14 | WARNING | [x] Equals sign not aligned with surrounding assignments; expected 6 spaces but found 1 space (Generic.Formatting.MultipleStatementAlignment.NotSameWarning) 26 | WARNING | [x] Equals sign not aligned with surrounding assignments; expected 5 spaces but found 1 space (Generic.Formatting.MultipleStatementAlignment.NotSameWarning) 80 | WARNING | [x] Equals sign not aligned with surrounding assignments; expected 8 spaces but found 7 spaces (Generic.Formatting.MultipleStatementAlignment.NotSameWarning) 81 | WARNING | [x] Equals sign not aligned with surrounding assignments; expected 8 spaces but found 1 space (Generic.Formatting.MultipleStatementAlignment.NotSameWarning) ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ FILE: modules/settings/settings.php --------------------------------------------------------------------------------------------------------------- FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE --------------------------------------------------------------------------------------------------------------- 125 | WARNING | The method parameter $args is never used (Generic.CodeAnalysis.UnusedFunctionParameter.Found) --------------------------------------------------------------------------------------------------------------- FILE: modules/shared/php/helper-utilities.php --------------------------------------------------------------------------------------------------------------------- FOUND 0 ERRORS AND 7 WARNINGS AFFECTING 6 LINES --------------------------------------------------------------------------------------------------------------------- 68 | WARNING | Processing form data without nonce verification. (WordPress.Security.NonceVerification.Recommended) 68 | WARNING | Processing form data without nonce verification. (WordPress.Security.NonceVerification.Recommended) 76 | WARNING | Processing form data without nonce verification. (WordPress.Security.NonceVerification.Recommended) 77 | WARNING | Processing form data without nonce verification. (WordPress.Security.NonceVerification.Recommended) 82 | WARNING | Processing form data without nonce verification. (WordPress.Security.NonceVerification.Recommended) 100 | WARNING | Processing form data without nonce verification. (WordPress.Security.NonceVerification.Recommended) 105 | WARNING | Processing form data without nonce verification. (WordPress.Security.NonceVerification.Recommended) --------------------------------------------------------------------------------------------------------------------- FILE: modules/settings/views/settings.php ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- 26 | WARNING | [x] Array double arrow not aligned correctly; expected 15 space(s) between "'type'" and double arrow, but found 1. (WordPress.Arrays.MultipleStatementAlignment.DoubleArrowNotAligned) 27 | WARNING | [x] Array double arrow not aligned correctly; expected 8 space(s) between "'dismissible'" and double arrow, but found 1. (WordPress.Arrays.MultipleStatementAlignment.DoubleArrowNotAligned) ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- FILE: modules/shared/php/meta-cleanup-utilities.php ------------------------------------------------------------------------------------------------------------------------------------------------------------ FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE ------------------------------------------------------------------------------------------------------------------------------------------------------------ 26 | WARNING | [x] Equals sign not aligned correctly; expected 1 space but found 2 spaces (Generic.Formatting.MultipleStatementAlignment.IncorrectWarning) ------------------------------------------------------------------------------------------------------------------------------------------------------------ PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY ------------------------------------------------------------------------------------------------------------------------------------------------------------ FILE: modules/preview/preview.php ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ 47 | WARNING | [x] Equals sign not aligned with surrounding assignments; expected 1 space but found 2 spaces (Generic.Formatting.MultipleStatementAlignment.NotSameWarning) 48 | WARNING | [x] Equals sign not aligned with surrounding assignments; expected 3 spaces but found 4 spaces (Generic.Formatting.MultipleStatementAlignment.NotSameWarning) ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ FILE: modules/editorial-metadata/editorial-metadata.php ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- FOUND 0 ERRORS AND 11 WARNINGS AFFECTING 11 LINES ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- 283 | WARNING | [x] Equals sign not aligned with surrounding assignments; expected 32 spaces but found 1 space (Generic.Formatting.MultipleStatementAlignment.NotSameWarning) 284 | WARNING | [x] Equals sign not aligned with surrounding assignments; expected 5 spaces but found 1 space (Generic.Formatting.MultipleStatementAlignment.NotSameWarning) 307 | WARNING | [x] Equals sign not aligned with surrounding assignments; expected 4 spaces but found 1 space (Generic.Formatting.MultipleStatementAlignment.NotSameWarning) 320 | WARNING | [x] Equals sign not aligned with surrounding assignments; expected 9 spaces but found 1 space (Generic.Formatting.MultipleStatementAlignment.NotSameWarning) 321 | WARNING | [x] Equals sign not aligned with surrounding assignments; expected 1 space but found 2 spaces (Generic.Formatting.MultipleStatementAlignment.NotSameWarning) 326 | WARNING | [x] Usage of ELSE IF is discouraged; use ELSEIF instead (PSR2.ControlStructures.ElseIfDeclaration.NotAllowed) 335 | WARNING | [x] Usage of ELSE IF is discouraged; use ELSEIF instead (PSR2.ControlStructures.ElseIfDeclaration.NotAllowed) 358 | WARNING | [x] Usage of ELSE IF is discouraged; use ELSEIF instead (PSR2.ControlStructures.ElseIfDeclaration.NotAllowed) 366 | WARNING | [x] Array double arrow not aligned correctly; expected 8 space(s) between "'name'" and double arrow, but found 4. (WordPress.Arrays.MultipleStatementAlignment.DoubleArrowNotAligned) 367 | WARNING | [x] Array double arrow not aligned correctly; expected 8 space(s) between "'slug'" and double arrow, but found 4. (WordPress.Arrays.MultipleStatementAlignment.DoubleArrowNotAligned) 394 | WARNING | [x] Usage of ELSE IF is discouraged; use ELSEIF instead (PSR2.ControlStructures.ElseIfDeclaration.NotAllowed) ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- PHPCBF CAN FIX THE 11 MARKED SNIFF VIOLATIONS AUTOMATICALLY ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- FILE: modules/notifications/notifications.php --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- 257 | WARNING | `wp_mail` should be used sparingly. For any bulk emailing should be handled by a 3rd party service, in order to prevent domain or IP addresses being flagged as spam. | | (WordPressVIPMinimum.Functions.RestrictedFunctions.wp_mail_wp_mail) --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- FILE: modules/custom-status/custom-status.php ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- FOUND 0 ERRORS AND 5 WARNINGS AFFECTING 3 LINES ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- 35 | WARNING | [x] Equals sign not aligned with surrounding assignments; expected 10 spaces but found 1 space (Generic.Formatting.MultipleStatementAlignment.NotSameWarning) 383 | WARNING | [ ] json_encode() is discouraged. Use wp_json_encode() instead. (WordPress.WP.AlternativeFunctions.json_encode_json_encode) 922 | WARNING | [ ] Processing form data without nonce verification. (WordPress.Security.NonceVerification.Recommended) 922 | WARNING | [ ] Processing form data without nonce verification. (WordPress.Security.NonceVerification.Recommended) 922 | WARNING | [ ] Detected usage of a non-sanitized input variable: $_REQUEST['post_status'] (WordPress.Security.ValidatedSanitizedInput.InputNotSanitized) ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- Time: 660ms; Memory: 14MB

It looks like most of these are alignment, with a few other changes like else if -> elseif and some nonce checks we'll either need to add or ignore.

Since we don't have a lot of these, I think it would make sense to remove -n in another PR along with the fixes. This would help avoid inconsistencies with local formatting tools, and I think our code is in a close enough state to just remove these last warnings. What do you think?

ingeniumed · 2024-10-10T23:47:20Z

modules/custom-status/custom-status.php

+		 * @param string $term_name The name of the custom status.
+		 * @param string $slug The slug of the custom status.
+		 */
+		do_action( 'vw_add_custom_status', $term_id, $term_name, $term_to_save['slug'] );


I'd actually not put this here. This should be on line 571 instead, right before the get_custom_status_by call. I've tried to make the whole process transactional:

Save the status

Save each of the metadata info as term_meta linked to the status

If any step fails, delete all the bad data

So, keeping this here would mess the event up if the status is deleted as bad data. Plus, we would be able to potentially have the entire status object be sent to the event tracking code. That would be useful for recording what type of EM fields are usually linked, etc.

Good thinking! I changed this in 8c42cb3 to go one step further and wait until after get_custom_status_by() is called, and pass that into the action.

On the receiving side, we now use those meta fields to track the count of required users and required EM fields on custom status changes using the method you suggested above.

ingeniumed · 2024-10-10T23:48:01Z

modules/custom-status/custom-status.php

+		 * @param WP_Term $updated_status The updated status WP_Term object.
+		 * @param array $update_args The arguments used to update the status.
+		 */
+		do_action( 'vw_update_custom_status', $updated_status, $args );


Same thing as add in that, we should provide the entire status imo. That way we can get access to the EM fields linked as well.

See this comment above, did exactly that.

ingeniumed · 2024-10-10T23:50:12Z

modules/notifications/notifications.php

@@ -223,8 +240,8 @@ public static function schedule_emails( string $action, WP_Post $post, string $s
 		 */
 		$message_headers = apply_filters( 'vw_notification_email_headers', $message_headers, $action, $post );

-		if ( ! empty( $email_recipients ) ) {
-			wp_schedule_single_event( time(), 'vw_send_scheduled_emails', [ $email_recipients, $subject, $message, $message_headers ] );
+		if ( ! empty( $recipients ) ) {


We should switch the use of empty for the actual [] check.

Hat tip to Alexey for that.

ingeniumed · 2024-10-10T23:50:35Z

modules/notifications/notifications.php

@@ -274,6 +286,11 @@ public static function schedule_webhook_notification( string $webhook_message, s
 	public static function send_to_webhook( string $message, string $message_type, string $timestamp ): bool {
 		$webhook_url = OptionsUtilities::get_options_by_key( 'webhook_url' );

+		if ( empty( $webhook_url ) ) {


Same thing here.

I know this is code that's just been moved but figured it's good to target it now.

Fixed in b67ae56. This one is a bit tricky because get_options_by_key() has a return type of string|array|bool|null, so we need to be extra explicit here about what type we're expecting.

ingeniumed · 2024-10-11T00:06:32Z

vip-workflow.php

@@ -62,8 +70,16 @@
 require_once VIP_WORKFLOW_ROOT . '/modules/shared/php/util.php';
 require_once VIP_WORKFLOW_ROOT . '/modules/shared/php/core-hacks.php';

-// Modules
+// Modules - Telemetry
+if ( class_exists( '\Automattic\VIP\Telemetry\Tracks' ) ) {


We should do more than this imo. We incorporate the same is_wpvip_site check that we have on the block data api. That way, it would only activate this class, and send telemetry data if its an actual VIP site with a valid ID that we can use.

I didn't implement this comment (yet) because I think is_wpvip_site() is actually a bit more restrictive than necessary. The Block Data API implementation has additional checks that we're not sandboxed and running on actual VIP infrastructure before we log data points. However, mu-plugin's Tracks library works fine on local installs and just tracks that information with the event (vipgo_env).

I think I've found that seeing local datapoints in Tracks is useful for testing, as well as a feature of the mu-plugins library that we don't need to work around. Instead of making our own decisions about the proper environment variables, we're leaving it up to the telemetry library to determine if it's proper to log data, which I think makes more sense here.

ingeniumed · 2024-10-11T00:16:36Z

I meant to say comments instead of approve but hit approve instead. The EM fields tracking is still left to be added in, that should complete this PR

…ount required fields in events

alecgeatches · 2024-10-14T17:56:45Z

@ingeniumed I'm going to merge this PR now and finish the EM telemetry in a separate PR.

hanifn added 7 commits August 21, 2024 22:30

Add tracks classes

13b4378

Add events

453ad23

Fix telemetry namespace

6796d68

Load telemetry module on init

594ca5d

Fix telemetry namespace reference

04d5d96

Fix namespace reference

ba30eed

Require the tracker class

3ca463c

hanifn self-assigned this Aug 21, 2024

hanifn and others added 11 commits August 22, 2024 11:15

Require events files

251c8e8

Add custom status added event

84b0bf8

Add custom status delete event

cf715c2

Add custom status change event

b7a2349

Add hooks into the new actions

b593e45

Add notification events

4622304

Fix phpcs errors

77b64a0

Add admin update event

aee4397

Merge branch 'trunk' into add/tracks

042f399

Fix merge error

f97a6f2

Add settings events

cf6b2de

hanifn marked this pull request as ready for review August 26, 2024 05:34

hanifn requested review from ingeniumed and alecgeatches August 26, 2024 05:34

hanifn added 2 commits August 26, 2024 13:35

Fix phpcs errors

f6d5163

Change event names to fit tracks convention

8c7f9de