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

feat: Enable consumers to access the full response payload #43

Merged

Conversation

billybonks
Copy link
Contributor

@billybonks billybonks commented Nov 12, 2024

Update the Response public API object to include full_response. this is non-breaking.

Passing the full response from the executing object to the client is currently done via the client checking with the object, versus it being part of the response,

In order to change this it requires changing the much more changes, as all the tests are using GraphQL::Schema for the execution of the graphql

@billybonks billybonks force-pushed the feat/access-full-response branch from 202b5be to 0d3af67 Compare November 12, 2024 08:20
Update the Response public API object to include full_response. this is non-breaking.

Passing the full response from the executing object to the client is currently done via the client checking with the object, versus it being part of the response,

In order to change this it requires changing the much more changes, as all the tests are using GraphQL::Schema for the execution of the graphql
@billybonks billybonks force-pushed the feat/access-full-response branch from 0d3af67 to 1d42e19 Compare November 12, 2024 08:28
@rmosolgo
Copy link
Collaborator

Hey, thanks for this contribution! It looks good to me.

lib/graphql/client.rb Outdated Show resolved Hide resolved
lib/graphql/client.rb Outdated Show resolved Hide resolved
@rmosolgo rmosolgo merged commit b547fa4 into github-community-projects:master Nov 13, 2024
81 checks passed
@rmosolgo
Copy link
Collaborator

🚢 In 0.24.0! Thanks again.

@billybonks
Copy link
Contributor Author

My pleasure thanks for the swift review and easy contribution :)

@billybonks billybonks mentioned this pull request Nov 14, 2024
3 tasks
@ticklemynausea
Copy link

Hi, thanks for this, this was actually very important for a use case that we had and we were considering implementing these changes.

That said, it is very confusing that the new method named full_response returns a Hash of containing just the headers of the response.

> result.full_response
=> {"server"=>["nginx"],
 "date"=>["Fri, 22 Nov 2024 16:08:46 GMT"],
 "content-type"=>["application/json"],
 "content-length"=>["1337050"],
 "connection"=>["close"],
 ...

Is this the actual intended behavior? I would believe that such would have the newly exposed method called headers, not full_response.

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.

3 participants