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

bug: Add / to graphql validation endpoint #1045

Merged
merged 2 commits into from
Mar 21, 2019

Conversation

sirugh
Copy link
Contributor

@sirugh sirugh commented Mar 21, 2019

Description

The graphql validation endpoint we added in #1004 did not have a /. Since we don't mandate that the MAGENTO_BACKEND_URL end with a forward slash we should add one here to be safe.

Motivation and Context

$ yarn run download-schema && graphql validate-magento-pwa-queries --project venia
$ graphql get-schema --project venia
 request to https://release-dev-231-npzdaky-zddsyhrdimyra.us-4.magentosite.cloudgraphql failed, reason: getaddrinfo ENOTFOUND release-dev-231-npzdaky-zddsyhrdimyra.us-4.magentosite.cloudgraphql release-dev-231-npzdaky-zddsyhrdimyra.us-4.magentosite.cloudgraphql:443
Validating GraphQL queries in venia project...
 An error occurred:
Could not find a schema at lastCachedGraphQLSchema.json.
 Run `graphql get-schema --project venia` to download the schema before running validate-magento-pwa-queries.
error Command failed with exit code 1.

Verification

To verify, set the MAGENT_BACKEND_URL in your .env to something without a / like MAGENTO_BACKEND_URL="https://release-dev-231-npzdaky-zddsyhrdimyra.us-4.magentosite.cloud"

Then from root run yarn run validate-queries. It should fail.
Then check out this PR and run it again, and it should pass.

How Has This Been Tested?

Above verification. Also tested with url ending with /.

Screenshots (if appropriate):

Proposed Labels for Change Type/Package

Checklist:

  • I have read the CONTRIBUTING document.
  • I have linked an issue to this PR.
  • I have indicated the change type and relevant package(s).
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • All CI checks are green (linting, build/deploy, etc).
  • At least one core contributor has approved this PR.

@sirugh sirugh added bug Something isn't working pkg:venia-concept labels Mar 21, 2019
@vercel
Copy link

vercel bot commented Mar 21, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

@zetlen
Copy link
Contributor

zetlen commented Mar 21, 2019

This is a good addition, but it's a reminder to me that we need proper URI construction as part of UPWARD.

@sirugh
Copy link
Contributor Author

sirugh commented Mar 21, 2019

Yea I felt like its sort of papering over an issue but // is better than nothing :)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 76.917% when pulling 9c150d9 on sirugh_fix_graphql_check into 4b82402 on develop.

@supernova-at supernova-at self-assigned this Mar 21, 2019
@supernova-at supernova-at added the version: Patch This changeset includes backwards compatible bug fixes. label Mar 21, 2019
@supernova-at supernova-at merged commit 8728c93 into develop Mar 21, 2019
@supernova-at supernova-at deleted the sirugh_fix_graphql_check branch March 21, 2019 19:55
@brendanfalkowski
Copy link
Contributor

brendanfalkowski commented Mar 21, 2019

Just tested this out, and yarn run build fails for me on the following:

⚠ request to https://m2pwa.test//graphql failed, reason: unable to verify the first certificate

Opening https://m2pwa.test//graphql and https://m2pwa.test/graphql in the browser, both lead to this:

Screen Shot 2019-03-21 at 3 09 48 PM

I tried changing .env to contain or omit the trailing slash, but no effect.

I'm working off "pwastudio :: develop" branch. Any ideas?

@sirugh
Copy link
Contributor Author

sirugh commented Mar 22, 2019

@brendanfalkowski I've not seen the certificate error before. I think that is unrelated to this fix. Open an issue please!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pkg:venia-concept version: Patch This changeset includes backwards compatible bug fixes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants