Always publish image alt text

Previous code would discard the image alt-text if the user finished writing the text before the image had finished uploading. This code ensures the text is set after the image has completed uploading.
2023-04-24 11:48:40 +02:00 · 2023-04-24 11:48:40 +02:00 · 24d7ef7ccb
commit 24d7ef7ccb
parent 168be9223d
53 changed files with 209 additions and 288 deletions
--- a/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeActivity.kt
+++ b/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeActivity.kt
@ -227,13 +227,13 @@ 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.description, item.uri).show(supportFragmentManager, "caption_dialog")
            },
            onAddFocus = { item ->
                makeFocusDialog(item.focus, item.uri) { newFocus ->
                    viewModel.updateFocus(item.localId, newFocus)
                }
+                // TODO this is inconsistent to CaptionDialog (device rotation)?
            },
            onEditImage = this::editImageInQueue,
            onRemove = this::removeMediaFromQueue
@ -1266,11 +1266,7 @@ class ComposeActivity :
    }

    override fun onUpdateDescription(localId: Int, description: String) {
-        lifecycleScope.launch {
-            if (!viewModel.updateDescription(localId, description)) {
-                Toast.makeText(this@ComposeActivity, R.string.error_failed_set_caption, Toast.LENGTH_SHORT).show()
-            }
-        }
+        viewModel.updateDescription(localId, description)
    }

    /**
--- a/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeViewModel.kt
+++ b/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeViewModel.kt
@ -48,7 +48,6 @@ import kotlinx.coroutines.flow.asFlow
 import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.flow.shareIn
 import kotlinx.coroutines.flow.update
-import kotlinx.coroutines.flow.updateAndGet
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
 import javax.inject.Inject
@ -130,7 +129,7 @@ class ComposeViewModel @Inject constructor(
    ): QueuedMedia {
        var stashMediaItem: QueuedMedia? = null

-        media.updateAndGet { mediaValue ->
+        media.update { mediaList ->
            val mediaItem = QueuedMedia(
                localId = mediaUploader.getNewLocalMediaId(),
                uri = uri,
@ -144,11 +143,11 @@ class ComposeViewModel @Inject constructor(

            if (replaceItem != null) {
                mediaUploader.cancelUploadScope(replaceItem.localId)
-                mediaValue.map {
+                mediaList.map {
                    if (it.localId == replaceItem.localId) mediaItem else it
                }
            } else { // Append
-                mediaValue + mediaItem
+                mediaList + mediaItem
            }
        }
        val mediaItem = stashMediaItem!! // stashMediaItem is always non-null and uncaptured at this point, but Kotlin doesn't know that
@ -169,13 +168,13 @@ class ComposeViewModel @Inject constructor(
                                state = if (event.processed) { QueuedMedia.State.PROCESSED } else { QueuedMedia.State.UNPROCESSED }
                            )
                        is UploadEvent.ErrorEvent -> {
-                            media.update { mediaValue -> mediaValue.filter { it.localId != mediaItem.localId } }
+                            media.update { mediaList -> mediaList.filter { it.localId != mediaItem.localId } }
                            uploadError.emit(event.error)
                            return@collect
                        }
                    }
-                    media.update { mediaValue ->
-                        mediaValue.map { mediaItem ->
+                    media.update { mediaList ->
+                        mediaList.map { mediaItem ->
                            if (mediaItem.localId == newMediaItem.localId) {
                                newMediaItem
                            } else {
@ -189,7 +188,7 @@ class ComposeViewModel @Inject constructor(
    }

    private fun addUploadedMedia(id: String, type: QueuedMedia.Type, uri: Uri, description: String?, focus: Attachment.Focus?) {
-        media.update { mediaValue ->
+        media.update { mediaList ->
            val mediaItem = QueuedMedia(
                localId = mediaUploader.getNewLocalMediaId(),
                uri = uri,
@ -201,13 +200,13 @@ class ComposeViewModel @Inject constructor(
                focus = focus,
                state = QueuedMedia.State.PUBLISHED
            )
-            mediaValue + mediaItem
+            mediaList + mediaItem
        }
    }

    fun removeMediaFromQueue(item: QueuedMedia) {
        mediaUploader.cancelUploadScope(item.localId)
-        media.update { mediaValue -> mediaValue.filter { it.localId != item.localId } }
+        media.update { mediaList -> mediaList.filter { it.localId != item.localId } }
    }

    fun toggleMarkSensitive() {
@ -322,10 +321,9 @@ class ComposeViewModel @Inject constructor(
        serviceClient.sendToot(tootToSend)
    }

-    // Updates a QueuedMedia item arbitrarily, then sends description and focus to server
-    private suspend fun updateMediaItem(localId: Int, mutator: (QueuedMedia) -> QueuedMedia): Boolean {
-        val newMediaList = media.updateAndGet { mediaValue ->
-            mediaValue.map { mediaItem ->
+    private fun updateMediaItem(localId: Int, mutator: (QueuedMedia) -> QueuedMedia) {
+        media.update { mediaList ->
+            mediaList.map { mediaItem ->
                if (mediaItem.localId == localId) {
                    mutator(mediaItem)
                } else {
@ -333,33 +331,16 @@ class ComposeViewModel @Inject constructor(
                }
            }
        }
-
-        if (!editing) {
-            // Updates to media for already-published statuses need to go through the status edit api
-            val updatedItem = newMediaList.find { it.localId == localId }
-            if (updatedItem?.id != null) {
-                val focus = updatedItem.focus
-                val focusString = if (focus != null) "${focus.x},${focus.y}" else null
-                return api.updateMedia(updatedItem.id, updatedItem.description, focusString)
-                    .fold({
-                        true
-                    }, { throwable ->
-                        Log.w(TAG, "failed to update media", throwable)
-                        false
-                    })
-            }
-        }
-        return true
    }

-    suspend fun updateDescription(localId: Int, description: String): Boolean {
-        return updateMediaItem(localId) { mediaItem ->
+    fun updateDescription(localId: Int, description: String) {
+        updateMediaItem(localId) { mediaItem ->
            mediaItem.copy(description = description)
        }
    }

-    suspend fun updateFocus(localId: Int, focus: Attachment.Focus): Boolean {
-        return updateMediaItem(localId) { mediaItem ->
+    fun updateFocus(localId: Int, focus: Attachment.Focus) {
+        updateMediaItem(localId) { mediaItem ->
            mediaItem.copy(focus = focus)
        }
    }
--- a/app/src/main/java/com/keylesspalace/tusky/components/compose/dialog/FocusDialog.kt
+++ b/app/src/main/java/com/keylesspalace/tusky/components/compose/dialog/FocusDialog.kt
@ -21,7 +21,6 @@ import android.graphics.drawable.Drawable
 import android.net.Uri
 import android.view.WindowManager
 import android.widget.FrameLayout
-import android.widget.Toast
 import androidx.appcompat.app.AlertDialog
 import androidx.lifecycle.LifecycleOwner
 import androidx.lifecycle.lifecycleScope
@ -31,7 +30,6 @@ import com.bumptech.glide.load.engine.GlideException
 import com.bumptech.glide.load.resource.bitmap.DownsampleStrategy
 import com.bumptech.glide.request.RequestListener
 import com.bumptech.glide.request.target.Target
-import com.keylesspalace.tusky.R
 import com.keylesspalace.tusky.databinding.DialogFocusBinding
 import com.keylesspalace.tusky.entity.Attachment.Focus
 import kotlinx.coroutines.launch
@ -39,7 +37,7 @@ import kotlinx.coroutines.launch
 fun <T> T.makeFocusDialog(
    existingFocus: Focus?,
    previewUri: Uri,
-    onUpdateFocus: suspend (Focus) -> Boolean
+    onUpdateFocus: suspend (Focus) -> Unit
 ) where T : Activity, T : LifecycleOwner {
    val focus = existingFocus ?: Focus(0.0f, 0.0f) // Default to center

@ -79,9 +77,7 @@ fun <T> T.makeFocusDialog(

    val okListener = { dialog: DialogInterface, _: Int ->
        lifecycleScope.launch {
-            if (!onUpdateFocus(dialogBinding.focusIndicator.getFocus())) {
-                showFailedFocusMessage()
-            }
+            onUpdateFocus(dialogBinding.focusIndicator.getFocus())
        }
        dialog.dismiss()
    }
@ -99,7 +95,3 @@ fun <T> T.makeFocusDialog(

    dialog.show()
 }
-
-private fun Activity.showFailedFocusMessage() {
-    Toast.makeText(this, R.string.error_failed_set_focus, Toast.LENGTH_SHORT).show()
-}
--- a/app/src/main/java/com/keylesspalace/tusky/network/ProgressRequestBody.java
+++ b/app/src/main/java/com/keylesspalace/tusky/network/ProgressRequestBody.java
@ -67,8 +67,10 @@ public final class ProgressRequestBody extends RequestBody {
                uploaded += read;
                sink.write(buffer, 0, read);
            }
+
+            uploadListener.onProgressUpdate((int)(100 * uploaded / contentLength));
        } finally {
            content.close();
        }
    }
-}
+}
--- a/app/src/main/java/com/keylesspalace/tusky/service/SendStatusService.kt
+++ b/app/src/main/java/com/keylesspalace/tusky/service/SendStatusService.kt
@ -183,6 +183,23 @@ class SendStatusService : Service(), Injectable {
                return@launch
            }

+            val isNew = statusToSend.statusId == null
+
+            if (isNew) {
+                media.forEach { mediaItem ->
+                    if (mediaItem.processed && (mediaItem.description != null || mediaItem.focus != null)) {
+                        mastodonApi.updateMedia(mediaItem.id!!, mediaItem.description, mediaItem.focus?.toMastodonApiString())
+                            .fold({
+                            }, { throwable ->
+                                Log.w(TAG, "failed to update media on status send", throwable)
+                                failOrRetry(throwable, statusId)
+
+                                return@launch
+                            })
+                    }
+                }
+            }
+
            // finally, send the new status
            val newStatus = NewStatus(
                status = statusToSend.text,
@ -204,17 +221,16 @@ class SendStatusService : Service(), Injectable {
                }
            )

-            val editing = (statusToSend.statusId != null)
-            val sendResult = if (editing) {
-                mastodonApi.editStatus(
-                    statusToSend.statusId!!,
+            val sendResult = if (isNew) {
+                mastodonApi.createStatus(
                    "Bearer " + account.accessToken,
                    account.domain,
                    statusToSend.idempotencyKey,
                    newStatus
                )
            } else {
-                mastodonApi.createStatus(
+                mastodonApi.editStatus(
+                    statusToSend.statusId!!,
                    "Bearer " + account.accessToken,
                    account.domain,
                    statusToSend.idempotencyKey,
@ -235,7 +251,7 @@ class SendStatusService : Service(), Injectable {

                if (scheduled) {
                    eventHub.dispatch(StatusScheduledEvent(sentStatus))
-                } else if (editing) {
+                } else if (!isNew) {
                    eventHub.dispatch(StatusEditedEvent(statusToSend.statusId!!, sentStatus))
                } else {
                    eventHub.dispatch(StatusComposedEvent(sentStatus))
@ -244,18 +260,22 @@ class SendStatusService : Service(), Injectable {
                notificationManager.cancel(statusId)
            }, { throwable ->
                Log.w(TAG, "failed sending status", throwable)
-                if (throwable is HttpException) {
-                    // the server refused to accept the status, save status & show error message
-                    failSending(statusId)
-                } else {
-                    // a network problem occurred, let's retry sending the status
-                    retrySending(statusId)
-                }
+                failOrRetry(throwable, statusId)
            })
            stopSelfWhenDone()
        }
    }

+    private suspend fun failOrRetry(throwable: Throwable, statusId: Int) {
+        if (throwable is HttpException) {
+            // the server refused to accept, save status & show error message
+            failSending(statusId)
+        } else {
+            // a network problem occurred, let's retry sending the status
+            retrySending(statusId)
+        }
+    }
+
    private suspend fun retrySending(statusId: Int) {
        // when statusToSend == null, sending has been canceled
        val statusToSend = statusesToSend[statusId] ?: return
@ -290,6 +310,9 @@ class SendStatusService : Service(), Injectable {
            notificationManager.cancel(statusId)
            notificationManager.notify(errorNotificationId++, notification)
        }
+
+        // NOTE only this removes the "Sending..." notification (added with startForeground() above)
+        stopSelfWhenDone()
    }

    private fun cancelSending(statusId: Int) = serviceScope.launch {