Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes Changing Account Settings > Notifications > Advanced Notifications on android causes discrepancies with web #3681

Merged
merged 8 commits into from
Jul 27, 2021
Merged

Fixes Changing Account Settings > Notifications > Advanced Notifications on android causes discrepancies with web #3681

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
dff8503
fix discrepancies with web
langleyd Jul 15, 2021
a29ccda
move mutable push rule logic to static declarations as on web
langleyd Jul 19, 2021
55902b5
checks
langleyd Jul 19, 2021
89ece22
Merge branch 'develop' of github.com:vector-im/element-android into f…
langleyd Jul 19, 2021
da993b5
lint
langleyd Jul 19, 2021
608a1d3
clarify sdk update and cleanup typos
langleyd Jul 20, 2021
5d092ce
simplify getHighlight logic
langleyd Jul 23, 2021
9c139b3
Merge branch 'develop' of github.com:vector-im/element-android into f…
langleyd Jul 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/pushrules/PushRuleService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ interface PushRuleService {

suspend fun addPushRule(kind: RuleKind, pushRule: PushRule)

suspend fun updatePushRuleActions(kind: RuleKind, oldPushRule: PushRule, newPushRule: PushRule)
suspend fun updatePushRuleActions(kind: RuleKind, ruleId: String, enable: Boolean, actions: List<Action>?)
bmarty marked this conversation as resolved.
Show resolved Hide resolved

suspend fun removePushRule(kind: RuleKind, pushRule: PushRule)

Expand Down
7 changes: 7 additions & 0 deletions matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/pushrules/rest/PushRule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,13 @@ data class PushRule(
)
}

/**
* Get the highlight status. As spec mentions assume false if no tweak present.
*/
fun getHighlight(): Boolean {
return (getActions().firstOrNull { it is Action.Highlight } as? Action.Highlight)?.highlight ?: false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can rewrite to
return getActions().filterIsInstance<Action.Highlight>().firstOrNull()?.highlight.orFalse()

}

/**
* Set the notification status.
*
Expand Down
4 changes: 2 additions & 2 deletions .../main/java/org/matrix/android/sdk/internal/session/notification/DefaultPushRuleService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ internal class DefaultPushRuleService @Inject constructor(
addPushRuleTask.execute(AddPushRuleTask.Params(kind, pushRule))
}

override suspend fun updatePushRuleActions(kind: RuleKind, oldPushRule: PushRule, newPushRule: PushRule) {
updatePushRuleActionsTask.execute(UpdatePushRuleActionsTask.Params(kind, oldPushRule, newPushRule))
override suspend fun updatePushRuleActions(kind: RuleKind, ruleId: String, enable: Boolean, actions: List<Action>?) {
updatePushRuleActionsTask.execute(UpdatePushRuleActionsTask.Params(kind, ruleId, enable, actions))
}

override suspend fun removePushRule(kind: RuleKind, pushRule: PushRule) {
Expand Down
26 changes: 11 additions & 15 deletions ...rc/main/java/org/matrix/android/sdk/internal/session/pushers/UpdatePushRuleActionsTask.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@
*/
package org.matrix.android.sdk.internal.session.pushers

import org.matrix.android.sdk.api.pushrules.Action
import org.matrix.android.sdk.api.pushrules.RuleKind
import org.matrix.android.sdk.api.pushrules.rest.PushRule
import org.matrix.android.sdk.api.pushrules.toJson
import org.matrix.android.sdk.internal.network.GlobalErrorReceiver
import org.matrix.android.sdk.internal.network.executeRequest
import org.matrix.android.sdk.internal.task.Task
Expand All @@ -25,8 +26,9 @@ import javax.inject.Inject
internal interface UpdatePushRuleActionsTask : Task<UpdatePushRuleActionsTask.Params, Unit> {
data class Params(
val kind: RuleKind,
val oldPushRule: PushRule,
val newPushRule: PushRule
val ruleId: String,
val enable: Boolean,
val actions: List<Action>?
)
}

