Dialog notifying user of failure when media upload fails (#3135)

* First attempt at user notifications of failure when media upload fails

* Drafts alert displays alert

* ktLint

* Fix defaced 46.json, add 47.json

* Mock draftsNeedUserAlert in MainActivityTest to prevent spurious failure

* Friendlier posts-failed message

* Create DraftsAlert object

* DraftsAlert works

* Not the cleanest, but DraftsAlert works with multiple accounts

* Use plural strings

* KtLint

* Clean up debug prints

* Simplify DraftsAlert per Conny suggestions

* Text change suggested by Conny

* ktLint again

* Back out test changes

* Fix MainActivityTest for new approach

* Tweak debug log

* Do not use GlobalScope for coroutines
This commit is contained in:
mcclure 2023-01-27 14:50:45 -05:00 committed by GitHub
commit b2511d782d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 1138 additions and 2 deletions

View file

@ -76,6 +76,7 @@ import com.keylesspalace.tusky.components.scheduled.ScheduledStatusActivity
import com.keylesspalace.tusky.components.search.SearchActivity
import com.keylesspalace.tusky.databinding.ActivityMainBinding
import com.keylesspalace.tusky.db.AccountEntity
import com.keylesspalace.tusky.db.DraftsAlert
import com.keylesspalace.tusky.entity.Account
import com.keylesspalace.tusky.entity.Notification
import com.keylesspalace.tusky.interfaces.AccountSelectionListener
@ -142,6 +143,9 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidInje
@Inject
lateinit var logoutUsecase: LogoutUsecase
@Inject
lateinit var draftsAlert: DraftsAlert
@Inject
lateinit var developerToolsUseCase: DeveloperToolsUseCase
@ -313,6 +317,9 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidInje
1
)
}
// "Post failed" dialog should display in this activity
draftsAlert.observeInContext(this, true)
}
override fun onResume() {

View file

@ -61,6 +61,7 @@ import com.keylesspalace.tusky.components.compose.ComposeActivity
import com.keylesspalace.tusky.components.report.ReportActivity
import com.keylesspalace.tusky.databinding.ActivityAccountBinding
import com.keylesspalace.tusky.db.AccountEntity
import com.keylesspalace.tusky.db.DraftsAlert
import com.keylesspalace.tusky.di.ViewModelFactory
import com.keylesspalace.tusky.entity.Account
import com.keylesspalace.tusky.entity.Relationship
@ -99,6 +100,8 @@ class AccountActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidI
lateinit var dispatchingAndroidInjector: DispatchingAndroidInjector<Any>
@Inject
lateinit var viewModelFactory: ViewModelFactory
@Inject
lateinit var draftsAlert: DraftsAlert
private val viewModel: AccountViewModel by viewModels { viewModelFactory }
@ -386,6 +389,9 @@ class AccountActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidI
viewModel.noteSaved.observe(this) {
binding.saveNoteInfo.visible(it, View.INVISIBLE)
}
// "Post failed" dialog should display in this activity
draftsAlert.observeInContext(this, true)
}
/**

View file

@ -123,6 +123,7 @@ class DraftHelper @Inject constructor(
attachments = attachments,
poll = poll,
failedToSend = failedToSend,
failedToSendNew = failedToSend,
scheduledAt = scheduledAt,
language = language,
statusId = statusId,

View file

@ -33,6 +33,7 @@ import com.keylesspalace.tusky.R
import com.keylesspalace.tusky.components.compose.ComposeActivity
import com.keylesspalace.tusky.databinding.ActivityDraftsBinding
import com.keylesspalace.tusky.db.DraftEntity
import com.keylesspalace.tusky.db.DraftsAlert
import com.keylesspalace.tusky.di.ViewModelFactory
import com.keylesspalace.tusky.util.parseAsMastodonHtml
import com.keylesspalace.tusky.util.visible
@ -46,6 +47,9 @@ class DraftsActivity : BaseActivity(), DraftActionListener {
@Inject
lateinit var viewModelFactory: ViewModelFactory
@Inject
lateinit var draftsAlert: DraftsAlert
private val viewModel: DraftsViewModel by viewModels { viewModelFactory }
private lateinit var binding: ActivityDraftsBinding
@ -83,6 +87,9 @@ class DraftsActivity : BaseActivity(), DraftActionListener {
adapter.addLoadStateListener {
binding.draftsErrorMessageView.visible(adapter.itemCount == 0)
}
// If a failed post is saved to drafts while this activity is up, do nothing; the user is already in the drafts view.
draftsAlert.observeInContext(this, false)
}
override fun onOpenDraft(draft: DraftEntity) {

View file

@ -31,7 +31,7 @@ import java.io.File;
*/
@Database(entities = { DraftEntity.class, AccountEntity.class, InstanceEntity.class, TimelineStatusEntity.class,
TimelineAccountEntity.class, ConversationEntity.class
}, version = 46)
}, version = 47)
public abstract class AppDatabase extends RoomDatabase {
public abstract AccountDao accountDao();
@ -639,4 +639,11 @@ public abstract class AppDatabase extends RoomDatabase {
database.execSQL("ALTER TABLE `DraftEntity` ADD COLUMN `statusId` TEXT");
}
};
public static final Migration MIGRATION_46_47 = new Migration(46, 47) {
@Override
public void migrate(@NonNull SupportSQLiteDatabase database) {
database.execSQL("ALTER TABLE `DraftEntity` ADD COLUMN `failedToSendNew` INTEGER NOT NULL DEFAULT 0");
}
};
}

