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

Extended Graphql support: Identify GraphQL requests fired as HTTP GET Requests. #884

Merged
merged 25 commits into from Sep 23, 2022

Conversation

ArjanSM
Copy link
Contributor

@ArjanSM ArjanSM commented Sep 14, 2022

πŸ“· Screenshots

Screen Shot 2022-09-14 at 5 09 32 PM

πŸ“„ Context

Based on top #805 to support identification of GraphQL requests fired as HTTP GET.
Also the approach taken is as mentioned in #116

πŸ“ Changes

πŸ› οΈ How to test

  1. Run the Sample app
  2. Click on "Do GraphQL Request"
  3. "Launch Chucker Directly" to see the GraphQL logo next to Get request as well.

Tests have been updated.

@cortinico
Copy link
Member

Thanks for this PR

Also the approach taken is as mentioned in #116

I'd like to challenge the approach a bit as this looks like a bit of an overkill to me. Could we just detect if the path ends with /graphql and add this as an heuristic to show the GraphQL icon (similarly to what we do for the Header?

@ArjanSM
Copy link
Contributor Author

ArjanSM commented Sep 15, 2022

@cortinico so I take it that we would also like to detect GQL request based on the operation names?

@ArjanSM
Copy link
Contributor Author

ArjanSM commented Sep 15, 2022

@cortinico done.

@cortinico
Copy link
Member

detect GQL request based on the operation names?

Yup we can have a combination of the two

@ArjanSM
Copy link
Contributor Author

ArjanSM commented Sep 15, 2022

@cortinico I've included the changes that ID requests based on Operation name.

Copy link
Member

@cortinico cortinico left a comment

Choose a reason for hiding this comment

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

We're getting closer to a mergeable state :) Thanks for your contrib @ArjanSM. I've done another pass and left several comments

Copy link
Member

@cortinico cortinico left a comment

Choose a reason for hiding this comment

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

Thanks for doing this work @ArjanSM

@ArjanSM
Copy link
Contributor Author

ArjanSM commented Sep 23, 2022

Thanks @cortinico. Any idea when do you plan to merge this PR?

@cortinico cortinico merged commit 3b1a89b into ChuckerTeam:develop Sep 23, 2022
@ArjanSM ArjanSM changed the title Extended Graphql support: Identify GraphQL fired as HTTP GET Requests. Extended Graphql support: Identify GraphQL requests fired as HTTP GET Requests. Sep 26, 2022
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

2 participants