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

fix missing argument to load method of Loader #3

Closed

Conversation

michaellopez
Copy link
Contributor

When recording is enabled and a graphql request comes in that should be persisted, the call to load (to see whether the query id already exists or not) does not have the correct amount of arguments. The call needs to pass the operation name to load.

This PR fixes this and adds a test for it. Let me know if you would like me to take a different approach to the test, specifically the enableRecording()/resetRecording() setup. Wasn't sure how you wanted to handle database mutations for the unit tests @esamattis

@michaellopez
Copy link
Contributor Author

@esamattis Oh right, also the functional tests don't work. It's the functional tests that are failing in the automated tests. I'm assuming it is out of scope for this issue to fix that.

@michaellopez
Copy link
Contributor Author

@esamattis Sorry to bother you, any chance of getting this merged soon?

@esamattis
Copy link
Member

Yeah, the functional test issue is unrelated. Merged in cleaned up commit in 3f0a3ee and released in 0.1.4

@esamattis esamattis closed this Jun 12, 2020
@michaellopez
Copy link
Contributor Author

@esamattis Thank you!

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.

2 participants