Improve transaction list for GraphQL requests by Canato · Pull Request #805 · ChuckerTeam/chucker

Canato · 2022-04-21T00:31:28Z

Close #69
Close #116

🎥 Video

SVID_20220421_011146_1.mp4

📷 Image

after iterative improvement

📄 Context

Based on #69, #116 and a little on #579 we have a desire for a more clean list when using GraphQL.
This helps to debug who uses Graphql without changing normal okhttp requests

📝 Changes

Retrieve request header for transaction list and display it if is a GraphQl request and have the operationName

🛠️ How to test

On the Sample code, there is a new button Do GraphQl Activity that can be pressed for testing.

⏱️ Next steps

Wanna be sure the code I add is following the library standards, open to suggestions!

Canato · 2022-04-21T00:35:33Z

Great library people, hope this is useful and we can release it soon
@cortinico @MiSikora @olivierperez @vbuberen

cortinico · 2022-05-02T20:23:55Z

Hey @Canato,
Have you seen the PR at #800?
I'd like to coordinate between the two to make sure we merge only one of the two approach.

Canato · 2022-05-03T07:37:09Z

Hey @Canato, Have you seen the PR at #800? I'd like to coordinate between the two to make sure we merge only one of the two approach.

Ohh @cortinico Thanks! I saw #70 because I come from the issue, but didn't spot #800 when I start to do these changes!

I'm happy to move with 800 if @alkber wanna fix the issues open there. I'm happy to close this one.

From another point of view, my implementation is a simple one and since the tests passed(if the code is valid) we could merge and use the 800 as an iterative improvement.
I was already planning to return with more requests/tests/icons, but I believe we should have small PRs and do them in small batches.

I see that my sample implementation differs from the other PR too because I'm using a real schema from the guides, would be interesting to know what will be the best solution.

That said, any of the solutions work for me if we can put them to work =D. Let me know @alkber

alkber · 2022-05-04T03:13:33Z

Hey @Canato, Have you seen the PR at #800? I'd like to coordinate between the two to make sure we merge only one of the two approach.

Ohh @cortinico Thanks! I saw #70 because I come from the issue, but didn't spot #800 when I start to do these changes!

I'm happy to move with 800 if @alkber wanna fix the issues open there. I'm happy to close this one.

From another point of view, my implementation is a simple one and since the tests passed(if the code is valid) we could merge and use the 800 as an iterative improvement. I was already planning to return with more requests/tests/icons, but I believe we should have small PRs and do them in small batches.

I see that my sample implementation differs from the other PR too because I'm using a real schema from the guides, would be interesting to know what will be the best solution.

That said, any of the solutions work for me if we can put them to work =D. Let me know @alkber

Hi @cortinico , isorry i was really busy with personal work and couldn't address the comments in PR #800. I believe @Canato approach is more generic and neat than mine as i read the code, and he has decorated the PR with more test cases too.. You may close PR #800 and use this one. However I have a small suggestion, to @Canato PR. If the UI item can be updated to the way PR #800 does then it would be much appreciated and visually appealing. @Canato can you take it up? you can reuse my work in your code.

Canato · 2022-05-04T06:53:58Z

If @cortinico is ok I can update the UI to use the graphql icon and add a new line ^^. Thanks @alkber

Canato · 2022-05-20T10:28:13Z

@cortinico news on this? =D

Canato · 2022-06-01T15:56:10Z

Add the GraphQL icon and new field for it.

cortinico

Great stuff! Thanks for doing this. Sorry for the late review 🙏
Let me know if you need some help integrating the changes I suggested

cortinico · 2022-06-03T13:23:20Z

Please add a #!/bin/bash on top

ArjanSM · 2022-07-05T21:52:54Z

Just curious, but wouldn't we need to include a check for this parameter in the fun hasTheSameContent()?

Good call, I've added it 👍

Canato · 2022-09-01T11:15:18Z

Any news on this? Worth to solve the conflicts ?

vbuberen · 2022-09-08T19:13:19Z

Any news on this? Worth to solve the conflicts ?

Yes, worth solving. Let's make it happen.

cortinico

I've rebased your changes @Canato and fixed the detekt failures 👍 This is good to merge on my end.

Canato · 2022-09-12T06:56:59Z

Thanks @cortinico how do we know when will be the next release or if this will be available in a snapshot?

cortinico · 2022-09-12T13:12:11Z

No sorry we don't have an ETA at the moment. I'm looking into wrapping up the work for a 4.x.
We probably want to include the #259 work which will take some time to do (as it hasn't started yet). In the meanwhile you can use the SNAPSHOT versions for 4.x

Canato · 2022-09-13T07:30:58Z

No issues, Thanks! =D

cortinico mentioned this pull request May 2, 2022

[feature/chucker-gql] GQL support against latest develop #800

Closed

cortinico requested changes Jun 3, 2022

View reviewed changes

Canato requested a review from cortinico June 6, 2022 12:17

ArjanSM reviewed Jul 5, 2022

View reviewed changes

Comment thread library/src/main/kotlin/com/chuckerteam/chucker/internal/data/entity/HttpTransaction.kt Outdated

ArjanSM reviewed Jul 5, 2022

View reviewed changes

This was referenced Jul 7, 2022

Update Graphql transaction list ArjanSM/chucker#1

Closed

Update transaction list to differentiate GraphQL requests #848

Closed

Canato and others added 11 commits September 10, 2022 16:20

Add operation name from graphql to main list title

8ec8fcc

Add graphql button in sample app

ce41ccc

Add apollo3 library

ed2b51b

Download apollo schema

947c057

Add graphql call

3b564bb

Update Changelog

f66484f

Improve graphql ui with new field and icon

083291a

Move from using request headers to specific parameter for graphql

982c017

Change graphql icon vector for better performance

3786319

Extend the hasTheSameContent function

395899d

Fix detetk failures

2af8feb

cortinico approved these changes Sep 10, 2022

View reviewed changes

cortinico merged commit 0e18ea6 into ChuckerTeam:develop Sep 10, 2022

ArjanSM mentioned this pull request Sep 14, 2022

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

Merged

This comment was marked as off-topic.

Sign in to view

Uh oh!

Conversation

Canato commented Apr 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎥 Video

📷 Image

📄 Context

📝 Changes

🛠️ How to test

⏱️ Next steps

Uh oh!

Canato commented Apr 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cortinico commented May 2, 2022

Uh oh!

Canato commented May 3, 2022

Uh oh!

alkber commented May 4, 2022

Uh oh!

Canato commented May 4, 2022

Uh oh!

Canato commented May 20, 2022

Uh oh!

Canato commented Jun 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cortinico left a comment

Choose a reason for hiding this comment

Uh oh!

cortinico Jun 3, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ArjanSM Jul 5, 2022

Choose a reason for hiding this comment

Uh oh!

cortinico Sep 10, 2022

Choose a reason for hiding this comment

Uh oh!

Canato commented Sep 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vbuberen commented Sep 8, 2022

Uh oh!

cortinico left a comment

Choose a reason for hiding this comment

Uh oh!

Canato commented Sep 12, 2022

Uh oh!

cortinico commented Sep 12, 2022

Uh oh!

Canato commented Sep 13, 2022

Uh oh!

This comment was marked as off-topic.

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Canato commented Apr 21, 2022 •

edited

Loading

Canato commented Apr 21, 2022 •

edited

Loading

Canato commented Jun 1, 2022 •

edited

Loading

Canato commented Sep 1, 2022 •

edited

Loading