fix(outcomes): restore Your Progress read — scope query to allowed dayKeys + coerce Long scores (I-001, I-002)
I-001: getOutcomes() did a bare collection list .get() on couples/{cid}/outcomes,
which firestore.rules denies (reads allowed only for dayKey in day_0/30/60/90) ->
always PERMISSION_DENIED, swallowed to emptyList(). Now scopes the query with
whereIn(FieldPath.documentId(), OUTCOME_DAY_KEYS) so it satisfies the rule.
I-002 (found while fixing I-001): toOutcomeScores() cast values to Map<String,Int>,
but Firestore returns integer fields as Long on Android -> ClassCastException ->
scores dropped (same shape submitOutcomeCallable writes, so the real path was broken
too). Now coerces (value as? Number)?.toInt().
Verified live: 0 outcomes PERMISSION_DENIED after relaunch; seeded a day_0 baseline
(int64) -> "Your Progress" shows "Baseline recorded" (was "No baseline yet"). Seed
removed, couple baseline restored (0 outcomes, 0 active sessions). Both pending one
re-QA confirmation round before pruning.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
null
parent
35d36e6851
commit
ab29f6b12f
|
|
@ -91,7 +91,7 @@ Route smoke-test checklist (re-runnable: `dumpsys gfxinfo closer.app reset` →
|
|||
- **Caching / lazy-load:** LazyColumn/Row/Grid in 17 files; Coil (AsyncImage) in 11; Room DAOs cache static question/category data locally — all in place, no load-all anti-patterns seen.
|
||||
- **Leak check:** conversation open/close ×6 → ViewRootImpl=1, Activities=1, Views +2, PSS bounded after trim → no window/Activity/listener leak.
|
||||
- **Redundant reads:** precise per-read counts need an instrumented/Perfetto build (Firestore success reads aren't in adb logcat); no failing-read spam **except I-001**; no leaked listeners.
|
||||
- **Finding: I-001 (P1)** — `getOutcomes()` bare-list query is rules-denied → "Your Progress"/outcomes silently broken (see ClaudeReport.md).
|
||||
- **Finding: I-001 (P1) — FIXED+VERIFIED** `getOutcomes()` bare-list query was rules-denied → fixed with `whereIn(documentId, dayKeys)`; 0 PERMISSION_DENIED after. **I-002 (P1) — FIXED+VERIFIED** (found fixing I-001): scores stored as int64 → read as Long → `Map<String,Int>` cast CCE → swallowed; fixed via `Number.toInt()`. Live: seeded day_0 → "Your Progress" shows "Baseline recorded". Both pending Round-9 confirm.
|
||||
|
||||
## Pass J — Accessibility (R8, emulator-5554)
|
||||
- **Font scaling (font_scale 2.0, worst case):** Home, Paywall, Settings all **reflow + scroll, no clipped/hidden buttons** — meets the acceptance bar. Minor: long subtitles/email ellipsize, bottom-nav labels wrap ("Mess ages"). Restored to 1.0. ✅
|
||||
|
|
|
|||
|
|
@ -1,12 +1,12 @@
|
|||
# Claude QA Report — Full-App QA (living report)
|
||||
|
||||
> **Verdict (2026-06-25, R8): 1 open P1 — I-001 ("Your Progress" outcomes read is rules-denied → feature silently broken). All else clean; security cornerstone clean.**
|
||||
> **Verdict (2026-06-25, R8): 0 open P0–P2 (1 P3 J-OBS, non-blocking). I-001 + I-002 (outcomes read) fixed + verified live. Security cornerstone clean. At the flawless bar.**
|
||||
>
|
||||
> This report shows **current state only**. Fixed issues live here for **one** confirmation round, then they're pruned
|
||||
> to the archived-ID line below (full detail stays in git history). See **Report hygiene** in `ClaudeQAPlan.md`.
|
||||
|
||||
## Run-state (current)
|
||||
`Round 8 (re-QA + Passes I/J) — IN PROGRESS | 1 open P1 (I-001) + 1 P3 (J-OBS) | Passes I+J done | NEXT ACTION: fix phase — I-001 (build+verify), then re-QA confirm.`
|
||||
`Round 8 — fix phase DONE | 0 open P0–P2 (1 P3 J-OBS) | I-001+I-002 fixed+verified live | NEXT ACTION: optional deferred C/E/F coverage; Round 9 re-confirm + prune I-001/I-002.`
|
||||
- **Build:** client HEAD `23dd6a7`, Cloud Functions deployed.
|
||||
- **Devices / accounts:** emulator-5554 = QA (`Y05AKO2IlTPMa0JQW1BiNIM0uzK2`) · emulator-5556 = Sam (`imDjjO…`) · paired, coupleId `Xal3Kw3gjSdn0niERYKJ`, both free (baseline restored).
|
||||
- **Docs:** Playbook `ClaudeQAPlan.md` · Coverage `ClaudeQACoverage.md` · Ideas `Future.md` `## QA` · Branding `ClaudeBrandingReview.md`.
|
||||
|
|
@ -15,15 +15,20 @@
|
|||
| Severity | Open | Fixed (pending 1 confirm) |
|
||||
|---|---|---|
|
||||
| P0 | 0 | 0 |
|
||||
| P1 | **1** | 0 |
|
||||
| P1 | 0 | 2 (I-001, I-002) |
|
||||
| P2 | 0 | 0 |
|
||||
| P3 | **1** | 0 |
|
||||
|
||||
## Open issues
|
||||
| ID | Sev | Area | Description | Repro | Suggested fix | Status |
|
||||
|---|---|---|---|---|---|---|
|
||||
| I-001 | **P1** | Outcomes / "Your Progress" read | `FirestoreOutcomeDataSource.getOutcomes()` (line 45-53) issues a **bare collection list** `.get()` on `couples/{cid}/outcomes`, but the rule (firestore.rules:658) only allows reading specific dayKey docs (`day_0/30/60/90`) and **denies list queries → always `PERMISSION_DENIED`**. `OutcomeRepositoryImpl.getOutcomes` (26-29) swallows it (records non-fatal → returns `emptyList()`). Net: recorded check-ins **never display** in Your Progress; Home/Settings reminder logic re-prompts for completed days; crashReporter spammed each load. Found via Pass I (perf) efficiency lens — masked from A/B/C because this couple has 0 outcomes + the failure is swallowed. | Open app → logcat: `Listen/Get for Query(couples/{cid}/outcomes …) failed: PERMISSION_DENIED` (live-confirmed R8). | Constrain the query to satisfy the rule: `.whereIn(FieldPath.documentId(), listOf("day_0","day_30","day_60","day_90")).get()` (or 4 parallel `getOutcome` gets). No rules change needed. | **Open** |
|
||||
| J-OBS | P3 | A11y / touch targets | A few conversation icon-buttons measure **~42–45dp wide** (48dp tall) — single-axis marginal miss of the 48dp target; fully operable. Most controls are 48dp. | Pass J: uiautomator bounds on conversation → 2–3 clickables `<126px` wide. | Bump those icon-buttons to 48dp min (e.g. `Modifier.minimumInteractiveComponentSize()` / `size(48.dp)`). | **Open** |
|
||||
| J-OBS | P3 | A11y / touch targets | A few conversation icon-buttons measure **~42–45dp wide** (48dp tall) — single-axis marginal miss of the 48dp target; fully operable. Most controls are 48dp. | Pass J: uiautomator bounds on conversation → 2–3 clickables `<126px` wide. | Bump those icon-buttons to 48dp min (e.g. `Modifier.minimumInteractiveComponentSize()` / `size(48.dp)`). | **Open (P3, non-blocking)** |
|
||||
|
||||
## Fixed this round — pending one confirmation round (then prune)
|
||||
| ID | Sev | Area | Fix | Status |
|
||||
|---|---|---|---|---|
|
||||
| I-001 | P1 | Outcomes read — query rules-denied | `getOutcomes()` now scopes the query with `.whereIn(FieldPath.documentId(), OUTCOME_DAY_KEYS)` (the 4 allowed dayKeys) so it satisfies firestore.rules:658 instead of issuing a denied bare-list. | **Fixed + verified live:** 0 `outcomes` PERMISSION_DENIED after relaunch (was firing every load). |
|
||||
| I-002 | P1 | Outcomes read — score parse (found fixing I-001) | `toOutcomeScores()` now coerces `(value as? Number)?.toInt()` instead of casting to `Map<String,Int>` — Firestore returns integer fields as **Long** on Android → the hard Int cast threw CCE → scores swallowed to `emptyList()`. Same shape `submitOutcomeCallable` writes, so it broke the real path too. | **Fixed + verified live:** seeded a day_0 baseline (int64, == real callable shape) → "Your Progress" now shows **"Baseline recorded"** (was "No baseline yet"). Seed removed, baseline restored. |
|
||||
|
||||
## Resolved & confirmed (archived — full detail in git history)
|
||||
A-001 · A-003 · A-OBS · B-001 · B-002 · B-003 · B-004 · C-CC-001 · C-DS-001 · C-NAV-001 · D-001 · E-001 · E-002 · E-003 · E-OBS · F-OBS · F-RACE-001 — all fixed and re-verified (commits in history; **F-RACE-001** fixed `23dd6a7`, re-confirmed live R8: race → 1 session, loser joins same set). Pruned per the one-confirmation-round rule. (C-OBS / `outcomes` list / SubscriptionScreen per-user gate = investigated, **not bugs**.)
|
||||
|
|
@ -35,7 +40,7 @@ A-001 · A-003 · A-OBS · B-001 · B-002 · B-003 · B-004 · C-CC-001 · C-DS-
|
|||
- **Robustness:** malformed/abusive deep-link intents (unknown type, missing extras, injection/path-traversal) → 0 crash; killed-state cold-start chat deep-link → conversation loads.
|
||||
|
||||
## Round history (one line each)
|
||||
- **R8** (in progress) — F-RACE-001 re-confirmed live (race → 1 session; loser joins winner's same-set session via "Join the game") + pruned; running Passes I/J.
|
||||
- **R8** — F-RACE-001 re-confirmed + pruned; Passes I (perf) + J (a11y) run; found+fixed+verified **I-001 & I-002** (outcomes read: query rules-denied + Long/Int parse CCE → "Your Progress" was silently dead). 0 open P0–P2.
|
||||
- **R7** — multi-angle security/concurrency deep dive → cornerstone fully clean; F-RACE-001 found + fixed + verified. 0 new open.
|
||||
- **R6** — branding drop + Future.md backlog regression (white-keyhole icons/loader/splash, inclusive gender, copy, rate-limit split, results-push suppression, paywall retry/offline) → 0 new open.
|
||||
- **R5** — Cloud Functions deployed (E-OBS channel fix, E-003 results routing) + new Pass G (account creation / fake-account abuse) clean → 0 open.
|
||||
|
|
|
|||
|
|
@ -1,9 +1,11 @@
|
|||
package app.closer.data.remote
|
||||
|
||||
import app.closer.domain.model.Outcome
|
||||
import app.closer.domain.model.OutcomeDay
|
||||
import app.closer.domain.model.OutcomeDayKey
|
||||
import app.closer.domain.model.OutcomeScores
|
||||
import com.google.firebase.firestore.DocumentSnapshot
|
||||
import com.google.firebase.firestore.FieldPath
|
||||
import com.google.firebase.firestore.FirebaseFirestore
|
||||
import com.google.firebase.functions.FirebaseFunctions
|
||||
import kotlinx.coroutines.suspendCancellableCoroutine
|
||||
|
|
@ -18,6 +20,11 @@ class FirestoreOutcomeDataSource @Inject constructor(
|
|||
private val db: FirebaseFirestore,
|
||||
private val functions: FirebaseFunctions
|
||||
) {
|
||||
private companion object {
|
||||
// The only outcome doc ids the security rules allow reading.
|
||||
val OUTCOME_DAY_KEYS: List<OutcomeDayKey> = OutcomeDay.entries.map { it.key }
|
||||
}
|
||||
|
||||
suspend fun submitOutcome(coupleId: String, dayKey: OutcomeDayKey, scores: OutcomeScores): Unit =
|
||||
suspendCancellableCoroutine { cont ->
|
||||
functions.getHttpsCallable("submitOutcomeCallable")
|
||||
|
|
@ -43,10 +50,14 @@ class FirestoreOutcomeDataSource @Inject constructor(
|
|||
}
|
||||
|
||||
suspend fun getOutcomes(coupleId: String): List<Outcome> {
|
||||
// Security rules permit reading only the fixed dayKey docs and DENY an
|
||||
// unconstrained list query, so the query must be scoped to those ids by
|
||||
// document id — otherwise every read fails PERMISSION_DENIED (I-001).
|
||||
val snapshot = db
|
||||
.collection(FirestoreCollections.COUPLES)
|
||||
.document(coupleId)
|
||||
.collection(FirestoreCollections.OUTCOMES)
|
||||
.whereIn(FieldPath.documentId(), OUTCOME_DAY_KEYS)
|
||||
.get()
|
||||
.await()
|
||||
return snapshot.documents.mapNotNull { it.toOutcome() }
|
||||
|
|
@ -73,13 +84,14 @@ class FirestoreOutcomeDataSource @Inject constructor(
|
|||
)
|
||||
}
|
||||
|
||||
@Suppress("UNCHECKED_CAST")
|
||||
private fun Map<*, *>.toOutcomeScores(): OutcomeScores? {
|
||||
val map = this as? Map<String, Int> ?: return null
|
||||
val connection = map["connection"] ?: return null
|
||||
val communication = map["communication"] ?: return null
|
||||
val intimacy = map["intimacy"] ?: return null
|
||||
val happiness = map["happiness"] ?: return null
|
||||
// Firestore returns integer fields as Long on Android, so coerce via Number
|
||||
// rather than casting to Int (a hard Int cast threw CCE → scores dropped, I-002).
|
||||
fun score(key: String): Int? = (this[key] as? Number)?.toInt()
|
||||
val connection = score("connection") ?: return null
|
||||
val communication = score("communication") ?: return null
|
||||
val intimacy = score("intimacy") ?: return null
|
||||
val happiness = score("happiness") ?: return null
|
||||
return runCatching { OutcomeScores(connection, communication, intimacy, happiness) }
|
||||
.getOrNull()
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue