Remove Stories codebase by pierlon · Pull Request #4315 · ampproject/amp-wp

pierlon · 2020-02-19T16:56:51Z

Summary

This PR removes the Stories codebase.

Fixes #4203.

Todo

Remove Stories related JS tests
Remove Stories related PHP code
Remove Story templates

Checklist

My pull request is addressing an open issue (please create one otherwise).
My code is tested and passes existing tests.
My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

Gruntfile.js

assets/src/common/helpers/index.js

package.json

readme.md

package.json

assets/src/common/helpers/index.js

package.json

assets/src/common/helpers/index.js

swissspidy · 2020-02-20T09:37:44Z

Video files in tests/e2e/assets can also be removed.

pierlon · 2020-02-21T00:42:46Z

Reviews addressed in cd77370 and 97ca07a.

pierlon · 2020-02-21T00:58:11Z

I suppose the Stories project management guidelines can also be removed?

swissspidy · 2020-02-21T10:00:34Z

@pierlon Unless you all are using those? No idea 🤷‍♂

swissspidy · 2020-02-21T10:12:19Z

Starting to look really good! No just need to address the todos.

28k deleted lines of code already, crazy!

pierlon · 2020-02-21T20:14:36Z

@westonruter would you consider keeping the Stories deprecation admin pointer? The notice on the AMP options page will still be present.

pierlon · 2020-02-21T20:26:41Z

Also, how much of an impact will removing the 'Experiences' option create? I know that its used in the meta[name='generator'] tag to list the enabled experiences, for example.

includes/amp-helper-functions.php

includes/options/class-amp-options-manager.php

pierlon · 2020-02-24T04:05:30Z

includes/options/class-amp-options-manager.php

 		'theme_support'           => AMP_Theme_Support::READER_MODE_SLUG,
 		'supported_post_types'    => [ 'post' ],
 		'analytics'               => [],
 		'all_templates_supported' => true,
 		'supported_templates'     => [ 'is_singular' ],
 		'enable_response_caching' => true,
 		'version'                 => AMP__VERSION,
-		'story_templates_version' => false,


Removed Story relation settings; see ceab012.

Shouldn't that be a database upgrade routine instead?

This right now causes database queries on every page load.

Addressed in d67421e.

amp.php

pierlon · 2020-02-24T04:43:38Z

includes/options/class-amp-options-menu.php

