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

Stop enforcing csrf checks in GraphQLTestCase #658

Merged
merged 1 commit into from Jun 9, 2019

Conversation

emilgoldsmith
Copy link
Contributor

Not sure what the purpose of that argument to the class instantiation was, maybe it was an artifact from when a wrapper class was called around the Client? But it acts as the enforce_csrf_checks argument which was botching up my tests (I copy pasted the class into my own project until you publish 2.3.0)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.718% when pulling e7e2560 on emilgoldsmith:patch-1 into 0916e03 on graphql-python:master.

Copy link
Collaborator

@zbyte64 zbyte64 left a comment

Choose a reason for hiding this comment

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

Indeed that is the parameter for csrf checks.

@emilgoldsmith
Copy link
Contributor Author

emilgoldsmith commented Jun 8, 2019

I have actually developed quite a bit on my copy paste version of this test case, was thinking of contributing back a lot of my changes, should I just make this PR bigger or do you think I should create a new one?

@jkimbo
Copy link
Member

jkimbo commented Jun 9, 2019

@emilgoldsmith could you create a new PR with any further improvements to the test class?

@jkimbo jkimbo merged commit 3cde872 into graphql-python:master Jun 9, 2019
@emilgoldsmith emilgoldsmith deleted the patch-1 branch June 10, 2019 17:21
@emilgoldsmith
Copy link
Contributor Author

@jkimbo will do!

@jkimbo jkimbo mentioned this pull request Jul 9, 2019
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.

None yet

4 participants