Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -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/
10 changes: 5 additions & 5 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
7 changes: 2 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
225 changes: 51 additions & 174 deletions docs/README.md

Large diffs are not rendered by default.

79 changes: 79 additions & 0 deletions docs/adr/0004-path-based-url-strategy.md
Original file line number Diff line number Diff line change
@@ -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
88 changes: 88 additions & 0 deletions docs/adr/0005-e2e-testing-strategy.md
Original file line number Diff line number Diff line change
@@ -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 `<canvas>` 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
2 changes: 2 additions & 0 deletions docs/adr/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
54 changes: 31 additions & 23 deletions docs/code/accessibility.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

---

Expand Down
File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -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)

---

Expand Down
File renamed without changes.
10 changes: 5 additions & 5 deletions docs/processes/ci-cd.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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**:
Expand Down
File renamed without changes.
Loading
Loading