fix: don't truncate the sites dropdown to 10 entries - #258
Open
dowands wants to merge 1 commit into
Open
Conversation
The dropdown query spans the full 90d retention window but inherited getSitesOrderedByHits' default limit of 10, so deployments with more than 10 active sites silently lost the rest, with no pagination or indication in the UI. Pass an explicit limit for the dropdown.
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.
Overview
The dashboard's site dropdown is capped at 10 sites. On a deployment with more than 10 active sites, the rest are unreachable from the UI: there is no pagination and no indication that the list is truncated.
The intent is stated in the comment directly above the call site:
But
getSitesOrderedByHits(interval, limit?)doeslimit = limit || 10, and this call site doesn't pass one, so the implementation contradicts the stated intent.The default looks deliberate for the other caller: the no-
?site=redirect a few lines up only readssitesByHits[0], where a small limit is a reasonable way to keep that query cheap. The dropdown was added against the same helper and inherited a limit meant for a different use case.I ran into this on a self-hosted instance with 16 active siteIds where only 10 were reachable from the dropdown. Sites outside the top 10 by hit count can still be opened by editing
?site=in the URL, but the selector then renders blank, since the value isn't among its items.Changes by Package
@counterscale/cli
No changes.
@counterscale/server
app/routes/dashboard.tsx: pass an explicitMAX_SITES = 1000for the dropdown query.getSitesOrderedByHits' own default of 10 is left alone, so the redirect call site is unaffected.app/routes/__tests__/dashboard.test.tsx: regression test asserting the dropdown query is not capped at the default. It fails onmainwithexpected 10 to be greater than 10.@counterscale/tracker
No changes.
Other Changes
None.
Additional Notes
Verified locally:
pnpm test(174 passed),pnpm typecheck,pnpm build, plus prettier and eslint clean on the touched files.1000 is intended as "effectively unbounded for any realistic deployment" while staying well under Analytics Engine's own row cap. Happy to use a different number, or to drop the
LIMITfrom that query entirely if you would rather the dropdown be truly unbounded.