-
Notifications
You must be signed in to change notification settings - Fork 1
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 Tracks integration #21
Changes from 46 commits
13b4378
453ad23
6796d68
594ca5d
04d5d96
ba30eed
3ca463c
251c8e8
84b0bf8
cf715c2
b7a2349
b593e45
4622304
77b64a0
aee4397
042f399
f97a6f2
cf6b2de
f6d5163
8c7f9de
1484e37
8259694
2c8ae47
5752761
a2dc1b5
4db9b2b
bd52c78
a14504b
df704fc
3a36954
a0f9415
07fb146
c9ac064
163dade
6a58f5a
6e7fdd2
5a5c762
1937f83
d742496
093419e
9a810f2
9178ddd
951e254
fbfac94
f74e798
e09f6aa
c764c4e
d3d6aa9
8c42cb3
b67ae56
a22064a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'; | ||
const METADATA_REQ_EDITORIAL_IDS_KEY = 'required_metadata_ids'; | ||
const METADATA_REQ_EDITORIALS_KEY = 'required_metadatas'; | ||
const METADATA_REQ_USER_IDS_KEY = 'required_user_ids'; | ||
const METADATA_REQ_USERS_KEY = 'required_users'; | ||
const METADATA_REQ_EDITORIALS_KEY = 'required_metadatas'; | ||
const METADATA_REQ_USER_IDS_KEY = 'required_user_ids'; | ||
const METADATA_REQ_USERS_KEY = 'required_users'; | ||
|
||
private static $custom_statuses_cache = []; | ||
|
||
|
@@ -155,10 +155,10 @@ public static function setup_install(): void { | |
'position' => 4, | ||
], | ||
[ | ||
'name' => __( 'Pending Review' ), | ||
'slug' => 'pending', | ||
'description' => __( 'Post needs to be reviewed by an editor.', 'vip-workflow' ), | ||
'position' => 5, | ||
'name' => __( 'Pending Review' ), | ||
'slug' => 'pending', | ||
'description' => __( 'Post needs to be reviewed by an editor.', 'vip-workflow' ), | ||
'position' => 5, | ||
], | ||
]; | ||
|
||
|
@@ -202,10 +202,10 @@ public static function action_admin_enqueue_scripts(): void { | |
wp_enqueue_style( 'vip-workflow-custom-status-styles', VIP_WORKFLOW_URL . 'dist/modules/custom-status/custom-status-configure.css', [ 'wp-components' ], $asset_file['version'] ); | ||
|
||
wp_localize_script( 'vip-workflow-custom-status-configure', 'VW_CUSTOM_STATUS_CONFIGURE', [ | ||
'custom_statuses' => self::modify_custom_statuses_with_editorial_metadata(), | ||
'custom_statuses' => self::modify_custom_statuses_with_editorial_metadata(), | ||
'editorial_metadatas' => EditorialMetadata::get_editorial_metadata_terms(), | ||
'url_edit_status' => CustomStatusEndpoint::get_crud_url(), | ||
'url_reorder_status' => CustomStatusEndpoint::get_reorder_url(), | ||
'url_edit_status' => CustomStatusEndpoint::get_crud_url(), | ||
'url_reorder_status' => CustomStatusEndpoint::get_reorder_url(), | ||
] ); | ||
} | ||
|
||
|
@@ -265,7 +265,7 @@ private static function modify_custom_statuses_with_editorial_metadata(): array | |
// Add the required editorial metadata to the custom statuses for UI purposes | ||
foreach ( $custom_statuses as $status ) { | ||
$required_metadata_ids = $status->meta[ self::METADATA_REQ_EDITORIAL_IDS_KEY ] ?? []; | ||
$required_metadatas = []; | ||
$required_metadatas = []; | ||
foreach ( $required_metadata_ids as $metadata_id ) { | ||
$required_metadatas[] = $editorial_metadatas[ $metadata_id ]; | ||
} | ||
|
@@ -327,7 +327,7 @@ public static function post_admin_header(): void { | |
$custom_statuses = self::get_custom_statuses(); | ||
|
||
// $selected can be empty, but must be set because it's used as a JS variable | ||
$selected = ''; | ||
$selected = ''; | ||
|
||
if ( ! empty( $post ) ) { | ||
// Get the status of the current post | ||
|
@@ -534,14 +534,23 @@ public static function add_custom_status( array $args ): WP_Term|WP_Error { | |
return $inserted_term; | ||
} | ||
|
||
$term_id = $inserted_term['term_id']; | ||
|
||
/** | ||
* Fires after a custom status is added to the database. | ||
* | ||
* @param int $term_id The ID of the custom status. | ||
* @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'] ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd actually not put this here. This should be on line 571 instead, right before the
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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good thinking! I changed this in 8c42cb3 to go one step further and wait until after 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. |
||
|
||
// Reset our internal object cache | ||
self::$custom_statuses_cache = []; | ||
|
||
$term_id = $inserted_term['term_id']; | ||
|
||
$position = $args[ self::METADATA_POSITION_KEY ]; | ||
$position = $args[ self::METADATA_POSITION_KEY ]; | ||
$required_metadata_ids = $args[ self::METADATA_REQ_EDITORIAL_IDS_KEY ] ?? []; | ||
$required_user_ids = $args[ self::METADATA_REQ_USER_IDS_KEY ] ?? []; | ||
$required_user_ids = $args[ self::METADATA_REQ_USER_IDS_KEY ] ?? []; | ||
|
||
// In case of failure, data cleanup happens which includes the term and the meta keys. | ||
|
||
|
@@ -576,7 +585,7 @@ public static function update_custom_status( int $status_id, array $args = [] ): | |
$old_status = self::get_custom_status_by( 'id', $status_id ); | ||
if ( is_wp_error( $old_status ) ) { | ||
return $old_status; | ||
} else if ( ! $old_status ) { | ||
} elseif ( ! $old_status ) { | ||
return new WP_Error( 'invalid', __( "Custom status doesn't exist.", 'vip-workflow' ) ); | ||
} | ||
|
||
|
@@ -610,8 +619,8 @@ public static function update_custom_status( int $status_id, array $args = [] ): | |
} | ||
|
||
$term_fields_to_update = [ | ||
'name' => isset( $args['name'] ) ? $args['name'] : $old_status->name, | ||
'slug' => isset( $args['slug'] ) ? $args['slug'] : $old_status->slug, | ||
'name' => isset( $args['name'] ) ? $args['name'] : $old_status->name, | ||
'slug' => isset( $args['slug'] ) ? $args['slug'] : $old_status->slug, | ||
'description' => isset( $args['description'] ) ? $args['description'] : $old_status->description, | ||
]; | ||
|
||
|
@@ -647,9 +656,17 @@ public static function update_custom_status( int $status_id, array $args = [] ): | |
return $updated_term; | ||
} | ||
|
||
$status_result = self::get_custom_status_by( 'id', $status_id ); | ||
$updated_status = self::get_custom_status_by( 'id', $status_id ); | ||
|
||
/** | ||
* Fires after a custom status is updated in the database. | ||
* | ||
* @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 ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See this comment above, did exactly that. |
||
|
||
return $status_result; | ||
return $updated_status; | ||
} | ||
|
||
/** | ||
|
@@ -663,7 +680,7 @@ public static function delete_custom_status( int $status_id ): bool|WP_Error { | |
$old_status = self::get_custom_status_by( 'id', $status_id ); | ||
if ( is_wp_error( $old_status ) ) { | ||
return $old_status; | ||
} else if ( ! $old_status ) { | ||
} elseif ( ! $old_status ) { | ||
return new WP_Error( 'invalid', __( "Custom status doesn't exist.", 'vip-workflow' ) ); | ||
} | ||
|
||
|
@@ -700,6 +717,14 @@ public static function delete_custom_status( int $status_id ): bool|WP_Error { | |
return new WP_Error( 'invalid', __( 'Unable to delete custom status.', 'vip-workflow' ) ); | ||
} | ||
|
||
/** | ||
* Fires after a custom status is deleted from the database. | ||
* | ||
* @param int $term_id The ID of the status being deleted | ||
* @param string $old_status_slug The slug of the status being deleted | ||
*/ | ||
do_action( 'vw_delete_custom_status', $status_id, $old_status_slug ); | ||
|
||
// Reset status cache again, as reassign_post_status() will recache prior statuses | ||
self::$custom_statuses_cache = []; | ||
|
||
|
@@ -737,9 +762,9 @@ public static function get_custom_statuses(): array { | |
$statuses = get_terms( [ | ||
'taxonomy' => self::TAXONOMY_KEY, | ||
'hide_empty' => false, | ||
'orderby' => 'meta_value_num', | ||
'order' => 'ASC', | ||
'meta_key' => self::METADATA_POSITION_KEY, | ||
'orderby' => 'meta_value_num', | ||
'order' => 'ASC', | ||
'meta_key' => self::METADATA_POSITION_KEY, | ||
]); | ||
|
||
if ( is_wp_error( $statuses ) || empty( $statuses ) ) { | ||
|
@@ -748,7 +773,7 @@ public static function get_custom_statuses(): array { | |
|
||
// Add metadata to each term | ||
$statuses = array_map( function ( $status ) { | ||
$term_meta = apply_filters( 'vw_register_custom_status_meta', [], $status ); | ||
$term_meta = apply_filters( 'vw_register_custom_status_meta', [], $status ); | ||
$status->meta = $term_meta; | ||
|
||
return $status; | ||
|
@@ -784,7 +809,7 @@ public static function get_custom_status_by( string $field, int|string $value ): | |
if ( is_wp_error( $custom_status ) || ! $custom_status ) { | ||
$custom_status = false; | ||
} else { | ||
$term_meta = apply_filters( 'vw_register_custom_status_meta', [], $custom_status ); | ||
$term_meta = apply_filters( 'vw_register_custom_status_meta', [], $custom_status ); | ||
$custom_status->meta = $term_meta; | ||
} | ||
|
||
|
@@ -806,9 +831,9 @@ public static function get_core_statuses(): array { | |
'description' => __( 'Post is a draft; not ready for review or publication.', 'vip-workflow' ), | ||
], | ||
[ | ||
'name' => __( 'Pending Review' ), | ||
'slug' => 'pending', | ||
'description' => __( 'Post needs to be reviewed by an editor.', 'vip-workflow' ), | ||
'name' => __( 'Pending Review' ), | ||
'slug' => 'pending', | ||
'description' => __( 'Post needs to be reviewed by an editor.', 'vip-workflow' ), | ||
], | ||
]; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,9 +54,9 @@ public static function notification_status_change( string $new_status, string $o | |
|
||
$body = ''; | ||
|
||
$post_id = $post->ID; | ||
$post_title = vw_draft_or_post_title( $post_id ); | ||
$post_type = $post->post_type; | ||
$post_id = $post->ID; | ||
$post_title = vw_draft_or_post_title( $post_id ); | ||
$post_type = $post->post_type; | ||
$subject_post_type = ucfirst( $post_type ); | ||
|
||
if ( 0 != $current_user->ID ) { | ||
|
@@ -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' ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ingeniumed The notification code has been modified to check for email/webhook presence in this upper function. We use this data ( |
||
$is_email_scheduled = '' !== $notification_email; | ||
|
||
/* translators: 1: user name, 2: post type, 3: post id, 4: edit link, 5: post title, 6: old status, 7: new status */ | ||
$webhook_format = __( '*%1$s* changed the status of *%2$s #%3$s - <%4$s|%5$s>* from *%6$s* to *%7$s*', 'vip-workflow' ); | ||
$webhook_message = sprintf( $webhook_format, $current_user_display_name, $post_type, $post_id, $edit_link, $post_title, $old_status, $new_status ); | ||
if ( $is_email_scheduled ) { | ||
$recipients = [ $notification_email ]; | ||
self::schedule_emails( $recipients, $action, $post, $subject, $body ); | ||
} | ||
|
||
$webhook_url = OptionsUtilities::get_options_by_key( 'webhook_url' ); | ||
$is_webhook_scheduled = '' !== $webhook_url; | ||
|
||
if ( $is_webhook_scheduled ) { | ||
/* translators: 1: user name, 2: post type, 3: post id, 4: edit link, 5: post title, 6: old status, 7: new status */ | ||
$webhook_format = __( '*%1$s* changed the status of *%2$s #%3$s - <%4$s|%5$s>* from *%6$s* to *%7$s*', 'vip-workflow' ); | ||
$webhook_message = sprintf( $webhook_format, $current_user_display_name, $post_type, $post_id, $edit_link, $post_title, $old_status, $new_status ); | ||
|
||
self::schedule_webhook_notification( $webhook_message, $action, $post->post_modified_gmt ); | ||
} | ||
|
||
self::schedule_webhook_notification( $webhook_message, $action, $post->post_modified_gmt ); | ||
// Fire the notification status change action if any notifications were scheduled | ||
if ( $is_email_scheduled || $is_webhook_scheduled ) { | ||
/** | ||
* Fires after a notification is sent | ||
* | ||
* @param int $post_id The post ID of the post that was updated. | ||
* @param bool $is_email_scheduled True if an email was scheduled as part of the notification, false otherwise. | ||
* @param bool $is_webhook_scheduled True if a webhook was scheduled as part of the notification, false otherwise. | ||
*/ | ||
do_action( 'vw_notification_status_change', $post->ID, $is_email_scheduled, $is_webhook_scheduled ); | ||
} | ||
} | ||
} | ||
|
||
|
@@ -174,27 +197,21 @@ public static function get_notification_footer(): string { | |
/** | ||
* Send email notifications | ||
* | ||
* @param array $recipients An array of string email addresses to send to | ||
* @param string $action (status-change) | ||
* @param string $subject Subject of the email | ||
* @param string $message Body of the email | ||
* @param string $message_headers. (optional) Message headers | ||
*/ | ||
public static function schedule_emails( string $action, WP_Post $post, string $subject, string $message, string $message_headers = '' ): void { | ||
// Ensure the email address is set from settings. | ||
if ( empty( OptionsUtilities::get_options_by_key( 'email_address' ) ) ) { | ||
return; | ||
} | ||
|
||
$email_recipients = [ OptionsUtilities::get_options_by_key( 'email_address' ) ]; | ||
|
||
public static function schedule_emails( array $recipients, string $action, WP_Post $post, string $subject, string $message, string $message_headers = '' ): void { | ||
/** | ||
* Filter the email recipients | ||
* | ||
* @param array $email_recipients Array of email recipients | ||
* @param array $recipients Array of email recipients | ||
* @param string $action Action being taken, eg. status-change | ||
* @param WP_Post $post Post object | ||
*/ | ||
$email_recipients = apply_filters( 'vw_notification_email_recipients', $email_recipients, $action, $post ); | ||
$recipients = apply_filters( 'vw_notification_email_recipients', $recipients, $action, $post ); | ||
|
||
/** | ||
* Filter the email subject | ||
|
@@ -203,7 +220,7 @@ public static function schedule_emails( string $action, WP_Post $post, string $s | |
* @param string $action Action being taken, eg. status-change | ||
* | ||
*/ | ||
$subject = apply_filters( 'vw_notification_email_subject', $subject, $action, $post ); | ||
$subject = apply_filters( 'vw_notification_email_subject', $subject, $action, $post ); | ||
|
||
/** | ||
* Filter the email message | ||
|
@@ -212,7 +229,7 @@ public static function schedule_emails( string $action, WP_Post $post, string $s | |
* @param string $action Action being taken, eg. status-change | ||
* @param WP_Post $post Post object | ||
*/ | ||
$message = apply_filters( 'vw_notification_email_message', $message, $action, $post ); | ||
$message = apply_filters( 'vw_notification_email_message', $message, $action, $post ); | ||
|
||
/** | ||
* Filter the email headers | ||
|
@@ -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 ) ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should switch the use of Hat tip to Alexey for that. |
||
wp_schedule_single_event( time(), 'vw_send_scheduled_emails', [ $recipients, $subject, $message, $message_headers ] ); | ||
} | ||
} | ||
|
||
|
@@ -248,16 +265,11 @@ public static function send_emails( array $recipients, string $subject, string $ | |
/** | ||
* Schedule a webhook notification | ||
* | ||
* @param string $webhook_message Message to be sent to webhook | ||
* @param string $webhook_url URL to be send the webhook to | ||
* @param string $action Action being taken, eg. status-change | ||
* @param string $timestamp Timestamp of the message, eg. the time at which the post was updated | ||
*/ | ||
public static function schedule_webhook_notification( string $webhook_message, string $action, string $timestamp ): void { | ||
// Ensure the webhook URL is set from settings. | ||
if ( empty( OptionsUtilities::get_options_by_key( 'webhook_url' ) ) ) { | ||
return; | ||
} | ||
|
||
$message_type = 'plugin:vip-workflow:' . $action; | ||
|
||
wp_schedule_single_event( time(), 'vw_send_scheduled_webhook', [ $webhook_message, $message_type, $timestamp ] ); | ||
|
@@ -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 ) ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same thing here. I know this is code that's just been moved but figured it's good to target it now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in b67ae56. This one is a bit tricky because |
||
// This can happen if the webhook URL was cleared after scheduling this notification | ||
return false; | ||
} | ||
|
||
// Set up the payload | ||
$payload = [ | ||
'type' => $message_type, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
From running
./vendor/bin/phpcs --help
, we can see the-n
parameter is doing this: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
:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?