diff --git a/CLAUDE.md b/CLAUDE.md index ffbcf0ac2dd1..a2984caea310 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -107,8 +107,8 @@ one immediately after `loadUri`, which only queues the read. ## Review invitations Count fresh document opens, excluding reloads and app launches. Ask on the landing screen -or when a document is closed, never while opening it or after a failed load. Space asks by -5, 10, 20, 50, and 100 additional documents, at least two weeks apart, with five asks total. +or when a document is closed, never while opening it or after a failed load. Ask after every +3 additional documents, at least two weeks apart, with no limit on the number of asks. Record an ask when handed to Play, even if Play's quota prevents display. ## Tests and screenshots diff --git a/app/src/main/java/app/opendocument/droid/background/ReviewInvitation.kt b/app/src/main/java/app/opendocument/droid/background/ReviewInvitation.kt index 147b59a57ad1..1372b3feee4d 100644 --- a/app/src/main/java/app/opendocument/droid/background/ReviewInvitation.kt +++ b/app/src/main/java/app/opendocument/droid/background/ReviewInvitation.kt @@ -8,21 +8,16 @@ import android.content.Context */ object ReviewInvitation { - /** - * Documents to read before each ask, counted from the one before it. Escalating, because - * someone who did not answer the first ask has to have come a good deal further before it is - * worth spending another interruption on them - and after the fifth they have answered. - */ - private val DOCUMENTS_BEFORE_ASK = intArrayOf(5, 10, 20, 50, 100) + /** Documents to read before each ask, counted from the one before it. */ + private const val DOCUMENTS_BEFORE_ASK = 3 /** - * The rail under [DOCUMENTS_BEFORE_ASK]: a folder walked through in one afternoon crosses two - * of those, and one sitting must never carry two asks. + * The rail between asks. There is no last ask: play's own quota decides whether the sheet + * shows, and an ask it swallows still counts here. */ private const val DAYS_BETWEEN_ASKS = 14 private const val KEY_DOCUMENT_OPENS = "usage_document_opens" - private const val KEY_ASKS = "usage_review_asks" private const val KEY_ASKED_AT = "usage_review_asked_at" private const val KEY_ASKED_AFTER = "usage_review_asked_after" @@ -41,7 +36,6 @@ object ReviewInvitation { return isEarned( documentOpens = preferences.getInt(KEY_DOCUMENT_OPENS, 0), - asks = preferences.getInt(KEY_ASKS, 0), askedAfterOpens = preferences.getInt(KEY_ASKED_AFTER, 0), askedAtMillis = preferences.getLong(KEY_ASKED_AT, 0), nowMillis = System.currentTimeMillis(), @@ -51,16 +45,11 @@ object ReviewInvitation { /** The decision alone, so the jvm test can reach every branch. */ internal fun isEarned( documentOpens: Int, - asks: Int, askedAfterOpens: Int, askedAtMillis: Long, nowMillis: Long, ): Boolean { - if (asks >= DOCUMENTS_BEFORE_ASK.size) { - return false - } - - if (documentOpens - askedAfterOpens < DOCUMENTS_BEFORE_ASK[asks]) { + if (documentOpens - askedAfterOpens < DOCUMENTS_BEFORE_ASK) { return false } @@ -82,7 +71,6 @@ object ReviewInvitation { preferences .edit() - .putInt(KEY_ASKS, preferences.getInt(KEY_ASKS, 0) + 1) .putLong(KEY_ASKED_AT, System.currentTimeMillis()) .putInt(KEY_ASKED_AFTER, preferences.getInt(KEY_DOCUMENT_OPENS, 0)) .apply() diff --git a/app/src/test/java/app/opendocument/droid/background/ReviewInvitationTest.kt b/app/src/test/java/app/opendocument/droid/background/ReviewInvitationTest.kt index a9cb74b2ea2d..2ca942a41b6a 100644 --- a/app/src/test/java/app/opendocument/droid/background/ReviewInvitationTest.kt +++ b/app/src/test/java/app/opendocument/droid/background/ReviewInvitationTest.kt @@ -11,33 +11,42 @@ import org.junit.Test class ReviewInvitationTest { @Test - fun theFirstAskWaitsForFiveDocuments() { - assertFalse(earned(documentOpens = 4)) - assertTrue(earned(documentOpens = 5)) + fun theFirstAskWaitsForThreeDocuments() { + assertFalse(earned(documentOpens = 2)) + assertTrue(earned(documentOpens = 3)) } @Test fun documentsAreCountedFromTheAskBefore() { - // ten more after the ask, not ten in all - assertFalse(earned(documentOpens = 14, asks = 1, askedAfterOpens = 5)) - assertTrue(earned(documentOpens = 15, asks = 1, askedAfterOpens = 5)) - } - - @Test - fun theLadderEscalates() { - // 5, then 10, 20, 50, 100 more - assertFalse(earned(documentOpens = 19, asks = 2, askedAfterOpens = 0)) - assertTrue(earned(documentOpens = 20, asks = 2, askedAfterOpens = 0)) - assertFalse(earned(documentOpens = 49, asks = 3, askedAfterOpens = 0)) - assertTrue(earned(documentOpens = 50, asks = 3, askedAfterOpens = 0)) - assertFalse(earned(documentOpens = 99, asks = 4, askedAfterOpens = 0)) - assertTrue(earned(documentOpens = 100, asks = 4, askedAfterOpens = 0)) + // three more after the ask, not three in all + assertFalse( + earned( + documentOpens = 5, + askedAfterOpens = 3, + askedAtMillis = ASKED_AT, + nowMillis = LATER, + ) + ) + assertTrue( + earned( + documentOpens = 6, + askedAfterOpens = 3, + askedAtMillis = ASKED_AT, + nowMillis = LATER, + ) + ) } @Test - fun theFifthAskIsTheLast() { - assertFalse(earned(documentOpens = 1000, asks = 5)) - assertFalse(earned(documentOpens = 1000, asks = 6)) + fun thereIsNoLastAsk() { + assertTrue( + earned( + documentOpens = 1003, + askedAfterOpens = 1000, + askedAtMillis = ASKED_AT, + nowMillis = LATER, + ) + ) } @Test @@ -45,7 +54,6 @@ class ReviewInvitationTest { assertFalse( earned( documentOpens = 15, - asks = 1, askedAfterOpens = 5, askedAtMillis = ASKED_AT, nowMillis = ASKED_AT + TWO_WEEKS - 1, @@ -54,7 +62,6 @@ class ReviewInvitationTest { assertTrue( earned( documentOpens = 15, - asks = 1, askedAfterOpens = 5, askedAtMillis = ASKED_AT, nowMillis = ASKED_AT + TWO_WEEKS, @@ -67,7 +74,6 @@ class ReviewInvitationTest { assertFalse( earned( documentOpens = 15, - asks = 1, askedAfterOpens = 5, askedAtMillis = ASKED_AT, nowMillis = ASKED_AT - TWO_WEEKS, @@ -78,7 +84,7 @@ class ReviewInvitationTest { @Test fun anInstallThatWasNeverAskedHasNoRail() { // zero is "never asked", not 1970 - assertTrue(earned(documentOpens = 5, askedAtMillis = 0, nowMillis = ASKED_AT)) + assertTrue(earned(documentOpens = 3, askedAtMillis = 0, nowMillis = ASKED_AT)) } @Test @@ -89,15 +95,14 @@ class ReviewInvitationTest { private fun earned( documentOpens: Int = 0, - asks: Int = 0, askedAfterOpens: Int = 0, askedAtMillis: Long = 0, nowMillis: Long = 0, - ): Boolean = - ReviewInvitation.isEarned(documentOpens, asks, askedAfterOpens, askedAtMillis, nowMillis) + ): Boolean = ReviewInvitation.isEarned(documentOpens, askedAfterOpens, askedAtMillis, nowMillis) private companion object { const val ASKED_AT = 1_700_000_000_000L const val TWO_WEEKS = 14L * 24 * 60 * 60 * 1000 + const val LATER = ASKED_AT + TWO_WEEKS } }