Expand All @@ -36,20 +38,14 @@ internal class DefaultUpdatePushRuleActionsTask @Inject constructor(
) : UpdatePushRuleActionsTask {

override suspend fun execute(params: UpdatePushRuleActionsTask.Params) {
if (params.oldPushRule.enabled != params.newPushRule.enabled) {
// First change enabled state
executeRequest(globalErrorReceiver) {
pushRulesApi.updateEnableRuleStatus(params.kind.value, params.newPushRule.ruleId, params.newPushRule.enabled)
pushRulesApi.updateEnableRuleStatus(params.kind.value, params.ruleId, enable = params.enable)
}
}

if (params.newPushRule.enabled) {
// Also ensure the actions are up to date
val body = mapOf("actions" to params.newPushRule.actions)

executeRequest(globalErrorReceiver) {
pushRulesApi.updateRuleActions(params.kind.value, params.newPushRule.ruleId, body)
if (params.actions != null) {
val body = mapOf("actions" to params.actions.toJson())
executeRequest(globalErrorReceiver) {
pushRulesApi.updateRuleActions(params.kind.value, params.ruleId, body)
}
}
}
}
}
6 changes: 3 additions & 3 deletions vector/src/main/java/im/vector/app/core/di/FragmentModule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,12 @@ import im.vector.app.features.roomprofile.settings.RoomSettingsFragment
import im.vector.app.features.roomprofile.uploads.RoomUploadsFragment
import im.vector.app.features.roomprofile.uploads.files.RoomUploadsFilesFragment
import im.vector.app.features.roomprofile.uploads.media.RoomUploadsMediaFragment
import im.vector.app.features.settings.VectorSettingsAdvancedNotificationPreferenceFragment
import im.vector.app.features.settings.notifications.VectorSettingsAdvancedNotificationPreferenceFragment
import im.vector.app.features.settings.VectorSettingsGeneralFragment
import im.vector.app.features.settings.VectorSettingsHelpAboutFragment
import im.vector.app.features.settings.VectorSettingsLabsFragment
import im.vector.app.features.settings.VectorSettingsNotificationPreferenceFragment
import im.vector.app.features.settings.VectorSettingsNotificationsTroubleshootFragment
import im.vector.app.features.settings.notifications.VectorSettingsNotificationPreferenceFragment
import im.vector.app.features.settings.notifications.VectorSettingsNotificationsTroubleshootFragment
import im.vector.app.features.settings.VectorSettingsPinFragment
import im.vector.app.features.settings.VectorSettingsPreferencesFragment
import im.vector.app.features.settings.VectorSettingsSecurityPrivacyFragment
Expand Down
148 changes: 26 additions & 122 deletions vector/src/main/java/im/vector/app/core/preference/PushRulePreference.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,23 @@ import android.view.View
import android.widget.RadioGroup
import androidx.preference.PreferenceViewHolder
import im.vector.app.R
import org.matrix.android.sdk.api.pushrules.Action
import org.matrix.android.sdk.api.pushrules.RuleIds
import org.matrix.android.sdk.api.pushrules.RuleSetKey
import org.matrix.android.sdk.api.pushrules.rest.PushRule
import org.matrix.android.sdk.api.pushrules.rest.PushRuleAndKind

class PushRulePreference : VectorPreference {

enum class NotificationIndex(val index: Int) {
OFF(0),
SILENT(1),
NOISEY(2);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo?


companion object {
fun fromInt(index: Int) = values().first { it.index == index }
}
}

/**
* @return the selected push rule and its kind
* @return the selected push rule index
*/
var ruleAndKind: PushRuleAndKind? = null
var index: NotificationIndex? = null
private set

constructor(context: Context) : super(context)
Expand All @@ -47,119 +52,26 @@ class PushRulePreference : VectorPreference {
}

/**
* @return the bing rule status index
*/
private val ruleStatusIndex: Int
get() {
val safeRule = ruleAndKind?.pushRule ?: return NOTIFICATION_OFF_INDEX

if (safeRule.ruleId == RuleIds.RULE_ID_SUPPRESS_BOTS_NOTIFICATIONS) {
if (safeRule.shouldNotNotify()) {
return if (safeRule.enabled) {
NOTIFICATION_OFF_INDEX
} else {
NOTIFICATION_SILENT_INDEX
}
} else if (safeRule.shouldNotify()) {
return NOTIFICATION_NOISY_INDEX
}
}

if (safeRule.enabled) {
return if (safeRule.shouldNotNotify()) {
NOTIFICATION_OFF_INDEX
} else if (safeRule.getNotificationSound() != null) {
NOTIFICATION_NOISY_INDEX
} else {
NOTIFICATION_SILENT_INDEX
}
}

return NOTIFICATION_OFF_INDEX
}

/**
* Update the push rule.
* Update the notification index.
*
* @param pushRule
*/
fun setPushRule(pushRuleAndKind: PushRuleAndKind?) {
ruleAndKind = pushRuleAndKind
fun setIndex(notificationIndex: NotificationIndex?) {
index = notificationIndex
refreshSummary()
}

/**
* Refresh the summary
*/
private fun refreshSummary() {
summary = context.getString(when (ruleStatusIndex) {
NOTIFICATION_OFF_INDEX -> R.string.notification_off
NOTIFICATION_SILENT_INDEX -> R.string.notification_silent
else -> R.string.notification_noisy
summary = context.getString(when (index) {
NotificationIndex.OFF -> R.string.notification_off
NotificationIndex.SILENT -> R.string.notification_silent
NotificationIndex.NOISEY, null -> R.string.notification_noisy
})
}

/**
* Create a push rule with the updated required at index.
*
* @param index index
* @return a push rule with the updated flags / null if there is no update
*/
fun createNewRule(index: Int): PushRule? {
val safeRule = ruleAndKind?.pushRule ?: return null
val safeKind = ruleAndKind?.kind ?: return null

return if (index != ruleStatusIndex) {
if (safeRule.ruleId == RuleIds.RULE_ID_SUPPRESS_BOTS_NOTIFICATIONS) {
when (index) {
NOTIFICATION_OFF_INDEX -> {
safeRule.copy(enabled = true)
.setNotify(false)
.removeNotificationSound()
}
NOTIFICATION_SILENT_INDEX -> {
safeRule.copy(enabled = false)
.setNotify(false)
}
NOTIFICATION_NOISY_INDEX -> {
safeRule.copy(enabled = true)
.setNotify(true)
.setNotificationSound()
}
else -> safeRule
}
} else {
if (NOTIFICATION_OFF_INDEX == index) {
if (safeKind == RuleSetKey.UNDERRIDE || safeRule.ruleId == RuleIds.RULE_ID_SUPPRESS_BOTS_NOTIFICATIONS) {
safeRule.setNotify(false)
} else {
safeRule.copy(enabled = false)
}
} else {
val newRule = safeRule.copy(enabled = true)
.setNotify(true)
.setHighlight(safeKind != RuleSetKey.UNDERRIDE
&& safeRule.ruleId != RuleIds.RULE_ID_INVITE_ME
&& NOTIFICATION_NOISY_INDEX == index)

if (NOTIFICATION_NOISY_INDEX == index) {
newRule.setNotificationSound(
if (safeRule.ruleId == RuleIds.RULE_ID_CALL) {
Action.ACTION_OBJECT_VALUE_VALUE_RING
} else {
Action.ACTION_OBJECT_VALUE_VALUE_DEFAULT
}
)
} else {
newRule.removeNotificationSound()
}
}
}
} else {
safeRule
}
}

override fun onBindViewHolder(holder: PreferenceViewHolder) {
super.onBindViewHolder(holder)

Expand All @@ -170,38 +82,30 @@ class PushRulePreference : VectorPreference {
val radioGroup = holder.findViewById(R.id.bingPreferenceRadioGroup) as? RadioGroup
radioGroup?.setOnCheckedChangeListener(null)

when (ruleStatusIndex) {
NOTIFICATION_OFF_INDEX -> {
when (index) {
NotificationIndex.OFF -> {
radioGroup?.check(R.id.bingPreferenceRadioBingRuleOff)
}
NOTIFICATION_SILENT_INDEX -> {
NotificationIndex.SILENT -> {
radioGroup?.check(R.id.bingPreferenceRadioBingRuleSilent)
}
else -> {
NotificationIndex.NOISEY -> {
radioGroup?.check(R.id.bingPreferenceRadioBingRuleNoisy)
}
}

radioGroup?.setOnCheckedChangeListener { _, checkedId ->
when (checkedId) {
R.id.bingPreferenceRadioBingRuleOff -> {
onPreferenceChangeListener?.onPreferenceChange(this, NOTIFICATION_OFF_INDEX)
onPreferenceChangeListener?.onPreferenceChange(this, NotificationIndex.OFF)
}
R.id.bingPreferenceRadioBingRuleSilent -> {
onPreferenceChangeListener?.onPreferenceChange(this, NOTIFICATION_SILENT_INDEX)
onPreferenceChangeListener?.onPreferenceChange(this, NotificationIndex.SILENT)
}
R.id.bingPreferenceRadioBingRuleNoisy -> {
onPreferenceChangeListener?.onPreferenceChange(this, NOTIFICATION_NOISY_INDEX)
onPreferenceChangeListener?.onPreferenceChange(this, NotificationIndex.NOISEY)
}
}
}
}

companion object {

// index in mRuleStatuses
private const val NOTIFICATION_OFF_INDEX = 0
private const val NOTIFICATION_SILENT_INDEX = 1
private const val NOTIFICATION_NOISY_INDEX = 2
}
}
1 change: 1 addition & 0 deletions vector/src/main/java/im/vector/app/features/settings/VectorSettingsActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import im.vector.app.core.extensions.replaceFragment
import im.vector.app.core.platform.VectorBaseActivity
import im.vector.app.databinding.ActivityVectorSettingsBinding
import im.vector.app.features.settings.devices.VectorSettingsDevicesFragment
import im.vector.app.features.settings.notifications.VectorSettingsNotificationPreferenceFragment

import org.matrix.android.sdk.api.failure.GlobalError
import org.matrix.android.sdk.api.session.Session
Expand Down
Loading