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.

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

Removing notification settings v1 #4627

Merged
merged 3 commits into from
Dec 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion vector/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ android {
resValue "string", "build_number", "\"${buildNumber}\""

buildConfigField "im.vector.app.features.VectorFeatures.LoginVersion", "LOGIN_VERSION", "im.vector.app.features.VectorFeatures.LoginVersion.V1"
buildConfigField "im.vector.app.features.VectorFeatures.NotificationSettingsVersion", "NOTIFICATION_SETTINGS_VERSION", "im.vector.app.features.VectorFeatures.NotificationSettingsVersion.V2"

buildConfigField "im.vector.app.features.crypto.keysrequest.OutboundSessionKeySharingStrategy", "outboundSessionKeySharingStrategy", "im.vector.app.features.crypto.keysrequest.OutboundSessionKeySharingStrategy.WhenTyping"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,29 +18,19 @@ package im.vector.app.ui.robot.settings

import androidx.test.espresso.Espresso.pressBack
import com.adevinta.android.barista.interaction.BaristaClickInteractions.clickOn
import im.vector.app.BuildConfig
import im.vector.app.R
import im.vector.app.espresso.tools.clickOnPreference
import im.vector.app.features.VectorFeatures

class SettingsNotificationsRobot {

fun crawl() {
when (BuildConfig.NOTIFICATION_SETTINGS_VERSION!!) {
VectorFeatures.NotificationSettingsVersion.V1 -> {
clickOn(R.string.settings_notification_advanced)
pressBack()
}
VectorFeatures.NotificationSettingsVersion.V2 -> {
clickOn(R.string.settings_notification_default)
pressBack()
clickOn(R.string.settings_notification_mentions_and_keywords)
// TODO Test adding a keyword?
pressBack()
clickOn(R.string.settings_notification_other)
pressBack()
}
}
clickOn(R.string.settings_notification_default)
pressBack()
clickOn(R.string.settings_notification_mentions_and_keywords)
// TODO Test adding a keyword?
pressBack()
clickOn(R.string.settings_notification_other)
pressBack()

/*
clickOn(R.string.settings_noisy_notifications_preferences)
Expand Down
2 changes: 0 additions & 2 deletions vector/src/main/java/im/vector/app/features/VectorFeatures.kt
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import im.vector.app.BuildConfig
interface VectorFeatures {

fun loginVersion(): LoginVersion
fun notificationSettingsVersion(): NotificationSettingsVersion

enum class LoginVersion {
V1,
Expand All @@ -36,5 +35,4 @@ interface VectorFeatures {

class DefaultVectorFeatures : VectorFeatures {
override fun loginVersion(): VectorFeatures.LoginVersion = BuildConfig.LOGIN_VERSION
override fun notificationSettingsVersion(): VectorFeatures.NotificationSettingsVersion = BuildConfig.NOTIFICATION_SETTINGS_VERSION
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ import im.vector.app.core.resources.UserPreferencesProvider
import im.vector.app.databinding.FragmentRoomListBinding
import im.vector.app.features.home.RoomListDisplayMode
import im.vector.app.features.home.room.filtered.FilteredRoomFooterItem
import im.vector.app.features.home.room.list.actions.RoomListActionsArgs
import im.vector.app.features.home.room.list.actions.RoomListQuickActionsBottomSheet
import im.vector.app.features.home.room.list.actions.RoomListQuickActionsSharedAction
import im.vector.app.features.home.room.list.actions.RoomListQuickActionsSharedActionViewModel
Expand Down Expand Up @@ -476,7 +475,7 @@ class RoomListFragment @Inject constructor(
footerController.setData(it)
}
RoomListQuickActionsBottomSheet
.newInstance(room.roomId, RoomListActionsArgs.Mode.FULL)
.newInstance(room.roomId)
.show(childFragmentManager, "ROOM_LIST_QUICK_ACTIONS")
return true
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,8 @@ import javax.inject.Inject

@Parcelize
data class RoomListActionsArgs(
val roomId: String,
val mode: Mode
) : Parcelable {

enum class Mode {
FULL,
NOTIFICATIONS
}
}
val roomId: String
) : Parcelable

/**
* Bottom sheet fragment that shows room information with list of contextual actions
Expand Down Expand Up @@ -124,9 +117,9 @@ class RoomListQuickActionsBottomSheet :
}

companion object {
fun newInstance(roomId: String, mode: RoomListActionsArgs.Mode): RoomListQuickActionsBottomSheet {
fun newInstance(roomId: String): RoomListQuickActionsBottomSheet {
return RoomListQuickActionsBottomSheet().apply {
setArguments(RoomListActionsArgs(roomId, mode))
setArguments(RoomListActionsArgs(roomId))
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import im.vector.app.core.epoxy.bottomsheet.bottomSheetRoomPreviewItem
import im.vector.app.core.epoxy.profiles.notifications.radioButtonItem
import im.vector.app.core.resources.ColorProvider
import im.vector.app.core.resources.StringProvider
import im.vector.app.features.VectorFeatures
import im.vector.app.features.home.AvatarRenderer
import im.vector.app.features.roomprofile.notifications.notificationOptions
import im.vector.app.features.roomprofile.notifications.notificationStateMapped
Expand All @@ -39,7 +38,6 @@ class RoomListQuickActionsEpoxyController @Inject constructor(
private val avatarRenderer: AvatarRenderer,
private val colorProvider: ColorProvider,
private val stringProvider: StringProvider,
private val features: VectorFeatures
) : TypedEpoxyController<RoomListQuickActionViewState>() {

var listener: Listener? = null
Expand All @@ -48,54 +46,38 @@ class RoomListQuickActionsEpoxyController @Inject constructor(
val notificationViewState = state.notificationSettingsViewState
val roomSummary = notificationViewState.roomSummary() ?: return
val host = this
val isV2 = features.notificationSettingsVersion() == VectorFeatures.NotificationSettingsVersion.V2
// V2 always shows full details as we no longer display the sheet from RoomProfile > Notifications
val showFull = state.roomListActionsArgs.mode == RoomListActionsArgs.Mode.FULL || isV2

if (showFull) {
// Preview, favorite, settings
bottomSheetRoomPreviewItem {
id("room_preview")
avatarRenderer(host.avatarRenderer)
matrixItem(roomSummary.toMatrixItem())
stringProvider(host.stringProvider)
colorProvider(host.colorProvider)
izLowPriority(roomSummary.isLowPriority)
izFavorite(roomSummary.isFavorite)
settingsClickListener { host.listener?.didSelectMenuAction(RoomListQuickActionsSharedAction.Settings(roomSummary.roomId)) }
favoriteClickListener { host.listener?.didSelectMenuAction(RoomListQuickActionsSharedAction.Favorite(roomSummary.roomId)) }
lowPriorityClickListener { host.listener?.didSelectMenuAction(RoomListQuickActionsSharedAction.LowPriority(roomSummary.roomId)) }
}
// Preview, favorite, settings
bottomSheetRoomPreviewItem {
Copy link
Member

Choose a reason for hiding this comment

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

Test state.roomListActionsArgs.mode == RoomListActionsArgs.Mode.FULL should stay

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the previous condition was

// V2 always shows full details as we no longer display the sheet from RoomProfile > Notifications
val showFull = state.roomListActionsArgs.mode == RoomListActionsArgs.Mode.FULL || isV2

now that v2 is always true, do we need the condition (perhaps it was wrong previously 🤔 )

Copy link
Member

Choose a reason for hiding this comment

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

I do not think it was wrong before, it was OK in v1 mode, but with this PR, RoomListActionsArgs.Mode.NOTIFICATIONS is never used, so maybe just delete RoomListActionsArgs.Mode.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will do!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed c208c2d

id("room_preview")
avatarRenderer(host.avatarRenderer)
matrixItem(roomSummary.toMatrixItem())
stringProvider(host.stringProvider)
colorProvider(host.colorProvider)
izLowPriority(roomSummary.isLowPriority)
izFavorite(roomSummary.isFavorite)
settingsClickListener { host.listener?.didSelectMenuAction(RoomListQuickActionsSharedAction.Settings(roomSummary.roomId)) }
favoriteClickListener { host.listener?.didSelectMenuAction(RoomListQuickActionsSharedAction.Favorite(roomSummary.roomId)) }
lowPriorityClickListener { host.listener?.didSelectMenuAction(RoomListQuickActionsSharedAction.LowPriority(roomSummary.roomId)) }
}

// Notifications
bottomSheetDividerItem {
id("notifications_separator")
}
// Notifications
bottomSheetDividerItem {
id("notifications_separator")
}

if (isV2) {
notificationViewState.notificationOptions.forEach { notificationState ->
val title = titleForNotificationState(notificationState)
radioButtonItem {
id(notificationState.name)
titleRes(title)
selected(notificationViewState.notificationStateMapped() == notificationState)
listener {
host.listener?.didSelectRoomNotificationState(notificationState)
}
notificationViewState.notificationOptions.forEach { notificationState ->
val title = titleForNotificationState(notificationState)
radioButtonItem {
id(notificationState.name)
titleRes(title)
selected(notificationViewState.notificationStateMapped() == notificationState)
listener {
host.listener?.didSelectRoomNotificationState(notificationState)
}
}
} else {
val selectedRoomState = notificationViewState.notificationState()
RoomListQuickActionsSharedAction.NotificationsAllNoisy(roomSummary.roomId).toBottomSheetItem(0, selectedRoomState)
RoomListQuickActionsSharedAction.NotificationsAll(roomSummary.roomId).toBottomSheetItem(1, selectedRoomState)
RoomListQuickActionsSharedAction.NotificationsMentionsOnly(roomSummary.roomId).toBottomSheetItem(2, selectedRoomState)
RoomListQuickActionsSharedAction.NotificationsMute(roomSummary.roomId).toBottomSheetItem(3, selectedRoomState)
}

if (showFull) {
RoomListQuickActionsSharedAction.Leave(roomSummary.roomId, showIcon = !isV2).toBottomSheetItem(5)
}
RoomListQuickActionsSharedAction.Leave(roomSummary.roomId, showIcon = !true).toBottomSheetItem()
}

@StringRes
Expand All @@ -106,18 +88,11 @@ class RoomListQuickActionsEpoxyController @Inject constructor(
else -> null
}

private fun RoomListQuickActionsSharedAction.toBottomSheetItem(index: Int, roomNotificationState: RoomNotificationState? = null) {
private fun RoomListQuickActionsSharedAction.Leave.toBottomSheetItem() {
val host = this@RoomListQuickActionsEpoxyController
val selected = when (this) {
is RoomListQuickActionsSharedAction.NotificationsAllNoisy -> roomNotificationState == RoomNotificationState.ALL_MESSAGES_NOISY
is RoomListQuickActionsSharedAction.NotificationsAll -> roomNotificationState == RoomNotificationState.ALL_MESSAGES
is RoomListQuickActionsSharedAction.NotificationsMentionsOnly -> roomNotificationState == RoomNotificationState.MENTIONS_ONLY
is RoomListQuickActionsSharedAction.NotificationsMute -> roomNotificationState == RoomNotificationState.MUTE
else -> false
}
return bottomSheetActionItem {
id("action_$index")
selected(selected)
id("action_leave")
selected(false)
if (iconResId != null) {
iconRes(iconResId)
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,10 @@ import im.vector.app.core.utils.copyToClipboard
import im.vector.app.core.utils.startSharePlainTextIntent
import im.vector.app.databinding.FragmentMatrixProfileBinding
import im.vector.app.databinding.ViewStubRoomProfileHeaderBinding
import im.vector.app.features.VectorFeatures
import im.vector.app.features.home.AvatarRenderer
import im.vector.app.features.home.room.detail.RoomDetailPendingAction
import im.vector.app.features.home.room.detail.RoomDetailPendingActionStore
import im.vector.app.features.home.room.detail.upgrade.MigrateRoomBottomSheet
import im.vector.app.features.home.room.list.actions.RoomListActionsArgs
import im.vector.app.features.home.room.list.actions.RoomListQuickActionsBottomSheet
import im.vector.app.features.home.room.list.actions.RoomListQuickActionsSharedAction
import im.vector.app.features.home.room.list.actions.RoomListQuickActionsSharedActionViewModel
import kotlinx.coroutines.flow.launchIn
Expand All @@ -69,8 +66,7 @@ data class RoomProfileArgs(
class RoomProfileFragment @Inject constructor(
private val roomProfileController: RoomProfileController,
private val avatarRenderer: AvatarRenderer,
private val roomDetailPendingActionStore: RoomDetailPendingActionStore,
private val features: VectorFeatures
private val roomDetailPendingActionStore: RoomDetailPendingActionStore
) :
VectorBaseFragment<FragmentMatrixProfileBinding>(),
RoomProfileController.Callback {
Expand Down Expand Up @@ -259,16 +255,7 @@ class RoomProfileFragment @Inject constructor(
}

override fun onNotificationsClicked() {
when (features.notificationSettingsVersion()) {
VectorFeatures.NotificationSettingsVersion.V1 -> {
RoomListQuickActionsBottomSheet
.newInstance(roomProfileArgs.roomId, RoomListActionsArgs.Mode.NOTIFICATIONS)
.show(childFragmentManager, "ROOM_PROFILE_NOTIFICATIONS")
}
VectorFeatures.NotificationSettingsVersion.V2 -> {
roomProfileSharedActionViewModel.post(RoomProfileSharedAction.OpenRoomNotificationSettings)
}
}
roomProfileSharedActionViewModel.post(RoomProfileSharedAction.OpenRoomNotificationSettings)
}

override fun onUploadsClicked() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,6 @@ class VectorPreferences @Inject constructor(private val context: Context) {
const val SETTINGS_PRIVACY_POLICY_PREFERENCE_KEY = "SETTINGS_PRIVACY_POLICY_PREFERENCE_KEY"
const val SETTINGS_DISCOVERY_PREFERENCE_KEY = "SETTINGS_DISCOVERY_PREFERENCE_KEY"

const val SETTINGS_NOTIFICATION_ADVANCED_PREFERENCE_KEY = "SETTINGS_NOTIFICATION_ADVANCED_PREFERENCE_KEY"
const val SETTINGS_NOTIFICATION_DEFAULT_PREFERENCE_KEY = "SETTINGS_NOTIFICATION_DEFAULT_PREFERENCE_KEY"
const val SETTINGS_NOTIFICATION_KEYWORD_AND_MENTIONS_PREFERENCE_KEY = "SETTINGS_NOTIFICATION_KEYWORD_AND_MENTIONS_PREFERENCE_KEY"
const val SETTINGS_NOTIFICATION_OTHER_PREFERENCE_KEY = "SETTINGS_NOTIFICATION_OTHER_PREFERENCE_KEY"
const val SETTINGS_THIRD_PARTY_NOTICES_PREFERENCE_KEY = "SETTINGS_THIRD_PARTY_NOTICES_PREFERENCE_KEY"
const val SETTINGS_OTHER_THIRD_PARTY_NOTICES_PREFERENCE_KEY = "SETTINGS_OTHER_THIRD_PARTY_NOTICES_PREFERENCE_KEY"
const val SETTINGS_COPYRIGHT_PREFERENCE_KEY = "SETTINGS_COPYRIGHT_PREFERENCE_KEY"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ import im.vector.app.core.pushers.PushersManager
import im.vector.app.core.services.GuardServiceStarter
import im.vector.app.core.utils.isIgnoringBatteryOptimizations
import im.vector.app.core.utils.requestDisablingBatteryOptimization
import im.vector.app.features.VectorFeatures
import im.vector.app.features.notifications.NotificationUtils
import im.vector.app.features.settings.BackgroundSyncMode
import im.vector.app.features.settings.BackgroundSyncModeChooserDialog
Expand All @@ -64,8 +63,7 @@ class VectorSettingsNotificationPreferenceFragment @Inject constructor(
private val pushManager: PushersManager,
private val activeSessionHolder: ActiveSessionHolder,
private val vectorPreferences: VectorPreferences,
private val guardServiceStarter: GuardServiceStarter,
private val features: VectorFeatures
private val guardServiceStarter: GuardServiceStarter
) : VectorSettingsBaseFragment(),
BackgroundSyncModeChooserDialog.InteractionListener {

Expand Down Expand Up @@ -147,7 +145,6 @@ class VectorSettingsNotificationPreferenceFragment @Inject constructor(
refreshBackgroundSyncPrefs()

handleSystemPreference()
handleVersionedSettings()
}

private fun bindEmailNotifications() {
Expand Down Expand Up @@ -312,15 +309,6 @@ class VectorSettingsNotificationPreferenceFragment @Inject constructor(
}
}

private fun handleVersionedSettings() {
val isNotificationSettingsV2Enabled = features.notificationSettingsVersion() == VectorFeatures.NotificationSettingsVersion.V2

findPreference<Preference>(VectorPreferences.SETTINGS_NOTIFICATION_ADVANCED_PREFERENCE_KEY)?.isVisible = !isNotificationSettingsV2Enabled
findPreference<Preference>(VectorPreferences.SETTINGS_NOTIFICATION_DEFAULT_PREFERENCE_KEY)?.isVisible = isNotificationSettingsV2Enabled
findPreference<Preference>(VectorPreferences.SETTINGS_NOTIFICATION_KEYWORD_AND_MENTIONS_PREFERENCE_KEY)?.isVisible = isNotificationSettingsV2Enabled
findPreference<Preference>(VectorPreferences.SETTINGS_NOTIFICATION_OTHER_PREFERENCE_KEY)?.isVisible = isNotificationSettingsV2Enabled
}

override fun onResume() {
super.onResume()
activeSessionHolder.getSafeActiveSession()?.refreshPushers()
Expand Down
8 changes: 0 additions & 8 deletions vector/src/main/res/xml/vector_settings_notifications.xml
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,6 @@
<!--android:key="SETTINGS_TURN_SCREEN_ON_PREFERENCE_KEY"-->
<!--android:title="@string/settings_turn_screen_on" />-->

<im.vector.app.core.preference.VectorPreference
android:dependency="SETTINGS_ENABLE_THIS_DEVICE_PREFERENCE_KEY"
android:key="SETTINGS_NOTIFICATION_ADVANCED_PREFERENCE_KEY"
android:persistent="false"
android:summary="@string/settings_notification_advanced_summary"
android:title="@string/settings_notification_advanced"
app:fragment="im.vector.app.features.settings.notifications.VectorSettingsAdvancedNotificationPreferenceFragment" />

<im.vector.app.core.preference.VectorPreference
android:dependency="SETTINGS_ENABLE_THIS_DEVICE_PREFERENCE_KEY"
android:key="SETTINGS_NOTIFICATION_DEFAULT_PREFERENCE_KEY"
Expand Down