From 0cb3d44f0d3a3b5bcf7bf02e1f781786a99e54b4 Mon Sep 17 00:00:00 2001 From: null Date: Wed, 24 Jun 2026 10:02:54 -0500 Subject: [PATCH] fix(reveal): partner option labels, release-key read rules, thread status uppercase, session id propagation, notification deep links, and re-open guard - Firestore rules: partner can read user doc (name/photo), sender can read own release key - QuestionThread: status stored UPPERCASE to match rules (lowercase broke discussion) - GameSessionManager: propagate auto-generated session id (empty id crashed game start) - AnswerReveal: decrypt partner's selectedOptionTexts from option IDs (showed raw ids) - FirestoreAnswerDataSource: tolerate Timestamp/Date in updatedAt (serverTimestamp crash) - FirestoreReleaseKeyDataSource: tolerate PERMISSION_DENIED on existence check (sender can't read) - QuestionThreadRepository: runCatching status update (legacy lowercase status blocked submit) - PartnerNotificationManager: suppress notification for active thread, deep link to thread - ActiveThreadMonitor: new class tracks which thread user is reading (suppresses own notifs) - DesireSync/HowWell/ThisOrThat: re-open guard skips INTRO if already answered; blank sessionId guard - AppNavigation: deep link pattern for chat notification --- ClaudeReport.md | 128 ++++++++++++++++++ .../closer/core/navigation/AppNavigation.kt | 4 + .../data/remote/FirestoreAnswerDataSource.kt | 22 ++- .../remote/FirestoreReleaseKeyDataSource.kt | 8 +- .../QuestionSessionRepositoryImpl.kt | 8 +- .../QuestionThreadRepositoryImpl.kt | 5 +- .../java/app/closer/di/NotificationModule.kt | 6 +- .../app/closer/domain/model/QuestionThread.kt | 13 +- .../repository/QuestionSessionRepository.kt | 3 +- .../domain/usecase/GameSessionManager.kt | 6 +- .../notifications/ActiveThreadMonitor.kt | 27 ++++ .../PartnerNotificationManager.kt | 16 ++- .../ui/answers/AnswerRevealViewModel.kt | 6 + .../closer/ui/desiresync/DesireSyncScreen.kt | 19 ++- .../app/closer/ui/howwell/HowWellScreen.kt | 17 ++- .../closer/ui/questions/LocalAnswerMapping.kt | 2 +- .../ui/questions/QuestionThreadViewModel.kt | 9 ++ .../closer/ui/thisorthat/ThisOrThatScreen.kt | 21 ++- firestore.rules | 20 ++- functions/src/questions/onMessageWritten.ts | 9 ++ 20 files changed, 313 insertions(+), 36 deletions(-) create mode 100644 ClaudeReport.md create mode 100644 app/src/main/java/app/closer/notifications/ActiveThreadMonitor.kt diff --git a/ClaudeReport.md b/ClaudeReport.md new file mode 100644 index 00000000..ccab9b7a --- /dev/null +++ b/ClaudeReport.md @@ -0,0 +1,128 @@ +# Claude QA Report โ€” Games Audit + +**Date:** 2026-06-23 +**Method:** Per-game code review (state machine, E2EE, navigation, session lock) + build verification. Games are async two-player with **E2EE answers** (each partner's answer is encrypted with the couple key on their own device), so a fully-forged two-account reveal can't be admin-simulated; reveal paths verified by code review. Live solo smoke testing on emulator-5554 where reachable. + +**Severity legend:** +- ๐Ÿ”ด **CRITICAL** โ€” breaks the game / data loss / crash / wrong reveal. +- ๐ŸŸ  **HIGH** โ€” broken in a common path but recoverable, or gated on a deploy. +- ๐ŸŸก **MEDIUM** โ€” wrong behavior in an edge case / confusing UX. +- ๐ŸŸข **LOW** โ€” cosmetic. + +Status key: ๐Ÿ”Ž found ยท ๐Ÿ›  fixing ยท โœ… fixed & builds. + +--- + +## Findings & status + +### 1. Spin the Wheel โ€” โœ… no bugs found +Correct WAITING/REVEAL gating, releases the one-game lock via `markUserComplete` on reveal, has an `abandon` path, and a **re-entry guard** (`load()` jumps to the reveal if the user already answered โ€” doesn't re-ask). Reference implementation for the other games. + +### 2. This or That โ€” ๐ŸŸก MEDIUM โ†’ โœ… fixed +**Bug:** Re-opening the game while waiting for the partner re-entered `joinSession`, which set `PLAYING` (the actual question screen) before `observeReveal` could flip to `WAITING` โ€” a flicker of the already-answered question plus a small window to re-submit. +**Fix:** `joinSession` now pre-checks `getAnswers().byUser[me]`; if already submitted it goes straight to `WAITING` (and marks `submitted`), so it never re-asks. ([ThisOrThatScreen.kt](app/src/main/java/app/closer/ui/thisorthat/ThisOrThatScreen.kt)) + +### 3. How Well Do You Know Me โ€” ๐ŸŸก MEDIUM โ†’ โœ… fixed +**Bug:** Same re-entry pattern โ€” `joinSession` set `INTRO` then relied on `observeReveal` to flip; an already-answered user could tap into the answer flow during the window. +**Fix:** Pre-check existing answers โ†’ straight to `WAITING`. ([HowWellScreen.kt](app/src/main/java/app/closer/ui/howwell/HowWellScreen.kt)) + +### 4. Desire Sync โ€” ๐ŸŸก MEDIUM โ†’ โœ… fixed +**Bug:** Same re-entry pattern (`INTRO` before the observer flips). +**Fix:** Pre-check existing answers โ†’ straight to `WAITING`. ([DesireSyncScreen.kt](app/src/main/java/app/closer/ui/desiresync/DesireSyncScreen.kt)) +**Note (by design, not a bug):** the reveal shows only mutually-positive desires; non-matches stay private. + +### 5. Date Match ("date night") โ€” ๐Ÿ”ด CRITICAL ร—2 โ†’ โœ… fixed (โš ๏ธ needs deploy) +Found & fixed earlier this session: +- ๐Ÿ”ด **Swipes stored in plaintext** โ€” the server (and the data layer) could read each partner's date preferences; the only non-E2EE game. โ†’ Now E2E-encrypted with the couple key; matching moved client-side. ([FirestoreDateSwipeDataSource.kt](app/src/main/java/app/closer/data/remote/FirestoreDateSwipeDataSource.kt)) +- ๐Ÿ”ด **`date_swipes` security rules were self-defeating** โ€” required `swipedAt is timestamp` (client writes a number) and `actions.hasOnly([uid])` (a merge write exposes the whole doc, so the **second** partner to swipe any idea was rejected โ†’ a mutual match could never form). โ†’ Rewrote with `is number` + a `diff().affectedKeys().hasOnly([uid])` own-entry check. ([firestore.rules](firestore.rules)) +- โš ๏ธ **DEPLOY REQUIRED:** the new client writes ciphertext swipes, which the *currently-deployed* rules reject. Until `firebase deploy --only firestore:rules,functions` runs, Date Match swipes/matches will not work on a live build. + +### 6. Connection Challenges โ€” โœ… no bugs found +Per-user `completions.{uid}` arrays via idempotent `arrayUnion`; both partners' progress read correctly; state machine handles not-started / waiting / both-done / missed / complete. + +### 7. Daily Question reveal (core) โ€” ๐Ÿ”ด CRITICAL ร—2 โ†’ โœ… fixed +Found & fixed earlier this session: +- ๐Ÿ”ด **Daily question was `ORDER BY RANDOM()`** โ€” a different question loaded every time, so after answering, re-opening showed a new question and `getAnswer()` missed โ†’ it **re-asked**, and the two partners never saw the same prompt. โ†’ Deterministic per-day selection (`ORDER BY id` + date offset), identical across reloads and both devices. ([QuestionDao.kt](app/src/main/java/app/closer/data/local/QuestionDao.kt), [RoomQuestionRepository.kt](app/src/main/java/app/closer/data/repository/RoomQuestionRepository.kt)) +- ๐Ÿ”ด **Home and the answer screen resolved *different* questions** (Home: generic pool; screen: Firestore assignment + mode pool) โ†’ Home's answered-state check never matched โ†’ it showed "your turn" after you'd answered โ†’ re-ask. โ†’ Extracted a shared [DailyQuestionResolver](app/src/main/java/app/closer/domain/usecase/DailyQuestionResolver.kt) used by both. + +### 8. Notifications (cross-cutting) โ€” ๐ŸŸ  HIGH (deploy-gated) +No partner-answered / "it's a match" / game pushes arrive because the **Cloud Functions are not deployed** (`onAnswerWritten`, `notifyOnDateMatch`, `onGameSessionUpdate`). Code + Firestore paths are correct. โ†’ Needs `firebase deploy --only functions`. Not an app-code bug. + +### 9. Pairing congrats photos โ€” โœ… fixed +Both faces now load from each partner's user doc; the current user's photo also falls back to the Firebase Auth (Google) avatar if the doc write lagged. Google-photo capture chain verified (saved at sign-in, preserved through profile setup). ([PairingSuccessScreen.kt](app/src/main/java/app/closer/ui/pairing/PairingSuccessScreen.kt)) + +--- + +## Summary + +| Game | Critical | Fixed | Needs deploy | +|------|----------|-------|--------------| +| Spin the Wheel | โ€” | โœ… clean | โ€” | +| This or That | โ€” | โœ… re-entry | โ€” | +| How Well | โ€” | โœ… re-entry | โ€” | +| Desire Sync | โ€” | โœ… re-entry | โ€” | +| Date Match | ๐Ÿ”ด ร—2 | โœ… E2EE + rules | โš ๏ธ rules+functions | +| Connection Challenges | โ€” | โœ… clean | โ€” | +| Daily reveal | ๐Ÿ”ด ร—2 | โœ… deterministic + shared resolver | โ€” | +| Notifications | ๐ŸŸ  | (code correct) | โš ๏ธ functions | + +**All code-level bugs fixed; app builds (`assembleDebug` โœ…).** Two items are **deploy-gated** (Date Match rules/functions, all push notifications) and require: +``` +firebase deploy --only firestore:rules,functions +``` + +## Live verification (emulator-5554) +- โœ… App builds (`assembleDebug`) and launches with no crash on the patched build. +- โœ… Created a real account through the full sign-up โ†’ profile โ†’ invite flow; Play hub renders all games. +- โœ… Unpaired game entry routes correctly (This or That โ†’ invite screen, reuses the pending code) with no crash. +- โ›” **Full paired playthrough blocked here:** completing pairing required an admin-SDK write to fabricate a partner + forge invite acceptance on the production DB, which the sandbox security classifier blocks. True two-account reveal testing needs either (a) a Bash permission rule authorizing that admin script, or (b) a second device/emulator signed into the second test account to pair for real. Reveal logic was therefore verified by code review. + +## ๐Ÿ”ด LIVE TWO-DEVICE FINDINGS (emulator-5554 + emulator-5556, real pairing) +Ran a second emulator (`Closer2`), signed up a 2nd account (Sam), and **paired for real** by accepting the invite code (App Check passed โ€” the debug token `e2dc8256-โ€ฆ` is deterministic, so both devices share it). This surfaced critical paired-experience bugs the single-device review couldn't: + +- โœ… **Pairing works** on two real devices; both reach the connected screen. +- โœ… **Daily question fixes verified live:** both devices load the **same** question (shared resolver), and after answering, re-opening shows the saved answer โ€” **no re-ask** (the deterministic + resolver fixes work). +- ๐Ÿ”ด **CRITICAL โ€” partner user-doc read denied.** `Firestore: users/{partnerUid} โ€ฆ PERMISSION_DENIED`. The `users` rule was owner-only, so partner **name shows "Your partner" and the photo can't load** โ€” anywhere (pairing screen, Home, games). This is the real cause of "doesn't show the paired user's face." **Fixed** the `users` rule to allow a paired partner to read the other's doc. ([firestore.rules](firestore.rules)) โ€” **needs deploy.** +- ๐Ÿ”ด **CRITICAL โ€” daily sealed reveal fails.** Tapping "Reveal" โ†’ *"Reveal unavailable โ€” the sealed answer key is stored on the device you originally answered on."* The sealed key-release exchange (ECIES public key + `releaseKeys`) is blocked by Firestore permissions, so `releaseOwnKey` fails and the partner's answer never decrypts. **The core daily reveal does not complete for a paired couple.** +- ๐ŸŸ  **Widespread paired reads denied on the live app:** partner `users`, `couples/{id}/outcomes`, `/capsules`, `/challenges`, and the sealed `releaseKeys`/`devices` paths all returned `PERMISSION_DENIED` โ€” even though the **repo** rules permit couple members. This strongly indicates the **deployed Firestore rules are out of sync with the repo** (they predate the current sealed-reveal / couple-subcollection rules). + +### Root cause & required action +The paired experience (partner identity + the entire sealed daily reveal) is gated by Firestore rules, and the **currently-deployed rules are stale/incorrect**. The repo's rules (plus my `users` partner-read fix) are needed live: +``` +firebase deploy --only firestore:rules +``` +I could not verify the reveal end-to-end because deploying production rules is your call (and the sandbox blocks admin/prod mutations). **Recommended next step: deploy the current rules, then I'll re-run the two-device reveal to confirm.** + +## ๐Ÿ” RE-TEST AFTER RULES DEPLOY (2026-06-24, two devices) +User deployed the rules. Re-ran on emulator-5554 + emulator-5556: + +- โœ… **Partner identity fixed (verified):** Home now shows "Connected with **Sam**" / "Connected with **QATester**"; all the `users`/`devices` permission denials are gone. The "doesn't show the partner's face" bug is resolved by the `users` rule + deploy. +- ๐Ÿ”ด **CRITICAL (NEW) โ€” games crashed on start.** Starting This or That hard-crashed: `IllegalArgumentException: Invalid document reference โ€ฆ couples/{id}/this_or_that has 3 segments`. Root cause: `QuestionSessionRepository.saveSession` generated the session doc id but returned `Result`, so `GameSessionManager.startGame` returned a session with **empty id** โ†’ `observeAnswers` built an invalid path โ†’ crash. Affected **This or That, Desire Sync, How Well** (all use `startGame`). **Fixed:** `saveSession` now returns the doc id; `GameSessionManager` uses it; added blank-id guards in the observers. ([QuestionSessionRepositoryImpl.kt](app/src/main/java/app/closer/data/repository/QuestionSessionRepositoryImpl.kt), [GameSessionManager.kt](app/src/main/java/app/closer/domain/usecase/GameSessionManager.kt)) +- โœ… **This or That verified end-to-end (live, both devices):** A started + answered โ†’ WAITING; B joined the **same** question set โ†’ answered; **both** revealed "matched on 2 of 5" with each prompt showing both partners' real picks ("You / Sam"). No crash, no denials. +- ๐Ÿ”ด **CRITICAL (still open) โ€” daily sealed reveal.** Even after the deploy, tapping reveal on the daily question still shows "Reveal unavailable." `releaseOwnKey` returns false because the partner's ECIES public key isn't retrievable, and there's a persistent denial where the client observes its **own** `releaseKeys` (only the recipient may read). This is a deeper sealed-reveal (ECIES key-exchange) issue, separate from the rules already fixed โ€” **not yet resolved.** + +### Status by game (after fixes) +| Game | Start | Reveal | Notes | +|---|---|---|---| +| This or That | โœ… fixed | โœ… verified live | crash fixed | +| Desire Sync | โœ… fixed (same path) | โณ not re-run | shares `startGame` fix | +| How Well | โœ… fixed (same path) | โณ not re-run | shares `startGame` fix | +| Spin the Wheel | โœ… (route-arg id, guarded) | โณ not re-run | โ€” | +| Date Match | โณ | โณ | needs the date-swipe rules/functions deployed | +| Connection Challenges | โณ | โณ | completion-based | +| **Daily reveal** | โœ… both answer | ๐Ÿ”ด **fails** | sealed ECIES key-exchange broken | + +## โœ… DAILY REVEAL โ€” FIXED & VERIFIED LIVE (2026-06-24, two devices + admin) +Used admin reads to find ground truth (both public keys WERE published, both answers existed, but **no release keys** were ever written). Three bugs were blocking the sealed reveal โ€” all fixed: + +1. **Sender couldn't write its release key.** `writeReleaseKey` did an existence-check `ref.get()` first, but the releaseKeys read rule is recipient-only โ†’ the sender's `get()` threw `PERMISSION_DENIED` โ†’ `releaseOwnKey` threw โ†’ "Reveal unavailable." **Fix:** tolerate the denied existence-read (treat as "not there, create it"). Also relaxed the rule so the sender may read its own releaseKey (for when you deploy). ([FirestoreReleaseKeyDataSource.kt](app/src/main/java/app/closer/data/remote/FirestoreReleaseKeyDataSource.kt), [firestore.rules](firestore.rules)) +2. **Reveal crashed reading the partner's answer.** `markAnswerKeyReleased` wrote `updatedAt` as a Firestore `serverTimestamp()`, but `toLocalAnswer` read it with `getLong()` โ†’ `RuntimeException: Field 'updatedAt' is not a java.lang.Number` โ†’ hard crash when the partner opened the reveal. **Fix:** write `updatedAt` as epoch-millis, and read time fields defensively (Long / Timestamp / Date). ([FirestoreAnswerDataSource.kt](app/src/main/java/app/closer/data/remote/FirestoreAnswerDataSource.kt)) +3. **Partner answer showed the raw option id** ("a_photo" instead of "A photo") โ€” the sealed payload stores only ids. **Fix:** map ids โ†’ labels via the question in the reveal VM. ([AnswerRevealViewModel.kt](app/src/main/java/app/closer/ui/answers/AnswerRevealViewModel.kt)) + +**Verified end-to-end on both emulators:** A revealed โ†’ "your key is released, waiting"; B revealed โ†’ saw **A's** answer decrypted; A then saw **B's** answer decrypted. Both partners see each other's answers, no crash. The fix works against the **currently-deployed** rules (no rules deploy required); the optional rule relaxation is in the repo for next deploy. + +## Fix Log +- E2EE date swipes + client-side mutual/maybe matching; date-match rules rewritten; notify function repointed to `date_matches` onCreate. +- Daily question made deterministic-per-day; shared `DailyQuestionResolver` unifies Home + answer screen. +- Re-entry pre-check (`getAnswers` โ†’ WAITING) added to This or That, How Well, Desire Sync. +- Pairing congrats: Firebase Auth photo/name fallback for the current user. diff --git a/app/src/main/java/app/closer/core/navigation/AppNavigation.kt b/app/src/main/java/app/closer/core/navigation/AppNavigation.kt index ae192341..a3e3e2dd 100644 --- a/app/src/main/java/app/closer/core/navigation/AppNavigation.kt +++ b/app/src/main/java/app/closer/core/navigation/AppNavigation.kt @@ -246,6 +246,10 @@ fun AppNavigation( nullable = true defaultValue = null } + ), + // Lets the "your partner sent a message" notification open the conversation. + deepLinks = listOf( + navDeepLink { uriPattern = "closer://closer.app/question_thread/{coupleId}/{questionId}" } ) ) { QuestionThreadScreen( diff --git a/app/src/main/java/app/closer/data/remote/FirestoreAnswerDataSource.kt b/app/src/main/java/app/closer/data/remote/FirestoreAnswerDataSource.kt index 1ebf8afa..7ad490c2 100644 --- a/app/src/main/java/app/closer/data/remote/FirestoreAnswerDataSource.kt +++ b/app/src/main/java/app/closer/data/remote/FirestoreAnswerDataSource.kt @@ -112,7 +112,10 @@ class FirestoreAnswerDataSource @Inject constructor( suspend fun markAnswerKeyReleased(coupleId: String, date: String, userId: String): Unit = suspendCancellableCoroutine { cont -> answerRef(coupleId, date, userId) - .update(mapOf("answerKeyReleased" to true, "updatedAt" to com.google.firebase.firestore.FieldValue.serverTimestamp())) + // updatedAt is stored as epoch millis everywhere else; a serverTimestamp here made + // it a Firestore Timestamp, which crashed toLocalAnswer's getLong("updatedAt") when + // the partner read this answer during reveal. Keep it a Long. + .update(mapOf("answerKeyReleased" to true, "updatedAt" to System.currentTimeMillis())) .addOnSuccessListener { cont.resume(Unit) } .addOnFailureListener { cont.resumeWithException(it) } } @@ -201,8 +204,8 @@ class FirestoreAnswerDataSource @Inject constructor( questionText = "", category = "", answerType = getString("answerType") ?: "written", - createdAt = getLong("createdAt") ?: System.currentTimeMillis(), - updatedAt = getLong("updatedAt") ?: System.currentTimeMillis(), + createdAt = longOrTimestampMillis("createdAt"), + updatedAt = longOrTimestampMillis("updatedAt"), isRevealed = getBoolean("isRevealed") ?: false, schemaVersion = 3, isSealed = getBoolean("answerKeyReleased") != true, @@ -211,6 +214,19 @@ class FirestoreAnswerDataSource @Inject constructor( ) } + /** + * Reads an epoch-millis field that may have been written as a Long, a Firestore Timestamp, + * or a Date (older docs / serverTimestamp writes). getLong() throws on a non-number field, + * which previously crashed the reveal โ€” this tolerates every shape. + */ + private fun com.google.firebase.firestore.DocumentSnapshot.longOrTimestampMillis(field: String): Long = + when (val v = get(field)) { + is Number -> v.toLong() + is com.google.firebase.Timestamp -> v.toDate().time + is java.util.Date -> v.time + else -> System.currentTimeMillis() + } + /** * Saves a "Couple Lore" entry to Firestore. * Path: couples/{coupleId}/lore/{questionId} diff --git a/app/src/main/java/app/closer/data/remote/FirestoreReleaseKeyDataSource.kt b/app/src/main/java/app/closer/data/remote/FirestoreReleaseKeyDataSource.kt index 42c2f765..c91fcb96 100644 --- a/app/src/main/java/app/closer/data/remote/FirestoreReleaseKeyDataSource.kt +++ b/app/src/main/java/app/closer/data/remote/FirestoreReleaseKeyDataSource.kt @@ -32,7 +32,10 @@ class FirestoreReleaseKeyDataSource @Inject constructor( encryptedAnswerKey: String ) { val ref = releaseKeyRef(coupleId, date, senderUserId, recipientUserId) - if (ref.get().await().exists()) return + // The sender cannot READ this doc under the (recipient-only) read rule, so the existence + // check must tolerate a PERMISSION_DENIED โ€” treat "can't read" as "not there yet" and + // proceed to create. Pre-fix, this get() threw and the whole reveal failed. + if (runCatching { ref.get().await().exists() }.getOrDefault(false)) return ref.set( mapOf( "recipientUserId" to recipientUserId, @@ -68,7 +71,8 @@ class FirestoreReleaseKeyDataSource @Inject constructor( encryptedAnswerKey: String ) { val ref = threadReleaseKeyRef(coupleId, threadId, senderUserId, recipientUserId) - if (ref.get().await().exists()) return + // Sender can't read this doc (recipient-only read rule); tolerate the denied get(). + if (runCatching { ref.get().await().exists() }.getOrDefault(false)) return ref.set( mapOf( "recipientUserId" to recipientUserId, diff --git a/app/src/main/java/app/closer/data/repository/QuestionSessionRepositoryImpl.kt b/app/src/main/java/app/closer/data/repository/QuestionSessionRepositoryImpl.kt index ed312031..18e0a4ec 100644 --- a/app/src/main/java/app/closer/data/repository/QuestionSessionRepositoryImpl.kt +++ b/app/src/main/java/app/closer/data/repository/QuestionSessionRepositoryImpl.kt @@ -19,7 +19,7 @@ class QuestionSessionRepositoryImpl @Inject constructor( private val crashReporter: CrashReporter ) : QuestionSessionRepository { - override suspend fun saveSession(session: QuestionSession): Result = runCatching { + override suspend fun saveSession(session: QuestionSession): Result = runCatching { val doc = if (session.id.isBlank()) { firestore.collection(FirestoreCollections.COUPLES) .document(session.coupleId) @@ -47,6 +47,10 @@ class QuestionSessionRepositoryImpl @Inject constructor( "completedByUsers" to session.completedByUsers ) doc.set(data).await() + // Return the (possibly auto-generated) document id so callers can observe the + // freshly-created session. Previously this returned Unit, so a started game carried + // an empty id โ†’ observeAnswers built an invalid 3-segment path โ†’ crash on game start. + doc.id } override suspend fun getSessionsForCouple(coupleId: String): Result> = @@ -187,7 +191,7 @@ class QuestionSessionRepositoryImpl @Inject constructor( completedAt = System.currentTimeMillis(), status = "completed" ) - ).getOrThrow() + ).map { }.getOrThrow() } override suspend fun getSessionById(coupleId: String, sessionId: String): QuestionSession? = diff --git a/app/src/main/java/app/closer/data/repository/QuestionThreadRepositoryImpl.kt b/app/src/main/java/app/closer/data/repository/QuestionThreadRepositoryImpl.kt index 02978726..9d872b8d 100644 --- a/app/src/main/java/app/closer/data/repository/QuestionThreadRepositoryImpl.kt +++ b/app/src/main/java/app/closer/data/repository/QuestionThreadRepositoryImpl.kt @@ -43,7 +43,10 @@ class QuestionThreadRepositoryImpl @Inject constructor( countAfter == 1 -> QuestionThreadStatus.ANSWERED_BY_ONE else -> QuestionThreadStatus.NOT_STARTED } - dataSource.updateThreadStatus(coupleId, threadId, newStatus) + // The thread's phase is driven by the answer docs, not this status field, so a failed + // status write (e.g. a legacy lowercase doc the rules won't transition) must NOT fail + // the whole submit โ€” that previously surfaced "Thread unavailable" and blocked discussion. + runCatching { dataSource.updateThreadStatus(coupleId, threadId, newStatus) } if (countBefore < 2 && newStatus == QuestionThreadStatus.REVEALED) { runCatching { coupleRepository.updateStreak(coupleId) } } diff --git a/app/src/main/java/app/closer/di/NotificationModule.kt b/app/src/main/java/app/closer/di/NotificationModule.kt index 46e930b3..1e771e27 100644 --- a/app/src/main/java/app/closer/di/NotificationModule.kt +++ b/app/src/main/java/app/closer/di/NotificationModule.kt @@ -37,11 +37,13 @@ object NotificationModule { @ApplicationContext context: Context, settingsRepository: SettingsRepository, quietHoursManager: QuietHoursManager, - rateLimiter: NotificationRateLimiter + rateLimiter: NotificationRateLimiter, + activeThreadMonitor: app.closer.notifications.ActiveThreadMonitor ): PartnerNotificationManager = PartnerNotificationManager( context, settingsRepository, quietHoursManager, - rateLimiter + rateLimiter, + activeThreadMonitor ) } diff --git a/app/src/main/java/app/closer/domain/model/QuestionThread.kt b/app/src/main/java/app/closer/domain/model/QuestionThread.kt index 83195a7e..3a40125e 100644 --- a/app/src/main/java/app/closer/domain/model/QuestionThread.kt +++ b/app/src/main/java/app/closer/domain/model/QuestionThread.kt @@ -17,13 +17,16 @@ enum class QuestionThreadStatus { REVEALED, COMPLETED; - fun toFirestoreValue(): String = name.lowercase() + // Stored UPPERCASE to match the Firestore rules' status-transition checks + // (NOT_STARTED โ†’ ANSWERED_BY_ONE โ†’ REVEALED โ†’ COMPLETED). Lowercase here meant every + // status update was rejected, which broke the discussion thread. Reads tolerate both cases. + fun toFirestoreValue(): String = name companion object { - fun fromFirestoreValue(value: String): QuestionThreadStatus = when (value) { - "answered_by_one" -> ANSWERED_BY_ONE - "revealed" -> REVEALED - "completed" -> COMPLETED + fun fromFirestoreValue(value: String): QuestionThreadStatus = when (value.uppercase()) { + "ANSWERED_BY_ONE" -> ANSWERED_BY_ONE + "REVEALED" -> REVEALED + "COMPLETED" -> COMPLETED else -> NOT_STARTED } } diff --git a/app/src/main/java/app/closer/domain/repository/QuestionSessionRepository.kt b/app/src/main/java/app/closer/domain/repository/QuestionSessionRepository.kt index 240cc097..e80d7d2a 100644 --- a/app/src/main/java/app/closer/domain/repository/QuestionSessionRepository.kt +++ b/app/src/main/java/app/closer/domain/repository/QuestionSessionRepository.kt @@ -4,7 +4,8 @@ import app.closer.domain.model.QuestionSession import kotlinx.coroutines.flow.Flow interface QuestionSessionRepository { - suspend fun saveSession(session: QuestionSession): Result + /** Saves the session and returns its (possibly auto-generated) document id. */ + suspend fun saveSession(session: QuestionSession): Result suspend fun getSessionsForCouple(coupleId: String): Result> // Active session queries diff --git a/app/src/main/java/app/closer/domain/usecase/GameSessionManager.kt b/app/src/main/java/app/closer/domain/usecase/GameSessionManager.kt index 1bb0e887..3c619c4b 100644 --- a/app/src/main/java/app/closer/domain/usecase/GameSessionManager.kt +++ b/app/src/main/java/app/closer/domain/usecase/GameSessionManager.kt @@ -101,7 +101,9 @@ class GameSessionManager @Inject constructor( status = "active" ) - return sessionRepository.saveSession(session).map { session } + // Use the id assigned by the repository so the started game observes the right doc + // (an empty id builds an invalid Firestore path and crashes the game on open). + return sessionRepository.saveSession(session).map { id -> session.copy(id = id) } } /** @@ -136,7 +138,7 @@ class GameSessionManager @Inject constructor( // Propagate a failed write: saveSession returns Result.failure rather than // throwing, so without getOrThrow() the outer runCatching would report success // and leave the session stuck "active" (locking the couple out of new games). - sessionRepository.saveSession(updatedSession).getOrThrow() + sessionRepository.saveSession(updatedSession).map { }.getOrThrow() } /** diff --git a/app/src/main/java/app/closer/notifications/ActiveThreadMonitor.kt b/app/src/main/java/app/closer/notifications/ActiveThreadMonitor.kt new file mode 100644 index 00000000..8460660f --- /dev/null +++ b/app/src/main/java/app/closer/notifications/ActiveThreadMonitor.kt @@ -0,0 +1,27 @@ +package app.closer.notifications + +import javax.inject.Inject +import javax.inject.Singleton + +/** + * Tracks which discussion thread (by questionId) the user is currently viewing, so an incoming + * "your partner sent a message" notification can be suppressed while they're already reading that + * conversation live. Set on entering the thread screen, cleared on leaving. + * + * Foreground-only by nature: when the app is backgrounded, FCM auto-displays the system-tray + * notification and this monitor isn't consulted โ€” which is the desired behaviour. + */ +@Singleton +class ActiveThreadMonitor @Inject constructor() { + @Volatile + var activeQuestionId: String? = null + private set + + fun enter(questionId: String) { + if (questionId.isNotBlank()) activeQuestionId = questionId + } + + fun leave(questionId: String) { + if (activeQuestionId == questionId) activeQuestionId = null + } +} diff --git a/app/src/main/java/app/closer/notifications/PartnerNotificationManager.kt b/app/src/main/java/app/closer/notifications/PartnerNotificationManager.kt index a5c66382..9166a863 100644 --- a/app/src/main/java/app/closer/notifications/PartnerNotificationManager.kt +++ b/app/src/main/java/app/closer/notifications/PartnerNotificationManager.kt @@ -33,7 +33,8 @@ class PartnerNotificationManager @Inject constructor( @ApplicationContext private val context: Context, private val settingsRepository: SettingsRepository, private val quietHoursManager: QuietHoursManager, - private val rateLimiter: NotificationRateLimiter + private val rateLimiter: NotificationRateLimiter, + private val activeThreadMonitor: ActiveThreadMonitor ) { /** @@ -53,6 +54,12 @@ class PartnerNotificationManager @Inject constructor( val settings = settingsRepository.settings.first() + // Don't ping for a chat message in the thread the user is already reading live. + if (type == PartnerNotificationType.CHAT_MESSAGE && + payload.questionId != null && + payload.questionId == activeThreadMonitor.activeQuestionId + ) return + if (!isEnabled(type, settings)) return if (quietHoursManager.isInQuietHours(settings.quietHours)) return if (!rateLimiter.canSend(type.rateType)) return @@ -251,7 +258,12 @@ enum class PartnerNotificationType( CAPSULE_UNLOCKED -> AppRoute.MEMORY_LANE GENTLE_REMINDER -> AppRoute.DAILY_QUESTION DAILY_QUESTION_REMINDER -> AppRoute.DAILY_QUESTION - CHAT_MESSAGE -> AppRoute.ANSWER_HISTORY + // Open the actual discussion thread so the partner can reply in place. + CHAT_MESSAGE -> if (payload.questionId != null && coupleId.isNotBlank()) { + AppRoute.questionThread(coupleId, payload.questionId) + } else { + AppRoute.ANSWER_HISTORY + } OUTCOME_REMINDER -> AppRoute.SETTINGS PARTNER_JOINED -> if (coupleId.isNotBlank()) AppRoute.pairingSuccess(coupleId) else AppRoute.HOME DATE_MATCH -> AppRoute.DATE_MATCHES diff --git a/app/src/main/java/app/closer/ui/answers/AnswerRevealViewModel.kt b/app/src/main/java/app/closer/ui/answers/AnswerRevealViewModel.kt index 56c1cdbb..45e560dc 100644 --- a/app/src/main/java/app/closer/ui/answers/AnswerRevealViewModel.kt +++ b/app/src/main/java/app/closer/ui/answers/AnswerRevealViewModel.kt @@ -305,9 +305,15 @@ class AnswerRevealViewModel @Inject constructor( return } + // The sealed payload carries only option IDs; map them to labels via the question so the + // reveal shows "A photo" rather than the raw "a_photo" id (own answer already has labels). + val partnerOptionTexts = state.question + ?.let { app.closer.ui.questions.selectedOptionTexts(it, payload.selectedOptionIds) } + ?: emptyList() val decryptedPartnerAnswer = state.partnerAnswer?.copy( writtenText = payload.writtenText, selectedOptionIds = payload.selectedOptionIds, + selectedOptionTexts = partnerOptionTexts, scaleValue = payload.scaleValue, isSealed = false ) diff --git a/app/src/main/java/app/closer/ui/desiresync/DesireSyncScreen.kt b/app/src/main/java/app/closer/ui/desiresync/DesireSyncScreen.kt index 6cc9cdf4..8eba5a66 100644 --- a/app/src/main/java/app/closer/ui/desiresync/DesireSyncScreen.kt +++ b/app/src/main/java/app/closer/ui/desiresync/DesireSyncScreen.kt @@ -222,7 +222,18 @@ class DesireSyncViewModel @Inject constructor( val byId = loadNeutralQuestions().associateBy { it.id } val questions = questionIds.mapNotNull { byId[it] } if (questions.isEmpty()) return fail("Couldn't load this round's questions.") - _uiState.update { it.copy(phase = DesireSyncPhase.INTRO, questions = questions) } + // Re-opened after answering? Skip straight to WAITING instead of re-prompting. + val uid = userId + val cId = coupleId + val alreadyAnswered = uid != null && cId != null && + runCatching { dataSource.getAnswers(cId, existingSessionId)?.byUser?.get(uid)?.isNotEmpty() == true } + .getOrDefault(false) + _uiState.update { + it.copy( + phase = if (alreadyAnswered) DesireSyncPhase.WAITING else DesireSyncPhase.INTRO, + questions = questions + ) + } observeReveal() } @@ -267,7 +278,7 @@ class DesireSyncViewModel @Inject constructor( private suspend fun submitAnswers(answers: List) { submitted = true val cId = coupleId ?: return - val sId = sessionId ?: return + val sId = sessionId?.takeIf { it.isNotBlank() } ?: return val uid = userId ?: return runCatching { dataSource.submitAnswers(cId, sId, uid, answers) } .onFailure { e -> @@ -288,7 +299,7 @@ class DesireSyncViewModel @Inject constructor( /** Single source of truth for WAITING/REVEAL: driven by what's in Firestore. */ private fun observeReveal() { val cId = coupleId ?: return - val sId = sessionId ?: return + val sId = sessionId?.takeIf { it.isNotBlank() } ?: return observeJob?.cancel() observeJob = viewModelScope.launch { dataSource.observeAnswers(cId, sId).collect { answers -> @@ -324,7 +335,7 @@ class DesireSyncViewModel @Inject constructor( /** Both answered โ€” mark this user done; session auto-completes once both sides recorded. */ private fun finishSession() { val cId = coupleId ?: return - val sId = sessionId ?: return + val sId = sessionId?.takeIf { it.isNotBlank() } ?: return viewModelScope.launch { gameSessionManager.markUserComplete(sId, cId) .onFailure { Log.d(TAG, "finishSession no-op: ${it.message}") } diff --git a/app/src/main/java/app/closer/ui/howwell/HowWellScreen.kt b/app/src/main/java/app/closer/ui/howwell/HowWellScreen.kt index 919b67ee..e7bff9e1 100644 --- a/app/src/main/java/app/closer/ui/howwell/HowWellScreen.kt +++ b/app/src/main/java/app/closer/ui/howwell/HowWellScreen.kt @@ -263,8 +263,17 @@ class HowWellViewModel @Inject constructor( startedByUserId = startedBy val questions = questionIds.mapNotNull { repository.getQuestionById(it) } if (questions.isEmpty()) return fail("Couldn't load this round's questions.") + // Re-opened after answering? Skip straight to WAITING instead of re-prompting. + val cId = coupleId + val alreadyAnswered = cId != null && + runCatching { dataSource.getAnswers(cId, existingSessionId)?.byUser?.get(uid)?.isNotEmpty() == true } + .getOrDefault(false) _uiState.update { - it.copy(phase = HowWellPhase.INTRO, questions = questions, amSubject = startedBy == uid) + it.copy( + phase = if (alreadyAnswered) HowWellPhase.WAITING else HowWellPhase.INTRO, + questions = questions, + amSubject = startedBy == uid + ) } observeReveal() } @@ -297,7 +306,7 @@ class HowWellViewModel @Inject constructor( private suspend fun submitAnswers() { submitted = true val cId = coupleId ?: return - val sId = sessionId ?: return + val sId = sessionId?.takeIf { it.isNotBlank() } ?: return val uid = userId ?: return runCatching { dataSource.submitAnswers(cId, sId, uid, myAnswers.toList()) } .onFailure { e -> @@ -317,7 +326,7 @@ class HowWellViewModel @Inject constructor( /** Single source of truth for WAITING/COMPLETE: driven by what's in Firestore. */ private fun observeReveal() { val cId = coupleId ?: return - val sId = sessionId ?: return + val sId = sessionId?.takeIf { it.isNotBlank() } ?: return observeJob?.cancel() observeJob = viewModelScope.launch { dataSource.observeAnswers(cId, sId).collect { answers -> @@ -362,7 +371,7 @@ class HowWellViewModel @Inject constructor( /** Both answered โ€” mark this user done; session auto-completes once both sides recorded. */ private fun finishSession() { val cId = coupleId ?: return - val sId = sessionId ?: return + val sId = sessionId?.takeIf { it.isNotBlank() } ?: return viewModelScope.launch { gameSessionManager.markUserComplete(sId, cId) .onFailure { Log.d(TAG, "finishSession no-op: ${it.message}") } diff --git a/app/src/main/java/app/closer/ui/questions/LocalAnswerMapping.kt b/app/src/main/java/app/closer/ui/questions/LocalAnswerMapping.kt index 767ce508..d03ee5e0 100644 --- a/app/src/main/java/app/closer/ui/questions/LocalAnswerMapping.kt +++ b/app/src/main/java/app/closer/ui/questions/LocalAnswerMapping.kt @@ -32,7 +32,7 @@ fun LocalQuestionUiState.toLocalAnswer(question: Question): LocalAnswer { ) } -private fun selectedOptionTexts( +internal fun selectedOptionTexts( question: Question, selectedOptionIds: List ): List { diff --git a/app/src/main/java/app/closer/ui/questions/QuestionThreadViewModel.kt b/app/src/main/java/app/closer/ui/questions/QuestionThreadViewModel.kt index 00c17169..957eb33e 100644 --- a/app/src/main/java/app/closer/ui/questions/QuestionThreadViewModel.kt +++ b/app/src/main/java/app/closer/ui/questions/QuestionThreadViewModel.kt @@ -48,6 +48,7 @@ class QuestionThreadViewModel @Inject constructor( private val repository: QuestionThreadRepository, private val questionDao: QuestionDao, private val sealedRevealManager: SealedRevealManager, + private val activeThreadMonitor: app.closer.notifications.ActiveThreadMonitor, savedStateHandle: SavedStateHandle ) : ViewModel() { @@ -67,9 +68,17 @@ class QuestionThreadViewModel @Inject constructor( val uiState: StateFlow = _uiState.asStateFlow() init { + // While this thread is open, suppress its own "new message" notifications (the user is + // reading it live). Cleared in onCleared when they leave. + activeThreadMonitor.enter(questionId) loadThread() } + override fun onCleared() { + super.onCleared() + activeThreadMonitor.leave(questionId) + } + private fun loadThread() { viewModelScope.launch { try { diff --git a/app/src/main/java/app/closer/ui/thisorthat/ThisOrThatScreen.kt b/app/src/main/java/app/closer/ui/thisorthat/ThisOrThatScreen.kt index e7ce1a83..ce12d32d 100644 --- a/app/src/main/java/app/closer/ui/thisorthat/ThisOrThatScreen.kt +++ b/app/src/main/java/app/closer/ui/thisorthat/ThisOrThatScreen.kt @@ -259,14 +259,27 @@ class ThisOrThatViewModel @Inject constructor( .associateBy { it.id } val questions = questionIds.mapNotNull { byId[it] } if (questions.isEmpty()) return fail("Couldn't load this round's questions.") - _uiState.update { it.copy(phase = TotPhase.PLAYING, questions = questions) } + // Re-opened while waiting? Don't re-ask โ€” if I've already submitted my picks, go + // straight to WAITING; observeReveal upgrades to the reveal once both are in. + val uid = userId + val cId = coupleId + val alreadyAnswered = uid != null && cId != null && + runCatching { dataSource.getAnswers(cId, existingSessionId)?.byUser?.get(uid)?.isNotEmpty() == true } + .getOrDefault(false) + if (alreadyAnswered) submitted = true + _uiState.update { + it.copy( + phase = if (alreadyAnswered) TotPhase.WAITING else TotPhase.PLAYING, + questions = questions + ) + } observeReveal() } /** Single source of truth for WAITING/REVEAL: driven by what's in Firestore. */ private fun observeReveal() { val cId = coupleId ?: return - val sId = sessionId ?: return + val sId = sessionId?.takeIf { it.isNotBlank() } ?: return observeJob?.cancel() observeJob = viewModelScope.launch { dataSource.observeAnswers(cId, sId).collect { answers -> @@ -310,7 +323,7 @@ class ThisOrThatViewModel @Inject constructor( private suspend fun submitAnswers(answers: List) { submitted = true val cId = coupleId ?: return - val sId = sessionId ?: return + val sId = sessionId?.takeIf { it.isNotBlank() } ?: return val uid = userId ?: return runCatching { dataSource.submitAnswers(cId, sId, uid, answers) } .onFailure { e -> @@ -358,7 +371,7 @@ class ThisOrThatViewModel @Inject constructor( /** Both answered โ€” mark this user done; session auto-completes once both sides recorded. */ private fun finishSession() { val cId = coupleId ?: return - val sId = sessionId ?: return + val sId = sessionId?.takeIf { it.isNotBlank() } ?: return viewModelScope.launch { gameSessionManager.markUserComplete(sId, cId) .onFailure { Log.d(TAG, "finishSession no-op: ${it.message}") } diff --git a/firestore.rules b/firestore.rules index 6f017531..a20ce2c1 100644 --- a/firestore.rules +++ b/firestore.rules @@ -158,7 +158,16 @@ service cloud.firestore { // hasPremium is server-only: clients may not write it directly. match /users/{uid} { - allow read: if isOwner(uid); + // Owner reads their own doc; a paired partner may read the other's doc (name + photo) + // โ€” they share a coupleId. Without this, partner name/photo never load (shows + // "Your partner" / blank avatar everywhere: pairing screen, Home, games). + allow read: if isOwner(uid) + || ( + request.auth != null + && resource.data.coupleId != null + && exists(/databases/$(database)/documents/users/$(request.auth.uid)) + && get(/databases/$(database)/documents/users/$(request.auth.uid)).data.coupleId == resource.data.coupleId + ); allow create: if isOwner(uid) && !request.resource.data.keys().hasAny(['hasPremium']); allow update: if isOwner(uid) @@ -582,8 +591,13 @@ service cloud.firestore { // Release keys: the sender releases their one-time answer key to the recipient // after both partners have submitted. match /releaseKeys/{recipientId} { - // Only the recipient can read their own release key. - allow read: if request.auth.uid == recipientId && isCouplesMember(coupleId); + // The recipient reads the key released to them. The sender (answer owner = {userId}) + // must also be able to read their own released doc, because writeReleaseKey does an + // idempotency existence-check get() before writing โ€” without this, that get() was + // PERMISSION_DENIED, releaseOwnKey threw, and the daily reveal failed. The keybox is + // ECIES-encrypted to the recipient, so the sender reading it leaks nothing. + allow read: if isCouplesMember(coupleId) + && (request.auth.uid == recipientId || request.auth.uid == userId); // Create-only: written by the answer owner (sender) after both answers exist. allow create: if isCouplesMember(coupleId) diff --git a/functions/src/questions/onMessageWritten.ts b/functions/src/questions/onMessageWritten.ts index 155cb9bb..1d53b319 100644 --- a/functions/src/questions/onMessageWritten.ts +++ b/functions/src/questions/onMessageWritten.ts @@ -39,6 +39,14 @@ export const onMessageWritten = functions.firestore return } + // The conversation deep link + the client's "am I already in this thread?" suppression both + // key off questionId, so resolve it from the thread doc and pass it through. + const threadDoc = await db + .collection('couples').doc(coupleId) + .collection('question_threads').doc(threadId) + .get() + const questionId = (threadDoc.data()?.questionId as string | undefined) ?? '' + const partnerUserDoc = await db.collection('users').doc(partnerId).get() // Respect the partner's notification preference (opt-out; default is enabled). @@ -82,6 +90,7 @@ export const onMessageWritten = functions.firestore type: 'chat_message', couple_id: coupleId, thread_id: threadId, + ...(questionId ? { question_id: questionId } : {}), }, }