From 26d3388ffff97c38ae77271ee313b6dae33524fd Mon Sep 17 00:00:00 2001 From: Adron Hall Date: Wed, 16 Sep 2026 13:24:05 -0700 Subject: [PATCH] feat(lists): saved views with shared/personal scope and fork-to-personal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lists could only ever render one way; they now carry saved views. Adds the DTOs, domain model, repository calls and a switcher on the list detail screen. - GET/POST/PUT/DELETE /api/lists/{id}/views, plus POST …/views/{viewId} to fork a view into a personal copy — the escape hatch when somebody else's shared view does not suit. - `scope` is an enum; a missing or unrecognised scope fails locally rather than spending a request the API answers with a 400. - `config` is kept as its raw JSON object so keys this client does not model survive a round-trip; mode/density/filters are projected from it. - Every write adopts the view the server returned, because the API silently drops config values it does not recognise instead of rejecting them. - Renaming and deleting are offered only for views the user owns; somebody else's shared view is forked instead, and a server refusal is surfaced with the view left in place. The switcher is a slot on the stateless detail screen with its own ViewModel on the same nav entry, so the detail screen and its tests stay independent of it. Tests: 12 MockWebServer round-trips over the real paths and bodies, 5 mapper cases, 12 ViewModel cases, and 6 Compose cases for the switcher. Closes #51 --- .../lists/ui/views/ListViewSwitcherTest.kt | 168 ++++++ .../lists/data/CurrentUserIdProvider.kt | 11 + .../lists/data/DefaultListsRepository.kt | 84 +++ .../feature/lists/data/ListViewMapper.kt | 26 + .../feature/lists/data/ListsRepository.kt | 51 ++ .../feature/lists/data/remote/ListsApi.kt | 42 ++ .../feature/lists/data/remote/dto/ViewDtos.kt | 93 ++++ .../android/feature/lists/di/ListsModule.kt | 8 + .../android/feature/lists/domain/ListView.kt | 140 +++++ .../lists/ui/detail/ListDetailScreen.kt | 13 +- .../lists/ui/views/ListViewSwitcher.kt | 526 ++++++++++++++++++ .../lists/ui/views/ListViewsViewModel.kt | 240 ++++++++ .../feature/lists/FakeListsRepository.kt | 68 +++ .../data/DefaultListsRepositoryViewsTest.kt | 326 +++++++++++ .../feature/lists/data/ListViewMapperTest.kt | 79 +++ .../lists/ui/views/ListViewsViewModelTest.kt | 287 ++++++++++ 16 files changed, 2161 insertions(+), 1 deletion(-) create mode 100644 feature/lists/src/androidTest/kotlin/com/interlinedlist/android/feature/lists/ui/views/ListViewSwitcherTest.kt create mode 100644 feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/data/CurrentUserIdProvider.kt create mode 100644 feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/data/ListViewMapper.kt create mode 100644 feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/data/remote/dto/ViewDtos.kt create mode 100644 feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/domain/ListView.kt create mode 100644 feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/ui/views/ListViewSwitcher.kt create mode 100644 feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/ui/views/ListViewsViewModel.kt create mode 100644 feature/lists/src/test/kotlin/com/interlinedlist/android/feature/lists/data/DefaultListsRepositoryViewsTest.kt create mode 100644 feature/lists/src/test/kotlin/com/interlinedlist/android/feature/lists/data/ListViewMapperTest.kt create mode 100644 feature/lists/src/test/kotlin/com/interlinedlist/android/feature/lists/ui/views/ListViewsViewModelTest.kt diff --git a/feature/lists/src/androidTest/kotlin/com/interlinedlist/android/feature/lists/ui/views/ListViewSwitcherTest.kt b/feature/lists/src/androidTest/kotlin/com/interlinedlist/android/feature/lists/ui/views/ListViewSwitcherTest.kt new file mode 100644 index 0000000..81b61f4 --- /dev/null +++ b/feature/lists/src/androidTest/kotlin/com/interlinedlist/android/feature/lists/ui/views/ListViewSwitcherTest.kt @@ -0,0 +1,168 @@ +package com.interlinedlist.android.feature.lists.ui.views + +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.performClick +import androidx.compose.ui.test.performTextInput +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.google.common.truth.Truth.assertThat +import com.interlinedlist.android.core.designsystem.theme.InterlinedListTheme +import com.interlinedlist.android.feature.lists.domain.ListView +import com.interlinedlist.android.feature.lists.domain.ListViewConfig +import com.interlinedlist.android.feature.lists.domain.ListViewScope +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith + +/** + * Verifies the saved-view switcher surfaces shared and personal views, marks the + * default, and offers only the actions the user is allowed: somebody else's + * shared view can be forked into a personal copy but not renamed or deleted. + */ +@RunWith(AndroidJUnit4::class) +class ListViewSwitcherTest { + + @get:Rule + val composeRule = createComposeRule() + + private val theirShared = ListView( + id = "v1", + listId = "L1", + userId = "someone-else", + name = "Roadmap", + scope = ListViewScope.SHARED, + config = ListViewConfig.DEFAULT, + isDefault = true, + position = 0, + ) + + private val myPersonal = theirShared.copy( + id = "v2", + userId = "me", + name = "My cut", + scope = ListViewScope.PERSONAL, + isDefault = false, + ) + + private fun state(vararg views: ListView) = ListViewsUiState( + views = views.toList(), + selectedViewId = views.firstOrNull()?.id, + currentUserId = "me", + isLoading = false, + ) + + private fun setContent( + state: ListViewsUiState, + onFork: (ListView) -> Unit = {}, + onRename: (ListView) -> Unit = {}, + onDelete: (ListView) -> Unit = {}, + onSetDefault: (ListView) -> Unit = {}, + onSelect: (ListView) -> Unit = {}, + onCreate: (String, ListViewScope) -> Unit = { _, _ -> }, + ) { + composeRule.setContent { + InterlinedListTheme { + ListViewSwitcherContent( + state = state, + onSelect = onSelect, + onSetDefault = onSetDefault, + onFork = onFork, + onRename = onRename, + onDelete = onDelete, + onCreate = onCreate, + ) + } + } + } + + @Test + fun barShowsTheSelectedViewAndItsDefaultBadge() { + composeRule.setContent { + InterlinedListTheme { + ListViewSwitcherBar(state = state(theirShared, myPersonal), onOpen = {}) + } + } + + composeRule.onNodeWithTag(ListViewSwitcherTestTags.BAR).assertIsDisplayed() + composeRule.onNodeWithText("Roadmap").assertIsDisplayed() + composeRule.onNodeWithText("Default").assertIsDisplayed() + } + + @Test + fun listsSharedAndPersonalViewsSeparately() { + setContent(state(theirShared, myPersonal)) + + composeRule.onNodeWithText("Shared").assertIsDisplayed() + composeRule.onNodeWithText("Personal").assertIsDisplayed() + composeRule.onNodeWithTag(ListViewSwitcherTestTags.view("v1")).assertIsDisplayed() + composeRule.onNodeWithTag(ListViewSwitcherTestTags.view("v2")).assertIsDisplayed() + } + + @Test + fun someoneElsesSharedViewOffersAForkButNotRenameOrDelete() { + var forked: ListView? = null + setContent(state(theirShared), onFork = { forked = it }) + + composeRule.onNodeWithTag(ListViewSwitcherTestTags.overflow("v1")).performClick() + + // Fork is the escape hatch, and says what it does. + composeRule.onNodeWithTag(ListViewSwitcherTestTags.fork("v1")).assertIsDisplayed() + composeRule.onNodeWithText("Copies this view to your own. The shared one is untouched.") + .assertIsDisplayed() + composeRule.onNodeWithTag(ListViewSwitcherTestTags.rename("v1")).assertDoesNotExist() + composeRule.onNodeWithTag(ListViewSwitcherTestTags.delete("v1")).assertDoesNotExist() + + composeRule.onNodeWithTag(ListViewSwitcherTestTags.fork("v1")).performClick() + assertThat(forked?.id).isEqualTo("v1") + } + + @Test + fun ownViewCanBeRenamedDeletedAndMadeDefault() { + var renamed: ListView? = null + var deleted: ListView? = null + var defaulted: ListView? = null + setContent( + state(myPersonal), + onRename = { renamed = it }, + onDelete = { deleted = it }, + onSetDefault = { defaulted = it }, + ) + + composeRule.onNodeWithTag(ListViewSwitcherTestTags.overflow("v2")).performClick() + composeRule.onNodeWithTag(ListViewSwitcherTestTags.setDefault("v2")).performClick() + assertThat(defaulted?.id).isEqualTo("v2") + + composeRule.onNodeWithTag(ListViewSwitcherTestTags.overflow("v2")).performClick() + composeRule.onNodeWithTag(ListViewSwitcherTestTags.rename("v2")).performClick() + assertThat(renamed?.id).isEqualTo("v2") + + composeRule.onNodeWithTag(ListViewSwitcherTestTags.overflow("v2")).performClick() + composeRule.onNodeWithTag(ListViewSwitcherTestTags.delete("v2")).performClick() + assertThat(deleted?.id).isEqualTo("v2") + } + + @Test + fun creatingAViewSendsTheNameAndTheChosenScope() { + var created: Pair? = null + setContent(state(), onCreate = { name, scope -> created = name to scope }) + + composeRule.onNodeWithTag(ListViewSwitcherTestTags.EMPTY).assertIsDisplayed() + composeRule.onNodeWithTag(ListViewSwitcherTestTags.CREATE_NAME).performTextInput("By status") + composeRule.onNodeWithTag(ListViewSwitcherTestTags.scope(ListViewScope.SHARED)).performClick() + composeRule.onNodeWithTag(ListViewSwitcherTestTags.CREATE_SUBMIT).performClick() + + assertThat(created).isEqualTo("By status" to ListViewScope.SHARED) + } + + @Test + fun aRefusalFromTheServerIsShownInTheSheet() { + setContent( + state(theirShared).copy(errorMessage = "You cannot modify this view"), + ) + + composeRule.onNodeWithTag(ListViewSwitcherTestTags.ERROR).assertIsDisplayed() + composeRule.onNodeWithText("You cannot modify this view").assertIsDisplayed() + } +} diff --git a/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/data/CurrentUserIdProvider.kt b/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/data/CurrentUserIdProvider.kt new file mode 100644 index 0000000..0a5a6b5 --- /dev/null +++ b/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/data/CurrentUserIdProvider.kt @@ -0,0 +1,11 @@ +package com.interlinedlist.android.feature.lists.data + +/** + * Supplies the signed-in user's id so the views UI can tell which saved views + * belong to the current user — only their own may be renamed or deleted, and + * somebody else's shared view is forked instead. Abstracted from `SessionStore` + * (which is Android-backed) so the ViewModel stays unit-testable on the JVM. + */ +fun interface CurrentUserIdProvider { + fun currentUserId(): String? +} diff --git a/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/data/DefaultListsRepository.kt b/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/data/DefaultListsRepository.kt index e6d5568..686aed4 100644 --- a/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/data/DefaultListsRepository.kt +++ b/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/data/DefaultListsRepository.kt @@ -2,6 +2,7 @@ package com.interlinedlist.android.feature.lists.data 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.common.result.map import com.interlinedlist.android.core.network.error.safeApiCall import com.interlinedlist.android.feature.lists.data.local.ListDao @@ -11,12 +12,15 @@ import com.interlinedlist.android.feature.lists.data.remote.dto.CreateConnection import com.interlinedlist.android.feature.lists.data.remote.dto.CreateFolderRequest import com.interlinedlist.android.feature.lists.data.remote.dto.CreateListRequest import com.interlinedlist.android.feature.lists.data.remote.dto.CreateShareLinkRequest +import com.interlinedlist.android.feature.lists.data.remote.dto.CreateViewRequest import com.interlinedlist.android.feature.lists.data.remote.dto.ListDto +import com.interlinedlist.android.feature.lists.data.remote.dto.ListViewEnvelope import com.interlinedlist.android.feature.lists.data.remote.dto.RowDto import com.interlinedlist.android.feature.lists.data.remote.dto.RowWriteRequest import com.interlinedlist.android.feature.lists.data.remote.dto.UpdateFolderRequest import com.interlinedlist.android.feature.lists.data.remote.dto.UpdateListRequest import com.interlinedlist.android.feature.lists.data.remote.dto.UpdateSchemaRequest +import com.interlinedlist.android.feature.lists.data.remote.dto.UpdateViewRequest import com.interlinedlist.android.feature.lists.data.remote.dto.UpdateWatcherRoleRequest import com.interlinedlist.android.feature.lists.domain.Contributor import com.interlinedlist.android.feature.lists.domain.ListConnection @@ -26,6 +30,9 @@ import com.interlinedlist.android.feature.lists.domain.ListRow import com.interlinedlist.android.feature.lists.domain.ListSchema import com.interlinedlist.android.feature.lists.domain.ListSource import com.interlinedlist.android.feature.lists.domain.ListSummary +import com.interlinedlist.android.feature.lists.domain.ListView +import com.interlinedlist.android.feature.lists.domain.ListViewConfig +import com.interlinedlist.android.feature.lists.domain.ListViewScope import com.interlinedlist.android.feature.lists.domain.Paged import com.interlinedlist.android.feature.lists.domain.RefreshResult import com.interlinedlist.android.feature.lists.domain.ShareLink @@ -420,6 +427,79 @@ class DefaultListsRepository @Inject constructor( safeApiCall(json) { api.deleteConnection(id) }.map { } } + override suspend fun getViews(listId: String): ApiResult> = + withContext(dispatchers.io) { + safeApiCall(json) { api.getViews(listId) } + .map { response -> response.items.map { ListViewMapper.fromDto(it, listId) } } + } + + override suspend fun createView( + listId: String, + name: String, + scope: ListViewScope?, + config: ListViewConfig?, + isDefault: Boolean, + ): ApiResult { + val trimmedName = name.trim() + if (trimmedName.isEmpty()) return ApiResult.Failure(AppError.Unknown(MISSING_VIEW_NAME)) + // The API 400s on a missing/unknown scope, so don't spend a request on one. + val resolvedScope = scope ?: return ApiResult.Failure(AppError.Unknown(MISSING_VIEW_SCOPE)) + return withContext(dispatchers.io) { + val body = CreateViewRequest( + name = trimmedName, + scope = resolvedScope.apiValue, + config = config?.raw, + isDefault = isDefault.takeIf { it }, + ) + safeApiCall(json) { api.createView(listId, body) }.requireView(listId) + } + } + + override suspend fun updateView( + listId: String, + viewId: String, + name: String?, + config: ListViewConfig?, + isDefault: Boolean?, + ): ApiResult { + val trimmedName = name?.trim() + if (trimmedName != null && trimmedName.isEmpty()) { + return ApiResult.Failure(AppError.Unknown(MISSING_VIEW_NAME)) + } + return withContext(dispatchers.io) { + val body = UpdateViewRequest( + name = trimmedName, + config = config?.raw, + isDefault = isDefault, + ) + safeApiCall(json) { api.updateView(listId, viewId, body) }.requireView(listId) + } + } + + override suspend fun forkView(listId: String, viewId: String): ApiResult = + withContext(dispatchers.io) { + safeApiCall(json) { api.forkView(listId, viewId) }.requireView(listId) + } + + override suspend fun deleteView(listId: String, viewId: String): ApiResult = + withContext(dispatchers.io) { + safeApiCall(json) { api.deleteView(listId, viewId) }.map { } + } + + /** + * Unwraps a create/update/fork response into the server's own copy of the + * view. That copy is authoritative: unrecognised `config` values are dropped + * server-side without complaint, so callers must render what came back + * rather than what they sent. + */ + private fun ApiResult.requireView(listId: String): ApiResult = + when (this) { + is ApiResult.Success -> data.viewOrSelf + ?.let { ApiResult.Success(ListViewMapper.fromDto(it, listId)) } + ?: ApiResult.Failure(AppError.Unknown(VIEW_NOT_RETURNED)) + is ApiResult.Failure -> this + } + override suspend fun getShareLinks(listId: String): ApiResult> = withContext(dispatchers.io) { safeApiCall(json) { api.getShareLinks(listId) } @@ -474,6 +554,10 @@ class DefaultListsRepository @Inject constructor( private companion object { /** Safety net for a breadcrumb walk: deep nesting is not worth the requests. */ const val MAX_PARENT_CHAIN = 10 + + const val MISSING_VIEW_NAME = "A view needs a name." + const val MISSING_VIEW_SCOPE = "Choose whether the view is shared or personal." + const val VIEW_NOT_RETURNED = "The view was saved but the server did not return it." } } diff --git a/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/data/ListViewMapper.kt b/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/data/ListViewMapper.kt new file mode 100644 index 0000000..0d6c421 --- /dev/null +++ b/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/data/ListViewMapper.kt @@ -0,0 +1,26 @@ +package com.interlinedlist.android.feature.lists.data + +import com.interlinedlist.android.feature.lists.data.remote.dto.ListViewDto +import com.interlinedlist.android.feature.lists.domain.ListView +import com.interlinedlist.android.feature.lists.domain.ListViewConfig +import com.interlinedlist.android.feature.lists.domain.ListViewScope + +/** DTO → domain mapping for saved views. */ +object ListViewMapper { + + /** + * Maps a server view, keeping its `config` verbatim. An unrecognised `scope` + * is treated as [ListViewScope.SHARED]: the conservative reading, since it + * stops the UI offering destructive actions on a view that may not be ours. + */ + fun fromDto(dto: ListViewDto, listId: String): ListView = ListView( + id = dto.id, + listId = dto.listId ?: listId, + userId = dto.userId, + name = dto.name, + scope = ListViewScope.fromApi(dto.scope) ?: ListViewScope.SHARED, + config = ListViewConfig.fromJson(dto.config), + isDefault = dto.isDefault, + position = dto.position, + ) +} diff --git a/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/data/ListsRepository.kt b/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/data/ListsRepository.kt index a3a2579..89678b6 100644 --- a/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/data/ListsRepository.kt +++ b/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/data/ListsRepository.kt @@ -9,6 +9,9 @@ import com.interlinedlist.android.feature.lists.domain.ListRow import com.interlinedlist.android.feature.lists.domain.ListSchema import com.interlinedlist.android.feature.lists.domain.ListSource import com.interlinedlist.android.feature.lists.domain.ListSummary +import com.interlinedlist.android.feature.lists.domain.ListView +import com.interlinedlist.android.feature.lists.domain.ListViewConfig +import com.interlinedlist.android.feature.lists.domain.ListViewScope import com.interlinedlist.android.feature.lists.domain.Paged import com.interlinedlist.android.feature.lists.domain.RefreshResult import com.interlinedlist.android.feature.lists.domain.ShareLink @@ -178,6 +181,54 @@ interface ListsRepository { /** Removes a connection between lists. */ suspend fun deleteConnection(id: String): ApiResult + // --- Saved views ------------------------------------------------------- + + /** + * Saved views for a list: every shared view plus the caller's own personal + * ones, in the order the server returns them. + */ + suspend fun getViews(listId: String): ApiResult> + + /** + * Creates a saved view. [scope] is nullable because the UI can ask before the + * user has chosen one; a missing or unrecognised scope fails locally without + * spending a request, since the server rejects it with a 400 anyway. + * + * The returned view is the server's own copy: it silently drops [config] + * values it does not recognise, so its echo — not the config sent — is what + * callers must render. + */ + suspend fun createView( + listId: String, + name: String, + scope: ListViewScope?, + config: ListViewConfig? = null, + isDefault: Boolean = false, + ): ApiResult + + /** + * Renames / re-configures a view or makes it the default. Only the supplied + * fields change, and the server's echo is returned for the same reason as + * [createView]. + */ + suspend fun updateView( + listId: String, + viewId: String, + name: String? = null, + config: ListViewConfig? = null, + isDefault: Boolean? = null, + ): ApiResult + + /** + * Forks a view into a personal copy named `" (copy)"` — the escape + * hatch when somebody else's shared view does not suit. The original is + * untouched. + */ + suspend fun forkView(listId: String, viewId: String): ApiResult + + /** Deletes a saved view. The server rejects views the caller does not own. */ + suspend fun deleteView(listId: String, viewId: String): ApiResult + // --- Sharing ----------------------------------------------------------- /** Existing public share links for a list. */ diff --git a/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/data/remote/ListsApi.kt b/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/data/remote/ListsApi.kt index 44aa00d..4efba67 100644 --- a/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/data/remote/ListsApi.kt +++ b/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/data/remote/ListsApi.kt @@ -7,10 +7,14 @@ import com.interlinedlist.android.feature.lists.data.remote.dto.ContributorsResp import com.interlinedlist.android.feature.lists.data.remote.dto.CreateConnectionRequest import com.interlinedlist.android.feature.lists.data.remote.dto.CreateFolderRequest import com.interlinedlist.android.feature.lists.data.remote.dto.CreateListRequest +import com.interlinedlist.android.feature.lists.data.remote.dto.CreateViewRequest +import com.interlinedlist.android.feature.lists.data.remote.dto.DeleteViewResponse import com.interlinedlist.android.feature.lists.data.remote.dto.FolderDto import com.interlinedlist.android.feature.lists.data.remote.dto.FolderEnvelope import com.interlinedlist.android.feature.lists.data.remote.dto.FoldersResponse import com.interlinedlist.android.feature.lists.data.remote.dto.ListEnvelope +import com.interlinedlist.android.feature.lists.data.remote.dto.ListViewEnvelope +import com.interlinedlist.android.feature.lists.data.remote.dto.ListViewsResponse import com.interlinedlist.android.feature.lists.data.remote.dto.ListsResponse import com.interlinedlist.android.feature.lists.data.remote.dto.RefreshResultDto import com.interlinedlist.android.feature.lists.data.remote.dto.RowEnvelope @@ -24,6 +28,7 @@ import com.interlinedlist.android.feature.lists.data.remote.dto.SharedListRespon import com.interlinedlist.android.feature.lists.data.remote.dto.UpdateFolderRequest import com.interlinedlist.android.feature.lists.data.remote.dto.UpdateListRequest import com.interlinedlist.android.feature.lists.data.remote.dto.UpdateSchemaRequest +import com.interlinedlist.android.feature.lists.data.remote.dto.UpdateViewRequest import com.interlinedlist.android.feature.lists.data.remote.dto.UpdateWatcherRoleRequest import com.interlinedlist.android.feature.lists.data.remote.dto.WatchingResponse import com.interlinedlist.android.feature.lists.data.remote.dto.WatcherUsersResponse @@ -173,6 +178,43 @@ interface ListsApi { @Path("rowId") rowId: String, ) + // --- Saved views ------------------------------------------------------- + + /** Every shared view on the list plus the caller's own personal views. */ + @GET("api/lists/{id}/views") + suspend fun getViews(@Path("id") id: String): ListViewsResponse + + /** Creates a view; `name` and `scope` are both required by the server. */ + @POST("api/lists/{id}/views") + suspend fun createView( + @Path("id") id: String, + @Body body: CreateViewRequest, + ): ListViewEnvelope + + /** Updates a view (rename / re-configure / set as default). */ + @PUT("api/lists/{id}/views/{viewId}") + suspend fun updateView( + @Path("id") id: String, + @Path("viewId") viewId: String, + @Body body: UpdateViewRequest, + ): ListViewEnvelope + + /** + * Forks a view into a personal copy named `" (copy)"` — the escape + * hatch when somebody else's shared view does not suit. Takes no body. + */ + @POST("api/lists/{id}/views/{viewId}") + suspend fun forkView( + @Path("id") id: String, + @Path("viewId") viewId: String, + ): ListViewEnvelope + + @DELETE("api/lists/{id}/views/{viewId}") + suspend fun deleteView( + @Path("id") id: String, + @Path("viewId") viewId: String, + ): DeleteViewResponse + @GET("api/folders") suspend fun getFolders(): FoldersResponse diff --git a/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/data/remote/dto/ViewDtos.kt b/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/data/remote/dto/ViewDtos.kt new file mode 100644 index 0000000..fc288f8 --- /dev/null +++ b/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/data/remote/dto/ViewDtos.kt @@ -0,0 +1,93 @@ +package com.interlinedlist.android.feature.lists.data.remote.dto + +import kotlinx.serialization.Serializable +import kotlinx.serialization.json.JsonObject + +/** + * Wire models for the saved-views endpoints (`/api/lists/{id}/views`), captured + * against the live API: a view is + * `{ id, listId, userId, name, scope, config, isDefault, position }`. + * + * `config` is kept as a raw [JsonObject] rather than a typed struct so keys this + * client does not model survive a round-trip; the server already drops what it + * does not recognise and the client must not compound that. + */ +@Serializable +data class ListViewDto( + val id: String = "", + val listId: String? = null, + val userId: String? = null, + val name: String = "", + val scope: String? = null, + val config: JsonObject? = null, + val isDefault: Boolean = false, + val position: Int = 0, +) + +/** Envelope for `GET /api/lists/{id}/views` — verified live shape uses `views`. */ +@Serializable +data class ListViewsResponse( + val views: List? = null, + val data: List? = null, +) { + val items: List get() = views ?: data ?: emptyList() +} + +/** + * Envelope for create / update / fork, all of which return `{ "view": { … } }`. + * `data` and a bare object are tolerated for forward-compatibility. + */ +@Serializable +data class ListViewEnvelope( + val view: ListViewDto? = null, + val data: ListViewDto? = null, + val id: String? = null, + val listId: String? = null, + val userId: String? = null, + val name: String? = null, + val scope: String? = null, + val config: JsonObject? = null, + val isDefault: Boolean = false, + val position: Int = 0, +) { + val viewOrSelf: ListViewDto? + get() = view ?: data ?: id?.let { + ListViewDto( + id = it, + listId = listId, + userId = userId, + name = name.orEmpty(), + scope = scope, + config = config, + isDefault = isDefault, + position = position, + ) + } +} + +/** + * Body for `POST /api/lists/{id}/views`. `name` and `scope` are both required — + * the server 400s with `scope must be "personal" or "shared"` otherwise — so the + * repository validates before sending. Null optionals are dropped by the shared Json. + */ +@Serializable +data class CreateViewRequest( + val name: String, + val scope: String, + val config: JsonObject? = null, + val isDefault: Boolean? = null, +) + +/** Body for `PUT /api/lists/{id}/views/{viewId}` — only the supplied fields change. */ +@Serializable +data class UpdateViewRequest( + val name: String? = null, + val config: JsonObject? = null, + val isDefault: Boolean? = null, +) + +/** Response for `DELETE /api/lists/{id}/views/{viewId}` — `{ "message": "View deleted" }`. */ +@Serializable +data class DeleteViewResponse( + val message: String? = null, +) diff --git a/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/di/ListsModule.kt b/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/di/ListsModule.kt index 734bf3b..0fa1b69 100644 --- a/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/di/ListsModule.kt +++ b/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/di/ListsModule.kt @@ -2,6 +2,8 @@ package com.interlinedlist.android.feature.lists.di import android.content.Context import androidx.room.Room +import com.interlinedlist.android.core.datastore.SessionStore +import com.interlinedlist.android.feature.lists.data.CurrentUserIdProvider import com.interlinedlist.android.feature.lists.data.DefaultListsRepository import com.interlinedlist.android.feature.lists.data.ListsRepository import com.interlinedlist.android.feature.lists.data.local.ListDao @@ -50,4 +52,10 @@ object ListsDataModule { @Provides fun provideListDao(db: ListsDatabase): ListDao = db.listDao() + + /** Adapts the Android-backed [SessionStore] to the module's id contract. */ + @Provides + @Singleton + fun provideCurrentUserIdProvider(sessionStore: SessionStore): CurrentUserIdProvider = + CurrentUserIdProvider { sessionStore.userId } } diff --git a/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/domain/ListView.kt b/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/domain/ListView.kt new file mode 100644 index 0000000..9478856 --- /dev/null +++ b/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/domain/ListView.kt @@ -0,0 +1,140 @@ +package com.interlinedlist.android.feature.lists.domain + +import kotlinx.serialization.json.JsonArray +import kotlinx.serialization.json.JsonElement +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.JsonPrimitive + +/** + * A saved view on a list: a named, reusable way of looking at the same rows. + * + * `GET /api/lists/{id}/views` returns every [ListViewScope.SHARED] view on the + * list plus the current user's own [ListViewScope.PERSONAL] views, so a shared + * view may well belong to somebody else — see [isOwnedBy], which gates renaming + * and deleting. When someone else's shared view does not suit, it can be forked + * into a personal copy instead. + */ +data class ListView( + val id: String, + val listId: String, + val userId: String?, + val name: String, + val scope: ListViewScope, + val config: ListViewConfig, + val isDefault: Boolean, + val position: Int, +) { + val isShared: Boolean get() = scope == ListViewScope.SHARED + + /** + * Whether [currentUserId] may rename or delete this view. When both ids are + * known it is a straight comparison; with an unknown id we fall back to the + * scope, because the endpoint only ever returns the caller's own personal + * views — a shared view of unknown ownership is treated as somebody else's. + */ + fun isOwnedBy(currentUserId: String?): Boolean = when { + userId != null && currentUserId != null -> userId == currentUserId + else -> scope == ListViewScope.PERSONAL + } +} + +/** + * Who a saved view belongs to. The API accepts exactly `"personal"` or + * `"shared"` and rejects anything else with a 400, so [fromApi] returns `null` + * for an unrecognised value rather than guessing — callers validate before they + * spend a request. + */ +enum class ListViewScope(val apiValue: String, val label: String) { + PERSONAL("personal", "Personal"), + SHARED("shared", "Shared"), + ; + + companion object { + fun fromApi(raw: String?): ListViewScope? = + entries.firstOrNull { it.apiValue == raw?.trim()?.lowercase() } + } +} + +/** + * A saved view's `config` blob. + * + * The server silently drops `config` values it does not recognise instead of + * rejecting them, so what it stores is authoritative and every write must be + * reconciled against the response. The whole object is kept in [raw] so keys + * this client does not model survive a round-trip untouched — the client must + * not compound the server's own lossiness — while [mode], [density] and + * [filters] project the parts we do understand. + */ +data class ListViewConfig(val raw: JsonObject) { + + /** The stored display mode, falling back to [ListViewMode.RECORDS]. */ + val mode: ListViewMode get() = ListViewMode.fromApi(string(KEY_MODE)) ?: ListViewMode.RECORDS + + /** The raw stored mode, which may be a value this client does not model. */ + val storedMode: String? get() = string(KEY_MODE) + + val density: ListViewDensity + get() = ListViewDensity.fromApi(string(KEY_DENSITY)) ?: ListViewDensity.COMFORTABLE + + /** Saved filters, passed through verbatim — their shape is the server's business. */ + val filters: JsonArray get() = raw[KEY_FILTERS] as? JsonArray ?: EMPTY_FILTERS + + /** Copies the config with one key replaced, leaving every other key intact. */ + fun with(key: String, value: JsonElement): ListViewConfig = + ListViewConfig(JsonObject(raw + (key to value))) + + private fun string(key: String): String? = + (raw[key] as? JsonPrimitive)?.takeIf { it.isString }?.content + + companion object { + const val KEY_MODE = "mode" + const val KEY_DENSITY = "density" + const val KEY_FILTERS = "filters" + + private val EMPTY_FILTERS = JsonArray(emptyList()) + + /** The config the server applies when none is supplied (verified live). */ + val DEFAULT = ListViewConfig( + JsonObject( + mapOf( + KEY_MODE to JsonPrimitive(ListViewMode.RECORDS.apiValue), + KEY_DENSITY to JsonPrimitive(ListViewDensity.COMFORTABLE.apiValue), + KEY_FILTERS to EMPTY_FILTERS, + ), + ), + ) + + /** Parses a server config, defaulting when the view carries none. */ + fun fromJson(raw: JsonObject?): ListViewConfig = raw?.let(::ListViewConfig) ?: DEFAULT + } +} + +/** + * How a view renders its rows. + * + * Only `records` exists: probing the live API showed every other candidate + * (`cards`, `grid`, `table`, `erd`, …) silently stored as `records`. Unknown + * stored modes still survive in [ListViewConfig.raw] and are readable through + * [ListViewConfig.storedMode]. + */ +enum class ListViewMode(val apiValue: String, val label: String) { + RECORDS("records", "Records"), + ; + + companion object { + fun fromApi(raw: String?): ListViewMode? = + entries.firstOrNull { it.apiValue == raw?.trim()?.lowercase() } + } +} + +/** Row spacing a view asks for. The server accepts these two values only. */ +enum class ListViewDensity(val apiValue: String, val label: String) { + COMFORTABLE("comfortable", "Comfortable"), + COMPACT("compact", "Compact"), + ; + + companion object { + fun fromApi(raw: String?): ListViewDensity? = + entries.firstOrNull { it.apiValue == raw?.trim()?.lowercase() } + } +} diff --git a/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/ui/detail/ListDetailScreen.kt b/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/ui/detail/ListDetailScreen.kt index ca52a36..4e28bd8 100644 --- a/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/ui/detail/ListDetailScreen.kt +++ b/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/ui/detail/ListDetailScreen.kt @@ -58,6 +58,7 @@ import com.interlinedlist.android.feature.lists.domain.ListRow import com.interlinedlist.android.feature.lists.domain.ListSchema import com.interlinedlist.android.feature.lists.domain.ListSummary import com.interlinedlist.android.feature.lists.domain.SchemaField +import com.interlinedlist.android.feature.lists.ui.views.ListViewSwitcher /** Stable test tags for the list detail screen. */ object ListDetailTestTags { @@ -131,6 +132,9 @@ fun ListDetailRoute( onOpenList = onOpenList, onNewChildList = { viewModel.createChildList(onOpenList) }, snackbarHostState = snackbarHostState, + // Saved views have their own ViewModel on the same nav entry, so the + // detail screen stays unaware of them beyond giving them a slot. + viewSwitcher = { ListViewSwitcher() }, modifier = modifier, ) @@ -179,7 +183,12 @@ private sealed interface EditorTarget { data class Existing(val row: ListRow) : EditorTarget } -/** Stateless list detail — schema-driven table with loading / empty / error states. */ +/** + * Stateless list detail — schema-driven table with loading / empty / error states. + * + * [viewSwitcher] is a slot for the saved-view switcher, which owns its own state; + * keeping it a slot means this screen (and its tests) stay independent of it. + */ @OptIn(ExperimentalMaterial3Api::class) @Composable fun ListDetailScreen( @@ -198,6 +207,7 @@ fun ListDetailScreen( onOpenList: (String) -> Unit = {}, onNewChildList: () -> Unit = {}, snackbarHostState: SnackbarHostState = remember { SnackbarHostState() }, + viewSwitcher: @Composable () -> Unit = {}, ) { var menuOpen by remember { mutableStateOf(false) } Scaffold( @@ -289,6 +299,7 @@ fun ListDetailScreen( else -> Column(Modifier.padding(padding)) { Breadcrumb(ancestors = state.breadcrumb, onOpenList = onOpenList) + viewSwitcher() if (!state.summary?.description.isNullOrBlank()) { Text( text = state.summary!!.description!!, diff --git a/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/ui/views/ListViewSwitcher.kt b/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/ui/views/ListViewSwitcher.kt new file mode 100644 index 0000000..5e051be --- /dev/null +++ b/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/ui/views/ListViewSwitcher.kt @@ -0,0 +1,526 @@ +package com.interlinedlist.android.feature.lists.ui.views + +import androidx.compose.foundation.clickable +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.width +import androidx.compose.foundation.rememberScrollState +import androidx.compose.foundation.verticalScroll +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.filled.ArrowDropDown +import androidx.compose.material.icons.filled.MoreVert +import androidx.compose.material.icons.filled.Star +import androidx.compose.material3.AlertDialog +import androidx.compose.material3.AssistChip +import androidx.compose.material3.AssistChipDefaults +import androidx.compose.material3.Button +import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.material3.DropdownMenu +import androidx.compose.material3.DropdownMenuItem +import androidx.compose.material3.ExperimentalMaterial3Api +import androidx.compose.material3.FilterChip +import androidx.compose.material3.Icon +import androidx.compose.material3.IconButton +import androidx.compose.material3.ListItem +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.ModalBottomSheet +import androidx.compose.material3.OutlinedTextField +import androidx.compose.material3.Text +import androidx.compose.material3.TextButton +import androidx.compose.material3.rememberModalBottomSheetState +import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.testTag +import androidx.compose.ui.text.style.TextOverflow +import androidx.compose.ui.tooling.preview.Preview +import androidx.compose.ui.unit.dp +import androidx.hilt.navigation.compose.hiltViewModel +import androidx.lifecycle.compose.collectAsStateWithLifecycle +import com.interlinedlist.android.core.designsystem.theme.InterlinedListTheme +import com.interlinedlist.android.feature.lists.domain.ListView +import com.interlinedlist.android.feature.lists.domain.ListViewConfig +import com.interlinedlist.android.feature.lists.domain.ListViewScope + +/** Stable test tags for the saved-view switcher. */ +object ListViewSwitcherTestTags { + const val BAR = "listViewSwitcherBar" + const val SHEET = "listViewSwitcherSheet" + const val EMPTY = "listViewSwitcherEmpty" + const val ERROR = "listViewSwitcherError" + const val CREATE_NAME = "listViewCreateName" + const val CREATE_SUBMIT = "listViewCreateSubmit" + const val RENAME_DIALOG = "listViewRenameDialog" + const val RENAME_FIELD = "listViewRenameField" + const val RENAME_CONFIRM = "listViewRenameConfirm" + fun scope(scope: ListViewScope) = "listViewScope_${scope.apiValue}" + fun view(id: String) = "listView_$id" + fun overflow(id: String) = "listViewOverflow_$id" + fun setDefault(id: String) = "listViewSetDefault_$id" + fun fork(id: String) = "listViewFork_$id" + fun rename(id: String) = "listViewRename_$id" + fun delete(id: String) = "listViewDelete_$id" +} + +/** + * Hilt-wired saved-view switcher for the list detail screen: a bar showing the + * view in use, and a sheet to switch between the list's shared views and the + * user's personal ones, create, rename, delete, re-default or fork them. + * + * It reads the same `listId` nav argument as the detail screen, so it drops into + * that route without any extra wiring. + */ +@Composable +fun ListViewSwitcher( + modifier: Modifier = Modifier, + viewModel: ListViewsViewModel = hiltViewModel(), +) { + val state by viewModel.uiState.collectAsStateWithLifecycle() + var renaming by remember { mutableStateOf(null) } + + ListViewSwitcherBar( + state = state, + onOpen = viewModel::openSwitcher, + modifier = modifier, + ) + + if (state.isSwitcherOpen) { + ListViewSwitcherSheet( + state = state, + onDismiss = viewModel::closeSwitcher, + onSelect = { viewModel.selectView(it.id) }, + onSetDefault = viewModel::setDefault, + onFork = viewModel::forkView, + onRename = { renaming = it }, + onDelete = viewModel::deleteView, + onCreate = { name, scope -> viewModel.createView(name, scope) }, + ) + } + + renaming?.let { view -> + RenameViewDialog( + view = view, + onDismiss = { renaming = null }, + onConfirm = { name -> + viewModel.renameView(view, name) + renaming = null + }, + ) + } +} + +/** The always-visible bar: which view is in use, and a way into the switcher. */ +@Composable +fun ListViewSwitcherBar( + state: ListViewsUiState, + onOpen: () -> Unit, + modifier: Modifier = Modifier, +) { + val selected = state.selectedView + Row( + modifier = modifier + .fillMaxWidth() + .clickable(onClick = onOpen) + .padding(horizontal = 16.dp, vertical = 8.dp) + .testTag(ListViewSwitcherTestTags.BAR), + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.spacedBy(8.dp), + ) { + Column(Modifier.weight(1f)) { + Text( + text = selected?.name?.ifBlank { UNTITLED_VIEW } ?: "All records", + style = MaterialTheme.typography.titleSmall, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + ) + Text( + text = selected?.let(::viewSummary) ?: "No saved view", + style = MaterialTheme.typography.labelSmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + ) + } + if (selected?.isDefault == true) { + DefaultBadge() + } + Icon(Icons.Default.ArrowDropDown, contentDescription = "Switch view") + } +} + +/** The switcher itself, as a modal sheet over the list. */ +@OptIn(ExperimentalMaterial3Api::class) +@Composable +fun ListViewSwitcherSheet( + state: ListViewsUiState, + onDismiss: () -> Unit, + onSelect: (ListView) -> Unit, + onSetDefault: (ListView) -> Unit, + onFork: (ListView) -> Unit, + onRename: (ListView) -> Unit, + onDelete: (ListView) -> Unit, + onCreate: (String, ListViewScope) -> Unit, + modifier: Modifier = Modifier, +) { + val sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true) + ModalBottomSheet( + onDismissRequest = onDismiss, + sheetState = sheetState, + modifier = modifier.testTag(ListViewSwitcherTestTags.SHEET), + ) { + ListViewSwitcherContent( + state = state, + onSelect = onSelect, + onSetDefault = onSetDefault, + onFork = onFork, + onRename = onRename, + onDelete = onDelete, + onCreate = onCreate, + ) + } +} + +/** Stateless switcher body — shared views, personal views, and a create form. */ +@Composable +fun ListViewSwitcherContent( + state: ListViewsUiState, + onSelect: (ListView) -> Unit, + onSetDefault: (ListView) -> Unit, + onFork: (ListView) -> Unit, + onRename: (ListView) -> Unit, + onDelete: (ListView) -> Unit, + onCreate: (String, ListViewScope) -> Unit, + modifier: Modifier = Modifier, +) { + Column( + modifier = modifier + .fillMaxWidth() + .verticalScroll(rememberScrollState()) + .padding(horizontal = 20.dp) + .padding(bottom = 24.dp), + ) { + Text("Views", style = MaterialTheme.typography.titleLarge) + Spacer(Modifier.height(4.dp)) + Text( + text = "Shared views are on the list for everyone. Personal views are only yours.", + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + + state.errorMessage?.let { message -> + Spacer(Modifier.height(12.dp)) + Text( + text = message, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.error, + modifier = Modifier.testTag(ListViewSwitcherTestTags.ERROR), + ) + } + + if (state.isLoading) { + Spacer(Modifier.height(16.dp)) + CircularProgressIndicator(Modifier.height(24.dp).width(24.dp)) + } + + if (state.isEmpty) { + Spacer(Modifier.height(16.dp)) + Text( + text = "No saved views yet. Create one below to keep a way of looking at this list.", + style = MaterialTheme.typography.bodyMedium, + modifier = Modifier.testTag(ListViewSwitcherTestTags.EMPTY), + ) + } + + ViewSection( + title = "Shared", + views = state.sharedViews, + state = state, + onSelect = onSelect, + onSetDefault = onSetDefault, + onFork = onFork, + onRename = onRename, + onDelete = onDelete, + ) + ViewSection( + title = "Personal", + views = state.personalViews, + state = state, + onSelect = onSelect, + onSetDefault = onSetDefault, + onFork = onFork, + onRename = onRename, + onDelete = onDelete, + ) + + Spacer(Modifier.height(20.dp)) + CreateViewForm(isSaving = state.isSaving, onCreate = onCreate) + } +} + +@Composable +private fun ViewSection( + title: String, + views: List, + state: ListViewsUiState, + onSelect: (ListView) -> Unit, + onSetDefault: (ListView) -> Unit, + onFork: (ListView) -> Unit, + onRename: (ListView) -> Unit, + onDelete: (ListView) -> Unit, +) { + if (views.isEmpty()) return + Spacer(Modifier.height(16.dp)) + Text( + text = title, + style = MaterialTheme.typography.labelLarge, + color = MaterialTheme.colorScheme.primary, + ) + views.forEach { view -> + ViewRow( + view = view, + isSelected = view.id == state.selectedViewId, + canModify = state.canModify(view), + canFork = state.canFork(view), + onSelect = { onSelect(view) }, + onSetDefault = { onSetDefault(view) }, + onFork = { onFork(view) }, + onRename = { onRename(view) }, + onDelete = { onDelete(view) }, + ) + } +} + +@Composable +private fun ViewRow( + view: ListView, + isSelected: Boolean, + canModify: Boolean, + canFork: Boolean, + onSelect: () -> Unit, + onSetDefault: () -> Unit, + onFork: () -> Unit, + onRename: () -> Unit, + onDelete: () -> Unit, +) { + var menuOpen by remember { mutableStateOf(false) } + ListItem( + modifier = Modifier + .clickable(onClick = onSelect) + .testTag(ListViewSwitcherTestTags.view(view.id)), + headlineContent = { + Row( + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.spacedBy(8.dp), + ) { + Text( + text = view.name.ifBlank { UNTITLED_VIEW }, + style = if (isSelected) { + MaterialTheme.typography.titleSmall + } else { + MaterialTheme.typography.bodyLarge + }, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + ) + if (view.isDefault) DefaultBadge() + } + }, + supportingContent = { + Text( + text = viewSummary(view) + if (canModify) "" else " · read-only", + style = MaterialTheme.typography.labelSmall, + ) + }, + trailingContent = { + Box { + IconButton( + onClick = { menuOpen = true }, + modifier = Modifier.testTag(ListViewSwitcherTestTags.overflow(view.id)), + ) { Icon(Icons.Default.MoreVert, contentDescription = "Actions for ${view.name}") } + DropdownMenu(expanded = menuOpen, onDismissRequest = { menuOpen = false }) { + if (!view.isDefault) { + DropdownMenuItem( + text = { Text("Set as default") }, + onClick = { menuOpen = false; onSetDefault() }, + modifier = Modifier.testTag(ListViewSwitcherTestTags.setDefault(view.id)), + ) + } + if (canFork) { + DropdownMenuItem( + text = { + Column { + Text("Make a personal copy") + Text( + text = "Copies this view to your own. The shared one is untouched.", + style = MaterialTheme.typography.labelSmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + }, + onClick = { menuOpen = false; onFork() }, + modifier = Modifier.testTag(ListViewSwitcherTestTags.fork(view.id)), + ) + } + // Renaming and deleting belong to whoever created the view. + if (canModify) { + DropdownMenuItem( + text = { Text("Rename") }, + onClick = { menuOpen = false; onRename() }, + modifier = Modifier.testTag(ListViewSwitcherTestTags.rename(view.id)), + ) + DropdownMenuItem( + text = { Text("Delete") }, + onClick = { menuOpen = false; onDelete() }, + modifier = Modifier.testTag(ListViewSwitcherTestTags.delete(view.id)), + ) + } + } + } + }, + ) +} + +/** Name + scope, the two things the API insists on when creating a view. */ +@Composable +private fun CreateViewForm( + isSaving: Boolean, + onCreate: (String, ListViewScope) -> Unit, +) { + var name by remember { mutableStateOf("") } + var scope by remember { mutableStateOf(ListViewScope.PERSONAL) } + Text("New view", style = MaterialTheme.typography.titleSmall) + Spacer(Modifier.height(8.dp)) + OutlinedTextField( + value = name, + onValueChange = { name = it }, + label = { Text("Name") }, + singleLine = true, + modifier = Modifier + .fillMaxWidth() + .testTag(ListViewSwitcherTestTags.CREATE_NAME), + ) + Spacer(Modifier.height(8.dp)) + Row(horizontalArrangement = Arrangement.spacedBy(8.dp)) { + ListViewScope.entries.forEach { option -> + FilterChip( + selected = scope == option, + onClick = { scope = option }, + label = { Text(option.label) }, + modifier = Modifier.testTag(ListViewSwitcherTestTags.scope(option)), + ) + } + } + Spacer(Modifier.height(12.dp)) + Button( + onClick = { + onCreate(name, scope) + name = "" + }, + enabled = name.isNotBlank() && !isSaving, + modifier = Modifier + .fillMaxWidth() + .testTag(ListViewSwitcherTestTags.CREATE_SUBMIT), + ) { Text("Create view") } +} + +@Composable +private fun RenameViewDialog( + view: ListView, + onDismiss: () -> Unit, + onConfirm: (String) -> Unit, +) { + var name by remember(view.id) { mutableStateOf(view.name) } + AlertDialog( + onDismissRequest = onDismiss, + modifier = Modifier.testTag(ListViewSwitcherTestTags.RENAME_DIALOG), + title = { Text("Rename view") }, + text = { + OutlinedTextField( + value = name, + onValueChange = { name = it }, + label = { Text("Name") }, + singleLine = true, + modifier = Modifier.testTag(ListViewSwitcherTestTags.RENAME_FIELD), + ) + }, + confirmButton = { + TextButton( + onClick = { onConfirm(name) }, + enabled = name.isNotBlank(), + modifier = Modifier.testTag(ListViewSwitcherTestTags.RENAME_CONFIRM), + ) { Text("Rename") } + }, + dismissButton = { TextButton(onClick = onDismiss) { Text("Cancel") } }, + ) +} + +@Composable +private fun DefaultBadge() { + AssistChip( + onClick = {}, + enabled = false, + label = { Text("Default") }, + leadingIcon = { Icon(Icons.Default.Star, contentDescription = null) }, + colors = AssistChipDefaults.assistChipColors(), + ) +} + +/** + * How a view is configured, in words. Read from what the server stored — it + * silently drops config values it does not recognise, so this is the truth about + * the view rather than whatever was last sent. + */ +private fun viewSummary(view: ListView): String { + val mode = view.config.storedMode?.replaceFirstChar { it.uppercase() } ?: view.config.mode.label + return "$mode · ${view.config.density.label} · ${view.scope.label}" +} + +private const val UNTITLED_VIEW = "Untitled view" + +@Preview(showBackground = true) +@Composable +private fun ListViewSwitcherContentPreview() { + val shared = ListView( + id = "v1", + listId = "L1", + userId = "someone-else", + name = "Roadmap", + scope = ListViewScope.SHARED, + config = ListViewConfig.DEFAULT, + isDefault = true, + position = 0, + ) + val personal = shared.copy( + id = "v2", + userId = "me", + name = "My cut", + scope = ListViewScope.PERSONAL, + isDefault = false, + ) + InterlinedListTheme { + ListViewSwitcherContent( + state = ListViewsUiState( + views = listOf(shared, personal), + selectedViewId = "v1", + currentUserId = "me", + isLoading = false, + ), + onSelect = {}, + onSetDefault = {}, + onFork = {}, + onRename = {}, + onDelete = {}, + onCreate = { _, _ -> }, + ) + } +} diff --git a/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/ui/views/ListViewsViewModel.kt b/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/ui/views/ListViewsViewModel.kt new file mode 100644 index 0000000..463e918 --- /dev/null +++ b/feature/lists/src/main/kotlin/com/interlinedlist/android/feature/lists/ui/views/ListViewsViewModel.kt @@ -0,0 +1,240 @@ +package com.interlinedlist.android.feature.lists.ui.views + +import androidx.lifecycle.SavedStateHandle +import androidx.lifecycle.ViewModel +import androidx.lifecycle.viewModelScope +import com.interlinedlist.android.core.common.result.ApiResult +import com.interlinedlist.android.feature.lists.data.CurrentUserIdProvider +import com.interlinedlist.android.feature.lists.data.ListsRepository +import com.interlinedlist.android.feature.lists.domain.ListView +import com.interlinedlist.android.feature.lists.domain.ListViewScope +import com.interlinedlist.android.feature.lists.ui.toUserMessage +import dagger.hilt.android.lifecycle.HiltViewModel +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.update +import kotlinx.coroutines.launch +import javax.inject.Inject + +/** The nav argument key the views switcher reads its list id from. */ +const val VIEWS_LIST_ID_ARG = "listId" + +/** UI state for the saved-view switcher on the list detail screen. */ +data class ListViewsUiState( + val views: List = emptyList(), + val selectedViewId: String? = null, + val currentUserId: String? = null, + val isLoading: Boolean = true, + val isSaving: Boolean = false, + val isSwitcherOpen: Boolean = false, + val errorMessage: String? = null, +) { + val selectedView: ListView? get() = views.firstOrNull { it.id == selectedViewId } + val sharedViews: List get() = views.filter { it.isShared } + val personalViews: List get() = views.filterNot { it.isShared } + val isEmpty: Boolean get() = views.isEmpty() && !isLoading + + /** Only the view's owner may rename or delete it; everyone else forks a copy. */ + fun canModify(view: ListView): Boolean = view.isOwnedBy(currentUserId) + + /** Forking is offered on shared views — the escape hatch when one doesn't suit. */ + fun canFork(view: ListView): Boolean = view.isShared +} + +/** + * Drives the saved-view switcher: loads the list's shared views plus the user's + * own personal ones, tracks which is selected (the default wins on first load), + * and creates / renames / deletes / re-defaults / forks them. + * + * Two rules shape every write: + * - the server silently drops `config` values it does not recognise, so the view + * it returns replaces the local one rather than the optimistic copy we sent; + * - renaming and deleting are offered only for views the user owns, and a server + * refusal is surfaced as a message with the view left untouched. + */ +@HiltViewModel +class ListViewsViewModel @Inject constructor( + private val repository: ListsRepository, + private val currentUserIdProvider: CurrentUserIdProvider, + savedStateHandle: SavedStateHandle, +) : ViewModel() { + + private val listId: String = requireNotNull(savedStateHandle[VIEWS_LIST_ID_ARG]) { + "ListViewsViewModel requires a '$VIEWS_LIST_ID_ARG' nav argument" + } + + private val _uiState = MutableStateFlow(ListViewsUiState(currentUserId = currentUserIdProvider.currentUserId())) + val uiState: StateFlow = _uiState.asStateFlow() + + init { + load() + } + + fun load() { + _uiState.update { it.copy(isLoading = true, errorMessage = null) } + viewModelScope.launch { refresh(showLoading = true) } + } + + fun openSwitcher() = _uiState.update { it.copy(isSwitcherOpen = true) } + + fun closeSwitcher() = _uiState.update { it.copy(isSwitcherOpen = false) } + + fun selectView(viewId: String) = _uiState.update { it.copy(selectedViewId = viewId) } + + fun clearError() = _uiState.update { it.copy(errorMessage = null) } + + /** + * Creates a view with the chosen [scope]. A missing scope never reaches the + * network — the API rejects it — and comes back as a message instead. + */ + fun createView(name: String, scope: ListViewScope?, onDone: () -> Unit = {}) { + if (_uiState.value.isSaving) return + _uiState.update { it.copy(isSaving = true, errorMessage = null) } + viewModelScope.launch { + when (val result = repository.createView(listId, name, scope)) { + is ApiResult.Success -> { + // The server's copy is authoritative — it may have dropped config values. + _uiState.update { + it.copy( + views = it.views + result.data, + selectedViewId = result.data.id, + isSaving = false, + ) + } + onDone() + } + is ApiResult.Failure -> fail(result) + } + } + } + + /** Renames a view the user owns; somebody else's shared view is refused locally. */ + fun renameView(view: ListView, name: String) { + if (!requireOwnership(view, RENAME_REFUSED)) return + if (_uiState.value.isSaving) return + _uiState.update { it.copy(isSaving = true, errorMessage = null) } + viewModelScope.launch { + when (val result = repository.updateView(listId, view.id, name = name)) { + is ApiResult.Success -> applyServerView(result.data) + is ApiResult.Failure -> fail(result) + } + } + } + + /** + * Makes a view the default. Whether the server lets a user re-default somebody + * else's shared view is its call, so this is attempted and any refusal shown; + * the list is re-read afterwards so sibling views lose the flag if it moved. + */ + fun setDefault(view: ListView) { + if (_uiState.value.isSaving) return + _uiState.update { it.copy(isSaving = true, errorMessage = null) } + viewModelScope.launch { + when (val result = repository.updateView(listId, view.id, isDefault = true)) { + is ApiResult.Success -> { + applyServerView(result.data) + refresh(showLoading = false) + } + is ApiResult.Failure -> fail(result) + } + } + } + + /** + * Forks a shared view into a personal copy and selects it. The original is + * untouched, which is the whole point: it is the way out when somebody else's + * shared view does not suit. + */ + fun forkView(view: ListView) { + if (_uiState.value.isSaving) return + _uiState.update { it.copy(isSaving = true, errorMessage = null) } + viewModelScope.launch { + when (val result = repository.forkView(listId, view.id)) { + is ApiResult.Success -> _uiState.update { + it.copy( + views = it.views + result.data, + selectedViewId = result.data.id, + isSaving = false, + ) + } + is ApiResult.Failure -> fail(result) + } + } + } + + /** Deletes a view the user owns; somebody else's shared view is refused locally. */ + fun deleteView(view: ListView) { + if (!requireOwnership(view, DELETE_REFUSED)) return + if (_uiState.value.isSaving) return + _uiState.update { it.copy(isSaving = true, errorMessage = null) } + viewModelScope.launch { + when (val result = repository.deleteView(listId, view.id)) { + is ApiResult.Success -> _uiState.update { state -> + // Removed only once the server confirms, so a refusal leaves it visible. + val remaining = state.views.filterNot { it.id == view.id } + state.copy( + views = remaining, + selectedViewId = state.selectedViewId + .takeIf { it != view.id } ?: remaining.preferredSelection(), + isSaving = false, + ) + } + is ApiResult.Failure -> fail(result) + } + } + } + + /** Re-reads the views, keeping the current selection when it still exists. */ + private suspend fun refresh(showLoading: Boolean) { + when (val result = repository.getViews(listId)) { + is ApiResult.Success -> _uiState.update { state -> + val views = result.data + state.copy( + views = views, + selectedViewId = state.selectedViewId?.takeIf { id -> views.any { it.id == id } } + ?: views.preferredSelection(), + currentUserId = currentUserIdProvider.currentUserId(), + isLoading = false, + isSaving = false, + ) + } + is ApiResult.Failure -> _uiState.update { + it.copy( + isLoading = false, + isSaving = false, + // A failed background reconcile shouldn't bury the write's own error. + errorMessage = if (showLoading) result.error.toUserMessage() else it.errorMessage, + ) + } + } + } + + /** Replaces a view with the server's copy of it, which is the authoritative one. */ + private fun applyServerView(view: ListView) = _uiState.update { state -> + state.copy( + views = state.views.map { if (it.id == view.id) view else it }, + isSaving = false, + ) + } + + private fun fail(result: ApiResult.Failure) = _uiState.update { + it.copy(isSaving = false, errorMessage = result.error.toUserMessage()) + } + + /** Refuses an action on a view the user does not own, without spending a request. */ + private fun requireOwnership(view: ListView, message: String): Boolean { + if (_uiState.value.canModify(view)) return true + _uiState.update { it.copy(errorMessage = message) } + return false + } + + private companion object { + const val RENAME_REFUSED = "Only the person who created this shared view can rename it. Make a personal copy instead." + const val DELETE_REFUSED = "Only the person who created this shared view can delete it." + } +} + +/** The default view if there is one, else the first — what a fresh screen shows. */ +private fun List.preferredSelection(): String? = + (firstOrNull { it.isDefault } ?: firstOrNull())?.id diff --git a/feature/lists/src/test/kotlin/com/interlinedlist/android/feature/lists/FakeListsRepository.kt b/feature/lists/src/test/kotlin/com/interlinedlist/android/feature/lists/FakeListsRepository.kt index 29dd5d7..ce115ed 100644 --- a/feature/lists/src/test/kotlin/com/interlinedlist/android/feature/lists/FakeListsRepository.kt +++ b/feature/lists/src/test/kotlin/com/interlinedlist/android/feature/lists/FakeListsRepository.kt @@ -11,6 +11,9 @@ import com.interlinedlist.android.feature.lists.domain.ListRow import com.interlinedlist.android.feature.lists.domain.ListSchema import com.interlinedlist.android.feature.lists.domain.ListSource import com.interlinedlist.android.feature.lists.domain.ListSummary +import com.interlinedlist.android.feature.lists.domain.ListView +import com.interlinedlist.android.feature.lists.domain.ListViewConfig +import com.interlinedlist.android.feature.lists.domain.ListViewScope import com.interlinedlist.android.feature.lists.domain.Paged import com.interlinedlist.android.feature.lists.domain.RefreshResult import com.interlinedlist.android.feature.lists.domain.ShareLink @@ -67,6 +70,13 @@ class FakeListsRepository : ListsRepository { var createConnectionResult: ApiResult? = null var deleteConnectionResult: ApiResult = ApiResult.Success(Unit) + // Saved views. + var viewsResult: ApiResult> = ApiResult.Success(emptyList()) + var createViewResult: ApiResult? = null + var updateViewResult: ApiResult? = null + var forkViewResult: ApiResult? = null + var deleteViewResult: ApiResult = ApiResult.Success(Unit) + // Sharing. var shareLinksResult: ApiResult> = ApiResult.Success(emptyList()) var createShareLinkResult: ApiResult? = null @@ -106,6 +116,15 @@ class FakeListsRepository : ListsRepository { var lastCreatedShareRole: ShareRole? = null var lastRevokedToken: String? = null var lastResolvedToken: String? = null + var viewsCount = 0 + var lastCreatedViewName: String? = null + var lastCreatedViewScope: ListViewScope? = null + var lastUpdatedViewId: String? = null + var lastUpdatedViewName: String? = null + var lastUpdatedViewConfig: ListViewConfig? = null + var lastUpdatedViewIsDefault: Boolean? = null + var lastForkedViewId: String? = null + var lastDeletedViewId: String? = null var lastClaimedToken: String? = null override fun observeLists(): Flow> = cache @@ -289,6 +308,55 @@ class FakeListsRepository : ListsRepository { override suspend fun deleteConnection(id: String): ApiResult = deleteConnectionResult + override suspend fun getViews(listId: String): ApiResult> { + viewsCount++ + return viewsResult + } + + override suspend fun createView( + listId: String, + name: String, + scope: ListViewScope?, + config: ListViewConfig?, + isDefault: Boolean, + ): ApiResult { + lastCreatedViewName = name + lastCreatedViewScope = scope + // Mirrors the real repository: an absent scope never reaches the network. + if (scope == null) return ApiResult.Failure(AppError.Unknown("Choose whether the view is shared or personal.")) + return createViewResult ?: ApiResult.Success( + ListView("v-new", listId, "me", name, scope, ListViewConfig.DEFAULT, isDefault, 0), + ) + } + + override suspend fun updateView( + listId: String, + viewId: String, + name: String?, + config: ListViewConfig?, + isDefault: Boolean?, + ): ApiResult { + lastUpdatedViewId = viewId + lastUpdatedViewName = name + lastUpdatedViewConfig = config + lastUpdatedViewIsDefault = isDefault + return updateViewResult ?: ApiResult.Success( + ListView(viewId, listId, "me", name.orEmpty(), ListViewScope.PERSONAL, ListViewConfig.DEFAULT, isDefault == true, 0), + ) + } + + override suspend fun forkView(listId: String, viewId: String): ApiResult { + lastForkedViewId = viewId + return forkViewResult ?: ApiResult.Success( + ListView("$viewId-copy", listId, "me", "Copy", ListViewScope.PERSONAL, ListViewConfig.DEFAULT, false, 0), + ) + } + + override suspend fun deleteView(listId: String, viewId: String): ApiResult { + lastDeletedViewId = viewId + return deleteViewResult + } + override suspend fun getShareLinks(listId: String): ApiResult> = shareLinksResult override suspend fun createShareLink(listId: String, role: ShareRole): ApiResult { diff --git a/feature/lists/src/test/kotlin/com/interlinedlist/android/feature/lists/data/DefaultListsRepositoryViewsTest.kt b/feature/lists/src/test/kotlin/com/interlinedlist/android/feature/lists/data/DefaultListsRepositoryViewsTest.kt new file mode 100644 index 0000000..9a9ddcd --- /dev/null +++ b/feature/lists/src/test/kotlin/com/interlinedlist/android/feature/lists/data/DefaultListsRepositoryViewsTest.kt @@ -0,0 +1,326 @@ +package com.interlinedlist.android.feature.lists.data + +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.feature.lists.data.local.CachedListEntity +import com.interlinedlist.android.feature.lists.data.local.ListDao +import com.interlinedlist.android.feature.lists.data.remote.ListsApi +import com.interlinedlist.android.feature.lists.domain.ListViewConfig +import com.interlinedlist.android.feature.lists.domain.ListViewDensity +import com.interlinedlist.android.feature.lists.domain.ListViewMode +import com.interlinedlist.android.feature.lists.domain.ListViewScope +import com.jakewharton.retrofit2.converter.kotlinx.serialization.asConverterFactory +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.runTest +import kotlinx.serialization.json.Json +import kotlinx.serialization.json.JsonPrimitive +import kotlinx.serialization.json.buildJsonObject +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 + +/** + * MockWebServer coverage for the saved-views endpoints (`/api/lists/{id}/views`): + * list/create/update/delete/fork round-trips against the real paths and bodies, + * local validation of `scope` before a request is spent, and the rule that the + * server's copy of a view — not the one we sent — is what comes back. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class DefaultListsRepositoryViewsTest { + + private lateinit var server: MockWebServer + private lateinit var api: ListsApi + private lateinit var repository: DefaultListsRepository + + // Mirrors the app's shared Json (explicit nulls off, coerce defaults on). + private val json = Json { ignoreUnknownKeys = true; explicitNulls = false; coerceInputValues = true } + private val dispatcher = StandardTestDispatcher() + + private val testDispatchers = object : DispatcherProvider { + override val io: CoroutineDispatcher get() = dispatcher + override val default: CoroutineDispatcher get() = dispatcher + override val main: CoroutineDispatcher get() = dispatcher + } + + @Before + fun setUp() { + server = MockWebServer().also { it.start() } + api = Retrofit.Builder() + .baseUrl(server.url("/")) + .addConverterFactory(json.asConverterFactory("application/json".toMediaType())) + .build() + .create(ListsApi::class.java) + repository = DefaultListsRepository(api, FakeViewsDao(), json, testDispatchers) + } + + @After + fun tearDown() = server.shutdown() + + @Test + fun `getViews parses shared and personal views and keeps unknown config keys`() = runTest(dispatcher) { + server.enqueue( + MockResponse().setBody( + """ + { + "views": [ + { "id": "v1", "listId": "L1", "userId": "u-other", "name": "Roadmap", + "scope": "shared", "isDefault": true, "position": 0, + "config": { "mode": "records", "density": "compact", "filters": [], + "groupBy": "status", "sort": { "by": "due" } } }, + { "id": "v2", "listId": "L1", "userId": "me", "name": "Mine", + "scope": "personal", "isDefault": false, "position": 1, + "config": { "mode": "erd", "density": "cozy", "filters": [] } } + ] + } + """.trimIndent(), + ), + ) + + val result = repository.getViews("L1") + + assertThat(result).isInstanceOf(ApiResult.Success::class.java) + val views = (result as ApiResult.Success).data + assertThat(views.map { it.id }).containsExactly("v1", "v2").inOrder() + + val shared = views[0] + assertThat(shared.scope).isEqualTo(ListViewScope.SHARED) + assertThat(shared.isDefault).isTrue() + assertThat(shared.config.density).isEqualTo(ListViewDensity.COMPACT) + // Keys this client does not model survive untouched, ready to be written back. + assertThat(shared.config.raw["groupBy"]).isEqualTo(JsonPrimitive("status")) + assertThat(shared.config.raw).containsKey("sort") + assertThat(shared.isOwnedBy("me")).isFalse() + + val personal = views[1] + assertThat(personal.scope).isEqualTo(ListViewScope.PERSONAL) + assertThat(personal.isOwnedBy("me")).isTrue() + // Values the client does not model fall back for display but are not lost. + assertThat(personal.config.mode).isEqualTo(ListViewMode.RECORDS) + assertThat(personal.config.storedMode).isEqualTo("erd") + assertThat(personal.config.density).isEqualTo(ListViewDensity.COMFORTABLE) + + val request = server.takeRequest() + assertThat(request.method).isEqualTo("GET") + assertThat(request.path).isEqualTo("/api/lists/L1/views") + } + + @Test + fun `createView posts name and scope and returns the created view`() = runTest(dispatcher) { + server.enqueue( + MockResponse().setResponseCode(201).setBody( + """ + { "view": { "id": "v9", "listId": "L1", "userId": "me", "name": "By status", + "scope": "shared", "isDefault": false, "position": 2, + "config": { "mode": "records", "density": "comfortable", "filters": [] } } } + """.trimIndent(), + ), + ) + + val result = repository.createView("L1", " By status ", ListViewScope.SHARED) + + assertThat(result).isInstanceOf(ApiResult.Success::class.java) + val view = (result as ApiResult.Success).data + assertThat(view.id).isEqualTo("v9") + assertThat(view.scope).isEqualTo(ListViewScope.SHARED) + assertThat(view.position).isEqualTo(2) + + val request = server.takeRequest() + assertThat(request.method).isEqualTo("POST") + assertThat(request.path).isEqualTo("/api/lists/L1/views") + val body = request.body.readUtf8() + assertThat(body).contains("\"name\":\"By status\"") + assertThat(body).contains("\"scope\":\"shared\"") + // isDefault is only sent when asked for, so the server keeps its own default. + assertThat(body).doesNotContain("isDefault") + } + + @Test + fun `createView rejects a missing scope before issuing a request`() = runTest(dispatcher) { + val result = repository.createView("L1", "Nameless scope", scope = null) + + assertThat(result).isInstanceOf(ApiResult.Failure::class.java) + assertThat((result as ApiResult.Failure).error).isInstanceOf(AppError.Unknown::class.java) + assertThat(result.error.message).contains("shared or personal") + assertThat(server.requestCount).isEqualTo(0) + } + + @Test + fun `createView rejects an unrecognised scope before issuing a request`() = runTest(dispatcher) { + // Anything the API would 400 on parses to null rather than being guessed at. + val parsed = ListViewScope.fromApi("team") + assertThat(parsed).isNull() + + val result = repository.createView("L1", "Team view", scope = parsed) + + assertThat(result).isInstanceOf(ApiResult.Failure::class.java) + assertThat(server.requestCount).isEqualTo(0) + } + + @Test + fun `createView rejects a blank name before issuing a request`() = runTest(dispatcher) { + val result = repository.createView("L1", " ", ListViewScope.PERSONAL) + + assertThat(result).isInstanceOf(ApiResult.Failure::class.java) + assertThat((result as ApiResult.Failure).error.message).contains("name") + assertThat(server.requestCount).isEqualTo(0) + } + + @Test + fun `updateView renames a view and sends only the changed field`() = runTest(dispatcher) { + server.enqueue( + MockResponse().setBody( + """ + { "view": { "id": "v1", "listId": "L1", "userId": "me", "name": "Renamed", + "scope": "personal", "isDefault": false, "position": 0, + "config": { "mode": "records", "density": "comfortable", "filters": [] } } } + """.trimIndent(), + ), + ) + + val result = repository.updateView("L1", "v1", name = "Renamed") + + assertThat(result).isInstanceOf(ApiResult.Success::class.java) + assertThat((result as ApiResult.Success).data.name).isEqualTo("Renamed") + + val request = server.takeRequest() + assertThat(request.method).isEqualTo("PUT") + assertThat(request.path).isEqualTo("/api/lists/L1/views/v1") + val body = request.body.readUtf8() + assertThat(body).contains("\"name\":\"Renamed\"") + assertThat(body).doesNotContain("config") + assertThat(body).doesNotContain("isDefault") + } + + @Test + fun `updateView marks a view as the default`() = runTest(dispatcher) { + server.enqueue( + MockResponse().setBody( + """{ "view": { "id": "v1", "listId": "L1", "name": "Roadmap", "scope": "shared", "isDefault": true } }""", + ), + ) + + val result = repository.updateView("L1", "v1", isDefault = true) + + assertThat(result).isInstanceOf(ApiResult.Success::class.java) + assertThat((result as ApiResult.Success).data.isDefault).isTrue() + assertThat(server.takeRequest().body.readUtf8()).contains("\"isDefault\":true") + } + + @Test + fun `updateView reports the config the server stored not the one that was sent`() = runTest(dispatcher) { + // The API silently drops config values it does not recognise, so the write + // we send and the state we end up in can differ. + server.enqueue( + MockResponse().setBody( + """ + { "view": { "id": "v1", "listId": "L1", "name": "Roadmap", "scope": "personal", + "config": { "mode": "records", "density": "comfortable", "filters": [] } } } + """.trimIndent(), + ), + ) + val optimistic = ListViewConfig( + buildJsonObject { + put("mode", JsonPrimitive("erd")) + put("density", JsonPrimitive("compact")) + }, + ) + + val result = repository.updateView("L1", "v1", config = optimistic) + + assertThat(result).isInstanceOf(ApiResult.Success::class.java) + val stored = (result as ApiResult.Success).data.config + assertThat(stored.storedMode).isEqualTo("records") + assertThat(stored.density).isEqualTo(ListViewDensity.COMFORTABLE) + + // We really did send the optimistic values; the server just did not keep them. + val body = server.takeRequest().body.readUtf8() + assertThat(body).contains("\"mode\":\"erd\"") + assertThat(body).contains("\"density\":\"compact\"") + } + + @Test + fun `forkView posts to the view and returns a personal copy`() = runTest(dispatcher) { + server.enqueue( + MockResponse().setResponseCode(201).setBody( + """ + { "view": { "id": "v-copy", "listId": "L1", "userId": "me", "name": "Roadmap (copy)", + "scope": "personal", "isDefault": false, "position": 3, + "config": { "mode": "records", "density": "compact", "filters": [] } } } + """.trimIndent(), + ), + ) + + val result = repository.forkView("L1", "v1") + + assertThat(result).isInstanceOf(ApiResult.Success::class.java) + val copy = (result as ApiResult.Success).data + assertThat(copy.id).isEqualTo("v-copy") + assertThat(copy.name).isEqualTo("Roadmap (copy)") + assertThat(copy.scope).isEqualTo(ListViewScope.PERSONAL) + assertThat(copy.isOwnedBy("me")).isTrue() + // The fork inherits the original's config, including a non-default density. + assertThat(copy.config.density).isEqualTo(ListViewDensity.COMPACT) + + val request = server.takeRequest() + assertThat(request.method).isEqualTo("POST") + assertThat(request.path).isEqualTo("/api/lists/L1/views/v1") + assertThat(request.body.size).isEqualTo(0) + } + + @Test + fun `deleteView deletes by view id`() = runTest(dispatcher) { + server.enqueue(MockResponse().setBody("""{ "message": "View deleted" }""")) + + val result = repository.deleteView("L1", "v1") + + assertThat(result).isInstanceOf(ApiResult.Success::class.java) + val request = server.takeRequest() + assertThat(request.method).isEqualTo("DELETE") + assertThat(request.path).isEqualTo("/api/lists/L1/views/v1") + } + + @Test + fun `deleteView surfaces a refusal to touch someone else's shared view`() = runTest(dispatcher) { + server.enqueue( + MockResponse().setResponseCode(403) + .setBody("""{ "error": "You cannot modify this view", "code": "forbidden" }"""), + ) + + val result = repository.deleteView("L1", "v1") + + assertThat(result).isInstanceOf(ApiResult.Failure::class.java) + val error = (result as ApiResult.Failure).error + assertThat(error).isInstanceOf(AppError.Forbidden::class.java) + assertThat(error.message).isEqualTo("You cannot modify this view") + } + + @Test + fun `createView fails cleanly when the server returns no view`() = runTest(dispatcher) { + server.enqueue(MockResponse().setResponseCode(201).setBody("{}")) + + val result = repository.createView("L1", "Ghost", ListViewScope.PERSONAL) + + assertThat(result).isInstanceOf(ApiResult.Failure::class.java) + assertThat((result as ApiResult.Failure).error).isInstanceOf(AppError.Unknown::class.java) + } +} + +/** Minimal no-op [ListDao] — the saved-view endpoints don't touch Room. */ +private class FakeViewsDao : ListDao { + private val state = MutableStateFlow>(emptyList()) + override fun observeLists(): Flow> = state + override suspend fun upsertAll(lists: List) {} + override suspend fun upsert(list: CachedListEntity) {} + override suspend fun deleteById(id: String) {} + override suspend fun clear() {} +} diff --git a/feature/lists/src/test/kotlin/com/interlinedlist/android/feature/lists/data/ListViewMapperTest.kt b/feature/lists/src/test/kotlin/com/interlinedlist/android/feature/lists/data/ListViewMapperTest.kt new file mode 100644 index 0000000..327c7c3 --- /dev/null +++ b/feature/lists/src/test/kotlin/com/interlinedlist/android/feature/lists/data/ListViewMapperTest.kt @@ -0,0 +1,79 @@ +package com.interlinedlist.android.feature.lists.data + +import com.google.common.truth.Truth.assertThat +import com.interlinedlist.android.feature.lists.data.remote.dto.ListViewDto +import com.interlinedlist.android.feature.lists.domain.ListViewConfig +import com.interlinedlist.android.feature.lists.domain.ListViewDensity +import com.interlinedlist.android.feature.lists.domain.ListViewMode +import com.interlinedlist.android.feature.lists.domain.ListViewScope +import kotlinx.serialization.json.JsonPrimitive +import kotlinx.serialization.json.buildJsonObject +import org.junit.Test + +/** Mapping rules for saved views: scope parsing, config passthrough, ownership. */ +class ListViewMapperTest { + + @Test + fun `an absent config falls back to the server's own defaults`() { + val view = ListViewMapper.fromDto( + ListViewDto(id = "v1", name = "Roadmap", scope = "personal"), + listId = "L1", + ) + + assertThat(view.listId).isEqualTo("L1") + assertThat(view.config).isEqualTo(ListViewConfig.DEFAULT) + assertThat(view.config.mode).isEqualTo(ListViewMode.RECORDS) + assertThat(view.config.density).isEqualTo(ListViewDensity.COMFORTABLE) + assertThat(view.config.filters).isEmpty() + } + + @Test + fun `config keys the client does not model are carried through untouched`() { + val stored = buildJsonObject { + put("mode", JsonPrimitive("records")) + put("density", JsonPrimitive("compact")) + put("groupBy", JsonPrimitive("status")) + } + + val view = ListViewMapper.fromDto( + ListViewDto(id = "v1", name = "Roadmap", scope = "shared", config = stored), + listId = "L1", + ) + + assertThat(view.config.raw).isEqualTo(stored) + assertThat(view.config.density).isEqualTo(ListViewDensity.COMPACT) + } + + @Test + fun `an unrecognised scope is treated as shared so no destructive action is offered`() { + val view = ListViewMapper.fromDto( + ListViewDto(id = "v1", name = "Odd", scope = "team", userId = null), + listId = "L1", + ) + + assertThat(view.scope).isEqualTo(ListViewScope.SHARED) + assertThat(view.isOwnedBy("me")).isFalse() + } + + @Test + fun `ownership compares user ids when both are known`() { + val mine = ListViewMapper.fromDto( + ListViewDto(id = "v1", name = "Mine", scope = "shared", userId = "me"), + listId = "L1", + ) + val theirs = mine.copy(userId = "someone-else") + + assertThat(mine.isOwnedBy("me")).isTrue() + assertThat(theirs.isOwnedBy("me")).isFalse() + } + + @Test + fun `a personal view with no ids is the caller's own, since the API returns no others`() { + val view = ListViewMapper.fromDto( + ListViewDto(id = "v1", name = "Mine", scope = "personal", userId = null), + listId = "L1", + ) + + assertThat(view.isOwnedBy(currentUserId = null)).isTrue() + } +} diff --git a/feature/lists/src/test/kotlin/com/interlinedlist/android/feature/lists/ui/views/ListViewsViewModelTest.kt b/feature/lists/src/test/kotlin/com/interlinedlist/android/feature/lists/ui/views/ListViewsViewModelTest.kt new file mode 100644 index 0000000..bebc921 --- /dev/null +++ b/feature/lists/src/test/kotlin/com/interlinedlist/android/feature/lists/ui/views/ListViewsViewModelTest.kt @@ -0,0 +1,287 @@ +package com.interlinedlist.android.feature.lists.ui.views + +import androidx.lifecycle.SavedStateHandle +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.lists.FakeListsRepository +import com.interlinedlist.android.feature.lists.data.CurrentUserIdProvider +import com.interlinedlist.android.feature.lists.domain.ListView +import com.interlinedlist.android.feature.lists.domain.ListViewConfig +import com.interlinedlist.android.feature.lists.domain.ListViewDensity +import com.interlinedlist.android.feature.lists.domain.ListViewScope +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 kotlinx.serialization.json.JsonPrimitive +import kotlinx.serialization.json.buildJsonObject +import org.junit.After +import org.junit.Before +import org.junit.Test + +@OptIn(ExperimentalCoroutinesApi::class) +class ListViewsViewModelTest { + + private val dispatcher = StandardTestDispatcher() + + private fun view( + id: String, + name: String = "View $id", + scope: ListViewScope = ListViewScope.PERSONAL, + ownerId: String? = "me", + isDefault: Boolean = false, + config: ListViewConfig = ListViewConfig.DEFAULT, + ) = ListView( + id = id, + listId = "L1", + userId = ownerId, + name = name, + scope = scope, + config = config, + isDefault = isDefault, + position = 0, + ) + + private fun viewModel(repo: FakeListsRepository, userId: String? = "me") = + ListViewsViewModel( + repository = repo, + currentUserIdProvider = CurrentUserIdProvider { userId }, + savedStateHandle = SavedStateHandle(mapOf(VIEWS_LIST_ID_ARG to "L1")), + ) + + @Before fun setUp() = Dispatchers.setMain(dispatcher) + + @After fun tearDown() = Dispatchers.resetMain() + + @Test + fun `loads shared and personal views and selects the default`() = runTest(dispatcher) { + val repo = FakeListsRepository().apply { + viewsResult = ApiResult.Success( + listOf( + view("v1", "Roadmap", ListViewScope.SHARED, ownerId = "someone-else"), + view("v2", "Mine", ListViewScope.PERSONAL, isDefault = true), + ), + ) + } + val vm = viewModel(repo) + advanceUntilIdle() + + val state = vm.uiState.value + assertThat(state.isLoading).isFalse() + assertThat(state.sharedViews.map { it.id }).containsExactly("v1") + assertThat(state.personalViews.map { it.id }).containsExactly("v2") + // The default view is what the screen opens on. + assertThat(state.selectedViewId).isEqualTo("v2") + // Someone else's shared view is read-only but forkable. + assertThat(state.canModify(state.sharedViews.single())).isFalse() + assertThat(state.canFork(state.sharedViews.single())).isTrue() + assertThat(state.canModify(state.personalViews.single())).isTrue() + } + + @Test + fun `load failure surfaces a message`() = runTest(dispatcher) { + val repo = FakeListsRepository().apply { + viewsResult = ApiResult.Failure(AppError.Server("boom")) + } + val vm = viewModel(repo) + advanceUntilIdle() + + assertThat(vm.uiState.value.isLoading).isFalse() + assertThat(vm.uiState.value.errorMessage).isNotNull() + } + + @Test + fun `create adds the view the server returned and selects it`() = runTest(dispatcher) { + val serverCopy = view("v-new", "By status", ListViewScope.SHARED) + val repo = FakeListsRepository().apply { createViewResult = ApiResult.Success(serverCopy) } + val vm = viewModel(repo) + advanceUntilIdle() + + vm.createView("By status", ListViewScope.SHARED) + advanceUntilIdle() + + assertThat(repo.lastCreatedViewScope).isEqualTo(ListViewScope.SHARED) + assertThat(vm.uiState.value.views.map { it.id }).containsExactly("v-new") + assertThat(vm.uiState.value.selectedViewId).isEqualTo("v-new") + assertThat(vm.uiState.value.isSaving).isFalse() + } + + @Test + fun `create without a scope reports the problem and adds nothing`() = runTest(dispatcher) { + val repo = FakeListsRepository() + val vm = viewModel(repo) + advanceUntilIdle() + + vm.createView("Scopeless", scope = null) + advanceUntilIdle() + + assertThat(vm.uiState.value.views).isEmpty() + assertThat(vm.uiState.value.errorMessage).contains("shared or personal") + } + + @Test + fun `rename adopts the server copy of the view, not the optimistic one`() = runTest(dispatcher) { + // The server silently rewrites config values it does not recognise, so what + // it returns — a compact density here — is what the UI must end up showing. + val stored = ListViewConfig( + buildJsonObject { + put("mode", JsonPrimitive("records")) + put("density", JsonPrimitive("compact")) + put("groupBy", JsonPrimitive("status")) + }, + ) + val repo = FakeListsRepository().apply { + viewsResult = ApiResult.Success(listOf(view("v1", "Old name"))) + updateViewResult = ApiResult.Success(view("v1", "New name", config = stored)) + } + val vm = viewModel(repo) + advanceUntilIdle() + assertThat(vm.uiState.value.views.single().config.density).isEqualTo(ListViewDensity.COMFORTABLE) + + vm.renameView(vm.uiState.value.views.single(), "New name") + advanceUntilIdle() + + val updated = vm.uiState.value.views.single() + assertThat(updated.name).isEqualTo("New name") + assertThat(updated.config.density).isEqualTo(ListViewDensity.COMPACT) + assertThat(updated.config.raw["groupBy"]).isEqualTo(JsonPrimitive("status")) + assertThat(repo.lastUpdatedViewName).isEqualTo("New name") + } + + @Test + fun `rename of someone else's shared view never reaches the server`() = runTest(dispatcher) { + val repo = FakeListsRepository().apply { + viewsResult = ApiResult.Success( + listOf(view("v1", "Roadmap", ListViewScope.SHARED, ownerId = "someone-else")), + ) + } + val vm = viewModel(repo) + advanceUntilIdle() + + vm.renameView(vm.uiState.value.views.single(), "Mine now") + advanceUntilIdle() + + assertThat(repo.lastUpdatedViewId).isNull() + assertThat(vm.uiState.value.errorMessage).contains("personal copy") + assertThat(vm.uiState.value.views.single().name).isEqualTo("Roadmap") + } + + @Test + fun `a server refusal to rename leaves the view untouched`() = runTest(dispatcher) { + val repo = FakeListsRepository().apply { + viewsResult = ApiResult.Success(listOf(view("v1", "Roadmap", ListViewScope.SHARED))) + updateViewResult = ApiResult.Failure(AppError.Forbidden("You cannot modify this view")) + } + val vm = viewModel(repo) + advanceUntilIdle() + + vm.renameView(vm.uiState.value.views.single(), "Renamed") + advanceUntilIdle() + + assertThat(vm.uiState.value.views.single().name).isEqualTo("Roadmap") + assertThat(vm.uiState.value.errorMessage).isEqualTo("You cannot modify this view") + assertThat(vm.uiState.value.isSaving).isFalse() + } + + @Test + fun `setting a default re-reads the views so the flag moves`() = runTest(dispatcher) { + val repo = FakeListsRepository().apply { + viewsResult = ApiResult.Success( + listOf(view("v1", "First", isDefault = true), view("v2", "Second")), + ) + } + val vm = viewModel(repo) + advanceUntilIdle() + // The server moves the flag; the re-read reflects it across both views. + repo.updateViewResult = ApiResult.Success(view("v2", "Second", isDefault = true)) + repo.viewsResult = ApiResult.Success( + listOf(view("v1", "First"), view("v2", "Second", isDefault = true)), + ) + + vm.setDefault(vm.uiState.value.views.last()) + advanceUntilIdle() + + assertThat(repo.lastUpdatedViewIsDefault).isTrue() + assertThat(vm.uiState.value.views.map { it.isDefault }).containsExactly(false, true).inOrder() + } + + @Test + fun `fork adds the personal copy the server made and selects it`() = runTest(dispatcher) { + val shared = view("v1", "Roadmap", ListViewScope.SHARED, ownerId = "someone-else") + val repo = FakeListsRepository().apply { + viewsResult = ApiResult.Success(listOf(shared)) + forkViewResult = ApiResult.Success( + view("v1-copy", "Roadmap (copy)", ListViewScope.PERSONAL), + ) + } + val vm = viewModel(repo) + advanceUntilIdle() + + vm.forkView(vm.uiState.value.views.single()) + advanceUntilIdle() + + assertThat(repo.lastForkedViewId).isEqualTo("v1") + val state = vm.uiState.value + assertThat(state.personalViews.map { it.name }).containsExactly("Roadmap (copy)") + assertThat(state.selectedViewId).isEqualTo("v1-copy") + // The shared original is left exactly as it was. + assertThat(state.sharedViews).containsExactly(shared) + } + + @Test + fun `delete removes the view once the server confirms and reselects`() = runTest(dispatcher) { + val repo = FakeListsRepository().apply { + viewsResult = ApiResult.Success( + listOf(view("v1", "First", isDefault = true), view("v2", "Second")), + ) + } + val vm = viewModel(repo) + advanceUntilIdle() + vm.selectView("v2") + + vm.deleteView(vm.uiState.value.views.last()) + advanceUntilIdle() + + assertThat(repo.lastDeletedViewId).isEqualTo("v2") + assertThat(vm.uiState.value.views.map { it.id }).containsExactly("v1") + assertThat(vm.uiState.value.selectedViewId).isEqualTo("v1") + } + + @Test + fun `a refused delete keeps the view in the list`() = runTest(dispatcher) { + val repo = FakeListsRepository().apply { + viewsResult = ApiResult.Success(listOf(view("v1", "Roadmap", ListViewScope.SHARED))) + deleteViewResult = ApiResult.Failure(AppError.Forbidden("You cannot modify this view")) + } + val vm = viewModel(repo) + advanceUntilIdle() + + vm.deleteView(vm.uiState.value.views.single()) + advanceUntilIdle() + + assertThat(vm.uiState.value.views.map { it.id }).containsExactly("v1") + assertThat(vm.uiState.value.errorMessage).isEqualTo("You cannot modify this view") + } + + @Test + fun `delete of someone else's shared view never reaches the server`() = runTest(dispatcher) { + val repo = FakeListsRepository().apply { + viewsResult = ApiResult.Success( + listOf(view("v1", "Roadmap", ListViewScope.SHARED, ownerId = "someone-else")), + ) + } + val vm = viewModel(repo) + advanceUntilIdle() + + vm.deleteView(vm.uiState.value.views.single()) + advanceUntilIdle() + + assertThat(repo.lastDeletedViewId).isNull() + assertThat(vm.uiState.value.views).hasSize(1) + assertThat(vm.uiState.value.errorMessage).contains("can delete it") + } +}