@@ -473,7 +297,7 @@ public function render_supported_templates() {
 				$checked = (
 					post_type_supports( $post_type->name, AMP_Post_Type_Support::SLUG )
 					||
-					( ! AMP_Options_Manager::is_website_experience_enabled() && in_array( $post_type->name, $supported_post_types, true ) )


With this change, the post post type will be enabled by default in the supported post types.

pierlon · 2020-02-24T04:58:58Z

Also the welcome notice may need to be reworded; it currently states:

... this release enables rich, performant experiences for your WordPress site.

amp-wp/includes/options/class-amp-options-manager.php

Line 415 in d24b79c

    
           <h3><?php esc_html_e( 'From granular controls that help you create AMP content, to Core Gutenberg support, to a sanitizer that only shows visitors error-free pages, to a full error workflow for developers, this release enables rich, performant experiences for your WordPress site.', 'amp' ); ?></h3>

With the Stories experience now being removed, that technically isn't true anymore.

pierlon · 2020-02-24T05:02:03Z

src/Admin/SiteHealth.php

 							'value'   => AMP_Theme_Support::get_support_mode(),
 							'private' => false,
 						],
-						'amp_experiences_enabled' => [


Site Health no longer shows the enabled experiences (since Website will be the only one).

pierlon · 2020-02-24T05:03:52Z

includes/amp-helper-functions.php

 	}
 	$content .= sprintf( '; mode=%s', $mode );

-	$content .= sprintf( '; experiences=%s', implode( ',', AMP_Options_Manager::get_option( 'experiences' ) ) );


experiences property is no longer shown in the meta generator tag.

pierlon · 2020-02-24T22:09:32Z

amp.php

 	add_action( 'parse_query', 'amp_correct_query_when_is_front_page' );
 	add_action( 'admin_bar_menu', 'amp_add_admin_bar_view_link', 100 );
 	add_action( 'wp_loaded', 'amp_editor_core_blocks' );
+	add_action( 'amp_plugin_update', 'remove_amp_story_templates' );


Story related templates will be removed after a plugin update; see d67421e.

kienstra · 2020-03-06T00:41:07Z

package.json

@@ -145,7 +118,7 @@
    "lint:pkg-json": "wp-scripts lint-pkg-json --ignorePath .gitignore",
    "start": "wp-scripts start",
    "test": "npm-run-all --parallel test:*",
-    "test:e2e": "cross-env WP_BASE_URL=http://localhost:8890 node ./bin/local-env/run-e2e-tests.js --config=tests/e2e/jest.config.js",
+    "test:e2e": "cross-env WP_BASE_URL=http://localhost:8890 wp-scripts test-e2e --config=tests/e2e/jest.config.js",


Good idea to use wp-scripts

westonruter · 2020-03-07T06:15:40Z

@pierlon Ready for merge conflicts to be resolved for merge.

@swissspidy Please approve the PR when you are satisfied.

…tories-removal # Conflicts: # includes/class-amp-theme-support.php # package-lock.json # package.json

…tories-removal # Conflicts: # tests/php/test-class-site-health.php

swissspidy

Let's do it

westonruter · 2020-03-09T14:51:15Z

@swissspidy or @pierlon would you resolve the package conflicts?

…tories-removal # Conflicts: # package-lock.json # package.json

pierlon · 2020-03-09T15:03:08Z

I can get this resolved 👷‍♂️ .

pierlon added 6 commits February 19, 2020 11:54

Remove Stories related media, JS and CSS code

fb6ff38

Fix failing test

8d64147

Remove unused npm dependencies

aa094d8

Update readme

fb1304b

Remove utils

3c2c06b

Remove fonts.json

ab2fa8a

googlebot added the cla: yes Signed the Google CLA label Feb 19, 2020

pierlon requested review from swissspidy and westonruter February 19, 2020 16:57

pierlon commented Feb 19, 2020

View reviewed changes

Gruntfile.js Show resolved Hide resolved

assets/src/common/helpers/index.js Outdated Show resolved Hide resolved

package.json Outdated Show resolved Hide resolved

readme.md Show resolved Hide resolved

pierlon changed the title ~~Enhancement/4203 stories removal~~ Remove Stories codebase Feb 19, 2020

swissspidy reviewed Feb 19, 2020

View reviewed changes

readme.md Show resolved Hide resolved

swissspidy reviewed Feb 19, 2020

View reviewed changes

package.json Outdated Show resolved Hide resolved

swissspidy reviewed Feb 19, 2020

View reviewed changes

package.json Outdated Show resolved Hide resolved

Re-add trailing whitespaces to readme.md

cd77370

swissspidy reviewed Feb 20, 2020

View reviewed changes

assets/src/common/helpers/index.js Show resolved Hide resolved

swissspidy reviewed Feb 20, 2020

View reviewed changes

assets/src/common/helpers/index.js Show resolved Hide resolved

swissspidy reviewed Feb 20, 2020

View reviewed changes

package.json Outdated Show resolved Hide resolved

swissspidy reviewed Feb 20, 2020

View reviewed changes

assets/src/common/helpers/index.js Outdated Show resolved Hide resolved

Remove unused JS methods & npm dependencies

97ca07a

Update package-lock.json

045d4ad

Remove Stories related PHP code

94fe951

Re-add Stories deprecation notice for AMP options page

258b445

Make phpunit happy

ca5a15e

pierlon commented Feb 24, 2020

View reviewed changes

pierlon mentioned this pull request Feb 24, 2020

Remove AMP Stories and eliminate the obsolete experiences UI #4203

Closed

pierlon marked this pull request as ready for review February 24, 2020 04:59

pierlon requested a review from swissspidy February 24, 2020 04:59

pierlon commented Feb 24, 2020

View reviewed changes

pierlon mentioned this pull request Feb 24, 2020

PM: Updating Project Management guidelines for AMP Plugin #4322

Closed

1 task

Remove Story templates upon plugin update

d67421e

pierlon commented Feb 24, 2020

View reviewed changes

kienstra mentioned this pull request Feb 29, 2020

Consider removing remaining e2e tests #4341

Closed

kienstra reviewed Mar 6, 2020

View reviewed changes

pierlon and others added 5 commits March 7, 2020 01:36

Merge remote-tracking branch 'origin/develop' into enhancement/4203-s…

8b3c8b6

…tories-removal # Conflicts: # includes/class-amp-theme-support.php # package-lock.json # package.json

Update package-lock.json

50a84a4

Merge remote-tracking branch 'origin/develop' into enhancement/4203-s…

18a64e0

…tories-removal # Conflicts: # tests/php/test-class-site-health.php

Re-add image assets for tests

050b5a0

Remove now unneeded dotenv dependency

318d78d

swissspidy approved these changes Mar 9, 2020

View reviewed changes

pierlon added 2 commits March 9, 2020 11:00

Merge remote-tracking branch 'origin/develop' into enhancement/4203-s…

e2b87cb

…tories-removal # Conflicts: # package-lock.json # package.json

Update package-lock.json

678a799

westonruter merged commit 7460a0e into develop Mar 9, 2020

westonruter deleted the enhancement/4203-stories-removal branch March 9, 2020 15:13

westonruter mentioned this pull request Mar 9, 2020

Remove dotenv config #4372

Merged

westonruter mentioned this pull request Mar 19, 2020

Reduce scenarios where DB writes are performed during frontend requests #3284

Closed

westonruter added this to the v1.5 milestone Mar 22, 2020

westonruter added the Changelogged label Mar 23, 2020

spacedmonkey mentioned this pull request Oct 7, 2020

Add support jetpack Metadata for web stories plugin Automattic/jetpack#17399

Merged

Conversation

pierlon commented Feb 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Todo

Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

swissspidy commented Feb 20, 2020

Uh oh!

pierlon commented Feb 21, 2020

Uh oh!

pierlon commented Feb 21, 2020

Uh oh!

swissspidy commented Feb 21, 2020

Uh oh!

swissspidy commented Feb 21, 2020

Uh oh!

pierlon commented Feb 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pierlon commented Feb 21, 2020

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pierlon commented Feb 24, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

westonruter commented Mar 7, 2020

Uh oh!

swissspidy left a comment

Choose a reason for hiding this comment

Uh oh!

westonruter commented Mar 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pierlon commented Mar 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pierlon commented Feb 19, 2020 •

edited

Loading

pierlon commented Feb 21, 2020 •

edited

Loading

westonruter commented Mar 9, 2020 •

edited

Loading