View file

@ -15,6 +15,7 @@
package com.keylesspalace.tusky.db
import androidx.lifecycle.LiveData
import androidx.paging.PagingSource
import androidx.room.Dao
import androidx.room.Insert
@ -30,6 +31,12 @@ interface DraftDao {
@Query("SELECT * FROM DraftEntity WHERE accountId = :accountId ORDER BY id ASC")
fun draftsPagingSource(accountId: Long): PagingSource<Int, DraftEntity>
@Query("SELECT COUNT(*) FROM DraftEntity where accountId = :accountId and failedToSendNew=true")
fun draftsNeedUserAlert(accountId: Long): LiveData<Int>
@Query("UPDATE DraftEntity SET failedToSendNew=false where accountId = :accountId and failedToSendNew=true")
suspend fun draftsClearNeedUserAlert(accountId: Long)
@Query("SELECT * FROM DraftEntity WHERE accountId = :accountId")
suspend fun loadDrafts(accountId: Long): List<DraftEntity>

View file

@ -40,6 +40,7 @@ data class DraftEntity(
val attachments: List<DraftAttachment>,
val poll: NewPoll?,
val failedToSend: Boolean,
val failedToSendNew: Boolean,
val scheduledAt: String?,
val language: String?,
val statusId: String?,

View file

@ -0,0 +1,99 @@
/* Copyright 2023 Andi McClure
*
* This file is a part of Tusky.
*
* This program is free software; you can redistribute it and/or modify it under the terms of the
* GNU General Public License as published by the Free Software Foundation; either version 3 of the
* License, or (at your option) any later version.
*
* Tusky is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even
* the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General
* Public License for more details.
*
* You should have received a copy of the GNU General Public License along with Tusky; if not,
* see <http://www.gnu.org/licenses>. */
package com.keylesspalace.tusky.db
import android.content.Context
import android.content.DialogInterface
import android.util.Log
import androidx.appcompat.app.AlertDialog
import androidx.lifecycle.LifecycleCoroutineScope
import androidx.lifecycle.LifecycleOwner
import androidx.lifecycle.lifecycleScope
import com.keylesspalace.tusky.R
import com.keylesspalace.tusky.components.drafts.DraftsActivity
import kotlinx.coroutines.launch
import javax.inject.Inject
import javax.inject.Singleton
/**
* This class manages an alert popup when a post has failed and been saved to drafts.
* It must be separately registered in each lifetime in which it is to appear,
* and it only appears if the post failure belongs to the current user.
*/
private const val TAG = "DraftsAlert"
@Singleton
class DraftsAlert @Inject constructor(db: AppDatabase) {
// For tracking when a media upload fails in the service
private val draftDao: DraftDao = db.draftDao()
@Inject
lateinit var accountManager: AccountManager
public fun <T> observeInContext(context: T, showAlert: Boolean) where T : Context, T : LifecycleOwner {
accountManager.activeAccount?.let { activeAccount ->
val coroutineScope = context.lifecycleScope
// Assume a single MainActivity, AccountActivity or DraftsActivity never sees more then one user id in its lifetime.
val activeAccountId = activeAccount.id
// This LiveData will be automatically disposed when the activity is destroyed.
val draftsNeedUserAlert = draftDao.draftsNeedUserAlert(activeAccountId)
// observe ensures that this gets called at the most appropriate moment wrt the context lifecycle—
// at init, at next onResume, or immediately if the context is resumed already.
if (showAlert) {
draftsNeedUserAlert.observe(context) { count ->
Log.d(TAG, "User id $activeAccountId changed: Notification-worthy draft count $count")
if (count > 0) {
AlertDialog.Builder(context)
.setTitle(R.string.action_post_failed)
.setMessage(
context.getResources().getQuantityString(R.plurals.action_post_failed_detail, count)
)
.setPositiveButton(R.string.action_post_failed_show_drafts) { _: DialogInterface?, _: Int ->
clearDraftsAlert(coroutineScope, activeAccountId) // User looked at drafts
val intent = DraftsActivity.newIntent(context)
context.startActivity(intent)
}
.setNegativeButton(R.string.action_post_failed_do_nothing) { _: DialogInterface?, _: Int ->
clearDraftsAlert(coroutineScope, activeAccountId) // User doesn't care
}
.show()
}
}
} else {
draftsNeedUserAlert.observe(context) { _ ->
Log.d(TAG, "User id $activeAccountId: Clean out notification-worthy drafts")
clearDraftsAlert(coroutineScope, activeAccountId)
}
}
} ?: run {
Log.w(TAG, "Attempted to observe drafts, but there is no active account")
}
}
/**
* Clear drafts alert for specified user
*/
fun clearDraftsAlert(coroutineScope: LifecycleCoroutineScope, id: Long) {
coroutineScope.launch {
draftDao.draftsClearNeedUserAlert(id)
}
}
}

View file

@ -67,7 +67,7 @@ class AppModule {
AppDatabase.MIGRATION_35_36, AppDatabase.MIGRATION_36_37, AppDatabase.MIGRATION_37_38,
AppDatabase.MIGRATION_38_39, AppDatabase.MIGRATION_39_40, AppDatabase.MIGRATION_40_41,
AppDatabase.MIGRATION_41_42, AppDatabase.MIGRATION_42_43, AppDatabase.MIGRATION_43_44,
AppDatabase.MIGRATION_44_45, AppDatabase.MIGRATION_45_46,
AppDatabase.MIGRATION_44_45, AppDatabase.MIGRATION_45_46, AppDatabase.MIGRATION_46_47
)
.build()
}

View file

@ -0,0 +1,6 @@
<resources>
<plurals name="action_post_failed_detail">
<item quantity="one">@string/action_post_failed_detail</item>
<item quantity="other">@string/action_post_failed_detail_plural</item>
</plurals>
</resources>

View file

@ -100,6 +100,11 @@
<string name="action_browser_login">Login with Browser</string>
<string name="action_logout">Log out</string>
<string name="action_logout_confirm">Are you sure you want to log out of the account %1$s?</string>
<string name="action_post_failed">Upload failed</string>
<string name="action_post_failed_detail">Your post failed to upload and has been saved to drafts.\n\nEither the server could not be contacted, or it rejected the post.</string>
<string name="action_post_failed_detail_plural">Your posts failed to upload and have been saved to drafts.\n\nEither the server could not be contacted, or it rejected the posts.</string>
<string name="action_post_failed_show_drafts">Show drafts</string>
<string name="action_post_failed_do_nothing">Dismiss</string>
<string name="action_follow">Follow</string>
<string name="action_unfollow">Unfollow</string>
<string name="action_block">Block</string>

View file

@ -121,6 +121,7 @@ class MainActivityTest {
activity.accountManager = mock {
on { activeAccount } doReturn accountEntity
}
activity.draftsAlert = mock {}
activity.mastodonApi = mock {
onBlocking { accountVerifyCredentials() } doReturn NetworkResult.success(account)
onBlocking { listAnnouncements(false) } doReturn NetworkResult.success(emptyList())