From 0d28b3a8600d3294a9e703b101343ee4dee776e8 Mon Sep 17 00:00:00 2001 From: Florian Renaud Date: Fri, 17 Feb 2023 09:59:17 +0100 Subject: [PATCH] Handle errors when updating push rules --- .../src/main/res/values/strings.xml | 2 + .../preference/VectorCheckboxPreference.kt | 7 ++ .../ConfigureAndStartSessionUseCase.kt | 2 +- .../notifications/PushRuleDefinitions.kt | 3 +- ...torSettingsPushRuleNotificationFragment.kt | 44 +++++--- ...orSettingsPushRuleNotificationViewEvent.kt | 2 +- ...orSettingsPushRuleNotificationViewModel.kt | 48 +++++++-- ...orSettingsPushRuleNotificationViewState.kt | 1 + .../GetPushRulesOnInvalidStateUseCase.kt | 44 ++++++++ .../usecase/UpdatePushRulesIfNeededUseCase.kt | 18 ++-- .../vector_settings_notification_default.xml | 2 +- ...ttingsPushRuleNotificationViewModelTest.kt | 29 ++--- .../GetPushRulesOnInvalidStateUseCaseTest.kt | 100 ++++++++++++++++++ .../UpdatePushRulesIfNeededUseCaseTest.kt | 54 +++++----- .../app/test/fixtures/PushRuleFixture.kt | 49 +++++++++ 15 files changed, 325 insertions(+), 80 deletions(-) create mode 100644 vector/src/main/java/im/vector/app/features/settings/notifications/usecase/GetPushRulesOnInvalidStateUseCase.kt create mode 100644 vector/src/test/java/im/vector/app/features/settings/notifications/usecase/GetPushRulesOnInvalidStateUseCaseTest.kt create mode 100644 vector/src/test/java/im/vector/app/test/fixtures/PushRuleFixture.kt diff --git a/library/ui-strings/src/main/res/values/strings.xml b/library/ui-strings/src/main/res/values/strings.xml index e06355a173..2ee623cf88 100644 --- a/library/ui-strings/src/main/res/values/strings.xml +++ b/library/ui-strings/src/main/res/values/strings.xml @@ -862,6 +862,8 @@ Keywords cannot start with \'.\' Keywords cannot contain \'%s\' + An error occurred when updating your notification preferences. Please try again. + Troubleshoot Notifications Troubleshooting diagnostics Run Tests diff --git a/vector/src/main/java/im/vector/app/core/preference/VectorCheckboxPreference.kt b/vector/src/main/java/im/vector/app/core/preference/VectorCheckboxPreference.kt index 13b65e11b4..355bab8da8 100644 --- a/vector/src/main/java/im/vector/app/core/preference/VectorCheckboxPreference.kt +++ b/vector/src/main/java/im/vector/app/core/preference/VectorCheckboxPreference.kt @@ -32,6 +32,8 @@ class VectorCheckboxPreference : CheckBoxPreference { constructor(context: Context) : super(context) + var summaryTextColor: Int? = null + init { // Set to false to remove the space when there is no icon isIconSpaceReserved = true @@ -42,4 +44,9 @@ class VectorCheckboxPreference : CheckBoxPreference { (holder.findViewById(android.R.id.title) as? TextView)?.isSingleLine = false super.onBindViewHolder(holder) } + + override fun syncSummaryView(holder: PreferenceViewHolder) { + super.syncSummaryView(holder) + summaryTextColor?.let { (holder.findViewById(android.R.id.summary) as? TextView)?.setTextColor(it) } + } } diff --git a/vector/src/main/java/im/vector/app/core/session/ConfigureAndStartSessionUseCase.kt b/vector/src/main/java/im/vector/app/core/session/ConfigureAndStartSessionUseCase.kt index b9573e9292..8a9fc9729d 100644 --- a/vector/src/main/java/im/vector/app/core/session/ConfigureAndStartSessionUseCase.kt +++ b/vector/src/main/java/im/vector/app/core/session/ConfigureAndStartSessionUseCase.kt @@ -52,7 +52,7 @@ class ConfigureAndStartSessionUseCase @Inject constructor( updateMatrixClientInfoIfNeeded(session) createNotificationSettingsAccountDataIfNeeded(session) notificationsSettingUpdater.onSessionStarted(session) - pushRulesUpdater.onSessionStarted(session) +// pushRulesUpdater.onSessionStarted(session) } private fun updateMatrixClientInfoIfNeeded(session: Session) { diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/PushRuleDefinitions.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/PushRuleDefinitions.kt index fff6ffe933..9e19e2afb4 100644 --- a/vector/src/main/java/im/vector/app/features/settings/notifications/PushRuleDefinitions.kt +++ b/vector/src/main/java/im/vector/app/features/settings/notifications/PushRuleDefinitions.kt @@ -42,8 +42,7 @@ fun getStandardAction(ruleId: String, index: NotificationIndex): StandardActions RuleIds.RULE_ID_POLL_START_ONE_TO_ONE, RuleIds.RULE_ID_POLL_START_ONE_TO_ONE_UNSTABLE, RuleIds.RULE_ID_POLL_END_ONE_TO_ONE, - RuleIds.RULE_ID_POLL_END_ONE_TO_ONE_UNSTABLE, - -> + RuleIds.RULE_ID_POLL_END_ONE_TO_ONE_UNSTABLE -> when (index) { NotificationIndex.OFF -> StandardActions.DontNotify NotificationIndex.SILENT -> StandardActions.Notify diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationFragment.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationFragment.kt index 0f991867fe..8a2099ecad 100644 --- a/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationFragment.kt +++ b/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationFragment.kt @@ -21,33 +21,35 @@ import android.view.View import androidx.preference.Preference import com.airbnb.mvrx.fragmentViewModel import com.airbnb.mvrx.withState +import im.vector.app.R import im.vector.app.core.preference.VectorCheckboxPreference import im.vector.app.features.settings.VectorSettingsBaseFragment +import im.vector.app.features.themes.ThemeUtils abstract class VectorSettingsPushRuleNotificationFragment : VectorSettingsBaseFragment() { - private val viewModel: VectorSettingsPushRuleNotificationViewModel by fragmentViewModel() + protected val viewModel: VectorSettingsPushRuleNotificationViewModel by fragmentViewModel() abstract val prefKeyToPushRuleId: Map override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) observeViewEvents() - viewModel.onEach(VectorSettingsPushRuleNotificationViewState::isLoading) { isLoading -> - if (isLoading) { - displayLoadingView() - } else { - hideLoadingView() - } - } + viewModel.onEach(VectorSettingsPushRuleNotificationViewState::isLoading) { updateLoadingView(it) } + viewModel.onEach(VectorSettingsPushRuleNotificationViewState::rulesOnError) { refreshErrors(it) } } private fun observeViewEvents() { viewModel.observeViewEvents { when (it) { - is VectorSettingsPushRuleNotificationViewEvent.Failure -> refreshDisplay() - is VectorSettingsPushRuleNotificationViewEvent.PushRuleUpdated -> updatePreference(it.ruleId, it.checked) + is VectorSettingsPushRuleNotificationViewEvent.Failure -> onFailure(it.ruleId) + is VectorSettingsPushRuleNotificationViewEvent.PushRuleUpdated -> { + updatePreference(it.ruleId, it.checked) + if (it.failure != null) { + onFailure(it.ruleId) + } + } } } } @@ -71,14 +73,28 @@ abstract class VectorSettingsPushRuleNotificationFragment : } } - override fun invalidate() = withState(viewModel) { state -> - if (state.isLoading) { + private fun updateLoadingView(isLoading: Boolean) { + if (isLoading) { displayLoadingView() } else { hideLoadingView() } } + private fun refreshErrors(rulesWithError: Set) { + if (withState(viewModel, VectorSettingsPushRuleNotificationViewState::isLoading)) return + prefKeyToPushRuleId.forEach { (preferenceKey, ruleId) -> + val preference = findPreference(preferenceKey)!! + if (ruleId in rulesWithError) { + preference.summaryTextColor = context?.let { ThemeUtils.getColor(it, R.attr.colorError) } + preference.setSummary(R.string.settings_notification_error_on_update) + } else { + preference.summaryTextColor = null + preference.summary = null + } + } + } + protected fun refreshDisplay() { listView?.adapter?.notifyDataSetChanged() } @@ -88,4 +104,8 @@ abstract class VectorSettingsPushRuleNotificationFragment : val preference = findPreference(preferenceKey) ?: return preference.isChecked = checked } + + protected open fun onFailure(ruleId: String) { + refreshDisplay() + } } diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewEvent.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewEvent.kt index adfc17f827..490a28c0b0 100644 --- a/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewEvent.kt +++ b/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewEvent.kt @@ -33,5 +33,5 @@ sealed interface VectorSettingsPushRuleNotificationViewEvent : VectorViewEvents * * @property throwable the related exception, if any. */ - data class Failure(val throwable: Throwable?) : VectorSettingsPushRuleNotificationViewEvent + data class Failure(val ruleId: String, val throwable: Throwable?) : VectorSettingsPushRuleNotificationViewEvent } diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewModel.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewModel.kt index 39969ec13e..eee199d550 100644 --- a/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewModel.kt @@ -26,13 +26,17 @@ import im.vector.app.core.di.hiltMavericksViewModelFactory import im.vector.app.core.platform.VectorViewModel import im.vector.app.features.settings.notifications.VectorSettingsPushRuleNotificationViewEvent.Failure import im.vector.app.features.settings.notifications.VectorSettingsPushRuleNotificationViewEvent.PushRuleUpdated +import im.vector.app.features.settings.notifications.usecase.GetPushRulesOnInvalidStateUseCase import kotlinx.coroutines.launch import org.matrix.android.sdk.api.failure.Failure.ServerError import org.matrix.android.sdk.api.failure.MatrixError +import org.matrix.android.sdk.api.session.accountdata.UserAccountDataTypes import org.matrix.android.sdk.api.session.pushrules.Action import org.matrix.android.sdk.api.session.pushrules.RuleIds import org.matrix.android.sdk.api.session.pushrules.RuleKind import org.matrix.android.sdk.api.session.pushrules.rest.PushRuleAndKind +import org.matrix.android.sdk.flow.flow +import org.matrix.android.sdk.flow.unwrap private typealias ViewModel = VectorSettingsPushRuleNotificationViewModel private typealias ViewState = VectorSettingsPushRuleNotificationViewState @@ -40,6 +44,7 @@ private typealias ViewState = VectorSettingsPushRuleNotificationViewState class VectorSettingsPushRuleNotificationViewModel @AssistedInject constructor( @Assisted initialState: ViewState, private val activeSessionHolder: ActiveSessionHolder, + private val getPushRulesOnInvalidStateUseCase: GetPushRulesOnInvalidStateUseCase, ) : VectorViewModel(initialState) { @@ -51,6 +56,17 @@ class VectorSettingsPushRuleNotificationViewModel @AssistedInject constructor( companion object : MavericksViewModelFactory by hiltMavericksViewModelFactory() + init { + val session = activeSessionHolder.getSafeActiveSession() + session?.flow() + ?.liveUserAccountData(UserAccountDataTypes.TYPE_PUSH_RULES) + ?.unwrap() + ?.setOnEach { + val rulesOnError = getPushRulesOnInvalidStateUseCase.execute(session).map { it.ruleId }.toSet() + copy(rulesOnError = rulesOnError) + } + } + override fun handle(action: VectorSettingsPushRuleNotificationViewAction) { when (action) { is VectorSettingsPushRuleNotificationViewAction.UpdatePushRule -> handleUpdatePushRule(action.pushRuleAndKind, action.checked) @@ -73,6 +89,7 @@ class VectorSettingsPushRuleNotificationViewModel @AssistedInject constructor( val standardAction = getStandardAction(ruleId, newIndex) ?: return val enabled = standardAction != StandardActions.Disabled val newActions = standardAction.actions + setState { copy(isLoading = true) } viewModelScope.launch { @@ -82,22 +99,31 @@ class VectorSettingsPushRuleNotificationViewModel @AssistedInject constructor( updatePushRule(kind, ruleId, enabled, newActions) } } - setState { copy(isLoading = false) } - val failure = results.firstNotNullOfOrNull { result -> + + val failures = results.mapNotNull { result -> // If the failure is a rule not found error, do not consider it result.exceptionOrNull()?.takeUnless { it is ServerError && it.error.code == MatrixError.M_NOT_FOUND } } - val newChecked = if (checked) { - // If any rule is checked, the global rule is checked - results.any { it.isSuccess } + val hasSuccess = results.any { it.isSuccess } + val hasFailures = failures.isNotEmpty() + + // Any rule has been checked or some rules have not been unchecked + val newChecked = (checked && hasSuccess) || (!checked && hasFailures) + if (hasSuccess) { + _viewEvents.post(PushRuleUpdated(ruleId, newChecked, failures.firstOrNull())) } else { - // If any rule has not been unchecked, the global rule remains checked - failure != null + _viewEvents.post(Failure(ruleId, failures.firstOrNull())) } - if (results.any { it.isSuccess }) { - _viewEvents.post(PushRuleUpdated(ruleId, newChecked, failure)) - } else { - _viewEvents.post(Failure(failure)) + + setState { + copy( + isLoading = false, + rulesOnError = when { + hasSuccess && hasFailures -> rulesOnError.plus(ruleId) // some failed + hasSuccess -> rulesOnError.minus(ruleId) // all succeed + else -> rulesOnError // all failed + } + ) } } } diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewState.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewState.kt index 477727aee6..392a8f3bb5 100644 --- a/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewState.kt +++ b/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewState.kt @@ -20,4 +20,5 @@ import com.airbnb.mvrx.MavericksState data class VectorSettingsPushRuleNotificationViewState( val isLoading: Boolean = false, + val rulesOnError: Set = emptySet() ) : MavericksState diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/usecase/GetPushRulesOnInvalidStateUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/usecase/GetPushRulesOnInvalidStateUseCase.kt new file mode 100644 index 0000000000..225be1ee62 --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/settings/notifications/usecase/GetPushRulesOnInvalidStateUseCase.kt @@ -0,0 +1,44 @@ +/* + * Copyright (c) 2023 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.settings.notifications.usecase + +import org.matrix.android.sdk.api.extensions.orTrue +import org.matrix.android.sdk.api.session.Session +import org.matrix.android.sdk.api.session.pushrules.RuleIds +import org.matrix.android.sdk.api.session.pushrules.getActions +import org.matrix.android.sdk.api.session.pushrules.getParentRule +import org.matrix.android.sdk.api.session.pushrules.getSyncedRules +import org.matrix.android.sdk.api.session.pushrules.rest.PushRule +import javax.inject.Inject + +class GetPushRulesOnInvalidStateUseCase @Inject constructor() { + + fun execute(session: Session): List { + val allRules = session.pushRuleService().getPushRules().getAllRules() + return allRules.filter { it.isOnInvalidState(allRules) } + } + + private fun PushRule.isOnInvalidState(allRules: List): Boolean { + val parent = RuleIds.getParentRule(ruleId)?.let { parentId -> allRules.find { it.ruleId == parentId } } + val children = RuleIds.getSyncedRules(ruleId).mapNotNull { childId -> allRules.find { it.ruleId == childId } } + val isAlignedWithParent = parent?.let { isAlignedWithParentRule(it) }.orTrue() + return !isAlignedWithParent || !isAlignedWithChildrenRules(children) + } + + private fun PushRule.isAlignedWithParentRule(parent: PushRule) = this.getActions() == parent.getActions() && this.enabled == parent.enabled + private fun PushRule.isAlignedWithChildrenRules(children: List) = children.all { it.isAlignedWithParentRule(this) } +} diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/usecase/UpdatePushRulesIfNeededUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/usecase/UpdatePushRulesIfNeededUseCase.kt index 6998065f01..12fc3d480d 100644 --- a/vector/src/main/java/im/vector/app/features/settings/notifications/usecase/UpdatePushRulesIfNeededUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/notifications/usecase/UpdatePushRulesIfNeededUseCase.kt @@ -24,18 +24,18 @@ import org.matrix.android.sdk.api.session.pushrules.rest.PushRule import org.matrix.android.sdk.api.session.pushrules.rest.PushRuleAndKind import javax.inject.Inject -class UpdatePushRulesIfNeededUseCase @Inject constructor() { +class UpdatePushRulesIfNeededUseCase @Inject constructor( + private val getPushRulesOnInvalidStateUseCase: GetPushRulesOnInvalidStateUseCase, +) { suspend fun execute(session: Session) { + val rulesOnError = getPushRulesOnInvalidStateUseCase.execute(session).takeUnless { it.isEmpty() } ?: return + val ruleSet = session.pushRuleService().getPushRules() - val pushRules = ruleSet.getAllRules() - val rulesToUpdate = pushRules.mapNotNull { rule -> - val parent = RuleIds.getParentRule(rule.ruleId)?.let { ruleId -> ruleSet.findDefaultRule(ruleId) } - if (parent != null && (rule.enabled != parent.pushRule.enabled || rule.actions != parent.pushRule.actions)) { - PushRuleWithParent(rule, parent) - } else { - null - } + val rulesToUpdate = rulesOnError.mapNotNull { rule -> + RuleIds.getParentRule(rule.ruleId) + ?.let { ruleId -> ruleSet.findDefaultRule(ruleId) } + ?.let { PushRuleWithParent(rule, it) } } rulesToUpdate.forEach { diff --git a/vector/src/main/res/xml/vector_settings_notification_default.xml b/vector/src/main/res/xml/vector_settings_notification_default.xml index fb565d2230..9319ef2b39 100644 --- a/vector/src/main/res/xml/vector_settings_notification_default.xml +++ b/vector/src/main/res/xml/vector_settings_notification_default.xml @@ -25,4 +25,4 @@ android:title="@string/settings_encrypted_group_messages" /> - \ No newline at end of file + diff --git a/vector/src/test/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewModelTest.kt b/vector/src/test/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewModelTest.kt index 04a22bc21f..7994d173b6 100644 --- a/vector/src/test/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewModelTest.kt +++ b/vector/src/test/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewModelTest.kt @@ -17,7 +17,9 @@ package im.vector.app.features.settings.notifications import com.airbnb.mvrx.test.MavericksTestRule +import im.vector.app.features.settings.notifications.usecase.GetPushRulesOnInvalidStateUseCase import im.vector.app.test.fakes.FakeActiveSessionHolder +import im.vector.app.test.fixtures.PushRuleFixture import im.vector.app.test.test import im.vector.app.test.testDispatcher import io.mockk.coVerifyOrder @@ -41,16 +43,19 @@ internal class VectorSettingsPushRuleNotificationViewModelTest { private val fakeActiveSessionHolder = FakeActiveSessionHolder() private val fakePushRuleService = fakeActiveSessionHolder.fakeSession.fakePushRuleService + private val fakeGetPushRulesOnInvalidStateUseCase = mockk() private val initialState = VectorSettingsPushRuleNotificationViewState() private fun createViewModel() = VectorSettingsPushRuleNotificationViewModel( initialState = initialState, activeSessionHolder = fakeActiveSessionHolder.instance, + fakeGetPushRulesOnInvalidStateUseCase, ) @Before fun setup() { mockkStatic("im.vector.app.features.settings.notifications.NotificationIndexKt") + every { fakeGetPushRulesOnInvalidStateUseCase.execute(any()) } returns emptyList() } @After @@ -98,8 +103,8 @@ internal class VectorSettingsPushRuleNotificationViewModelTest { { copy(isLoading = false) }, ) .assertEvents( - VectorSettingsPushRuleNotificationViewEvent.PushRuleUpdated(RuleIds.RULE_ID_ONE_TO_ONE_ROOM, true), - VectorSettingsPushRuleNotificationViewEvent.PushRuleUpdated(RuleIds.RULE_ID_ALL_OTHER_MESSAGES_ROOMS, false), + VectorSettingsPushRuleNotificationViewEvent.PushRuleUpdated(firstRuleId, true), + VectorSettingsPushRuleNotificationViewEvent.PushRuleUpdated(secondRuleId, false), ) .finish() } @@ -149,13 +154,13 @@ internal class VectorSettingsPushRuleNotificationViewModelTest { .assertStatesChanges( initialState, { copy(isLoading = true) }, - { copy(isLoading = false) }, + { copy(isLoading = false, setOf(firstRuleId)) }, { copy(isLoading = true) }, { copy(isLoading = false) }, ) .assertEvents( - VectorSettingsPushRuleNotificationViewEvent.PushRuleUpdated(RuleIds.RULE_ID_ONE_TO_ONE_ROOM, true, failure), - VectorSettingsPushRuleNotificationViewEvent.Failure(failure), + VectorSettingsPushRuleNotificationViewEvent.PushRuleUpdated(firstRuleId, true, failure), + VectorSettingsPushRuleNotificationViewEvent.Failure(secondRuleId, failure), ) .finish() } @@ -205,14 +210,14 @@ internal class VectorSettingsPushRuleNotificationViewModelTest { .assertStatesChanges( initialState, { copy(isLoading = true) }, - { copy(isLoading = false) }, + { copy(isLoading = false, rulesOnError = setOf(firstRuleId)) }, { copy(isLoading = true) }, { copy(isLoading = false) }, ) .assertEvents( // The global rule remains checked if all the rules are not unchecked - VectorSettingsPushRuleNotificationViewEvent.PushRuleUpdated(RuleIds.RULE_ID_ONE_TO_ONE_ROOM, true, failure), - VectorSettingsPushRuleNotificationViewEvent.Failure(failure), + VectorSettingsPushRuleNotificationViewEvent.PushRuleUpdated(firstRuleId, true, failure), + VectorSettingsPushRuleNotificationViewEvent.Failure(secondRuleId, failure), ) .finish() } @@ -245,11 +250,9 @@ internal class VectorSettingsPushRuleNotificationViewModelTest { } private fun givenARuleId(ruleId: String, notificationIndex: NotificationIndex = NotificationIndex.NOISY): PushRuleAndKind { - val ruleAndKind = mockk { - every { pushRule.ruleId } returns ruleId - every { pushRule.notificationIndex } returns notificationIndex - every { kind } returns mockk() - } + val pushRule = PushRuleFixture.aPushRule(ruleId) + every { pushRule.notificationIndex } returns notificationIndex + val ruleAndKind = PushRuleFixture.aPushRuleAndKind(pushRule) every { fakePushRuleService.getPushRules().findDefaultRule(ruleId) } returns ruleAndKind diff --git a/vector/src/test/java/im/vector/app/features/settings/notifications/usecase/GetPushRulesOnInvalidStateUseCaseTest.kt b/vector/src/test/java/im/vector/app/features/settings/notifications/usecase/GetPushRulesOnInvalidStateUseCaseTest.kt new file mode 100644 index 0000000000..a434ac93d3 --- /dev/null +++ b/vector/src/test/java/im/vector/app/features/settings/notifications/usecase/GetPushRulesOnInvalidStateUseCaseTest.kt @@ -0,0 +1,100 @@ +/* + * Copyright (c) 2023 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.settings.notifications.usecase + +import im.vector.app.test.fakes.FakeSession +import im.vector.app.test.fixtures.PushRuleFixture +import io.mockk.every +import io.mockk.mockkStatic +import io.mockk.unmockkAll +import org.amshove.kluent.shouldBeEqualTo +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.matrix.android.sdk.api.session.pushrules.Action +import org.matrix.android.sdk.api.session.pushrules.RuleIds +import org.matrix.android.sdk.api.session.pushrules.rest.PushRule + +internal class GetPushRulesOnInvalidStateUseCaseTest { + + private val fakeSession = FakeSession() + private val getPushRulesOnInvalidStateUseCase = GetPushRulesOnInvalidStateUseCase() + + @Before + fun setup() { + mockkStatic("org.matrix.android.sdk.api.session.pushrules.ActionKt") + } + + @After + fun tearDown() { + unmockkAll() + } + + @Test + fun `given a list of push rules with children not matching their parent when execute then returns the list of not matching rules`() { + // Given + val firstActions = listOf(Action.Notify) + val secondActions = listOf(Action.DoNotNotify) + givenARuleList( + listOf( + // first set of related rules + givenARuleId(RuleIds.RULE_ID_ONE_TO_ONE_ROOM, true, firstActions), + givenARuleId(RuleIds.RULE_ID_POLL_START_ONE_TO_ONE, true, listOf(Action.DoNotNotify)), // diff + givenARuleId(RuleIds.RULE_ID_POLL_START_ONE_TO_ONE_UNSTABLE, true, emptyList()), // diff + givenARuleId(RuleIds.RULE_ID_POLL_END_ONE_TO_ONE, false, listOf(Action.Notify)), // diff + givenARuleId(RuleIds.RULE_ID_POLL_END_ONE_TO_ONE_UNSTABLE, true, listOf(Action.Notify)), + // second set of related rules + givenARuleId(RuleIds.RULE_ID_ALL_OTHER_MESSAGES_ROOMS, false, secondActions), + givenARuleId(RuleIds.RULE_ID_POLL_START, true, listOf(Action.Notify)), // diff + givenARuleId(RuleIds.RULE_ID_POLL_START_UNSTABLE, false, listOf(Action.DoNotNotify)), + givenARuleId(RuleIds.RULE_ID_POLL_END, false, listOf(Action.Notify)), // diff + givenARuleId(RuleIds.RULE_ID_POLL_END_UNSTABLE, true, listOf()), // diff + // Another rule + givenARuleId(RuleIds.RULE_ID_CONTAIN_USER_NAME, true, listOf(Action.Notify)), + ) + ) + + // When + val result = getPushRulesOnInvalidStateUseCase.execute(fakeSession).map { it.ruleId } + + // Then + result shouldBeEqualTo listOf( + RuleIds.RULE_ID_ONE_TO_ONE_ROOM, // parent rule + RuleIds.RULE_ID_POLL_START_ONE_TO_ONE, + RuleIds.RULE_ID_POLL_START_ONE_TO_ONE_UNSTABLE, + RuleIds.RULE_ID_POLL_END_ONE_TO_ONE, + RuleIds.RULE_ID_ALL_OTHER_MESSAGES_ROOMS, // parent rule + RuleIds.RULE_ID_POLL_START, + RuleIds.RULE_ID_POLL_END, + RuleIds.RULE_ID_POLL_END_UNSTABLE, + ) + } + + private fun givenARuleList(rules: List) { + every { fakeSession.fakePushRuleService.getPushRules().getAllRules() } returns rules + } + + private fun givenARuleId(ruleId: String, enabled: Boolean, actions: List): PushRule { + val ruleAndKind = PushRuleFixture.aPushRuleAndKind( + PushRuleFixture.aPushRule(ruleId, enabled, actions), + ) + + every { fakeSession.fakePushRuleService.getPushRules().findDefaultRule(ruleId) } returns ruleAndKind + + return ruleAndKind.pushRule + } +} diff --git a/vector/src/test/java/im/vector/app/features/settings/notifications/usecase/UpdatePushRulesIfNeededUseCaseTest.kt b/vector/src/test/java/im/vector/app/features/settings/notifications/usecase/UpdatePushRulesIfNeededUseCaseTest.kt index 7dcbe68d4b..1f76a7f9a5 100644 --- a/vector/src/test/java/im/vector/app/features/settings/notifications/usecase/UpdatePushRulesIfNeededUseCaseTest.kt +++ b/vector/src/test/java/im/vector/app/features/settings/notifications/usecase/UpdatePushRulesIfNeededUseCaseTest.kt @@ -17,6 +17,7 @@ package im.vector.app.features.settings.notifications.usecase import im.vector.app.test.fakes.FakeSession +import im.vector.app.test.fixtures.PushRuleFixture import io.mockk.coVerifySequence import io.mockk.every import io.mockk.mockk @@ -28,14 +29,13 @@ import org.junit.Before import org.junit.Test import org.matrix.android.sdk.api.session.pushrules.Action import org.matrix.android.sdk.api.session.pushrules.RuleIds -import org.matrix.android.sdk.api.session.pushrules.getActions import org.matrix.android.sdk.api.session.pushrules.rest.PushRule -import org.matrix.android.sdk.api.session.pushrules.rest.PushRuleAndKind internal class UpdatePushRulesIfNeededUseCaseTest { private val fakeSession = FakeSession() - private val updatePushRulesIfNeededUseCase = UpdatePushRulesIfNeededUseCase() + private val fakeGetPushRulesOnInvalidStateUseCase = mockk() + private val updatePushRulesIfNeededUseCase = UpdatePushRulesIfNeededUseCase(fakeGetPushRulesOnInvalidStateUseCase) @Before fun setup() { @@ -50,25 +50,26 @@ internal class UpdatePushRulesIfNeededUseCaseTest { @Test fun test() = runTest { // Given - val firstActions = listOf(Action.Notify) - val secondActions = listOf(Action.DoNotNotify) - val rules = listOf( + val firstParentEnabled = true + val firstParentActions = listOf(Action.Notify) + val firstParent = givenARuleId(RuleIds.RULE_ID_ONE_TO_ONE_ROOM, firstParentEnabled, firstParentActions) + val secondParentEnabled = false + val secondParentActions = listOf(Action.DoNotNotify) + val secondParent = givenARuleId(RuleIds.RULE_ID_ALL_OTHER_MESSAGES_ROOMS, secondParentEnabled, secondParentActions) + val rulesOnError = listOf( // first set of related rules - givenARuleId(RuleIds.RULE_ID_ONE_TO_ONE_ROOM, true, firstActions), + firstParent, givenARuleId(RuleIds.RULE_ID_POLL_START_ONE_TO_ONE, true, listOf(Action.DoNotNotify)), // diff givenARuleId(RuleIds.RULE_ID_POLL_START_ONE_TO_ONE_UNSTABLE, true, emptyList()), // diff givenARuleId(RuleIds.RULE_ID_POLL_END_ONE_TO_ONE, false, listOf(Action.Notify)), // diff - givenARuleId(RuleIds.RULE_ID_POLL_END_ONE_TO_ONE_UNSTABLE, true, listOf(Action.Notify)), // second set of related rules - givenARuleId(RuleIds.RULE_ID_ALL_OTHER_MESSAGES_ROOMS, false, secondActions), + secondParent, givenARuleId(RuleIds.RULE_ID_POLL_START, true, listOf(Action.Notify)), // diff - givenARuleId(RuleIds.RULE_ID_POLL_START_UNSTABLE, false, listOf(Action.DoNotNotify)), givenARuleId(RuleIds.RULE_ID_POLL_END, false, listOf(Action.Notify)), // diff givenARuleId(RuleIds.RULE_ID_POLL_END_UNSTABLE, true, listOf()), // diff - // Another rule - givenARuleId(RuleIds.RULE_ID_CONTAIN_USER_NAME, true, listOf(Action.Notify)), ) - every { fakeSession.fakePushRuleService.getPushRules().getAllRules() } returns rules + every { fakeGetPushRulesOnInvalidStateUseCase.execute(fakeSession) } returns rulesOnError + every { fakeSession.fakePushRuleService.getPushRules().getAllRules() } returns rulesOnError // When updatePushRulesIfNeededUseCase.execute(fakeSession) @@ -77,30 +78,23 @@ internal class UpdatePushRulesIfNeededUseCaseTest { coVerifySequence { fakeSession.fakePushRuleService.getPushRules() // first set - fakeSession.fakePushRuleService.updatePushRuleActions(any(), RuleIds.RULE_ID_POLL_START_ONE_TO_ONE, true, firstActions) - fakeSession.fakePushRuleService.updatePushRuleActions(any(), RuleIds.RULE_ID_POLL_START_ONE_TO_ONE_UNSTABLE, true, firstActions) - fakeSession.fakePushRuleService.updatePushRuleActions(any(), RuleIds.RULE_ID_POLL_END_ONE_TO_ONE, true, firstActions) + fakeSession.fakePushRuleService.updatePushRuleActions(any(), RuleIds.RULE_ID_POLL_START_ONE_TO_ONE, firstParentEnabled, firstParentActions) + fakeSession.fakePushRuleService.updatePushRuleActions(any(), RuleIds.RULE_ID_POLL_START_ONE_TO_ONE_UNSTABLE, firstParentEnabled, firstParentActions) + fakeSession.fakePushRuleService.updatePushRuleActions(any(), RuleIds.RULE_ID_POLL_END_ONE_TO_ONE, firstParentEnabled, firstParentActions) // second set - fakeSession.fakePushRuleService.updatePushRuleActions(any(), RuleIds.RULE_ID_POLL_START, false, secondActions) - fakeSession.fakePushRuleService.updatePushRuleActions(any(), RuleIds.RULE_ID_POLL_END, false, secondActions) - fakeSession.fakePushRuleService.updatePushRuleActions(any(), RuleIds.RULE_ID_POLL_END_UNSTABLE, false, secondActions) + fakeSession.fakePushRuleService.updatePushRuleActions(any(), RuleIds.RULE_ID_POLL_START, secondParentEnabled, secondParentActions) + fakeSession.fakePushRuleService.updatePushRuleActions(any(), RuleIds.RULE_ID_POLL_END, secondParentEnabled, secondParentActions) + fakeSession.fakePushRuleService.updatePushRuleActions(any(), RuleIds.RULE_ID_POLL_END_UNSTABLE, secondParentEnabled, secondParentActions) } } private fun givenARuleId(ruleId: String, enabled: Boolean, actions: List): PushRule { - val pushRule = mockk { - every { this@mockk.ruleId } returns ruleId - every { this@mockk.enabled } returns enabled - every { this@mockk.actions } returns actions - every { getActions() } returns actions - } - val ruleAndKind = mockk { - every { this@mockk.pushRule } returns pushRule - every { kind } returns mockk() - } + val ruleAndKind = PushRuleFixture.aPushRuleAndKind( + pushRule = PushRuleFixture.aPushRule(ruleId, enabled, actions), + ) every { fakeSession.fakePushRuleService.getPushRules().findDefaultRule(ruleId) } returns ruleAndKind - return pushRule + return ruleAndKind.pushRule } } diff --git a/vector/src/test/java/im/vector/app/test/fixtures/PushRuleFixture.kt b/vector/src/test/java/im/vector/app/test/fixtures/PushRuleFixture.kt new file mode 100644 index 0000000000..2170d60228 --- /dev/null +++ b/vector/src/test/java/im/vector/app/test/fixtures/PushRuleFixture.kt @@ -0,0 +1,49 @@ +/* + * Copyright (c) 2023 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.test.fixtures + +import io.mockk.every +import io.mockk.mockk +import org.matrix.android.sdk.api.session.pushrules.Action +import org.matrix.android.sdk.api.session.pushrules.RuleSetKey +import org.matrix.android.sdk.api.session.pushrules.getActions +import org.matrix.android.sdk.api.session.pushrules.rest.PushRule +import org.matrix.android.sdk.api.session.pushrules.rest.PushRuleAndKind + +object PushRuleFixture { + + private const val A_RULE_ID = "a-rule-id" + + // Needs: mockkStatic("org.matrix.android.sdk.api.session.pushrules.ActionKt") + fun aPushRule( + ruleId: String = A_RULE_ID, + enabled: Boolean = true, + actions: List = listOf(Action.Notify), + ): PushRule = mockk { + every { this@mockk.ruleId } returns ruleId + every { this@mockk.enabled } returns enabled + every { getActions() } returns actions + } + + fun aPushRuleAndKind( + pushRule: PushRule = aPushRule(), + kind: RuleSetKey = RuleSetKey.UNDERRIDE, + ): PushRuleAndKind = mockk { + every { this@mockk.pushRule } returns pushRule + every { this@mockk.kind } returns kind + } +}