Add online rating service design plan

[richardthe3rd]

Cloudflare Workers + D1 backend with Firebase Anonymous Auth
for cross-device identity. Covers API design, Flutter integration,
offline sync strategy, and phased implementation plan.

https://claude.ai/code/session_01SusUN4s9TQ4AqeoyoGAGYa

[Copilot]

Pull request overview

Adds a detailed design document for introducing an online, cross-device rating system to the Cambridge Beer Festival app, centered on a Cloudflare Workers + D1 backend and Firebase Anonymous Auth for identity.

Changes:

• Introduces an end-to-end architecture and API design for a ratings service (single-drink + batch endpoints).
• Proposes Flutter-side integration (services/models/provider updates) including offline-first sync behavior.
• Outlines deployment environments, migrations, and a phased implementation/testing plan.

[Copilot]

The example ratingsApiBaseUrl routing uses EnvironmentService.isStaging(), but EnvironmentService currently only exposes isProduction() and getEnvironmentName() (no isStaging). Update this snippet to match the real API (or explicitly call out that isStaging() must be added before implementing).

Suggested change

	} else if (EnvironmentService.isStaging()) {
	return 'https://ratings-staging.cambeerfestival.app';
	} else {
	return 'https://ratings-dev.cambeerfestival.app';
	}
	}
	final envName = EnvironmentService.getEnvironmentName();
	if (envName == 'staging') {
	return 'https://ratings-staging.cambeerfestival.app';
	}
	// Default to dev / local
	return 'https://ratings-dev.cambeerfestival.app';

[Copilot]

OnlineRatingService method name ratedrink doesn’t follow Dart camelCase / naming conventions used elsewhere (e.g., setRating, getRating). Consider renaming to something like rateDrink / setDrinkRating for consistency and readability.

[Copilot]

RatingSyncService.queueRating is defined here as queueRating(String festivalId, String drinkId, int rating), but later the plan uses null rating to represent deletes. Consider updating the design to support deletes explicitly (e.g., separate queueRemoveRating or a queued action type that can represent remove).

Suggested change

	/// Queue a rating for sync (called when offline or as fire-and-forget)
	Future<void> queueRating(String festivalId, String drinkId, int rating);
	/// Queue a rating set/update for sync (called when offline or as fire-and-forget)
	Future<void> queueRating(String festivalId, String drinkId, int rating);
	/// Queue removal of a rating (user cleared their rating)
	Future<void> queueRemoveRating(String festivalId, String drinkId);

[Copilot]

In the BeerProvider.setRating example, _ratingSyncService.queueRating(...) is called with rating where the variable is int? (null means remove). This won’t match a queueRating(..., int rating) signature—either update the sync API to accept null/removals, or call a dedicated remove-queue method when rating == null.

Suggested change

	_ratingSyncService.queueRating(currentFestival.id, drink.id, rating);
	if (rating == null) {
	_ratingSyncService.queueRemoveRating(currentFestival.id, drink.id);
	} else {
	_ratingSyncService.queueRating(currentFestival.id, drink.id, rating);
	}

[Copilot]

AuthService.getIdToken() shows return await user.getIdToken() ?? '';, but User.getIdToken() returns a non-null Future<String> (so ?? '' is misleading). Returning an empty string on failure would also cause opaque 401s downstream; prefer throwing if token retrieval fails (or forcing a refresh when needed).

Suggested change

	return await user.getIdToken() ?? '';
	return await user.getIdToken();

[Copilot]

The plan states the custom domain "simplifies CORS since *.cambeerfestival.app patterns are already handled", but the existing Worker’s CORS logic uses an explicit allowlist (e.g., https://cambeerfestival.app, https://staging.cambeerfestival.app) rather than a *.cambeerfestival.app wildcard. Update this wording so it doesn’t imply wildcard support that isn’t present today.

Suggested change

	1. **Custom domain?** Yes — `ratings.cambeerfestival.app` (+ `ratings-staging`, `ratings-dev`). Simplifies CORS since `*.cambeerfestival.app` patterns are already handled.
	1. **Custom domain?** Yes — `ratings.cambeerfestival.app` (+ `ratings-staging`, `ratings-dev`). Simplifies CORS since it can reuse the existing explicit cambeerfestival.app origin allowlist used by the Worker.

[Copilot]

Decision summary says "Trust client (don't validate)" for drink IDs. Even if you don’t validate against the festival drink list, it’s still worth specifying basic constraints (max length/charset) to prevent malicious clients from creating unlimited distinct drink_ids that bloat D1 and batch responses.

Suggested change

	| Drink ID validation | Trust client (don't validate) | No incentive to fabricate; orphan rows are harmless |
	| Drink ID validation | Basic format/length validation only | Constrain charset + max length to avoid D1/batch bloat; no lookup against festival drink list |

[Copilot]

Section numbering jumps from "## 6" to "## 8" (missing section 7). Consider renumbering (or adding a placeholder heading) so future references don’t drift as the plan changes.

Suggested change

	## 7. Future Work (placeholder)
	_This section is reserved for future expansion of the rating service design plan._

[Copilot]

The final summary table appears to have a duplicated header block (the | # | Action | Effort | Status | rows repeat twice). Removing the duplicate will make the "What to do before starting Phase 1" section easier to read.

Suggested change

	| # | Action | Effort | Status |
	|---|--------|--------|--------|