fix(watchlist): prevent phantom miners from leaking into watchlist via /miners/details with an unknown id#1157
Open
jack-stormentswe wants to merge 1 commit into
Conversation
…a /miners/details with an unknown id
Author
|
@anderdc Could you take a look when you have a moment? Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1159
Summary
The Miner Details page renders the "add to watchlist" star button as soon as a
githubIdis present in the URL - before the underlying miner data has been fetched. This makes it possible to pin a miner that doesn't actually exist (e.g. by manually visiting/miners/details?githubId=123456789), which leaks a phantom entry into the watchlist that can never be opened to anything but a "No data found" error page.It also produces a visible inconsistency in the watchlist itself: the sidebar/category badge counts every id in storage, but the miners table inner-joins against
useAllMiners()and silently drops anything that doesn't resolve - so the badge can read "Watchlist 2" while the visible list shows zero or one miner, with no way for the user to clean up the difference.The repository and PR/issue detail pages already gate their watchlist buttons behind a successful data fetch - only the miner details page was missing this guard.
Changes
src/pages/MinerDetailsPage.tsx- calluseMinerStats(githubId)at the page level and render<WatchlistButton>only after the fetch resolves with a real record. While loading, or when the miner is unknown, the star is hidden. (TanStack Query dedupes against the existing call insideMinerScoreCard, so this adds no extra network traffic.)src/pages/WatchlistPage.tsx-MinersListnow detects unresolved watched ids (entries that aren't in the all-miners cache once it's loaded) and surfaces them as a warning card with a one-click Remove unresolved action. The visible list is unchanged for resolved miners.Verification
/miners/details?githubId=123456789in an incognito window. While the request is in flight the star is absent; once the API responds with no record the star stays hidden and the page collapses to the existing "No data found" card. The star can no longer be clicked.localStorage.setItem('gittensor.watchlist.v2', JSON.stringify({ miners: ['999999'], repos: [], bounties: [], prs: [], issues: [] }))- then reload/watchlist. The sidebar badge reads "1", and the miners tab now shows a warning card explaining the entry couldn't be loaded with a Remove unresolved button that clears it. Clicking it removes the entry from localStorage and the badge updates to "0".useAllMiners()has finished loading, so a slow network won't cause a false positive on first paint.Recording
83.mp4