Skip to content

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

Merged
cortinico merged 25 commits into
ChuckerTeam:developfrom
ArjanSM:extended_graphql_support
Sep 23, 2022
Merged

Extended Graphql support: Identify GraphQL requests fired as HTTP GET Requests.#884
cortinico merged 25 commits into
ChuckerTeam:developfrom
ArjanSM:extended_graphql_support

Conversation

@ArjanSM

@ArjanSM ArjanSM commented Sep 14, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

📷 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

cortinico commented Sep 14, 2022

Copy link
Copy Markdown
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

ArjanSM commented Sep 15, 2022

Copy link
Copy Markdown
Contributor Author

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

@ArjanSM

ArjanSM commented Sep 15, 2022

Copy link
Copy Markdown
Contributor Author

@cortinico done.

@cortinico

cortinico commented Sep 15, 2022

Copy link
Copy Markdown
Member

detect GQL request based on the operation names?

Yup we can have a combination of the two

@ArjanSM

ArjanSM commented Sep 15, 2022

Copy link
Copy Markdown
Contributor Author

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

cortinico
cortinico reviewed Sep 17, 2022
View reviewed changes
Comment thread library/src/main/kotlin/com/chuckerteam/chucker/internal/data/entity/HttpTransaction.kt Outdated
Comment thread library/src/main/kotlin/com/chuckerteam/chucker/internal/support/RequestProcessor.kt Outdated
Comment thread library/src/main/kotlin/com/chuckerteam/chucker/internal/ui/transaction/TransactionAdapter.kt
@ArjanSM ArjanSM requested a review from cortinico September 19, 2022 22:47
cortinico
cortinico reviewed Sep 20, 2022
View reviewed changes

@cortinico cortinico left a comment

Copy link
Copy Markdown
Member

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

Comment thread library/src/main/kotlin/com/chuckerteam/chucker/internal/data/entity/HttpTransaction.kt Outdated
Comment thread library/src/main/kotlin/com/chuckerteam/chucker/internal/ui/transaction/TransactionAdapter.kt Outdated
Comment thread library/src/main/kotlin/com/chuckerteam/chucker/internal/ui/transaction/TransactionAdapter.kt Outdated
Comment thread sample/src/main/kotlin/com/chuckerteam/chucker/sample/GraphQlTask.kt Outdated
Comment thread sample/src/main/kotlin/com/chuckerteam/chucker/sample/GraphQlTask.kt Outdated
Comment thread sample/src/main/kotlin/com/chuckerteam/chucker/sample/GraphQlTask.kt Outdated
Comment thread sample/src/main/kotlin/com/chuckerteam/chucker/sample/GraphQlTask.kt Outdated
Comment thread sample/src/main/kotlin/com/chuckerteam/chucker/sample/GraphQlTask.kt Outdated
Comment thread sample/src/main/kotlin/com/chuckerteam/chucker/sample/GraphQlTask.kt Outdated
@ArjanSM ArjanSM requested a review from cortinico September 20, 2022 16:15
cortinico
cortinico reviewed Sep 20, 2022
View reviewed changes
Comment thread sample/src/main/kotlin/com/chuckerteam/chucker/sample/GraphQlTask.kt Outdated
@ArjanSM ArjanSM requested a review from cortinico September 20, 2022 18:07
cortinico
cortinico approved these changes Sep 23, 2022
View reviewed changes

@cortinico cortinico left a comment

Copy link
Copy Markdown
Member

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

ArjanSM commented Sep 23, 2022

Copy link
Copy Markdown
Contributor Author

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

Reviewers

@cortinico cortinico cortinico approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ArjanSM @cortinico