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.

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

Shopping: Products JSON Schema Validation #11250

Merged
merged 17 commits into from
Apr 30, 2022
Merged

Shopping: Products JSON Schema Validation #11250

merged 17 commits into from
Apr 30, 2022

Conversation

timarney
Copy link
Contributor

@timarney timarney commented Apr 12, 2022

Context

In support of #11210 we need make the WoocCommerce plugin available with some sample products to query against to ensure a valid JSON schema is returned.

Using this JSON schema for shoppable products, we can verify that the data we return from WooCommerce & Shopify matches the expected format. So most likely this is something for the PHPUnit integration tests.

Summary

  • Adds WoocCommerce plugin installation step to start script
  • Adds sample product import

Relevant Technical Choices

To-do

  • e2e test is failing which is expected as the value for product price isn't matching the schema

User-facing changes

Testing Instructions

  • npm run env:start
  • note CLI outputs that WooCommerce is installed
  • note in the WP Admin - WooCommerce has imported sample products (via the sample xml file)
  • This is a non-user-facing change and requires no QA

This PR can be tested by following these steps:

  1. Run npm run env:start --- this should install WooCommerce + sample products
  2. Activate the WooCommerce plugin --- ensure sample products exists

Reviews

Does this PR have a security-related impact?

No

Does this PR change what data or activity we track or use?

No

Does this PR have a legal-related impact?

No

Checklist

  • This PR addresses an existing issue and I have linked this PR to it in ZenHub
  • I have tested this code to the best of my abilities
  • I have verified accessibility to the best of my abilities (docs)
  • I have verified i18n and l10n (translation, right-to-left layout) to the best of my abilities
  • This code is covered by automated tests (unit, integration, and/or e2e) to verify it works as intended (docs)
  • I have added documentation where necessary
  • I have added a matching Type: XYZ label to the PR

Partially addresses #11210

@timarney timarney self-assigned this Apr 12, 2022
@timarney timarney force-pushed the feature/woocommerce branch 3 times, most recently from f0142e4 to bb56e22 Compare April 18, 2022 10:04
@timarney timarney added Pod: WP Type: Infrastructure Changes impacting testing infrastructure or build tooling labels Apr 18, 2022
@timarney
Copy link
Contributor Author

Noting this was failing on WP 5.5 -- not sure if that's a plugin issue or something else happening. For now I have wrapped a WP latest check around the install.

https://github.com/GoogleForCreators/web-stories-wp/runs/6061324126?check_suite_focus=true

@timarney timarney marked this pull request as ready for review April 18, 2022 11:24
@timarney timarney added Group: Shopping Shop-able products in stories Type: Enhancement New feature or improvement of an existing feature labels Apr 18, 2022
@timarney timarney marked this pull request as draft April 18, 2022 12:10
@timarney timarney changed the title Add woocommerce plugin Shopping: Products JSON Schema Validation Apr 19, 2022
@timarney timarney force-pushed the feature/woocommerce branch 4 times, most recently from 5e56f9f to 8b7d7b0 Compare April 20, 2022 09:02
@timarney timarney force-pushed the feature/woocommerce branch from 8b7d7b0 to 0916a61 Compare April 20, 2022 10:07
@timarney timarney marked this pull request as ready for review April 20, 2022 11:00
includes/REST_API/Products_Controller.php Outdated Show resolved Hide resolved
bin/local-env/install-wordpress.sh Show resolved Hide resolved
bin/local-env/install-wordpress.sh Outdated Show resolved Hide resolved
bin/local-env/install-wordpress.sh Show resolved Hide resolved
@timarney timarney marked this pull request as draft April 20, 2022 11:21
@timarney timarney force-pushed the feature/woocommerce branch from 265f14f to 08876dd Compare April 21, 2022 10:30
@swissspidy
Copy link
Collaborator

Hmm alright, then I didn‘t look close enough. Will re-check on Monday.

@swissspidy swissspidy mentioned this pull request Apr 26, 2022
8 tasks
@timarney timarney mentioned this pull request Apr 26, 2022
8 tasks
@timarney
Copy link
Contributor Author

timarney commented Apr 26, 2022

@swissspidy I tested locally with your composer update commit merged back to this one it's s now working as expected (fails on the schema check string vs number)

@swissspidy
Copy link
Collaborator

