-
Notifications
You must be signed in to change notification settings - Fork 13
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: Add transaction message filter in GraphQL #20
feat: Add transaction message filter in GraphQL #20
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The feature added seems ok, but it is really difficult to review because we have on the same PR a refactoring and a feature.
I would suggest keeping this PR as a feature implementation and avoiding refactors on the code.
@ajnavarro, Your comments have given me some direction. |
1f17630
to
b11d85c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really good work you did here. Just a few more comments UX related, and performance-related.
Thank you very much!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM apart from a final request:
- Filter by params. Let me know what you think about that idea.
…-transaction-message-filter
I had been playing with the new GraphQL API and it is amazing the amount of info you can get, and how fast it is. I just added minor comments on schema parameters that are using uppercase moving them to lowercase for consistency. 🎉 |
Co-authored-by: Antonio Navarro Perez <antnavper@gmail.com>
Co-authored-by: Antonio Navarro Perez <antnavper@gmail.com>
Co-authored-by: Antonio Navarro Perez <antnavper@gmail.com>
Co-authored-by: Antonio Navarro Perez <antnavper@gmail.com>
…on-message-filter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've gone through @ajnavarro's comments after my own review, and have to say that this looks amazing 💯
Thank you for contributing back to the tool, it really means a lot to the team 🙏
Description
I added the data that is fetched by the query.
Then I decoded and processed the data in std.Tx and added filterable parameters.
Example Usage
Querying transaction messages
Filter transactio messages