Skip to content
This repository has been archived by the owner on Nov 23, 2021. It is now read-only.

feat: support graphql body mode #37

Merged
merged 4 commits into from
Aug 22, 2019
Merged

feat: support graphql body mode #37

merged 4 commits into from
Aug 22, 2019

Conversation

dmhalejr
Copy link
Contributor

@dmhalejr dmhalejr commented Jul 4, 2019

Closes #36

Following this blog post, I've switched all my API testing collections to graphql body mode.
https://blog.getpostman.com/2019/06/18/postman-v7-2-supports-graphql/

There's probably more nuance in the solution but wanted to get something going as a starting point. Ran all tests locally and they still continued to pass and ran bin/postman-to-k6.js locally gave me appropriate output.

Let me know if there's anything else I can assist with! Thanks for all you do! 馃檱

@bookmoons
Copy link
Contributor

Thanks a lot for this, it looks great. Does just the right thing.

Could you add a test case? There's a test I exported, or whatever test you had. You could put it in eg test/material/2.1/graphql.json then add a test to test/int/basic.js.

{
	"info": {
		"_postman_id": "505fff05-4b0c-4356-8964-ca8a1ae68999",
		"name": "GraphQL",
		"schema": "https://schema.getpostman.com/json/collection/v2.1.0/collection.json"
	},
	"item": [
		{
			"name": "Test Request",
			"protocolProfileBehavior": {
				"disableBodyPruning": true
			},
			"request": {
				"method": "GET",
				"header": [],
				"body": {
					"mode": "graphql",
					"graphql": {
						"query": "query getCollectionDetails {\n    collections {\n        name\n    }\n}",
						"variables": "{\n\t\"name\": \"{{artist}}\"\n}"
					}
				},
				"url": {
					"raw": "http://example.com/graphql",
					"protocol": "http",
					"host": [
						"example",
						"com"
					],
					"path": [
						"graphql"
					]
				}
			},
			"response": []
		}
	]
}

lib/generate/Request.js Outdated Show resolved Hide resolved
@bookmoons
Copy link
Contributor

What do you think about the GraphQL variables? Are you using them? They need some additional logic but if you're not using them maybe they could be left out of the first version.

@dmhalejr
Copy link
Contributor Author

dmhalejr commented Jul 5, 2019

@bookmoons GraphQL variables make life so much easier. I use them heavily so for the first version it'd probably be ideal.

Just to note, Postman exports variables in a simple JSON object and the test added to the repository reflects that the data.graphql entry lifts out the query and variables entry appropriately. On GET requests they are just leveraged as a query parameter.

I'm not sure the implications of GraphQL variables linked with Postman environment variables.

Any additional guidance would be appreciated! 馃憤

@bookmoons
Copy link
Contributor

Alright, I think this is what it looks like:

  • GraphQL vars can be parsed to an object and passed down in a new postman[Request] arg. The GraphQL query can be passed as the body.
  • Postman vars need to be replaced at runtime. This happens in lib/shim/core.js. postman[Request] can detect graphql vars, run evaluate on each one to replace vars, then run a GraphQL var replacement on the body.

There's a standard Postman var replacement that happens on the string body. Probably it needs to be determined whether that still happens for graphql and in what order with graphql vars, so it can be either disabled or correctly ordered when doing a graphql request.

@dmhalejr
Copy link
Contributor Author

@bookmoons Just getting back to this. I found out my implementation will work for our purposes without the variables. Went ahead and got this PR to cleanly merge with the last few weeks of changes. Let me know if it's good to go or should await the full implementation.

@bookmoons
Copy link
Contributor

Thanks @dmhalejr! Will look this over soon.

.gitignore Outdated Show resolved Hide resolved
@bookmoons
Copy link
Contributor

Tested this against the GitHub API and it worked just fine. It also happens to run the query through Postman var replacement, so that's a usable alternative to GraphQL vars. Thanks a lot @dmhalejr.

@robingustafsson After the little update above this change has my approval.

@bookmoons
Copy link
Contributor

Thanks for updating that @dmhalejr.

@robingustafsson
Copy link
Member

Great, thanks @dmhalejr for contribution and @bookmoons for review. Sorry for delay, I'm merging this.

@robingustafsson robingustafsson merged commit 95f8e38 into grafana:master Aug 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support New Body Mode GraphQL for Postman Collections
3 participants