Skip to content

MYJP-268 Update My Jetpack red bubble notifications #46396

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.

Sign up for GitHub

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

Open
grzegorz-cp wants to merge 10 commits into
base: trunk
Choose a base branch
from
+140 −7
Open

MYJP-268 Update My Jetpack red bubble notifications #46396

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8ba66ea
SOCIAL-268 Prototype async My Jetpack red bubble
Dec 23, 2025
692b471
changelog
Dec 23, 2025
a5bc52f
Merge branch 'trunk' into SOCIAL-268-bubble-async
Jan 7, 2026
bdb63ad
Replacing inline script
Jan 7, 2026
09b6907
Merge branch 'trunk' into SOCIAL-268-bubble-async
Jan 8, 2026
1e690ee
Split logic to accomodate My Jetpack data fetching and window object …
Jan 8, 2026
d30696e
Replacing incorrect ticket number
Jan 8, 2026
4a9f43b
Replacing incorrect ticket number
Jan 8, 2026
71f1af4
Code comment suggestions
grzegorz-cp Jan 8, 2026
7e0455b
Better cache check
grzegorz-cp Jan 8, 2026
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
46 changes: 46 additions & 0 deletions projects/packages/my-jetpack/_inc/utils/async-notification-bubble.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/**
* My Jetpack Notification Bubble async loader.
* Fetches fresh alert data via REST API without blocking page load.
*/
import apiFetch from '@wordpress/api-fetch';

// Minimal type for counting non-silent alerts.
type Alert = {
is_silent?: boolean;
};

type AlertsResponse = Record< string, Alert >;

apiFetch< AlertsResponse >( {
path: 'my-jetpack/v1/red-bubble-notifications',
method: 'POST',
} )
.then( alerts => {
const count = Object.values( alerts ).filter( a => ! a.is_silent ).length;
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

If the API returns null, undefined, or a non-object value, calling Object.values() will throw a runtime error. While apiFetch with proper typing should prevent this, consider adding a defensive check to ensure alerts is a valid object before processing it.

Suggested change
const count = Object.values( alerts ).filter( a => ! a.is_silent ).length;
const alertValues = alerts && typeof alerts === 'object' ? Object.values( alerts ) : [];
const count = alertValues.filter( a => ! a.is_silent ).length;

Copilot uses AI. Check for mistakes.
const menuItem = document.querySelector( '#toplevel_page_jetpack .wp-menu-name' );

if ( ! menuItem ) {
return;
}

const bubble = menuItem.querySelector( '.awaiting-mod' );

if ( count > 0 ) {
if ( bubble ) {
bubble.className = 'awaiting-mod';
bubble.textContent = String( count );
} else {
const span = document.createElement( 'span' );
span.className = 'awaiting-mod';
span.textContent = String( count );
menuItem.appendChild( document.createTextNode( ' ' ) );
menuItem.appendChild( span );
}
} else if ( bubble ) {
bubble.remove();
}
} )
.catch( ( error: Error ) => {
// eslint-disable-next-line no-console
console.error( '[My Jetpack] Failed to fetch notification alerts:', error );
Copy link
Contributor Author

@grzegorz-cp grzegorz-cp Jan 8, 2026

Choose a reason for hiding this comment

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

I'm not sure if this is bringing any value.

Copy link
Contributor

@robertsreberski robertsreberski Jan 9, 2026

Choose a reason for hiding this comment

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

we should for sure not implement console.error; one thing I'm thinking about is callback to the backend my-jetpack/v1/red-bubble-notifications-error with error message payload, and using Tracks error event from there. If the callback would fail for some reason - it is okay to fail silently.

What do you think about it?

} );
Comment on lines +1 to +46
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The new async-notification-bubble.ts utility lacks test coverage. Consider adding unit tests to verify the DOM manipulation logic (bubble creation, update, and removal) and the error handling for API failures. This is particularly important since this code runs on every admin page when the cache is empty.

Copilot uses AI. Check for mistakes.
4 changes: 4 additions & 0 deletions projects/packages/my-jetpack/changelog/MYJP-268-bubble-async
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

