fix(routing): deep links and browser refresh load wrong festival data - #275
Merged
Conversation
Cold-loading a festival-scoped URL (browser refresh or shared link) failed to sync the provider to the festival named in the URL because _handlePostInitRedirect early-returned for valid festival IDs without calling setFestival, leaving the app on its default festival. Detail routes (drink/brewery/style/info) also had no redirect at all, so navigating to them while the provider held a different festival showed stale or missing data (e.g. "Drink Not Found" for valid shared links). Changes: - lib/main.dart: in _handlePostInitRedirect, call provider.setFestival when the URL's valid festival ID differs from the current one. - lib/router.dart: extract shared _festivalScopeRedirect helper and apply it to the /:festivalId route (refactor, no behaviour change) and add it to the four detail routes so hot navigation also triggers a switch. - lib/providers/beer_provider.dart: add _drinksLoadToken guard so that a concurrent loadDrinks() call that was started before a festival switch cannot overwrite the drinks from the newer setFestival() call. - test/router_test.dart: two regression tests that construct the router with a non-default initialLocation (true cold load), verifying the provider is synced to the URL festival after initialization. Fixes #266
Contributor
LCOV of commit
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses festival-scoped routing so cold-loaded and shared URLs sync BeerProvider to the festival in the URL, reducing stale or wrong-festival data on deep links.
Changes:
- Sync provider state from valid festival URLs after initialization.
- Add shared festival-scope redirect logic and apply it to several detail routes.
- Add regression tests for cold-loading non-default festival URLs and update tool lock metadata.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
lib/main.dart |
Syncs provider to the URL festival during post-initialization redirect handling. |
lib/router.dart |
Adds shared festival-scope redirect helper and applies it to home/detail routes. |
lib/providers/beer_provider.dart |
Adjusts drink-load token sequencing around loading state updates. |
test/router_test.dart |
Adds regression tests for cold-load festival/deep-link provider syncing. |
mise.lock |
Adds Flutter Linux platform lock entry. |
Comments suppressed due to low confidence (3)
lib/router.dart:126
- This detail route also calls
_festivalScopeRedirectwithoutonInvalidFestival, so an invalid brewery URL is not redirected. BecauseBreweryScreenreads from the current provider but uses the routefestivalIdfor navigation, users can see current-festival data under an invalid festival URL.
redirect: (context, state) => _festivalScopeRedirect(context, state),
lib/router.dart:138
- This detail route also omits
onInvalidFestival, so invalid style URLs are accepted instead of being redirected. The screen can then filter the current provider data while subsequent links are built with the invalidfestivalId.
redirect: (context, state) => _festivalScopeRedirect(context, state),
lib/router.dart:151
- This detail route also omits an invalid-festival handler, so
/invalid/infois allowed through andFestivalInfoScreendisplaysprovider.currentFestivalunder the invalid URL. It should redirect consistently with the festival home route instead of showing mismatched data.
redirect: (context, state) => _festivalScopeRedirect(context, state),
| // Detail routes - Provider initialized, but no navigation bar | ||
| GoRoute( | ||
| path: '/:festivalId/drink/:id', | ||
| redirect: (context, state) => _festivalScopeRedirect(context, state), |
| // Detail routes - Provider initialized, but no navigation bar | ||
| GoRoute( | ||
| path: '/:festivalId/drink/:id', | ||
| redirect: (context, state) => _festivalScopeRedirect(context, state), |
…to all detail routes The favorites route was missing the festival-scope redirect entirely, so hot navigation to a different festival's favorites URL left the provider on the wrong festival. Detail routes were calling _festivalScopeRedirect without onInvalidFestival, meaning invalid festival IDs silently passed through instead of being redirected to the current festival's equivalent path. Addresses review comments on PR #275.
Contributor
🚀 Cloudflare Pages PreviewYour preview deployment is ready! Preview URL: https://claude-address-issue-266-um6.staging-cambeerfestival.pages.dev This preview will be automatically updated when you push new commits to this PR. |
…l route builders Three new tests cover the paths flagged by codecov/patch: - Invalid festival ID on all detail + favorites routes redirects to the current festival's equivalent path (exercises onInvalidFestival lambdas) - Hot navigation to a different festival's favorites URL switches the provider - Brewery, style and info routes are reachable for a valid festival (exercises the route builders that were previously unreached)
Contributor
🚀 Cloudflare Pages PreviewYour preview deployment is ready! Preview URL: https://claude-address-issue-266-um6.staging-cambeerfestival.pages.dev This preview will be automatically updated when you push new commits to this PR. |
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.
Cold-loading a festival-scoped URL (browser refresh or shared link) failed
to sync the provider to the festival named in the URL because
_handlePostInitRedirect early-returned for valid festival IDs without
calling setFestival, leaving the app on its default festival.
Detail routes (drink/brewery/style/info) also had no redirect at all, so
navigating to them while the provider held a different festival showed
stale or missing data (e.g. "Drink Not Found" for valid shared links).
Changes:
when the URL's valid festival ID differs from the current one.
it to the /:festivalId route (refactor, no behaviour change) and add it
to the four detail routes so hot navigation also triggers a switch.
concurrent loadDrinks() call that was started before a festival switch
cannot overwrite the drinks from the newer setFestival() call.
with a non-default initialLocation (true cold load), verifying the
provider is synced to the URL festival after initialization.
Fixes #266