Skip to content
This repository has been archived by the owner on Feb 2, 2024. It is now read-only.

[Batch Viewer] Avoid multiple requests to Tenderly API #128

Merged
merged 5 commits into from
Jun 22, 2022

Conversation

matextrem
Copy link
Contributor

@matextrem matextrem commented Jun 16, 2022

Summary

Closes #121

Updated Hook in order not to re-render it unnecessarily when there's no changes

To Test

  1. Go to a tx page in Batch Viewer view (i.e: /rinkeby/tx/0xaa5daa29e997cbf9b6307c707f13c76c3e2cb53ee41b8e85dc3e36e808cbbaf0/?tab=graph)
  2. Open network tab and search for tenderly requests
  • You will only see 2 requests (1 for getting trades and transfers and the other one for getting accounts).

@matextrem matextrem added Bug Something isn't working Protofire labels Jun 16, 2022
@matextrem matextrem self-assigned this Jun 16, 2022
@github-actions
Copy link

@coveralls
Copy link

coveralls commented Jun 16, 2022

Pull Request Test Coverage Report for Build 2511187032

Warning: This coverage report may be inaccurate.

We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
To ensure accuracy in future PRs, please see these guidelines.
A quick fix for this PR: rebase it; your next report should be accurate.

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 43.872%

Totals Coverage Status
Change from base Build 2508806191: 0.0%
Covered Lines: 2162
Relevant Lines: 4177

💛 - Coveralls

Copy link
Contributor

@henrypalacios henrypalacios left a comment

Choose a reason for hiding this comment

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

Nice catch!

@matextrem matextrem requested a review from ramirotw June 17, 2022 02:58
Copy link

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

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

LGTM

@alfetopito
Copy link
Collaborator

The behaviour described here is working nicely 👍

However, I think I stumbled upon an edge case, which might or might not be related to the changes here.

To reproduce:

  1. Load the PR url, on mainnet (the default) https://pr128--explorer.review.gnosisdev.com
  2. Search for the tx 0xaa5daa29e997cbf9b6307c707f13c76c3e2cb53ee41b8e85dc3e36e808cbbaf0
  3. Click on the graph tab
  • The chart is not rendered
    Screen Shot 2022-06-17 at 15 03 05

@elena-zh
Copy link

elena-zh commented Jun 17, 2022

To reproduce:

  1. Load the PR url, on mainnet (the default) https://pr128--explorer.review.gnosisdev.com
  2. Search for the tx 0xaa5daa29e997cbf9b6307c707f13c76c3e2cb53ee41b8e85dc3e36e808cbbaf0
  3. Click on the graph tab
  • The chart is not rendered
    Screen Shot 2022-06-17 at 15 03 05

Hey @alfetopito , I can reproduce this issue only when I open the app for the 1st time. Then, all charts are loaded nicely.

@alfetopito
Copy link
Collaborator

To reproduce:

  1. Load the PR url, on mainnet (the default) https://pr128--explorer.review.gnosisdev.com
  2. Search for the tx 0xaa5daa29e997cbf9b6307c707f13c76c3e2cb53ee41b8e85dc3e36e808cbbaf0
  3. Click on the graph tab
  • The chart is not rendered
    Screen Shot 2022-06-17 at 15 03 05

Hey @alfetopito , I can reproduce this issue only when I open the app for the 1st time. Then, all charts are loaded nicely.

Indeed, same as I observed.

@matextrem
Copy link
Contributor Author

@alfetopito It seems it is not related to this PR. The tenderly API is returning 404 for that txHash in Rinkeby (IDK why in consequent times it just works fine). I was not able to reproduce it locally.

Screen Shot 2022-06-17 at 21 44 25

@matextrem matextrem requested a review from alfetopito June 18, 2022 00:51
@elena-zh
Copy link

I'm not able to reproduce this issue today

src/hooks/useTxBatchTrades.tsx Outdated Show resolved Hide resolved
@matextrem matextrem requested a review from ramirotw June 22, 2022 15:29
@henrypalacios henrypalacios merged commit 7981c2f into develop Jun 22, 2022
@henrypalacios henrypalacios deleted the 121/tenderly-requests branch June 22, 2022 18:34
@henrypalacios henrypalacios mentioned this pull request Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Avoid making multiple request to Tenderly API
6 participants