fix updating filter expiration to indefinite (#4743)

Before we would not send `expires_in` when "indefinite" was selected.
But that left the expiration at the value it was before. To actually set
it to indefinite we need to send `expires_in`, but leave it empty.
With a value class this was actually really nice to fix, the code now
self-documents what the special values mean.

Also fixes a regression from the Material 3 redesign where the filter
duration drop down would not get populated when creating a filter.

Found while working on https://github.com/tuskyapp/Tusky/pull/4742
This commit is contained in:
Konrad Pozniak 2024-11-05 20:44:08 +01:00 committed by GitHub
parent 0d34804359
commit e758321866
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 108 additions and 52 deletions

View File

@ -28,6 +28,7 @@ import com.google.android.material.snackbar.Snackbar
import com.keylesspalace.tusky.appstore.EventHub
import com.keylesspalace.tusky.appstore.FilterUpdatedEvent
import com.keylesspalace.tusky.components.filters.EditFilterActivity
import com.keylesspalace.tusky.components.filters.FilterExpiration
import com.keylesspalace.tusky.components.filters.FiltersActivity
import com.keylesspalace.tusky.components.timeline.TimelineFragment
import com.keylesspalace.tusky.components.timeline.viewmodel.TimelineViewModel.Kind
@ -251,7 +252,7 @@ class StatusListActivity : BottomSheetActivity() {
title = "#$tag",
context = listOf(Filter.Kind.HOME.kind),
filterAction = Filter.Action.WARN.action,
expiresInSeconds = null
expiresIn = FilterExpiration.never
).fold(
{ filter ->
if (mastodonApi.addFilterKeyword(
@ -281,7 +282,7 @@ class StatusListActivity : BottomSheetActivity() {
listOf(Filter.Kind.HOME.kind),
irreversible = false,
wholeWord = true,
expiresInSeconds = null
expiresIn = FilterExpiration.never
).fold(
{ filter ->
mutedFilterV1 = filter
@ -358,7 +359,7 @@ class StatusListActivity : BottomSheetActivity() {
context = filter.context.filter { it != Filter.Kind.HOME.kind },
irreversible = null,
wholeWord = null,
expiresInSeconds = null
expiresIn = FilterExpiration.never
)
} else {
mastodonApi.deleteFilterV1(filter.id)

View File

@ -1,6 +1,20 @@
/* Copyright 2024 Tusky contributors
*
* 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.components.filters
import android.content.Context
import android.content.DialogInterface.BUTTON_POSITIVE
import android.os.Bundle
import android.view.WindowManager
@ -28,7 +42,6 @@ import com.keylesspalace.tusky.util.isHttpNotFound
import com.keylesspalace.tusky.util.viewBinding
import com.keylesspalace.tusky.util.visible
import dagger.hilt.android.AndroidEntryPoint
import java.util.Date
import javax.inject.Inject
import kotlinx.coroutines.launch
@ -124,6 +137,7 @@ class EditFilterActivity : BaseActivity() {
if (originalFilter == null) {
binding.filterActionWarn.isChecked = true
initializeDurationDropDown(false)
} else {
loadFilter()
}
@ -165,7 +179,11 @@ class EditFilterActivity : BaseActivity() {
// Populate the UI from the filter's members
private fun loadFilter() {
viewModel.load(filter)
val durationNames = if (filter.expiresAt != null) {
initializeDurationDropDown(withNoChange = filter.expiresAt != null)
}
private fun initializeDurationDropDown(withNoChange: Boolean) {
val durationNames = if (withNoChange) {
arrayOf(getString(R.string.duration_no_change)) + resources.getStringArray(R.array.filter_duration_names)
} else {
resources.getStringArray(R.array.filter_duration_names)
@ -322,19 +340,5 @@ class EditFilterActivity : BaseActivity() {
companion object {
const val FILTER_TO_EDIT = "FilterToEdit"
// Mastodon *stores* the absolute date in the filter,
// but create/edit take a number of seconds (relative to the time the operation is posted)
fun getSecondsForDurationIndex(index: Int, context: Context?, default: Date? = null): Int? {
return when (index) {
-1 -> if (default == null) {
default
} else {
((default.time - System.currentTimeMillis()) / 1000).toInt()
}
0 -> null
else -> context?.resources?.getIntArray(R.array.filter_duration_values)?.get(index)
}
}
}
}

View File

@ -1,9 +1,25 @@
/* Copyright 2024 Tusky contributors
*
* 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.components.filters
import android.content.Context
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import at.connyduck.calladapter.networkresult.fold
import com.keylesspalace.tusky.R
import com.keylesspalace.tusky.entity.Filter
import com.keylesspalace.tusky.entity.FilterKeyword
import com.keylesspalace.tusky.network.MastodonApi
@ -112,12 +128,12 @@ class EditFilterViewModel @Inject constructor(val api: MastodonApi) : ViewModel(
durationIndex: Int,
context: Context
): Boolean {
val expiresInSeconds = EditFilterActivity.getSecondsForDurationIndex(durationIndex, context)
val expiration = getExpirationForDurationIndex(durationIndex, context)
api.createFilter(
title = title,
context = contexts,
filterAction = action,
expiresInSeconds = expiresInSeconds
expiresIn = expiration
).fold(
{ newFilter ->
// This is _terrible_, but the all-in-one update filter api Just Doesn't Work
@ -133,7 +149,7 @@ class EditFilterViewModel @Inject constructor(val api: MastodonApi) : ViewModel(
return (
throwable.isHttpNotFound() &&
// Endpoint not found, fall back to v1 api
createFilterV1(contexts, expiresInSeconds)
createFilterV1(contexts, expiration)
)
}
)
@ -147,13 +163,13 @@ class EditFilterViewModel @Inject constructor(val api: MastodonApi) : ViewModel(
durationIndex: Int,
context: Context
): Boolean {
val expiresInSeconds = EditFilterActivity.getSecondsForDurationIndex(durationIndex, context)
val expiration = getExpirationForDurationIndex(durationIndex, context)
api.updateFilter(
id = originalFilter.id,
title = title,
context = contexts,
filterAction = action,
expiresInSeconds = expiresInSeconds
expires = expiration
).fold(
{
// This is _terrible_, but the all-in-one update filter api Just Doesn't Work
@ -173,7 +189,7 @@ class EditFilterViewModel @Inject constructor(val api: MastodonApi) : ViewModel(
{ throwable ->
if (throwable.isHttpNotFound()) {
// Endpoint not found, fall back to v1 api
if (updateFilterV1(contexts, expiresInSeconds)) {
if (updateFilterV1(contexts, expiration)) {
return true
}
}
@ -182,13 +198,13 @@ class EditFilterViewModel @Inject constructor(val api: MastodonApi) : ViewModel(
)
}
private suspend fun createFilterV1(context: List<String>, expiresInSeconds: Int?): Boolean {
private suspend fun createFilterV1(context: List<String>, expiration: FilterExpiration?): Boolean {
return _keywords.value.map { keyword ->
api.createFilterV1(keyword.keyword, context, false, keyword.wholeWord, expiresInSeconds)
api.createFilterV1(keyword.keyword, context, false, keyword.wholeWord, expiration)
}.none { it.isFailure }
}
private suspend fun updateFilterV1(context: List<String>, expiresInSeconds: Int?): Boolean {
private suspend fun updateFilterV1(context: List<String>, expiration: FilterExpiration?): Boolean {
val results = _keywords.value.map { keyword ->
if (originalFilter == null) {
api.createFilterV1(
@ -196,7 +212,7 @@ class EditFilterViewModel @Inject constructor(val api: MastodonApi) : ViewModel(
context = context,
irreversible = false,
wholeWord = keyword.wholeWord,
expiresInSeconds = expiresInSeconds
expiresIn = expiration
)
} else {
api.updateFilterV1(
@ -205,7 +221,7 @@ class EditFilterViewModel @Inject constructor(val api: MastodonApi) : ViewModel(
context = context,
irreversible = false,
wholeWord = keyword.wholeWord,
expiresInSeconds = expiresInSeconds
expiresIn = expiration
)
}
}
@ -213,4 +229,18 @@ class EditFilterViewModel @Inject constructor(val api: MastodonApi) : ViewModel(
return results.none { it.isFailure }
}
companion object {
// Mastodon *stores* the absolute date in the filter,
// but create/edit take a number of seconds (relative to the time the operation is posted)
private fun getExpirationForDurationIndex(index: Int, context: Context): FilterExpiration? {
return when (index) {
-1 -> FilterExpiration.unchanged
0 -> FilterExpiration.never
else -> FilterExpiration.seconds(
context.resources.getIntArray(R.array.filter_duration_values)[index]
)
}
}
}
}

View File

@ -0,0 +1,37 @@
/* Copyright 2024 Tusky contributors
*
* 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.components.filters
import kotlin.jvm.JvmInline
/**
* Custom class to have typesafety for filter expirations.
* Retrofit will call toString when sending this class as part of a form-urlencoded body.
*/
@JvmInline
value class FilterExpiration private constructor(val seconds: Int) {
override fun toString(): String {
return if (seconds < 0) "" else seconds.toString()
}
companion object {
val unchanged: FilterExpiration? = null
val never: FilterExpiration = FilterExpiration(-1)
fun seconds(seconds: Int): FilterExpiration = FilterExpiration(seconds)
}
}

View File

@ -16,6 +16,7 @@
package com.keylesspalace.tusky.network
import at.connyduck.calladapter.networkresult.NetworkResult
import com.keylesspalace.tusky.components.filters.FilterExpiration
import com.keylesspalace.tusky.entity.AccessToken
import com.keylesspalace.tusky.entity.Account
import com.keylesspalace.tusky.entity.Announcement
@ -540,7 +541,7 @@ interface MastodonApi {
@Field("context[]") context: List<String>,
@Field("irreversible") irreversible: Boolean?,
@Field("whole_word") wholeWord: Boolean?,
@Field("expires_in") expiresInSeconds: Int?
@Field("expires_in") expiresIn: FilterExpiration?
): NetworkResult<FilterV1>
@FormUrlEncoded
@ -551,7 +552,7 @@ interface MastodonApi {
@Field("context[]") context: List<String>,
@Field("irreversible") irreversible: Boolean?,
@Field("whole_word") wholeWord: Boolean?,
@Field("expires_in") expiresInSeconds: Int?
@Field("expires_in") expiresIn: FilterExpiration?
): NetworkResult<FilterV1>
@DELETE("api/v1/filters/{id}")
@ -563,7 +564,7 @@ interface MastodonApi {
@Field("title") title: String,
@Field("context[]") context: List<String>,
@Field("filter_action") filterAction: String,
@Field("expires_in") expiresInSeconds: Int?
@Field("expires_in") expiresIn: FilterExpiration?
): NetworkResult<Filter>
@FormUrlEncoded
@ -573,7 +574,7 @@ interface MastodonApi {
@Field("title") title: String? = null,
@Field("context[]") context: List<String>? = null,
@Field("filter_action") filterAction: String? = null,
@Field("expires_in") expiresInSeconds: Int? = null
@Field("expires_in") expires: FilterExpiration? = null
): NetworkResult<Filter>
@DELETE("api/v2/filters/{id}")

View File

@ -19,7 +19,6 @@ package com.keylesspalace.tusky
import androidx.test.ext.junit.runners.AndroidJUnit4
import at.connyduck.calladapter.networkresult.NetworkResult
import com.keylesspalace.tusky.components.filters.EditFilterActivity
import com.keylesspalace.tusky.components.instanceinfo.InstanceInfoRepository
import com.keylesspalace.tusky.entity.Attachment
import com.keylesspalace.tusky.entity.Filter
@ -277,22 +276,6 @@ class FilterV1Test {
)
}
@Test
fun unchangedExpiration_shouldBeNegative_whenFilterIsExpired() {
val expiredBySeconds = 3600
val expiredDate = Date.from(Instant.now().minusSeconds(expiredBySeconds.toLong()))
val updatedDuration = EditFilterActivity.getSecondsForDurationIndex(-1, null, expiredDate)
assert(updatedDuration != null && updatedDuration <= -expiredBySeconds)
}
@Test
fun unchangedExpiration_shouldBePositive_whenFilterIsUnexpired() {
val expiresInSeconds = 3600
val expiredDate = Date.from(Instant.now().plusSeconds(expiresInSeconds.toLong()))
val updatedDuration = EditFilterActivity.getSecondsForDurationIndex(-1, null, expiredDate)
assert(updatedDuration != null && updatedDuration > (expiresInSeconds - 60))
}
companion object {
fun mockStatus(
content: String = "",