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

🔍 Filter transactions based on graphqlOperationName #920

Conversation

ArjanSM
Copy link
Contributor

@ArjanSM ArjanSM commented Nov 8, 2022

📷 Screenshots

📄 Context

issue#847

📝 Changes

  1. Added a clause to the getFilteredTuples in the HttpTransactionDao.kt which enables filtering on the basis of GraphQLOperationName.
    In order to avoid any breaking changes the pathQuery argument is utilized to filter on the basis of graphQLOperationName.
    I'm wondering if the argument name could be renamed to filterQuery?
  2. Tests in HttpTransactionDatabaseRepositoryTest.kt & HttpTransactionDaoTest.kt have been updated

📎 Related PR

🚫

🚫 Breaking

🚫

🛠️ How to test

  1. Launch Chucker Sample.
  2. Click on DO HTTP ACTIVITY & DO GRAPHQL ACTIVITY
  3. Click LAUNCH CHUCKER ACTIVITY
  4. Click on the Search icon. And type "sea"
    Chucker should filter the transactions and display a GraphQL transaction with the operation name as SearchCharacters

⏱️ Next steps

@ArjanSM ArjanSM changed the title Filter transaction based graphql operation name 🔍 Filter transactions based on graphqlOperationName Nov 8, 2022
@ArjanSM
Copy link
Contributor Author

ArjanSM commented Nov 16, 2022

@cortinico Apologies if you're busy! Any idea when would you be able to review this PR?

@cortinico
Copy link
Member

@cortinico Apologies if you're busy! Any idea when would you be able to review this PR?

I'll look into this over the weekend/next week 👍

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 sending this PR @ArjanSM
There are a couple of bits to fix and then we can merge it 👍

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.

Let's add a small comment and is good to go for me 👍

Copy link
Collaborator

@vbuberen vbuberen left a comment

Choose a reason for hiding this comment

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

Looks good to me as well. Thanks a lot for your contributions with GraphQL stuff!

@cortinico cortinico merged commit a16ed75 into ChuckerTeam:develop Dec 8, 2022
@ArjanSM ArjanSM deleted the filter_transaction_based_graphqlOperationName branch December 8, 2022 14:37
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