Fix missing toast message after write external storage permission is denied while saving an attachment.

`AttachmentSaver` was missing logic to show a toast message after the user denies `WRITE_EXTERNAL_STORAGE` permission.

#### Changeset
- Add missing toast after write external storage permission is denied.
- Add unit test coverage for `AttachmentSaver` result messages.
- Rename `AttachmentSaver` string resource names so they all have the same prefix.
This commit is contained in:
Jeffrey Starke
2025-03-24 14:19:02 -04:00
committed by Cody Henthorne
parent b6f98521c8
commit 0f72c6face
10 changed files with 295 additions and 257 deletions

View File

@@ -6,7 +6,6 @@
package org.thoughtcrime.securesms.attachments
import android.Manifest
import android.content.Context
import android.widget.CheckBox
import android.widget.Toast
import androidx.fragment.app.Fragment
@@ -27,6 +26,7 @@ import org.thoughtcrime.securesms.keyvalue.SignalStore
import org.thoughtcrime.securesms.permissions.Permissions
import org.thoughtcrime.securesms.util.SaveAttachmentUtil
import org.thoughtcrime.securesms.util.SaveAttachmentUtil.SaveAttachment
import org.thoughtcrime.securesms.util.SaveAttachmentUtil.SaveAttachmentsResult
import org.thoughtcrime.securesms.util.StorageUtil
import kotlin.coroutines.resume
import kotlin.coroutines.suspendCoroutine
@@ -63,6 +63,7 @@ class AttachmentSaver(private val host: Host) {
saveToStorage(attachments)
} else {
Log.d(TAG, "Cancel saving ${attachments.size} attachments: media store permission denied.")
host.showSaveResult(SaveAttachmentsResult.WriteStoragePermissionDenied)
}
} else {
Log.d(TAG, "Cancel saving ${attachments.size} attachments: save to storage warning denied.")
@@ -83,12 +84,12 @@ class AttachmentSaver(private val host: Host) {
return host.requestWriteExternalStoragePermission()
}
private suspend fun saveToStorage(attachments: Set<SaveAttachment>): SaveAttachmentUtil.SaveAttachmentsResult {
private suspend fun saveToStorage(attachments: Set<SaveAttachment>): SaveAttachmentsResult {
host.showSaveProgress(attachmentCount = attachments.size)
return try {
val result = SaveAttachmentUtil.saveAttachments(attachments)
withContext(SignalDispatchers.Main) {
host.showToast { context -> result.getMessage(context) }
host.showSaveResult(result)
}
result
} finally {
@@ -101,23 +102,23 @@ class AttachmentSaver(private val host: Host) {
interface Host {
suspend fun showSaveToStorageWarning(attachmentCount: Int): SaveToStorageWarningResult
suspend fun requestWriteExternalStoragePermission(): RequestPermissionResult
fun showToast(getMessage: (Context) -> CharSequence)
fun showSaveProgress(attachmentCount: Int)
fun showSaveResult(result: SaveAttachmentsResult)
fun dismissSaveProgress()
}
data class FragmentHost(private val fragment: Fragment) : Host {
override fun showToast(getMessage: (Context) -> CharSequence) {
Toast.makeText(fragment.requireContext(), getMessage(fragment.requireContext()), Toast.LENGTH_LONG).show()
override fun showSaveResult(result: SaveAttachmentsResult) {
Toast.makeText(fragment.requireContext(), result.getMessage(fragment.requireContext()), Toast.LENGTH_LONG).show()
}
override suspend fun showSaveToStorageWarning(attachmentCount: Int): SaveToStorageWarningResult = withContext(SignalDispatchers.Main) {
val dialog = MaterialAlertDialogBuilder(fragment.requireContext())
.setView(R.layout.dialog_save_attachment)
.setTitle(R.string.ConversationFragment__save_to_phone)
.setTitle(R.string.AttachmentSaver__save_to_phone)
.setCancelable(true)
.setMessage(fragment.resources.getQuantityString(R.plurals.ConversationFragment__this_media_will_be_saved, attachmentCount, attachmentCount))
.setMessage(fragment.resources.getQuantityString(R.plurals.AttachmentSaver__this_media_will_be_saved, attachmentCount, attachmentCount))
.create()
val result = dialog.awaitResult(
@@ -140,7 +141,7 @@ class AttachmentSaver(private val host: Host) {
Permissions.with(fragment)
.request(Manifest.permission.WRITE_EXTERNAL_STORAGE)
.ifNecessary()
.withPermanentDenialDialog(fragment.getString(R.string.MediaPreviewActivity_signal_needs_the_storage_permission_in_order_to_write_to_external_storage_but_it_has_been_permanently_denied))
.withPermanentDenialDialog(fragment.getString(R.string.AttachmentSaver__signal_needs_the_storage_permission_in_order_to_write_to_external_storage_but_it_has_been_permanently_denied))
.onAnyDenied {
Log.d(TAG, "WRITE_EXTERNAL_STORAGE permission request denied.")
continuation.resume(RequestPermissionResult.DENIED)

View File

@@ -47,8 +47,8 @@ final class MediaActions {
SaveAttachmentTask.showWarningDialogIfNecessary(context, mediaRecords.size(), () -> Permissions.with(fragment)
.request(Manifest.permission.WRITE_EXTERNAL_STORAGE)
.ifNecessary()
.withPermanentDenialDialog(fragment.getString(R.string.MediaPreviewActivity_signal_needs_the_storage_permission_in_order_to_write_to_external_storage_but_it_has_been_permanently_denied))
.onAnyDenied(() -> Toast.makeText(context, R.string.MediaPreviewActivity_unable_to_write_to_external_storage_without_permission, Toast.LENGTH_LONG).show())
.withPermanentDenialDialog(fragment.getString(R.string.AttachmentSaver__signal_needs_the_storage_permission_in_order_to_write_to_external_storage_but_it_has_been_permanently_denied))
.onAnyDenied(() -> Toast.makeText(context, R.string.AttachmentSaver__unable_to_write_to_external_storage_without_permission, Toast.LENGTH_LONG).show())
.onAllGranted(() -> performSaveToDisk(context, mediaRecords, postExecute))
.execute());
}

View File

@@ -49,19 +49,19 @@ object StoryContextMenu {
).map { (_, deletedThread) -> deletedThread }
}
suspend fun save(host: AttachmentSaver.Host, messageRecord: MessageRecord) {
suspend fun save(fragment: Fragment, messageRecord: MessageRecord) {
val mediaMessageRecord = messageRecord as? MmsMessageRecord
val uri: Uri? = mediaMessageRecord?.slideDeck?.firstSlide?.uri
val contentType: String? = mediaMessageRecord?.slideDeck?.firstSlide?.contentType
when {
mediaMessageRecord?.storyType?.isTextStory == true -> saveTextStory(host, mediaMessageRecord)
uri == null || contentType == null -> showErrorCantSaveStory(host, uri, contentType)
else -> saveMediaStory(host, uri, contentType, mediaMessageRecord)
mediaMessageRecord?.storyType?.isTextStory == true -> saveTextStory(fragment, mediaMessageRecord)
uri == null || contentType == null -> showErrorCantSaveStory(fragment, uri, contentType)
else -> saveMediaStory(fragment, uri, contentType, mediaMessageRecord)
}
}
private suspend fun saveTextStory(host: AttachmentSaver.Host, messageRecord: MmsMessageRecord) {
private suspend fun saveTextStory(fragment: Fragment, messageRecord: MmsMessageRecord) {
val saveAttachment = withContext(Dispatchers.Main) {
val model = StoryTextPostModel.parseFrom(messageRecord)
val decoder = StoryTextPostModel.Decoder()
@@ -78,17 +78,17 @@ object StoryContextMenu {
)
}
AttachmentSaver(host).saveAttachments(setOf(saveAttachment))
AttachmentSaver(fragment).saveAttachments(setOf(saveAttachment))
}
private suspend fun saveMediaStory(host: AttachmentSaver.Host, uri: Uri, contentType: String, mediaMessageRecord: MmsMessageRecord) {
private suspend fun saveMediaStory(fragment: Fragment, uri: Uri, contentType: String, mediaMessageRecord: MmsMessageRecord) {
val saveAttachment = SaveAttachmentUtil.SaveAttachment(uri = uri, contentType = contentType, date = mediaMessageRecord.dateSent, fileName = null)
AttachmentSaver(host).saveAttachments(setOf(saveAttachment))
AttachmentSaver(fragment).saveAttachments(setOf(saveAttachment))
}
private fun showErrorCantSaveStory(host: AttachmentSaver.Host, uri: Uri?, contentType: String?) {
private fun showErrorCantSaveStory(fragment: Fragment, uri: Uri?, contentType: String?) {
Log.w(TAG, "Unable to save story media uri: $uri contentType: $contentType")
host.showToast { context -> context.getString(R.string.MyStories__unable_to_save) }
Toast.makeText(fragment.requireContext(), R.string.MyStories__unable_to_save, Toast.LENGTH_LONG).show()
}
fun share(fragment: Fragment, messageRecord: MmsMessageRecord) {

View File

@@ -28,7 +28,6 @@ import kotlinx.coroutines.launch
import org.signal.core.util.concurrent.LifecycleDisposable
import org.thoughtcrime.securesms.MainActivity
import org.thoughtcrime.securesms.R
import org.thoughtcrime.securesms.attachments.AttachmentSaver
import org.thoughtcrime.securesms.banner.BannerManager
import org.thoughtcrime.securesms.banner.banners.DeprecatedBuildBanner
import org.thoughtcrime.securesms.banner.banners.UnauthorizedBanner
@@ -331,7 +330,7 @@ class StoriesLandingFragment : DSLSettingsFragment(layoutId = R.layout.stories_l
onSave = {
lifecycleScope.launch {
StoryContextMenu.save(
host = AttachmentSaver.FragmentHost(this@StoriesLandingFragment),
fragment = this@StoriesLandingFragment,
messageRecord = it.data.primaryStory.messageRecord
)
}

View File

@@ -10,7 +10,6 @@ import androidx.lifecycle.lifecycleScope
import kotlinx.coroutines.launch
import org.signal.core.util.concurrent.LifecycleDisposable
import org.thoughtcrime.securesms.R
import org.thoughtcrime.securesms.attachments.AttachmentSaver
import org.thoughtcrime.securesms.components.settings.DSLConfiguration
import org.thoughtcrime.securesms.components.settings.DSLSettingsFragment
import org.thoughtcrime.securesms.components.settings.DSLSettingsText
@@ -84,7 +83,7 @@ class MyStoriesFragment : DSLSettingsFragment(
onSaveClick = {
lifecycleScope.launch {
StoryContextMenu.save(
host = AttachmentSaver.FragmentHost(this@MyStoriesFragment),
fragment = this@MyStoriesFragment,
messageRecord = it.distributionStory.messageRecord
)
}

View File

@@ -49,7 +49,6 @@ import org.signal.core.util.getParcelableCompat
import org.signal.core.util.logging.Log
import org.thoughtcrime.securesms.R
import org.thoughtcrime.securesms.animation.AnimationCompleteListener
import org.thoughtcrime.securesms.attachments.AttachmentSaver
import org.thoughtcrime.securesms.components.AvatarImageView
import org.thoughtcrime.securesms.components.emoji.EmojiTextView
import org.thoughtcrime.securesms.components.segmentedprogressbar.SegmentedProgressBar
@@ -1200,7 +1199,7 @@ class StoryViewerPageFragment :
lifecycleScope.launch {
viewModel.setIsSavingMedia(true)
StoryContextMenu.save(
host = AttachmentSaver.FragmentHost(this@StoryViewerPageFragment),
fragment = this@StoryViewerPageFragment,
messageRecord = it.conversationMessage.messageRecord
)
viewModel.setIsSavingMedia(false)

View File

@@ -447,9 +447,9 @@ public class SaveAttachmentTask extends ProgressDialogAsyncTask<SaveAttachmentTa
} else {
new MaterialAlertDialogBuilder(context)
.setView(R.layout.dialog_save_attachment)
.setTitle(R.string.ConversationFragment__save_to_phone)
.setTitle(R.string.AttachmentSaver__save_to_phone)
.setCancelable(true)
.setMessage(context.getResources().getQuantityString(R.plurals.ConversationFragment__this_media_will_be_saved, count, count))
.setMessage(context.getResources().getQuantityString(R.plurals.AttachmentSaver__this_media_will_be_saved, count, count))
.setPositiveButton(R.string.save, ((dialog, i) -> {
CheckBox checkbox = ((AlertDialog) dialog).findViewById(R.id.checkbox);
if (checkbox.isChecked()) {

View File

@@ -21,6 +21,7 @@ import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext
import org.signal.core.util.StreamUtil
import org.signal.core.util.logging.Log
import org.signal.core.util.logging.logI
import org.thoughtcrime.securesms.R
import org.thoughtcrime.securesms.dependencies.AppDependencies
import org.thoughtcrime.securesms.mms.PartAuthority
@@ -50,21 +51,20 @@ object SaveAttachmentUtil {
check(attachments.isNotEmpty()) { "must pass in at least one attachment" }
if (!StorageUtil.canWriteToMediaStore()) {
return SaveAttachmentsResult.ErrorNoWriteAccess(errorCount = attachments.size)
return SaveAttachmentsResult.ErrorNoWriteAccess
}
val nameCache: BatchOperationNameCache = HashMap()
val (successes, errors) = attachments
val (successes, failures) = attachments
.map { saveAttachment(it, nameCache) }
.partition { saveResult -> saveResult is SaveAttachmentResult.Success }
return SaveAttachmentsResult.Completed(
successCount = successes.size,
errorCount = errors.size
).also {
Log.i(TAG, "Save attachments completed (${it.successCount} of ${attachments.size} saved successfully).")
}
return when {
failures.isEmpty() -> SaveAttachmentsResult.Success(successesCount = successes.size)
successes.isEmpty() -> SaveAttachmentsResult.Failure(failuresCount = failures.size)
else -> SaveAttachmentsResult.PartialSuccess(successesCount = successes.size, failuresCount = failures.size)
}.logI(TAG, "Save attachments completed (${successes.size} of ${attachments.size} saved successfully).")
}
private suspend fun saveAttachment(attachment: SaveAttachment, nameCache: BatchOperationNameCache): SaveAttachmentResult = withContext(Dispatchers.IO) {
@@ -284,38 +284,39 @@ object SaveAttachmentUtil {
}
sealed interface SaveAttachmentsResult {
val successCount: Int
val errorCount: Int
fun getMessage(context: Context): CharSequence
data class Completed(
override val successCount: Int,
override val errorCount: Int
) : SaveAttachmentsResult {
data class Success(val successesCount: Int) : SaveAttachmentsResult {
override fun getMessage(context: Context): CharSequence {
return when {
errorCount == 0 -> context.resources.getQuantityText(R.plurals.SaveAttachment_saved_success, successCount)
successCount == 0 -> context.resources.getQuantityText(R.plurals.SaveAttachment_error_while_saving_attachments_to_sd_card, errorCount)
else -> {
val numberFormat = NumberFormat.getInstance()
context.resources.getQuantityString(
R.plurals.SaveAttachment_saved_success_n_failures,
errorCount,
numberFormat.format(errorCount),
numberFormat.format(errorCount + successCount)
)
}
}
return context.resources.getQuantityText(R.plurals.SaveAttachment_saved_success, successesCount)
}
}
data class ErrorNoWriteAccess(
override val errorCount: Int
) : SaveAttachmentsResult {
override val successCount: Int = 0
data class PartialSuccess(val successesCount: Int, val failuresCount: Int) : SaveAttachmentsResult {
override fun getMessage(context: Context): CharSequence {
val numberFormat = NumberFormat.getInstance()
return context.resources.getQuantityString(
R.plurals.SaveAttachment_saved_success_n_failures,
failuresCount,
numberFormat.format(failuresCount),
numberFormat.format(failuresCount + successesCount)
)
}
}
data class Failure(val failuresCount: Int) : SaveAttachmentsResult {
override fun getMessage(context: Context): CharSequence {
return context.resources.getQuantityText(R.plurals.SaveAttachment_error_while_saving_attachments_to_sd_card, failuresCount)
}
}
data object WriteStoragePermissionDenied : SaveAttachmentsResult {
override fun getMessage(context: Context): CharSequence {
return context.getString(R.string.AttachmentSaver__unable_to_write_to_external_storage_without_permission)
}
}
data object ErrorNoWriteAccess : SaveAttachmentsResult {
override fun getMessage(context: Context): CharSequence {
return context.getString(R.string.SaveAttachment_unable_to_write_to_sd_card_exclamation)
}