diff --git a/.gitignore b/.gitignore index 00b9ccf2..27355354 100644 --- a/.gitignore +++ b/.gitignore @@ -150,5 +150,6 @@ app.*.symbols !/dev/ci/**/Gemfile.lock # Keep manually created test mocks (build_runner has version compatibility issues) -!test/*.mocks.dartscreenshots/ +!test/*.mocks.dart +screenshots/ test/failures/ diff --git a/AGENTS.md b/AGENTS.md index d37e2fa0..26092ce1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -351,12 +351,12 @@ void main() { ### API Documentation -Full API documentation and JSON schemas are in `docs/api/`: +Full API documentation and JSON schemas are in `docs/code/api/`: -- **[docs/api/README.md](docs/api/README.md)** - Overview and quick reference -- **[docs/api/data-api-reference.md](docs/api/data-api-reference.md)** - Complete API reference -- **[docs/api/beer-list-schema.json](docs/api/beer-list-schema.json)** - JSON Schema for beverage data -- **[docs/api/festival-registry-schema.json](docs/api/festival-registry-schema.json)** - JSON Schema for festival config +- **[docs/code/api/README.md](docs/code/api/README.md)** - Overview and quick reference +- **[docs/code/api/data-api-reference.md](docs/code/api/data-api-reference.md)** - Complete API reference +- **[docs/code/api/beer-list-schema.json](docs/code/api/beer-list-schema.json)** - JSON Schema for beverage data +- **[docs/code/api/festival-registry-schema.json](docs/code/api/festival-registry-schema.json)** - JSON Schema for festival config ### Validating festivals.json diff --git a/README.md b/README.md index f268a5a8..2baaa0cc 100644 --- a/README.md +++ b/README.md @@ -128,10 +128,9 @@ Coverage fails if it drops below 70% overall, helping maintain code quality. ### E2E Testing -- **Web E2E Tests**: Playwright tests for Flutter web builds - [Testing Flutter Web Guide](docs/tooling/flutter-web-testing.md) -- **Mobile E2E Tests**: Patrol + Firebase Test Lab integration (planned) - [Testing Plan](docs/planning/patrol-firebase-testing/plan.md) +- **Web E2E Tests**: Playwright tests for URL routing and accessibility smoke tests - [Testing Flutter Web Guide](docs/tooling/flutter-web-testing.md) -See the [Patrol Firebase Testing Summary](docs/planning/patrol-firebase-testing/summary.md) for implementation status and plan overview. +See [ADR 0005](docs/adr/0005-e2e-testing-strategy.md) for the rationale behind this approach. ## Data API @@ -159,8 +158,6 @@ Technical documentation is available in the [docs](docs/) directory - see [docs/ ### Testing & Quality - [Testing Flutter Web](docs/tooling/flutter-web-testing.md) - E2E testing with Playwright -- [Patrol Firebase Testing Plan](docs/planning/patrol-firebase-testing/plan.md) - Mobile E2E testing strategy -- [Patrol Testing Summary](docs/planning/patrol-firebase-testing/summary.md) - Quick overview and status ### Architecture & Deployment - [URL Routing](docs/code/routing.md) - Path-based routing implementation diff --git a/docs/README.md b/docs/README.md index 1f796e97..422fc61c 100644 --- a/docs/README.md +++ b/docs/README.md @@ -4,14 +4,16 @@ Organized documentation for the Cambridge Beer Festival app. ## 📁 Structure -The documentation is organized into four main categories: - ### 📘 code/ - Current Code Documentation -Documentation of how the current codebase works - implementation guides, architecture decisions, and technical references. +How the codebase works -- implementation guides, architecture, and technical references. -- **[accessibility.md](code/accessibility.md)** - Accessibility implementation guide (WCAG 2.1 Level AA compliance) -- **[routing.md](code/routing.md)** - URL routing strategy (path-based routing with go_router) +- **[accessibility.md](code/accessibility.md)** - Accessibility implementation (WCAG 2.1 Level AA) +- **[domain-architecture.md](code/domain-architecture.md)** - Domain layer architecture (filter/sort services, repositories) +- **[routing.md](code/routing.md)** - URL routing (path-based with GoRouter) +- **[navigation.md](code/navigation.md)** - Navigation helper API reference +- **[widget-standards.md](code/widget-standards.md)** - Widget patterns and standards +- **[ui-components.md](code/ui-components.md)** - Shared UI components (OverflowMenu, BreadcrumbBar) - **[network.md](code/network.md)** - Network security configuration and allowlist - **[api/](code/api/)** - API documentation - [README.md](code/api/README.md) - API overview @@ -19,82 +21,49 @@ Documentation of how the current codebase works - implementation guides, archite - [beer-list-schema.json](code/api/beer-list-schema.json) - JSON Schema for beverage data - [festival-registry-schema.json](code/api/festival-registry-schema.json) - JSON Schema for festival config -### 🔄 processes/ - Development & Operational Processes +### 📐 adr/ - Architecture Decision Records -Documented processes for development workflows, CI/CD, and handling contributions. +Key decisions with context, alternatives considered, and consequences. + +- **[0001](adr/0001-github-actions-caching-strategy.md)** - GitHub Actions Caching Strategy +- **[0002](adr/0002-composite-actions-and-test-deduplication.md)** - Composite Actions and Test Deduplication +- **[0003](adr/0003-parallel-build-strategy.md)** - Parallel Build Strategy for Android Releases +- **[0004](adr/0004-path-based-url-strategy.md)** - Path-Based URL Strategy for Deep Linking +- **[0005](adr/0005-e2e-testing-strategy.md)** - E2E Testing Strategy (Playwright for URL smoke tests) + +### 🔄 processes/ - Development & Operational Processes - **[development.md](processes/development.md)** - Development workflow and best practices -- **[ci-cd.md](processes/ci-cd.md)** - Continuous integration and deployment processes +- **[ci-cd.md](processes/ci-cd.md)** - CI/CD workflows and pipeline - **[festival-data-prs.md](processes/festival-data-prs.md)** - FAQ for handling festival data pull requests +- **[safe-cache-strategy.md](processes/safe-cache-strategy.md)** - What to cache (and avoid) in GitHub Actions ### 🛠️ tooling/ - Setup & Configuration Guides -Step-by-step guides for setting up and configuring development tools, build systems, and deployment platforms. - - **[android-debug.md](tooling/android-debug.md)** - Android debug build configuration - **[android-release.md](tooling/android-release.md)** - Android release build process - **[firebase.md](tooling/firebase.md)** - Firebase setup and configuration - **[cloudflare-pages.md](tooling/cloudflare-pages.md)** - Cloudflare Pages deployment setup - **[github-secrets.md](tooling/github-secrets.md)** - GitHub secrets management -- **[flutter-web-testing.md](tooling/flutter-web-testing.md)** - Flutter web testing setup (Playwright E2E) +- **[flutter-web-testing.md](tooling/flutter-web-testing.md)** - Flutter web testing (Playwright E2E) - **[play-store.md](tooling/play-store.md)** - Play Store metadata and publishing -### 📋 planning/ - Design Docs, Proposals & Reviews - -Planning documents, architecture decision records, design reviews, and future enhancement proposals. **These are historical records or proposals, not current implementation.** - -#### Deep Linking Implementation - -Complete planning documentation for the deep linking feature: - -- **[deep-linking/design.md](planning/deep-linking/design.md)** - Initial design proposal -- **[deep-linking/design-review.md](planning/deep-linking/design-review.md)** - Design review and feedback -- **[deep-linking/implementation-plan.md](planning/deep-linking/implementation-plan.md)** - Step-by-step implementation plan (Phases 1-2) -- **[deep-linking/phase-0-guide.md](planning/deep-linking/phase-0-guide.md)** - Detailed Phase 0 guide with complete code, tests, and success criteria -- **[deep-linking/testing-strategy.md](planning/deep-linking/testing-strategy.md)** - Testing approach and E2E test plan -- **[deep-linking/architecture-readonly-urls.md](planning/deep-linking/architecture-readonly-urls.md)** - ADR for path-based URLs - -**Status**: 🚧 In Progress - Phase 0-2 planned, implementation starting - -#### Festival Log / "My Festival" Feature - -Planning documentation for transforming favorites into a comprehensive tasting tracker: - -- **[festival-log/design.md](planning/festival-log/design.md)** - Feature overview, data model, UX decisions, and design alternatives -- **[festival-log/implementation-plan.md](planning/festival-log/implementation-plan.md)** - Step-by-step implementation plan (Phases 3-4) +### 📋 planning/ - Active Proposals -**Status**: 💡 Proposal - Depends on deep linking (Phases 0-2) completion +- **[festival-log/](planning/festival-log/)** - Festival tasting log feature (proposal, not yet implemented) +- **[ux-improvements.md](planning/ux-improvements.md)** - UX enhancements (8 of 30 implemented, 7 more recommended) -**Key features:** -- Track drinks as "Want to Try" or "Tasted" -- Multiple tasting timestamps per drink -- Festival-scoped data (separate logs per festival) -- Visual status badges on drink cards -- Clean data model from day 1 (pre-release advantage) +### 🗄️ planning/archive/ - Historical Records -**Prerequisites**: Deep linking must be implemented first (festival-scoped data structure) +Completed or superseded planning documents retained for context: -**Pre-release context**: No migration needed - implementing optimal structure from the start +- **[archive/deep-linking/](planning/archive/deep-linking/)** - Deep linking design, implementation plans, and reviews (Phase 1 complete, decisions captured in ADR 0004) +- **[archive/patrol-firebase-testing/](planning/archive/patrol-firebase-testing/)** - Patrol + Firebase Test Lab evaluation (not implemented, decision captured in ADR 0005) +- **[archive/ci-review/](planning/archive/ci-review/)** - CI/CD review and optimisations (implemented, decisions captured in ADRs 0001-0003) -#### Patrol Firebase Testing +### 📋 Project Tracking -Exploration of Patrol testing framework with Firebase Test Lab (not implemented): - -- **[patrol-firebase-testing/plan.md](planning/patrol-firebase-testing/plan.md)** - Testing plan proposal -- **[patrol-firebase-testing/review.md](planning/patrol-firebase-testing/review.md)** - Review and evaluation -- **[patrol-firebase-testing/summary.md](planning/patrol-firebase-testing/summary.md)** - Summary and decision - -**Status**: ❌ Not Implemented - Decided against Patrol, using Playwright for E2E testing instead - -**Alternative**: See [tooling/flutter-web-testing.md](tooling/flutter-web-testing.md) for current Playwright-based E2E testing approach - -#### Other Planning Documents - -- **[readme-review.md](planning/readme-review.md)** - Review and improvement proposals for README -- **[ux-improvements.md](planning/ux-improvements.md)** - Comprehensive UX enhancement proposals - - **Status**: Partially implemented (8 of 30 features completed as of Dec 2025) - - **Recommendations**: Phase 1-3 high-value features (~33 hours), remaining 22 features not recommended - - See document for implementation status and value assessment +- **[todos.md](todos.md)** - Active TODO list with bugs, improvements, and project status ## 🔍 Quick Reference @@ -104,143 +73,51 @@ Exploration of Patrol testing framework with Firebase Test Lab (not implemented) - How accessibility works → [code/accessibility.md](code/accessibility.md) - How routing works → [code/routing.md](code/routing.md) - How the API works → [code/api/data-api-reference.md](code/api/data-api-reference.md) +- Navigation helpers → [code/navigation.md](code/navigation.md) +- Shared UI components → [code/ui-components.md](code/ui-components.md) + +**Understand a past decision:** +- Why path-based URLs? → [ADR 0004](adr/0004-path-based-url-strategy.md) +- Why Playwright for E2E? → [ADR 0005](adr/0005-e2e-testing-strategy.md) +- Why cache pub/npm but not build artifacts? → [ADR 0001](adr/0001-github-actions-caching-strategy.md) **Set up development tools:** -- Set up Android builds → [tooling/android-debug.md](tooling/android-debug.md) or [tooling/android-release.md](tooling/android-release.md) -- Set up Firebase → [tooling/firebase.md](tooling/firebase.md) -- Set up E2E testing → [tooling/flutter-web-testing.md](tooling/flutter-web-testing.md) -- Deploy to Cloudflare → [tooling/cloudflare-pages.md](tooling/cloudflare-pages.md) +- Android builds → [tooling/android-debug.md](tooling/android-debug.md) or [tooling/android-release.md](tooling/android-release.md) +- Firebase → [tooling/firebase.md](tooling/firebase.md) +- E2E testing → [tooling/flutter-web-testing.md](tooling/flutter-web-testing.md) +- Cloudflare deployment → [tooling/cloudflare-pages.md](tooling/cloudflare-pages.md) **Follow a process:** - Contribute code → [processes/development.md](processes/development.md) - Understand CI/CD → [processes/ci-cd.md](processes/ci-cd.md) - Handle festival data PRs → [processes/festival-data-prs.md](processes/festival-data-prs.md) -**Review a design decision:** -- Why path-based URLs? → [planning/deep-linking/architecture-readonly-urls.md](planning/deep-linking/architecture-readonly-urls.md) -- Why not Patrol testing? → [planning/patrol-firebase-testing/summary.md](planning/patrol-firebase-testing/summary.md) - **Plan future work:** -- Deep linking implementation → [planning/deep-linking/phase-0-guide.md](planning/deep-linking/phase-0-guide.md) (ready to start) -- Festival log feature → [planning/festival-log/design.md](planning/festival-log/design.md) (after deep linking) -- UX improvements → [planning/ux-improvements.md](planning/ux-improvements.md) (prioritized recommendations) - -## 📝 Documentation Status - -### Current Implementation Docs (✅ Up to Date) - -These docs reflect the current state of the codebase: - -- [code/accessibility.md](code/accessibility.md) -- [code/routing.md](code/routing.md) -- [code/network.md](code/network.md) -- [code/api/](code/api/) - -### Process Docs (✅ Up to Date) - -These docs describe active processes: - -- [processes/development.md](processes/development.md) -- [processes/ci-cd.md](processes/ci-cd.md) -- [processes/festival-data-prs.md](processes/festival-data-prs.md) - -### Tooling Docs (✅ Up to Date) - -These guides are current and accurate: - -- All docs in [tooling/](tooling/) - -### Planning Docs (⚠️ Historical/Proposals) - -These docs are **not current implementation** - they are historical records or future proposals: - -- 🚧 **In Progress**: [planning/deep-linking/](planning/deep-linking/) - Phases 0-2 planned and ready to implement -- 💡 **Proposal**: [planning/festival-log/](planning/festival-log/) - Phases 3-4 planned, depends on deep linking completion -- ❌ **Not Implemented**: [planning/patrol-firebase-testing/](planning/patrol-firebase-testing/) - See [tooling/flutter-web-testing.md](tooling/flutter-web-testing.md) for alternative -- ⏳ **Partially Implemented**: [planning/ux-improvements.md](planning/ux-improvements.md) - 8 of 30 features completed, document includes implementation status and prioritized recommendations - -## 🔄 Alternatives & Conflicting Approaches - -### Testing Strategy - -**Conflicting Plans**: - -1. **Patrol + Firebase Test Lab** ([planning/patrol-firebase-testing/](planning/patrol-firebase-testing/)) - ❌ Not Implemented - - Pros: Native Flutter testing, realistic device conditions - - Cons: Complex setup, Firebase Test Lab costs, limited to mobile platforms - -2. **Playwright E2E** ([tooling/flutter-web-testing.md](tooling/flutter-web-testing.md)) - ✅ Currently Used - - Pros: Simple setup, free, works for web platform, familiar to web developers - - Cons: Web-only, doesn't test native mobile features - - **Decision**: Chosen for simplicity and web-first approach - -**Recommendation**: Continue with Playwright for web E2E. Consider Patrol only if extensive native mobile testing becomes necessary. - -### URL Routing Approaches - -**Considered Approaches** (documented in [planning/deep-linking/architecture-readonly-urls.md](planning/deep-linking/architecture-readonly-urls.md)): - -1. **Hash-based URLs** (`/#/drink/123`) - ❌ Rejected - - Simple, no server config needed - - Poor SEO, unprofessional appearance - -2. **Path-based URLs** (`/drink/123`) - ✅ Implemented - - Better SEO, clean URLs, shareable links - - Requires server-side routing config (SPA fallback) - - **Decision**: Chosen for better UX and SEO - -**Current Implementation**: See [code/routing.md](code/routing.md) - -### UX Enhancement Priorities - -**Multiple Phases Proposed** (documented in [planning/ux-improvements.md](planning/ux-improvements.md)): - -The original document proposed 30 UX improvements across 4 phases. After implementation review and value assessment: - -- ✅ **Phase 1 (Implemented)**: 8 features - Similar drinks, visual variety, filtering, search, favorites, ratings, themes -- 🟢 **Phase 1 Recommended**: 4 features (14 hours) - Bar location, allergen warnings, result count, ABV filters -- 🟢 **Phase 2 Recommended**: 1 feature (12 hours) - "Tried" vs "Want to Try" tracking -- 🟡 **Phase 3 Optional**: 2 features (7 hours) - Filter count badge, clear all filters -- 🔴 **Not Recommended**: 22 features (~236 hours) - Comparison mode, A-Z navigation, social features, etc. - -**Decision**: Focus on Phases 1-3 only (33 hours total). Remaining features don't align with festival app needs or have poor effort/value ratios. - -**Conflicting Approaches**: -- Original plan: Implement all 30 features (~269 hours) -- Current recommendation: Implement 7 high-value features (33 hours), focus remaining effort on data quality and stability - -See [planning/ux-improvements.md](planning/ux-improvements.md) for detailed implementation status and value assessment. +- Festival log feature → [planning/festival-log/design.md](planning/festival-log/design.md) +- UX improvements → [planning/ux-improvements.md](planning/ux-improvements.md) +- Bugs and TODOs → [todos.md](todos.md) ## 🤝 Contributing to Docs -When adding or updating documentation: - 1. **Choose the right category:** - Documenting existing code? → `code/` + - Recording a decision? → `adr/` (use next sequential number) - Describing a process? → `processes/` - Writing a setup guide? → `tooling/` - Proposing future work? → `planning/` 2. **Use clear, descriptive filenames** (lowercase, kebab-case) -3. **Update this README** if you add new documents - -4. **Mark planning docs with status:** - - ✅ Implemented (link to current implementation doc) - - ❌ Not Implemented (explain why, link to alternative if applicable) - - ⏳ Partially Implemented (describe current status) - - 🚧 In Progress - - 💡 Proposal +3. **Update this README** when adding new documents -5. **Document alternatives** when multiple approaches exist (add to "Alternatives & Conflicting Approaches" section) +4. **Completed planning docs** should be archived to `planning/archive/` with decisions extracted into ADRs ## 📚 Related Documentation -- **[../CLAUDE.md](../CLAUDE.md)** - Instructions for Claude AI (references these docs) -- **[../AGENTS.md](../AGENTS.md)** - Complete guide for AI agents using this repository -- **[../README.md](../README.md)** - Project overview and quick start guide +- **[../CLAUDE.md](../CLAUDE.md)** - Instructions for Claude AI +- **[../AGENTS.md](../AGENTS.md)** - Guide for AI agents +- **[../README.md](../README.md)** - Project overview --- -**Last Updated**: December 2025 -**Maintainers**: Development Team +**Last Updated**: February 2026 diff --git a/docs/adr/0004-path-based-url-strategy.md b/docs/adr/0004-path-based-url-strategy.md new file mode 100644 index 00000000..35b38605 --- /dev/null +++ b/docs/adr/0004-path-based-url-strategy.md @@ -0,0 +1,79 @@ +# ADR 0004: Path-Based URL Strategy for Deep Linking + +**Status**: Accepted + +**Date**: 2025-12-21 + +**Deciders**: Engineering Team + +**Context**: The app needed shareable, bookmarkable URLs for festival drinks, breweries, and styles. Two approaches were considered: hash-based URLs (`/#/drink/123`) and path-based URLs (`/drink/123`). The app was pre-release with no existing shared URLs or search engine indexing, so there were no backward-compatibility constraints. + +--- + +## Decision + +We adopted **festival-scoped, path-based URLs** with GoRouter and `usePathUrlStrategy()`. + +### URL Structure + +``` +/{festivalId} → Festival home (drinks list) +/{festivalId}/favorites → Favorites for this festival +/{festivalId}/drink/{drinkId} → Drink detail +/{festivalId}/brewery/{id} → Brewery detail +/{festivalId}/style/{styleName} → Style detail (lowercase canonical) +/{festivalId}/info → Festival info +/about → About (global, not festival-scoped) +``` + +### Key Design Choices + +1. **Festival ID as URL root** -- every drink/brewery/style URL is scoped to a festival, enabling cross-festival deep links +2. **Lowercase canonical style URLs** -- `buildStylePath()` lowercases style names for consistent URLs +3. **URL encoding** -- all user-provided IDs are encoded via `Uri.encodeComponent()` +4. **Pre-release advantage** -- no redirect logic or legacy URL support needed + +--- + +## Alternatives Considered + +### Hash-Based URLs (`/#/drink/123`) + +- Simpler: no server-side SPA routing config needed +- Rejected because: poor SEO, unprofessional appearance, not shareable on social media + +### Flat URLs without festival scoping (`/drink/123`) + +- Simpler routing, fewer path segments +- Rejected because: can't distinguish same drink ID across different festivals; can't share a link to "this year's festival" + +--- + +## Consequences + +### Positive + +- Clean, shareable URLs that work on social media +- Festival context is always visible in the URL +- Browser back/forward works correctly +- Bookmarks and shared links are self-contained + +### Negative + +- Requires SPA fallback routing on the server (Cloudflare Pages `_redirects` or `--proxy` flag on http-server) +- Detail routes currently lack festival ID validation (documented as known limitation, see todos.md H3) +- Festival selector UI doesn't update the URL when switching festivals (see todos.md C3) + +--- + +## Implementation + +- **Router**: `lib/router.dart` (GoRouter configuration) +- **URL builders**: `lib/utils/navigation_helpers.dart` +- **E2E tests**: `test-e2e/routing.spec.ts` (Playwright URL smoke tests) +- **Server config**: `--proxy` flag on http-server for SPA fallback + +## Related Documents + +- `docs/code/routing.md` -- current routing implementation details +- `docs/code/navigation.md` -- navigation helper API reference diff --git a/docs/adr/0005-e2e-testing-strategy.md b/docs/adr/0005-e2e-testing-strategy.md new file mode 100644 index 00000000..1d87a26f --- /dev/null +++ b/docs/adr/0005-e2e-testing-strategy.md @@ -0,0 +1,88 @@ +# ADR 0005: E2E Testing Strategy -- Playwright for URL Smoke Tests + +**Status**: Accepted + +**Date**: 2025-12-21 + +**Deciders**: Engineering Team + +**Context**: The app needed automated end-to-end testing to validate routing and deep linking. Two approaches were evaluated: Patrol with Firebase Test Lab (native Flutter E2E on real Android devices) and Playwright (browser-based testing). Flutter web renders to a `` element, which makes traditional DOM-based testing largely ineffective for UI interactions. + +--- + +## Decision + +We adopted **Playwright for URL and routing smoke tests only**, and deferred native E2E testing. + +Playwright tests verify: +- URL routing works (correct URLs after navigation) +- Browser back/forward/refresh preserves routes +- No critical console errors on page load +- Basic ARIA label presence (accessibility smoke test) + +Playwright tests **do not** verify: +- Visual appearance, layout, or rendered text +- Widget interactions (tapping buttons, filling forms) +- User flows (search, filter, favorite) +- Canvas-rendered content + +Widget interactions are covered by Flutter's own `testWidgets` framework in `test/`. + +--- + +## Alternatives Considered + +### Patrol + Firebase Test Lab + +A detailed plan was created (see `docs/planning/archive/patrol-firebase-testing/`) proposing: +- Native Flutter E2E tests using the Patrol framework +- Execution on real Android devices via Firebase Test Lab free tier (15 tests/day) +- 4-5 week implementation timeline across 5 phases + +**Why it was not implemented:** +- Significant setup complexity (Firebase Test Lab, GCP service accounts, Android instrumentation builds) +- 4-5 week implementation investment for a pre-release app +- Free tier limit (15 tests/day) constrains CI usage +- Flutter widget tests already cover interaction flows effectively +- The immediate need was validating URL routing for the deep linking feature, not full native E2E + +**When to reconsider:** +- If the app ships on Android/iOS and needs device-specific testing (permissions, system dialogs, push notifications) +- If visual regression testing becomes important +- If Flutter widget tests prove insufficient for catching real-world bugs + +### Flutter Integration Tests + +Flutter's built-in `integration_test` package was considered but not prioritised. It would run the full app in a test harness and can interact with widgets directly. This remains a valid option for future investment (tracked in todos.md item #1). + +--- + +## Consequences + +### Positive + +- Fast to implement (2 test files, ~440 lines) +- Validates the most critical web concern: URL routing works correctly +- Runs in CI without special infrastructure +- ARIA label checks enforce accessibility as a side effect +- No ongoing cost or quota limits + +### Negative + +- Cannot test actual user flows through the UI +- Cannot verify that the correct screen renders for a given URL +- Flutter canvas rendering means Playwright can never do meaningful UI testing for this app +- Gap between "URL works" and "screen works" -- a route could return 200 but render an error state + +--- + +## Implementation + +- **Config**: `playwright.config.ts` +- **Tests**: `test-e2e/app.spec.ts` (loading, console errors, ARIA), `test-e2e/routing.spec.ts` (URL routing, browser history) +- **Approach doc**: `docs/tooling/flutter-web-testing.md` + +## Related Documents + +- `docs/tooling/flutter-web-testing.md` -- how Playwright works with Flutter's canvas renderer +- `docs/planning/archive/patrol-firebase-testing/` -- the Patrol evaluation that was not implemented diff --git a/docs/adr/README.md b/docs/adr/README.md index 57bb1d6d..920c18ec 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -30,6 +30,8 @@ Each ADR follows this structure: | [0001](0001-github-actions-caching-strategy.md) | GitHub Actions Caching Strategy | Accepted | 2025-12-27 | | [0002](0002-composite-actions-and-test-deduplication.md) | Composite Actions and Test Deduplication | Accepted | 2025-12-27 | | [0003](0003-parallel-build-strategy.md) | Parallel Build Strategy for Android Releases | Accepted | 2025-12-27 | +| [0004](0004-path-based-url-strategy.md) | Path-Based URL Strategy for Deep Linking | Accepted | 2025-12-21 | +| [0005](0005-e2e-testing-strategy.md) | E2E Testing Strategy (Playwright for URL Smoke Tests) | Accepted | 2025-12-21 | ## Creating a New ADR diff --git a/docs/code/accessibility.md b/docs/code/accessibility.md index fe6c162c..e0db448c 100644 --- a/docs/code/accessibility.md +++ b/docs/code/accessibility.md @@ -68,29 +68,37 @@ Comprehensive accessibility guidelines for the Cambridge Beer Festival app. ## Implementation Status -### Current Status: ❌ NOT IMPLEMENTED - -**No accessibility features currently exist in the codebase.** - -Zero `Semantics` widgets found in: -- ❌ `lib/widgets/drink_card.dart` -- ❌ `lib/screens/drinks_screen.dart` -- ❌ `lib/screens/festival_info_screen.dart` -- ❌ `lib/main.dart` -- ❌ `lib/widgets/star_rating.dart` - -**Impact:** App is currently unusable for screen reader users. - -### What Needs Implementation - -See [todos.md](../todos.md) item #6 for full details: -- Favorite buttons need labels -- Filter chips need state announcements -- Navigation bar needs descriptive labels -- Search interface needs proper semantics -- Drink cards need summaries -- Star ratings need value announcements -- Action buttons need clear descriptions +### Current Status: ✅ Implemented + +**53+ `Semantics` widgets** are implemented across the app, with **9 dedicated accessibility tests** in `test/accessibility_test.dart`. + +Coverage by file: +- ✅ `lib/widgets/drink_card.dart` -- card semantic labels, favorite button semantics +- ✅ `lib/screens/drinks_screen.dart` -- search clear button, filter chips +- ✅ `lib/screens/festival_info_screen.dart` -- map, website, and GitHub buttons +- ✅ `lib/main.dart` -- bottom navigation bar with descriptive labels for both tabs +- ✅ `lib/widgets/star_rating.dart` -- parent rating label, individual star semantics +- ✅ `lib/widgets/bottom_action_bar.dart` -- action button semantics +- ✅ `lib/widgets/breadcrumb_bar.dart` -- back navigation semantics +- ✅ `lib/widgets/overflow_menu.dart` -- menu button with `ExcludeSemantics` on decorative icons +- ✅ `lib/widgets/info_chip.dart` -- chip semantics +- ✅ `lib/widgets/festival_menu_sheets.dart` -- festival selector, settings, theme selector +- ✅ `lib/widgets/environment_badge.dart` -- environment indicator semantics +- ✅ `lib/screens/about_screen.dart` -- theme, GitHub, issues, licenses buttons +- ✅ `lib/screens/drink_detail_screen.dart` -- action buttons, brewery link + +### Automated Tests + +`test/accessibility_test.dart` verifies: +- Favorite button semantic labels (add/remove states) +- ABV chip `ExcludeSemantics` for decorative elements +- Card semantic structure and labels +- Environment badge semantics +- Button property (`button: true`) on interactive elements +- Hint instructions for screen reader users +- `ExcludeSemantics` usage on decorative icons +- Filter chip selection state announcements +- Retry button semantics on error states --- diff --git a/docs/navigation.md b/docs/code/navigation.md similarity index 100% rename from docs/navigation.md rename to docs/code/navigation.md diff --git a/docs/ui-components.md b/docs/code/ui-components.md similarity index 100% rename from docs/ui-components.md rename to docs/code/ui-components.md diff --git a/CI_IMPROVEMENTS_SUMMARY.md b/docs/planning/archive/ci-review/ci-improvements-summary.md similarity index 96% rename from CI_IMPROVEMENTS_SUMMARY.md rename to docs/planning/archive/ci-review/ci-improvements-summary.md index edc57b3d..f8775991 100644 --- a/CI_IMPROVEMENTS_SUMMARY.md +++ b/docs/planning/archive/ci-review/ci-improvements-summary.md @@ -340,11 +340,11 @@ Settings → Actions → Caches → Delete all ## 📎 Links -- [ADR 0001: Caching Strategy](docs/adr/0001-github-actions-caching-strategy.md) -- [ADR 0002: Composite Actions](docs/adr/0002-composite-actions-and-test-deduplication.md) -- [Safe Cache Strategy Guide](SAFE_CACHE_STRATEGY.md) -- [Complete CI Review](CI_REVIEW.md) -- [Naming Recommendations](CI_NAMING_RECOMMENDATIONS.md) +- [ADR 0001: Caching Strategy](../adr/0001-github-actions-caching-strategy.md) +- [ADR 0002: Composite Actions](../adr/0002-composite-actions-and-test-deduplication.md) +- [Safe Cache Strategy Guide](safe-cache-strategy.md) +- [Complete CI Review](ci-review.md) +- [Naming Recommendations](ci-naming-recommendations.md) --- diff --git a/CI_NAMING_RECOMMENDATIONS.md b/docs/planning/archive/ci-review/ci-naming-recommendations.md similarity index 100% rename from CI_NAMING_RECOMMENDATIONS.md rename to docs/planning/archive/ci-review/ci-naming-recommendations.md diff --git a/CI_REVIEW.md b/docs/planning/archive/ci-review/ci-review.md similarity index 100% rename from CI_REVIEW.md rename to docs/planning/archive/ci-review/ci-review.md diff --git a/docs/planning/deep-linking/PHASE-1-COMPLETE.md b/docs/planning/archive/deep-linking/PHASE-1-COMPLETE.md similarity index 100% rename from docs/planning/deep-linking/PHASE-1-COMPLETE.md rename to docs/planning/archive/deep-linking/PHASE-1-COMPLETE.md diff --git a/docs/planning/deep-linking/PHASE-1-HANDOFF.md b/docs/planning/archive/deep-linking/PHASE-1-HANDOFF.md similarity index 100% rename from docs/planning/deep-linking/PHASE-1-HANDOFF.md rename to docs/planning/archive/deep-linking/PHASE-1-HANDOFF.md diff --git a/docs/planning/deep-linking/architecture-readonly-urls.md b/docs/planning/archive/deep-linking/architecture-readonly-urls.md similarity index 100% rename from docs/planning/deep-linking/architecture-readonly-urls.md rename to docs/planning/archive/deep-linking/architecture-readonly-urls.md diff --git a/docs/planning/deep-linking/design-review.md b/docs/planning/archive/deep-linking/design-review.md similarity index 100% rename from docs/planning/deep-linking/design-review.md rename to docs/planning/archive/deep-linking/design-review.md diff --git a/docs/planning/deep-linking/design.md b/docs/planning/archive/deep-linking/design.md similarity index 100% rename from docs/planning/deep-linking/design.md rename to docs/planning/archive/deep-linking/design.md diff --git a/docs/planning/deep-linking/implementation-plan.md b/docs/planning/archive/deep-linking/implementation-plan.md similarity index 100% rename from docs/planning/deep-linking/implementation-plan.md rename to docs/planning/archive/deep-linking/implementation-plan.md diff --git a/docs/planning/deep-linking/phase-0-guide.md b/docs/planning/archive/deep-linking/phase-0-guide.md similarity index 100% rename from docs/planning/deep-linking/phase-0-guide.md rename to docs/planning/archive/deep-linking/phase-0-guide.md diff --git a/docs/planning/deep-linking/testing-strategy.md b/docs/planning/archive/deep-linking/testing-strategy.md similarity index 100% rename from docs/planning/deep-linking/testing-strategy.md rename to docs/planning/archive/deep-linking/testing-strategy.md diff --git a/docs/planning/patrol-firebase-testing/plan.md b/docs/planning/archive/patrol-firebase-testing/plan.md similarity index 100% rename from docs/planning/patrol-firebase-testing/plan.md rename to docs/planning/archive/patrol-firebase-testing/plan.md diff --git a/docs/planning/readme-review.md b/docs/planning/archive/patrol-firebase-testing/readme-review.md similarity index 100% rename from docs/planning/readme-review.md rename to docs/planning/archive/patrol-firebase-testing/readme-review.md diff --git a/docs/planning/patrol-firebase-testing/review.md b/docs/planning/archive/patrol-firebase-testing/review.md similarity index 100% rename from docs/planning/patrol-firebase-testing/review.md rename to docs/planning/archive/patrol-firebase-testing/review.md diff --git a/docs/planning/patrol-firebase-testing/summary.md b/docs/planning/archive/patrol-firebase-testing/summary.md similarity index 100% rename from docs/planning/patrol-firebase-testing/summary.md rename to docs/planning/archive/patrol-firebase-testing/summary.md diff --git a/docs/processes/ci-cd.md b/docs/processes/ci-cd.md index 49e2b640..bca09dee 100644 --- a/docs/processes/ci-cd.md +++ b/docs/processes/ci-cd.md @@ -8,16 +8,16 @@ The project uses **3 separate workflows** to handle different aspects of the CI/ | Workflow | File | Purpose | Triggers | |----------|------|---------|----------| -| **Flutter App CI/CD** | `ci.yml` | Build, test, and deploy Flutter app | Push to `main`, PRs to `main` | +| **CI** | `ci.yml` | Build, test, and deploy Flutter app | Push to `main`, PRs to `main` | | **Cloudflare Worker** | `deploy-worker.yml` | Deploy API proxy worker and festivals data | Push to `main`, PRs (when worker/festivals.json changes) | | **Release Web** | `release-web.yml` | Production web releases to Cloudflare Pages | Version tags (`v*`) | --- -## 1. Flutter App CI/CD +## 1. CI **File**: `.github/workflows/ci.yml` -**Name**: `Flutter App CI/CD` +**Name**: `CI` ### Purpose @@ -69,7 +69,7 @@ Runs Flutter tests with coverage reporting. 7. Report coverage to GitHub PR comments and Codecov **Coverage Requirements:** -- Minimum: 25% (TODO: increase to 70%) +- Minimum: 70% - Reports posted as PR comments - Uploaded to Codecov @@ -466,7 +466,7 @@ concurrency: **Check**: 1. Run tests locally: `flutter test` -2. Check coverage meets minimum (25%) +2. Check coverage meets minimum (70%) 3. Review test failure logs in GitHub Actions **Fix**: diff --git a/SAFE_CACHE_STRATEGY.md b/docs/processes/safe-cache-strategy.md similarity index 100% rename from SAFE_CACHE_STRATEGY.md rename to docs/processes/safe-cache-strategy.md diff --git a/todos.md b/docs/todos.md similarity index 60% rename from todos.md rename to docs/todos.md index 28e9ec80..6a5ca55e 100644 --- a/todos.md +++ b/docs/todos.md @@ -1,6 +1,6 @@ # Cambridge Beer Festival App - TODO List -**Last Updated:** 2025-12-21 +**Last Updated:** 2026-02-06 **Overall Status:** Production-ready with improvements needed --- @@ -24,6 +24,44 @@ --- +## 🔴 CRITICAL (Bugs) + +### C1. `dart:io` Import Breaks Web Builds +**Status:** ❌ Not Started +**Location:** `lib/providers/beer_provider.dart:2` + +**Issue:** +`BeerProvider` imports `dart:io` to catch `SocketException` (line 364). `dart:io` is not available on web, the primary target platform. This causes a compile error or runtime crash on web builds. The `SocketException` catch clause is dead code on web. + +**Solution:** +Remove the `dart:io` import and `SocketException` catch clause, or use a conditional import. + +--- + +### C2. Sequential API Fetching Causes Slow Load Times +**Status:** ❌ Not Started +**Location:** `lib/services/beer_api_service.dart:48-56` + +**Issue:** +`fetchAllDrinks` fetches each beverage type sequentially in a `for` loop with `await`. With 7 beverage types and a 30-second timeout each, worst-case is 3.5 minutes. Even in the happy path, 7 sequential HTTP requests make the initial load ~7x slower than necessary. + +**Solution:** +Use `Future.wait` to parallelize the HTTP requests. + +--- + +### C3. Festival Selector Doesn't Update URL +**Status:** ❌ Not Started +**Location:** `lib/widgets/festival_menu_sheets.dart:188` + +**Issue:** +When selecting a festival via the browser sheet, `provider.setFestival(festival)` is called but the URL is never updated to `/${festival.id}`. The user stays on the old festival's URL path while viewing drinks from the newly selected festival. This breaks deep-linking, bookmarking, and the browser back button. + +**Solution:** +After `provider.setFestival(festival)`, navigate to `/${festival.id}` using GoRouter. + +--- + ## 🔴 HIGH PRIORITY ### 1. Add Integration Tests @@ -55,6 +93,66 @@ No path sanitization before proxying requests. Validate/whitelist acceptable pat --- +### H3. Festival Validation Missing on Detail Routes +**Status:** ❌ Not Started +**Location:** `lib/router.dart:103-143` + +**Issue:** +The `/:festivalId` main route validates the festival ID and switches festivals, but detail routes (`/:festivalId/drink/:id`, `/:festivalId/brewery/:id`, etc.) have no validation or festival switching. Deep-linking to `/invalid-fest/drink/abc` bypasses validation entirely and leads to broken state. Documented as a known limitation at `lib/main.dart:146-149`. + +**Solution:** +Add festival ID validation to all route builders, or extract validation into a shared redirect. + +--- + +### H4. Mutable Drink State Mutated Without Rollback +**Status:** ❌ Not Started +**Location:** `lib/providers/beer_provider.dart:462,494,509` + +**Issue:** +`toggleFavorite`, `setRating`, and `toggleTasted` mutate `Drink` object fields in place after the repository call. If the repository call throws, in-memory state diverges from persisted state with no rollback. Widgets holding a reference to the drink also see the mutation before `notifyListeners()` is called. + +**Solution:** +Use optimistic update with rollback on error, or rebuild the drink list from the repository after mutation. + +--- + +### H5. "Clear Filters" Button Only Clears Category +**Status:** ❌ Not Started +**Location:** `lib/screens/drinks_screen.dart:460` + +**Issue:** +When no drinks match the active filters, the empty state "Clear Filters" button only calls `provider.setCategory(null)`. Style filters and search query remain active, so the user may still see zero results after clicking. + +**Solution:** +Clear all filters: category, styles, and search query. + +--- + +### H6. No Way to Navigate Back from About Screen +**Status:** ❌ Not Started +**Location:** `lib/router.dart:28-31`, `lib/screens/about_screen.dart` + +**Issue:** +The About screen (`/about`) is a global route outside the ShellRoute. If a user deep-links to `/about`, there is no back navigation -- no leading button, no bottom nav bar. The only way back is the browser back button. + +**Solution:** +Add a home/back button to the About screen AppBar, or include it within the shell route. + +--- + +### H7. Favorites Screen Doesn't Respond to Festival Switches +**Status:** ❌ Not Started +**Location:** `lib/main.dart:354-401` + +**Issue:** +`FavoritesScreen` displays `provider.favoriteDrinks` from `_allDrinks` (the currently loaded festival). If a user navigates to `/cbf2024/favorites` while `cbf2025` drinks are loaded, they see `cbf2025` favorites on a page claiming to show `cbf2024`. + +**Solution:** +Ensure festival switch completes before rendering, or filter favorites by the URL's `festivalId`. + +--- + ## 🟡 MEDIUM PRIORITY ### 3. Implement Retry Logic for Failed API Requests @@ -93,6 +191,72 @@ Consider Firebase Firestore or Supabase for cross-device sync. --- +### M1. `getTastedDrinkIds` Matches Keys from Other Festivals +**Status:** ❌ Not Started +**Location:** `lib/services/tasting_log_service.dart:56-59` + +**Issue:** +The prefix `tasting_log_cbf2025` also matches keys for a hypothetical festival `cbf20250`. The prefix should include the trailing `_` separator (i.e., `tasting_log_cbf2025_`). Same issue in `clearFestivalLog` at line 69. + +--- + +### M2. `FestivalService` Doesn't Decode UTF-8 +**Status:** ❌ Not Started +**Location:** `lib/services/festival_service.dart:79` + +**Issue:** +`BeerApiService.fetchDrinks` correctly uses `utf8.decode(response.bodyBytes)` to handle non-ASCII characters, but `FestivalService.fetchFestivals` uses `response.body` directly. Festival names or descriptions with non-ASCII characters will display as mojibake. + +--- + +### M3. Drink Detail App Bar Shows Raw Festival ID +**Status:** ❌ Not Started +**Location:** `lib/screens/drink_detail_screen.dart:118` + +**Issue:** +The app bar subtitle shows `${provider.currentFestival.id} > ${drink.breweryName}` (e.g., "cbf2025 > Brewery Name"). Every other screen uses `provider.currentFestival.name`. Exposes internal identifiers to users. + +--- + +### M4. No Debouncing on Search Input +**Status:** ❌ Not Started +**Location:** `lib/screens/drinks_screen.dart:106` + +**Issue:** +Every keystroke triggers `setSearchQuery`, which applies all filters, creates new lists, calls `notifyListeners()`, and fires an analytics event. With hundreds of drinks, this causes jank during fast typing and spams analytics. + +**Solution:** +Add a debounce (e.g., 300ms) before applying the search query. + +--- + +### M5. Filter Button Screen Reader Hint Is Misleading +**Status:** ❌ Not Started +**Location:** `lib/screens/drinks_screen.dart:555` + +**Issue:** +`_FilterButton` semantic hint says "Double tap to clear filter" when active, but tapping opens the filter selection bottom sheet rather than clearing the filter. Misleading for screen reader users. + +--- + +### M6. Availability Toggle Label Is Ambiguous +**Status:** ❌ Not Started +**Location:** `lib/screens/drinks_screen.dart:640-641` + +**Issue:** +When active (unavailable drinks hidden), the label says "Show unavailable". It's unclear whether this describes the current state or the action the button performs. Combined with the icon toggle, users can't distinguish current state from desired action. + +--- + +### M7. `_handlePostInitRedirect` May Use Context After Disposal +**Status:** ❌ Not Started +**Location:** `lib/main.dart:204` + +**Issue:** +In the error handler, `context.read()` is called inside a catch block. If an exception is thrown between the `mounted` check (line 156) and the catch block, context may be used on an unmounted widget. + +--- + ### 6. Apply SliverAppBar to FavoritesScreen **Status:** ❌ Not Started **Location:** `lib/main.dart:99-150` @@ -322,11 +486,12 @@ These items have been deferred pending further analysis or user feedback. ## 📊 Summary ### By Priority -- **HIGH Priority:** 2 issues -- **MEDIUM Priority:** 4 issues +- **CRITICAL (Bugs):** 3 issues (C1-C3) +- **HIGH Priority:** 7 issues (1-2 original + H3-H7 from review) +- **MEDIUM Priority:** 11 issues (3-6 original + M1-M7 from review) - **MOBILE UI (conditional on user feedback):** 5 issues - **LOW Priority:** 7 issues -- **ACTIVE TOTAL:** 18 issues +- **ACTIVE TOTAL:** 33 issues - **DEFERRED:** 6 issues (see Deferred section) ### Recently Completed @@ -347,7 +512,7 @@ These items have been deferred pending further analysis or user feedback. ✅ ListView performance optimization with keys ### Next Focus -Focus on HIGH priority items (#1-3), then mobile UX improvements (#10-14) for better user experience on phones. +Fix CRITICAL bugs first (C1-C3), then HIGH priority items (H3-H7, #1-2), then mobile UX improvements. --- diff --git a/docs/tooling/flutter-web-testing.md b/docs/tooling/flutter-web-testing.md index 3d1cf3b6..a3ffef6c 100644 --- a/docs/tooling/flutter-web-testing.md +++ b/docs/tooling/flutter-web-testing.md @@ -160,11 +160,6 @@ test-e2e/ ## Current Usage in This App -This app has **24 Semantics widgets** across various screens: -- Navigation tabs (Drinks, Favorites) -- About screen buttons (GitHub, Issues, Licenses, Theme toggle) -- Search field -- Filter chips -- Info buttons - +This app has **53+ Semantics widgets** across all screens and widgets. +See [../code/accessibility.md](../code/accessibility.md) for the full inventory. These provide both accessibility and testability. diff --git a/web/index.html b/web/index.html index e3942ce5..e02a1532 100644 --- a/web/index.html +++ b/web/index.html @@ -4,7 +4,7 @@ - + diff --git a/web/manifest.json b/web/manifest.json index beb9a2af..02ddf305 100644 --- a/web/manifest.json +++ b/web/manifest.json @@ -1,4 +1,5 @@ { + "id": "/", "name": "Cambridge Beer Festival", "short_name": "Cambridge Beer Festival", "start_url": ".",