From 67fe600f2c164ff562ea3c5470946de0e7358fbf Mon Sep 17 00:00:00 2001 From: Nik Clayton Date: Sat, 26 Oct 2024 22:21:33 +0200 Subject: [PATCH] Revert "fix: Don't lose images / captions when editing with failed uploads" This reverts commit 7abd74ad88168ad4534f3b1603a6d31d5fe2a27b. --- .../components/compose/ComposeActivity.kt | 82 ++++++------ .../components/compose/ComposeViewModel.kt | 91 +++++++------- .../components/compose/MediaPreviewAdapter.kt | 49 ++------ .../components/compose/MediaUploader.kt | 117 ++++++++---------- .../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, 174 insertions(+), 276 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 ec6bf31af..45904447b 100644 --- a/app/src/main/java/app/pachli/components/compose/ComposeActivity.kt +++ b/app/src/main/java/app/pachli/components/compose/ComposeActivity.kt @@ -109,9 +109,7 @@ 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 @@ -255,7 +253,7 @@ class ComposeActivity : val mediaAdapter = MediaPreviewAdapter( this, onAddCaption = { item -> - CaptionDialog.newInstance(item.localId, item.serverId, 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 -> @@ -526,6 +524,14 @@ 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 */ @@ -1266,8 +1272,14 @@ 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 builder = AlertDialog.Builder(this) - .setTitle(R.string.compose_save_draft) + 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) .setPositiveButton(R.string.action_save) { _, _ -> viewModel.stopUploads() saveDraftAndFinish(contentText, contentWarning) @@ -1276,12 +1288,6 @@ class ComposeActivity : viewModel.stopUploads() deleteDraftAndFinish() } - - if (viewModel.media.value.isNotEmpty()) { - builder.setMessage(R.string.compose_save_draft_loses_media) - } - - return builder } /** @@ -1289,8 +1295,14 @@ class ComposeActivity : * discard them. */ private fun getUpdateDraftOrDiscardDialog(contentText: String, contentWarning: String): AlertDialog.Builder { - val builder = AlertDialog.Builder(this) - .setTitle(R.string.compose_save_draft) + 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) .setPositiveButton(R.string.action_save) { _, _ -> viewModel.stopUploads() saveDraftAndFinish(contentText, contentWarning) @@ -1299,12 +1311,6 @@ class ComposeActivity : viewModel.stopUploads() finish() } - - if (viewModel.media.value.isNotEmpty()) { - builder.setMessage(R.string.compose_save_draft_loses_media) - } - - return builder } /** @@ -1386,39 +1392,23 @@ 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 uploadState: Result, + val state: State, ) { enum class Type { IMAGE, VIDEO, AUDIO, } - - /** - * 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 - } - }, - ) + enum class State { + UPLOADING, + UNPROCESSED, + PROCESSED, + PUBLISHED, + } } override fun onTimeSet(time: Date?) { @@ -1435,8 +1425,8 @@ class ComposeActivity : scheduleBehavior.state = BottomSheetBehavior.STATE_HIDDEN } - override fun onUpdateDescription(localId: Int, serverId: String?, description: String) { - viewModel.updateDescription(localId, serverId, description) + override fun onUpdateDescription(localId: Int, description: String) { + viewModel.updateDescription(localId, 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 87a862eaf..94534f5f6 100644 --- a/app/src/main/java/app/pachli/components/compose/ComposeViewModel.kt +++ b/app/src/main/java/app/pachli/components/compose/ComposeViewModel.kt @@ -28,7 +28,6 @@ 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 @@ -53,20 +52,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 @@ -145,6 +144,8 @@ 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) @@ -226,7 +227,7 @@ class ComposeViewModel @Inject constructor( mediaSize = mediaSize, description = description, focus = focus, - uploadState = Ok(UploadState.Uploading(percentage = 0)), + state = QueuedMedia.State.UPLOADING, ) stashMediaItem = mediaItem @@ -244,8 +245,35 @@ class ComposeViewModel @Inject constructor( viewModelScope.launch { mediaUploader .uploadMedia(mediaItem, instanceInfo.value) - .collect { uploadResult -> - updateMediaItem(mediaItem.localId) { it.copy(uploadState = uploadResult) } + .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 } + } + } } } @@ -260,9 +288,11 @@ class ComposeViewModel @Inject constructor( uri = uri, type = type, mediaSize = 0, + uploadPercent = -1, + id = id, description = description, focus = focus, - uploadState = Ok(Uploaded.Published(id)), + state = QueuedMedia.State.PUBLISHED, ) mediaList + mediaItem } @@ -427,11 +457,11 @@ class ComposeViewModel @Inject constructor( val attachedMedia = media.value.map { item -> MediaToSend( localId = item.localId, - id = item.serverId, + id = item.id, uri = item.uri.toString(), description = item.description, focus = item.focus, - processed = item.uploadState.get() is Uploaded.Processed || item.uploadState.get() is Uploaded.Published, + processed = item.state == QueuedMedia.State.PROCESSED || item.state == QueuedMedia.State.PUBLISHED, ) } val tootToSend = StatusToSend( @@ -468,45 +498,16 @@ class ComposeViewModel @Inject constructor( } } - 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 updateDescription(localId: Int, description: String) { + updateMediaItem(localId) { mediaItem -> + mediaItem.copy(description = description) } } 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 1b7e4a601..c37957017 100644 --- a/app/src/main/java/app/pachli/components/compose/MediaPreviewAdapter.kt +++ b/app/src/main/java/app/pachli/components/compose/MediaPreviewAdapter.kt @@ -21,44 +21,30 @@ 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: (QueuedMedia) -> Unit, - private val onAddFocus: (QueuedMedia) -> Unit, - private val onEditImage: (QueuedMedia) -> Unit, - private val onRemove: (QueuedMedia) -> Unit, + private val onAddCaption: (ComposeActivity.QueuedMedia) -> Unit, + private val onAddFocus: (ComposeActivity.QueuedMedia) -> Unit, + private val onEditImage: (ComposeActivity.QueuedMedia) -> Unit, + private val onRemove: (ComposeActivity.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 @@ -66,9 +52,9 @@ class MediaPreviewAdapter( val removeId = 4 popup.menu.add(0, addCaptionId, 0, R.string.action_set_caption) - if (item.type == QueuedMedia.Type.IMAGE) { + if (item.type == ComposeActivity.QueuedMedia.Type.IMAGE) { popup.menu.add(0, addFocusId, 0, R.string.action_set_focus) - if (item.uploadState.get() !is Uploaded.Published) { + if (item.state != ComposeActivity.QueuedMedia.State.PUBLISHED) { // Already-published items can't be edited popup.menu.add(0, editImageId, 0, R.string.action_edit_image) } @@ -86,15 +72,6 @@ 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) @@ -107,10 +84,8 @@ class MediaPreviewAdapter( override fun onBindViewHolder(holder: PreviewViewHolder, position: Int) { val item = differ.currentList[position] holder.progressImageView.setChecked(!item.description.isNullOrEmpty()) - - holder.progressImageView.setResult(item.uploadState) - - if (item.type == QueuedMedia.Type.AUDIO) { + holder.progressImageView.setProgress(item.uploadPercent) + if (item.type == ComposeActivity.QueuedMedia.Type.AUDIO) { // TODO: Fancy waveform display? holder.progressImageView.setImageResource(R.drawable.ic_music_box_preview_24dp) } else { @@ -139,12 +114,12 @@ class MediaPreviewAdapter( private val differ = AsyncListDiffer( this, - object : DiffUtil.ItemCallback() { - override fun areItemsTheSame(oldItem: QueuedMedia, newItem: QueuedMedia): Boolean { + object : DiffUtil.ItemCallback() { + override fun areItemsTheSame(oldItem: ComposeActivity.QueuedMedia, newItem: ComposeActivity.QueuedMedia): Boolean { return oldItem.localId == newItem.localId } - override fun areContentsTheSame(oldItem: QueuedMedia, newItem: QueuedMedia): Boolean { + override fun areContentsTheSame(oldItem: ComposeActivity.QueuedMedia, newItem: ComposeActivity.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 d5188b59b..f0c7b7f53 100644 --- a/app/src/main/java/app/pachli/components/compose/MediaUploader.kt +++ b/app/src/main/java/app/pachli/components/compose/MediaUploader.kt @@ -29,7 +29,6 @@ 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 @@ -58,7 +57,6 @@ 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 @@ -70,6 +68,18 @@ 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. * @@ -154,66 +164,44 @@ sealed interface MediaUploaderError : PachliError { } } - /** - * An error occurred uploading the media, and there is no remote ID. - * - * [ApiError] wrapper. - */ + /** [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 } -} -/** State of a media upload. */ -sealed interface UploadState { - /** - * Upload is in progress, but incomplete. - * - * @property percentage What percent of the file has been uploaded. - */ - data class Uploading(val percentage: Int) : UploadState - - 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 + /** 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 { + /** + * Upload has made progress. + * + * @property percentage What percent of the file has been uploaded. + */ + data class ProgressEvent(val percentage: Int) : UploadEvent + + /** + * Upload has finished. + * + * @property media The uploaded media + */ + data class FinishedEvent(val media: UploadedMedia) : UploadEvent +} + data class UploadData( - val flow: Flow>, + val flow: Flow>, val scope: CoroutineScope, ) @@ -243,29 +231,28 @@ class MediaUploader @Inject constructor( return mostRecentId++ } - /** - * 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 + 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 ?: 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 { @@ -384,7 +371,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) @@ -418,7 +405,7 @@ class MediaUploader @Inject constructor( media.mediaSize, ) { percentage -> if (percentage != lastProgress) { - trySend(Ok(UploadState.Uploading(percentage))) + trySend(Ok(UploadEvent.ProgressEvent(percentage))) } lastProgress = percentage } @@ -437,13 +424,7 @@ class MediaUploader @Inject constructor( val uploadResult = mediaUploadApi.uploadMedia(body, description, focus) .mapEither( - { - if (it.code == 200) { - Uploaded.Processed(it.body.id) - } else { - Uploaded.Processing(it.body.id) - } - }, + { UploadEvent.FinishedEvent(UploadedMedia(it.body.id, it.code == 200)) }, { 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 c007e5593..1acc3d1b9 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,12 +68,10 @@ 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, serverId, input.text.toString()) + listener.onUpdateDescription(localId, input.text.toString()) } .setNegativeButton(android.R.string.cancel, null) .create() @@ -127,25 +125,22 @@ class CaptionDialog : DialogFragment() { } interface Listener { - fun onUpdateDescription(localId: Int, serverId: String?, description: String) + fun onUpdateDescription(localId: Int, 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 9e8ac10eb..1fc2daa19 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,22 +24,12 @@ 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( @@ -47,7 +37,7 @@ class ProgressImageView attrs: AttributeSet? = null, defStyleAttr: Int = 0, ) : MediaPreviewImageView(context, attrs, defStyleAttr) { - private var result: Result = Ok(UploadState.Uploading(0)) + private var progress = -1 private val progressRect = RectF() private val biggerRect = RectF() private val circlePaint = Paint(Paint.ANTI_ALIAS_FLAG).apply { @@ -70,15 +60,14 @@ class ProgressImageView } private val circleRadius = Utils.dpToPx(context, 14) private val circleMargin = Utils.dpToPx(context, 14) - private val uploadErrorRadius = Utils.dpToPx(context, 24) - private val uploadErrorDrawable = makeIcon(context, GoogleMaterial.Icon.gmd_error, 48).apply { - setTint(Color.WHITE) - } - - @OptIn(ExperimentalBadgeUtils::class) - fun setResult(result: Result) { - this.result = result + fun setProgress(progress: Int) { + this.progress = progress + if (progress != -1) { + setColorFilter(Color.rgb(123, 123, 123), PorterDuff.Mode.MULTIPLY) + } else { + clearColorFilter() + } invalidate() } @@ -93,24 +82,6 @@ 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 @@ -136,18 +107,4 @@ 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 b0b9ab118..604ab011d 100644 --- a/app/src/main/java/app/pachli/service/SendStatusService.kt +++ b/app/src/main/java/app/pachli/service/SendStatusService.kt @@ -38,8 +38,6 @@ 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 @@ -147,14 +145,14 @@ class SendStatusService : Service() { // first, wait for media uploads to finish val media = statusToSend.media.map { mediaItem -> if (mediaItem.id == null) { - val uploadState = mediaUploader.waitForUploadToFinish(mediaItem.localId) + val uploadState = mediaUploader.getMediaUploadState(mediaItem.localId) val media = uploadState.getOrElse { Timber.w("failed uploading media: %s", it.fmt(this@SendStatusService)) failSending(statusId) stopSelfWhenDone() return@launch - } - mediaItem.copy(id = media.serverId) + }.media + mediaItem.copy(id = media.mediaId, processed = media.processed) } else { mediaItem } @@ -167,13 +165,15 @@ class SendStatusService : Service() { delay(1000L * mediaCheckRetries) media.forEach { mediaItem -> if (!mediaItem.processed) { - mastodonApi.getMedia(mediaItem.id!!) - .onSuccess { mediaItem.processed = it.code == 200 } - .onFailure { + 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 failSending(statusId) stopSelfWhenDone() return@launch } + } } } mediaCheckRetries++ @@ -190,11 +190,13 @@ class SendStatusService : Service() { media.forEach { mediaItem -> if (mediaItem.processed && (mediaItem.description != null || mediaItem.focus != null)) { mastodonApi.updateMedia(mediaItem.id!!, mediaItem.description, mediaItem.focus?.toMastodonApiString()) - .onFailure { error -> - Timber.w("failed to update media on status send: %s", error) - failOrRetry(error.throwable, statusId) + .fold({ + }, { throwable -> + Timber.w(throwable, "failed to update media on status send") + failOrRetry(throwable, statusId) + return@launch - } + }) } } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index a88e927d0..9458afe24 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -853,7 +853,4 @@ 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 8ac6510eb..e03245664 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? = null, - @Field("focus") focus: String? = null, - ): ApiResult + @Field("description") description: String?, + @Field("focus") focus: String?, + ): NetworkResult @GET("api/v1/media/{mediaId}") suspend fun getMedia( @Path("mediaId") mediaId: String, - ): ApiResult + ): Response @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 ac5f6a2c0..6ecb876d1 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 - %1$s - your server does not support this feature: %1$s + An error occurred: %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