My Jetpack: Check red bubble notification async when cache is not available.
53 changes: 46 additions & 7 deletions projects/packages/my-jetpack/src/class-initializer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -688,12 +688,15 @@
}

/**
* Conditionally append the red bubble notification to the "Jetpack" menu item if there are alerts to show
* Conditionally append the red bubble notification to the "Jetpack" menu item if there are alerts to show.
*
* On My Jetpack page: Uses blocking behavior to fetch fresh data.
* On other admin pages: Uses cached data only to avoid blocking, with async JS fetch if cache is empty.
*
* @return void
*/
public static function maybe_show_red_bubble() {
global $menu;
global $menu, $pagenow;

// Don't show red bubble alerts for non-admin users
// These alerts are generally only actionable for admins
Expand All @@ -708,14 +711,32 @@
return;
}

$rbn = new Red_Bubble_Notifications();
// Check if we're on the My Jetpack page
Copy link
Member

@manzoorwanijk manzoorwanijk Jan 8, 2026

Choose a reason for hiding this comment

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

Let us avoid adding PHPCS errors. This file already has too many of them.

Suggested change
// Check if we're on the My Jetpack page
// Check if we're on the My Jetpack page.

// phpcs:ignore WordPress.Security.NonceVerification.Recommended
$page = isset( $_GET['page'] ) ? sanitize_text_field( wp_unslash( $_GET['page'] ) ) : '';
$is_my_jetpack_page = $pagenow === 'admin.php' && $page === 'my-jetpack';

if ( $is_my_jetpack_page ) {
// On My Jetpack page: use blocking behavior for fresh data.
add_filter( 'my_jetpack_red_bubble_notification_slugs', array( Red_Bubble_Notifications::class, 'add_red_bubble_alerts' ) );
$red_bubble_alerts = Red_Bubble_Notifications::get_red_bubble_alerts();
} else {
// On other pages: use cached data only to avoid blocking.
$cached_alerts = Red_Bubble_Notifications::get_cached_alerts();

if ( false === $cached_alerts ) {

Check failure on line 727 in projects/packages/my-jetpack/src/class-initializer.php

View workflow job for this annotation

GitHub Actions / Static analysis 

TypeError PhanTypeComparisonToArray false to array comparison

FAQ on Phan issues: pdWQjU-Jb-p2
// No cache - fetch asynchronously via JS.
add_action( 'admin_enqueue_scripts', array( __CLASS__, 'enqueue_red_bubble_script' ) );
return;
}

$red_bubble_alerts = $cached_alerts;
}

// filters for the items in this file
add_filter( 'my_jetpack_red_bubble_notification_slugs', array( $rbn, 'add_red_bubble_alerts' ) );
// Filter out silent alerts
Copy link
Member

@manzoorwanijk manzoorwanijk Jan 8, 2026

Choose a reason for hiding this comment

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

Suggested change
// Filter out silent alerts
// Filter out silent alerts.

$red_bubble_alerts = array_filter(
$rbn::get_red_bubble_alerts(),
$red_bubble_alerts,
function ( $alert ) {
// We don't want to show the red bubble for silent alerts
return empty( $alert['is_silent'] );
}
);
Expand All @@ -731,6 +752,24 @@
}
}

/**
* Enqueue the notification bubble script.
* Fetches fresh alert data via REST API without blocking page load.
*
* @return void
*/
public static function enqueue_red_bubble_script() {
Assets::register_script(
'my_jetpack_notification_bubble',
Copy link
Member

@manzoorwanijk manzoorwanijk Jan 8, 2026

Choose a reason for hiding this comment

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

Handles are usually with a hyphen instead of underscore.

Suggested change
'my_jetpack_notification_bubble',
'my-jetpack-notification-bubble',

Copy link
Contributor Author

@grzegorz-cp grzegorz-cp Jan 8, 2026

Choose a reason for hiding this comment

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

I haven't seen a string with mixed hyphens and underscores in the codebase, but there are places where everything has underscores, even if it contains "my_jetpack", so I think it's better to keep the proposed value.

Copy link
Member

@manzoorwanijk manzoorwanijk Jan 9, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

You can search for Assets::register_script in the codebase to see how script handles are used.

'../build/async-notification-bubble.js',
__FILE__,
array(
'enqueue' => true,
'in_footer' => true,
)
);
}

