Podcast: write Activity Log entries on podcast publish#48693
Closed
robertbpugh wants to merge 8 commits into
Closed
Podcast: write Activity Log entries on podcast publish#48693robertbpugh wants to merge 8 commits into
robertbpugh wants to merge 8 commits into
Conversation
Adds a `podcast_show_launched` Jetpack Sync Activity Log entry inside the existing first-episode atomic guard in `Tracks::record_episode_published`. The entry syncs to WPcom as a `jp_act_log_event` post; a downstream wpcom listener hooks `source='podcast_show_launched'` to fire the `#podcast-alerts` Slack ping that the legacy mu-plugin path stopped emitting under the podcast untangle rollout. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
github-actions
Bot
added
[Package] Podcast
[Plugin] Jetpack
Contributor
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! Jetpack plugin: The Jetpack plugin has different release cadences depending on the platform:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Mu Wpcom plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Wpcomsh plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
github-actions
Bot
added
the
[Status] Needs Author Reply
Code Coverage SummaryThis PR did not change code coverage! That could be good or bad, depending on the situation. Everything covered before, and still is? Great! Nothing was covered before? Not so great. 🤷 |
- Jetpack plugin changelog used Type: changed which is not in its allowed set (major/enhancement/compat/bugfix/other). Switch to "other". - Adding jetpack-sync as a podcast dep transitively pulls in jetpack-connection, so phan now sees Tracking::tracks_record_event and flags the @phan-suppress-next-line as unused. Remove it.
Adds a `podcast_episode_published` Activity Log entry on every podcast episode publish (alongside the existing first-episode-only `podcast_show_launched`). Per Mikael's suggestion on #48693, gives WPcom downstream listeners a per-episode hook in the same payload shape as the launch entry. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
robertbpugh
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds Jetpack Sync Activity Log entries to the Jetpack Podcast package’s publish instrumentation so downstream WordPress.com listeners can react to podcast launches and episode publishes without directly calling WPcom-only helpers.
Changes:
- Create a
podcast_episode_publishedActivity Log event on every eligible episode publish. - Create a once-per-site
podcast_show_launchedActivity Log event on the first eligible episode publish (behind the existing atomicadd_optionguard). - Add
automattic/jetpack-syncas a dependency of the podcast package and refresh lockfiles/changelog entries in affected plugins.
Reviewed changes
Copilot reviewed 6 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| projects/packages/podcast/src/class-tracks.php | Adds Activity Log event creation alongside existing Tracks events. |
| projects/packages/podcast/composer.json | Adds automattic/jetpack-sync as a required dependency. |
| projects/packages/podcast/changelog/add-podcast-show-launched-activity-log | Changelog entry describing the new Activity Log events. |
| projects/plugins/jetpack/composer.lock | Updates podcast package path reference and records new transitive dependency (jetpack-sync). |
| projects/plugins/jetpack/changelog/add-podcast-show-launched-activity-log | Changelog entry for lockfile/dependency update. |
| projects/plugins/mu-wpcom-plugin/composer.lock | Updates podcast package path reference and records new transitive dependency (jetpack-sync). |
| projects/plugins/mu-wpcom-plugin/changelog/add-podcast-show-launched-activity-log | Changelog entry for lockfile/dependency update. |
| projects/plugins/wpcomsh/composer.lock | Updates podcast package path reference and records new transitive dependency (jetpack-sync). |
| projects/plugins/wpcomsh/changelog/add-podcast-show-launched-activity-log | Changelog entry for lockfile/dependency update. |
| return tracks_record_event( $user, $event_name, $properties ); | ||
| } | ||
|
|
||
| if ( class_exists( '\Automattic\Jetpack\Tracking' ) ) { |
Contributor
Author
There was a problem hiding this comment.
Adding automattic/jetpack-sync as a podcast package dep transitively pulls in automattic/jetpack-connection (Sync requires it), so \Automattic\Jetpack\Tracking resolves for Phan from this package now. Restoring the @phan-suppress-next-line PhanUndeclaredClassMethod would itself be flagged as an unused suppression. Leaving it removed is correct in the new dep state.
Comment on lines
113
to
117
| self::record_episode_published_activity( $post ); | ||
|
|
||
| // Atomic INSERT — only one concurrent caller per site wins, so | ||
| // `show_launched` fires exactly once per site. | ||
| if ( $is_first && add_option( 'podcast_show_launched_tracked', time(), '', false ) ) { |
| Significance: patch | ||
| Type: changed | ||
|
|
||
| Updated package dependencies. |
| Significance: patch | ||
| Type: changed | ||
|
|
||
| Updated package dependencies. |
| Significance: patch | ||
| Type: other | ||
|
|
||
| Updated package dependencies. |
Contributor
Author
|
@copilot apply changes based on the comments in this thread |
Extends Activity_Log_Event::create() with an optional `extra` array of scalar key/value pairs, sanitized and persisted alongside the human-readable content. Caps at 20 entries, 64-char keys, 2000-char values. Drops non-scalar values, non-string keys, and entries past the cap. Wires the podcast package's `podcast_show_launched` and `podcast_episode_published` Activity Log entries to include `post_id`, `post_url`, and raw `post_title` in `extra`, so the WPcom-side listener can consume the structured data without parsing translated content strings. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Add decode_activity_log_payload() helper with wp_unslash() fallback to handle WorDBless slashed post_content (same pattern as Activity_Log_Event::decode_payload() in the sync package) - Update activity_log_posts_with_source() to use the new helper - Update test assertions to use decode_activity_log_payload() - Fix 3 plugin changelog entries: "Updated" → "Update" (imperative mood) Agent-Logs-Url: https://github.com/Automattic/jetpack/sessions/2498c408-1ebb-48d4-9f6c-9664a26efcb5 Co-authored-by: robertbpugh <52668747+robertbpugh@users.noreply.github.com>
Contributor
Applied all actionable review changes in commit Tests (Activity Log side effects): Added Changelog entries: Fixed all three plugin changelog files to use imperative mood — "Update package dependencies." (not "Updated …"). Phan suppression: Left removed. The Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
fgiannar
requested changes
May 11, 2026
Contributor
fgiannar
left a comment
fgiannar
left a comment
There was a problem hiding this comment.
Hey @robertbpugh, thanks for the PR! I think you can accomplish both use cases without Activity_Log_Event or the jp_act_log_event CPT, which would avoid adding automattic/jetpack-sync as a dependency of the podcast package and avoid writing CPT posts to the customer's DB for what are essentially internal notifications.
For podcast_show_launched:
The podcast_show_launched_tracked option is already created via add_option(). If you add it to Sync's options whitelist (also possible via a filter), Sync will automatically transport it to WPcom when it's set. On the WPcom side, you can hook the jetpack_sync_remote_action for the added_option event to trigger the Slack notification. The atomic once-per-site guard you already have stays exactly the same.
For podcast_episode_published:
Sync already fires jetpack_published_post for every post publish, with the full post object and flags including post_type. The WPcom side already receives this data. A WPcom-side listener can check whether the published post is a podcast episode (by checking the podcast category or however the podcast package identifies episodes) and trigger the notification from there. No additional Jetpack-side code needed.
This approach:
- Removes the need for automattic/jetpack-sync as a podcast package dependency
- No CPT posts written to the customer's DB
- No extra field addition to Activity_Log_Event
- No changes to the Sync package at all (aside from the one-line options whitelist addition)
- The WPcom listener logic is roughly the same either way — it just hooks option sync / post sync events instead of jp_act_log_event
posts
The one caveat: the options whitelist addition needs a corresponding WPcom-side whitelist entry for the replicastore to store it. But that's a standard pattern for Sync options.
What do you think?
Contributor
Author
|
Thanks for taking a look @fgiannar! But we also want these to write customer-visible Activity Log entries, I rewrote the description a little to make that clearer. From what I can tell your approach gets us the Slack trigger but doesn't land anything in the customer's Activity Log UI. Am I missing something? |
Contributor
The Activity Log is in general powered by Jetpack Sync. As various Sync actions are flowing from the remote sites, we listen for the ones we're interested in and store them in a corresponding ES index that powers the Activity Log. The rest logic needs to be implemented on WPCOM, in order to extend the Activity Log to display a corresponding activity, when the option is added and when a podcast is published - I can provide further details on Slack on how to implement this if needed. |
Member
|
Seems like it would just be simpler to listen to syncing of episodes and podcast settings and add activity log things on the server side in traditional way without using this helper method? Would be mich less code, no new post type, etc. |
Per @fgiannar and @simison feedback (#48693 review): drop the `Activity_Log_Event`-based approach entirely. Instead, whitelist `podcast_show_launched_tracked` for Jetpack Sync via `jetpack_sync_options_whitelist`. The `added_option` event reaches WPcom, where a downstream listener will surface the launch in the customer's Activity Log UI. Episode publishes already flow as `jetpack_published_post` sync actions, so no extra package-side code is needed. - Drop `automattic/jetpack-sync` require from the podcast package. - Drop the two Activity-Log helper methods and their call sites. - Revert the `extra` payload addition to `Activity_Log_Event::create()` in the sync package (only existed to support the now-removed approach). - Refresh path-repo lockfiles (revert cleanly to trunk state). - Drop the three plugin lockfile-bump changelogs. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two MINOR fixes from the codex review pass: - Add a hook-level test that calls `Tracks::init()` and asserts `podcast_show_launched_tracked` is in the filtered `jetpack_sync_options_whitelist`. Catches an accidental `add_filter` deletion in `init()`. - Capitalize "Whitelist" in the changelog entry to match the sync package precedent (e.g. "Sync: Whitelist the jetpack_social_message_template option..."). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
Author
|
@copilot apply changes based on the comments in this thread |
Contributor
Author
Member
|
Do we necessarily need |
Contributor
The pivot to the Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Contributor
Author
|
Thanks @simison, you're right. Wpcom already has the synced post and |
github-actions
Bot
removed
[Status] Needs Author Reply
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes # (no Linear yet. Tony flagged in #podcast-tools.)
Proposed changes
Surface the first podcast publish in the customer's Activity Log UI by whitelisting
podcast_show_launched_trackedfor Jetpack Sync. This replaces the originalActivity_Log_Event-based approach per @fgiannar's review feedback.jetpack_sync_options_whitelistfilter in the podcast package'sTracksclass that appendspodcast_show_launched_trackedto the synced options list.add_option( 'podcast_show_launched_tracked', time(), '', false )guard insiderecord_episode_published(). Sync now propagates that write to WPcom as anadded_optionaction, where a server-side listener (separate sandbox PR) translates it into a customer-visible Activity Log entry.jetpack_published_postsync actions, so no extra package-side code is needed.Why
Primary goal: site owners see "Podcast show launched" in their site's Activity Log UI alongside other events. Reviewer guidance was to avoid the new
Activity_Log_Event+jp_act_log_eventCPT for this case and instead let WPcom's existing sync infrastructure carry the signal.Secondary: a WPcom-side listener will hook the
added_optionevent to restore the#podcast-alertsSlack notification that regressed when the legacy wpcom mu-plugin'sa8c_slack()call was untangled into the Jetpack package. That listener lives in a separate sandbox PR.This code path is dormant in production until
jetpack_podcast_untangleflips on for a site.Related product discussion/links
Does this pull request change what data or activity we track or use?
No new tracks events. No new Activity Log entry types are written from the package itself. This PR only whitelists an existing option for sync. The customer-visible Activity Log entry is produced server-side on WPcom by a downstream listener (separate PR).
Testing instructions
This path is dormant until
jetpack_podcast_untanglereturns true.add_filter( 'jetpack_podcast_untangle', '__return_true' );.delete_option( 'podcast_show_launched_tracked' );.wp option get podcast_show_launched_trackedreturns a timestamp.var_export( apply_filters( 'jetpack_sync_options_whitelist', array() ) )includespodcast_show_launched_tracked.