fix: Android's admin requests names who asked from the request itself (#6267)
release / govulncheck (push) Successful in 17s
release / web (push) Successful in 1m35s
release / go (push) Successful in 1m46s
release / integration (push) Successful in 5m27s
release / android (push) Successful in 5m33s
release / Build signed APK (releases and dev) (push) Successful in 5m51s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 14s
release / Verify release artifacts (tag releases only) (push) Skipped
release / govulncheck (push) Successful in 17s
release / web (push) Successful in 1m35s
release / go (push) Successful in 1m46s
release / integration (push) Successful in 5m27s
release / android (push) Successful in 5m33s
release / Build signed APK (releases and dev) (push) Successful in 5m51s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 14s
release / Verify release artifacts (tag releases only) (push) Skipped
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 <noreply@anthropic.com>
This commit is contained in:
+1
@@ -33,6 +33,7 @@ class AdminRequestsRepository @Inject constructor(
|
|||||||
private fun RequestWire.toDomain(): RequestRef = RequestRef(
|
private fun RequestWire.toDomain(): RequestRef = RequestRef(
|
||||||
id = id,
|
id = id,
|
||||||
userId = userId,
|
userId = userId,
|
||||||
|
username = username,
|
||||||
status = RequestStatus.fromWire(status),
|
status = RequestStatus.fromWire(status),
|
||||||
kind = kind,
|
kind = kind,
|
||||||
artistName = artistName,
|
artistName = artistName,
|
||||||
|
|||||||
@@ -71,7 +71,6 @@ fun AdminRequestsScreen(
|
|||||||
)
|
)
|
||||||
is AdminRequestsUiState.Success -> RequestList(
|
is AdminRequestsUiState.Success -> RequestList(
|
||||||
rows = s.rows,
|
rows = s.rows,
|
||||||
usernames = s.usernames,
|
|
||||||
onApprove = viewModel::approve,
|
onApprove = viewModel::approve,
|
||||||
onReject = viewModel::reject,
|
onReject = viewModel::reject,
|
||||||
)
|
)
|
||||||
@@ -83,7 +82,6 @@ fun AdminRequestsScreen(
|
|||||||
@Composable
|
@Composable
|
||||||
private fun RequestList(
|
private fun RequestList(
|
||||||
rows: List<RequestRef>,
|
rows: List<RequestRef>,
|
||||||
usernames: Map<String, String>,
|
|
||||||
onApprove: (String) -> Unit,
|
onApprove: (String) -> Unit,
|
||||||
onReject: (String) -> Unit,
|
onReject: (String) -> Unit,
|
||||||
) {
|
) {
|
||||||
@@ -91,9 +89,9 @@ private fun RequestList(
|
|||||||
items(items = rows, key = { it.id }) { req ->
|
items(items = rows, key = { it.id }) { req ->
|
||||||
AdminRequestRow(
|
AdminRequestRow(
|
||||||
req = req,
|
req = req,
|
||||||
requester = usernames[req.userId]
|
requester = req.username.ifEmpty {
|
||||||
?: req.userId.takeIf { it.isNotEmpty() }?.take(USER_ID_PREFIX_LEN)
|
req.userId.take(USER_ID_PREFIX_LEN).ifEmpty { "unknown" }
|
||||||
?: "unknown",
|
},
|
||||||
onApprove = { onApprove(req.id) },
|
onApprove = { onApprove(req.id) },
|
||||||
onReject = { onReject(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
|
// UUID prefix length used as a friendlier-than-raw-UUID fallback when a
|
||||||
// the admin users list fetch fails and we can't resolve userId → name.
|
// row comes back without the requester's username.
|
||||||
private const val USER_ID_PREFIX_LEN = 8
|
private const val USER_ID_PREFIX_LEN = 8
|
||||||
|
|||||||
+4
-19
@@ -3,7 +3,6 @@ package com.fabledsword.minstrel.admin.ui
|
|||||||
import androidx.lifecycle.ViewModel
|
import androidx.lifecycle.ViewModel
|
||||||
import androidx.lifecycle.viewModelScope
|
import androidx.lifecycle.viewModelScope
|
||||||
import com.fabledsword.minstrel.admin.data.AdminRequestsRepository
|
import com.fabledsword.minstrel.admin.data.AdminRequestsRepository
|
||||||
import com.fabledsword.minstrel.admin.data.AdminUsersRepository
|
|
||||||
import com.fabledsword.minstrel.api.ErrorCopy
|
import com.fabledsword.minstrel.api.ErrorCopy
|
||||||
import com.fabledsword.minstrel.connectivity.NetworkStatusController
|
import com.fabledsword.minstrel.connectivity.NetworkStatusController
|
||||||
import com.fabledsword.minstrel.connectivity.recoveries
|
import com.fabledsword.minstrel.connectivity.recoveries
|
||||||
@@ -11,8 +10,6 @@ import com.fabledsword.minstrel.events.EventsStream
|
|||||||
import com.fabledsword.minstrel.models.RequestRef
|
import com.fabledsword.minstrel.models.RequestRef
|
||||||
import dagger.hilt.android.lifecycle.HiltViewModel
|
import dagger.hilt.android.lifecycle.HiltViewModel
|
||||||
import kotlinx.coroutines.Job
|
import kotlinx.coroutines.Job
|
||||||
import kotlinx.coroutines.async
|
|
||||||
import kotlinx.coroutines.coroutineScope
|
|
||||||
import kotlinx.coroutines.flow.MutableStateFlow
|
import kotlinx.coroutines.flow.MutableStateFlow
|
||||||
import kotlinx.coroutines.flow.StateFlow
|
import kotlinx.coroutines.flow.StateFlow
|
||||||
import kotlinx.coroutines.flow.asStateFlow
|
import kotlinx.coroutines.flow.asStateFlow
|
||||||
@@ -25,17 +22,13 @@ private val RELEVANT_EVENT_KINDS = setOf("request.status_changed")
|
|||||||
sealed interface AdminRequestsUiState {
|
sealed interface AdminRequestsUiState {
|
||||||
data object Loading : AdminRequestsUiState
|
data object Loading : AdminRequestsUiState
|
||||||
data object Empty : AdminRequestsUiState
|
data object Empty : AdminRequestsUiState
|
||||||
data class Success(
|
data class Success(val rows: List<RequestRef>) : AdminRequestsUiState
|
||||||
val rows: List<RequestRef>,
|
|
||||||
val usernames: Map<String, String>,
|
|
||||||
) : AdminRequestsUiState
|
|
||||||
data class Error(val message: String) : AdminRequestsUiState
|
data class Error(val message: String) : AdminRequestsUiState
|
||||||
}
|
}
|
||||||
|
|
||||||
@HiltViewModel
|
@HiltViewModel
|
||||||
class AdminRequestsViewModel @Inject constructor(
|
class AdminRequestsViewModel @Inject constructor(
|
||||||
private val repository: AdminRequestsRepository,
|
private val repository: AdminRequestsRepository,
|
||||||
private val usersRepository: AdminUsersRepository,
|
|
||||||
private val eventsStream: EventsStream,
|
private val eventsStream: EventsStream,
|
||||||
networkStatus: NetworkStatusController,
|
networkStatus: NetworkStatusController,
|
||||||
) : ViewModel() {
|
) : ViewModel() {
|
||||||
@@ -62,19 +55,11 @@ class AdminRequestsViewModel @Inject constructor(
|
|||||||
fun refresh(): Job = viewModelScope.launch {
|
fun refresh(): Job = viewModelScope.launch {
|
||||||
internal.value = AdminRequestsUiState.Loading
|
internal.value = AdminRequestsUiState.Loading
|
||||||
try {
|
try {
|
||||||
val (rows, usernames) = coroutineScope {
|
val rows = repository.list()
|
||||||
val requestsJob = async { repository.list() }
|
|
||||||
val usernamesJob = async {
|
|
||||||
runCatching {
|
|
||||||
usersRepository.list().associate { it.id to it.username }
|
|
||||||
}.getOrDefault(emptyMap())
|
|
||||||
}
|
|
||||||
requestsJob.await() to usernamesJob.await()
|
|
||||||
}
|
|
||||||
internal.value = if (rows.isEmpty()) {
|
internal.value = if (rows.isEmpty()) {
|
||||||
AdminRequestsUiState.Empty
|
AdminRequestsUiState.Empty
|
||||||
} else {
|
} else {
|
||||||
AdminRequestsUiState.Success(rows = rows, usernames = usernames)
|
AdminRequestsUiState.Success(rows = rows)
|
||||||
}
|
}
|
||||||
} catch (
|
} catch (
|
||||||
@Suppress("TooGenericExceptionCaught") e: Throwable,
|
@Suppress("TooGenericExceptionCaught") e: Throwable,
|
||||||
@@ -101,7 +86,7 @@ class AdminRequestsViewModel @Inject constructor(
|
|||||||
internal.value = if (filtered.isEmpty()) {
|
internal.value = if (filtered.isEmpty()) {
|
||||||
AdminRequestsUiState.Empty
|
AdminRequestsUiState.Empty
|
||||||
} else {
|
} else {
|
||||||
AdminRequestsUiState.Success(rows = filtered, usernames = before.usernames)
|
AdminRequestsUiState.Success(rows = filtered)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
viewModelScope.launch {
|
viewModelScope.launch {
|
||||||
|
|||||||
@@ -41,6 +41,7 @@ enum class RequestStatus {
|
|||||||
data class RequestRef(
|
data class RequestRef(
|
||||||
val id: String,
|
val id: String,
|
||||||
val userId: String,
|
val userId: String,
|
||||||
|
val username: String = "",
|
||||||
val status: RequestStatus,
|
val status: RequestStatus,
|
||||||
val kind: String,
|
val kind: String,
|
||||||
val artistName: String,
|
val artistName: String,
|
||||||
|
|||||||
@@ -20,6 +20,8 @@ import kotlinx.serialization.Serializable
|
|||||||
data class RequestWire(
|
data class RequestWire(
|
||||||
val id: String,
|
val id: String,
|
||||||
@SerialName("user_id") val userId: 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 status: String = "pending",
|
||||||
val kind: String = "artist",
|
val kind: String = "artist",
|
||||||
@SerialName("artist_name") val artistName: String = "",
|
@SerialName("artist_name") val artistName: String = "",
|
||||||
|
|||||||
+39
@@ -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)
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user