Skip to content

Update queries for latest 2.3 schema and add validator script #318

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

Merged
merged 2 commits into from
Oct 4, 2018

Conversation

zetlen
Copy link
Contributor

@zetlen zetlen commented Oct 2, 2018

This PR is a:

[ ] New feature
[ ] Enhancement/Optimization
[ ] Refactor
[x] Bugfix
[ ] Test for existing code
[ ] Documentation

Summary

As of magento/graphql-ce#132 the Product schema has changed; the small_image is now an object with url and path parameters. This PR updates the corresponding queries, prop types, and tests to handle this change.

When merged, this will make Venia incompatible with Magento 2.3 prior to this commit.

For developer discoverability, this PR adds a script which validates queries against the connected Magento instance: npm run-script validate:venia:gql.

image

Error example:

image

In a later PR, it would be a good idea to run this validator every time a query changes, or when a Magento instance is connected...

@zetlen
Copy link
Contributor Author

zetlen commented Oct 2, 2018

Because this validation is dependent on a connected Magento instance, I haven't integrated it into the test suite or CI. Right now, we don't have a connected Magento instance for either.

@zetlen zetlen force-pushed the zetlen/update-small-image-query branch from 16ba8d4 to a82f27f Compare October 2, 2018 21:07
@PWAStudioBot
Copy link
Contributor

PWAStudioBot commented Oct 2, 2018

Fails
🚫

The following unit tests did not pass 😔. All tests must pass before this PR can be merged

packages/pwa-buildpack/src/WebpackTools/__tests__/PWADevServer.spec.js
�[1m�[31m  �[1m● �[1m.configure() is backwards compatible with `id` param�[39m�[22m
TypeError: Cannot set property 'spdy' of undefined

�[2m�[22m
�[2m �[0m �[90m 216 | �[39m )�[33m;�[39m�[0m�[22m
�[2m �[0m �[90m 217 | �[39m �[90m// workaround for https://github.com/webpack/webpack-dev-server/issues/1491�[39m�[0m�[22m
�[2m �[0m�[31m�[1m>�[2m�[39m�[90m 218 | �[39m devServerConfig�[33m.�[39mhttps�[33m.�[39mspdy �[33m=�[39m {�[0m�[22m
�[2m �[0m �[90m | �[39m �[31m�[1m^�[2m�[39m�[0m�[22m
�[2m �[0m �[90m 219 | �[39m protocols�[33m:�[39m [�[32m'http/1.1'�[39m]�[0m�[22m
�[2m �[0m �[90m 220 | �[39m }�[33m;�[39m�[0m�[22m
�[2m �[0m �[90m 221 | �[39m }�[0m�[22m
�[2m�[22m
�[2m �[2mat Object.configure (�[2m�[0m�[36msrc/WebpackTools/PWADevServer.js�[39m�[0m�[2m:218:13)�[2m�[22m


Generated by 🚫 dangerJS

@coveralls
Copy link

coveralls commented Oct 2, 2018

Pull Request Test Coverage Report for Build 1099

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.04%) to 20.376%

Totals Coverage Status
Change from base Build 1097: 0.04%
Covered Lines: 387
Relevant Lines: 1977

💛 - Coveralls

Copy link
Contributor

@jimbo jimbo left a comment

Choose a reason for hiding this comment

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

Seems to work as advertised. This will be a helpful addition. 👍

Just two minor changes requested.

@zetlen zetlen force-pushed the zetlen/update-small-image-query branch from 9b0a9ad to a8ce14d Compare October 3, 2018 02:43
@pcvonz
Copy link
Contributor

pcvonz commented Oct 3, 2018

Works fine on my end!

jimbo
jimbo previously approved these changes Oct 3, 2018
@zetlen zetlen force-pushed the zetlen/update-small-image-query branch from 7d714ac to 48dcb3c Compare October 3, 2018 23:35
@zetlen zetlen merged commit ef05a2d into release/2.0 Oct 4, 2018
@zetlen zetlen deleted the zetlen/update-small-image-query branch October 4, 2018 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants