Refactor testnet status update #1748
Conversation
Jennievon
requested review from
zkokelj,
otherview,
StefanIliev545 and
BedrockSquirrel
WalkthroughThe recent updates across the tenscan and walletextension frontend codebases focus on refining the handling and display of testnet status. The changes include a more specific return type for the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (5)
- tools/tenscan/frontend/api/general.ts (1 hunks)
- tools/tenscan/frontend/src/components/health-indicator.tsx (1 hunks)
- tools/tenscan/frontend/src/services/useGeneralService.ts (1 hunks)
- tools/tenscan/frontend/src/types/interfaces/index.ts (1 hunks)
- tools/walletextension/frontend/src/components/health-indicator.tsx (2 hunks)
Additional comments: 5
tools/tenscan/frontend/api/general.ts (1)
- 7-9: The return type of
fetchTestnetStatushas been correctly updated toResponseDataInterface<boolean>, which provides stricter type checking and should help prevent type-related bugs.tools/tenscan/frontend/src/components/health-indicator.tsx (2)
- 6-20: The logic for setting the
statusstate based ontestnetStatus.resultandtestnetStatus.errorsis clear and should work as intended. However, ensure that the error string "[p2p]" is a standardized and documented way of indicating a specific error condition.- 27-30: The badge variant and text are now correctly determined by the
statusstate, which should make the display of health status more responsive to changes.tools/tenscan/frontend/src/types/interfaces/index.ts (1)
- 60-60: The addition of an optional
errorsfield toResponseDataInterfaceis a good practice for error handling. Ensure that all parts of the application that use this interface are updated to handle this new field.tools/walletextension/frontend/src/components/health-indicator.tsx (1)
- 33-39: The logic for setting the
statusstate based onOverallHealthand errors containing "[p2p]" is consistent with the changes in the otherHealthIndicatorcomponent. Ensure that this logic is in line with the application's error handling strategy.
| const { data: testnetStatus, isLoading: isStatusLoading } = useQuery({ | ||
| queryKey: ["testnetStatus"], | ||
| queryFn: () => fetchTestnetStatus(), | ||
| // refetchInterval: noPolling ? false : pollingInterval, | ||
| refetchInterval: 10000, | ||
| }); | ||
|
|
||
| return { testnetStatus, isStatusLoading, setNoPolling, refetchTestnetStatus }; | ||
| return { testnetStatus, isStatusLoading }; |
There was a problem hiding this comment.
The refetchInterval is now set to a constant value of 10000, which standardizes the polling mechanism. Ensure that this interval is appropriate for the application's needs and does not lead to unnecessary network traffic or user interface updates.
| let isMounted = true; | ||
|
|
||
| // if overall health is true, status is set to true | ||
| // if overall health is false but if the error includes [p2p], status is set to true | ||
|
|
||
| testnetStatus().then((res) => { | ||
| if (isMounted.current) { | ||
| setStatus(res?.result); | ||
| if (isMounted) { | ||
| // setStatus(res?.result?.OverallHealth); | ||
| if (res?.result?.OverallHealth) { | ||
| setStatus(true); | ||
| } else if (res?.result?.Errors?.includes("[p2p]")) { | ||
| setStatus(true); | ||
| } else { | ||
| setStatus(false); | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| return () => { | ||
| isMounted.current = false; | ||
| isMounted = false; |
There was a problem hiding this comment.
The use of isMounted to guard against state updates on an unmounted component is a good practice to avoid memory leaks. However, consider using a more robust solution like a cleanup function or the AbortController API to cancel any in-flight requests when the component unmounts.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- tools/walletextension/frontend/src/components/health-indicator.tsx (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- tools/walletextension/frontend/src/components/health-indicator.tsx
Why this change is needed
Please provide a description and a link to the underlying ticket
https://github.com/ten-protocol/ten-internal/issues/2775
What changes were made as part of this PR
Please provide a high level list of the changes made
Update Testnet Status on Tenscan and the Gateway to be healthy, regardless of p2p errors
PR checks pre-merging
Please indicate below by ticking the checkbox that you have read and performed the required
PR checks