diff --git a/core/network/src/main/kotlin/com/interlinedlist/android/core/network/dto/UserDto.kt b/core/network/src/main/kotlin/com/interlinedlist/android/core/network/dto/UserDto.kt index e160003..a165b8e 100644 --- a/core/network/src/main/kotlin/com/interlinedlist/android/core/network/dto/UserDto.kt +++ b/core/network/src/main/kotlin/com/interlinedlist/android/core/network/dto/UserDto.kt @@ -25,6 +25,14 @@ data class UserDto( * resolves it. */ val viewingPreference: String? = null, + /** + * How many notifications the bell tray holds before older ones drop off + * (`/help/settings`: "default is 20 and you can set any value from 10 to 40"). + * Nullable because public/partial user payloads omit it; + * [com.interlinedlist.android.core.network.preferences.NotificationTrayLimitStore] + * resolves the absent case. + */ + val notificationTrayLimit: Int? = null, ) /** Maps the wire model into the domain [User]. */ diff --git a/core/network/src/main/kotlin/com/interlinedlist/android/core/network/preferences/NotificationTrayLimitStore.kt b/core/network/src/main/kotlin/com/interlinedlist/android/core/network/preferences/NotificationTrayLimitStore.kt new file mode 100644 index 0000000..d5cdeef --- /dev/null +++ b/core/network/src/main/kotlin/com/interlinedlist/android/core/network/preferences/NotificationTrayLimitStore.kt @@ -0,0 +1,87 @@ +package com.interlinedlist.android.core.network.preferences + +import com.interlinedlist.android.core.common.result.ApiResult +import com.interlinedlist.android.core.network.api.InterlinedListApi +import com.interlinedlist.android.core.network.error.safeApiCall +import kotlinx.serialization.json.Json +import javax.inject.Inject +import javax.inject.Singleton + +/** + * Reads the account's `notificationTrayLimit` — how many notifications the tray + * holds — for the consumers that have to size themselves by it. + * + * It lives in `:core:network` for the same reason [ViewingPreferenceStore] does: + * **no feature module in this repo depends on another feature module**, yet the + * preference is written by `:feature:profile`'s Settings screen and read by + * `:feature:notifications` (the in-app list's page size and the system-tray group + * cap). A narrow accessor on the shared `GET /api/user` is the seam they can both + * reach. + * + * The value is cached for the process so a poll or a pull-to-refresh does not pay + * for an extra `GET /api/user` every time; `:feature:profile` calls [publish] after + * every settings read or write, so a change made in Settings takes effect at once + * instead of waiting for a restart. + * + * Consequence to be aware of: this is the **second** narrow account-preference + * accessor in `:core:network`, sitting alongside `:feature:profile`'s full + * `SettingsRepository`. Issue #104 ("Consolidate the two writers of account + * preferences behind one owner") already tracks untangling that ownership, and this + * store adds a reader to the same pile — so extend it rather than introducing a third + * mechanism. + */ +@Singleton +class NotificationTrayLimitStore @Inject constructor( + private val api: InterlinedListApi, + private val json: Json, +) { + + /** Last known limit, or null until something reads or publishes one. */ + @Volatile + private var cached: Int? = null + + /** + * The limit to size the tray by: the cached value, otherwise a fresh read of + * `GET /api/user`. + * + * A failed read answers [DEFAULT] and is deliberately **not** cached, so the next + * caller retries — notifications must still load when the user endpoint is down, + * but one blip must not pin the limit for the whole process. + */ + suspend fun current(): Int { + cached?.let { return it } + return when ( + val result = safeApiCall(json) { api.getCurrentUser().user.notificationTrayLimit } + ) { + is ApiResult.Success -> publish(result.data) + is ApiResult.Failure -> DEFAULT + } + } + + /** + * Records the limit the server reports, clamped to [RANGE], and returns the value + * that took effect. A null [limit] (the account has no stored value) resolves to + * [DEFAULT]. + * + * Clamping matters because the server's own range is the contract the tray is + * sized by: a stored value from elsewhere must never make the list ask for a page + * the endpoint would refuse. + */ + fun publish(limit: Int?): Int = + (limit?.coerceIn(RANGE) ?: DEFAULT).also { cached = it } + + companion object { + /** + * The limit a fresh account gets. `/help/settings`: "The default is 20 and you + * can set any value from 10 to 40" — matching the live `GET /api/user` value. + */ + const val DEFAULT: Int = 20 + + /** + * The values the server accepts, published in two places: `/help/settings` + * ("any value from 10 to 40") and `/help/api/notifications` ("the user's + * configured tray limit (default 20, clamped to 10-40)"). + */ + val RANGE: IntRange = 10..40 + } +} diff --git a/core/network/src/test/kotlin/com/interlinedlist/android/core/network/preferences/NotificationTrayLimitStoreTest.kt b/core/network/src/test/kotlin/com/interlinedlist/android/core/network/preferences/NotificationTrayLimitStoreTest.kt new file mode 100644 index 0000000..253e19f --- /dev/null +++ b/core/network/src/test/kotlin/com/interlinedlist/android/core/network/preferences/NotificationTrayLimitStoreTest.kt @@ -0,0 +1,129 @@ +package com.interlinedlist.android.core.network.preferences + +import com.google.common.truth.Truth.assertThat +import com.interlinedlist.android.core.network.api.InterlinedListApi +import com.jakewharton.retrofit2.converter.kotlinx.serialization.asConverterFactory +import kotlinx.coroutines.runBlocking +import kotlinx.serialization.json.Json +import okhttp3.MediaType.Companion.toMediaType +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.junit.After +import org.junit.Before +import org.junit.Test +import retrofit2.Retrofit + +/** + * The shared `notificationTrayLimit` accessor: the one number both the in-app + * notifications list and the system-tray group are sized by. + * + * The bounds come from the help centre, which publishes them twice — `/help/settings` + * ("The default is 20 and you can set any value from 10 to 40") and + * `/help/api/notifications` ("up to the user's configured tray limit (default 20, + * clamped to 10-40)") — and the default matches the live `GET /api/user`. + */ +class NotificationTrayLimitStoreTest { + + private lateinit var server: MockWebServer + private lateinit var store: NotificationTrayLimitStore + + // Mirrors the production Json (see NetworkModule). + private val json = Json { + ignoreUnknownKeys = true + explicitNulls = false + coerceInputValues = true + } + + @Before + fun setUp() { + server = MockWebServer() + server.start() + store = newStore() + } + + @After + fun tearDown() = server.shutdown() + + /** A store with an empty cache, pointed at the test server. */ + private fun newStore(): NotificationTrayLimitStore { + val api = Retrofit.Builder() + .baseUrl(server.url("/")) + .addConverterFactory(json.asConverterFactory("application/json".toMediaType())) + .build() + .create(InterlinedListApi::class.java) + return NotificationTrayLimitStore(api, json) + } + + private fun enqueueUser(trayLimit: Int?) { + val field = trayLimit?.let { """, "notificationTrayLimit": $it""" } ?: "" + server.enqueue( + MockResponse().setBody("""{ "user": { "id": "u1", "username": "me"$field } }"""), + ) + } + + @Test + fun `current reads the limit from GET api user`() = runBlocking { + enqueueUser(40) + + assertThat(store.current()).isEqualTo(40) + assertThat(server.takeRequest().path).isEqualTo("/api/user") + } + + @Test + fun `an absent limit falls back to the documented default`() = runBlocking { + enqueueUser(null) + + assertThat(store.current()).isEqualTo(20) + } + + @Test + fun `the limit is read once and then served from the cache`() = runBlocking { + enqueueUser(30) + + assertThat(store.current()).isEqualTo(30) + assertThat(store.current()).isEqualTo(30) + + // A second GET would have no queued response; only one request was made. + assertThat(server.requestCount).isEqualTo(1) + } + + @Test + fun `a failed read falls back to the default without caching it`() = runBlocking { + server.enqueue(MockResponse().setResponseCode(500).setBody("""{ "error": "boom" }""")) + enqueueUser(35) + + // The tray still has a size to work with... + assertThat(store.current()).isEqualTo(20) + // ...and the blip is not pinned for the rest of the process. + assertThat(store.current()).isEqualTo(35) + } + + @Test + fun `a stored value outside the server's range is clamped`() = runBlocking { + enqueueUser(400) + assertThat(store.current()).isEqualTo(40) + + // A fresh store, so the clamp is exercised on the way in rather than from cache. + enqueueUser(1) + assertThat(newStore().current()).isEqualTo(10) + } + + @Test + fun `a published limit takes effect without a request`() = runBlocking { + assertThat(store.publish(25)).isEqualTo(25) + + assertThat(store.current()).isEqualTo(25) + assertThat(server.requestCount).isEqualTo(0) + } + + @Test + fun `publishing an absent limit resolves to the default`() { + assertThat(store.publish(null)).isEqualTo(20) + } + + @Test + fun `the documented bounds match the help centre`() { + assertThat(NotificationTrayLimitStore.DEFAULT).isEqualTo(20) + assertThat(NotificationTrayLimitStore.RANGE).isEqualTo(10..40) + } +} diff --git a/feature/notifications/src/main/kotlin/com/interlinedlist/android/feature/notifications/data/DefaultNotificationsRepository.kt b/feature/notifications/src/main/kotlin/com/interlinedlist/android/feature/notifications/data/DefaultNotificationsRepository.kt index c8e8d7d..0961933 100644 --- a/feature/notifications/src/main/kotlin/com/interlinedlist/android/feature/notifications/data/DefaultNotificationsRepository.kt +++ b/feature/notifications/src/main/kotlin/com/interlinedlist/android/feature/notifications/data/DefaultNotificationsRepository.kt @@ -3,12 +3,12 @@ package com.interlinedlist.android.feature.notifications.data import com.interlinedlist.android.core.common.dispatcher.DispatcherProvider import com.interlinedlist.android.core.common.result.ApiResult import com.interlinedlist.android.core.network.error.safeApiCall +import com.interlinedlist.android.core.network.preferences.NotificationTrayLimitStore import com.interlinedlist.android.feature.notifications.data.local.NotificationDao import com.interlinedlist.android.feature.notifications.data.local.NotificationEntity import com.interlinedlist.android.feature.notifications.data.local.toDomain import com.interlinedlist.android.feature.notifications.data.local.toEntity import com.interlinedlist.android.feature.notifications.data.remote.NotificationsApi -import com.interlinedlist.android.feature.notifications.data.remote.dto.PaginationDto import com.interlinedlist.android.feature.notifications.data.remote.dto.toDomain import com.interlinedlist.android.feature.notifications.domain.Notification import kotlinx.coroutines.flow.Flow @@ -18,9 +18,19 @@ import kotlinx.coroutines.withContext import kotlinx.serialization.json.Json import javax.inject.Inject +/** + * Notifications backed by Room, refreshed from the API. + * + * Every page is sized by the account's `notificationTrayLimit` rather than by a + * constant, so the app's list holds what the web's bell tray holds. The limit comes + * from [NotificationTrayLimitStore] in `:core:network` because the preference is + * written by `:feature:profile`'s Settings screen and no feature module here may + * depend on another. + */ class DefaultNotificationsRepository @Inject constructor( private val api: NotificationsApi, private val notificationDao: NotificationDao, + private val trayLimitStore: NotificationTrayLimitStore, private val json: Json, private val dispatchers: DispatcherProvider, ) : NotificationsRepository { @@ -31,14 +41,14 @@ class DefaultNotificationsRepository @Inject constructor( override fun observeUnreadCount(): Flow = notificationDao.observeUnreadCount() override suspend fun fetchLatest(): ApiResult> = withContext(dispatchers.io) { - when (val result = safeCall { api.getNotifications(limit = PaginationDto.DEFAULT_LIMIT, offset = 0) }) { + when (val result = safeCall { api.getNotifications(limit = pageSize(), offset = 0) }) { is ApiResult.Success -> ApiResult.Success(result.data.items.map { it.toDomain() }) is ApiResult.Failure -> result } } override suspend fun refresh(): ApiResult = withContext(dispatchers.io) { - when (val result = safeCall { api.getNotifications(limit = PaginationDto.DEFAULT_LIMIT, offset = 0) }) { + when (val result = safeCall { api.getNotifications(limit = pageSize(), offset = 0) }) { is ApiResult.Success -> { val page = result.data val entities = page.items.mapIndexed { index, dto -> @@ -54,7 +64,7 @@ class DefaultNotificationsRepository @Inject constructor( override suspend fun loadMore(currentCount: Int): ApiResult = withContext(dispatchers.io) { when (val result = safeCall { - api.getNotifications(limit = PaginationDto.DEFAULT_LIMIT, offset = currentCount) + api.getNotifications(limit = pageSize(), offset = currentCount) }) { is ApiResult.Success -> { val page = result.data @@ -106,6 +116,13 @@ class DefaultNotificationsRepository @Inject constructor( // --- helpers ----------------------------------------------------------- + /** + * How many notifications one page holds: the account's tray limit. The endpoint's + * own `limit` accepts 1-50 (`/help/api/notifications`), which comfortably contains + * the 10-40 the tray limit is clamped to, so the preference can be sent verbatim. + */ + private suspend fun pageSize(): Int = trayLimitStore.current() + private suspend fun safeCall(block: suspend () -> T): ApiResult = safeApiCall(json, block) } diff --git a/feature/notifications/src/main/kotlin/com/interlinedlist/android/feature/notifications/push/NotificationPollRunner.kt b/feature/notifications/src/main/kotlin/com/interlinedlist/android/feature/notifications/push/NotificationPollRunner.kt index 654b2c1..9ca2e53 100644 --- a/feature/notifications/src/main/kotlin/com/interlinedlist/android/feature/notifications/push/NotificationPollRunner.kt +++ b/feature/notifications/src/main/kotlin/com/interlinedlist/android/feature/notifications/push/NotificationPollRunner.kt @@ -1,6 +1,7 @@ package com.interlinedlist.android.feature.notifications.push import com.interlinedlist.android.core.common.result.ApiResult +import com.interlinedlist.android.core.network.preferences.NotificationTrayLimitStore import com.interlinedlist.android.feature.notifications.data.NotificationPreferencesRepository import com.interlinedlist.android.feature.notifications.data.NotificationsRepository import javax.inject.Inject @@ -10,6 +11,12 @@ import javax.inject.Inject * preferences, decide what is NEW and push-enabled (via the pure [NotificationPollProcessor]), * raise the tray notifications, and advance the persisted last-seen marker. * + * The account's `notificationTrayLimit` is what the system tray is sized by: the + * repository already fetches a page of that size, and this runner passes the same + * number to the raiser so the shade never shows more separate notifications than the + * user asked the tray to hold. The value is cached by [NotificationTrayLimitStore], + * so reading it here costs no second request. + * * Holding this outside the `@HiltWorker` keeps it free of the Android `CoroutineWorker` * superclass, so it is fully unit-testable; [NotificationsPollWorker] is a thin adapter * that maps the [Result] onto WorkManager's outcome. @@ -18,6 +25,7 @@ class NotificationPollRunner @Inject constructor( private val notificationsRepository: NotificationsRepository, private val preferencesRepository: NotificationPreferencesRepository, private val lastSeenStore: LastSeenNotificationStore, + private val trayLimitStore: NotificationTrayLimitStore, private val raiser: SystemNotificationRaiser, ) { @@ -51,7 +59,7 @@ class NotificationPollRunner @Inject constructor( ) if (outcome.toPost.isNotEmpty()) { - raiser.post(outcome.toPost) + raiser.post(outcome.toPost, maxIndividual = trayLimitStore.current()) } // Advance the marker after posting, so a crash before posting doesn't skip items. outcome.newLastSeenId?.let(lastSeenStore::setLastSeenId) diff --git a/feature/notifications/src/main/kotlin/com/interlinedlist/android/feature/notifications/push/SystemNotificationPoster.kt b/feature/notifications/src/main/kotlin/com/interlinedlist/android/feature/notifications/push/SystemNotificationPoster.kt index b52c5ae..4ec044f 100644 --- a/feature/notifications/src/main/kotlin/com/interlinedlist/android/feature/notifications/push/SystemNotificationPoster.kt +++ b/feature/notifications/src/main/kotlin/com/interlinedlist/android/feature/notifications/push/SystemNotificationPoster.kt @@ -14,15 +14,24 @@ import com.interlinedlist.android.feature.notifications.domain.Notification * side-effects live only in [SystemNotificationPoster]. */ interface SystemNotificationRaiser { - /** Raises tray notifications for [items] (newest-first, already push-filtered). */ - fun post(items: List) + /** + * Raises tray notifications for [items] (newest-first, already push-filtered), + * showing at most [maxIndividual] of them separately. + */ + fun post(items: List, maxIndividual: Int) } /** * Posts system-tray notifications for a batch of NEW notifications. The batch is - * capped so a large backlog can't spam the tray: at most [MAX_INDIVIDUAL] individual + * capped so a large backlog can't spam the tray: at most `maxIndividual` individual * items are shown, and when there are more, only a single summary is posted. * + * The cap is the account's `notificationTrayLimit` — the same preference that sizes + * the web's bell tray and this app's notifications list — rather than a constant of + * this class's own choosing, so "how many notifications the tray holds" means one + * thing everywhere. [NotificationPollRunner] resolves it and passes it in, keeping + * this class free of the preference lookup. + * * Each notification's tap opens the app's launcher activity (resolved via the package * manager, so this module needs no compile-time reference to `MainActivity`) carrying * the [NotificationDeepLink] extras the app reads to route. @@ -37,13 +46,16 @@ class SystemNotificationPoster( /** * Posts [items] (newest-first, already push-filtered). No-op when the list is empty * or the user has notifications disabled at the OS level. + * + * [maxIndividual] is coerced to at least 1 so a nonsensical cap still surfaces the + * activity as a summary rather than swallowing it. */ - override fun post(items: List) { + override fun post(items: List, maxIndividual: Int) { if (items.isEmpty()) return val manager = NotificationManagerCompat.from(context) if (!manager.areNotificationsEnabled()) return - if (items.size > MAX_INDIVIDUAL) { + if (collapsesToSummary(items.size, maxIndividual)) { postSummaryOnly(manager, items) return } @@ -139,8 +151,18 @@ class SystemNotificationPoster( } companion object { - /** Max individual notifications before collapsing to a single summary. */ - const val MAX_INDIVIDUAL = 5 + /** + * Whether a batch of [count] items collapses to a single summary instead of + * being posted one by one: true once it exceeds [maxIndividual], the account's + * `notificationTrayLimit`. + * + * Pure, and separate from [post], because this is the rule the tray group is + * sized by and it has to be assertable without an Android notification manager. + * A nonsensical cap is coerced to at least 1, so activity still surfaces as a + * summary rather than being swallowed. + */ + fun collapsesToSummary(count: Int, maxIndividual: Int): Boolean = + count > maxIndividual.coerceAtLeast(1) /** Shared group key so the shade collapses our notifications together. */ const val GROUP_KEY = "il.notifications.group" diff --git a/feature/notifications/src/test/kotlin/com/interlinedlist/android/feature/notifications/data/DefaultNotificationsRepositoryTest.kt b/feature/notifications/src/test/kotlin/com/interlinedlist/android/feature/notifications/data/DefaultNotificationsRepositoryTest.kt index 6d4a5ec..738413e 100644 --- a/feature/notifications/src/test/kotlin/com/interlinedlist/android/feature/notifications/data/DefaultNotificationsRepositoryTest.kt +++ b/feature/notifications/src/test/kotlin/com/interlinedlist/android/feature/notifications/data/DefaultNotificationsRepositoryTest.kt @@ -3,6 +3,8 @@ package com.interlinedlist.android.feature.notifications.data import com.google.common.truth.Truth.assertThat import com.interlinedlist.android.core.common.result.ApiResult import com.interlinedlist.android.core.common.result.AppError +import com.interlinedlist.android.core.network.api.InterlinedListApi +import com.interlinedlist.android.core.network.preferences.NotificationTrayLimitStore import com.interlinedlist.android.feature.notifications.data.remote.NotificationsApi import com.jakewharton.retrofit2.converter.kotlinx.serialization.asConverterFactory import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -27,18 +29,23 @@ class DefaultNotificationsRepositoryTest { private lateinit var server: MockWebServer private lateinit var api: NotificationsApi private lateinit var dao: FakeNotificationDao + private lateinit var trayLimitStore: NotificationTrayLimitStore @Before fun setUp() { server = MockWebServer() server.start() val contentType = "application/json".toMediaType() - api = Retrofit.Builder() + val retrofit = Retrofit.Builder() .baseUrl(server.url("/")) .addConverterFactory(json.asConverterFactory(contentType)) .build() - .create(NotificationsApi::class.java) + api = retrofit.create(NotificationsApi::class.java) dao = FakeNotificationDao() + trayLimitStore = + NotificationTrayLimitStore(retrofit.create(InterlinedListApi::class.java), json) + // Pre-seeded so each test's queued responses are spent on notifications alone. + trayLimitStore.publish(NotificationTrayLimitStore.DEFAULT) } @After @@ -47,6 +54,7 @@ class DefaultNotificationsRepositoryTest { private fun repository() = DefaultNotificationsRepository( api = api, notificationDao = dao, + trayLimitStore = trayLimitStore, json = json, dispatchers = TestDispatcherProvider(dispatcher), ) diff --git a/feature/notifications/src/test/kotlin/com/interlinedlist/android/feature/notifications/data/NotificationListTrayLimitTest.kt b/feature/notifications/src/test/kotlin/com/interlinedlist/android/feature/notifications/data/NotificationListTrayLimitTest.kt new file mode 100644 index 0000000..1616ddd --- /dev/null +++ b/feature/notifications/src/test/kotlin/com/interlinedlist/android/feature/notifications/data/NotificationListTrayLimitTest.kt @@ -0,0 +1,140 @@ +package com.interlinedlist.android.feature.notifications.data + +import com.google.common.truth.Truth.assertThat +import com.interlinedlist.android.core.network.api.InterlinedListApi +import com.interlinedlist.android.core.network.preferences.NotificationTrayLimitStore +import com.interlinedlist.android.feature.notifications.data.remote.NotificationsApi +import com.jakewharton.retrofit2.converter.kotlinx.serialization.asConverterFactory +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.runTest +import kotlinx.serialization.json.Json +import okhttp3.MediaType.Companion.toMediaType +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.junit.After +import org.junit.Before +import org.junit.Test +import retrofit2.Retrofit + +/** + * Issue #35: the in-app notifications list is sized by the account's + * `notificationTrayLimit` instead of a hard-coded page size. + * + * `/help/settings` describes the preference as "How many notifications the bell tray + * holds before older ones drop off", so the app's list has to hold the same number the + * web's tray does. `/help/api/notifications` documents the endpoint's own `limit` as + * 1-50, which comfortably contains the 10-40 the preference is clamped to, so the + * value can be sent verbatim. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class NotificationListTrayLimitTest { + + private val dispatcher = StandardTestDispatcher() + private val json = Json { ignoreUnknownKeys = true; explicitNulls = false } + + private lateinit var server: MockWebServer + private lateinit var api: NotificationsApi + private lateinit var userApi: InterlinedListApi + private lateinit var dao: FakeNotificationDao + + @Before + fun setUp() { + server = MockWebServer() + server.start() + val retrofit = Retrofit.Builder() + .baseUrl(server.url("/")) + .addConverterFactory(json.asConverterFactory("application/json".toMediaType())) + .build() + api = retrofit.create(NotificationsApi::class.java) + userApi = retrofit.create(InterlinedListApi::class.java) + dao = FakeNotificationDao() + } + + @After + fun tearDown() = server.shutdown() + + /** A store already holding [limit], or one that still has to read it from the API. */ + private fun store(limit: Int?) = + NotificationTrayLimitStore(userApi, json).apply { limit?.let { publish(it) } } + + private fun repository(store: NotificationTrayLimitStore) = DefaultNotificationsRepository( + api = api, + notificationDao = dao, + trayLimitStore = store, + json = json, + dispatchers = TestDispatcherProvider(dispatcher), + ) + + private fun enqueuePage(hasMore: Boolean = false, ids: List = listOf("1")) { + val rows = ids.joinToString(",") { """{ "id": "$it", "type": "follow", "subject": "s$it" }""" } + server.enqueue( + MockResponse().setBody("""{ "data": [ $rows ], "pagination": { "hasMore": $hasMore } }"""), + ) + } + + @Test + fun `refresh asks for a page the size of the account's tray limit`() = runTest(dispatcher) { + enqueuePage() + + repository(store(40)).refresh() + + val url = server.takeRequest().requestUrl!! + assertThat(url.encodedPath).isEqualTo("/api/notifications") + assertThat(url.queryParameter("limit")).isEqualTo("40") + assertThat(url.queryParameter("offset")).isEqualTo("0") + } + + @Test + fun `loadMore pages by the same limit from the current offset`() = runTest(dispatcher) { + enqueuePage(hasMore = true) + enqueuePage(ids = listOf("2")) + val repo = repository(store(10)) + + repo.refresh() + repo.loadMore(currentCount = 10) + + server.takeRequest() + val url = server.takeRequest().requestUrl!! + assertThat(url.queryParameter("limit")).isEqualTo("10") + assertThat(url.queryParameter("offset")).isEqualTo("10") + } + + @Test + fun `the poll's fetch is sized by the limit too, so it never sees less than the tray holds`() = + runTest(dispatcher) { + enqueuePage() + + repository(store(35)).fetchLatest() + + assertThat(server.takeRequest().requestUrl!!.queryParameter("limit")).isEqualTo("35") + } + + @Test + fun `an account with no stored limit falls back to the documented default of 20`() = + runTest(dispatcher) { + // Nothing cached, so `GET /api/user` answers first, then the list. + server.enqueue(MockResponse().setBody("""{ "user": { "id": "u1" } }""")) + enqueuePage() + + repository(store(null)).refresh() + + assertThat(server.takeRequest().path).isEqualTo("/api/user") + assertThat(server.takeRequest().requestUrl!!.queryParameter("limit")).isEqualTo("20") + } + + @Test + fun `a change saved in Settings is honoured by the very next refresh`() = runTest(dispatcher) { + val store = store(20) + enqueuePage() + repository(store).refresh() + assertThat(server.takeRequest().requestUrl!!.queryParameter("limit")).isEqualTo("20") + + // Settings PATCHed a new value and published it to the shared store. + store.publish(40) + enqueuePage() + repository(store).refresh() + + assertThat(server.takeRequest().requestUrl!!.queryParameter("limit")).isEqualTo("40") + } +} diff --git a/feature/notifications/src/test/kotlin/com/interlinedlist/android/feature/notifications/push/NotificationPollRunnerTest.kt b/feature/notifications/src/test/kotlin/com/interlinedlist/android/feature/notifications/push/NotificationPollRunnerTest.kt index f43c65c..1ca64a7 100644 --- a/feature/notifications/src/test/kotlin/com/interlinedlist/android/feature/notifications/push/NotificationPollRunnerTest.kt +++ b/feature/notifications/src/test/kotlin/com/interlinedlist/android/feature/notifications/push/NotificationPollRunnerTest.kt @@ -2,6 +2,8 @@ package com.interlinedlist.android.feature.notifications.push import com.google.common.truth.Truth.assertThat import com.interlinedlist.android.core.common.result.ApiResult +import com.interlinedlist.android.core.network.api.InterlinedListApi +import com.interlinedlist.android.core.network.preferences.NotificationTrayLimitStore import com.interlinedlist.android.feature.notifications.data.DefaultNotificationsRepository import com.interlinedlist.android.feature.notifications.data.FakeNotificationDao import com.interlinedlist.android.feature.notifications.data.NotificationPreferencesRepository @@ -31,18 +33,24 @@ class NotificationPollRunnerTest { private lateinit var server: MockWebServer private lateinit var api: NotificationsApi private lateinit var dao: FakeNotificationDao + private lateinit var trayLimitStore: NotificationTrayLimitStore @Before fun setUp() { server = MockWebServer() server.start() val contentType = "application/json".toMediaType() - api = Retrofit.Builder() + val retrofit = Retrofit.Builder() .baseUrl(server.url("/")) .addConverterFactory(json.asConverterFactory(contentType)) .build() - .create(NotificationsApi::class.java) + api = retrofit.create(NotificationsApi::class.java) dao = FakeNotificationDao() + trayLimitStore = + NotificationTrayLimitStore(retrofit.create(InterlinedListApi::class.java), json) + // Pre-seeded so the poll spends its queued responses on notifications, not on + // `GET /api/user`; the fetch itself is covered by NotificationTrayLimitStoreTest. + trayLimitStore.publish(NotificationTrayLimitStore.DEFAULT) } @After @@ -51,6 +59,7 @@ class NotificationPollRunnerTest { private fun notificationsRepo() = DefaultNotificationsRepository( api = api, notificationDao = dao, + trayLimitStore = trayLimitStore, json = json, dispatchers = TestDispatcherProvider(dispatcher), ) @@ -72,6 +81,7 @@ class NotificationPollRunnerTest { notificationsRepository = notificationsRepo(), preferencesRepository = prefs, lastSeenStore = store, + trayLimitStore = trayLimitStore, raiser = raiser, ) diff --git a/feature/notifications/src/test/kotlin/com/interlinedlist/android/feature/notifications/push/RecordingSystemNotificationRaiser.kt b/feature/notifications/src/test/kotlin/com/interlinedlist/android/feature/notifications/push/RecordingSystemNotificationRaiser.kt index af9166e..18cc79e 100644 --- a/feature/notifications/src/test/kotlin/com/interlinedlist/android/feature/notifications/push/RecordingSystemNotificationRaiser.kt +++ b/feature/notifications/src/test/kotlin/com/interlinedlist/android/feature/notifications/push/RecordingSystemNotificationRaiser.kt @@ -5,13 +5,21 @@ import com.interlinedlist.android.feature.notifications.domain.Notification /** Records every [post] call so tests can assert which notifications were raised. */ class RecordingSystemNotificationRaiser : SystemNotificationRaiser { + /** Each element is one [post] call: the batch and the cap it was given. */ + data class Posted(val items: List, val maxIndividual: Int) + + val posts = mutableListOf() + /** Each element is the batch handed to one [post] call. */ - val batches = mutableListOf>() + val batches: List> get() = posts.map { it.items } /** Flattened ids across all batches, in order. */ val postedIds: List get() = batches.flatten().map { it.id } - override fun post(items: List) { - batches += items + /** The cap handed to the single [post] call a test made. */ + val maxIndividual: Int get() = posts.single().maxIndividual + + override fun post(items: List, maxIndividual: Int) { + posts += Posted(items, maxIndividual) } } diff --git a/feature/notifications/src/test/kotlin/com/interlinedlist/android/feature/notifications/push/SystemTrayGroupLimitTest.kt b/feature/notifications/src/test/kotlin/com/interlinedlist/android/feature/notifications/push/SystemTrayGroupLimitTest.kt new file mode 100644 index 0000000..232063b --- /dev/null +++ b/feature/notifications/src/test/kotlin/com/interlinedlist/android/feature/notifications/push/SystemTrayGroupLimitTest.kt @@ -0,0 +1,168 @@ +package com.interlinedlist.android.feature.notifications.push + +import com.google.common.truth.Truth.assertThat +import com.interlinedlist.android.core.common.result.ApiResult +import com.interlinedlist.android.core.network.api.InterlinedListApi +import com.interlinedlist.android.core.network.preferences.NotificationTrayLimitStore +import com.interlinedlist.android.feature.notifications.data.DefaultNotificationsRepository +import com.interlinedlist.android.feature.notifications.data.FakeNotificationDao +import com.interlinedlist.android.feature.notifications.data.NotificationPreferencesRepository +import com.interlinedlist.android.feature.notifications.data.TestDispatcherProvider +import com.interlinedlist.android.feature.notifications.data.remote.NotificationsApi +import com.interlinedlist.android.feature.notifications.domain.NotificationPreference +import com.jakewharton.retrofit2.converter.kotlinx.serialization.asConverterFactory +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.runTest +import kotlinx.serialization.json.Json +import okhttp3.MediaType.Companion.toMediaType +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.junit.After +import org.junit.Before +import org.junit.Test +import retrofit2.Retrofit + +/** + * Issue #35: the system-tray group is capped by the account's `notificationTrayLimit` + * rather than by a constant this module chose for itself. + * + * Before, [SystemNotificationPoster] collapsed anything over five items into a single + * summary regardless of what the user had configured. Now the poll resolves the same + * preference the in-app list uses and hands it to the raiser, so "how many + * notifications the tray holds" (`/help/settings`) means one thing everywhere. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class SystemTrayGroupLimitTest { + + private val dispatcher = StandardTestDispatcher() + private val json = Json { ignoreUnknownKeys = true; explicitNulls = false } + + private lateinit var server: MockWebServer + private lateinit var api: NotificationsApi + private lateinit var userApi: InterlinedListApi + private lateinit var dao: FakeNotificationDao + + @Before + fun setUp() { + server = MockWebServer() + server.start() + val retrofit = Retrofit.Builder() + .baseUrl(server.url("/")) + .addConverterFactory(json.asConverterFactory("application/json".toMediaType())) + .build() + api = retrofit.create(NotificationsApi::class.java) + userApi = retrofit.create(InterlinedListApi::class.java) + dao = FakeNotificationDao() + } + + @After + fun tearDown() = server.shutdown() + + // --- the cap the poll hands to the tray ---------------------------------- + + @Test + fun `the poll caps the tray group at the account's limit`() = runTest(dispatcher) { + enqueuePage("3", "2") + val raiser = RecordingSystemNotificationRaiser() + + runner(store(12), raiser).run() + + assertThat(raiser.postedIds).containsExactly("3", "2").inOrder() + assertThat(raiser.maxIndividual).isEqualTo(12) + } + + @Test + fun `a different account limit produces a different cap`() = runTest(dispatcher) { + enqueuePage("3", "2") + val raiser = RecordingSystemNotificationRaiser() + + runner(store(40), raiser).run() + + assertThat(raiser.maxIndividual).isEqualTo(40) + } + + @Test + fun `resolving the cap costs no extra request once the page has been fetched`() = + runTest(dispatcher) { + // Only `GET /api/user` and the notifications page are queued: if the runner + // read the preference twice over the network, the second read would hang. + server.enqueue(MockResponse().setBody("""{ "user": { "id": "u1", "notificationTrayLimit": 30 } }""")) + enqueuePage("3", "2") + val raiser = RecordingSystemNotificationRaiser() + + runner(store(null), raiser).run() + + assertThat(raiser.maxIndividual).isEqualTo(30) + assertThat(server.requestCount).isEqualTo(2) + } + + // --- the grouping rule itself -------------------------------------------- + + @Test + fun `a batch within the limit is posted item by item`() { + assertThat(SystemNotificationPoster.collapsesToSummary(count = 10, maxIndividual = 10)) + .isFalse() + assertThat(SystemNotificationPoster.collapsesToSummary(count = 9, maxIndividual = 10)) + .isFalse() + } + + @Test + fun `a batch over the limit collapses to a single summary`() { + assertThat(SystemNotificationPoster.collapsesToSummary(count = 11, maxIndividual = 10)) + .isTrue() + } + + @Test + fun `the old fixed cap of five no longer decides anything`() { + // Six items used to collapse; with the account's limit they no longer do. + assertThat(SystemNotificationPoster.collapsesToSummary(count = 6, maxIndividual = 20)) + .isFalse() + // ...and a deliberately small tray collapses well before five. + assertThat(SystemNotificationPoster.collapsesToSummary(count = 3, maxIndividual = 2)) + .isTrue() + } + + @Test + fun `a nonsensical cap still surfaces the activity as a summary`() { + assertThat(SystemNotificationPoster.collapsesToSummary(count = 1, maxIndividual = 0)) + .isFalse() + assertThat(SystemNotificationPoster.collapsesToSummary(count = 2, maxIndividual = -5)) + .isTrue() + } + + // --- helpers ------------------------------------------------------------- + + private fun store(limit: Int?) = + NotificationTrayLimitStore(userApi, json).apply { limit?.let { publish(it) } } + + private fun enqueuePage(vararg ids: String) { + val rows = ids.joinToString(",") { """{ "id": "$it", "type": "follow", "subject": "s$it" }""" } + server.enqueue( + MockResponse().setBody("""{ "data": [ $rows ], "pagination": { "hasMore": false } }"""), + ) + } + + private fun runner( + trayLimitStore: NotificationTrayLimitStore, + raiser: RecordingSystemNotificationRaiser, + ) = NotificationPollRunner( + notificationsRepository = DefaultNotificationsRepository( + api = api, + notificationDao = dao, + trayLimitStore = trayLimitStore, + json = json, + dispatchers = TestDispatcherProvider(dispatcher), + ), + preferencesRepository = object : NotificationPreferencesRepository { + override suspend fun getPreferences(): ApiResult> = + ApiResult.Success(emptyList()) + + override suspend fun updatePreference(preference: NotificationPreference) = + ApiResult.Success(Unit) + }, + lastSeenStore = FakeLastSeenNotificationStore(initial = "1"), + trayLimitStore = trayLimitStore, + raiser = raiser, + ) +} diff --git a/feature/profile/src/androidTest/kotlin/com/interlinedlist/android/feature/profile/ui/SettingsScreenTest.kt b/feature/profile/src/androidTest/kotlin/com/interlinedlist/android/feature/profile/ui/SettingsScreenTest.kt index 90bab43..12361e1 100644 --- a/feature/profile/src/androidTest/kotlin/com/interlinedlist/android/feature/profile/ui/SettingsScreenTest.kt +++ b/feature/profile/src/androidTest/kotlin/com/interlinedlist/android/feature/profile/ui/SettingsScreenTest.kt @@ -41,6 +41,7 @@ class SettingsScreenTest { onSelectViewingPreference: (ViewingPreference) -> Unit = {}, onToggleShowPreviews: (Boolean) -> Unit = {}, onSetMessagesPerPage: (Int) -> Unit = {}, + onSetNotificationTrayLimit: (Int) -> Unit = {}, onSetMaxMessageLength: (Int) -> Unit = {}, onToggleDefaultPubliclyVisible: (Boolean) -> Unit = {}, onToggleShowAdvancedPostSettings: (Boolean) -> Unit = {}, @@ -57,6 +58,7 @@ class SettingsScreenTest { onSelectViewingPreference = onSelectViewingPreference, onToggleShowPreviews = onToggleShowPreviews, onSetMessagesPerPage = onSetMessagesPerPage, + onSetNotificationTrayLimit = onSetNotificationTrayLimit, onSetMaxMessageLength = onSetMaxMessageLength, onToggleDefaultPubliclyVisible = onToggleDefaultPubliclyVisible, onToggleShowAdvancedPostSettings = onToggleShowAdvancedPostSettings, @@ -282,6 +284,78 @@ class SettingsScreenTest { assert(saved == 25) } + // --- Notification tray limit (issue #35) --------------------------------- + // The web files this under View preferences (/help/settings lists it there, and + // its Notifications section points back with "see View Preferences above"), so it + // sits in the same group here. + + @Test + fun notificationTrayLimit_isOfferedUnderViewPreferences() { + setContent( + state = SettingsUiState(settings = UserSettings(notificationTrayLimit = 30)), + ) + + composeRule.onNodeWithTag(SettingsTestTags.VIEW_PREFERENCES).assertIsDisplayed() + composeRule.onNodeWithTag(SettingsTestTags.NOTIFICATION_TRAY_LIMIT) + .assertTextEquals("30") + } + + @Test + fun notificationTrayLimit_showsTheServerDefaultWhenTheAccountHasNone() { + setContent( + state = SettingsUiState(settings = UserSettings(notificationTrayLimit = null)), + ) + + composeRule.onNodeWithTag(SettingsTestTags.NOTIFICATION_TRAY_LIMIT) + .assertTextEquals("20") + } + + @Test + fun notificationTrayLimit_cannotStepAboveTheDocumentedMaximum() { + var saved: Int? = null + setContent( + state = SettingsUiState(settings = UserSettings(notificationTrayLimit = 40)), + onSetNotificationTrayLimit = { saved = it }, + ) + + composeRule.onNodeWithTag(settingsIncrementTag(SettingsTestTags.NOTIFICATION_TRAY_LIMIT)) + .assertIsNotEnabled() + + assert(saved == null) + } + + @Test + fun notificationTrayLimit_outOfRangeEntryIsRejectedWithoutReportingAValue() { + var saved: Int? = null + setContent( + state = SettingsUiState(settings = UserSettings(notificationTrayLimit = 20)), + onSetNotificationTrayLimit = { saved = it }, + ) + + composeRule.onNodeWithTag(SettingsTestTags.NOTIFICATION_TRAY_LIMIT).performTextClearance() + composeRule.onNodeWithTag(SettingsTestTags.NOTIFICATION_TRAY_LIMIT).performTextInput("41") + composeRule.onNodeWithTag(SettingsTestTags.NOTIFICATION_TRAY_LIMIT).performImeAction() + + composeRule.onNodeWithTag(settingsNumberErrorTag(SettingsTestTags.NOTIFICATION_TRAY_LIMIT)) + .assertIsDisplayed() + assert(saved == null) { "an out-of-range entry must not be saved, got $saved" } + } + + @Test + fun notificationTrayLimit_inRangeEntryIsReported() { + var saved: Int? = null + setContent( + state = SettingsUiState(settings = UserSettings(notificationTrayLimit = 20)), + onSetNotificationTrayLimit = { saved = it }, + ) + + composeRule.onNodeWithTag(SettingsTestTags.NOTIFICATION_TRAY_LIMIT).performTextClearance() + composeRule.onNodeWithTag(SettingsTestTags.NOTIFICATION_TRAY_LIMIT).performTextInput("35") + composeRule.onNodeWithTag(SettingsTestTags.NOTIFICATION_TRAY_LIMIT).performImeAction() + + assert(saved == 35) + } + @Test fun characterLimit_rolledBackSaveRestoresTheFieldToTheStoredValue() { // The screen is recomposed with the previous value after a rejected save; @@ -296,6 +370,7 @@ class SettingsScreenTest { onSelectViewingPreference = {}, onToggleShowPreviews = {}, onSetMessagesPerPage = {}, + onSetNotificationTrayLimit = {}, // Optimistic apply, then the server refuses and it rolls back. onSetMaxMessageLength = { settings = settings.copy(maxMessageLength = it) }, onToggleDefaultPubliclyVisible = {}, @@ -381,6 +456,7 @@ class SettingsScreenTest { onSelectViewingPreference = {}, onToggleShowPreviews = {}, onSetMessagesPerPage = {}, + onSetNotificationTrayLimit = {}, onSetMaxMessageLength = {}, onToggleDefaultPubliclyVisible = {}, onToggleShowAdvancedPostSettings = {}, diff --git a/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/data/DefaultSettingsRepository.kt b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/data/DefaultSettingsRepository.kt index 699f715..c039cde 100644 --- a/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/data/DefaultSettingsRepository.kt +++ b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/data/DefaultSettingsRepository.kt @@ -4,6 +4,7 @@ import com.interlinedlist.android.core.common.dispatcher.DispatcherProvider import com.interlinedlist.android.core.common.result.ApiResult import com.interlinedlist.android.core.common.result.AppError import com.interlinedlist.android.core.network.error.safeApiCall +import com.interlinedlist.android.core.network.preferences.NotificationTrayLimitStore import com.interlinedlist.android.feature.profile.data.mapper.toRequest import com.interlinedlist.android.feature.profile.data.mapper.toUserSettings import com.interlinedlist.android.feature.profile.data.remote.ProfileApi @@ -21,10 +22,17 @@ import javax.inject.Singleton * Network-backed settings with a process-scoped in-memory cache: every successful * read or write publishes the new value to [observeSettings], so the Settings screen * and the feed see the same preferences without either re-fetching. + * + * `notificationTrayLimit` is additionally forwarded to [NotificationTrayLimitStore] in + * `:core:network`, because `:feature:notifications` sizes its list and its system-tray + * group by that preference and no feature module here may depend on another. Without + * the forward, a limit changed in Settings would not take effect until the process + * restarted. */ @Singleton class DefaultSettingsRepository @Inject constructor( private val api: ProfileApi, + private val trayLimitStore: NotificationTrayLimitStore, private val json: Json, private val dispatchers: DispatcherProvider, ) : SettingsRepository { @@ -59,6 +67,7 @@ class DefaultSettingsRepository @Inject constructor( private fun publish(settings: UserSettings): UserSettings { cached.value = settings + trayLimitStore.publish(settings.notificationTrayLimit) return settings } } diff --git a/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/domain/SettingsBounds.kt b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/domain/SettingsBounds.kt index baeb714..937601f 100644 --- a/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/domain/SettingsBounds.kt +++ b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/domain/SettingsBounds.kt @@ -1,5 +1,7 @@ package com.interlinedlist.android.feature.profile.domain +import com.interlinedlist.android.core.network.preferences.NotificationTrayLimitStore + /** * Bounds and server defaults for the numeric message preferences, plus the fallbacks * the Settings UI shows when `GET /api/user` omits a preference. @@ -36,6 +38,21 @@ object SettingsBounds { /** The page size a fresh account gets (observed live on a real account). */ const val DEFAULT_MESSAGES_PER_PAGE: Int = 20 + /** + * How many notifications the tray holds. The help centre publishes this range + * outright — `/help/settings`: "The default is 20 and you can set any value from + * 10 to 40", corroborated by `/help/api/notifications` ("clamped to 10-40"). + * + * The bounds are taken from [NotificationTrayLimitStore] rather than restated + * here, because that store is what `:feature:notifications` sizes its list and its + * system-tray group by: one source of truth, so Settings can never offer a value + * the readers would clamp away. + */ + val NOTIFICATION_TRAY_LIMIT: IntRange = NotificationTrayLimitStore.RANGE + + /** The tray limit a fresh account gets (help centre: "The default is 20"). */ + const val DEFAULT_NOTIFICATION_TRAY_LIMIT: Int = NotificationTrayLimitStore.DEFAULT + /** New messages start public unless the account says otherwise. */ const val DEFAULT_PUBLICLY_VISIBLE: Boolean = true @@ -75,3 +92,7 @@ val UserSettings.showAdvancedPostSettingsOrDefault: Boolean /** Whether the account is private, falling back to public when the API omits it. */ val UserSettings.isPrivateAccountOrDefault: Boolean get() = isPrivateAccount ?: SettingsBounds.DEFAULT_PRIVATE_ACCOUNT + +/** The notification tray limit to show, falling back to the server default. */ +val UserSettings.notificationTrayLimitOrDefault: Int + get() = notificationTrayLimit ?: SettingsBounds.DEFAULT_NOTIFICATION_TRAY_LIMIT diff --git a/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/ui/settings/SettingsScreen.kt b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/ui/settings/SettingsScreen.kt index bb9dae6..e89efd1 100644 --- a/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/ui/settings/SettingsScreen.kt +++ b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/ui/settings/SettingsScreen.kt @@ -41,6 +41,7 @@ import com.interlinedlist.android.feature.profile.domain.defaultPubliclyVisibleO import com.interlinedlist.android.feature.profile.domain.isPrivateAccountOrDefault import com.interlinedlist.android.feature.profile.domain.maxMessageLengthOrDefault import com.interlinedlist.android.feature.profile.domain.messagesPerPageOrDefault +import com.interlinedlist.android.feature.profile.domain.notificationTrayLimitOrDefault import com.interlinedlist.android.feature.profile.domain.showAdvancedPostSettingsOrDefault /** Stable test tags for the Settings screen. */ @@ -57,6 +58,7 @@ object SettingsTestTags { const val SHOW_PREVIEWS = "settingsShowPreviews" const val MAX_MESSAGE_LENGTH = "settingsMaxMessageLength" const val MESSAGES_PER_PAGE = "settingsMessagesPerPage" + const val NOTIFICATION_TRAY_LIMIT = "settingsNotificationTrayLimit" const val DEFAULT_PUBLICLY_VISIBLE = "settingsDefaultPubliclyVisible" const val SHOW_ADVANCED_POST_SETTINGS = "settingsShowAdvancedPostSettings" const val PRIVATE_ACCOUNT = "settingsPrivateAccount" @@ -104,6 +106,7 @@ fun SettingsRoute( onSelectViewingPreference = viewModel::setViewingPreference, onToggleShowPreviews = viewModel::setShowPreviews, onSetMessagesPerPage = viewModel::setMessagesPerPage, + onSetNotificationTrayLimit = viewModel::setNotificationTrayLimit, onSetMaxMessageLength = viewModel::setMaxMessageLength, onToggleDefaultPubliclyVisible = viewModel::setDefaultPubliclyVisible, onToggleShowAdvancedPostSettings = viewModel::setShowAdvancedPostSettings, @@ -123,6 +126,7 @@ fun SettingsScreen( onSelectViewingPreference: (ViewingPreference) -> Unit, onToggleShowPreviews: (Boolean) -> Unit, onSetMessagesPerPage: (Int) -> Unit, + onSetNotificationTrayLimit: (Int) -> Unit, onSetMaxMessageLength: (Int) -> Unit, onToggleDefaultPubliclyVisible: (Boolean) -> Unit, onToggleShowAdvancedPostSettings: (Boolean) -> Unit, @@ -163,6 +167,7 @@ fun SettingsScreen( onSelectViewingPreference = onSelectViewingPreference, onToggleShowPreviews = onToggleShowPreviews, onSetMessagesPerPage = onSetMessagesPerPage, + onSetNotificationTrayLimit = onSetNotificationTrayLimit, ) MessageSettingsGroup( settings = settings, @@ -206,8 +211,14 @@ fun SettingsScreen( } /** - * "View preferences": which messages the Home feed shows, and whether link-preview - * cards render at all. + * "View preferences": which messages the Home feed shows, whether link-preview cards + * render at all, and how much the feed and the notification tray hold. + * + * The notification tray limit is filed here rather than under a notifications group + * because that is where the web keeps it: `/help/settings` lists it under **View + * preferences** alongside Messages per page, and the same page's Notifications section + * points back at it ("up to your Notification tray limit (see View Preferences + * above)"). */ @Composable private fun ViewPreferencesGroup( @@ -215,10 +226,11 @@ private fun ViewPreferencesGroup( onSelectViewingPreference: (ViewingPreference) -> Unit, onToggleShowPreviews: (Boolean) -> Unit, onSetMessagesPerPage: (Int) -> Unit, + onSetNotificationTrayLimit: (Int) -> Unit, ) { SettingsGroup( title = "View preferences", - description = "Control what appears in your Home feed.", + description = "Control what appears in your Home feed and your notification tray.", modifier = Modifier.testTag(SettingsTestTags.VIEW_PREFERENCES), ) { ViewingPreference.entries.forEach { option -> @@ -248,6 +260,16 @@ private fun ViewPreferencesGroup( onValueChange = onSetMessagesPerPage, tag = SettingsTestTags.MESSAGES_PER_PAGE, ) + HorizontalDivider(color = MaterialTheme.colorScheme.outlineVariant) + SettingsNumberRow( + label = "Notification tray limit", + description = "How many notifications the tray holds before older ones " + + "drop off (10 to 40).", + value = settings.notificationTrayLimitOrDefault, + range = SettingsBounds.NOTIFICATION_TRAY_LIMIT, + onValueChange = onSetNotificationTrayLimit, + tag = SettingsTestTags.NOTIFICATION_TRAY_LIMIT, + ) } } @@ -403,6 +425,7 @@ private fun SettingsScreenPreview() { maxMessageLength = 666, defaultPubliclyVisible = true, messagesPerPage = 20, + notificationTrayLimit = 20, viewingPreference = ViewingPreference.FOLLOWING, showPreviews = true, showAdvancedPostSettings = false, @@ -414,6 +437,7 @@ private fun SettingsScreenPreview() { onSelectViewingPreference = {}, onToggleShowPreviews = {}, onSetMessagesPerPage = {}, + onSetNotificationTrayLimit = {}, onSetMaxMessageLength = {}, onToggleDefaultPubliclyVisible = {}, onToggleShowAdvancedPostSettings = {}, diff --git a/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/ui/settings/SettingsViewModel.kt b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/ui/settings/SettingsViewModel.kt index 5fce0c2..4fa483c 100644 --- a/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/ui/settings/SettingsViewModel.kt +++ b/feature/profile/src/main/kotlin/com/interlinedlist/android/feature/profile/ui/settings/SettingsViewModel.kt @@ -12,6 +12,7 @@ import com.interlinedlist.android.feature.profile.domain.defaultPubliclyVisibleO import com.interlinedlist.android.feature.profile.domain.isPrivateAccountOrDefault import com.interlinedlist.android.feature.profile.domain.maxMessageLengthOrDefault import com.interlinedlist.android.feature.profile.domain.messagesPerPageOrDefault +import com.interlinedlist.android.feature.profile.domain.notificationTrayLimitOrDefault import com.interlinedlist.android.feature.profile.domain.showAdvancedPostSettingsOrDefault import com.interlinedlist.android.feature.profile.ui.common.toUserMessage import dagger.hilt.android.lifecycle.HiltViewModel @@ -170,6 +171,34 @@ class SettingsViewModel @Inject constructor( ) } + /** + * Sets how many notifications the tray holds before older ones drop off. The help + * centre documents the supported range as 10 to 40 + * ([SettingsBounds.NOTIFICATION_TRAY_LIMIT]); anything else is refused without a + * request. + * + * The saved value is not cosmetic: `:feature:notifications` sizes both its list + * and its system-tray group by it, so a rejected save must roll back rather than + * leave the two disagreeing about how much the tray holds. + */ + fun setNotificationTrayLimit(notifications: Int) { + val current = _uiState.value.settings ?: return + if (!withinRange( + notifications, + SettingsBounds.NOTIFICATION_TRAY_LIMIT, + "Notification tray limit", + ) + ) { + return + } + if (current.notificationTrayLimitOrDefault == notifications) return + save( + optimistic = current.copy(notificationTrayLimit = notifications), + previous = current, + update = UserSettingsUpdate(notificationTrayLimit = notifications), + ) + } + fun dismissError() = _uiState.update { it.copy(errorMessage = null) } /** diff --git a/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/data/DefaultSettingsRepositoryTest.kt b/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/data/DefaultSettingsRepositoryTest.kt index 238a4fe..41b237d 100644 --- a/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/data/DefaultSettingsRepositoryTest.kt +++ b/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/data/DefaultSettingsRepositoryTest.kt @@ -4,6 +4,8 @@ import com.google.common.truth.Truth.assertThat import com.interlinedlist.android.core.common.dispatcher.DispatcherProvider import com.interlinedlist.android.core.common.result.ApiResult import com.interlinedlist.android.core.common.result.AppError +import com.interlinedlist.android.core.network.api.InterlinedListApi +import com.interlinedlist.android.core.network.preferences.NotificationTrayLimitStore import com.interlinedlist.android.feature.profile.data.remote.ProfileApi import com.interlinedlist.android.feature.profile.domain.UserSettingsUpdate import com.interlinedlist.android.feature.profile.domain.ViewingPreference @@ -33,6 +35,7 @@ class DefaultSettingsRepositoryTest { private lateinit var server: MockWebServer private lateinit var api: ProfileApi private lateinit var repository: DefaultSettingsRepository + private lateinit var trayLimitStore: NotificationTrayLimitStore private val json = Json { ignoreUnknownKeys = true; explicitNulls = false } private val testDispatcher = StandardTestDispatcher() @@ -52,7 +55,9 @@ class DefaultSettingsRepositoryTest { .addConverterFactory(json.asConverterFactory("application/json".toMediaType())) .build() api = retrofit.create(ProfileApi::class.java) - repository = DefaultSettingsRepository(api, json, dispatchers) + trayLimitStore = + NotificationTrayLimitStore(retrofit.create(InterlinedListApi::class.java), json) + repository = DefaultSettingsRepository(api, trayLimitStore, json, dispatchers) } @After @@ -66,6 +71,7 @@ class DefaultSettingsRepositoryTest { messagesPerPage: Int = 20, showAdvancedPostSettings: Boolean = false, isPrivateAccount: Boolean = false, + notificationTrayLimit: Int = 25, ) = server.enqueue( MockResponse().setResponseCode(200).setBody( """ @@ -85,7 +91,7 @@ class DefaultSettingsRepositoryTest { "longitude": -122.68, "isPrivateAccount": $isPrivateAccount, "githubDefaultRepo": "adron/notes", - "notificationTrayLimit": 25 + "notificationTrayLimit": $notificationTrayLimit } } """.trimIndent(), @@ -346,4 +352,57 @@ class DefaultSettingsRepositoryTest { assertThat(result).isInstanceOf(ApiResult.Failure::class.java) assertThat(repository.observeSettings().first()?.isPrivateAccount).isFalse() } + + // --- Notification tray limit (issue #35) --------------------------------- + // The one preference a *different* feature module reads, so it has to reach the + // shared :core:network accessor as well as the settings cache. + + @Test + fun `notificationTrayLimit PATCHes alone as a JSON number`() = runTest(testDispatcher) { + enqueueUser(notificationTrayLimit = 40) + + val result = repository.update(UserSettingsUpdate(notificationTrayLimit = 40)) + + val body = server.takeJsonBody() + assertThat(body.keys).containsExactly("notificationTrayLimit") + val sent = body.getValue("notificationTrayLimit").jsonPrimitive + assertThat(sent.isString).isFalse() + assertThat(sent.intOrNull).isEqualTo(40) + assertThat((result as ApiResult.Success).data.notificationTrayLimit).isEqualTo(40) + assertThat(repository.observeSettings().first()?.notificationTrayLimit).isEqualTo(40) + } + + @Test + fun `a saved tray limit reaches the shared store the notifications feature reads`() = + runTest(testDispatcher) { + enqueueUser(notificationTrayLimit = 40) + + repository.update(UserSettingsUpdate(notificationTrayLimit = 40)) + + // No further response is queued: the store answers from what was published, + // so :feature:notifications sees the change without another GET /api/user. + assertThat(trayLimitStore.current()).isEqualTo(40) + } + + @Test + fun `a refresh republishes the tray limit to the shared store`() = runTest(testDispatcher) { + enqueueUser(notificationTrayLimit = 10) + + repository.refresh() + + assertThat(trayLimitStore.current()).isEqualTo(10) + } + + @Test + fun `an account without a stored tray limit publishes the documented default`() = + runTest(testDispatcher) { + server.enqueue( + MockResponse().setResponseCode(200) + .setBody("""{ "user": { "id": "u1", "username": "adron" } }"""), + ) + + repository.refresh() + + assertThat(trayLimitStore.current()).isEqualTo(20) + } } diff --git a/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/ui/FakeSettingsRepository.kt b/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/ui/FakeSettingsRepository.kt index 64ed648..edd4003 100644 --- a/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/ui/FakeSettingsRepository.kt +++ b/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/ui/FakeSettingsRepository.kt @@ -50,6 +50,8 @@ class FakeSettingsRepository : SettingsRepository { showAdvancedPostSettings = update.showAdvancedPostSettings ?: current.showAdvancedPostSettings, isPrivateAccount = update.isPrivateAccount ?: current.isPrivateAccount, + notificationTrayLimit = + update.notificationTrayLimit ?: current.notificationTrayLimit, ) } } diff --git a/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/ui/SettingsNotificationTrayLimitTest.kt b/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/ui/SettingsNotificationTrayLimitTest.kt new file mode 100644 index 0000000..e099138 --- /dev/null +++ b/feature/profile/src/test/kotlin/com/interlinedlist/android/feature/profile/ui/SettingsNotificationTrayLimitTest.kt @@ -0,0 +1,178 @@ +package com.interlinedlist.android.feature.profile.ui + +import app.cash.turbine.test +import com.google.common.truth.Truth.assertThat +import com.interlinedlist.android.core.common.result.ApiResult +import com.interlinedlist.android.core.common.result.AppError +import com.interlinedlist.android.feature.profile.domain.SettingsBounds +import com.interlinedlist.android.feature.profile.domain.UserSettings +import com.interlinedlist.android.feature.profile.ui.settings.SettingsViewModel +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain +import org.junit.After +import org.junit.Before +import org.junit.Test + +/** + * Issue #35: the notification tray limit, saved from Settings. + * + * The help centre publishes the range outright — `/help/settings`: "Notification tray + * limit: How many notifications the bell tray holds before older ones drop off. The + * default is 20 and you can set any value from 10 to 40" — and `/help/api/notifications` + * corroborates it ("default 20, clamped to 10-40"), so out-of-range input is refused + * here rather than spent on a request the server would reject. + * + * Like every other preference it PATCHes alone, applies optimistically and rolls back + * when the save fails — which matters more here than for a cosmetic toggle, because + * `:feature:notifications` sizes both its list and its system-tray group by the value. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class SettingsNotificationTrayLimitTest { + + private val dispatcher = StandardTestDispatcher() + private lateinit var repo: FakeSettingsRepository + + @Before + fun setUp() { + Dispatchers.setMain(dispatcher) + repo = FakeSettingsRepository() + } + + @After + fun tearDown() = Dispatchers.resetMain() + + private fun loadedViewModel(settings: UserSettings): SettingsViewModel { + repo.refreshResult = ApiResult.Success(settings) + return SettingsViewModel(repo) + } + + private val serverErrorMessage = "InterlinedList is having trouble right now. Try again shortly." + + private val outOfRangeMessage = + "Notification tray limit must be between " + + "${SettingsBounds.NOTIFICATION_TRAY_LIMIT.first} and " + + "${SettingsBounds.NOTIFICATION_TRAY_LIMIT.last}." + + @Test + fun `setting the tray limit PATCHes only that field`() = runTest(dispatcher) { + val vm = loadedViewModel(UserSettings(notificationTrayLimit = 20)) + + vm.uiState.test { + advanceUntilIdle() + vm.setNotificationTrayLimit(40) + + // Applied optimistically, before the request comes back. + assertThat(vm.uiState.value.settings?.notificationTrayLimit).isEqualTo(40) + assertThat(vm.uiState.value.isSaving).isTrue() + advanceUntilIdle() + + val sent = repo.updates.single() + assertThat(sent.notificationTrayLimit).isEqualTo(40) + assertThat(sent.touchedFieldNames()).containsExactly("notificationTrayLimit") + val state = expectMostRecentItem() + assertThat(state.settings?.notificationTrayLimit).isEqualTo(40) + assertThat(state.isSaving).isFalse() + assertThat(state.errorMessage).isNull() + } + } + + @Test + fun `a limit above the documented maximum is refused without a request`() = runTest(dispatcher) { + val vm = loadedViewModel(UserSettings(notificationTrayLimit = 20)) + + vm.uiState.test { + advanceUntilIdle() + vm.setNotificationTrayLimit(SettingsBounds.NOTIFICATION_TRAY_LIMIT.last + 1) + advanceUntilIdle() + + assertThat(repo.updates).isEmpty() + val state = expectMostRecentItem() + assertThat(state.settings?.notificationTrayLimit).isEqualTo(20) + assertThat(state.isSaving).isFalse() + assertThat(state.errorMessage).isEqualTo(outOfRangeMessage) + } + } + + @Test + fun `a limit below the documented minimum is refused without a request`() = runTest(dispatcher) { + val vm = loadedViewModel(UserSettings(notificationTrayLimit = 20)) + + vm.uiState.test { + advanceUntilIdle() + vm.setNotificationTrayLimit(SettingsBounds.NOTIFICATION_TRAY_LIMIT.first - 1) + advanceUntilIdle() + + assertThat(repo.updates).isEmpty() + val state = expectMostRecentItem() + assertThat(state.settings?.notificationTrayLimit).isEqualTo(20) + assertThat(state.errorMessage).isEqualTo(outOfRangeMessage) + } + } + + @Test + fun `both ends of the documented range are accepted`() = runTest(dispatcher) { + val vm = loadedViewModel(UserSettings(notificationTrayLimit = 20)) + advanceUntilIdle() + + vm.setNotificationTrayLimit(SettingsBounds.NOTIFICATION_TRAY_LIMIT.first) + advanceUntilIdle() + vm.setNotificationTrayLimit(SettingsBounds.NOTIFICATION_TRAY_LIMIT.last) + advanceUntilIdle() + + assertThat(repo.updates.map { it.notificationTrayLimit }).containsExactly(10, 40).inOrder() + assertThat(vm.uiState.value.errorMessage).isNull() + } + + @Test + fun `a failed save rolls back to the stored limit and surfaces the error`() = runTest(dispatcher) { + val vm = loadedViewModel(UserSettings(notificationTrayLimit = 20)) + repo.updateResult = { ApiResult.Failure(AppError.Server("boom")) } + + vm.uiState.test { + advanceUntilIdle() + vm.setNotificationTrayLimit(40) + assertThat(vm.uiState.value.settings?.notificationTrayLimit).isEqualTo(40) + advanceUntilIdle() + + val state = expectMostRecentItem() + assertThat(state.settings?.notificationTrayLimit).isEqualTo(20) + assertThat(state.isSaving).isFalse() + assertThat(state.errorMessage).isEqualTo(serverErrorMessage) + } + } + + @Test + fun `re-entering the current limit does not call the API`() = runTest(dispatcher) { + val vm = loadedViewModel(UserSettings(notificationTrayLimit = 30)) + advanceUntilIdle() + + vm.setNotificationTrayLimit(30) + advanceUntilIdle() + + assertThat(repo.updates).isEmpty() + } + + @Test + fun `entering the server default when the account has no stored limit does not call the API`() = + runTest(dispatcher) { + // The row shows 20 for a null stored value, so "setting" 20 changes nothing. + val vm = loadedViewModel(UserSettings(notificationTrayLimit = null)) + advanceUntilIdle() + + vm.setNotificationTrayLimit(SettingsBounds.DEFAULT_NOTIFICATION_TRAY_LIMIT) + advanceUntilIdle() + + assertThat(repo.updates).isEmpty() + } + + @Test + fun `the documented bounds match the help centre`() { + assertThat(SettingsBounds.NOTIFICATION_TRAY_LIMIT).isEqualTo(10..40) + assertThat(SettingsBounds.DEFAULT_NOTIFICATION_TRAY_LIMIT).isEqualTo(20) + } +}