/**
* Get list of module names sorted by their recommendation score
*
Expand Down
11 changes: 11 additions & 0 deletions projects/packages/my-jetpack/src/class-red-bubble-notifications.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,17 @@ public static function add_red_bubble_alerts( array $red_bubble_slugs ) {
}
}

/**
* Get cached red bubble alerts without triggering expensive computation.
* Returns the cached transient value or an empty array if not cached.
*
* @return array Cached alerts or empty array.
*/
public static function get_cached_alerts() {
$stored_alerts = get_transient( self::MY_JETPACK_RED_BUBBLE_TRANSIENT_KEY );
return $stored_alerts !== false ? $stored_alerts : array();
Comment on lines +373 to +379
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The get_cached_alerts() method returns an empty array when there's no cache, making it impossible to distinguish between "cache doesn't exist" and "cache exists but contains no alerts". This causes an unnecessary async script load when the cache legitimately contains an empty array (no alerts to show). The check if ( empty( $cached_alerts ) ) will trigger an async fetch in both cases. Consider modifying get_cached_alerts() to return false when the cache doesn't exist, preserving the ability to distinguish these two states.

Suggested change
* Returns the cached transient value or an empty array if not cached.
*
* @return array Cached alerts or empty array.
*/
public static function get_cached_alerts() {
$stored_alerts = get_transient( self::MY_JETPACK_RED_BUBBLE_TRANSIENT_KEY );
return $stored_alerts !== false ? $stored_alerts : array();
* Returns the cached transient value or false if not cached.
*
* @return array|false Cached alerts array or false if no cache exists.
*/
public static function get_cached_alerts() {
return get_transient( self::MY_JETPACK_RED_BUBBLE_TRANSIENT_KEY );

Copilot uses AI. Check for mistakes.
}
Comment on lines +371 to +380
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The new get_cached_alerts() method lacks test coverage. Consider adding unit tests to verify it correctly returns cached values when available and an empty array when the cache doesn't exist.

Copilot uses AI. Check for mistakes.

/**
* Collect all possible alerts that we might use a red bubble notification for
*
Expand Down
1 change: 1 addition & 0 deletions projects/packages/my-jetpack/webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ module.exports = [
{
entry: {
index: './_inc/admin.jsx',
'async-notification-bubble': './_inc/utils/async-notification-bubble.ts',
},
mode: jetpackWebpackConfig.mode,
devtool: jetpackWebpackConfig.devtool,
Expand Down
4 changes: 4 additions & 0 deletions projects/plugins/backup/changelog/MYJP-268-bubble-async
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

My Jetpack: Check red bubble notification async when cache is not available.
4 changes: 4 additions & 0 deletions projects/plugins/boost/changelog/MYJP-268-bubble-async
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

My Jetpack: Check red bubble notification async when cache is not available.
4 changes: 4 additions & 0 deletions projects/plugins/jetpack/changelog/MYJP-268-bubble-async
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: enhancement

My Jetpack: Check red bubble notification async when cache is not available.
4 changes: 4 additions & 0 deletions projects/plugins/protect/changelog/MYJP-268-bubble-async
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

My Jetpack: Check red bubble notification async when cache is not available.
4 changes: 4 additions & 0 deletions projects/plugins/search/changelog/MYJP-268-bubble-async
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

My Jetpack: Check red bubble notification async when cache is not available.
4 changes: 4 additions & 0 deletions projects/plugins/social/changelog/MYJP-268-bubble-async
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

My Jetpack: Check red bubble notification async when cache is not available.
4 changes: 4 additions & 0 deletions projects/plugins/starter-plugin/changelog/MYJP-268-bubble-async
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

My Jetpack: Check red bubble notification async when cache is not available.
4 changes: 4 additions & 0 deletions projects/plugins/videopress/changelog/MYJP-268-bubble-async
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

My Jetpack: Check red bubble notification async when cache is not available.
Loading