fix: field-test hardening, testing campaign, and doc/seed polish - #31
Conversation
Second-player join was routing through a stale/cached match probe and getting rejected or misreported as a network error; resolve it via the facility's own currentMatchId instead. Also stop seeding phantom live matches so every facility starts free.
Removes the profile-setup/preset-avatar screen in favor of provisioning the profile doc directly on register or on recovery (profile-missing session), and keeps the register button's spinner state correctly gated so recovery mode never flashes during a normal signup.
Landing/login/register navigated between auth screens with push / pushReplacement, creating an imperative /register match. While the router redirect holds on /register during `registrationInProgress`, go_router stacks a fresh, empty declarative RegisterScreen over the filled pushed one — the "middle" register page (empty fields, still-spinning button) seen between submitting and the tutorial. Switch all auth-screen transitions to `context.go` so the redirect is the sole navigation authority and only one /register is ever mounted. Adds a regression test that reproduces the duplicate (asserts a single RegisterScreen throughout the register -> onboarding flow).
QrScanView created its own MobileScannerController and passed it to MobileScanner. Per the package contract, supplying a controller makes the widget skip its WidgetsBindingObserver, so the app must drive the camera lifecycle itself. It didn't — on background/resume or hot-restart the native session desynced from the Dart state and start() threw MOBILE_SCANNER_ALREADY_STARTED_ERROR, surfaced as the intermittent "already started" error over the viewfinder. The controller was never used (no torch/lens control), so let MobileScanner own it: the widget then registers its lifecycle observer and handles start/stop on resume/inactive plus the internal hot-restart mitigation.
An accepted outgoing challenge lingered on the dashboard forever: nothing ever marked it done. Complete it lazily, client-side (no write back to the challenge doc): ChallengeCubit now also watches the signed-in user's own recent matches and hides an accepted challenge once a confirmed match with the same opponent and sport, played after the challenge was accepted, appears. The match query reuses the dashboard's exact `watchUserMatches(uid, limit: 20)` so the Firestore SDK shares one server-side listener rather than paying for a second. Pending / declined / expired rows are untouched. Send-only cubits (friends list) never open the stream. Wires matchRepository through HomeScreen into the ChallengeCubit; adds cubit coverage for the played / not-played / order-independent / pre-accept-match / unconfirmed cases.
A fast pinch zoom-out crosses several zoom levels in a few hundred ms. With flutter_map's default tile-update transformer, every intermediate camera frame kicks off a full tile reload, flooding the cached provider's dio + sqlite pipeline and stalling the UI on a physical device. Wire TileUpdateTransformers.debounce(100ms) on the TileLayer so only the resting zoom fetches tiles — the flutter_map-recommended pairing with a cancellable provider (CachedTileProvider is one), which also cancels the superseded intermediate requests. Adds a regression test asserting a non-default transformer is wired.
…epted Accepting an incoming challenge from the dashboard only flipped its status; the pending card then left the inbox and the recipient was dropped back onto an empty dashboard with no way to reach the accepted challenge's "scan to play" screen. accept() now reports whether the write committed, and a successful accept routes the recipient to /challenge/:id — the accepted-state screen with the QR check-in CTA the design intends. Splits the section's accept/decline widget test to assert the navigation on accept and its absence on decline.
An accepted challenge's scan-to-play routed to the blind scanner with no knowledge of the challenge's sport, so a ping-pong challenge could be played on a foosball table. Thread the challenge's sportId through /play/scan?sport= into MatchCubit, which now rejects a facility of a different sport with a new sportMismatch error instead of starting the match.
Add app/test/integration/ exercising each shared contract's real writer transaction and real reader over one shared FakeFirebaseFirestore: - match confirm -> profile history (singles uid-keyed, doubles pairId-keyed) - facility status flip -> map FacilityCubit listener - queue join transaction -> match check-in derived standing Fix QueueEntryModel.fromMap coercing position via 'as int' (a Firestore double would throw and drop the ticket); use 'as num' like the sibling facility numeric fields, with a regression assertion.
…tasks - Rename the stale CHATBOT_API_KEY references to ASSISTANT_API_KEY (the actual --dart-define the assistant reads) in README + brain docs. - Mark the Phase 7 cross-vertical integration spine items done.
Move the EN/IT segmented-control labels out of settings_screen into languageEnglishShort/languageItalianShort ARB keys (values stay EN/IT in both locales) so no user-facing string literals remain in lib/. Add test/l10n/locale_render_test.dart: boots the app shell in each supported locale and asserts no RenderFlex overflow / FlutterError is captured -- guards against longer Italian copy overflowing tight rows, which ARB-parity checks cannot catch.
… reach the map listener; tick localization spine Contract #2 (facility status flip -> map listens) previously only drove the match vertical as the status writer, and the test comment wrongly called it the 'canonical' writer. joinQueue/leaveQueue are a second writer of facilities/{id}.status. Add a test that occupies a table via a real match, then drives the real joinQueue (occupied -> queued) and leaveQueue (queued -> occupied) transactions, asserting the real FacilityCubit stream observes both. Tick the Phase-7 Localization Pass spine items: the audit found no hardcoded user-facing strings, app_en/app_it are at full 433-key parity, and locale_render_test boots both locales without overflow.
…ll scan-to-ELO flow Drives createMatch → joinMatch → submitResult → confirmResult as one continuous sequence for singles and doubles, asserting the shared contracts (match doc shape, facility status flip, queue position) hold across the whole chain rather than at isolated states.
Adds §5.4 to the design document, referencing docs/design/CampusArena.dc.html as the mockup source (no separate Figma file) and embedding the docs/design/screenshots/ catalog grouped by the §5.2 screen inventory.
…tric The l10n string tables (app_localizations_en/it.dart) and firebase_options.dart are machine-generated and contribute ~557 permanently-uncovered lines, dragging the reported total well below the code that is actually tested. Strip them with lcov --remove after the coverage run so the number reflects hand-written code.
…ts, tool_results - llm_client_test: MockClient-backed coverage of every provider transport (OpenAI/OpenRouter/Regolo/Anthropic/Gemini), HTTP error-code mapping, timeout/socket/generic failure -> LlmException, and testConnection. - assistant_settings_cubit_test: load, provider/model/key edits, testConnection success+failure, and save persistence. - chatbot_repository_test: extend to the tournaments/queue/general intents, non-empty grounding, and per-user personalization context. - tool_results_test: value-equality of the four typed tool results.
Closes the two spine-named gaps: the FoosballResultEntry widget (its strategy logic was tested but the stepper UI was not) and the password-reset auth form (validation, submit, success notice, and error path).
… widgets - team_profile_screen: loading/error/ready states, two-member header, ELO chips (rated + placeholder), and empty vs populated match history. - assistant_settings_screen: banner, provider-change reset prompt, key visibility toggle, save, and the missing-key Test Connection path (no network). - confirm_dialog, result_message_view, typing_indicator, chatbot tournament_card: render + interaction tests. - ci: lcov 2.5 needs --ignore-errors unused,empty for the filter step.
- match_flow_page: MatchFlowView builder across idle/active/readyToConfirm (deep-link + far-blind -> setup, nearby-blind -> auto check-in) and the error listener (snackbar + scanner re-arm). Was 0% covered. - organizer_sheets: forfeit and advance bottom sheets, empty and populated paths through to slot/winner selection.
- active_match_screen: doubles roster (join slots + rostered tiles + waiting slots), resultSubmitted (submitter vs reviewer CTA), and disputed states. - result_flow_page: ResultFlowView loading/closed(3 kinds)/error branches with routing + retry. - privacy_screen: static-content render in both locales.
…tail - result_summary_view: win/loss headers, animated ELO block, rank pill (singles only), achievement reveal, and Done -> home. - qr_scan_screen: manual fallback-code entry (submit + empty-ignore) and the busy overlay. - tournament_browser: empty state, ongoing section, and stream-error retry. - tournament_detail: stream-error retry, register-bar registered/full labels, and the organizer cancel menu + confirm dismiss.
Both variants (submitter waiting footer vs opponent Confirm/Dispute bar), the confirm action, the dispute confirmation dialog, the in-flight spinner, and the doubles team-orientation of the score card.
Adds an inline lcov report (read-lcov-report-action) and a totals block written to the job's Summary tab, both reading the generated-file-filtered lcov.info.
…kens - QR capture fires HapticFeedback.selectionClick so a lock-on is felt while the phone is over the table, not just shown as a spinner. - Queue section animates height (AnimatedSize) and cross-fades the position counter as the line advances, instead of snapping. - FacilityStatusCard reuses the canonical facilityStatusColor helper (error/tertiary/primary roles) instead of raw Colors.green/red/amber.
… statuses Facility docs are now seeded from the hand-verified production data (coordinates, QR codes, fallbackCodes, isOutdoor) instead of being treated as read-only prerequisites. Statuses are mixed for the demo: fac_fb is occupied by the active tournament's live semifinal, leo_gardella_pp_2 is queued (walk-up occupant + two waiting players), every other table free so the Assistant 'empty tables' query stays rich. queue subcollections cleared on wipe; facility docs overwritten, never deleted, to keep FK IDs stable. Marks the seeding-script and demo-dataset task-board items done.
536f5b6 to
07f1995
Compare
LCOV of commit
|
ChristianPrendin
left a comment
There was a problem hiding this comment.
Reviewed against the described changes. Verified locally: flutter analyze clean, full suite green (770 passing), CI green (90.4% lines). Approving.
Verified against the code, not just the writeup:
getActiveMatchForFacility— resolves the live match through the facility'scurrentMatchIdpointer (facility read →_readMatch), returns null on absent pointer / dangling doc (StateErrorcaught) / non-live state, and lets genuine read failures propagate. The caller (MatchCubit._attachExistingMatch) catches that failure and emitscheckInrather than folding it into "no live match" and falling through tocreateMatch— the exact regression the doc claims to fix is actually closed.- Queue parse —
QueueEntryModel.positionis now(map['position'] as num?)?.toInt() ?? 0, so thedouble-from-Firestore cast no longer throws and drops the entry. OnboardingCubit— emits its initial state synchronously viasuper(OnboardingStatus.idle); error path logs the real cause instead of a barecatch. Compliant with the sync-initial-state rule.- Challenges — the played-challenge outbox filter (
_wasPlayed) is gated on confirmed state + sport + opponent +playedAt >= respondedAt; sport-mismatch table rejection present. No write-back to the challenge doc. - Map — debounce uses
TileUpdateTransformers.debounce, a flutter_map built-in; no hand-rolled timer to leak. - QR — hands controller ownership to
MobileScannerto clearMOBILE_SCANNER_ALREADY_STARTED_ERROR; theenableCameratest gate is preserved.
One thing to call out (non-blocking, Shared Contract): the users/{uid} rule now lets any signed-in user update another user's doc when the write touches only badges. Necessary for the both-players badge stamp in the confirm/dispute transaction, and consistent with the Spark-tier trust boundary (client-computed, no Cloud Functions) — but it is a real relaxation: a client can forge or wipe another user's badges map. Accepted per the documented trust model; flagging it so it's a conscious sign-off, not an oversight.
Everything else matches the description and the diff introduces no regression I can find. LGTM.
Description
Phase-7 field-test hardening, testing campaign, and doc/seed polish, gathered on a single
fix/branch. Despite the branch name it spans all three verticals' Phase-7 lanes — it is the convergence of the Ship-phase work, not a docs-only change. Highlights:Bug fixes (field-test findings)
MatchRepository.getActiveMatchForFacilityresolves the live match via the facility's authoritativecurrentMatchIdpointer (two server-fresh single-doc reads) instead of a collection scan that could surface a stale/phantom match or settle on a cold cache read. Genuine read failures propagate so the caller can distinguish "no live match" from "probe broke".positionthroughnum.toInt()— Firestore hands numeric fields back asdouble, and the oldas intthrew and silently dropped the queue entry.RegisterScreenflashing during registration; reliable profile provisioning;authState.profileMissingdrives the onboarding gate. Onboarding was restructured (profile_setup_*→onboarding_cubit+mini_tutorial_screen).mobile_scanner"already started" on the QR screen; debounce map tile loads to stop the freeze on fast zoom-out.Testing campaign (Phase 7B)
app/test/integration/): facility-status→map, match-confirm→profile, queue-join→check-in; plus a full scan→ELO flow onfake_cloud_firestore.firebase_options.Localization / UI polish (Phase 7A/7B)
app_en.arbandapp_it.arb.facilityStatusColorhelper).Docs & seeding (Phase 7C)
script/populate_firestore.pyfolds real campus facilities with mixed statuses (+ a tournament "today") so both flagship Assistant queries return non-empty answers.brain/doc corrections.Related Task
Shared Contracts
facilities/{id}.status/.currentMatchId)facilities/{id}/queue/{uid}.position)Detail:
currentMatchId: newgetActiveMatchForFacilityreads thecurrentMatchIdpointer written bycreateMatch/joinMatch. Read-side only; the write contract is unchanged. (Writer: match vertical; reader: this change + map listener.)num.toInt()coercion inQueueEntryModel). The write transaction andpositionsemantics are unchanged. (Writer: queue vertical; reader: check-in/this parse.)Shared Files
app_router.dartand the ARB files (app_en.arb/app_it.arb) — open the PR the same day and ping the other two devs pergit_workflow.md.main.dartis untouched.Breaking Changes
ProfileSetupState/profile_setup_cubitwere replaced byonboarding_cubit;AuthStategains an additiveprofileMissingfield. No repository method or model field consumed by another vertical was renamed or removed (getActiveMatchForFacilityis additive).Checklist
developand rebased ondevelopbefore opening this PR (no merge commits).dart formatapplied).flutter analyzeandflutter testlocally, OR the GitHub Actions check on this PR has passed).ScoringStrategycontract remains unmodified. (N/A — no new sport.)onboarding_cubit.)availabilityDisclaimerShortARB key.UI Changes (If applicable)
docs/design/screenshots/.docs/design/screenshots/