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.

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()) {
+        val builder = AlertDialog.Builder(this)
-            R.string.compose_save_draft_loses_media
+            .setTitle(R.string.compose_save_draft)
-        } else {
-            R.string.compose_save_draft
-        }
-
-        return AlertDialog.Builder(this)
-            .setMessage(warning)
             .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()) {
+        val builder = AlertDialog.Builder(this)
-            R.string.compose_save_draft_loses_media
+            .setTitle(R.string.compose_save_draft)
-        } else {
-            R.string.compose_save_draft
-        }
-
-        return AlertDialog.Builder(this)
-            .setMessage(warning)
             .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<UploadState, MediaUploaderError>,
     ) {
         enum class Type {
             IMAGE,
             VIDEO,
             AUDIO,
         }
-        enum class State {
+
-            UPLOADING,
+        /**
-            UNPROCESSED,
+         * Server's ID for this attachment. May be null if the media is still
-            PROCESSED,
+         * being uploaded, or it was uploaded and there was an error that
-            PUBLISHED,
+         * 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) {
+    override fun onUpdateDescription(localId: Int, serverId: String?, description: String) {
-        viewModel.updateDescription(localId, description)
+        viewModel.updateDescription(localId, serverId, description)
     }
 
     companion object {

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<List<QueuedMedia>> = MutableStateFlow(emptyList())
     val media = _media.asStateFlow()
-    private val _uploadError = MutableSharedFlow<MediaUploaderError>(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 ->
+                .collect { uploadResult ->
-                    val item = media.value.find { it.localId == mediaItem.localId }
+                    updateMediaItem(mediaItem.localId) { it.copy(uploadState = uploadResult) }
-                        ?: 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 }
-                        }
-                    }
                 }
         }
 
@@ -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) {
+    fun updateDescription(localId: Int, serverId: String?, description: String) {
-        updateMediaItem(localId) { mediaItem ->
+        // If the image hasn't been uploaded then update the state locally.
-            mediaItem.copy(description = description)
+        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 ->
+        updateMediaItem(localId) { mediaItem -> mediaItem.copy(focus = focus) }
-            mediaItem.copy(focus = focus)
-        }
     }
 
     suspend fun searchAutocompleteSuggestions(token: String): List<AutocompleteResult> {

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 onAddCaption: (QueuedMedia) -> Unit,
-    private val onAddFocus: (ComposeActivity.QueuedMedia) -> Unit,
+    private val onAddFocus: (QueuedMedia) -> Unit,
-    private val onEditImage: (ComposeActivity.QueuedMedia) -> Unit,
+    private val onEditImage: (QueuedMedia) -> Unit,
-    private val onRemove: (ComposeActivity.QueuedMedia) -> Unit,
+    private val onRemove: (QueuedMedia) -> Unit,
 ) : RecyclerView.Adapter<MediaPreviewAdapter.PreviewViewHolder>() {
 
-    fun submitList(list: List<ComposeActivity.QueuedMedia>) {
+    fun submitList(list: List<QueuedMedia>) {
         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<ComposeActivity.QueuedMedia>() {
+        object : DiffUtil.ItemCallback<QueuedMedia>() {
-            override fun areItemsTheSame(oldItem: ComposeActivity.QueuedMedia, newItem: ComposeActivity.QueuedMedia): Boolean {
+            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
             }
         },

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. */
+/** State of a media upload. */
-sealed interface UploadEvent {
+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
 
-    /**
+    sealed interface Uploaded : UploadState {
-     * Upload has finished.
+        val serverId: String
-     *
+
-     * @property media The uploaded media
+        /**
-     */
+         * Upload has completed, but the server is still processing the media.
-    data class FinishedEvent(val media: UploadedMedia) : UploadEvent
+         *
+         * @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<Result<UploadEvent, MediaUploaderError>>,
+    val flow: Flow<Result<UploadState, MediaUploaderError>>,
     val scope: CoroutineScope,
 )
 
@@ -231,28 +243,29 @@ class MediaUploader @Inject constructor(
         return mostRecentId++
     }
 
-    suspend fun getMediaUploadState(localId: Int): Result<UploadEvent.FinishedEvent, MediaUploaderError> {
+    /**
-        return uploads[localId]?.flow
+     * Waits for the upload with [localId] to finish (Ok state is one of the
-            // Can't use filterIsInstance<Ok<UploadEvent.FinishedEvent>> here because the type
+     * [Uploaded][UploadState.Uploaded] types), or return an error.
-            // inside Ok<...> is erased, so the first Ok<_> result is returned, crashing with a
+     */
-            // class cast error if it's a ProgressEvent.
+    suspend fun waitForUploadToFinish(localId: Int): Result<Uploaded, MediaUploaderError> {
-            // Kotlin doesn't warn about this, see
+        return uploads[localId]?.flow?.filter {
-            // https://discuss.kotlinlang.org/t/is-as-operators-are-unsafe-for-reified-types/22470
+            it.get() is Uploaded || it.get() == null
-            ?.first { it.get() is UploadEvent.FinishedEvent } as? Ok<UploadEvent.FinishedEvent>
+        }?.first() as? Result<Uploaded, MediaUploaderError>
             ?: 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<Result<UploadEvent, MediaUploaderError>> {
+    fun uploadMedia(media: QueuedMedia, instanceInfo: InstanceInfo): Flow<Result<UploadState, MediaUploaderError>> {
         val uploadScope = CoroutineScope(Dispatchers.IO)
-        val uploadFlow: Flow<Result<UploadEvent, MediaUploaderError>> = flow {
+        val uploadFlow: Flow<Result<UploadState, MediaUploaderError>> = 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<Result<UploadEvent, MediaUploaderError.UploadMediaError>> {
+    private suspend fun upload(media: QueuedMedia): Flow<Result<UploadState, MediaUploaderError.UploadMediaError>> {
         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)

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,
             )

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<UploadState, MediaUploaderError> = 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) {
+    private val uploadErrorDrawable = makeIcon(context, GoogleMaterial.Icon.gmd_error, 48).apply {
-        this.progress = progress
+        setTint(Color.WHITE)
-        if (progress != -1) {
+    }
-            setColorFilter(Color.rgb(123, 123, 123), PorterDuff.Mode.MULTIPLY)
+
-        } else {
+    @OptIn(ExperimentalBadgeUtils::class)
-            clearColorFilter()
+    fun setResult(result: Result<UploadState, MediaUploaderError>) {
-        }
+        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)
+    }
 }

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()) {
+                            mastodonApi.getMedia(mediaItem.id!!)
-                                200 -> mediaItem.processed = true // success
+                                .onSuccess { mediaItem.processed = it.code == 200 }
-                                206 -> { } // media is still being processed, continue checking
+                                .onFailure {
-                                else -> { // some kind of server error, retrying probably doesn't make sense
                                     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({
+                            .onFailure { error ->
-                            }, { throwable ->
+                                Timber.w("failed to update media on status send: %s", error)
-                                Timber.w(throwable, "failed to update media on status send")
+                                failOrRetry(error.throwable, statusId)
-                                failOrRetry(throwable, statusId)
-
                                 return@launch
-                            })
+                            }
                     }
                 }
             }

app/src/main/res/values/strings.xml

@@ -853,4 +853,7 @@
 
     <string name="action_copy_item">Copy item</string>
     <string name="item_copied">Text copied</string>
+
+    <string name="upload_failed_msg_fmt">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</string>
+    <string name="upload_failed_modify_attachment">Modify attachment</string>
 </resources>

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("description") description: String? = null,
-        @Field("focus") focus: String?,
+        @Field("focus") focus: String? = null,
-    ): NetworkResult<Attachment>
+    ): ApiResult<Attachment>
 
     @GET("api/v1/media/{mediaId}")
     suspend fun getMedia(
         @Path("mediaId") mediaId: String,
-    ): Response<MediaUploadResult>
+    ): ApiResult<MediaUploadResult>
 
     @POST("api/v1/statuses")
     suspend fun createStatus(

core/network/src/main/res/values/strings.xml

@@ -21,8 +21,8 @@
     <string name="node_info_error_no_software_version">software version is missing, empty, or blank</string>
 
     <string name="server_error_unparseable_version">could not parse \"%1$s\" as a version: %2$s</string>
-    <string name="error_generic_fmt">An error occurred: %s</string>
+    <string name="error_generic_fmt">%1$s</string>
-    <string name="error_404_not_found_fmt">Your server does not support this feature: %1$s</string>
+    <string name="error_404_not_found_fmt">your server does not support this feature: %1$s</string>
     <string name="error_429_rate_limit_fmt">your server is rate-limiting your requests: %1$s</string>
     <string name="error_json_data_fmt">Your server returned an invalid response: %1$s</string>
     <string name="error_network_fmt">A network error occurred: %s</string>