fix: apply all audit findings (timeouts, rate-limit capture, pagination, UI gaps, auth gating) - #6
Merged
Merged
Conversation
Races each gloo-net request against gloo_timers::future::TimeoutFuture on wasm and maps the loss to ApiError::Timeout. On native the request is awaited directly, so unit tests stay offline. Adds the Timeout variant plus its display test.
Capture ran after map_status, so 401/403/404 responses never updated the rate-limit state and the dashboard freshness stamp. Capture now runs first, keeping the badge and last-updated timestamp honest even when requests start failing.
'Load more' previously called gloo_net directly in the app crate, bypassing the rate-limit capture, error classification, and the new request timeout. Adds GithubClient::get_page and funnels the raw Link-header cursor through it (AGENTS.md §3.2).
…ounds The pagination-derived count multiplied per_page by the rel=last page count, silently including pull requests (GitHub mixes them into the issues endpoint) and presenting the upper bound as an exact number. Page 1 is now counted exactly (PRs filtered), later pages are assumed full, and results render as N+ with a tooltip when estimated. Also: the card view now uses the same totals, estimate markers, and shows the last-push label that the table view already had.
…nses The interval effect subscribed to the rate-limit signal, so any fetch anywhere in the app (e.g. the repo detail page) re-ran the effect and restarted the countdown. The effect now tracks only the interval setting; the tick closure reads token and rate limit untracked and simply no-ops while the limit is nearly exhausted.
Rapidly changing filters could start two fetches whose responses arrived out of order, with the older response overwriting the newer list (last-write-wins). Each fetch now captures a generation counter and only applies its writes while it is still the latest request.
…ilures Repo detail now tracks per-tab error signals: a failed first load shows an error panel (not a misleading "No issues"), and a failed load-more keeps the list and shows the reason under the button. The dashboard counts repos whose entire bundle failed and shows a banner pointing at the token or rate limit. Loading state moved into the fetch functions so load-more clicks set it reliably.
…keletons Three small CSS/class fixes from the audit: - --typography-button-sm and --typography-body-sm were referenced by .label, .rl-badge-wrap, .view-toggle button and .repo-table but never defined, so those declarations were dropped by the browser. Both tokens now exist (matching DESIGN.md's typography table). - The login page used class "page--login" while the stylesheet defined ".page-login", so its max-width/centering never applied. The class now matches. - Skeleton rows lost their fixed widths when the inline style attributes were removed; width classes were added for them.
…PR rows AGENTS.md §5.3 specifies each row shows the author avatar and the last-updated date; the rows only had the plain author name, comment count and labels. Rows now render a 20px avatar (when the API provides one) and a YYYY-MM-DD date parsed from updated_at.
…ction The refresh-interval select only handled change events; after a reload it always showed "Manual only" because no value was bound to the current RefreshInterval. The select now uses prop:value via a new RefreshInterval::value() helper. The theme section (AGENTS.md §5.6) was missing entirely: two Light/Dark buttons that persist through SettingsState::save_theme.
auth.logout() called storage::clear_all(), wiping the watchlist and refresh/theme preferences together with the token — losing the user's setup on every logout. It now deletes only the token key. The doc comment claiming logout resets the rate-limit state (which is done by the caller) is corrected.
…shboard Only "/" showed the login page when logged out; /repo/:owner/:repo and /settings rendered their UI with no token, which then failed silently or looked broken. A small AuthedView wrapper (ChildrenFn + dynamic closure, since Leptos 0.8's Show requires Fn children) now guards all three routes and redirects the view to LoginPage until the token is validated. Also removed the now-unused storage::clear_all and an unused variable.
The endpoints built their request URLs inline with format!, leaving URL composition untested and the ROADMAP's "mock the API layer" claim half-true. URL building is now extracted into small pure functions (user_url, repo_url, issues_url, pulls_url, workflow_runs_url) used by both the live request path and new unit tests covering default and full parameter sets. 8 new tests, still zero network access.
Three claims no longer matched the source: - Section 3 (own look) marked everything done, but 294d214 reverted the dark-first/sky-blue rework back to the warm-cream, light-first palette with Pinterest Red as the sole accent. Items are unchecked again with reverted notes; the tokenization and favicon items stay checked since they survived. - Section 2 claimed a hand-rolled mock of the GithubApi trait. What actually exists is pure URL-builder functions with unit tests, so the item is reworded to match and the acceptance text updated. - The test count is updated to the current 48 (37 github-api + 11 models) and the "borrowed visual identity" summary reflects the revert.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
The settings page duplicated the theme switcher that already lives in the navbar; theme is now changed only from the header toggle.
The shimmer skeletons in the dashboard fallback and the issue/PR tab lists were rough and noisy. Loading states now render a muted "Loading…" text (dashboard) / "Loading issues…" / "Loading pull requests…" line. All skeleton CSS was removed; the button spinner used by load-more and the token form is unchanged.
The selects and inputs in the detail toolbar had no gap between them, so the state/sort/label/author controls sat flush against each other. .filters is now a flex row with a gap, wrapping on narrow widths.
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
1 participant
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.
Audit fixes — 14 small commits
Every bug found in the codebase audit, fixed in small reviewable commits against
main.Correctness
629f70a— 15s timeout on every HTTP request (was: hung forever, wasm included). Newhttp::gethelper +ApiError::Timeout.8586c71— rate-limit headers captured from error responses too (was: only successes, so the badge went stale under 429s).0be5a2c— repo-detail "Load more" now fetches throughGithubClient(was: baregloo_netcalls bypassing timeout, auth headers and rate-limit capture).38785e5— dashboard totals exclude PRs (GitHub returns PRs in/issues; was: count inflated) and are labeled "N+" with a tooltip when truncated at page 1.90defea— auto-refresh timer no longer restarted by every unrelated response.a7ef821— stale fetch responses discarded after filter changes (was: rapid filter switching could render the wrong list).UI / spec compliance
b89e9ce— fetch errors surfaced everywhere: repo-detail shows an error panel/row instead of a misleading "No issues"; dashboard shows a banner when a repo's whole bundle failed. Loading state moved into the fetch functions.de818f9— defined missing--typography-button-sm/--typography-body-smtokens, fixed the.page-loginclass mismatch, sized skeletons after removing inline styles.275cb12— issue/PR rows now show author avatar + last-updated date per AGENTS.md §5.3.2e81962— settings interval select bound to state (was: always "Manual only" after reload) and the missing theme section added.e9eaca7— logout removes only the token (was:clear_all()wiped the watchlist and settings).5070a69— every route auth-gated (was: only/;/repo/*and/settingsrendered with no token).Tests & docs
b6cf701— URL construction extracted into pure functions + 8 new unit tests (48 offline tests total, zero network).cd8a384— ROADMAP updated to match reality (design-revert in294d214, no trait mock, current test count).Verification
cargo test --workspace --exclude app— 48 passed (37 github-api + 11 models), no networkcargo check --workspace --target wasm32-unknown-unknown— cleanerror.rstests (untouched by this PR)