@swissspidy I tested locally with your composer update commit merged back to this one it's s now working as expected (fails on the schema check string vs number)

Okay and which schema check is exactly failing? Can we fix our code?

I only see this error message:

  ● Shopping › Shopping schema › should should match a valid schema

    TimeoutError: waiting for selector `amp-story-shopping-attachment script` failed: timeout 30000ms exceeded

https://github.com/GoogleForCreators/web-stories-wp/runs/6174672630?check_suite_focus=true

@timarney
Copy link
Contributor Author

@swissspidy I tested locally with your composer update commit merged back to this one it's s now working as expected (fails on the schema check string vs number)

Okay and which schema check is exactly failing? Can we fix our code?

I only see this error message:

  ● Shopping › Shopping schema › should should match a valid schema

    TimeoutError: waiting for selector `amp-story-shopping-attachment script` failed: timeout 30000ms exceeded

https://github.com/GoogleForCreators/web-stories-wp/runs/6174672630?check_suite_focus=true

Right for CI --- need to investigate why it's not finding that selector

Locally for me I'm getting a schema error because product price is returning a string vs the schema is looking for a number. So yes that part can be fixed --- but maybe that fix should be in the product query PR?

@swissspidy
Copy link
Collaborator

So yes that part can be fixed --- but maybe that fix should be in the product query PR?

Works for me; just keep in mind that then the test needs to be disabled until that gets fixed.

@timarney timarney marked this pull request as ready for review April 27, 2022 00:50
@github-actions
Copy link
Contributor

github-actions bot commented Apr 27, 2022

Size Change: +5 B (0%)

Total Size: 2.6 MB

