From 6413ef771fb69232326b320a93df943f4259f1d3 Mon Sep 17 00:00:00 2001 From: markm39 Date: Wed, 23 Sep 2026 22:16:13 -0500 Subject: [PATCH 1/2] fix(reviews): restore earlier usage-based review prompts --- app/folder/[id].tsx | 4 + app/index.tsx | 4 + app/note/[id].tsx | 4 + package.json | 4 +- scripts/lifecyclePolicy.test.mjs | 39 +----- scripts/reviewPromptService.test.mjs | 179 +++++++++++++++++++++++++++ src/hooks/useLibrarySupport.ts | 8 +- src/services/lifecyclePolicy.ts | 37 +----- src/services/lifecycleService.ts | 45 ------- src/services/reviewPromptService.ts | 121 ++++++++++++++++++ 10 files changed, 322 insertions(+), 123 deletions(-) create mode 100644 scripts/reviewPromptService.test.mjs create mode 100644 src/services/reviewPromptService.ts diff --git a/app/folder/[id].tsx b/app/folder/[id].tsx index dbefc57..2129b2b 100644 --- a/app/folder/[id].tsx +++ b/app/folder/[id].tsx @@ -27,6 +27,7 @@ import { listFolders, renameFolder, } from '../../src/services/foldersRepo'; +import { recordReviewSignal } from '../../src/services/reviewPromptService'; import type { BackgroundType, FolderMetadata, NoteMetadata } from '../../src/types/note'; type Action = @@ -82,12 +83,14 @@ export default function FolderScreen() { backgroundType, title: title.trim() || undefined, }); + void recordReviewSignal('note_created'); router.push(`/note/${meta.id}`); return; } const meta = await createPdfNoteFromPicker({ folderId: folder.id, title }); if (meta) { + void recordReviewSignal('note_created'); router.push(`/note/${meta.id}`); } } catch (error) { @@ -166,6 +169,7 @@ export default function FolderScreen() { key={note.id} note={note} onPress={() => { + void recordReviewSignal('note_opened'); router.push(`/note/${note.id}`); }} onLongPress={() => { diff --git a/app/index.tsx b/app/index.tsx index c4e8ea7..5d3288d 100644 --- a/app/index.tsx +++ b/app/index.tsx @@ -41,6 +41,7 @@ import { listFolders, renameFolder, } from '../src/services/foldersRepo'; +import { recordReviewSignal } from '../src/services/reviewPromptService'; import type { BackgroundType, FolderMetadata, NoteMetadata } from '../src/types/note'; import { t } from '../src/i18n'; @@ -127,6 +128,7 @@ export default function LibraryScreen() { const openNote = useCallback( (id: string) => { void Haptics.selectionAsync(); + void recordReviewSignal('note_opened'); router.push(`/note/${id}`); }, [router], @@ -151,12 +153,14 @@ export default function LibraryScreen() { backgroundType, title: title.trim() || undefined, }); + void recordReviewSignal('note_created'); openNote(meta.id); return; } const meta = await createPdfNoteFromPicker({ folderId: null, title }); if (meta) { + void recordReviewSignal('note_created'); openNote(meta.id); } } catch (error) { diff --git a/app/note/[id].tsx b/app/note/[id].tsx index de495e3..784b0a6 100644 --- a/app/note/[id].tsx +++ b/app/note/[id].tsx @@ -60,6 +60,7 @@ import { type PickedImageResult, } from '../../src/services/imageInsertStorage'; import { exportNotebookAsPdf } from '../../src/services/exportService'; +import { recordReviewSignal } from '../../src/services/reviewPromptService'; import { recordSuccessfulNoteSave } from '../../src/services/lifecycleService'; import { textBoxId, insertedElementId } from '../../src/utils/id'; import { spacing } from '../../src/theme/spacing'; @@ -223,6 +224,7 @@ export default function NoteScreen() { if (!result.ok) { throw new Error('Note body storage did not complete successfully.'); } + void recordReviewSignal('note_saved'); await recordSuccessfulNoteSave(id); }, [id, mergeStoredPreviews, rememberPagePreviews]); @@ -719,6 +721,8 @@ export default function NoteScreen() { t.editor.exportFailedTitle, result.error ?? t.editor.exportFailedBody, ); + } else { + void recordReviewSignal('note_exported'); } } catch (error) { if (__DEV__) console.warn('[NoteScreen] export failed', error); diff --git a/package.json b/package.json index 58859e7..4c89739 100644 --- a/package.json +++ b/package.json @@ -17,9 +17,9 @@ "start": "expo start", "ios": "expo run:ios", "android": "expo run:android", - "test:lifecycle": "node --experimental-strip-types --test scripts/lifecyclePolicy.test.mjs", + "test:lifecycle": "node --experimental-strip-types --test scripts/lifecyclePolicy.test.mjs scripts/reviewPromptService.test.mjs", "test:catalog": "node --experimental-strip-types --test scripts/catalogStore.test.mjs", - "test": "node --experimental-strip-types --test scripts/lifecyclePolicy.test.mjs scripts/catalogStore.test.mjs scripts/backupEngine.test.mjs", + "test": "node --experimental-strip-types --test scripts/lifecyclePolicy.test.mjs scripts/reviewPromptService.test.mjs scripts/catalogStore.test.mjs scripts/backupEngine.test.mjs", "test:backup": "node --experimental-strip-types --test scripts/backupEngine.test.mjs", "typecheck": "tsc --noEmit -p tsconfig.json" }, diff --git a/scripts/lifecyclePolicy.test.mjs b/scripts/lifecyclePolicy.test.mjs index 2ef5187..3b66cc0 100644 --- a/scripts/lifecyclePolicy.test.mjs +++ b/scripts/lifecyclePolicy.test.mjs @@ -2,14 +2,10 @@ import assert from 'node:assert/strict'; import test from 'node:test'; import { COMMUNITY_NOTE_THRESHOLD, - REVIEW_AFTER_COMMUNITY_DELAY_MS, - REVIEW_MIN_AGE_MS, - REVIEW_NOTE_THRESHOLD, createLifecycleState, normalizeLifecycleState, recordUniqueNoteSave, shouldOfferCommunity, - shouldRequestReview, } from '../src/services/lifecyclePolicy.ts'; const startedAt = '2026-01-01T00:00:00.000Z'; @@ -50,52 +46,19 @@ test('community prompt never returns after it is resolved', () => { } }); -test('review waits for five unique notes, seven days, and community handling', () => { - const now = Date.parse(startedAt) + REVIEW_MIN_AGE_MS; - const enoughNotes = stateWithSaves(REVIEW_NOTE_THRESHOLD); - const handled = { - ...enoughNotes, - communityPromptState: 'dismissed', - communityHandledAt: new Date( - now - REVIEW_AFTER_COMMUNITY_DELAY_MS, - ).toISOString(), - }; - - assert.equal( - shouldRequestReview(handled, '1.0', now - 1), - false, - ); - assert.equal(shouldRequestReview(enoughNotes, '1.0', now), false); - assert.equal(shouldRequestReview(handled, '1.0', now), true); -}); - -test('review is limited to once per app version', () => { - const state = { - ...stateWithSaves(REVIEW_NOTE_THRESHOLD), - communityPromptState: 'joined', - communityHandledAt: new Date(Date.parse(startedAt)).toISOString(), - reviewPromptedVersions: ['1.0'], - }; - const now = Date.parse(startedAt) + REVIEW_MIN_AGE_MS; - assert.equal(shouldRequestReview(state, '1.0', now), false); - assert.equal(shouldRequestReview(state, '1.1', now), true); -}); - test('normalization repairs corrupt fields and bounds saved note ids', () => { const normalized = normalizeLifecycleState( { firstSeenAt: 'not-a-date', savedNoteIds: ['a', 'a', 'b', 'c', 'd', 'e', 'f'], communityPromptState: 'unexpected', - reviewPromptedVersions: ['1.0', '1.0', '1.1'], }, startedAt, ); assert.equal(normalized.firstSeenAt, startedAt); - assert.equal(normalized.savedNoteIds.length, REVIEW_NOTE_THRESHOLD); + assert.equal(normalized.savedNoteIds.length, COMMUNITY_NOTE_THRESHOLD); assert.equal(normalized.communityPromptState, 'pending'); - assert.deepEqual(normalized.reviewPromptedVersions, ['1.0', '1.1']); }); test('normalization recovers the legacy shown state after an interrupted prompt', () => { diff --git a/scripts/reviewPromptService.test.mjs b/scripts/reviewPromptService.test.mjs new file mode 100644 index 0000000..5cbb02a --- /dev/null +++ b/scripts/reviewPromptService.test.mjs @@ -0,0 +1,179 @@ +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import test from 'node:test'; +import vm from 'node:vm'; +import ts from 'typescript'; +import { createPromiseQueue } from '../src/utils/promiseQueue.ts'; + +const key = '@opennotes:reviewPrompt:v1'; +const minute = 60 * 1000; +const day = 24 * 60 * minute; +const source = ts.transpileModule( + readFileSync(new URL('../src/services/reviewPromptService.ts', import.meta.url), 'utf8'), + { compilerOptions: { module: ts.ModuleKind.CommonJS } }, +).outputText; + +function harness(initial) { + let now = Date.parse('2026-01-01T00:00:00Z'); + let raw = initial ? JSON.stringify(initial) : null; + const calls = []; + const store = { + available: true, + action: true, + fail: false, + async isAvailableAsync() { return this.available; }, + async hasAction() { return this.action; }, + async requestReview() { + if (this.fail) throw new Error('native request failed'); + calls.push(now); + }, + }; + const storage = { + async getItem(requestedKey) { + assert.equal(requestedKey, key); + return raw; + }, + async setItem(requestedKey, value) { + assert.equal(requestedKey, key); + raw = value; + }, + }; + class Clock extends Date { + constructor(...args) { super(...(args.length ? args : [now])); } + static now() { return now; } + } + const exports = {}; + vm.runInNewContext(source, { + exports, + __DEV__: false, + Date: Clock, + require(name) { + if (name === '@react-native-async-storage/async-storage') return { default: storage }; + if (name === 'expo-store-review') return store; + if (name === '../utils/promiseQueue') return { createPromiseQueue }; + throw new Error(`Unexpected import: ${name}`); + }, + }); + return { + ...exports, calls, store, + advance(ms) { now += ms; }, + state() { return JSON.parse(raw); }, + async signals(...signals) { + await Promise.all(signals.map(exports.recordReviewSignal)); + }, + }; +} + +test('creator use prompts at 15 minutes without five unique notes or Discord', async () => { + const h = harness(); + await h.signals('note_created', 'note_created', 'note_saved', 'note_saved', 'note_saved'); + h.advance(15 * minute - 1); + await h.requestReviewAfterPositiveMoment(); + assert.equal(h.calls.length, 0); + h.advance(1); + await h.requestReviewAfterPositiveMoment(); + assert.equal(h.calls.length, 1); + assert.equal(h.state().pendingPositiveMoment, false); +}); + +test('steady use and successful export independently qualify', async () => { + for (const signals of [ + [...Array(5).fill('note_saved'), ...Array(3).fill('note_opened')], + ['note_exported', 'note_saved', 'note_saved'], + ]) { + const h = harness(); + await h.signals(...signals); + h.advance(15 * minute); + await h.requestReviewAfterPositiveMoment(); + assert.equal(h.calls.length, 1); + } +}); + +test('insufficient usage does not prompt even after seven days', async () => { + const h = harness(); + await h.signals('note_created', 'note_opened', 'note_saved'); + h.advance(7 * day); + await h.requestReviewAfterPositiveMoment(); + assert.equal(h.calls.length, 0); +}); + +test('concurrent signals and requests preserve counts and request only once', async () => { + const h = harness(); + await h.signals('note_created', 'note_created', ...Array(8).fill('note_saved')); + assert.equal(h.state().notesSaved, 8); + h.advance(15 * minute); + await Promise.all(Array.from({ length: 5 }, () => h.requestReviewAfterPositiveMoment())); + assert.equal(h.calls.length, 1); +}); + +test('cooldown requires 120 days and a new action, with a three-request cap', async () => { + const h = harness(); + await h.signals('note_exported', 'note_saved', 'note_saved'); + h.advance(15 * minute); + await h.requestReviewAfterPositiveMoment(); + h.advance(120 * day); + await h.requestReviewAfterPositiveMoment(); + assert.equal(h.calls.length, 1); + await h.signals('note_saved'); + await h.requestReviewAfterPositiveMoment(); + assert.equal(h.calls.length, 2); + await h.signals('note_saved'); + h.advance(120 * day - 1); + await h.requestReviewAfterPositiveMoment(); + assert.equal(h.calls.length, 2); + h.advance(1); + await h.requestReviewAfterPositiveMoment(); + assert.equal(h.calls.length, 3); + await h.signals('note_saved'); + h.advance(120 * day); + await h.requestReviewAfterPositiveMoment(); + assert.equal(h.calls.length, 3); +}); + +test('unavailable or failed native requests remain eligible for retry', async () => { + const h = harness(); + await h.signals('note_exported', 'note_saved', 'note_saved'); + h.advance(15 * minute); + h.store.available = false; + await h.requestReviewAfterPositiveMoment(); + h.store.available = true; + h.store.action = false; + await h.requestReviewAfterPositiveMoment(); + h.store.action = true; + h.store.fail = true; + await assert.rejects(h.requestReviewAfterPositiveMoment(), /native request failed/); + assert.equal(h.state().promptCount, 0); + assert.equal(h.state().pendingPositiveMoment, true); + h.store.fail = false; + await h.requestReviewAfterPositiveMoment(); + assert.equal(h.calls.length, 1); +}); + +test('restored service honors review history from the original release', async () => { + const h = harness({ + firstSeenAt: '2025-01-01T00:00:00Z', + lastPromptedAt: '2025-12-31T00:00:00Z', + promptCount: 1, + notesCreated: 2, + notesSaved: 3, + pendingPositiveMoment: true, + }); + await h.requestReviewAfterPositiveMoment(); + assert.equal(h.calls.length, 0); + h.advance(119 * day); + await h.requestReviewAfterPositiveMoment(); + assert.equal(h.calls.length, 1); + assert.equal(h.state().promptCount, 2); +}); + +test('manual rating works immediately and starts the automatic cooldown', async () => { + const h = harness(); + assert.equal(await h.requestManualReview(), true); + await h.signals('note_exported', 'note_saved', 'note_saved'); + h.advance(15 * minute); + await h.requestReviewAfterPositiveMoment(); + assert.equal(h.calls.length, 1); + h.advance(120 * day); + await h.requestReviewAfterPositiveMoment(); + assert.equal(h.calls.length, 2); +}); diff --git a/src/hooks/useLibrarySupport.ts b/src/hooks/useLibrarySupport.ts index 6577458..07a0fa6 100644 --- a/src/hooks/useLibrarySupport.ts +++ b/src/hooks/useLibrarySupport.ts @@ -4,9 +4,11 @@ import { useFocusEffect } from 'expo-router'; import { OPEN_NOTES_LINKS, openExternalLink } from '../services/externalLinks'; import { t } from '../i18n'; import { - claimCommunityPrompt, - requestAutomaticReviewIfEligible, requestManualReview, + requestReviewAfterPositiveMoment, +} from '../services/reviewPromptService'; +import { + claimCommunityPrompt, resolveCommunityPrompt, } from '../services/lifecycleService'; @@ -38,7 +40,7 @@ export function useLibrarySupport({ if (active) onShowCommunity(); return; } - await requestAutomaticReviewIfEligible(); + if (active) await requestReviewAfterPositiveMoment(); } catch (error) { if (__DEV__) console.warn('[useLibrarySupport] prompt failed', error); } diff --git a/src/services/lifecyclePolicy.ts b/src/services/lifecyclePolicy.ts index 368ca4c..0903adb 100644 --- a/src/services/lifecyclePolicy.ts +++ b/src/services/lifecyclePolicy.ts @@ -1,7 +1,4 @@ export const COMMUNITY_NOTE_THRESHOLD = 3; -export const REVIEW_NOTE_THRESHOLD = 5; -export const REVIEW_MIN_AGE_MS = 7 * 24 * 60 * 60 * 1000; -export const REVIEW_AFTER_COMMUNITY_DELAY_MS = 24 * 60 * 60 * 1000; export type CommunityPromptState = 'pending' | 'joined' | 'dismissed'; @@ -11,7 +8,6 @@ export interface LifecycleState { savedNoteIds: string[]; communityPromptState: CommunityPromptState; communityHandledAt: string | null; - reviewPromptedVersions: string[]; } export function createLifecycleState(now: string): LifecycleState { @@ -21,7 +17,6 @@ export function createLifecycleState(now: string): LifecycleState { savedNoteIds: [], communityPromptState: 'pending', communityHandledAt: null, - reviewPromptedVersions: [], }; } @@ -43,12 +38,11 @@ export function normalizeLifecycleState( lastSuccessfulSaveAt: isIsoDate(value.lastSuccessfulSaveAt) ? value.lastSuccessfulSaveAt : null, - savedNoteIds: uniqueStrings(value.savedNoteIds).slice(0, REVIEW_NOTE_THRESHOLD), + savedNoteIds: uniqueStrings(value.savedNoteIds).slice(0, COMMUNITY_NOTE_THRESHOLD), communityPromptState, communityHandledAt: isIsoDate(value.communityHandledAt) ? value.communityHandledAt : null, - reviewPromptedVersions: uniqueStrings(value.reviewPromptedVersions), }; } @@ -59,7 +53,7 @@ export function recordUniqueNoteSave( ): LifecycleState { const savedNoteIds = state.savedNoteIds.includes(noteId) ? state.savedNoteIds - : [...state.savedNoteIds, noteId].slice(0, REVIEW_NOTE_THRESHOLD); + : [...state.savedNoteIds, noteId].slice(0, COMMUNITY_NOTE_THRESHOLD); return { ...state, @@ -75,33 +69,6 @@ export function shouldOfferCommunity(state: LifecycleState): boolean { ); } -export function shouldRequestReview( - state: LifecycleState, - appVersion: string, - nowMs: number, -): boolean { - if (state.savedNoteIds.length < REVIEW_NOTE_THRESHOLD) return false; - if ( - state.communityPromptState !== 'joined' && - state.communityPromptState !== 'dismissed' - ) { - return false; - } - if (state.reviewPromptedVersions.includes(appVersion)) return false; - - const firstSeenMs = Date.parse(state.firstSeenAt); - const communityHandledMs = state.communityHandledAt - ? Date.parse(state.communityHandledAt) - : Number.NaN; - if (!Number.isFinite(firstSeenMs) || !Number.isFinite(communityHandledMs)) { - return false; - } - return ( - nowMs - firstSeenMs >= REVIEW_MIN_AGE_MS && - nowMs - communityHandledMs >= REVIEW_AFTER_COMMUNITY_DELAY_MS - ); -} - function isIsoDate(value: unknown): value is string { return typeof value === 'string' && Number.isFinite(Date.parse(value)); } diff --git a/src/services/lifecycleService.ts b/src/services/lifecycleService.ts index d21dd40..3f19bb4 100644 --- a/src/services/lifecycleService.ts +++ b/src/services/lifecycleService.ts @@ -1,22 +1,17 @@ import AsyncStorage from '@react-native-async-storage/async-storage'; -import Constants from 'expo-constants'; -import * as StoreReview from 'expo-store-review'; import { createLifecycleState, normalizeLifecycleState, recordUniqueNoteSave, shouldOfferCommunity, - shouldRequestReview, type CommunityPromptState, type LifecycleState, } from './lifecyclePolicy'; import { createPromiseQueue } from '../utils/promiseQueue'; const LIFECYCLE_KEY = '@opennotes:lifecycle:v1'; -const APP_VERSION = Constants.expoConfig?.version ?? 'unknown'; const stateQueue = createPromiseQueue(); -let reviewRequest: Promise | null = null; let communityPromptClaimedThisSession = false; function withStateLock(operation: () => Promise): Promise { @@ -74,43 +69,3 @@ export async function resolveCommunityPrompt( }); }); } - -export function requestAutomaticReviewIfEligible(): Promise { - if (reviewRequest) return reviewRequest; - reviewRequest = requestAutomaticReview().finally(() => { - reviewRequest = null; - }); - return reviewRequest; -} - -async function requestAutomaticReview(): Promise { - const state = await withStateLock(readStateUnlocked); - if (!shouldRequestReview(state, APP_VERSION, Date.now())) return false; - - const available = await StoreReview.isAvailableAsync(); - if (!available || !(await StoreReview.hasAction())) return false; - - await StoreReview.requestReview(); - await markReviewRequested(); - return true; -} - -export async function requestManualReview(): Promise { - const available = await StoreReview.isAvailableAsync(); - if (!available || !(await StoreReview.hasAction())) return false; - - await StoreReview.requestReview(); - await markReviewRequested(); - return true; -} - -async function markReviewRequested(): Promise { - await withStateLock(async () => { - const state = await readStateUnlocked(); - if (state.reviewPromptedVersions.includes(APP_VERSION)) return; - await writeStateUnlocked({ - ...state, - reviewPromptedVersions: [...state.reviewPromptedVersions, APP_VERSION], - }); - }); -} diff --git a/src/services/reviewPromptService.ts b/src/services/reviewPromptService.ts new file mode 100644 index 0000000..8799f8a --- /dev/null +++ b/src/services/reviewPromptService.ts @@ -0,0 +1,121 @@ +import AsyncStorage from '@react-native-async-storage/async-storage'; +import * as StoreReview from 'expo-store-review'; +import { createPromiseQueue } from '../utils/promiseQueue'; + +const stateQueue = createPromiseQueue(); + +type ReviewSignal = 'note_created' | 'note_opened' | 'note_saved' | 'note_exported'; + +interface ReviewPromptState { + firstSeenAt: string; + lastPromptedAt: string | null; + promptCount: number; + notesCreated: number; + notesOpened: number; + notesSaved: number; + notesExported: number; + pendingPositiveMoment: boolean; +} + +const KEY = '@opennotes:reviewPrompt:v1'; +const MIN_SESSION_AGE_MS = 15 * 60 * 1000; +const PROMPT_COOLDOWN_MS = 120 * 24 * 60 * 60 * 1000; +const MAX_PROMPTS = 3; + +function initialState(now: string): ReviewPromptState { + return { + firstSeenAt: now, + lastPromptedAt: null, + promptCount: 0, + notesCreated: 0, + notesOpened: 0, + notesSaved: 0, + notesExported: 0, + pendingPositiveMoment: false, + }; +} + +async function readState(): Promise { + const now = new Date().toISOString(); + const raw = await AsyncStorage.getItem(KEY); + if (!raw) return initialState(now); + try { + return { ...initialState(now), ...(JSON.parse(raw) as Partial) }; + } catch (error) { + if (__DEV__) console.warn('[reviewPromptService] invalid stored state', error); + return initialState(now); + } +} + +async function writeState(state: ReviewPromptState): Promise { + await AsyncStorage.setItem(KEY, JSON.stringify(state)); +} + +async function recordSignal(signal: ReviewSignal): Promise { + const state = await readState(); + switch (signal) { + case 'note_created': + state.notesCreated += 1; + break; + case 'note_opened': + state.notesOpened += 1; + break; + case 'note_saved': + state.notesSaved += 1; + break; + case 'note_exported': + state.notesExported += 1; + break; + } + state.pendingPositiveMoment = true; + await writeState(state); +} + +async function requestAutomaticReview(): Promise { + const state = await readState(); + if (!state.pendingPositiveMoment || state.promptCount >= MAX_PROMPTS) return; + + const now = Date.now(); + const firstSeen = Date.parse(state.firstSeenAt); + const lastPrompted = state.lastPromptedAt ? Date.parse(state.lastPromptedAt) : 0; + if (Number.isFinite(firstSeen) && now - firstSeen < MIN_SESSION_AGE_MS) return; + if (lastPrompted && now - lastPrompted < PROMPT_COOLDOWN_MS) return; + + const steadyUse = state.notesSaved >= 5 && state.notesOpened >= 3; + const creatorUse = state.notesCreated >= 2 && state.notesSaved >= 3; + const exportSuccess = state.notesExported >= 1 && state.notesSaved >= 2; + if (!steadyUse && !creatorUse && !exportSuccess) return; + + await requestReview(state); +} + +async function requestReview(state: ReviewPromptState): Promise { + const available = await StoreReview.isAvailableAsync(); + if (!available) return false; + + const hasAction = await StoreReview.hasAction(); + if (!hasAction) return false; + + await StoreReview.requestReview(); + await writeState({ + ...state, + pendingPositiveMoment: false, + promptCount: state.promptCount + 1, + lastPromptedAt: new Date().toISOString(), + }); + return true; +} + +export function recordReviewSignal(signal: ReviewSignal): Promise { + return stateQueue.enqueue(() => recordSignal(signal)).catch((error) => { + if (__DEV__) console.warn('[reviewPromptService] signal failed', error); + }); +} + +export function requestReviewAfterPositiveMoment(): Promise { + return stateQueue.enqueue(requestAutomaticReview); +} + +export function requestManualReview(): Promise { + return stateQueue.enqueue(async () => requestReview(await readState())); +} From 2b0e394b727a9b7dd7a090d3097d333b3a4dedf0 Mon Sep 17 00:00:00 2001 From: markm39 Date: Wed, 23 Sep 2026 22:24:58 -0500 Subject: [PATCH 2/2] fix(reviews): request a review after the first successful save --- app/folder/[id].tsx | 4 -- app/index.tsx | 4 -- app/note/[id].tsx | 4 -- scripts/reviewPromptService.test.mjs | 59 ++++++++++---------------- src/services/lifecycleService.ts | 2 + src/services/reviewPromptService.ts | 62 +++++++--------------------- 6 files changed, 38 insertions(+), 97 deletions(-) diff --git a/app/folder/[id].tsx b/app/folder/[id].tsx index 2129b2b..dbefc57 100644 --- a/app/folder/[id].tsx +++ b/app/folder/[id].tsx @@ -27,7 +27,6 @@ import { listFolders, renameFolder, } from '../../src/services/foldersRepo'; -import { recordReviewSignal } from '../../src/services/reviewPromptService'; import type { BackgroundType, FolderMetadata, NoteMetadata } from '../../src/types/note'; type Action = @@ -83,14 +82,12 @@ export default function FolderScreen() { backgroundType, title: title.trim() || undefined, }); - void recordReviewSignal('note_created'); router.push(`/note/${meta.id}`); return; } const meta = await createPdfNoteFromPicker({ folderId: folder.id, title }); if (meta) { - void recordReviewSignal('note_created'); router.push(`/note/${meta.id}`); } } catch (error) { @@ -169,7 +166,6 @@ export default function FolderScreen() { key={note.id} note={note} onPress={() => { - void recordReviewSignal('note_opened'); router.push(`/note/${note.id}`); }} onLongPress={() => { diff --git a/app/index.tsx b/app/index.tsx index 5d3288d..c4e8ea7 100644 --- a/app/index.tsx +++ b/app/index.tsx @@ -41,7 +41,6 @@ import { listFolders, renameFolder, } from '../src/services/foldersRepo'; -import { recordReviewSignal } from '../src/services/reviewPromptService'; import type { BackgroundType, FolderMetadata, NoteMetadata } from '../src/types/note'; import { t } from '../src/i18n'; @@ -128,7 +127,6 @@ export default function LibraryScreen() { const openNote = useCallback( (id: string) => { void Haptics.selectionAsync(); - void recordReviewSignal('note_opened'); router.push(`/note/${id}`); }, [router], @@ -153,14 +151,12 @@ export default function LibraryScreen() { backgroundType, title: title.trim() || undefined, }); - void recordReviewSignal('note_created'); openNote(meta.id); return; } const meta = await createPdfNoteFromPicker({ folderId: null, title }); if (meta) { - void recordReviewSignal('note_created'); openNote(meta.id); } } catch (error) { diff --git a/app/note/[id].tsx b/app/note/[id].tsx index 784b0a6..de495e3 100644 --- a/app/note/[id].tsx +++ b/app/note/[id].tsx @@ -60,7 +60,6 @@ import { type PickedImageResult, } from '../../src/services/imageInsertStorage'; import { exportNotebookAsPdf } from '../../src/services/exportService'; -import { recordReviewSignal } from '../../src/services/reviewPromptService'; import { recordSuccessfulNoteSave } from '../../src/services/lifecycleService'; import { textBoxId, insertedElementId } from '../../src/utils/id'; import { spacing } from '../../src/theme/spacing'; @@ -224,7 +223,6 @@ export default function NoteScreen() { if (!result.ok) { throw new Error('Note body storage did not complete successfully.'); } - void recordReviewSignal('note_saved'); await recordSuccessfulNoteSave(id); }, [id, mergeStoredPreviews, rememberPagePreviews]); @@ -721,8 +719,6 @@ export default function NoteScreen() { t.editor.exportFailedTitle, result.error ?? t.editor.exportFailedBody, ); - } else { - void recordReviewSignal('note_exported'); } } catch (error) { if (__DEV__) console.warn('[NoteScreen] export failed', error); diff --git a/scripts/reviewPromptService.test.mjs b/scripts/reviewPromptService.test.mjs index 5cbb02a..1fc47bd 100644 --- a/scripts/reviewPromptService.test.mjs +++ b/scripts/reviewPromptService.test.mjs @@ -58,73 +58,58 @@ function harness(initial) { ...exports, calls, store, advance(ms) { now += ms; }, state() { return JSON.parse(raw); }, - async signals(...signals) { - await Promise.all(signals.map(exports.recordReviewSignal)); - }, }; } -test('creator use prompts at 15 minutes without five unique notes or Discord', async () => { +test('first successful save qualifies immediately without a timer or other actions', async () => { const h = harness(); - await h.signals('note_created', 'note_created', 'note_saved', 'note_saved', 'note_saved'); - h.advance(15 * minute - 1); - await h.requestReviewAfterPositiveMoment(); - assert.equal(h.calls.length, 0); - h.advance(1); + await h.recordReviewSave(); + assert.equal(h.calls.length, 0, 'saving only records eligibility'); await h.requestReviewAfterPositiveMoment(); assert.equal(h.calls.length, 1); assert.equal(h.state().pendingPositiveMoment, false); }); -test('steady use and successful export independently qualify', async () => { - for (const signals of [ - [...Array(5).fill('note_saved'), ...Array(3).fill('note_opened')], - ['note_exported', 'note_saved', 'note_saved'], - ]) { - const h = harness(); - await h.signals(...signals); - h.advance(15 * minute); +test('no automatic prompt before a successful save, even after seven days', async () => { + for (const initial of [undefined, { + notesCreated: 20, + notesOpened: 20, + notesExported: 1, + pendingPositiveMoment: true, + }]) { + const h = harness(initial); + h.advance(7 * day); await h.requestReviewAfterPositiveMoment(); - assert.equal(h.calls.length, 1); + assert.equal(h.calls.length, 0); } }); -test('insufficient usage does not prompt even after seven days', async () => { - const h = harness(); - await h.signals('note_created', 'note_opened', 'note_saved'); - h.advance(7 * day); - await h.requestReviewAfterPositiveMoment(); - assert.equal(h.calls.length, 0); -}); - -test('concurrent signals and requests preserve counts and request only once', async () => { +test('concurrent saves and library checks preserve counts and request only once', async () => { const h = harness(); - await h.signals('note_created', 'note_created', ...Array(8).fill('note_saved')); + await Promise.all(Array.from({ length: 8 }, () => h.recordReviewSave())); assert.equal(h.state().notesSaved, 8); - h.advance(15 * minute); await Promise.all(Array.from({ length: 5 }, () => h.requestReviewAfterPositiveMoment())); assert.equal(h.calls.length, 1); }); test('cooldown requires 120 days and a new action, with a three-request cap', async () => { const h = harness(); - await h.signals('note_exported', 'note_saved', 'note_saved'); - h.advance(15 * minute); + await h.recordReviewSave(); await h.requestReviewAfterPositiveMoment(); h.advance(120 * day); await h.requestReviewAfterPositiveMoment(); assert.equal(h.calls.length, 1); - await h.signals('note_saved'); + await h.recordReviewSave(); await h.requestReviewAfterPositiveMoment(); assert.equal(h.calls.length, 2); - await h.signals('note_saved'); + await h.recordReviewSave(); h.advance(120 * day - 1); await h.requestReviewAfterPositiveMoment(); assert.equal(h.calls.length, 2); h.advance(1); await h.requestReviewAfterPositiveMoment(); assert.equal(h.calls.length, 3); - await h.signals('note_saved'); + await h.recordReviewSave(); h.advance(120 * day); await h.requestReviewAfterPositiveMoment(); assert.equal(h.calls.length, 3); @@ -132,8 +117,7 @@ test('cooldown requires 120 days and a new action, with a three-request cap', as test('unavailable or failed native requests remain eligible for retry', async () => { const h = harness(); - await h.signals('note_exported', 'note_saved', 'note_saved'); - h.advance(15 * minute); + await h.recordReviewSave(); h.store.available = false; await h.requestReviewAfterPositiveMoment(); h.store.available = true; @@ -169,8 +153,7 @@ test('restored service honors review history from the original release', async ( test('manual rating works immediately and starts the automatic cooldown', async () => { const h = harness(); assert.equal(await h.requestManualReview(), true); - await h.signals('note_exported', 'note_saved', 'note_saved'); - h.advance(15 * minute); + await h.recordReviewSave(); await h.requestReviewAfterPositiveMoment(); assert.equal(h.calls.length, 1); h.advance(120 * day); diff --git a/src/services/lifecycleService.ts b/src/services/lifecycleService.ts index 3f19bb4..96aebf9 100644 --- a/src/services/lifecycleService.ts +++ b/src/services/lifecycleService.ts @@ -8,6 +8,7 @@ import { type LifecycleState, } from './lifecyclePolicy'; import { createPromiseQueue } from '../utils/promiseQueue'; +import { recordReviewSave } from './reviewPromptService'; const LIFECYCLE_KEY = '@opennotes:lifecycle:v1'; @@ -40,6 +41,7 @@ async function writeStateUnlocked(state: LifecycleState): Promise { export async function recordSuccessfulNoteSave(noteId: string): Promise { if (!noteId) return; + await recordReviewSave(); await withStateLock(async () => { const state = await readStateUnlocked(); const next = recordUniqueNoteSave(state, noteId, new Date().toISOString()); diff --git a/src/services/reviewPromptService.ts b/src/services/reviewPromptService.ts index 8799f8a..a0c6a73 100644 --- a/src/services/reviewPromptService.ts +++ b/src/services/reviewPromptService.ts @@ -4,46 +4,34 @@ import { createPromiseQueue } from '../utils/promiseQueue'; const stateQueue = createPromiseQueue(); -type ReviewSignal = 'note_created' | 'note_opened' | 'note_saved' | 'note_exported'; - interface ReviewPromptState { - firstSeenAt: string; lastPromptedAt: string | null; promptCount: number; - notesCreated: number; - notesOpened: number; notesSaved: number; - notesExported: number; pendingPositiveMoment: boolean; } const KEY = '@opennotes:reviewPrompt:v1'; -const MIN_SESSION_AGE_MS = 15 * 60 * 1000; const PROMPT_COOLDOWN_MS = 120 * 24 * 60 * 60 * 1000; const MAX_PROMPTS = 3; -function initialState(now: string): ReviewPromptState { +function initialState(): ReviewPromptState { return { - firstSeenAt: now, lastPromptedAt: null, promptCount: 0, - notesCreated: 0, - notesOpened: 0, notesSaved: 0, - notesExported: 0, pendingPositiveMoment: false, }; } async function readState(): Promise { - const now = new Date().toISOString(); const raw = await AsyncStorage.getItem(KEY); - if (!raw) return initialState(now); + if (!raw) return initialState(); try { - return { ...initialState(now), ...(JSON.parse(raw) as Partial) }; + return { ...initialState(), ...(JSON.parse(raw) as Partial) }; } catch (error) { if (__DEV__) console.warn('[reviewPromptService] invalid stored state', error); - return initialState(now); + return initialState(); } } @@ -51,41 +39,14 @@ async function writeState(state: ReviewPromptState): Promise { await AsyncStorage.setItem(KEY, JSON.stringify(state)); } -async function recordSignal(signal: ReviewSignal): Promise { - const state = await readState(); - switch (signal) { - case 'note_created': - state.notesCreated += 1; - break; - case 'note_opened': - state.notesOpened += 1; - break; - case 'note_saved': - state.notesSaved += 1; - break; - case 'note_exported': - state.notesExported += 1; - break; - } - state.pendingPositiveMoment = true; - await writeState(state); -} - async function requestAutomaticReview(): Promise { const state = await readState(); - if (!state.pendingPositiveMoment || state.promptCount >= MAX_PROMPTS) return; + if (!state.pendingPositiveMoment || state.notesSaved < 1 || state.promptCount >= MAX_PROMPTS) return; const now = Date.now(); - const firstSeen = Date.parse(state.firstSeenAt); const lastPrompted = state.lastPromptedAt ? Date.parse(state.lastPromptedAt) : 0; - if (Number.isFinite(firstSeen) && now - firstSeen < MIN_SESSION_AGE_MS) return; if (lastPrompted && now - lastPrompted < PROMPT_COOLDOWN_MS) return; - const steadyUse = state.notesSaved >= 5 && state.notesOpened >= 3; - const creatorUse = state.notesCreated >= 2 && state.notesSaved >= 3; - const exportSuccess = state.notesExported >= 1 && state.notesSaved >= 2; - if (!steadyUse && !creatorUse && !exportSuccess) return; - await requestReview(state); } @@ -106,9 +67,16 @@ async function requestReview(state: ReviewPromptState): Promise { return true; } -export function recordReviewSignal(signal: ReviewSignal): Promise { - return stateQueue.enqueue(() => recordSignal(signal)).catch((error) => { - if (__DEV__) console.warn('[reviewPromptService] signal failed', error); +export function recordReviewSave(): Promise { + return stateQueue.enqueue(async () => { + const state = await readState(); + await writeState({ + ...state, + notesSaved: state.notesSaved + 1, + pendingPositiveMoment: true, + }); + }).catch((error) => { + if (__DEV__) console.warn('[reviewPromptService] save tracking failed', error); }); }