Skip to content
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

Remove React 17 requirement for AfterActivationSiteScan service #7490

Merged
merged 1 commit into from
Mar 16, 2023
Merged

Remove React 17 requirement for AfterActivationSiteScan service #7490

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 0 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ node_modules

# Generated via bin/transform-readme.php
/readme.txt
/tests/e2e/plugins/gutenberg.14.7.3.zip

# Generated via phpstan analyse --generate-baseline temp-baseline.php
/temp-baseline.php
3 changes: 0 additions & 3 deletions bin/local-env/start.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@ cd "$(dirname "$0")/../.."
# done on every time `npm run test-e2e` is run.
. "$(dirname "$0")/install-wordpress.sh"

# Install specific version of Gutenberg plugin for e2e tests.
download_gutenberg "14.7.3" "tests/e2e/plugins/gutenberg.14.7.3.zip"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just in case we ever need it, should we leave the download_gutenberg util function in place?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same for uploads.ini

amp-wp/bin/local-env/docker-compose.yml

Line 13 in 96cdfb9

- ./uploads.ini:/usr/local/etc/php/conf.d/uploads.ini

Copy link
Member

Choose a reason for hiding this comment

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

We could, but probably better to remove it. It can be resurrected via version control if needed.


! read -d '' AMP <<"EOT"
MMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMM
MMMMMMMMMMMMMMMMWNXK0OOkkkkOO0KXNWMMMMMMMMMMMMMMMM
Expand Down
12 changes: 0 additions & 12 deletions src/Admin/AfterActivationSiteScan.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

namespace AmpProject\AmpWP\Admin;

use _WP_Dependency;
use AMP_Options_Manager;
use AMP_Validation_Manager;
use AmpProject\AmpWP\Infrastructure\Conditional;
Expand Down Expand Up @@ -42,13 +41,6 @@ final class AfterActivationSiteScan implements Conditional, Delayed, HasRequirem
*/
const APP_ROOT_ID = 'amp-site-scan-notice';

/**
* React dependency handle.
*
* @var string
*/
const REACT = 'react';

/**
* HTML ID for the app root sibling element.
*
Expand Down Expand Up @@ -92,17 +84,13 @@ public function __construct( RESTPreloader $rest_preloader ) {
public static function is_needed() {
global $pagenow;

$react = wp_scripts()->query( self::REACT );

return (
is_admin()
&&
Services::get( 'dependency_support' )->has_support()
&&
! is_network_admin()
&&
( $react instanceof _WP_Dependency && version_compare( $react->ver, '18', '<' ) )
&&
AMP_Validation_Manager::has_cap()
&&
(
Expand Down
20 changes: 0 additions & 20 deletions tests/e2e/specs/admin/after-plugin-activation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,6 @@ import { visitAdminPage } from '@wordpress/e2e-test-utils';
* Internal dependencies
*/
import { completeWizard } from '../../utils/onboarding-wizard-utils';
import {
installPlugin,
installLocalPlugin,
uninstallPlugin,
} from '../../utils/amp-settings-utils';

describe('After plugin activation', () => {
const timeout = 30000;
Expand All @@ -27,23 +22,8 @@ describe('After plugin activation', () => {
}

beforeAll(async () => {
// Uninstall the latest Gutenberg plugin.
await uninstallPlugin('gutenberg');

// Install the Gutenberg 14.7.3 plugin for React 17 compatibility and test SiteScanNotice on plugins page.
await installLocalPlugin('gutenberg.14.7.3');

await completeWizard({ mode: 'transitional' });
await visitAdminPage('plugins.php', '');

await activate('gutenberg');
});

afterAll(async () => {
await uninstallPlugin('gutenberg');

// Install the latest Gutenberg plugin.
await installPlugin('gutenberg');
});

it('site scan is triggered automatically and displays no validation issues for AMP-compatible plugin', async () => {
Expand Down
10 changes: 3 additions & 7 deletions tests/php/src/Admin/AfterActivationSiteScanTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
use AmpProject\AmpWP\Tests\Helpers\PrivateAccess;
use AmpProject\AmpWP\Tests\Helpers\MockAdminUser;
use AMP_Validation_Manager;
use _WP_Dependency;

/**
* Tests for AfterActivationSiteScan class.
Expand Down Expand Up @@ -68,9 +67,6 @@ public function test__construct() {

/** @return array */
public function get_data_to_test_is_needed() {
$react = wp_scripts()->query( 'react' );
$should_get_site_activation_notice = $react instanceof _WP_Dependency && version_compare( $react->ver, '18', '<' );

return [
'not_admin_screen' => [
'screen_hook' => '',
Expand Down Expand Up @@ -105,7 +101,7 @@ public function get_data_to_test_is_needed() {
'plugins_screen_with_get_activate' => [
'screen_hook' => 'plugins.php',
'query_params' => [ 'activate' ],
'expected' => $should_get_site_activation_notice,
'expected' => true,
'role' => 'administrator',
],
'plugins_screen_with_get_activate_not_admin' => [
Expand All @@ -123,7 +119,7 @@ public function get_data_to_test_is_needed() {
'plugins_screen_with_get_activate_multi' => [
'screen_hook' => 'plugins.php',
'query_params' => [ 'activate-multi' ],
'expected' => $should_get_site_activation_notice,
'expected' => true,
'role' => 'administrator',
],
'plugins_screen_with_get_activated' => [
Expand All @@ -147,7 +143,7 @@ public function get_data_to_test_is_needed() {
'themes_screen_with_get_activated' => [
'screen_hook' => 'themes.php',
'query_params' => [ 'activated' ],
'expected' => $should_get_site_activation_notice,
'expected' => true,
'role' => 'administrator',
],
'themes_screen_with_get_activated_not_admin' => [
Expand Down