From e7583218662327bcfd53882739bdccd344b648f4 Mon Sep 17 00:00:00 2001 From: Konrad Pozniak Date: Tue, 5 Nov 2024 20:44:08 +0100 Subject: [PATCH] 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 --- .../keylesspalace/tusky/StatusListActivity.kt | 7 +-- .../components/filters/EditFilterActivity.kt | 38 ++++++++------ .../components/filters/EditFilterViewModel.kt | 52 +++++++++++++++---- .../components/filters/FilterExpiration.kt | 37 +++++++++++++ .../tusky/network/MastodonApi.kt | 9 ++-- .../com/keylesspalace/tusky/FilterV1Test.kt | 17 ------ 6 files changed, 108 insertions(+), 52 deletions(-) create mode 100644 app/src/main/java/com/keylesspalace/tusky/components/filters/FilterExpiration.kt diff --git a/app/src/main/java/com/keylesspalace/tusky/StatusListActivity.kt b/app/src/main/java/com/keylesspalace/tusky/StatusListActivity.kt index 2ffcc894b..d0cbc949d 100644 --- a/app/src/main/java/com/keylesspalace/tusky/StatusListActivity.kt +++ b/app/src/main/java/com/keylesspalace/tusky/StatusListActivity.kt @@ -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) diff --git a/app/src/main/java/com/keylesspalace/tusky/components/filters/EditFilterActivity.kt b/app/src/main/java/com/keylesspalace/tusky/components/filters/EditFilterActivity.kt index e3daa6f7f..227cdcacb 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/filters/EditFilterActivity.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/filters/EditFilterActivity.kt @@ -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 . */ + 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) - } - } } } diff --git a/app/src/main/java/com/keylesspalace/tusky/components/filters/EditFilterViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/filters/EditFilterViewModel.kt index 9bde72187..881fd70e9 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/filters/EditFilterViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/filters/EditFilterViewModel.kt @@ -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 . */ + 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, expiresInSeconds: Int?): Boolean { + private suspend fun createFilterV1(context: List, 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, expiresInSeconds: Int?): Boolean { + private suspend fun updateFilterV1(context: List, 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] + ) + } + } + } } diff --git a/app/src/main/java/com/keylesspalace/tusky/components/filters/FilterExpiration.kt b/app/src/main/java/com/keylesspalace/tusky/components/filters/FilterExpiration.kt new file mode 100644 index 000000000..27d1290d4 --- /dev/null +++ b/app/src/main/java/com/keylesspalace/tusky/components/filters/FilterExpiration.kt @@ -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 . */ + +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) + } +} diff --git a/app/src/main/java/com/keylesspalace/tusky/network/MastodonApi.kt b/app/src/main/java/com/keylesspalace/tusky/network/MastodonApi.kt index 4c265d4f8..1611697ce 100644 --- a/app/src/main/java/com/keylesspalace/tusky/network/MastodonApi.kt +++ b/app/src/main/java/com/keylesspalace/tusky/network/MastodonApi.kt @@ -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, @Field("irreversible") irreversible: Boolean?, @Field("whole_word") wholeWord: Boolean?, - @Field("expires_in") expiresInSeconds: Int? + @Field("expires_in") expiresIn: FilterExpiration? ): NetworkResult @FormUrlEncoded @@ -551,7 +552,7 @@ interface MastodonApi { @Field("context[]") context: List, @Field("irreversible") irreversible: Boolean?, @Field("whole_word") wholeWord: Boolean?, - @Field("expires_in") expiresInSeconds: Int? + @Field("expires_in") expiresIn: FilterExpiration? ): NetworkResult @DELETE("api/v1/filters/{id}") @@ -563,7 +564,7 @@ interface MastodonApi { @Field("title") title: String, @Field("context[]") context: List, @Field("filter_action") filterAction: String, - @Field("expires_in") expiresInSeconds: Int? + @Field("expires_in") expiresIn: FilterExpiration? ): NetworkResult @FormUrlEncoded @@ -573,7 +574,7 @@ interface MastodonApi { @Field("title") title: String? = null, @Field("context[]") context: List? = null, @Field("filter_action") filterAction: String? = null, - @Field("expires_in") expiresInSeconds: Int? = null + @Field("expires_in") expires: FilterExpiration? = null ): NetworkResult @DELETE("api/v2/filters/{id}") diff --git a/app/src/test/java/com/keylesspalace/tusky/FilterV1Test.kt b/app/src/test/java/com/keylesspalace/tusky/FilterV1Test.kt index b28447aba..fd3912cb7 100644 --- a/app/src/test/java/com/keylesspalace/tusky/FilterV1Test.kt +++ b/app/src/test/java/com/keylesspalace/tusky/FilterV1Test.kt @@ -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 = "",