Fix some filter bugs (#4501)

closes #4499 

This restores support for v1 filters. The problem was that the state was
uncoditionally set to error instead of checking the v1 response.
While checking the code I found some other problems:
- Two error messages that were shown to users were not translatable
- When filters were updated sometimes `PreferenceChangedEvent` was sent
instead of `FilterUpdatedEvent`
- The notifications fragment was not listening to the
`FilterUpdatedEvent`
This commit is contained in:
Konrad Pozniak 2024-06-12 17:17:08 +02:00 committed by GitHub
commit 8d65feadd6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 36 additions and 26 deletions

View file

@ -26,7 +26,7 @@ import androidx.lifecycle.lifecycleScope
import at.connyduck.calladapter.networkresult.fold
import com.google.android.material.snackbar.Snackbar
import com.keylesspalace.tusky.appstore.EventHub
import com.keylesspalace.tusky.appstore.PreferenceChangedEvent
import com.keylesspalace.tusky.appstore.FilterUpdatedEvent
import com.keylesspalace.tusky.components.filters.EditFilterActivity
import com.keylesspalace.tusky.components.filters.FiltersActivity
import com.keylesspalace.tusky.components.timeline.TimelineFragment
@ -263,8 +263,7 @@ class StatusListActivity : BottomSheetActivity() {
// must be requested again; otherwise does not contain the keyword (but server does)
mutedFilter = mastodonApi.getFilter(filter.id).getOrNull()
// TODO the preference key here ("home") is not meaningful; should probably be another event if any
eventHub.dispatch(PreferenceChangedEvent(filter.context[0]))
eventHub.dispatch(FilterUpdatedEvent(filter.context))
filterCreateSuccess = true
} else {
Snackbar.make(
@ -286,7 +285,7 @@ class StatusListActivity : BottomSheetActivity() {
).fold(
{ filter ->
mutedFilterV1 = filter
eventHub.dispatch(PreferenceChangedEvent(filter.context[0]))
eventHub.dispatch(FilterUpdatedEvent(filter.context))
filterCreateSuccess = true
},
{ throwable2 ->
@ -372,7 +371,7 @@ class StatusListActivity : BottomSheetActivity() {
result?.fold(
{
updateTagMuteState(false)
eventHub.dispatch(PreferenceChangedEvent(Filter.Kind.HOME.kind))
eventHub.dispatch(FilterUpdatedEvent(listOf(Filter.Kind.HOME.kind)))
mutedFilterV1 = null
mutedFilter = null

View file

@ -276,7 +276,7 @@ class EditFilterActivity : BaseActivity() {
} else {
Snackbar.make(
binding.root,
"Error saving filter '${viewModel.title.value}'",
getString(R.string.error_deleting_filter, viewModel.title.value),
Snackbar.LENGTH_SHORT
).show()
}
@ -299,7 +299,7 @@ class EditFilterActivity : BaseActivity() {
{
Snackbar.make(
binding.root,
"Error deleting filter '${filter.title}'",
getString(R.string.error_deleting_filter, filter.title),
Snackbar.LENGTH_SHORT
).show()
}
@ -307,7 +307,7 @@ class EditFilterActivity : BaseActivity() {
} else {
Snackbar.make(
binding.root,
"Error deleting filter '${filter.title}'",
getString(R.string.error_deleting_filter, filter.title),
Snackbar.LENGTH_SHORT
).show()
}

View file

@ -4,7 +4,6 @@ import android.content.Context
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import at.connyduck.calladapter.networkresult.fold
import com.keylesspalace.tusky.appstore.EventHub
import com.keylesspalace.tusky.entity.Filter
import com.keylesspalace.tusky.entity.FilterKeyword
import com.keylesspalace.tusky.network.MastodonApi
@ -17,7 +16,7 @@ import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.withContext
@HiltViewModel
class EditFilterViewModel @Inject constructor(val api: MastodonApi, val eventHub: EventHub) : ViewModel() {
class EditFilterViewModel @Inject constructor(val api: MastodonApi) : ViewModel() {
private var originalFilter: Filter? = null
private val _title = MutableStateFlow("")

View file

@ -1,12 +1,14 @@
package com.keylesspalace.tusky.components.filters
import android.util.Log
import android.view.View
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import at.connyduck.calladapter.networkresult.fold
import com.google.android.material.snackbar.Snackbar
import com.keylesspalace.tusky.R
import com.keylesspalace.tusky.appstore.EventHub
import com.keylesspalace.tusky.appstore.PreferenceChangedEvent
import com.keylesspalace.tusky.appstore.FilterUpdatedEvent
import com.keylesspalace.tusky.entity.Filter
import com.keylesspalace.tusky.network.MastodonApi
import com.keylesspalace.tusky.util.isHttpNotFound
@ -49,26 +51,28 @@ class FiltersViewModel @Inject constructor(
private suspend fun observeLoad() {
loadTrigger.collectLatest {
this@FiltersViewModel._state.update { it.copy(loadingState = LoadingState.LOADING) }
_state.update { it.copy(loadingState = LoadingState.LOADING) }
api.getFilters().fold(
{ filters ->
this@FiltersViewModel._state.value = State(filters, LoadingState.LOADED)
_state.value = State(filters, LoadingState.LOADED)
},
{ throwable ->
if (throwable.isHttpNotFound()) {
Log.i(TAG, "failed loading filters v2, falling back to v1", throwable)
api.getFiltersV1().fold(
{ filters ->
this@FiltersViewModel._state.value = State(filters.map { it.toFilter() }, LoadingState.LOADED)
_state.value = State(filters.map { it.toFilter() }, LoadingState.LOADED)
},
{ _ ->
// TODO log errors (also below)
this@FiltersViewModel._state.update { it.copy(loadingState = LoadingState.ERROR_OTHER) }
{ t ->
Log.w(TAG, "failed loading filters v1", t)
_state.value = State(emptyList(), LoadingState.ERROR_OTHER)
}
)
this@FiltersViewModel._state.update { it.copy(loadingState = LoadingState.ERROR_OTHER) }
} else {
this@FiltersViewModel._state.update { it.copy(loadingState = LoadingState.ERROR_NETWORK) }
Log.w(TAG, "failed loading filters v2", throwable)
_state.update { it.copy(loadingState = LoadingState.ERROR_NETWORK) }
}
}
)
@ -85,21 +89,19 @@ class FiltersViewModel @Inject constructor(
api.deleteFilter(filter.id).fold(
{
this@FiltersViewModel._state.update { currentState ->
_state.update { currentState ->
State(
currentState.filters.filter { it.id != filter.id },
LoadingState.LOADED
)
}
for (context in filter.context) {
eventHub.dispatch(PreferenceChangedEvent(context))
}
eventHub.dispatch(FilterUpdatedEvent(filter.context))
},
{ throwable ->
if (throwable.isHttpNotFound()) {
api.deleteFilterV1(filter.id).fold(
{
this@FiltersViewModel._state.update { currentState ->
_state.update { currentState ->
State(
currentState.filters.filter { it.id != filter.id },
LoadingState.LOADED
@ -109,7 +111,7 @@ class FiltersViewModel @Inject constructor(
{
Snackbar.make(
parent,
"Error deleting filter '${filter.title}'",
parent.context.getString(R.string.error_deleting_filter, filter.title),
Snackbar.LENGTH_SHORT
).show()
}
@ -117,11 +119,15 @@ class FiltersViewModel @Inject constructor(
} else {
Snackbar.make(
parent,
"Error deleting filter '${filter.title}'",
parent.context.getString(R.string.error_deleting_filter, filter.title),
Snackbar.LENGTH_SHORT
).show()
}
}
)
}
companion object {
private const val TAG = "FiltersViewModel"
}
}

View file

@ -31,6 +31,7 @@ import at.connyduck.calladapter.networkresult.fold
import at.connyduck.calladapter.networkresult.map
import at.connyduck.calladapter.networkresult.onFailure
import com.keylesspalace.tusky.appstore.EventHub
import com.keylesspalace.tusky.appstore.FilterUpdatedEvent
import com.keylesspalace.tusky.appstore.PreferenceChangedEvent
import com.keylesspalace.tusky.components.preference.PreferencesFragment.ReadingOrder
import com.keylesspalace.tusky.components.timeline.Placeholder
@ -125,6 +126,9 @@ class NotificationsViewModel @Inject constructor(
if (event is PreferenceChangedEvent) {
onPreferenceChanged(event.preferenceKey)
}
if (event is FilterUpdatedEvent && event.filterContext.contains(Filter.Kind.NOTIFICATIONS.kind)) {
refreshTrigger.value += 1
}
}
}
filterModel.kind = Filter.Kind.NOTIFICATIONS