ℹ️ View Unchanged
Filename Size Change
assets/css/carousel-view-rtl.css 702 B 0 B
assets/css/carousel-view.css 701 B 0 B
assets/css/web-stories-block-rtl.css 4.5 kB 0 B
assets/css/web-stories-block.css 4.55 kB 0 B
assets/css/web-stories-embed-rtl.css 317 B 0 B
assets/css/web-stories-embed.css 317 B 0 B
assets/css/web-stories-list-styles-rtl.css 2.34 kB 0 B
assets/css/web-stories-list-styles.css 2.37 kB 0 B
assets/css/web-stories-theme-style-twentyeleven-rtl.css 102 B 0 B
assets/css/web-stories-theme-style-twentyeleven.css 102 B 0 B
assets/css/web-stories-theme-style-twentyfifteen-rtl.css 251 B 0 B
assets/css/web-stories-theme-style-twentyfifteen.css 251 B 0 B
assets/css/web-stories-theme-style-twentyfourteen-rtl.css 287 B 0 B
assets/css/web-stories-theme-style-twentyfourteen.css 287 B 0 B
assets/css/web-stories-theme-style-twentyseventeen-rtl.css 288 B 0 B
assets/css/web-stories-theme-style-twentyseventeen.css 288 B 0 B
assets/css/web-stories-theme-style-twentysixteen-rtl.css 224 B 0 B
assets/css/web-stories-theme-style-twentysixteen.css 224 B 0 B
assets/css/web-stories-theme-style-twentyten-rtl.css 143 B 0 B
assets/css/web-stories-theme-style-twentyten.css 143 B 0 B
assets/css/web-stories-theme-style-twentytwelve-rtl.css 256 B 0 B
assets/css/web-stories-theme-style-twentytwelve.css 256 B 0 B
assets/css/web-stories-theme-style-twentytwenty-rtl.css 86 B 0 B
assets/css/web-stories-theme-style-twentytwenty.css 86 B 0 B
assets/css/web-stories-theme-style-twentytwentyone-rtl.css 326 B 0 B
assets/css/web-stories-theme-style-twentytwentyone.css 326 B 0 B
assets/css/web-stories-widget-rtl.css 482 B 0 B
assets/css/web-stories-widget.css 482 B 0 B
assets/css/wp-dashboard-rtl.css 657 B 0 B
assets/css/wp-dashboard.css 659 B 0 B
assets/css/wp-story-editor-rtl.css 607 B 0 B
assets/css/wp-story-editor.css 609 B 0 B
assets/js/4422.js 49.3 kB 0 B
assets/js/4610.js 1.14 MB 0 B
assets/js/5756.js 215 kB 0 B
assets/js/5825.js 26.5 kB 0 B
assets/js/5980.js 5.48 kB 0 B
assets/js/9860.js 7.38 kB 0 B
assets/js/carousel-view.js 3.41 kB 0 B
assets/js/chunk-colorthief.js 2.64 kB 0 B
assets/js/chunk-ffmpeg.js 5.64 kB 0 B
assets/js/chunk-focus-visible.js 1.01 kB 0 B
assets/js/chunk-getStoryMarkup.js 5.65 kB 0 B
assets/js/chunk-html-to-image.js 4.6 kB 0 B
assets/js/chunk-opentype.js 96 B 0 B
assets/js/chunk-react-calendar.js 12.4 kB 0 B
assets/js/chunk-react-color.js 44.3 kB 0 B
assets/js/chunk-resize-observer-polyfill.js 2.57 kB 0 B
assets/js/chunk-web-animations-js.js 14.6 kB 0 B
assets/js/chunk-web-stories-template-0-metaData.js 546 B 0 B
assets/js/chunk-web-stories-template-0.js 10.8 kB 0 B
assets/js/chunk-web-stories-template-1-metaData.js 540 B 0 B
assets/js/chunk-web-stories-template-1.js 9.04 kB 0 B
assets/js/chunk-web-stories-template-10-metaData.js 533 B 0 B
assets/js/chunk-web-stories-template-10.js 7.01 kB 0 B
assets/js/chunk-web-stories-template-11-metaData.js 540 B 0 B
assets/js/chunk-web-stories-template-11.js 8.51 kB 0 B
assets/js/chunk-web-stories-template-12-metaData.js 496 B 0 B
assets/js/chunk-web-stories-template-12.js 9.59 kB 0 B
assets/js/chunk-web-stories-template-13-metaData.js 525 B 0 B
assets/js/chunk-web-stories-template-13.js 7.42 kB 0 B
assets/js/chunk-web-stories-template-14-metaData.js 582 B 0 B
assets/js/chunk-web-stories-template-14.js 7.58 kB 0 B
assets/js/chunk-web-stories-template-15-metaData.js 544 B 0 B
assets/js/chunk-web-stories-template-15.js 8.27 kB 0 B
assets/js/chunk-web-stories-template-16-metaData.js 588 B 0 B
assets/js/chunk-web-stories-template-16.js 10.4 kB 0 B
assets/js/chunk-web-stories-template-17-metaData.js 539 B 0 B
assets/js/chunk-web-stories-template-17.js 8.54 kB 0 B
assets/js/chunk-web-stories-template-18-metaData.js 585 B 0 B
assets/js/chunk-web-stories-template-18.js 9.33 kB 0 B
assets/js/chunk-web-stories-template-19-metaData.js 501 B 0 B
assets/js/chunk-web-stories-template-19.js 10.1 kB 0 B
assets/js/chunk-web-stories-template-2-metaData.js 586 B 0 B
assets/js/chunk-web-stories-template-2.js 9.2 kB 0 B
assets/js/chunk-web-stories-template-20-metaData.js 548 B 0 B
assets/js/chunk-web-stories-template-20.js 8.69 kB 0 B
assets/js/chunk-web-stories-template-21-metaData.js 534 B 0 B
assets/js/chunk-web-stories-template-21.js 9.23 kB 0 B
assets/js/chunk-web-stories-template-22-metaData.js 525 B 0 B
assets/js/chunk-web-stories-template-22.js 7.46 kB 0 B
assets/js/chunk-web-stories-template-23-metaData.js 605 B 0 B
assets/js/chunk-web-stories-template-23.js 7.13 kB 0 B
assets/js/chunk-web-stories-template-24-metaData.js 518 B 0 B
assets/js/chunk-web-stories-template-24.js 10.9 kB 0 B
assets/js/chunk-web-stories-template-25-metaData.js 544 B 0 B
assets/js/chunk-web-stories-template-25.js 7.23 kB 0 B
assets/js/chunk-web-stories-template-26-metaData.js 601 B 0 B
assets/js/chunk-web-stories-template-26.js 6.85 kB 0 B
assets/js/chunk-web-stories-template-27-metaData.js 543 B 0 B
assets/js/chunk-web-stories-template-27.js 7.63 kB 0 B
assets/js/chunk-web-stories-template-28-metaData.js 532 B 0 B
assets/js/chunk-web-stories-template-28.js 8.63 kB 0 B
assets/js/chunk-web-stories-template-29-metaData.js 561 B 0 B
assets/js/chunk-web-stories-template-29.js 8.57 kB 0 B
assets/js/chunk-web-stories-template-3-metaData.js 540 B 0 B
assets/js/chunk-web-stories-template-3.js 8.29 kB 0 B
assets/js/chunk-web-stories-template-30-metaData.js 576 B 0 B
assets/js/chunk-web-stories-template-30.js 7.87 kB 0 B
assets/js/chunk-web-stories-template-31-metaData.js 503 B 0 B
assets/js/chunk-web-stories-template-31.js 9.64 kB 0 B
assets/js/chunk-web-stories-template-32-metaData.js 551 B 0 B
assets/js/chunk-web-stories-template-32.js 12.3 kB 0 B
assets/js/chunk-web-stories-template-33-metaData.js 492 B 0 B
assets/js/chunk-web-stories-template-33.js 8.94 kB 0 B
assets/js/chunk-web-stories-template-34-metaData.js 571 B 0 B
assets/js/chunk-web-stories-template-34.js 7.72 kB 0 B
assets/js/chunk-web-stories-template-35-metaData.js 565 B 0 B
assets/js/chunk-web-stories-template-35.js 8.97 kB 0 B
assets/js/chunk-web-stories-template-36-metaData.js 576 B 0 B
assets/js/chunk-web-stories-template-36.js 11.7 kB 0 B
assets/js/chunk-web-stories-template-37-metaData.js 528 B 0 B
assets/js/chunk-web-stories-template-37.js 6.5 kB 0 B
assets/js/chunk-web-stories-template-38-metaData.js 572 B 0 B
assets/js/chunk-web-stories-template-38.js 8 kB 0 B
assets/js/chunk-web-stories-template-39-metaData.js 589 B 0 B
assets/js/chunk-web-stories-template-39.js 7.71 kB 0 B
assets/js/chunk-web-stories-template-4-metaData.js 565 B 0 B
assets/js/chunk-web-stories-template-4.js 11.6 kB 0 B
assets/js/chunk-web-stories-template-40-metaData.js 556 B 0 B
assets/js/chunk-web-stories-template-40.js 9.17 kB 0 B
assets/js/chunk-web-stories-template-41-metaData.js 572 B 0 B
assets/js/chunk-web-stories-template-41.js 7.86 kB 0 B
assets/js/chunk-web-stories-template-42-metaData.js 522 B 0 B
assets/js/chunk-web-stories-template-42.js 7.12 kB 0 B
assets/js/chunk-web-stories-template-43-metaData.js 558 B 0 B
assets/js/chunk-web-stories-template-43.js 8.47 kB 0 B
assets/js/chunk-web-stories-template-44-metaData.js 582 B 0 B
assets/js/chunk-web-stories-template-44.js 10.4 kB 0 B
assets/js/chunk-web-stories-template-45-metaData.js 564 B 0 B
assets/js/chunk-web-stories-template-45.js 7.36 kB 0 B
assets/js/chunk-web-stories-template-46-metaData.js 531 B 0 B
assets/js/chunk-web-stories-template-46.js 5.05 kB 0 B
assets/js/chunk-web-stories-template-47-metaData.js 592 B 0 B
assets/js/chunk-web-stories-template-47.js 8.46 kB 0 B
assets/js/chunk-web-stories-template-48-metaData.js 556 B 0 B
assets/js/chunk-web-stories-template-48.js 8.35 kB 0 B
assets/js/chunk-web-stories-template-49-metaData.js 518 B 0 B
assets/js/chunk-web-stories-template-49.js 9.84 kB 0 B
assets/js/chunk-web-stories-template-5-metaData.js 555 B 0 B
assets/js/chunk-web-stories-template-5.js 9.44 kB 0 B
assets/js/chunk-web-stories-template-50-metaData.js 504 B 0 B
assets/js/chunk-web-stories-template-50.js 8.45 kB 0 B
assets/js/chunk-web-stories-template-51-metaData.js 527 B 0 B
assets/js/chunk-web-stories-template-51.js 10 kB 0 B
assets/js/chunk-web-stories-template-52-metaData.js 602 B 0 B
assets/js/chunk-web-stories-template-52.js 10.2 kB 0 B
assets/js/chunk-web-stories-template-53-metaData.js 553 B 0 B
assets/js/chunk-web-stories-template-53.js 5.93 kB 0 B
assets/js/chunk-web-stories-template-54-metaData.js 547 B 0 B
assets/js/chunk-web-stories-template-54.js 7.59 kB 0 B
assets/js/chunk-web-stories-template-55-metaData.js 574 B 0 B
assets/js/chunk-web-stories-template-55.js 6.79 kB 0 B
assets/js/chunk-web-stories-template-56-metaData.js 543 B 0 B
assets/js/chunk-web-stories-template-56.js 9.6 kB 0 B
assets/js/chunk-web-stories-template-57-metaData.js 528 B 0 B
assets/js/chunk-web-stories-template-57.js 14.2 kB 0 B
assets/js/chunk-web-stories-template-58-metaData.js 556 B 0 B
assets/js/chunk-web-stories-template-58.js 5.72 kB 0 B
assets/js/chunk-web-stories-template-59-metaData.js 588 B 0 B
assets/js/chunk-web-stories-template-59.js 8.76 kB 0 B
assets/js/chunk-web-stories-template-6-metaData.js 569 B 0 B
assets/js/chunk-web-stories-template-6.js 7.11 kB 0 B
assets/js/chunk-web-stories-template-60-metaData.js 509 B 0 B
assets/js/chunk-web-stories-template-60.js 9.09 kB 0 B
assets/js/chunk-web-stories-template-7-metaData.js 569 B 0 B
assets/js/chunk-web-stories-template-7.js 7.25 kB 0 B
assets/js/chunk-web-stories-template-8-metaData.js 569 B 0 B
assets/js/chunk-web-stories-template-8.js 8.48 kB 0 B
assets/js/chunk-web-stories-template-9-metaData.js 581 B 0 B
assets/js/chunk-web-stories-template-9.js 8.54 kB 0 B
assets/js/chunk-web-stories-templates.js 442 B 0 B
assets/js/chunk-web-stories-textset-0.js 5.08 kB 0 B
assets/js/chunk-web-stories-textset-1.js 6.64 kB 0 B
assets/js/chunk-web-stories-textset-2.js 7.67 kB 0 B
assets/js/chunk-web-stories-textset-3.js 15.1 kB 0 B
assets/js/chunk-web-stories-textset-4.js 4.16 kB 0 B
assets/js/chunk-web-stories-textset-5.js 5.49 kB 0 B
assets/js/chunk-web-stories-textset-6.js 5.29 kB 0 B
assets/js/chunk-web-stories-textset-7.js 10.2 kB 0 B
assets/js/generateBlurhash.worker.worker.js 1.09 kB 0 B
assets/js/imgareaselect.js 3.77 kB 0 B
assets/js/lightbox.js 550 B 0 B
assets/js/tinymce-button.js 2.84 kB 0 B
assets/js/web-stories-activation-notice.js 22.2 kB 0 B
assets/js/web-stories-block.js 18 kB 0 B
assets/js/web-stories-embed.js 20 B 0 B
assets/js/web-stories-widget.js 587 B 0 B
assets/js/wp-dashboard.js 69.9 kB +5 B (0%)
assets/js/wp-story-editor.js 299 kB 0 B

compressed-size-action

@googleforcreators-bot
Copy link
Collaborator

googleforcreators-bot commented Apr 27, 2022

Plugin builds for 17ea300 are ready 🛎️!

@swissspidy
Copy link
Collaborator

@timarney Heads up! Currently working on some updates to help make the tests a bit less flakey. Please do not rebase/override my changes.

@swissspidy
Copy link
Collaborator

All e2e tests seem to be passing now! 🎉

@swissspidy
Copy link
Collaborator

@spacedmonkey ping 👋 to review the latest version of this

@swissspidy swissspidy merged commit 42f741d into main Apr 30, 2022
@swissspidy swissspidy deleted the feature/woocommerce branch April 30, 2022 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Group: Shopping Shop-able products in stories Type: Enhancement New feature or improvement of an existing feature Type: Infrastructure Changes impacting testing infrastructure or build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants