Skip to content

Blaze: ensure the Dashboard menu is only inserted once. #31617

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

Merged
merged 7 commits into from
Jul 3, 2023
Merged

Blaze: ensure the Dashboard menu is only inserted once. #31617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
70a4596
Blaze: ensure the Dashboard menu is only inserted once.
jeherve Jun 28, 2023
6e4d9c7
Remove nav-unification handling, let the package register things
jeherve Jun 28, 2023
1c4cc63
Always prefer calypos view in nav unification
jeherve Jun 28, 2023
64ce134
Remove Blaze from test now that menu is handled in package
jeherve Jun 28, 2023
bd8045c
Move more of the logic into the package to simplify things
jeherve Jun 28, 2023
ff950a9
Force the dashboard off on WordPress.com sites.
jeherve Jun 29, 2023
bcd0337
Reverse menu registration logic to prioritize new dash registration
jeherve Jun 29, 2023
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
4 changes: 4 additions & 0 deletions projects/packages/blaze/changelog/update-blaze-menu-masterbar
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

Dashboard Menu: change priority.
42 changes: 33 additions & 9 deletions projects/packages/blaze/src/class-blaze.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use Automattic\Jetpack\Connection\Initial_State as Connection_Initial_State;
use Automattic\Jetpack\Connection\Manager as Jetpack_Connection;
use Automattic\Jetpack\Status as Jetpack_Status;
use Automattic\Jetpack\Status\Host;
use Automattic\Jetpack\Sync\Settings as Sync_Settings;

/**
Expand Down Expand Up @@ -67,18 +68,27 @@ public static function add_post_links_actions() {

/**
* Is the wp-admin Dashboard enabled?
* That dashboard is not available or necessary on WordPress.com sites.
*
* @return bool
*/
public static function is_dashboard_enabled() {
// Right now the dashboard is disabled by default.
$is_dashboard_enabled = false;

// On WordPress.com sites, the dashboard is not needed.
if ( ( new Host() )->is_wpcom_platform() ) {
$is_dashboard_enabled = false;
}

/**
* Enable a wp-admin dashboard for Blaze campaign management.
*
* @since 0.7.0
*
* @param bool $should_enable Should the dashboard be enabled? False by default for now.
* @param bool $should_enable Should the dashboard be enabled?
*/
return apply_filters( 'jetpack_blaze_dashboard_enable', false );
return apply_filters( 'jetpack_blaze_dashboard_enable', $is_dashboard_enabled );
}

/**
Expand All @@ -87,19 +97,33 @@ public static function is_dashboard_enabled() {
* @return void
*/
public static function enable_blaze_menu() {
if (
self::should_initialize()
&& self::is_dashboard_enabled()
) {
$blaze_dashboard = new Blaze_Dashboard();
$page_suffix = add_submenu_page(
if ( ! self::should_initialize() ) {
return;
}

$blaze_dashboard = new Blaze_Dashboard();

if ( self::is_dashboard_enabled() ) {
$page_suffix = add_submenu_page(
'tools.php',
esc_attr__( 'Advertising', 'jetpack-blaze' ),
__( 'Advertising', 'jetpack-blaze' ),
'manage_options',
'advertising',
array( $blaze_dashboard, 'render' ),
100
1
);
add_action( 'load-' . $page_suffix, array( $blaze_dashboard, 'admin_init' ) );
} elseif ( ( new Host() )->is_wpcom_platform() ) {
$domain = ( new Jetpack_Status() )->get_site_suffix();
$page_suffix = add_submenu_page(
'tools.php',
esc_attr__( 'Advertising', 'jetpack-blaze' ),
__( 'Advertising', 'jetpack-blaze' ),
'manage_options',
'https://wordpress.com/advertising/' . $domain,
null,
1
);
add_action( 'load-' . $page_suffix, array( $blaze_dashboard, 'admin_init' ) );
}
Expand Down
4 changes: 4 additions & 0 deletions projects/plugins/jetpack/changelog/update-blaze-menu-masterbar
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: other

WordPress.com Toolbar: only add Blaze Dashboard link once.
5 changes: 0 additions & 5 deletions projects/plugins/jetpack/modules/masterbar/admin-menu/class-atomic-admin-menu.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

namespace Automattic\Jetpack\Dashboard_Customizations;

use Automattic\Jetpack\Blaze;
use Automattic\Jetpack\Connection\Client;
use Automattic\Jetpack\Current_Plan as Jetpack_Plan;
use Automattic\Jetpack\Status;
Expand Down Expand Up @@ -381,10 +380,6 @@ public function add_options_menu() {
// would conflict with our own Settings > Performance that links to Calypso, so we hide it it since the Calypso
// performance settings already have a link to Page Optimize settings page.
$this->hide_submenu_page( 'options-general.php', 'page-optimize' );

if ( Blaze::should_initialize() ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I checked and this code is now inside the Blaze package. This means that the Advertising menu will appear only if the Blaze module is enabled. Right?

I think it is already like this right now, you are just moving this code to the blaze package.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's correct, yes.

add_submenu_page( 'tools.php', esc_attr__( 'Advertising', 'jetpack' ), __( 'Advertising', 'jetpack' ), 'manage_options', 'https://wordpress.com/advertising/' . $this->domain, null, 1 );
}
}

/**
Expand Down
5 changes: 1 addition & 4 deletions projects/plugins/jetpack/modules/masterbar/admin-menu/class-jetpack-admin-menu.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

namespace Automattic\Jetpack\Dashboard_Customizations;

use Automattic\Jetpack\Blaze;
use Automattic\Jetpack\Current_Plan as Jetpack_Plan;

require_once __DIR__ . '/class-admin-menu.php';
Expand Down Expand Up @@ -244,9 +243,7 @@ public function add_users_menu() {
*/
public function add_tools_menu() {
add_menu_page( esc_attr__( 'Tools', 'jetpack' ), __( 'Tools', 'jetpack' ), 'publish_posts', 'tools.php', null, 'dashicons-admin-tools', 75 );
if ( Blaze::should_initialize() ) {
add_submenu_page( 'tools.php', esc_attr__( 'Advertising', 'jetpack' ), __( 'Advertising', 'jetpack' ), 'manage_options', 'https://wordpress.com/advertising/' . $this->domain, null, 1 );
}

add_submenu_page( 'tools.php', esc_attr__( 'Marketing', 'jetpack' ), __( 'Marketing', 'jetpack' ), 'publish_posts', 'https://wordpress.com/marketing/tools/' . $this->domain );
add_submenu_page( 'tools.php', esc_attr__( 'Earn', 'jetpack' ), __( 'Earn', 'jetpack' ), 'manage_options', 'https://wordpress.com/earn/' . $this->domain );

Expand Down
4 changes: 0 additions & 4 deletions projects/plugins/jetpack/modules/masterbar/admin-menu/class-wpcom-admin-menu.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

namespace Automattic\Jetpack\Dashboard_Customizations;

use Automattic\Jetpack\Blaze;
use Automattic\Jetpack\Status;
use Jetpack_Custom_CSS;
use JITM;
Expand Down Expand Up @@ -367,9 +366,6 @@ public function add_users_menu() {
public function add_options_menu() {
parent::add_options_menu();

if ( Blaze::should_initialize() ) {
add_submenu_page( 'tools.php', esc_attr__( 'Advertising', 'jetpack' ), __( 'Advertising', 'jetpack' ), 'manage_options', 'https://wordpress.com/advertising/' . $this->domain, null, 1 );
}
add_submenu_page( 'options-general.php', esc_attr__( 'Hosting Configuration', 'jetpack' ), __( 'Hosting Configuration', 'jetpack' ), 'manage_options', 'https://wordpress.com/hosting-config/' . $this->domain, null, 10 );
}

Expand Down
3 changes: 0 additions & 3 deletions projects/plugins/jetpack/tests/php/modules/masterbar/test-class-jetpack-admin-menu.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,6 @@ public function test_add_jetpack_menu_not_english() {
public function test_add_tools_menu() {
global $submenu;

// Enable blaze
add_filter( 'jetpack_blaze_enabled', '__return_true' );
static::$admin_menu->add_tools_menu();

// Check Import/Export menu always links to WP Admin.
Expand All @@ -136,7 +134,6 @@ public function test_add_tools_menu() {

$this->assertSame( 'https://wordpress.com/earn/' . static::$domain, array_pop( $submenu['tools.php'] )[2] );
$this->assertSame( 'https://wordpress.com/marketing/tools/' . static::$domain, array_pop( $submenu['tools.php'] )[2] );
$this->assertSame( 'https://wordpress.com/advertising/' . static::$domain, array_pop( $submenu['tools.php'] )[2] );
}

/**
Expand Down