From 7abd74ad88168ad4534f3b1603a6d31d5fe2a27b Mon Sep 17 00:00:00 2001 From: Nik Clayton Date: Sat, 26 Oct 2024 22:19:14 +0200 Subject: [PATCH] fix: Don't lose images / captions when editing with failed uploads Previous code would remove image attachments from the compose editor if there was a problem uploading or updating them. This caused a particular problem with image captions. You could attach a valid image, then write a caption that was too long for the server. The server would reject the status, and the status was saved to drafts. Then you open the draft, which tries to upload the image again with a too-long caption. The upload is rejected, and the image, along with the caption, is removed. Fix this. - Change `QueuedMedia` to track the upload state as a `Result<_,_>`, so any error messages are preserved and available to the UI. - The different `Ok` types for the upload state contain the upload progress percentage (if appropriate) or the server's ID for the uploaded media. - Change `ProgressImageView` to accept the upload state `Result`. If the result is an error the image is drawn with a red overlay and white "error" icon. - If an upload is in an error state allow the user to click on it. That shows a dialog explaining the error, and provides options to edit the image, change the caption, etc. - When changing the caption make the API call to change it on the server (if the attachment has been uploaded). This makes the user aware of any errors sooner in the process, so they can correct them. --- .../components/compose/ComposeActivity.kt | 82 ++++++++------ .../components/compose/ComposeViewModel.kt | 91 ++++++++------- .../components/compose/MediaPreviewAdapter.kt | 49 ++++++-- .../components/compose/MediaUploader.kt | 107 +++++++++++------- .../compose/dialog/CaptionDialog.kt | 11 +- .../compose/view/ProgressImageView.kt | 59 ++++++++-- .../app/pachli/service/SendStatusService.kt | 26 ++--- app/src/main/res/values/strings.xml | 3 + .../core/network/retrofit/MastodonApi.kt | 8 +- core/network/src/main/res/values/strings.xml | 4 +- 10 files changed, 271 insertions(+), 169 deletions(-) diff --git a/app/src/main/java/app/pachli/components/compose/ComposeActivity.kt b/app/src/main/java/app/pachli/components/compose/ComposeActivity.kt index 45904447b..ec6bf31af 100644 --- a/app/src/main/java/app/pachli/components/compose/ComposeActivity.kt +++ b/app/src/main/java/app/pachli/components/compose/ComposeActivity.kt @@ -109,7 +109,9 @@ import app.pachli.util.setDrawableTint import com.canhub.cropper.CropImage import com.canhub.cropper.CropImageContract import com.canhub.cropper.options +import com.github.michaelbull.result.Result import com.github.michaelbull.result.getOrElse +import com.github.michaelbull.result.mapBoth import com.github.michaelbull.result.onFailure import com.google.android.material.bottomsheet.BottomSheetBehavior import com.google.android.material.color.MaterialColors @@ -253,7 +255,7 @@ class ComposeActivity : val mediaAdapter = MediaPreviewAdapter( this, onAddCaption = { item -> - CaptionDialog.newInstance(item.localId, item.description, item.uri).show(supportFragmentManager, "caption_dialog") + CaptionDialog.newInstance(item.localId, item.serverId, item.description, item.uri).show(supportFragmentManager, "caption_dialog") }, onAddFocus = { item -> makeFocusDialog(item.focus, item.uri) { newFocus -> @@ -524,14 +526,6 @@ class ComposeActivity : enablePollButton(media.isEmpty()) }.collect() } - - lifecycleScope.launch { - viewModel.uploadError.collect { mediaUploaderError -> - val message = mediaUploaderError.fmt(this@ComposeActivity) - - displayPermamentMessage(getString(R.string.error_media_upload_sending_fmt, message)) - } - } } /** @return List of states of the different bottomsheets */ @@ -1272,14 +1266,8 @@ class ComposeActivity : * User is editing a new post, and can either save the changes as a draft or discard them. */ private fun getSaveAsDraftOrDiscardDialog(contentText: String, contentWarning: String): AlertDialog.Builder { - val warning = if (viewModel.media.value.isNotEmpty()) { - R.string.compose_save_draft_loses_media - } else { - R.string.compose_save_draft - } - - return AlertDialog.Builder(this) - .setMessage(warning) + val builder = AlertDialog.Builder(this) + .setTitle(R.string.compose_save_draft) .setPositiveButton(R.string.action_save) { _, _ -> viewModel.stopUploads() saveDraftAndFinish(contentText, contentWarning) @@ -1288,6 +1276,12 @@ class ComposeActivity : viewModel.stopUploads() deleteDraftAndFinish() } + + if (viewModel.media.value.isNotEmpty()) { + builder.setMessage(R.string.compose_save_draft_loses_media) + } + + return builder } /** @@ -1295,14 +1289,8 @@ class ComposeActivity : * discard them. */ private fun getUpdateDraftOrDiscardDialog(contentText: String, contentWarning: String): AlertDialog.Builder { - val warning = if (viewModel.media.value.isNotEmpty()) { - R.string.compose_save_draft_loses_media - } else { - R.string.compose_save_draft - } - - return AlertDialog.Builder(this) - .setMessage(warning) + val builder = AlertDialog.Builder(this) + .setTitle(R.string.compose_save_draft) .setPositiveButton(R.string.action_save) { _, _ -> viewModel.stopUploads() saveDraftAndFinish(contentText, contentWarning) @@ -1311,6 +1299,12 @@ class ComposeActivity : viewModel.stopUploads() finish() } + + if (viewModel.media.value.isNotEmpty()) { + builder.setMessage(R.string.compose_save_draft_loses_media) + } + + return builder } /** @@ -1392,23 +1386,39 @@ class ComposeActivity : val uri: Uri, val type: Type, val mediaSize: Long, - val uploadPercent: Int = 0, - val id: String? = null, val description: String? = null, val focus: Attachment.Focus? = null, - val state: State, + val uploadState: Result, ) { enum class Type { IMAGE, VIDEO, AUDIO, } - enum class State { - UPLOADING, - UNPROCESSED, - PROCESSED, - PUBLISHED, - } + + /** + * Server's ID for this attachment. May be null if the media is still + * being uploaded, or it was uploaded and there was an error that + * meant it couldn't be processed. Attachments that have an error + * *after* processing have a non-null `serverId`. + */ + val serverId: String? + get() = uploadState.mapBoth( + { state -> + when (state) { + is UploadState.Uploading -> null + is UploadState.Uploaded.Processing -> state.serverId + is UploadState.Uploaded.Processed -> state.serverId + is UploadState.Uploaded.Published -> state.serverId + } + }, + { error -> + when (error) { + is MediaUploaderError.UpdateMediaError -> error.serverId + else -> null + } + }, + ) } override fun onTimeSet(time: Date?) { @@ -1425,8 +1435,8 @@ class ComposeActivity : scheduleBehavior.state = BottomSheetBehavior.STATE_HIDDEN } - override fun onUpdateDescription(localId: Int, description: String) { - viewModel.updateDescription(localId, description) + override fun onUpdateDescription(localId: Int, serverId: String?, description: String) { + viewModel.updateDescription(localId, serverId, description) } companion object { diff --git a/app/src/main/java/app/pachli/components/compose/ComposeViewModel.kt b/app/src/main/java/app/pachli/components/compose/ComposeViewModel.kt index 94534f5f6..87a862eaf 100644 --- a/app/src/main/java/app/pachli/components/compose/ComposeViewModel.kt +++ b/app/src/main/java/app/pachli/components/compose/ComposeViewModel.kt @@ -28,6 +28,7 @@ import androidx.lifecycle.viewModelScope import app.pachli.R import app.pachli.components.compose.ComposeActivity.QueuedMedia import app.pachli.components.compose.ComposeAutoCompleteAdapter.AutocompleteResult +import app.pachli.components.compose.UploadState.Uploaded import app.pachli.components.drafts.DraftHelper import app.pachli.components.search.SearchType import app.pachli.core.common.PachliError @@ -52,20 +53,20 @@ import at.connyduck.calladapter.networkresult.fold import com.github.michaelbull.result.Err import com.github.michaelbull.result.Ok import com.github.michaelbull.result.Result +import com.github.michaelbull.result.andThen import com.github.michaelbull.result.get import com.github.michaelbull.result.getOrElse import com.github.michaelbull.result.mapBoth import com.github.michaelbull.result.mapError +import com.github.michaelbull.result.onFailure +import com.github.michaelbull.result.onSuccess import dagger.hilt.android.lifecycle.HiltViewModel import io.github.z4kn4fein.semver.constraints.toConstraint import java.util.Date import javax.inject.Inject import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.channels.BufferOverflow -import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted -import kotlinx.coroutines.flow.asSharedFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn @@ -144,8 +145,6 @@ class ComposeViewModel @Inject constructor( private val _media: MutableStateFlow> = MutableStateFlow(emptyList()) val media = _media.asStateFlow() - private val _uploadError = MutableSharedFlow(replay = 0, extraBufferCapacity = 1, onBufferOverflow = BufferOverflow.DROP_OLDEST) - val uploadError = _uploadError.asSharedFlow() private val _closeConfirmation = MutableStateFlow(ConfirmationKind.NONE) val closeConfirmation = _closeConfirmation.asStateFlow() private val _statusLength = MutableStateFlow(0) @@ -227,7 +226,7 @@ class ComposeViewModel @Inject constructor( mediaSize = mediaSize, description = description, focus = focus, - state = QueuedMedia.State.UPLOADING, + uploadState = Ok(UploadState.Uploading(percentage = 0)), ) stashMediaItem = mediaItem @@ -245,35 +244,8 @@ class ComposeViewModel @Inject constructor( viewModelScope.launch { mediaUploader .uploadMedia(mediaItem, instanceInfo.value) - .collect { event -> - val item = media.value.find { it.localId == mediaItem.localId } - ?: return@collect - var newMediaItem: QueuedMedia? = null - val uploadEvent = event.getOrElse { - _media.update { mediaList -> mediaList.filter { it.localId != mediaItem.localId } } - _uploadError.emit(it) - return@collect - } - - newMediaItem = when (uploadEvent) { - is UploadEvent.ProgressEvent -> item.copy(uploadPercent = uploadEvent.percentage) - is UploadEvent.FinishedEvent -> { - item.copy( - id = uploadEvent.media.mediaId, - uploadPercent = -1, - state = if (uploadEvent.media.processed) { - QueuedMedia.State.PROCESSED - } else { - QueuedMedia.State.UNPROCESSED - }, - ) - } - } - newMediaItem.let { - _media.update { mediaList -> - mediaList.map { mediaItem -> if (mediaItem.localId == it.localId) it else mediaItem } - } - } + .collect { uploadResult -> + updateMediaItem(mediaItem.localId) { it.copy(uploadState = uploadResult) } } } @@ -288,11 +260,9 @@ class ComposeViewModel @Inject constructor( uri = uri, type = type, mediaSize = 0, - uploadPercent = -1, - id = id, description = description, focus = focus, - state = QueuedMedia.State.PUBLISHED, + uploadState = Ok(Uploaded.Published(id)), ) mediaList + mediaItem } @@ -457,11 +427,11 @@ class ComposeViewModel @Inject constructor( val attachedMedia = media.value.map { item -> MediaToSend( localId = item.localId, - id = item.id, + id = item.serverId, uri = item.uri.toString(), description = item.description, focus = item.focus, - processed = item.state == QueuedMedia.State.PROCESSED || item.state == QueuedMedia.State.PUBLISHED, + processed = item.uploadState.get() is Uploaded.Processed || item.uploadState.get() is Uploaded.Published, ) } val tootToSend = StatusToSend( @@ -498,16 +468,45 @@ class ComposeViewModel @Inject constructor( } } - fun updateDescription(localId: Int, description: String) { - updateMediaItem(localId) { mediaItem -> - mediaItem.copy(description = description) + fun updateDescription(localId: Int, serverId: String?, description: String) { + // If the image hasn't been uploaded then update the state locally. + if (serverId == null) { + updateMediaItem(localId) { mediaItem -> mediaItem.copy(description = description) } + return + } + + // Update the remote description and report any errors. Update the local description + // if there are errors so the user still has the text and can try and correct it. + viewModelScope.launch { + api.updateMedia(serverId, description = description) + .andThen { api.getMedia(serverId) } + .onSuccess { response -> + val state = if (response.code == 200) { + Uploaded.Processed(serverId) + } else { + Uploaded.Processing(serverId) + } + updateMediaItem(localId) { + it.copy( + description = description, + uploadState = Ok(state), + ) + } + } + .mapError { MediaUploaderError.UpdateMediaError(serverId, it) } + .onFailure { error -> + updateMediaItem(localId) { + it.copy( + description = description, + uploadState = Err(error), + ) + } + } } } fun updateFocus(localId: Int, focus: Attachment.Focus) { - updateMediaItem(localId) { mediaItem -> - mediaItem.copy(focus = focus) - } + updateMediaItem(localId) { mediaItem -> mediaItem.copy(focus = focus) } } suspend fun searchAutocompleteSuggestions(token: String): List { diff --git a/app/src/main/java/app/pachli/components/compose/MediaPreviewAdapter.kt b/app/src/main/java/app/pachli/components/compose/MediaPreviewAdapter.kt index c37957017..1b7e4a601 100644 --- a/app/src/main/java/app/pachli/components/compose/MediaPreviewAdapter.kt +++ b/app/src/main/java/app/pachli/components/compose/MediaPreviewAdapter.kt @@ -21,30 +21,44 @@ import android.view.View import android.view.ViewGroup import android.widget.ImageView import android.widget.PopupMenu +import androidx.appcompat.app.AlertDialog import androidx.constraintlayout.widget.ConstraintLayout import androidx.recyclerview.widget.AsyncListDiffer import androidx.recyclerview.widget.DiffUtil import androidx.recyclerview.widget.RecyclerView import app.pachli.R +import app.pachli.components.compose.ComposeActivity.QueuedMedia +import app.pachli.components.compose.UploadState.Uploaded import app.pachli.components.compose.view.ProgressImageView import app.pachli.core.designsystem.R as DR import com.bumptech.glide.Glide import com.bumptech.glide.load.engine.DiskCacheStrategy +import com.github.michaelbull.result.get +import com.github.michaelbull.result.onFailure +import com.github.michaelbull.result.onSuccess class MediaPreviewAdapter( context: Context, - private val onAddCaption: (ComposeActivity.QueuedMedia) -> Unit, - private val onAddFocus: (ComposeActivity.QueuedMedia) -> Unit, - private val onEditImage: (ComposeActivity.QueuedMedia) -> Unit, - private val onRemove: (ComposeActivity.QueuedMedia) -> Unit, + private val onAddCaption: (QueuedMedia) -> Unit, + private val onAddFocus: (QueuedMedia) -> Unit, + private val onEditImage: (QueuedMedia) -> Unit, + private val onRemove: (QueuedMedia) -> Unit, ) : RecyclerView.Adapter() { - fun submitList(list: List) { + fun submitList(list: List) { this.differ.submitList(list) } private fun onMediaClick(position: Int, view: View) { val item = differ.currentList[position] + + // Handle error + item.uploadState + .onSuccess { showMediaPopup(item, view) } + .onFailure { showMediaError(item, it, view) } + } + + private fun showMediaPopup(item: QueuedMedia, view: View) { val popup = PopupMenu(view.context, view) val addCaptionId = 1 val addFocusId = 2 @@ -52,9 +66,9 @@ class MediaPreviewAdapter( val removeId = 4 popup.menu.add(0, addCaptionId, 0, R.string.action_set_caption) - if (item.type == ComposeActivity.QueuedMedia.Type.IMAGE) { + if (item.type == QueuedMedia.Type.IMAGE) { popup.menu.add(0, addFocusId, 0, R.string.action_set_focus) - if (item.state != ComposeActivity.QueuedMedia.State.PUBLISHED) { + if (item.uploadState.get() !is Uploaded.Published) { // Already-published items can't be edited popup.menu.add(0, editImageId, 0, R.string.action_edit_image) } @@ -72,6 +86,15 @@ class MediaPreviewAdapter( popup.show() } + private fun showMediaError(item: QueuedMedia, error: MediaUploaderError, view: View) { + AlertDialog.Builder(view.context) + .setTitle(R.string.action_post_failed) + .setMessage(view.context.getString(R.string.upload_failed_msg_fmt, error.fmt(view.context))) + .setPositiveButton(android.R.string.ok) { _, _ -> } + .setNegativeButton(R.string.upload_failed_modify_attachment) { _, _ -> showMediaPopup(item, view) } + .show() + } + private val thumbnailViewSize = context.resources.getDimensionPixelSize(DR.dimen.compose_media_preview_size) @@ -84,8 +107,10 @@ class MediaPreviewAdapter( override fun onBindViewHolder(holder: PreviewViewHolder, position: Int) { val item = differ.currentList[position] holder.progressImageView.setChecked(!item.description.isNullOrEmpty()) - holder.progressImageView.setProgress(item.uploadPercent) - if (item.type == ComposeActivity.QueuedMedia.Type.AUDIO) { + + holder.progressImageView.setResult(item.uploadState) + + if (item.type == QueuedMedia.Type.AUDIO) { // TODO: Fancy waveform display? holder.progressImageView.setImageResource(R.drawable.ic_music_box_preview_24dp) } else { @@ -114,12 +139,12 @@ class MediaPreviewAdapter( private val differ = AsyncListDiffer( this, - object : DiffUtil.ItemCallback() { - override fun areItemsTheSame(oldItem: ComposeActivity.QueuedMedia, newItem: ComposeActivity.QueuedMedia): Boolean { + object : DiffUtil.ItemCallback() { + override fun areItemsTheSame(oldItem: QueuedMedia, newItem: QueuedMedia): Boolean { return oldItem.localId == newItem.localId } - override fun areContentsTheSame(oldItem: ComposeActivity.QueuedMedia, newItem: ComposeActivity.QueuedMedia): Boolean { + override fun areContentsTheSame(oldItem: QueuedMedia, newItem: QueuedMedia): Boolean { return oldItem == newItem } }, diff --git a/app/src/main/java/app/pachli/components/compose/MediaUploader.kt b/app/src/main/java/app/pachli/components/compose/MediaUploader.kt index f0c7b7f53..d5188b59b 100644 --- a/app/src/main/java/app/pachli/components/compose/MediaUploader.kt +++ b/app/src/main/java/app/pachli/components/compose/MediaUploader.kt @@ -29,6 +29,7 @@ import app.pachli.BuildConfig import app.pachli.R import app.pachli.components.compose.ComposeActivity.QueuedMedia import app.pachli.components.compose.MediaUploaderError.PrepareMediaError +import app.pachli.components.compose.UploadState.Uploaded import app.pachli.core.common.PachliError import app.pachli.core.common.string.randomAlphanumericString import app.pachli.core.common.util.formatNumber @@ -57,6 +58,7 @@ import kotlinx.coroutines.channels.awaitClose import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.callbackFlow +import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flow @@ -68,18 +70,6 @@ import okio.sink import okio.source import timber.log.Timber -/** - * Media that has been fully uploaded to the server and may still be being - * processed. - * - * @property mediaId Server-side identifier for this media item - * @property processed True if the server has finished processing this media item - */ -data class UploadedMedia( - val mediaId: String, - val processed: Boolean, -) - /** * Media that has been prepared for uploading. * @@ -164,44 +154,66 @@ sealed interface MediaUploaderError : PachliError { } } - /** [ApiError] wrapper. */ + /** + * An error occurred uploading the media, and there is no remote ID. + * + * [ApiError] wrapper. + */ @JvmInline value class UploadMediaError(private val error: ApiError) : MediaUploaderError, PachliError by error + /** + * An error occurred updating media that has already been uploaded. + * + * @param serverId Server's ID for the media + */ + data class UpdateMediaError(val serverId: String, val error: ApiError) : MediaUploaderError, PachliError by error + /** Server did return media with ID [uploadId]. */ data class UploadIdNotFoundError(val uploadId: Int) : MediaUploaderError { override val resourceId = R.string.error_media_uploader_upload_not_found_fmt override val formatArgs = arrayOf(uploadId.toString()) override val cause = null } - - /** Catch-all for arbitrary throwables */ - data class ThrowableError(private val throwable: Throwable) : MediaUploaderError { - override val resourceId = R.string.error_media_uploader_throwable_fmt - override val formatArgs = arrayOf(throwable.localizedMessage ?: "") - override val cause = null - } } -/** Events that happen over the life of a media upload. */ -sealed interface UploadEvent { +/** State of a media upload. */ +sealed interface UploadState { /** - * Upload has made progress. + * Upload is in progress, but incomplete. * * @property percentage What percent of the file has been uploaded. */ - data class ProgressEvent(val percentage: Int) : UploadEvent + data class Uploading(val percentage: Int) : UploadState - /** - * Upload has finished. - * - * @property media The uploaded media - */ - data class FinishedEvent(val media: UploadedMedia) : UploadEvent + sealed interface Uploaded : UploadState { + val serverId: String + + /** + * Upload has completed, but the server is still processing the media. + * + * @property serverId Server-side identifier for this media item + */ + data class Processing(override val serverId: String) : UploadState.Uploaded + + /** + * Upload has completed, and the server has processed the media. + * + * @property serverId Server-side identifier for this media item + */ + data class Processed(override val serverId: String) : UploadState.Uploaded + + /** + * Post has been published, editing is impossible. + * + * @property serverId Server-side identifier for this media item + */ + data class Published(override val serverId: String) : UploadState.Uploaded + } } data class UploadData( - val flow: Flow>, + val flow: Flow>, val scope: CoroutineScope, ) @@ -231,28 +243,29 @@ class MediaUploader @Inject constructor( return mostRecentId++ } - suspend fun getMediaUploadState(localId: Int): Result { - return uploads[localId]?.flow - // Can't use filterIsInstance> here because the type - // inside Ok<...> is erased, so the first Ok<_> result is returned, crashing with a - // class cast error if it's a ProgressEvent. - // Kotlin doesn't warn about this, see - // https://discuss.kotlinlang.org/t/is-as-operators-are-unsafe-for-reified-types/22470 - ?.first { it.get() is UploadEvent.FinishedEvent } as? Ok + /** + * Waits for the upload with [localId] to finish (Ok state is one of the + * [Uploaded][UploadState.Uploaded] types), or return an error. + */ + suspend fun waitForUploadToFinish(localId: Int): Result { + return uploads[localId]?.flow?.filter { + it.get() is Uploaded || it.get() == null + }?.first() as? Result ?: Err(MediaUploaderError.UploadIdNotFoundError(localId)) } /** * Uploads media. + * * @param media the media to upload * @param instanceInfo info about the current media to make sure the media gets resized correctly * @return A Flow emitting upload events. * The Flow is hot, in order to cancel upload or clear resources call [cancelUploadScope]. */ @OptIn(ExperimentalCoroutinesApi::class) - fun uploadMedia(media: QueuedMedia, instanceInfo: InstanceInfo): Flow> { + fun uploadMedia(media: QueuedMedia, instanceInfo: InstanceInfo): Flow> { val uploadScope = CoroutineScope(Dispatchers.IO) - val uploadFlow: Flow> = flow { + val uploadFlow: Flow> = flow { if (shouldResizeMedia(media, instanceInfo)) { emit(downsize(media, instanceInfo)) } else { @@ -371,7 +384,7 @@ class MediaUploader @Inject constructor( private val contentResolver = context.contentResolver - private suspend fun upload(media: QueuedMedia): Flow> { + private suspend fun upload(media: QueuedMedia): Flow> { return callbackFlow { var mimeType = contentResolver.getType(media.uri) @@ -405,7 +418,7 @@ class MediaUploader @Inject constructor( media.mediaSize, ) { percentage -> if (percentage != lastProgress) { - trySend(Ok(UploadEvent.ProgressEvent(percentage))) + trySend(Ok(UploadState.Uploading(percentage))) } lastProgress = percentage } @@ -424,7 +437,13 @@ class MediaUploader @Inject constructor( val uploadResult = mediaUploadApi.uploadMedia(body, description, focus) .mapEither( - { UploadEvent.FinishedEvent(UploadedMedia(it.body.id, it.code == 200)) }, + { + if (it.code == 200) { + Uploaded.Processed(it.body.id) + } else { + Uploaded.Processing(it.body.id) + } + }, { MediaUploaderError.UploadMediaError(it) }, ) send(uploadResult) diff --git a/app/src/main/java/app/pachli/components/compose/dialog/CaptionDialog.kt b/app/src/main/java/app/pachli/components/compose/dialog/CaptionDialog.kt index 1acc3d1b9..c007e5593 100644 --- a/app/src/main/java/app/pachli/components/compose/dialog/CaptionDialog.kt +++ b/app/src/main/java/app/pachli/components/compose/dialog/CaptionDialog.kt @@ -68,10 +68,12 @@ class CaptionDialog : DialogFragment() { input.requestFocus() val localId = arguments?.getInt(ARG_LOCAL_ID) ?: error("Missing localId") + val serverId = arguments?.getString(ARG_SERVER_ID) + val dialog = AlertDialog.Builder(context) .setView(binding.root) .setPositiveButton(android.R.string.ok) { _, _ -> - listener.onUpdateDescription(localId, input.text.toString()) + listener.onUpdateDescription(localId, serverId, input.text.toString()) } .setNegativeButton(android.R.string.cancel, null) .create() @@ -125,22 +127,25 @@ class CaptionDialog : DialogFragment() { } interface Listener { - fun onUpdateDescription(localId: Int, description: String) + fun onUpdateDescription(localId: Int, serverId: String?, description: String) } companion object { private const val KEY_DESCRIPTION = "app.pachli.KEY_DESCRIPTION" + private const val ARG_LOCAL_ID = "app.pachli.ARG_LOCAL_ID" + private const val ARG_SERVER_ID = "app.pachli.ARG_SERVER_ID" private const val ARG_EXISTING_DESCRIPTION = "app.pachli.ARG_EXISTING_DESCRIPTION" private const val ARG_PREVIEW_URI = "app.pachli.ARG_PREVIEW_URI" - private const val ARG_LOCAL_ID = "app.pachli.ARG_LOCAL_ID" fun newInstance( localId: Int, + serverId: String? = null, existingDescription: String?, previewUri: Uri, ) = CaptionDialog().apply { arguments = bundleOf( ARG_LOCAL_ID to localId, + ARG_SERVER_ID to serverId, ARG_EXISTING_DESCRIPTION to existingDescription, ARG_PREVIEW_URI to previewUri, ) diff --git a/app/src/main/java/app/pachli/components/compose/view/ProgressImageView.kt b/app/src/main/java/app/pachli/components/compose/view/ProgressImageView.kt index 1fc2daa19..9e8ac10eb 100644 --- a/app/src/main/java/app/pachli/components/compose/view/ProgressImageView.kt +++ b/app/src/main/java/app/pachli/components/compose/view/ProgressImageView.kt @@ -24,12 +24,22 @@ import android.graphics.PorterDuff import android.graphics.PorterDuffXfermode import android.graphics.RectF import android.util.AttributeSet +import androidx.annotation.OptIn import androidx.appcompat.content.res.AppCompatResources import app.pachli.R +import app.pachli.components.compose.MediaUploaderError +import app.pachli.components.compose.UploadState import app.pachli.core.designsystem.R as DR +import app.pachli.core.ui.makeIcon import app.pachli.view.MediaPreviewImageView import at.connyduck.sparkbutton.helpers.Utils +import com.github.michaelbull.result.Ok +import com.github.michaelbull.result.Result +import com.github.michaelbull.result.onFailure +import com.github.michaelbull.result.onSuccess +import com.google.android.material.badge.ExperimentalBadgeUtils import com.google.android.material.color.MaterialColors +import com.mikepenz.iconics.typeface.library.googlematerial.GoogleMaterial class ProgressImageView @JvmOverloads constructor( @@ -37,7 +47,7 @@ class ProgressImageView attrs: AttributeSet? = null, defStyleAttr: Int = 0, ) : MediaPreviewImageView(context, attrs, defStyleAttr) { - private var progress = -1 + private var result: Result = Ok(UploadState.Uploading(0)) private val progressRect = RectF() private val biggerRect = RectF() private val circlePaint = Paint(Paint.ANTI_ALIAS_FLAG).apply { @@ -60,14 +70,15 @@ class ProgressImageView } private val circleRadius = Utils.dpToPx(context, 14) private val circleMargin = Utils.dpToPx(context, 14) + private val uploadErrorRadius = Utils.dpToPx(context, 24) - fun setProgress(progress: Int) { - this.progress = progress - if (progress != -1) { - setColorFilter(Color.rgb(123, 123, 123), PorterDuff.Mode.MULTIPLY) - } else { - clearColorFilter() - } + private val uploadErrorDrawable = makeIcon(context, GoogleMaterial.Icon.gmd_error, 48).apply { + setTint(Color.WHITE) + } + + @OptIn(ExperimentalBadgeUtils::class) + fun setResult(result: Result) { + this.result = result invalidate() } @@ -82,6 +93,24 @@ class ProgressImageView override fun onDraw(canvas: Canvas) { super.onDraw(canvas) + + result.onSuccess { value -> + val percentage = when (value) { + is UploadState.Uploading -> value.percentage + else -> -1 + } + onDrawSuccess(canvas, percentage) + }.onFailure { error -> + onDrawError(canvas) + } + } + + private fun onDrawSuccess(canvas: Canvas, progress: Int) { + clearColorFilter() + if (progress != -1) { + setColorFilter(Color.rgb(123, 123, 123), PorterDuff.Mode.MULTIPLY) + } + val angle = progress / 100f * 360 - 90 val halfWidth = width / 2f val halfHeight = height / 2f @@ -107,4 +136,18 @@ class ProgressImageView ) captionDrawable.draw(canvas) } + + private fun onDrawError(canvas: Canvas) { + setColorFilter( + MaterialColors.getColor(this, androidx.appcompat.R.attr.colorError), + PorterDuff.Mode.DARKEN, + ) + uploadErrorDrawable.setBounds( + (width / 2) - uploadErrorRadius, + (height / 2) - uploadErrorRadius, + (width / 2) + uploadErrorRadius, + (height / 2) + uploadErrorRadius, + ) + uploadErrorDrawable.draw(canvas) + } } diff --git a/app/src/main/java/app/pachli/service/SendStatusService.kt b/app/src/main/java/app/pachli/service/SendStatusService.kt index 604ab011d..b0b9ab118 100644 --- a/app/src/main/java/app/pachli/service/SendStatusService.kt +++ b/app/src/main/java/app/pachli/service/SendStatusService.kt @@ -38,6 +38,8 @@ import app.pachli.core.network.model.Status import app.pachli.core.network.retrofit.MastodonApi import at.connyduck.calladapter.networkresult.fold import com.github.michaelbull.result.getOrElse +import com.github.michaelbull.result.onFailure +import com.github.michaelbull.result.onSuccess import dagger.hilt.android.AndroidEntryPoint import java.io.IOException import java.util.Date @@ -145,14 +147,14 @@ class SendStatusService : Service() { // first, wait for media uploads to finish val media = statusToSend.media.map { mediaItem -> if (mediaItem.id == null) { - val uploadState = mediaUploader.getMediaUploadState(mediaItem.localId) + val uploadState = mediaUploader.waitForUploadToFinish(mediaItem.localId) val media = uploadState.getOrElse { Timber.w("failed uploading media: %s", it.fmt(this@SendStatusService)) failSending(statusId) stopSelfWhenDone() return@launch - }.media - mediaItem.copy(id = media.mediaId, processed = media.processed) + } + mediaItem.copy(id = media.serverId) } else { mediaItem } @@ -165,15 +167,13 @@ class SendStatusService : Service() { delay(1000L * mediaCheckRetries) media.forEach { mediaItem -> if (!mediaItem.processed) { - when (mastodonApi.getMedia(mediaItem.id!!).code()) { - 200 -> mediaItem.processed = true // success - 206 -> { } // media is still being processed, continue checking - else -> { // some kind of server error, retrying probably doesn't make sense + mastodonApi.getMedia(mediaItem.id!!) + .onSuccess { mediaItem.processed = it.code == 200 } + .onFailure { failSending(statusId) stopSelfWhenDone() return@launch } - } } } mediaCheckRetries++ @@ -190,13 +190,11 @@ class SendStatusService : Service() { media.forEach { mediaItem -> if (mediaItem.processed && (mediaItem.description != null || mediaItem.focus != null)) { mastodonApi.updateMedia(mediaItem.id!!, mediaItem.description, mediaItem.focus?.toMastodonApiString()) - .fold({ - }, { throwable -> - Timber.w(throwable, "failed to update media on status send") - failOrRetry(throwable, statusId) - + .onFailure { error -> + Timber.w("failed to update media on status send: %s", error) + failOrRetry(error.throwable, statusId) return@launch - }) + } } } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 9458afe24..a88e927d0 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -853,4 +853,7 @@ Copy item Text copied + + The upload will be retried when you send the post. If it fails again the post will be saved in your drafts.\n\nThe error was: %1$s + Modify attachment diff --git a/core/network/src/main/kotlin/app/pachli/core/network/retrofit/MastodonApi.kt b/core/network/src/main/kotlin/app/pachli/core/network/retrofit/MastodonApi.kt index e03245664..8ac6510eb 100644 --- a/core/network/src/main/kotlin/app/pachli/core/network/retrofit/MastodonApi.kt +++ b/core/network/src/main/kotlin/app/pachli/core/network/retrofit/MastodonApi.kt @@ -201,14 +201,14 @@ interface MastodonApi { @PUT("api/v1/media/{mediaId}") suspend fun updateMedia( @Path("mediaId") mediaId: String, - @Field("description") description: String?, - @Field("focus") focus: String?, - ): NetworkResult + @Field("description") description: String? = null, + @Field("focus") focus: String? = null, + ): ApiResult @GET("api/v1/media/{mediaId}") suspend fun getMedia( @Path("mediaId") mediaId: String, - ): Response + ): ApiResult @POST("api/v1/statuses") suspend fun createStatus( diff --git a/core/network/src/main/res/values/strings.xml b/core/network/src/main/res/values/strings.xml index 6ecb876d1..ac5f6a2c0 100644 --- a/core/network/src/main/res/values/strings.xml +++ b/core/network/src/main/res/values/strings.xml @@ -21,8 +21,8 @@ software version is missing, empty, or blank could not parse \"%1$s\" as a version: %2$s - An error occurred: %s - Your server does not support this feature: %1$s + %1$s + your server does not support this feature: %1$s your server is rate-limiting your requests: %1$s Your server returned an invalid response: %1$s A network error occurred: %s