From fefc51439e9e471889fe34a36e5eb6d98bf94fd6 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 22 Oct 2026 21:10:17 -0400 Subject: [PATCH] fix: Android's admin requests names who asked from the request itself (#6267) The admin requests list now sends each requester's username (#6265), so the Android queue reads it off the row instead of fetching the whole admin users list alongside and mapping ids to names. The short id stays as the fallback for a row that comes back without a name. Co-Authored-By: Claude Opus 5.5 --- .../admin/data/AdminRequestsRepository.kt | 1 + .../minstrel/admin/ui/AdminRequestsScreen.kt | 12 +++--- .../admin/ui/AdminRequestsViewModel.kt | 23 ++--------- .../fabledsword/minstrel/models/Request.kt | 1 + .../minstrel/models/wire/RequestWire.kt | 2 + .../admin/data/AdminRequestsRepositoryTest.kt | 39 +++++++++++++++++++ 6 files changed, 52 insertions(+), 26 deletions(-) create mode 100644 android/app/src/test/java/com/fabledsword/minstrel/admin/data/AdminRequestsRepositoryTest.kt diff --git a/android/app/src/main/java/com/fabledsword/minstrel/admin/data/AdminRequestsRepository.kt b/android/app/src/main/java/com/fabledsword/minstrel/admin/data/AdminRequestsRepository.kt index c6ee38f5..2d0afb55 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/admin/data/AdminRequestsRepository.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/admin/data/AdminRequestsRepository.kt @@ -33,6 +33,7 @@ class AdminRequestsRepository @Inject constructor( private fun RequestWire.toDomain(): RequestRef = RequestRef( id = id, userId = userId, + username = username, status = RequestStatus.fromWire(status), kind = kind, artistName = artistName, diff --git a/android/app/src/main/java/com/fabledsword/minstrel/admin/ui/AdminRequestsScreen.kt b/android/app/src/main/java/com/fabledsword/minstrel/admin/ui/AdminRequestsScreen.kt index d3be7555..f0269712 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/admin/ui/AdminRequestsScreen.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/admin/ui/AdminRequestsScreen.kt @@ -71,7 +71,6 @@ fun AdminRequestsScreen( ) is AdminRequestsUiState.Success -> RequestList( rows = s.rows, - usernames = s.usernames, onApprove = viewModel::approve, onReject = viewModel::reject, ) @@ -83,7 +82,6 @@ fun AdminRequestsScreen( @Composable private fun RequestList( rows: List, - usernames: Map, onApprove: (String) -> Unit, onReject: (String) -> Unit, ) { @@ -91,9 +89,9 @@ private fun RequestList( items(items = rows, key = { it.id }) { req -> AdminRequestRow( req = req, - requester = usernames[req.userId] - ?: req.userId.takeIf { it.isNotEmpty() }?.take(USER_ID_PREFIX_LEN) - ?: "unknown", + requester = req.username.ifEmpty { + req.userId.take(USER_ID_PREFIX_LEN).ifEmpty { "unknown" } + }, onApprove = { onApprove(req.id) }, onReject = { onReject(req.id) }, ) @@ -160,6 +158,6 @@ private fun AdminRequestRow( } } -// UUID prefix length used as a friendlier-than-raw-UUID fallback when -// the admin users list fetch fails and we can't resolve userId → name. +// UUID prefix length used as a friendlier-than-raw-UUID fallback when a +// row comes back without the requester's username. private const val USER_ID_PREFIX_LEN = 8 diff --git a/android/app/src/main/java/com/fabledsword/minstrel/admin/ui/AdminRequestsViewModel.kt b/android/app/src/main/java/com/fabledsword/minstrel/admin/ui/AdminRequestsViewModel.kt index 95a5af32..c265924a 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/admin/ui/AdminRequestsViewModel.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/admin/ui/AdminRequestsViewModel.kt @@ -3,7 +3,6 @@ package com.fabledsword.minstrel.admin.ui import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import com.fabledsword.minstrel.admin.data.AdminRequestsRepository -import com.fabledsword.minstrel.admin.data.AdminUsersRepository import com.fabledsword.minstrel.api.ErrorCopy import com.fabledsword.minstrel.connectivity.NetworkStatusController import com.fabledsword.minstrel.connectivity.recoveries @@ -11,8 +10,6 @@ import com.fabledsword.minstrel.events.EventsStream import com.fabledsword.minstrel.models.RequestRef import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.Job -import kotlinx.coroutines.async -import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow @@ -25,17 +22,13 @@ private val RELEVANT_EVENT_KINDS = setOf("request.status_changed") sealed interface AdminRequestsUiState { data object Loading : AdminRequestsUiState data object Empty : AdminRequestsUiState - data class Success( - val rows: List, - val usernames: Map, - ) : AdminRequestsUiState + data class Success(val rows: List) : AdminRequestsUiState data class Error(val message: String) : AdminRequestsUiState } @HiltViewModel class AdminRequestsViewModel @Inject constructor( private val repository: AdminRequestsRepository, - private val usersRepository: AdminUsersRepository, private val eventsStream: EventsStream, networkStatus: NetworkStatusController, ) : ViewModel() { @@ -62,19 +55,11 @@ class AdminRequestsViewModel @Inject constructor( fun refresh(): Job = viewModelScope.launch { internal.value = AdminRequestsUiState.Loading try { - val (rows, usernames) = coroutineScope { - val requestsJob = async { repository.list() } - val usernamesJob = async { - runCatching { - usersRepository.list().associate { it.id to it.username } - }.getOrDefault(emptyMap()) - } - requestsJob.await() to usernamesJob.await() - } + val rows = repository.list() internal.value = if (rows.isEmpty()) { AdminRequestsUiState.Empty } else { - AdminRequestsUiState.Success(rows = rows, usernames = usernames) + AdminRequestsUiState.Success(rows = rows) } } catch ( @Suppress("TooGenericExceptionCaught") e: Throwable, @@ -101,7 +86,7 @@ class AdminRequestsViewModel @Inject constructor( internal.value = if (filtered.isEmpty()) { AdminRequestsUiState.Empty } else { - AdminRequestsUiState.Success(rows = filtered, usernames = before.usernames) + AdminRequestsUiState.Success(rows = filtered) } } viewModelScope.launch { diff --git a/android/app/src/main/java/com/fabledsword/minstrel/models/Request.kt b/android/app/src/main/java/com/fabledsword/minstrel/models/Request.kt index bd11ea50..dd04374f 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/models/Request.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/models/Request.kt @@ -41,6 +41,7 @@ enum class RequestStatus { data class RequestRef( val id: String, val userId: String, + val username: String = "", val status: RequestStatus, val kind: String, val artistName: String, diff --git a/android/app/src/main/java/com/fabledsword/minstrel/models/wire/RequestWire.kt b/android/app/src/main/java/com/fabledsword/minstrel/models/wire/RequestWire.kt index f6be7723..548a3fe2 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/models/wire/RequestWire.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/models/wire/RequestWire.kt @@ -20,6 +20,8 @@ import kotlinx.serialization.Serializable data class RequestWire( val id: String, @SerialName("user_id") val userId: String = "", + // The requester's username. Only `GET /api/admin/requests` sends it. + val username: String = "", val status: String = "pending", val kind: String = "artist", @SerialName("artist_name") val artistName: String = "", diff --git a/android/app/src/test/java/com/fabledsword/minstrel/admin/data/AdminRequestsRepositoryTest.kt b/android/app/src/test/java/com/fabledsword/minstrel/admin/data/AdminRequestsRepositoryTest.kt new file mode 100644 index 00000000..729a9e3c --- /dev/null +++ b/android/app/src/test/java/com/fabledsword/minstrel/admin/data/AdminRequestsRepositoryTest.kt @@ -0,0 +1,39 @@ +package com.fabledsword.minstrel.admin.data + +import com.fabledsword.minstrel.api.endpoints.AdminRequestsApi +import com.fabledsword.minstrel.models.wire.RequestWire +import io.mockk.coEvery +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.test.runTest +import org.junit.jupiter.api.Test +import retrofit2.Retrofit +import kotlin.test.assertEquals + +/** + * The admin queue names who asked from the row itself (#6265): the server + * sends the requester's username, so the screen needs no users-list fetch. + */ +class AdminRequestsRepositoryTest { + private val api: AdminRequestsApi = mockk(relaxed = true) + private val retrofit: Retrofit = mockk { + every { create(AdminRequestsApi::class.java) } returns api + } + private val repo = AdminRequestsRepository(retrofit) + + @Test + fun `a request carries the requester's username`() = runTest { + coEvery { api.list() } returns listOf( + RequestWire(id = "r1", userId = "f7e1ec86-0e76-4cad-8191-b0cc78492851", username = "alice"), + ) + + assertEquals("alice", repo.list().single().username) + } + + @Test + fun `a request without a username keeps it empty`() = runTest { + coEvery { api.list() } returns listOf(RequestWire(id = "r1", userId = "u1")) + + assertEquals("", repo.list().single().username) + } +}