From 3fc6066d67d06ca1d31b9bd1ea2df59b11ff4f51 Mon Sep 17 00:00:00 2001 From: Altay Date: Thu, 1 Oct 2026 09:58:32 +0300 Subject: [PATCH 1/3] feat(files): remember the Move and Copy target folder on the device Web's folder picker has a "Remember target folder" toggle for Move and Make a copy, kept in web's own /config. Android always opened at root. The mobile picker now shows the toggle, off by default. On, Move here or Copy here records the chosen folder's path, and the next picker opens there; Back reads each ancestor as it reaches it. A remembered folder that can't be read reopens at root, one moved since keeps only root above it, and a move never opens inside the item it moves. Both values live in private SharedPreferences per signed-in account, and the auth controller's local-session cleanup clears them on sign-out or an expired session. TV has no Move picker. Closes #276 --- .../files/FilesMoveDestinationController.kt | 31 ++++-- .../files/FilesMoveDestinationStart.kt | 37 +++++++ .../files/FilesMoveDestinationState.kt | 29 +++++- .../android/files/FilesMoveTargetMemory.kt | 28 ++++++ .../io/putdotio/android/MobileNavigation.kt | 6 ++ .../android/auth/MobileAuthController.kt | 3 + .../android/auth/MobileOAuthRuntime.kt | 2 + .../files/MobileFilesMoveDestination.kt | 18 ++++ .../android/files/MobileFilesRoute.kt | 43 ++++++++- .../android/files/MobileMoveTargetStore.kt | 60 ++++++++++++ app/src/mobile/res/values/strings.xml | 1 + .../FilesMoveDestinationControllerTest.kt | 96 +++++++++++++++++++ .../files/FilesMoveTargetMemoryTest.kt | 40 ++++++++ .../putdotio/android/MobileFilesCopyTest.kt | 39 ++++++++ .../putdotio/android/MobileFilesMoveTest.kt | 44 +++++++++ .../android/auth/MobileAuthControllerTest.kt | 17 ++++ .../android/files/InMemoryMoveTargetStore.kt | 10 ++ .../files/MobileMoveTargetStoreTest.kt | 43 +++++++++ docs/behavior.md | 24 ++++- 19 files changed, 558 insertions(+), 13 deletions(-) create mode 100644 app/src/main/kotlin/io/putdotio/android/files/FilesMoveDestinationStart.kt create mode 100644 app/src/main/kotlin/io/putdotio/android/files/FilesMoveTargetMemory.kt create mode 100644 app/src/mobile/kotlin/io/putdotio/android/files/MobileMoveTargetStore.kt create mode 100644 app/src/test/kotlin/io/putdotio/android/files/FilesMoveTargetMemoryTest.kt create mode 100644 app/src/testMobile/kotlin/io/putdotio/android/files/InMemoryMoveTargetStore.kt create mode 100644 app/src/testMobile/kotlin/io/putdotio/android/files/MobileMoveTargetStoreTest.kt diff --git a/app/src/main/kotlin/io/putdotio/android/files/FilesMoveDestinationController.kt b/app/src/main/kotlin/io/putdotio/android/files/FilesMoveDestinationController.kt index 4eceb7b0..be4dc03e 100644 --- a/app/src/main/kotlin/io/putdotio/android/files/FilesMoveDestinationController.kt +++ b/app/src/main/kotlin/io/putdotio/android/files/FilesMoveDestinationController.kt @@ -17,33 +17,48 @@ class FilesMoveDestinationController private constructor( sourceFolderId: FilesItemId?, private val repository: FilesRepository, parentScope: CoroutineScope, + startPath: List, @Suppress("UNUSED_PARAMETER") marker: Unit, ) : Closeable { + /** [startPath] is the folder to open at and its ancestors below root; empty opens at root. */ constructor( sourceItem: FilesItem, sourceFolderId: FilesItemId, repository: FilesRepository, parentScope: CoroutineScope, - ) : this(sourceItem, sourceFolderId, repository, parentScope, Unit) + startPath: List = emptyList(), + ) : this(sourceItem, sourceFolderId, repository, parentScope, startPath, Unit) /** Picks a folder for new content; every folder, root included, is a valid destination. */ - constructor(repository: FilesRepository, parentScope: CoroutineScope) : - this(null, null, repository, parentScope, Unit) + constructor( + repository: FilesRepository, + parentScope: CoroutineScope, + startPath: List = emptyList(), + ) : this(null, null, repository, parentScope, startPath, Unit) init { require( sourceItem == null && sourceFolderId == null || sourceItem != null && sourceItem.id.value > 0L && sourceFolderId != null && sourceFolderId.value >= 0L, ) { "Move picker requires a non-root source" } + require(startPath.all { it.id.value > 0L }) { "A start path lies below root" } } private val lock = Any() private val scope = CoroutineScope(parentScope.coroutineContext + SupervisorJob(parentScope.coroutineContext[Job])) - private val firstRequest = FilesMoveDestinationRequest(FilesFolder.Root.id, FilesRequestId(1L)) + private val firstRequest = FilesMoveDestinationRequest( + startPath.lastOrNull()?.id ?: FilesFolder.Root.id, + FilesRequestId(1L), + ) private val mutableState = MutableStateFlow(FilesMoveDestinationState( sourceItem, sourceFolderId, - listOf(FilesMoveDestinationFolder(FilesFolder.Root, FilesContent.Loading(firstRequest.requestId))), + (listOf(FilesFolder.Root) + startPath).dropLast(1).map(::unreadFolder) + + FilesMoveDestinationFolder( + startPath.lastOrNull() ?: FilesFolder.Root, + FilesContent.Loading(firstRequest.requestId), + ), nextRequestValue = 2L, + opensRememberedTarget = startPath.isNotEmpty(), )) private var job: Job? = null private var closed = false @@ -69,7 +84,11 @@ class FilesMoveDestinationController private constructor( val next = scope.launch(start = CoroutineStart.LAZY) { val result = load(request) synchronized(lock) { - if (!closed) mutableState.value = mutableState.value.complete(request, result) + if (!closed) { + val transition = mutableState.value.complete(request, result) + mutableState.value = transition.state + transition.request?.let(::startRequest) + } } } job = next diff --git a/app/src/main/kotlin/io/putdotio/android/files/FilesMoveDestinationStart.kt b/app/src/main/kotlin/io/putdotio/android/files/FilesMoveDestinationStart.kt new file mode 100644 index 00000000..e36aeb51 --- /dev/null +++ b/app/src/main/kotlin/io/putdotio/android/files/FilesMoveDestinationStart.kt @@ -0,0 +1,37 @@ +package io.putdotio.android.files + +internal fun FilesMoveDestinationState.withoutRememberedTargetCheck() = + if (opensRememberedTarget) copy(opensRememberedTarget = false) else this + +/** + * Web reopens at root when the remembered folder can't be read. A folder read since it was + * remembered takes its current name, and one moved elsewhere keeps only root above it. + */ +internal fun FilesMoveDestinationState.completeRememberedTarget( + request: FilesMoveDestinationRequest, + result: FilesRepositoryResult, +): FilesMoveDestinationTransition { + val checked = copy(opensRememberedTarget = false) + if (result !is FilesRepositoryResult.Success) { + val requestId = FilesRequestId(nextRequestValue) + return FilesMoveDestinationTransition( + checked.copy( + stack = listOf(FilesMoveDestinationFolder(FilesFolder.Root, FilesContent.Loading(requestId))), + nextRequestValue = nextRequestValue + 1, + ), + FilesMoveDestinationRequest(FilesFolder.Root.id, requestId), + ) + } + val listed = result.value.parent?.takeIf { it.id == current.folder.id } + val target = current.copy(folder = current.folder.copy(name = listed?.name ?: current.folder.name)) + val ancestors = stack.dropLast(1) + val placed = listed?.parentId == null || listed.parentId == ancestors.last().folder.id + return FilesMoveDestinationTransition(checked.copy( + stack = (if (placed) ancestors else listOf(unreadFolder(FilesFolder.Root))) + target, + ).completeListing(request, result)) +} + +internal fun unreadFolder(folder: FilesFolder) = FilesMoveDestinationFolder(folder, FilesContent.Loading(UNREAD)) + +/** Never issued: request IDs start at 1. */ +private val UNREAD = FilesRequestId(0L) diff --git a/app/src/main/kotlin/io/putdotio/android/files/FilesMoveDestinationState.kt b/app/src/main/kotlin/io/putdotio/android/files/FilesMoveDestinationState.kt index cdade62d..86ae83c2 100644 --- a/app/src/main/kotlin/io/putdotio/android/files/FilesMoveDestinationState.kt +++ b/app/src/main/kotlin/io/putdotio/android/files/FilesMoveDestinationState.kt @@ -14,6 +14,8 @@ data class FilesMoveDestinationState internal constructor( val sourceFolderId: FilesItemId?, internal val stack: List, internal val nextRequestValue: Long, + /** The picker opened at a remembered folder whose first read is pending; a failed read restarts at root. */ + internal val opensRememberedTarget: Boolean = false, ) { val current: FilesMoveDestinationFolder get() = stack.last() val path: List get() = stack.map { it.folder } @@ -47,10 +49,13 @@ internal data class FilesMoveDestinationTransition( ) internal fun FilesMoveDestinationState.reduce(event: FilesMoveDestinationEvent): FilesMoveDestinationTransition = + withoutRememberedTargetCheck().reduceEvent(event).let { if (it.consumed) it else it.copy(state = this) } + +private fun FilesMoveDestinationState.reduceEvent(event: FilesMoveDestinationEvent): FilesMoveDestinationTransition = when (event) { is FilesMoveDestinationEvent.OpenFolder -> openDestination(event.itemId) FilesMoveDestinationEvent.NavigateBack -> if (canNavigateBack) { - FilesMoveDestinationTransition(copy(stack = stack.dropLast(1))) + copy(stack = stack.dropLast(1)).loadUnreadFolder() } else { FilesMoveDestinationTransition(this, consumed = false) } @@ -67,6 +72,17 @@ internal fun FilesMoveDestinationState.reduce(event: FilesMoveDestinationEvent): } } +/** A remembered path's folders above the target are read only when Back reaches them. */ +private fun FilesMoveDestinationState.loadUnreadFolder(): FilesMoveDestinationTransition { + if (current.content !is FilesContent.Loading) return FilesMoveDestinationTransition(this) + val requestId = FilesRequestId(nextRequestValue) + return FilesMoveDestinationTransition( + copy(stack = stack.replaceLast(current.copy(content = FilesContent.Loading(requestId))), + nextRequestValue = nextRequestValue + 1), + FilesMoveDestinationRequest(current.folder.id, requestId), + ) +} + private fun FilesMoveDestinationState.openDestination(itemId: FilesItemId): FilesMoveDestinationTransition { if (!canOpenFolder(itemId)) return FilesMoveDestinationTransition(this, consumed = false) val item = current.content.items().first { it.id == itemId } @@ -101,8 +117,17 @@ private fun FilesMoveDestinationState.loadDestinationPage(retry: Boolean): Files internal fun FilesMoveDestinationState.complete( request: FilesMoveDestinationRequest, result: FilesRepositoryResult, +): FilesMoveDestinationTransition = when { + current.folder.id != request.folderId || current.requestId() != request.requestId -> + FilesMoveDestinationTransition(this) + opensRememberedTarget -> completeRememberedTarget(request, result) + else -> FilesMoveDestinationTransition(completeListing(request, result)) +} + +internal fun FilesMoveDestinationState.completeListing( + request: FilesMoveDestinationRequest, + result: FilesRepositoryResult, ): FilesMoveDestinationState { - if (current.folder.id != request.folderId || current.requestId() != request.requestId) return this val consumed = if (request.cursor == null) emptySet() else current.consumedCursors + request.cursor val content = when (result) { is FilesRepositoryResult.Success -> { diff --git a/app/src/main/kotlin/io/putdotio/android/files/FilesMoveTargetMemory.kt b/app/src/main/kotlin/io/putdotio/android/files/FilesMoveTargetMemory.kt new file mode 100644 index 00000000..4fe37492 --- /dev/null +++ b/app/src/main/kotlin/io/putdotio/android/files/FilesMoveTargetMemory.kt @@ -0,0 +1,28 @@ +package io.putdotio.android.files + +/** + * Web's "Remember target folder" for the Move and Make a copy picker, kept on this device for one + * account rather than in web's `/config`. [lastTarget] is the path below root to the folder last + * chosen while [remember] was on; turning [remember] off keeps it, as web does. + */ +data class FilesMoveTargetMemory( + val remember: Boolean = false, + val lastTarget: List = emptyList(), +) { + /** Where the picker opens, below root; a move never opens inside the item it moves. */ + fun startPath(sourceItem: FilesItem?): List = when { + !remember -> emptyList() + sourceItem != null && lastTarget.any { it.id == sourceItem.id } -> emptyList() + else -> lastTarget + } + + /** Web records the chosen folder only while the toggle is on. */ + fun chosen(path: List): FilesMoveTargetMemory = + if (remember) copy(lastTarget = path.filter { it.id.value > 0L }) else this +} + +interface FilesMoveTargetStore { + fun read(): FilesMoveTargetMemory + + fun write(memory: FilesMoveTargetMemory) +} diff --git a/app/src/mobile/kotlin/io/putdotio/android/MobileNavigation.kt b/app/src/mobile/kotlin/io/putdotio/android/MobileNavigation.kt index cc06de28..1bafa33c 100644 --- a/app/src/mobile/kotlin/io/putdotio/android/MobileNavigation.kt +++ b/app/src/mobile/kotlin/io/putdotio/android/MobileNavigation.kt @@ -4,8 +4,10 @@ import android.net.Uri import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue +import androidx.compose.runtime.remember import androidx.compose.runtime.rememberUpdatedState import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.LocalContext import androidx.lifecycle.compose.LifecycleStartEffect import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.lifecycle.viewmodel.compose.viewModel @@ -30,6 +32,7 @@ import io.putdotio.android.files.FilesItem import io.putdotio.android.files.FilesItemId import io.putdotio.android.files.FilesRepository import io.putdotio.android.files.MobileFilesRoute +import io.putdotio.android.files.MobileMoveTargetStore import io.putdotio.android.playback.MobilePlaybackViewModel import io.putdotio.android.playback.MobilePlayerFactory import io.putdotio.android.playback.MobilePlayerScreen @@ -99,6 +102,8 @@ internal fun MobileNavHost( val currentTransfersState by rememberUpdatedState(transfersState) val currentTransfersSessionId by rememberUpdatedState(transfersSessionId) val currentOnTransfersEvent by rememberUpdatedState(onTransfersEvent) + val appContext = LocalContext.current.applicationContext + val moveTargetStore = remember(appContext, account.userId) { MobileMoveTargetStore(appContext, account.userId) } NavHost( navController = navController, startDestination = MobileDestination.start.route, @@ -118,6 +123,7 @@ internal fun MobileNavHost( }, onShareItem = onShareItem, onViewTrash = trashController?.let { { navController.navigateToTrash() } }, + moveTargetStore = moveTargetStore, ) } composable(MobileDestination.Search.route) { diff --git a/app/src/mobile/kotlin/io/putdotio/android/auth/MobileAuthController.kt b/app/src/mobile/kotlin/io/putdotio/android/auth/MobileAuthController.kt index 79da8871..e91a26ff 100644 --- a/app/src/mobile/kotlin/io/putdotio/android/auth/MobileAuthController.kt +++ b/app/src/mobile/kotlin/io/putdotio/android/auth/MobileAuthController.kt @@ -80,6 +80,8 @@ class MobileAuthController internal constructor( private val tokenRevocations: TokenRevocations, private val stateGenerator: OAuthStateGenerator = SecureOAuthStateGenerator(), private val clock: OAuthAttemptClock = SystemOAuthAttemptClock, + /** Drops what this device keeps for the account, such as the Move picker's remembered folder. */ + private val clearAccountLocalState: () -> Unit = {}, ) { private val operationMutex = Mutex() private val mutableState = MutableStateFlow(MobileAuthState.Initializing) @@ -328,6 +330,7 @@ class MobileAuthController internal constructor( } private suspend fun clearLocalSession(): Boolean { + clearAccountLocalState() val storageCleared = try { tokenStore.clear() true diff --git a/app/src/mobile/kotlin/io/putdotio/android/auth/MobileOAuthRuntime.kt b/app/src/mobile/kotlin/io/putdotio/android/auth/MobileOAuthRuntime.kt index 9a419c6c..d951f01d 100644 --- a/app/src/mobile/kotlin/io/putdotio/android/auth/MobileOAuthRuntime.kt +++ b/app/src/mobile/kotlin/io/putdotio/android/auth/MobileOAuthRuntime.kt @@ -5,6 +5,7 @@ import android.util.Log import io.putdotio.android.BuildConfig import io.putdotio.android.playback.SdkPlaybackPositionRepository import io.putdotio.android.downloads.MobileDownloadCache +import io.putdotio.android.files.MobileMoveTargetStore import io.putdotio.android.share.MobileFileShareService import io.putdotio.sdk.PutioClient import io.putdotio.sdk.PutioConfig @@ -137,6 +138,7 @@ class MobileOAuthRuntime internal constructor( revoker = PutioAuthTokenRevoker(putioClient.config), scope = applicationScope, ), + clearAccountLocalState = { MobileMoveTargetStore.clearAll(context) }, ) return MobileOAuthRuntime( putioClient = putioClient, diff --git a/app/src/mobile/kotlin/io/putdotio/android/files/MobileFilesMoveDestination.kt b/app/src/mobile/kotlin/io/putdotio/android/files/MobileFilesMoveDestination.kt index 51dad7f8..d1faa458 100644 --- a/app/src/mobile/kotlin/io/putdotio/android/files/MobileFilesMoveDestination.kt +++ b/app/src/mobile/kotlin/io/putdotio/android/files/MobileFilesMoveDestination.kt @@ -11,6 +11,7 @@ import androidx.compose.foundation.layout.widthIn import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.items import androidx.compose.foundation.lazy.rememberLazyListState +import androidx.compose.foundation.selection.toggleable import androidx.compose.material3.Button import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.HorizontalDivider @@ -19,6 +20,7 @@ import androidx.compose.material3.IconButton import androidx.compose.material3.ListItem import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Surface +import androidx.compose.material3.Switch import androidx.compose.material3.Text import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable @@ -47,6 +49,7 @@ internal const val MOBILE_FILES_MOVE_BACK_TAG = "mobile-files-move-back" internal const val MOBILE_FILES_MOVE_RETRY_TAG = "mobile-files-move-retry" internal const val MOBILE_FILES_MOVE_LOAD_MORE_TAG = "mobile-files-move-load-more" internal const val MOBILE_FILES_MOVE_FOLDER_TAG = "mobile-files-move-folder" +internal const val MOBILE_FILES_MOVE_REMEMBER_TAG = "mobile-files-move-remember" internal fun mobileFilesMoveFolderTag(id: FilesItemId): String = "mobile-files-move-folder-${id.value}" @@ -60,6 +63,8 @@ internal fun MobileFilesMoveDestination( title: String = stringResource(R.string.mobile_files_move), confirmLabel: String = stringResource(R.string.mobile_files_move_here), sourceName: String? = state.sourceItem?.name, + rememberTarget: Boolean? = null, + onRememberTargetChange: (Boolean) -> Unit = {}, ) { val back = { if (state.canNavigateBack) onEvent(FilesMoveDestinationEvent.NavigateBack) else onCancel() @@ -84,6 +89,19 @@ internal fun MobileFilesMoveDestination( MobileMoveFolderContent(state, sourceName, onEvent, back, Modifier.weight(1f)) } HorizontalDivider() + rememberTarget?.let { checked -> + Row( + verticalAlignment = Alignment.CenterVertically, + modifier = Modifier.fillMaxWidth() + .toggleable(checked, role = Role.Switch, onValueChange = onRememberTargetChange) + .padding(start = 16.dp, end = 16.dp, top = 8.dp) + .testTag(MOBILE_FILES_MOVE_REMEMBER_TAG), + ) { + Text(stringResource(R.string.mobile_files_move_remember_target), + style = MaterialTheme.typography.bodyLarge, modifier = Modifier.weight(1f)) + Switch(checked = checked, onCheckedChange = null) + } + } Button(onClick = onConfirm, enabled = canSubmit && state.canMoveHere, modifier = Modifier.padding(16.dp).fillMaxWidth().testTag(MOBILE_FILES_MOVE_HERE_TAG)) { Text(confirmLabel) diff --git a/app/src/mobile/kotlin/io/putdotio/android/files/MobileFilesRoute.kt b/app/src/mobile/kotlin/io/putdotio/android/files/MobileFilesRoute.kt index a53437c9..4acea744 100644 --- a/app/src/mobile/kotlin/io/putdotio/android/files/MobileFilesRoute.kt +++ b/app/src/mobile/kotlin/io/putdotio/android/files/MobileFilesRoute.kt @@ -2,6 +2,7 @@ package io.putdotio.android.files import androidx.compose.runtime.Composable import androidx.compose.runtime.DisposableEffect +import androidx.compose.runtime.MutableState import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.compose.runtime.key @@ -29,6 +30,7 @@ internal fun MobileFilesRoute( onDownloadItem: ((FilesItem) -> Unit)? = null, onShareItem: ((FilesItem) -> Unit)? = null, onViewTrash: (() -> Unit)? = null, + moveTargetStore: FilesMoveTargetStore? = null, ) { key(repository, state.current.folder.id.value) { var movingItemId by rememberSaveable { mutableStateOf(null) } @@ -56,6 +58,7 @@ internal fun MobileFilesRoute( sourceFolderId = state.current.folder.id, repository = repository, canSubmit = state.canStartMove, + targetStore = moveTargetStore, onAuthenticationRequired = onAuthenticationRequired, onEvent = onEvent, onDismiss = { movingItemId = null }, @@ -67,6 +70,7 @@ internal fun MobileFilesRoute( folderId = state.current.folder.id, repository = repository, canSubmit = state.canStartCopy, + targetStore = moveTargetStore, onAuthenticationRequired = onAuthenticationRequired, onEvent = onEvent, onDismiss = { copyingItemId = null }, @@ -77,7 +81,7 @@ internal fun MobileFilesRoute( /** * The move picker without a source item, so every folder of the viewer's own, root included, is a - * destination. It opens at root, as web's does unless its "Remember target folder" is on. + * destination. Like Move, it opens at root unless "Remember target folder" is on. */ @Composable private fun MobileFilesCopySession( @@ -85,12 +89,16 @@ private fun MobileFilesCopySession( folderId: FilesItemId, repository: FilesRepository, canSubmit: Boolean, + targetStore: FilesMoveTargetStore?, onAuthenticationRequired: suspend () -> Unit, onEvent: (FilesBrowserEvent) -> Boolean, onDismiss: () -> Unit, ) { val scope = rememberCoroutineScope() - val controller = remember(item.id, repository) { FilesMoveDestinationController(repository, scope) } + val memory = rememberMoveTargetMemory(targetStore, item.id) + val controller = remember(item.id, repository) { + FilesMoveDestinationController(repository, scope, memory.value.startPath(sourceItem = null)) + } var finished by remember(controller) { mutableStateOf(false) } DisposableEffect(controller) { onDispose { @@ -115,6 +123,7 @@ private fun MobileFilesCopySession( ) { finished = true if (onEvent(FilesBrowserEvent.Copy(folderId, item.id, current.current.folder))) { + memory.choose(targetStore, current.path) onDismiss() } else { finished = false @@ -125,6 +134,8 @@ private fun MobileFilesCopySession( title = stringResource(R.string.mobile_files_make_copy), confirmLabel = stringResource(R.string.mobile_files_copy_here), sourceName = item.name, + rememberTarget = targetStore?.let { memory.value.remember }, + onRememberTargetChange = { memory.setRemember(targetStore, it) }, ) } @@ -134,13 +145,15 @@ private fun MobileFilesMoveSession( sourceFolderId: FilesItemId, repository: FilesRepository, canSubmit: Boolean, + targetStore: FilesMoveTargetStore?, onAuthenticationRequired: suspend () -> Unit, onEvent: (FilesBrowserEvent) -> Boolean, onDismiss: () -> Unit, ) { val scope = rememberCoroutineScope() + val memory = rememberMoveTargetMemory(targetStore, item.id) val controller = remember(item.id, sourceFolderId, repository) { - FilesMoveDestinationController(item, sourceFolderId, repository, scope) + FilesMoveDestinationController(item, sourceFolderId, repository, scope, memory.value.startPath(item)) } var finished by remember(controller) { mutableStateOf(false) } DisposableEffect(controller) { @@ -168,6 +181,7 @@ private fun MobileFilesMoveSession( current.current.content.authoritativeSessionFailure() == null) { finished = true if (onEvent(FilesBrowserEvent.Move(sourceFolderId, item.id, current.current.folder.id))) { + memory.choose(targetStore, current.path) // The session-owned browser retains the submitted operation after the picker closes. onDismiss() } else { @@ -175,5 +189,28 @@ private fun MobileFilesMoveSession( } } }, + rememberTarget = targetStore?.let { memory.value.remember }, + onRememberTargetChange = { memory.setRemember(targetStore, it) }, ) } + +/** Read once per picker, so a toggle changes where the next picker opens, as on web. */ +@Composable +private fun rememberMoveTargetMemory( + store: FilesMoveTargetStore?, + itemId: FilesItemId, +): MutableState = + remember(store, itemId) { mutableStateOf(store?.read() ?: FilesMoveTargetMemory()) } + +private fun MutableState.setRemember(store: FilesMoveTargetStore?, enabled: Boolean) { + value = value.copy(remember = enabled) + store?.write(value) +} + +private fun MutableState.choose(store: FilesMoveTargetStore?, path: List) { + val chosen = value.chosen(path) + if (chosen != value) { + value = chosen + store?.write(chosen) + } +} diff --git a/app/src/mobile/kotlin/io/putdotio/android/files/MobileMoveTargetStore.kt b/app/src/mobile/kotlin/io/putdotio/android/files/MobileMoveTargetStore.kt new file mode 100644 index 00000000..84480a6c --- /dev/null +++ b/app/src/mobile/kotlin/io/putdotio/android/files/MobileMoveTargetStore.kt @@ -0,0 +1,60 @@ +package io.putdotio.android.files + +import android.content.Context +import android.content.SharedPreferences +import androidx.core.content.edit +import org.json.JSONArray +import org.json.JSONException +import org.json.JSONObject + +/** One JSON document per user in private SharedPreferences; sign-out clears every user's. */ +internal class MobileMoveTargetStore internal constructor( + private val preferences: SharedPreferences, + private val key: String, +) : FilesMoveTargetStore { + constructor(context: Context, userId: Long) : this(preferences(context), "user-$userId") + + override fun read(): FilesMoveTargetMemory = + preferences.getString(key, null)?.let { raw -> + try { + JSONObject(raw).toMemory() + } catch (_: JSONException) { + null + } + } ?: FilesMoveTargetMemory() + + override fun write(memory: FilesMoveTargetMemory) { + preferences.edit { putString(key, memory.toJson().toString()) } + } + + companion object { + private const val PREFERENCES_NAME = "io.putdotio.android.files.move-target" + + private fun preferences(context: Context): SharedPreferences = + context.applicationContext.getSharedPreferences(PREFERENCES_NAME, Context.MODE_PRIVATE) + + fun clearAll(context: Context) { + preferences(context).edit { clear() } + } + } +} + +private fun FilesMoveTargetMemory.toJson(): JSONObject = + JSONObject() + .put("remember", remember) + .put("lastTarget", JSONArray().also { array -> + lastTarget.forEach { array.put(JSONObject().put("id", it.id.value).put("name", it.name)) } + }) + +private fun JSONObject.toMemory(): FilesMoveTargetMemory { + val path = getJSONArray("lastTarget") + val folders = (0 until path.length()).map { index -> + val folder = path.getJSONObject(index) + FilesFolder(FilesItemId(folder.getLong("id")), folder.optString("name").takeIf { folder.has("name") }) + } + // A damaged path opens at root rather than somewhere unexpected. + return FilesMoveTargetMemory( + remember = getBoolean("remember"), + lastTarget = folders.takeIf { list -> list.all { it.id.value > 0L } }.orEmpty(), + ) +} diff --git a/app/src/mobile/res/values/strings.xml b/app/src/mobile/res/values/strings.xml index e5ea931e..67d407e7 100644 --- a/app/src/mobile/res/values/strings.xml +++ b/app/src/mobile/res/values/strings.xml @@ -12,6 +12,7 @@ No folders here. No folders on this page. More folders + Remember target folder This folder is unavailable. Choose another folder. Moving… Checking the item’s location… diff --git a/app/src/test/kotlin/io/putdotio/android/files/FilesMoveDestinationControllerTest.kt b/app/src/test/kotlin/io/putdotio/android/files/FilesMoveDestinationControllerTest.kt index 733f4d4b..9c304e51 100644 --- a/app/src/test/kotlin/io/putdotio/android/files/FilesMoveDestinationControllerTest.kt +++ b/app/src/test/kotlin/io/putdotio/android/files/FilesMoveDestinationControllerTest.kt @@ -16,6 +16,7 @@ import org.junit.Test class FilesMoveDestinationControllerTest { private val source = folder(7L) private val target = folder(8L) + private val nested = folder(9L) @Test fun rootNavigationEmptyPagingRetryAndCycleGuardsKeepTheirContracts() = runBlocking { @@ -166,6 +167,101 @@ class FilesMoveDestinationControllerTest { } } + @Test + fun aRememberedPathOpensAtItsFolderWithItsCurrentNameAndBackReadsEachAncestor() = runBlocking { + val calls = mutableListOf() + val repository = object : StubFilesRepository() { + override suspend fun loadFolder(folderId: FilesItemId): FilesRepositoryResult = + error("Unexpected source read") + override suspend fun loadMoveDestinations( + folderId: FilesItemId, + cursor: FilesCursor?, + ): FilesRepositoryResult { + calls += folderId + val parent = when (folderId) { + nested.id -> nested.copy(parentId = target.id, name = "Renamed folder") + target.id -> target + else -> null + } + return FilesRepositoryResult.Success(FilesPage(emptyList(), null, parent = parent)) + } + } + val path = listOf(FilesFolder(target.id, target.name), FilesFolder(nested.id, nested.name)) + val controller = FilesMoveDestinationController(source, FilesFolder.Root.id, repository, this, path) + try { + val opened = controller.awaitState { it.current.content is FilesContent.Empty } + assertEquals(listOf(FilesFolder.Root.id, target.id, nested.id), opened.path.map { it.id }) + assertEquals("Renamed folder", opened.current.folder.name) + assertTrue(opened.canMoveHere) + assertTrue(controller.dispatch(FilesMoveDestinationEvent.NavigateBack)) + controller.awaitEmpty(target.id) + assertTrue(controller.dispatch(FilesMoveDestinationEvent.NavigateBack)) + controller.awaitEmpty(FilesFolder.Root.id) + assertEquals(listOf(nested.id, target.id, FilesFolder.Root.id), calls) + } finally { + controller.close() + } + } + + @Test + fun aRememberedFolderThatCannotBeReadReopensAtRoot() = runBlocking { + val calls = mutableListOf() + val repository = object : StubFilesRepository() { + override suspend fun loadFolder(folderId: FilesItemId): FilesRepositoryResult = + error("Unexpected source read") + override suspend fun loadMoveDestinations( + folderId: FilesItemId, + cursor: FilesCursor?, + ): FilesRepositoryResult { + calls += folderId + return if (folderId == FilesFolder.Root.id) { + FilesRepositoryResult.Success(FilesPage(listOf(target), null)) + } else { + FilesRepositoryResult.Failure(FilesFailure.Unexpected(IllegalStateException("not found"))) + } + } + } + val controller = FilesMoveDestinationController(repository, this, listOf(FilesFolder(nested.id, "Gone"))) + try { + val root = controller.awaitState { it.current.content is FilesContent.Ready } + assertEquals(listOf(FilesFolder.Root), root.path) + assertFalse(root.canNavigateBack) + assertEquals(listOf(nested.id, FilesFolder.Root.id), calls) + } finally { + controller.close() + } + } + + @Test + fun aRememberedFolderMovedElsewhereKeepsOnlyRootAboveIt() = runBlocking { + val calls = mutableListOf() + val repository = object : StubFilesRepository() { + override suspend fun loadFolder(folderId: FilesItemId): FilesRepositoryResult = + error("Unexpected source read") + override suspend fun loadMoveDestinations( + folderId: FilesItemId, + cursor: FilesCursor?, + ): FilesRepositoryResult { + calls += folderId + val parent = nested.copy(parentId = FilesItemId(42L)).takeIf { folderId == nested.id } + return FilesRepositoryResult.Success(FilesPage(emptyList(), null, parent = parent)) + } + } + val path = listOf(FilesFolder(target.id, target.name), FilesFolder(nested.id, nested.name)) + val controller = FilesMoveDestinationController(repository, this, path) + try { + val opened = controller.awaitState { it.current.content is FilesContent.Empty } + assertEquals(listOf(FilesFolder.Root.id, nested.id), opened.path.map { it.id }) + assertTrue(controller.dispatch(FilesMoveDestinationEvent.NavigateBack)) + controller.awaitEmpty(FilesFolder.Root.id) + assertEquals(listOf(nested.id, FilesFolder.Root.id), calls) + } finally { + controller.close() + } + } + + private suspend fun FilesMoveDestinationController.awaitEmpty(folderId: FilesItemId) = + awaitState { it.current.folder.id == folderId && it.current.content is FilesContent.Empty } private suspend fun FilesMoveDestinationController.awaitState(predicate: (FilesMoveDestinationState) -> Boolean) = withTimeout(5_000) { state.first(predicate) } private fun folder(id: Long) = FilesItem( diff --git a/app/src/test/kotlin/io/putdotio/android/files/FilesMoveTargetMemoryTest.kt b/app/src/test/kotlin/io/putdotio/android/files/FilesMoveTargetMemoryTest.kt new file mode 100644 index 00000000..bdef4675 --- /dev/null +++ b/app/src/test/kotlin/io/putdotio/android/files/FilesMoveTargetMemoryTest.kt @@ -0,0 +1,40 @@ +package io.putdotio.android.files + +import io.putdotio.sdk.files.PutioFileType +import org.junit.Assert.assertEquals +import org.junit.Assert.assertSame +import org.junit.Test + +class FilesMoveTargetMemoryTest { + private val folder = FilesFolder(FilesItemId(8L), "Sample folder") + private val nested = FilesFolder(FilesItemId(9L), "Archive été 東京") + + @Test + fun thePickerOpensAtTheLastTargetOnlyWhileRememberIsOn() { + val path = listOf(folder, nested) + val off = FilesMoveTargetMemory(remember = false, lastTarget = path) + assertEquals(emptyList(), off.startPath(null)) + assertEquals(path, FilesMoveTargetMemory(remember = true, lastTarget = path).startPath(null)) + assertEquals(path, FilesMoveTargetMemory(remember = true, lastTarget = path).startPath(item(7L))) + } + + @Test + fun aMoveNeverOpensInsideTheItemItMoves() { + val memory = FilesMoveTargetMemory(remember = true, lastTarget = listOf(folder, nested)) + assertEquals(emptyList(), memory.startPath(item(folder.id.value))) + assertEquals(emptyList(), memory.startPath(item(nested.id.value))) + } + + @Test + fun aChoiceIsRecordedBelowRootOnlyWhileRememberIsOn() { + val off = FilesMoveTargetMemory(remember = false, lastTarget = listOf(folder)) + assertSame(off, off.chosen(listOf(FilesFolder.Root, nested))) + val on = FilesMoveTargetMemory(remember = true, lastTarget = listOf(folder)) + assertEquals(listOf(nested), on.chosen(listOf(FilesFolder.Root, nested)).lastTarget) + assertEquals(emptyList(), on.chosen(listOf(FilesFolder.Root)).lastTarget) + } + + private fun item(id: Long) = FilesItem( + FilesItemId(id), FilesFolder.Root.id, "Moving folder", PutioFileType.FOLDER, 1L, "2026-10-01", + ) +} diff --git a/app/src/testMobile/kotlin/io/putdotio/android/MobileFilesCopyTest.kt b/app/src/testMobile/kotlin/io/putdotio/android/MobileFilesCopyTest.kt index f37ec8be..1eb6b6e4 100644 --- a/app/src/testMobile/kotlin/io/putdotio/android/MobileFilesCopyTest.kt +++ b/app/src/testMobile/kotlin/io/putdotio/android/MobileFilesCopyTest.kt @@ -5,6 +5,9 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.setValue import androidx.compose.ui.test.assertIsDisplayed import androidx.compose.ui.test.assertIsEnabled +import androidx.compose.ui.test.assertIsOff +import androidx.compose.ui.test.assertIsOn +import androidx.compose.ui.test.assertTextEquals import androidx.compose.ui.test.assertIsNotEnabled import androidx.compose.ui.test.hasAnyAncestor import androidx.compose.ui.test.hasTestTag @@ -34,6 +37,10 @@ import io.putdotio.android.files.MOBILE_FILES_COPY_ACTION_TAG import io.putdotio.android.files.MOBILE_FILES_COPY_DISMISS_TAG import io.putdotio.android.files.MOBILE_FILES_COPY_STATUS_TAG import io.putdotio.android.files.MOBILE_FILES_MOVE_CANCEL_TAG +import io.putdotio.android.files.MOBILE_FILES_MOVE_FOLDER_TAG +import io.putdotio.android.files.MOBILE_FILES_MOVE_REMEMBER_TAG +import io.putdotio.android.files.FilesMoveTargetMemory +import io.putdotio.android.files.InMemoryMoveTargetStore import io.putdotio.android.files.MOBILE_FILES_MOVE_HERE_TAG import io.putdotio.android.files.MOBILE_FILES_MOVE_PICKER_TAG import io.putdotio.android.files.MobileFilesRoute @@ -115,6 +122,38 @@ class MobileFilesCopyTest { compose.onNodeWithTag(MOBILE_FILES_COPY_ACTION_TAG).assertDoesNotExist() } + @Test + fun makeACopyOpensAtTheRememberedFolderUntilRememberIsTurnedOff() { + val remembered = listOf(FilesFolder(destination.id, destination.name)) + val store = InMemoryMoveTargetStore(FilesMoveTargetMemory(remember = true, lastTarget = remembered)) + val repository = object : StubFilesRepository() { + override suspend fun loadFolder(folderId: FilesItemId): FilesRepositoryResult = + error("Unexpected source read") + override suspend fun loadMoveDestinations(folderId: FilesItemId, cursor: FilesCursor?) = + FilesRepositoryResult.Success( + FilesPage(if (folderId == FilesFolder.Root.id) listOf(destination) else emptyList(), null), + ) + } + compose.setContent { + PutioTheme { + MobileFilesRoute(loaded(listOf(sharedVideo)), repository, { true }, {}, true, {}, + moveTargetStore = store) + } + } + compose.onNodeWithContentDescription("Actions for ${sharedVideo.name}").performClick() + compose.onNodeWithTag(MOBILE_FILES_COPY_ACTION_TAG).performClick() + compose.onNodeWithTag(MOBILE_FILES_MOVE_FOLDER_TAG).assertTextEquals(destination.name) + compose.onNodeWithTag(MOBILE_FILES_MOVE_REMEMBER_TAG).assertIsOn().performClick().assertIsOff() + compose.onNodeWithTag(MOBILE_FILES_MOVE_CANCEL_TAG).performClick() + compose.runOnIdle { assertEquals(FilesMoveTargetMemory(remember = false, lastTarget = remembered), store.memory) } + + compose.onNodeWithContentDescription("Actions for ${sharedVideo.name}").performClick() + compose.onNodeWithTag(MOBILE_FILES_COPY_ACTION_TAG).performClick() + compose.onNodeWithTag(mobileFilesMoveFolderTag(destination.id)).assertIsDisplayed() + compose.onNodeWithTag(MOBILE_FILES_MOVE_REMEMBER_TAG).assertIsOff() + compose.onNodeWithTag(MOBILE_FILES_MOVE_CANCEL_TAG).performClick() + } + @Test fun theCopyLineFollowsTheCopyAndClearsOnceSettled() { val start = FilesBrowserReducer.reduce( diff --git a/app/src/testMobile/kotlin/io/putdotio/android/MobileFilesMoveTest.kt b/app/src/testMobile/kotlin/io/putdotio/android/MobileFilesMoveTest.kt index 4e8cd5e3..c7ba33c5 100644 --- a/app/src/testMobile/kotlin/io/putdotio/android/MobileFilesMoveTest.kt +++ b/app/src/testMobile/kotlin/io/putdotio/android/MobileFilesMoveTest.kt @@ -6,6 +6,9 @@ import androidx.compose.runtime.setValue import androidx.compose.ui.semantics.SemanticsActions import androidx.compose.ui.test.assertIsDisplayed import androidx.compose.ui.test.assertIsEnabled +import androidx.compose.ui.test.assertIsOff +import androidx.compose.ui.test.assertIsOn +import androidx.compose.ui.test.assertTextEquals import androidx.compose.ui.test.assertIsNotEnabled import androidx.compose.ui.test.junit4.createComposeRule import androidx.compose.ui.test.onNodeWithContentDescription @@ -49,6 +52,10 @@ import org.robolectric.annotation.Config import org.robolectric.annotation.GraphicsMode import io.putdotio.android.files.MOBILE_FILES_MOVE_BACK_TAG import io.putdotio.android.files.MOBILE_FILES_MOVE_CANCEL_TAG +import io.putdotio.android.files.MOBILE_FILES_MOVE_FOLDER_TAG +import io.putdotio.android.files.MOBILE_FILES_MOVE_REMEMBER_TAG +import io.putdotio.android.files.FilesMoveTargetMemory +import io.putdotio.android.files.InMemoryMoveTargetStore import io.putdotio.android.files.MOBILE_FILES_MOVE_HERE_TAG import io.putdotio.android.files.MOBILE_FILES_MOVE_PICKER_TAG import io.putdotio.android.files.MOBILE_FILES_MOVE_RETRY_TAG @@ -433,6 +440,43 @@ class MobileFilesMoveTest { compose.onNodeWithTag(MOBILE_FILES_MOVE_CANCEL_TAG).performClick() } + @Test + fun rememberTargetFolderIsOffByDefaultAndOnceOnReopensMoveAtTheChosenFolder() { + val store = InMemoryMoveTargetStore() + val events = mutableListOf() + compose.setContent { + PutioTheme { + MobileFilesRoute(loadedRoot(), destinationRepository(), events::add, {}, true, {}, + moveTargetStore = store) + } + } + openMove() + compose.onNodeWithTag(MOBILE_FILES_MOVE_REMEMBER_TAG).assertIsOff() + compose.onNodeWithTag(mobileFilesMoveFolderTag(destination.id)).performClick() + compose.onNodeWithTag(MOBILE_FILES_MOVE_HERE_TAG).performClick() + compose.runOnIdle { assertEquals(FilesMoveTargetMemory(), store.memory) } + + openMove() + compose.onNodeWithTag(mobileFilesMoveFolderTag(destination.id)).assertIsDisplayed() + compose.onNodeWithTag(MOBILE_FILES_MOVE_REMEMBER_TAG).performClick().assertIsOn() + compose.onNodeWithTag(mobileFilesMoveFolderTag(destination.id)).performClick() + compose.onNodeWithTag(MOBILE_FILES_MOVE_HERE_TAG).performClick() + val remembered = FilesMoveTargetMemory(true, listOf(FilesFolder(destination.id, destination.name))) + compose.runOnIdle { assertEquals(remembered, store.memory) } + + openMove() + compose.onNodeWithTag(MOBILE_FILES_MOVE_FOLDER_TAG).assertTextEquals(destination.name) + compose.onNodeWithTag(MOBILE_FILES_MOVE_REMEMBER_TAG).assertIsOn() + compose.onNodeWithTag(MOBILE_FILES_MOVE_HERE_TAG).assertIsEnabled() + compose.onNodeWithTag(MOBILE_FILES_MOVE_BACK_TAG).performClick() + compose.onNodeWithTag(mobileFilesMoveFolderTag(destination.id)).assertIsDisplayed() + compose.onNodeWithTag(MOBILE_FILES_MOVE_CANCEL_TAG).performClick() + compose.runOnIdle { + assertEquals(2, events.filterIsInstance().size) + assertEquals(remembered, store.memory) + } + } + private fun openMove() { compose.onNodeWithContentDescription("Actions for ${source.name}").performClick() compose.onNodeWithText("Move").performClick() diff --git a/app/src/testMobile/kotlin/io/putdotio/android/auth/MobileAuthControllerTest.kt b/app/src/testMobile/kotlin/io/putdotio/android/auth/MobileAuthControllerTest.kt index 69b65038..e34e4e66 100644 --- a/app/src/testMobile/kotlin/io/putdotio/android/auth/MobileAuthControllerTest.kt +++ b/app/src/testMobile/kotlin/io/putdotio/android/auth/MobileAuthControllerTest.kt @@ -685,6 +685,21 @@ class MobileAuthControllerTest { assertNull(fixture.revocationStore.token) } + @Test + fun `sign-out and a rejected session drop what the device keeps for the account`() = runBlocking { + val fixture = Fixture(storedToken = TOKEN) + fixture.controller.restoreSession() + assertEquals(0, fixture.accountLocalStateClears) + + fixture.controller.logout() + assertEquals(1, fixture.accountLocalStateClears) + + val rejected = Fixture(storedToken = TOKEN) + rejected.controller.restoreSession() + assertTrue(rejected.controller.rejectAuthoritativeSession()) + assertEquals(1, rejected.accountLocalStateClears) + } + @Test fun `failed revocation is retried with backoff until put io confirms it`() = runBlocking { val fixture = Fixture( @@ -852,6 +867,7 @@ class MobileAuthControllerTest { val revocationStore = InMemoryAuthTokenStore(pendingRevocation?.let { checkNotNull(AccessToken.parse(it)) }) val revoker = ScriptedTokenRevoker(*revocationResults.toTypedArray()) val revocationScope = TestScope(UnconfinedTestDispatcher()) + var accountLocalStateClears = 0 val controller = MobileAuthController( oauthConfiguration = configuration, tokenStore = tokenStore, @@ -860,6 +876,7 @@ class MobileAuthControllerTest { tokenRevocations = PendingTokenRevocations(revocationStore, tokenStore, revoker, revocationScope), stateGenerator = stateGenerator, clock = clock, + clearAccountLocalState = { accountLocalStateClears += 1 }, ) } diff --git a/app/src/testMobile/kotlin/io/putdotio/android/files/InMemoryMoveTargetStore.kt b/app/src/testMobile/kotlin/io/putdotio/android/files/InMemoryMoveTargetStore.kt new file mode 100644 index 00000000..19499753 --- /dev/null +++ b/app/src/testMobile/kotlin/io/putdotio/android/files/InMemoryMoveTargetStore.kt @@ -0,0 +1,10 @@ +package io.putdotio.android.files + +internal class InMemoryMoveTargetStore(var memory: FilesMoveTargetMemory = FilesMoveTargetMemory()) : + FilesMoveTargetStore { + override fun read(): FilesMoveTargetMemory = memory + + override fun write(memory: FilesMoveTargetMemory) { + this.memory = memory + } +} diff --git a/app/src/testMobile/kotlin/io/putdotio/android/files/MobileMoveTargetStoreTest.kt b/app/src/testMobile/kotlin/io/putdotio/android/files/MobileMoveTargetStoreTest.kt new file mode 100644 index 00000000..c796231f --- /dev/null +++ b/app/src/testMobile/kotlin/io/putdotio/android/files/MobileMoveTargetStoreTest.kt @@ -0,0 +1,43 @@ +package io.putdotio.android.files + +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.junit.Assert.assertEquals +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.annotation.Config + +@RunWith(AndroidJUnit4::class) +@Config(sdk = [35]) +class MobileMoveTargetStoreTest { + private val context: Context = ApplicationProvider.getApplicationContext() + private val path = listOf(FilesFolder(FilesItemId(8L), "Sample folder"), FilesFolder(FilesItemId(9L), "Archive été 東京")) + + @Test + fun eachAccountKeepsItsOwnChoiceUntilSignOutClearsThemAll() { + val first = MobileMoveTargetStore(context, userId = 1L) + val second = MobileMoveTargetStore(context, userId = 2L) + assertEquals(FilesMoveTargetMemory(), first.read()) + + first.write(FilesMoveTargetMemory(remember = true, lastTarget = path)) + assertEquals(FilesMoveTargetMemory(remember = true, lastTarget = path), MobileMoveTargetStore(context, 1L).read()) + assertEquals(FilesMoveTargetMemory(), second.read()) + second.write(FilesMoveTargetMemory(remember = true)) + + MobileMoveTargetStore.clearAll(context) + + assertEquals(FilesMoveTargetMemory(), first.read()) + assertEquals(FilesMoveTargetMemory(), second.read()) + } + + @Test + fun aDamagedRecordOpensThePickerAtRoot() { + val preferences = context.getSharedPreferences("move-target-test", Context.MODE_PRIVATE) + val store = MobileMoveTargetStore(preferences, "user-1") + preferences.edit().putString("user-1", "{\"remember\":true,\"lastTarget\":[{\"id\":-3}]}").commit() + assertEquals(FilesMoveTargetMemory(remember = true), store.read()) + preferences.edit().putString("user-1", "not json").commit() + assertEquals(FilesMoveTargetMemory(), store.read()) + } +} diff --git a/docs/behavior.md b/docs/behavior.md index 87553a2e..0d34b480 100644 --- a/docs/behavior.md +++ b/docs/behavior.md @@ -88,8 +88,8 @@ On mobile, a friend's shared file or folder, and anything inside one, offers Make a copy, as web and iOS do; the shared root and each friend's folder offer nothing, so they have no actions button. The Files move picker chooses the destination among the viewer's own folders, root included, and -opens at root, as web's does unless its "Remember target folder" setting is -on (Android has no such setting). put.io copies in the background +opens where Move's does (see [Move and copy target folder](#move-and-copy-target-folder)). +put.io copies in the background (`POST /v2/sharing/clone`), so a line under the folder shows the copy until it is dismissed and stays across folder navigation. The app checks the copy every 1.5 s, as web does, for up to 200 checks. A finished copy reloads the @@ -107,6 +107,26 @@ Tests: `SdkFilesRepositoryTest`, `MobileFilesScreenTest`, `TvFilesScreenTest`, `FilesCopyTest`, `MobileFilesCopyTest`, `MobileSharedItemsProofTest` (opt-in synthetic device proof; see [Harness](./harness.md#shared-with-me-items-proof)). +## Move and copy target folder + +On mobile, the Move and Make a copy picker shows web's "Remember target +folder" toggle, off by default. Off, the picker opens at root. On, Move here or +Copy here records the chosen folder and its path, and the next Move or Make a +copy opens there; Back walks that path up to root, reading each folder as it +is reached. Turning the toggle off keeps the last folder, as web does, and the +toggle takes effect from the next picker. A move never opens inside the item it +moves. A remembered folder that can't be read opens the picker at root; one +moved since keeps only root above it, and a renamed one shows its new name. + +Web keeps both values in its own `/config`, which Android can't read, so +Android keeps them on the device, per signed-in account, in private +SharedPreferences. Sign-out and an expired session clear them for every +account. Android TV has no Move picker. + +Tests: `FilesMoveTargetMemoryTest`, `FilesMoveDestinationControllerTest`, +`MobileMoveTargetStoreTest`, `MobileAuthControllerTest`, `MobileFilesMoveTest`, +`MobileFilesCopyTest`. + ## Files delete and paging With the account's `trash_enabled` confirmed on, Move to trash runs from the From 29f658b6a995910870291ec26a64c8dfee708fcd Mon Sep 17 00:00:00 2001 From: Altay Date: Thu, 1 Oct 2026 10:01:58 +0300 Subject: [PATCH 2/3] test(files): add a synthetic device proof for the remembered target folder MobileMoveTargetProofTest drives the production Files route, controller and on-device store over a faked repository. FilesMoveRecoveryUiProofTest follows complete() now returning a transition. --- .../android/FilesMoveRecoveryUiProofTest.kt | 8 +- .../android/MobileMoveTargetProofTest.kt | 256 ++++++++++++++++++ docs/behavior.md | 3 +- docs/harness.md | 18 ++ 4 files changed, 280 insertions(+), 5 deletions(-) create mode 100644 app/src/androidTestMobile/kotlin/io/putdotio/android/MobileMoveTargetProofTest.kt diff --git a/app/src/androidTestMobile/kotlin/io/putdotio/android/FilesMoveRecoveryUiProofTest.kt b/app/src/androidTestMobile/kotlin/io/putdotio/android/FilesMoveRecoveryUiProofTest.kt index 682e4e4f..1be937e4 100644 --- a/app/src/androidTestMobile/kotlin/io/putdotio/android/FilesMoveRecoveryUiProofTest.kt +++ b/app/src/androidTestMobile/kotlin/io/putdotio/android/FilesMoveRecoveryUiProofTest.kt @@ -143,7 +143,7 @@ class FilesMoveRecoveryUiProofTest { assertNull(request.cursor) preview.picker = preview.picker.complete(request, FilesRepositoryResult.Success( FilesPage(listOf(preview.source), FilesCursor("synthetic-page-two")), - )) + )).state } // A one-folder page ends inside the paging margin, so the picker asks for the next page itself. val destination = preview.source.copy(id = FilesItemId(18), name = "Empty destination 東京") @@ -151,7 +151,7 @@ class FilesMoveRecoveryUiProofTest { val request = checkNotNull(preview.pickerRequest) assertEquals(FilesCursor("synthetic-page-two"), request.cursor) preview.picker = preview.picker.complete(request, - FilesRepositoryResult.Success(FilesPage(listOf(destination), null))) + FilesRepositoryResult.Success(FilesPage(listOf(destination), null))).state } assertCurrentParentAndSelfAreDisabled(preview) compose.onNodeWithTag(mobileFilesMoveFolderTag(destination.id)).performClick() @@ -159,7 +159,7 @@ class FilesMoveRecoveryUiProofTest { val request = checkNotNull(preview.pickerRequest) assertEquals(destination.id, request.folderId) preview.picker = preview.picker.complete(request, - FilesRepositoryResult.Success(FilesPage(emptyList(), null))) + FilesRepositoryResult.Success(FilesPage(emptyList(), null))).state } val context = InstrumentationRegistry.getInstrumentation().targetContext compose.onNodeWithText(context.getString(R.string.mobile_files_move_empty)).assertIsDisplayed() @@ -178,7 +178,7 @@ class FilesMoveRecoveryUiProofTest { val request = checkNotNull(preview.pickerRequest) assertEquals(preview.folder.id, request.folderId) preview.picker = preview.picker.complete(request, - FilesRepositoryResult.Success(FilesPage(listOf(preview.item), null))) + FilesRepositoryResult.Success(FilesPage(listOf(preview.item), null))).state } compose.onNodeWithTag(mobileFilesMoveFolderTag(preview.item.id)).assertIsNotEnabled() compose.onNodeWithTag(MOBILE_FILES_MOVE_HERE_TAG).assertIsNotEnabled() diff --git a/app/src/androidTestMobile/kotlin/io/putdotio/android/MobileMoveTargetProofTest.kt b/app/src/androidTestMobile/kotlin/io/putdotio/android/MobileMoveTargetProofTest.kt new file mode 100644 index 00000000..2a9e8f2d --- /dev/null +++ b/app/src/androidTestMobile/kotlin/io/putdotio/android/MobileMoveTargetProofTest.kt @@ -0,0 +1,256 @@ +package io.putdotio.android + +import android.graphics.Bitmap +import androidx.compose.foundation.layout.WindowInsets +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.safeDrawing +import androidx.compose.foundation.layout.windowInsetsPadding +import androidx.compose.material3.Surface +import androidx.compose.runtime.collectAsState +import androidx.compose.runtime.getValue +import androidx.compose.ui.Modifier +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.assertIsOff +import androidx.compose.ui.test.assertIsOn +import androidx.compose.ui.test.assertTextEquals +import androidx.compose.ui.test.junit4.v2.createComposeRule +import androidx.compose.ui.test.onAllNodesWithTag +import androidx.compose.ui.test.onNodeWithContentDescription +import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.performClick +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import io.putdotio.android.design.PutioTheme +import io.putdotio.android.files.FilesBrowserController +import io.putdotio.android.files.FilesContent +import io.putdotio.android.files.FilesCopyId +import io.putdotio.android.files.FilesCopyProgress +import io.putdotio.android.files.FilesCursor +import io.putdotio.android.files.FilesDeleteMode +import io.putdotio.android.files.FilesFailure +import io.putdotio.android.files.FilesFolder +import io.putdotio.android.files.FilesItem +import io.putdotio.android.files.FilesItemId +import io.putdotio.android.files.FilesMoveTargetMemory +import io.putdotio.android.files.FilesPage +import io.putdotio.android.files.FilesRepository +import io.putdotio.android.files.FilesRepositoryResult +import io.putdotio.android.files.FilesSort +import io.putdotio.android.files.MOBILE_FILES_COPY_ACTION_TAG +import io.putdotio.android.files.MOBILE_FILES_MOVE_BACK_TAG +import io.putdotio.android.files.MOBILE_FILES_MOVE_CANCEL_TAG +import io.putdotio.android.files.MOBILE_FILES_MOVE_FOLDER_TAG +import io.putdotio.android.files.MOBILE_FILES_MOVE_HERE_TAG +import io.putdotio.android.files.MOBILE_FILES_MOVE_REMEMBER_TAG +import io.putdotio.android.files.MobileFilesRoute +import io.putdotio.android.files.MobileMoveTargetStore +import io.putdotio.android.files.mobileFilesMoveFolderTag +import io.putdotio.sdk.files.FileMoveError +import io.putdotio.sdk.files.PutioFileType +import java.io.File +import java.util.UUID +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel +import org.junit.Assume.assumeTrue +import org.junit.Rule +import org.junit.Test +import org.junit.rules.RuleChain +import org.junit.rules.TestRule +import org.junit.runner.RunWith +import org.junit.runners.model.Statement + +/** + * Synthetic proof of "Remember target folder" through the production Files route, controller, + * move picker and on-device store over a faked repository; no API calls. + */ +@RunWith(AndroidJUnit4::class) +class MobileMoveTargetProofTest { + private val compose = createComposeRule() + private val optIn = TestRule { base, _ -> + object : Statement() { + override fun evaluate() { + assumeTrue("Synthetic move-target proof requires opt-in", + InstrumentationRegistry.getArguments().getString("putio.movetarget.enabled") == "true") + base.evaluate() + } + } + } + @get:Rule val rules: RuleChain = RuleChain.outerRule(optIn).around(compose) + + @Test + fun rememberedFolderReopensMoveAndCopyUntilSignOutAndAMissingOneOpensAtRoot() { + val context = InstrumentationRegistry.getInstrumentation().targetContext + MobileMoveTargetStore.clearAll(context) + val store = MobileMoveTargetStore(context, USER_ID) + val repository = MoveTargetRepository() + val scope = CoroutineScope(SupervisorJob() + Dispatchers.Main.immediate) + val controller = FilesBrowserController(repository, scope) + try { + compose.setContent { + val state by controller.state.collectAsState() + PutioTheme { + Surface(Modifier.fillMaxSize().windowInsetsPadding(WindowInsets.safeDrawing)) { + MobileFilesRoute( + state = state, + repository = repository, + onEvent = controller::dispatch, + onPlayMedia = {}, + confirmedTrashEnabled = true, + onAuthenticationRequired = {}, + moveTargetStore = store, + ) + } + } + } + compose.waitUntil(5_000) { controller.state.value.current.content is FilesContent.Ready } + + openPicker(CLIP.name, "Move") + awaitFolderRow(SAMPLE_FOLDER) + compose.onNodeWithTag(MOBILE_FILES_MOVE_REMEMBER_TAG).assertIsOff() + screenshot("01-move-default-root") + + compose.onNodeWithTag(MOBILE_FILES_MOVE_REMEMBER_TAG).performClick().assertIsOn() + compose.onNodeWithTag(mobileFilesMoveFolderTag(SAMPLE_FOLDER.id)).performClick() + awaitFolderRow(ARCHIVE) + compose.onNodeWithTag(mobileFilesMoveFolderTag(ARCHIVE.id)).performClick() + compose.onNodeWithText("No folders here.").assertIsDisplayed() + screenshot("02-remember-on-chosen-folder") + compose.onNodeWithTag(MOBILE_FILES_MOVE_HERE_TAG).performClick() + compose.waitUntil(5_000) { + (controller.state.value.current.content as? FilesContent.Ready)?.items?.none { it.id == CLIP.id } == true + } + check(repository.moves == listOf(CLIP.id to ARCHIVE.id)) { "Moved ${repository.moves}" } + val remembered = FilesMoveTargetMemory(true, listOf(SAMPLE_FOLDER, ARCHIVE).map { FilesFolder(it.id, it.name) }) + check(MobileMoveTargetStore(context, USER_ID).read() == remembered) { "Stored ${store.read()}" } + check(MobileMoveTargetStore(context, OTHER_USER_ID).read() == FilesMoveTargetMemory()) { "Leaked" } + + openPicker(NOTES.name, "Move") + awaitEmptyFolder() + compose.onNodeWithTag(MOBILE_FILES_MOVE_FOLDER_TAG).assertTextEquals(ARCHIVE.name) + compose.onNodeWithTag(MOBILE_FILES_MOVE_REMEMBER_TAG).assertIsOn() + screenshot("03-move-reopens-at-remembered") + compose.onNodeWithTag(MOBILE_FILES_MOVE_BACK_TAG).performClick() + awaitFolderRow(ARCHIVE) + compose.onNodeWithTag(MOBILE_FILES_MOVE_FOLDER_TAG).assertTextEquals(SAMPLE_FOLDER.name) + screenshot("04-back-reads-parent") + compose.onNodeWithTag(MOBILE_FILES_MOVE_CANCEL_TAG).performClick() + + openPicker(SHARED_VIDEO.name, null) + awaitEmptyFolder() + compose.onNodeWithTag(MOBILE_FILES_MOVE_FOLDER_TAG).assertTextEquals(ARCHIVE.name) + compose.onNodeWithText("Make a copy").assertIsDisplayed() + screenshot("05-copy-reopens-at-remembered") + compose.onNodeWithTag(MOBILE_FILES_MOVE_CANCEL_TAG).performClick() + + // The auth runtime runs this on sign-out and on an expired session. + MobileMoveTargetStore.clearAll(context) + openPicker(NOTES.name, "Move") + awaitFolderRow(SAMPLE_FOLDER) + compose.onNodeWithTag(MOBILE_FILES_MOVE_REMEMBER_TAG).assertIsOff() + screenshot("06-after-sign-out-root") + compose.onNodeWithTag(MOBILE_FILES_MOVE_CANCEL_TAG).performClick() + + store.write(FilesMoveTargetMemory(true, listOf(FilesFolder(REMOVED_ID, "Removed folder")))) + openPicker(NOTES.name, "Move") + awaitFolderRow(SAMPLE_FOLDER) + compose.onNodeWithTag(MOBILE_FILES_MOVE_REMEMBER_TAG).assertIsOn() + check(REMOVED_ID in repository.listed) { "Never read the removed folder" } + screenshot("07-missing-folder-root") + compose.onNodeWithTag(MOBILE_FILES_MOVE_CANCEL_TAG).performClick() + } finally { + controller.close() + scope.cancel() + MobileMoveTargetStore.clearAll(context) + } + } + + private fun openPicker(itemName: String, action: String?) { + compose.onNodeWithContentDescription("Actions for $itemName").performClick() + if (action == null) { + compose.onNodeWithTag(MOBILE_FILES_COPY_ACTION_TAG).performClick() + } else { + compose.onNodeWithText(action).performClick() + } + } + + private fun awaitFolderRow(folder: FilesItem) = compose.waitUntil(5_000) { + compose.onAllNodesWithTag(mobileFilesMoveFolderTag(folder.id)).fetchSemanticsNodes().isNotEmpty() + } + + private fun awaitEmptyFolder() = compose.waitUntil(5_000) { + compose.onAllNodesWithTag(MOBILE_FILES_MOVE_HERE_TAG).fetchSemanticsNodes().isNotEmpty() && + runCatching { compose.onNodeWithText("No folders here.").assertIsDisplayed() }.isSuccess + } + + private fun screenshot(label: String) { + val instrumentation = InstrumentationRegistry.getInstrumentation() + val runId = UUID.fromString( + requireNotNull(InstrumentationRegistry.getArguments().getString("putio.movetarget.runId")), + ) + val directory = File( + requireNotNull(instrumentation.targetContext.getExternalFilesDir(null)), "move-target-proof-$runId", + ) + check(directory.mkdirs() || directory.isDirectory) + compose.waitForIdle() + instrumentation.uiAutomation.waitForIdle(100, 3_000) + val bitmap = requireNotNull(instrumentation.uiAutomation.takeScreenshot()) + try { + File(directory, "$label.png").outputStream().use { check(bitmap.compress(Bitmap.CompressFormat.PNG, 100, it)) } + } finally { bitmap.recycle() } + } +} + +private const val USER_ID = 900_001L +private const val OTHER_USER_ID = 900_002L +private val REMOVED_ID = FilesItemId(99L) + +private fun item(id: Long, name: String, type: PutioFileType, parent: Long = 0L) = + FilesItem(FilesItemId(id), FilesItemId(parent), name, type, 128_000_000, "2026-10-01T12:00:00Z") + +private val SAMPLE_FOLDER = item(20, "Sample folder", PutioFileType.FOLDER) +private val ARCHIVE = item(21, "Archive été 東京", PutioFileType.FOLDER, parent = 20) +private val CLIP = item(14, "Sample clip.mp4", PutioFileType.VIDEO) +private val NOTES = item(15, "Sample notes.txt", PutioFileType.TEXT) +private val SHARED_VIDEO = item(13, "Harbor film.mp4", PutioFileType.VIDEO).copy(isShared = true) + +private class MoveTargetRepository : FilesRepository { + val moves = mutableListOf>() + val listed = mutableListOf() + + override suspend fun loadFolder(folderId: FilesItemId) = FilesRepositoryResult.Success(FilesPage( + listOf(SAMPLE_FOLDER, SHARED_VIDEO, NOTES) + listOf(CLIP).filter { clip -> moves.none { it.first == clip.id } }, + null, + )) + + override suspend fun loadMoveDestinations(folderId: FilesItemId, cursor: FilesCursor?): FilesRepositoryResult { + listed += folderId + return when (folderId) { + FilesFolder.Root.id -> FilesRepositoryResult.Success(FilesPage(listOf(SAMPLE_FOLDER), null)) + SAMPLE_FOLDER.id -> FilesRepositoryResult.Success(FilesPage(listOf(ARCHIVE), null, parent = SAMPLE_FOLDER)) + ARCHIVE.id -> FilesRepositoryResult.Success(FilesPage(emptyList(), null, parent = ARCHIVE)) + else -> FilesRepositoryResult.Failure(FilesFailure.Unexpected(IllegalStateException("No such folder"))) + } + } + + override suspend fun move( + itemId: FilesItemId, + destinationId: FilesItemId, + ): FilesRepositoryResult> { + moves += itemId to destinationId + return FilesRepositoryResult.Success(emptyList()) + } + + override suspend fun resolveItem(itemId: FilesItemId) = + FilesRepositoryResult.Success(CLIP.copy(parentId = moves.last { it.first == itemId }.second)) + + override suspend fun startCopy(itemId: FilesItemId, destinationId: FilesItemId): FilesRepositoryResult = + error("No copy") + override suspend fun checkCopy(copyId: FilesCopyId): FilesRepositoryResult = error("No copy") + override suspend fun loadNextPage(cursor: FilesCursor) = error("No paging") + override suspend fun persistSort(folderId: FilesItemId, sort: FilesSort) = error("No sort") + override suspend fun rename(itemId: FilesItemId, name: String) = error("No rename") + override suspend fun delete(itemId: FilesItemId, mode: FilesDeleteMode) = error("No delete") +} diff --git a/docs/behavior.md b/docs/behavior.md index 0d34b480..84245088 100644 --- a/docs/behavior.md +++ b/docs/behavior.md @@ -125,7 +125,8 @@ account. Android TV has no Move picker. Tests: `FilesMoveTargetMemoryTest`, `FilesMoveDestinationControllerTest`, `MobileMoveTargetStoreTest`, `MobileAuthControllerTest`, `MobileFilesMoveTest`, -`MobileFilesCopyTest`. +`MobileFilesCopyTest`, `MobileMoveTargetProofTest` (opt-in synthetic device +proof; see [Harness](./harness.md#move-and-copy-target-folder-proof)). ## Files delete and paging diff --git a/docs/harness.md b/docs/harness.md index 159b82d2..359e3aa0 100644 --- a/docs/harness.md +++ b/docs/harness.md @@ -1214,6 +1214,24 @@ require `OK (1 test)`. Screenshots land in `shared-proof-/`: - `05-copied`: the line after the faked check reports done - `06-owned-file-actions`: Rename, Move, Move to trash, and no Make a copy +## Move and copy target folder proof + +Behaviour: [Move and copy target folder](./behavior.md#move-and-copy-target-folder). +`MobileMoveTargetProofTest` mounts the production Files route and controller +on a faked repository with the on-device `MobileMoveTargetStore`, which it +clears before and after the run. It makes no API calls, so report it as +synthetic proof. Opt in with `putio.movetarget.enabled=true` and +`putio.movetarget.runId=` and require `OK (1 test)`. Screenshots land +in `move-target-proof-/`: + +- `01-move-default-root`: the picker at root with the toggle off +- `02-remember-on-chosen-folder`: the toggle on, at a nested folder +- `03-move-reopens-at-remembered`: the next Move opens there +- `04-back-reads-parent`: Back reads the folder above it +- `05-copy-reopens-at-remembered`: Make a copy opens there too +- `06-after-sign-out-root`: after the sign-out cleanup, root with the toggle off +- `07-missing-folder-root`: a remembered folder that can't be read opens at root + ## Transfer retry proof Behaviour: [Transfer failures and retry](./behavior.md#transfer-failures-and-retry). From 2fc01cf8ec10225747e8071ecdf322276306c0e9 Mon Sep 17 00:00:00 2001 From: Altay Date: Thu, 1 Oct 2026 10:36:52 +0300 Subject: [PATCH 3/3] fix(files): check each remembered folder when it is first read Keep a rejected session visible instead of falling back to root, take a remembered ancestor's current name and place when Back reads it, and reopen at root when a remembered folder now sits directly inside the item being moved. --- .../files/FilesMoveDestinationController.kt | 1 + .../files/FilesMoveDestinationStart.kt | 59 +++++++++----- .../files/FilesMoveDestinationState.kt | 6 +- .../FilesMoveDestinationControllerTest.kt | 78 +++++++++++++++++++ .../putdotio/android/MobileFilesCopyTest.kt | 32 ++++++++ docs/behavior.md | 11 ++- 6 files changed, 163 insertions(+), 24 deletions(-) diff --git a/app/src/main/kotlin/io/putdotio/android/files/FilesMoveDestinationController.kt b/app/src/main/kotlin/io/putdotio/android/files/FilesMoveDestinationController.kt index be4dc03e..33744baf 100644 --- a/app/src/main/kotlin/io/putdotio/android/files/FilesMoveDestinationController.kt +++ b/app/src/main/kotlin/io/putdotio/android/files/FilesMoveDestinationController.kt @@ -56,6 +56,7 @@ class FilesMoveDestinationController private constructor( FilesMoveDestinationFolder( startPath.lastOrNull() ?: FilesFolder.Root, FilesContent.Loading(firstRequest.requestId), + remembered = startPath.isNotEmpty(), ), nextRequestValue = 2L, opensRememberedTarget = startPath.isNotEmpty(), diff --git a/app/src/main/kotlin/io/putdotio/android/files/FilesMoveDestinationStart.kt b/app/src/main/kotlin/io/putdotio/android/files/FilesMoveDestinationStart.kt index e36aeb51..84d3e050 100644 --- a/app/src/main/kotlin/io/putdotio/android/files/FilesMoveDestinationStart.kt +++ b/app/src/main/kotlin/io/putdotio/android/files/FilesMoveDestinationStart.kt @@ -4,34 +4,55 @@ internal fun FilesMoveDestinationState.withoutRememberedTargetCheck() = if (opensRememberedTarget) copy(opensRememberedTarget = false) else this /** - * Web reopens at root when the remembered folder can't be read. A folder read since it was - * remembered takes its current name, and one moved elsewhere keeps only root above it. + * Web reopens at root when the remembered folder can't be read; a rejected session stays visible so + * the app signs out. Null leaves the read to the ordinary listing. */ -internal fun FilesMoveDestinationState.completeRememberedTarget( +internal fun FilesMoveDestinationState.completeRememberedRead( request: FilesMoveDestinationRequest, result: FilesRepositoryResult, +): FilesMoveDestinationTransition? = when (result) { + is FilesRepositoryResult.Failure -> + if (opensRememberedTarget && result.failure !is FilesFailure.AuthenticationRequired) restartAtRoot() else null + is FilesRepositoryResult.Success -> + if (current.remembered && request.cursor == null) reconcileRemembered(request, result) else null +} + +/** + * Each remembered folder is checked when first read: it takes its current name, one moved elsewhere + * keeps only root above it, and one now directly inside the item being moved restarts at root. + */ +private fun FilesMoveDestinationState.reconcileRemembered( + request: FilesMoveDestinationRequest, + result: FilesRepositoryResult.Success, ): FilesMoveDestinationTransition { - val checked = copy(opensRememberedTarget = false) - if (result !is FilesRepositoryResult.Success) { - val requestId = FilesRequestId(nextRequestValue) - return FilesMoveDestinationTransition( - checked.copy( - stack = listOf(FilesMoveDestinationFolder(FilesFolder.Root, FilesContent.Loading(requestId))), - nextRequestValue = nextRequestValue + 1, - ), - FilesMoveDestinationRequest(FilesFolder.Root.id, requestId), - ) - } val listed = result.value.parent?.takeIf { it.id == current.folder.id } - val target = current.copy(folder = current.folder.copy(name = listed?.name ?: current.folder.name)) + if (sourceItem != null && listed?.parentId == sourceItem.id) return restartAtRoot() + val read = current.copy( + folder = current.folder.copy(name = listed?.name ?: current.folder.name), + remembered = false, + ) val ancestors = stack.dropLast(1) - val placed = listed?.parentId == null || listed.parentId == ancestors.last().folder.id - return FilesMoveDestinationTransition(checked.copy( - stack = (if (placed) ancestors else listOf(unreadFolder(FilesFolder.Root))) + target, + val placed = ancestors.isEmpty() || listed?.parentId == null || listed.parentId == ancestors.last().folder.id + return FilesMoveDestinationTransition(withoutRememberedTargetCheck().copy( + stack = (if (placed) ancestors else listOf(unreadFolder(FilesFolder.Root))) + read, ).completeListing(request, result)) } -internal fun unreadFolder(folder: FilesFolder) = FilesMoveDestinationFolder(folder, FilesContent.Loading(UNREAD)) +private fun FilesMoveDestinationState.restartAtRoot(): FilesMoveDestinationTransition { + val requestId = FilesRequestId(nextRequestValue) + return FilesMoveDestinationTransition( + copy( + stack = listOf(FilesMoveDestinationFolder(FilesFolder.Root, FilesContent.Loading(requestId))), + nextRequestValue = nextRequestValue + 1, + opensRememberedTarget = false, + ), + FilesMoveDestinationRequest(FilesFolder.Root.id, requestId), + ) +} + +/** Root keeps its localized heading, so it is never checked against a listing. */ +internal fun unreadFolder(folder: FilesFolder) = + FilesMoveDestinationFolder(folder, FilesContent.Loading(UNREAD), remembered = folder.id != FilesFolder.Root.id) /** Never issued: request IDs start at 1. */ private val UNREAD = FilesRequestId(0L) diff --git a/app/src/main/kotlin/io/putdotio/android/files/FilesMoveDestinationState.kt b/app/src/main/kotlin/io/putdotio/android/files/FilesMoveDestinationState.kt index 86ae83c2..b79c46e7 100644 --- a/app/src/main/kotlin/io/putdotio/android/files/FilesMoveDestinationState.kt +++ b/app/src/main/kotlin/io/putdotio/android/files/FilesMoveDestinationState.kt @@ -5,6 +5,8 @@ data class FilesMoveDestinationFolder internal constructor( val folder: FilesFolder, val content: FilesContent, internal val consumedCursors: Set = emptySet(), + /** Taken from a remembered path and not yet read; its first read checks its name and place. */ + internal val remembered: Boolean = false, ) /** A folder picker; without a [sourceItem] it picks a destination for new content, such as a transfer. */ @@ -120,8 +122,8 @@ internal fun FilesMoveDestinationState.complete( ): FilesMoveDestinationTransition = when { current.folder.id != request.folderId || current.requestId() != request.requestId -> FilesMoveDestinationTransition(this) - opensRememberedTarget -> completeRememberedTarget(request, result) - else -> FilesMoveDestinationTransition(completeListing(request, result)) + else -> completeRememberedRead(request, result) + ?: FilesMoveDestinationTransition(withoutRememberedTargetCheck().completeListing(request, result)) } internal fun FilesMoveDestinationState.completeListing( diff --git a/app/src/test/kotlin/io/putdotio/android/files/FilesMoveDestinationControllerTest.kt b/app/src/test/kotlin/io/putdotio/android/files/FilesMoveDestinationControllerTest.kt index 9c304e51..7a694356 100644 --- a/app/src/test/kotlin/io/putdotio/android/files/FilesMoveDestinationControllerTest.kt +++ b/app/src/test/kotlin/io/putdotio/android/files/FilesMoveDestinationControllerTest.kt @@ -1,5 +1,6 @@ package io.putdotio.android.files +import io.putdotio.sdk.errors.PutioConfigurationException import io.putdotio.sdk.files.PutioFileType import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.NonCancellable @@ -260,6 +261,83 @@ class FilesMoveDestinationControllerTest { } } + @Test + fun aRememberedAncestorReadOnBackTakesItsCurrentNameAndPlace() = runBlocking { + val deeper = folder(10L) + val calls = mutableListOf() + val repository = destinations(calls) { folderId -> + val parent = when (folderId) { + deeper.id -> deeper.copy(parentId = nested.id) + nested.id -> nested.copy(parentId = FilesItemId(42L), name = "Renamed folder") + else -> null + } + FilesRepositoryResult.Success(FilesPage(emptyList(), null, parent = parent)) + } + val path = listOf(target, nested, deeper).map { FilesFolder(it.id, it.name) } + val controller = FilesMoveDestinationController(source, FilesFolder.Root.id, repository, this, path) + try { + controller.awaitEmpty(deeper.id) + assertTrue(controller.dispatch(FilesMoveDestinationEvent.NavigateBack)) + val back = controller.awaitEmpty(nested.id) + assertEquals(listOf(FilesFolder.Root.id, nested.id), back.path.map { it.id }) + assertEquals("Renamed folder", back.current.folder.name) + assertTrue(controller.dispatch(FilesMoveDestinationEvent.NavigateBack)) + controller.awaitEmpty(FilesFolder.Root.id) + assertEquals(listOf(deeper.id, nested.id, FilesFolder.Root.id), calls) + } finally { + controller.close() + } + } + + @Test + fun aRememberedFolderNowInsideTheMovedItemReopensAtRoot() = runBlocking { + val calls = mutableListOf() + val repository = destinations(calls) { folderId -> + val parent = nested.copy(parentId = source.id).takeIf { folderId == nested.id } + FilesRepositoryResult.Success(FilesPage(emptyList(), null, parent = parent)) + } + val path = listOf(FilesFolder(target.id, target.name), FilesFolder(nested.id, nested.name)) + val controller = FilesMoveDestinationController(source, FilesFolder.Root.id, repository, this, path) + try { + val root = controller.awaitEmpty(FilesFolder.Root.id) + assertEquals(listOf(FilesFolder.Root), root.path) + assertEquals(listOf(nested.id, FilesFolder.Root.id), calls) + } finally { + controller.close() + } + } + + @Test + fun aRejectedSessionOpeningARememberedFolderStaysVisible() = runBlocking { + val rejected = FilesFailure.AuthenticationRequired(PutioConfigurationException("expired")) + val calls = mutableListOf() + val repository = destinations(calls) { FilesRepositoryResult.Failure(rejected) } + val controller = FilesMoveDestinationController(repository, this, listOf(FilesFolder(nested.id, nested.name))) + try { + val failed = controller.awaitState { it.current.content is FilesContent.Failed } + assertEquals(nested.id, failed.current.folder.id) + assertSame(rejected, failed.current.content.authoritativeSessionFailure()) + assertEquals(listOf(nested.id), calls) + } finally { + controller.close() + } + } + + private fun destinations( + calls: MutableList, + respond: (FilesItemId) -> FilesRepositoryResult, + ) = object : StubFilesRepository() { + override suspend fun loadFolder(folderId: FilesItemId): FilesRepositoryResult = + error("Unexpected source read") + override suspend fun loadMoveDestinations( + folderId: FilesItemId, + cursor: FilesCursor?, + ): FilesRepositoryResult { + calls += folderId + return respond(folderId) + } + } + private suspend fun FilesMoveDestinationController.awaitEmpty(folderId: FilesItemId) = awaitState { it.current.folder.id == folderId && it.current.content is FilesContent.Empty } private suspend fun FilesMoveDestinationController.awaitState(predicate: (FilesMoveDestinationState) -> Boolean) = diff --git a/app/src/testMobile/kotlin/io/putdotio/android/MobileFilesCopyTest.kt b/app/src/testMobile/kotlin/io/putdotio/android/MobileFilesCopyTest.kt index 1eb6b6e4..93ab3489 100644 --- a/app/src/testMobile/kotlin/io/putdotio/android/MobileFilesCopyTest.kt +++ b/app/src/testMobile/kotlin/io/putdotio/android/MobileFilesCopyTest.kt @@ -154,6 +154,38 @@ class MobileFilesCopyTest { compose.onNodeWithTag(MOBILE_FILES_MOVE_CANCEL_TAG).performClick() } + @Test + fun copyHereWithRememberOnRecordsTheFolderTheNextPickerOpensAt() { + val store = InMemoryMoveTargetStore(FilesMoveTargetMemory(remember = true)) + val repository = object : StubFilesRepository() { + override suspend fun loadFolder(folderId: FilesItemId): FilesRepositoryResult = + error("Unexpected source read") + override suspend fun loadMoveDestinations(folderId: FilesItemId, cursor: FilesCursor?) = + FilesRepositoryResult.Success( + FilesPage(if (folderId == FilesFolder.Root.id) listOf(destination) else emptyList(), null), + ) + } + compose.setContent { + PutioTheme { + MobileFilesRoute(loaded(listOf(sharedVideo)), repository, { true }, {}, true, {}, + moveTargetStore = store) + } + } + compose.onNodeWithContentDescription("Actions for ${sharedVideo.name}").performClick() + compose.onNodeWithTag(MOBILE_FILES_COPY_ACTION_TAG).performClick() + compose.onNodeWithTag(mobileFilesMoveFolderTag(destination.id)).performClick() + compose.onNodeWithText("No folders here.").assertIsDisplayed() + compose.onNodeWithTag(MOBILE_FILES_MOVE_HERE_TAG).performClick() + compose.onNodeWithTag(MOBILE_FILES_MOVE_PICKER_TAG).assertDoesNotExist() + val chosen = listOf(FilesFolder(destination.id, destination.name)) + compose.runOnIdle { assertEquals(FilesMoveTargetMemory(remember = true, lastTarget = chosen), store.memory) } + + compose.onNodeWithContentDescription("Actions for ${sharedVideo.name}").performClick() + compose.onNodeWithTag(MOBILE_FILES_COPY_ACTION_TAG).performClick() + compose.onNodeWithTag(MOBILE_FILES_MOVE_FOLDER_TAG).assertTextEquals(destination.name) + compose.onNodeWithTag(MOBILE_FILES_MOVE_CANCEL_TAG).performClick() + } + @Test fun theCopyLineFollowsTheCopyAndClearsOnceSettled() { val start = FilesBrowserReducer.reduce( diff --git a/docs/behavior.md b/docs/behavior.md index 84245088..859ef588 100644 --- a/docs/behavior.md +++ b/docs/behavior.md @@ -114,9 +114,14 @@ folder" toggle, off by default. Off, the picker opens at root. On, Move here or Copy here records the chosen folder and its path, and the next Move or Make a copy opens there; Back walks that path up to root, reading each folder as it is reached. Turning the toggle off keeps the last folder, as web does, and the -toggle takes effect from the next picker. A move never opens inside the item it -moves. A remembered folder that can't be read opens the picker at root; one -moved since keeps only root above it, and a renamed one shows its new name. +toggle takes effect from the next picker. A remembered folder that can't be +read opens the picker at root, unless put.io rejected the session, which signs +out as anywhere else. Each remembered folder is checked when it is first read: +a renamed one shows its new name, and one moved since keeps only root above it. +A move never opens inside the item it moves: a remembered path through that +item opens at root, as does a remembered folder whose parent is now that item. +A remembered folder nested deeper inside that item is not caught before the +move request goes out. Web keeps both values in its own `/config`, which Android can't read, so Android keeps them on the device, per signed-in account, in private