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

Deferred DMs - Add and enable the feature by default in the labs settings #7180

Merged
merged 5 commits into from
Sep 21, 2022
Merged
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: 1 addition & 0 deletions changelog.d/7180.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Deferred DMs - Enable and move the feature to labs settings
3 changes: 3 additions & 0 deletions library/ui-strings/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
@@ -442,6 +442,9 @@
<string name="labs_enable_new_app_layout_title">Enable new layout</string>
<string name="labs_enable_new_app_layout_summary">A simplified Element with optional tabs</string>

<string name="labs_enable_deferred_dm_title">Enable deferred DMs</string>
<string name="labs_enable_deferred_dm_summary">Create DM only on first message</string>

<!-- Home fragment -->
<string name="invitations_header">Invites</string>
<string name="low_priority_header">Low priority</string>
Original file line number Diff line number Diff line change
@@ -80,11 +80,6 @@ class DebugFeaturesStateFactory @Inject constructor(
key = DebugFeatureKeys.forceUsageOfOpusEncoder,
factory = VectorFeatures::forceUsageOfOpusEncoder
),
createBooleanFeature(
label = "Start DM on first message",
key = DebugFeatureKeys.startDmOnFirstMsg,
factory = VectorFeatures::shouldStartDmOnFirstMessage
),
createBooleanFeature(
label = "Enable New App Layout",
key = DebugFeatureKeys.newAppLayoutEnabled,
Original file line number Diff line number Diff line change
@@ -73,9 +73,6 @@ class DebugVectorFeatures(
override fun forceUsageOfOpusEncoder(): Boolean = read(DebugFeatureKeys.forceUsageOfOpusEncoder)
?: vectorFeatures.forceUsageOfOpusEncoder()

override fun shouldStartDmOnFirstMessage(): Boolean = read(DebugFeatureKeys.startDmOnFirstMsg)
?: vectorFeatures.shouldStartDmOnFirstMessage()

override fun isNewAppLayoutFeatureEnabled(): Boolean = read(DebugFeatureKeys.newAppLayoutEnabled)
?: vectorFeatures.isNewAppLayoutFeatureEnabled()

2 changes: 2 additions & 0 deletions vector-config/src/main/res/values/config-settings.xml
Original file line number Diff line number Diff line change
@@ -37,6 +37,8 @@
<bool name="settings_ignored_users_visible">true</bool>

<!-- Level 1: Labs -->
<bool name="settings_labs_deferred_dm_visible">true</bool>
<bool name="settings_labs_deferred_dm_default">true</bool>
Copy link
Member

Choose a reason for hiding this comment

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

See the doc at the top of this file, you should add another key with visible suffix:

<bool name="settings_labs_deferred_dm_visible">true</bool>

and use it in the labs pref file.
Maybe for this very temporary setting you could take this remark as optional.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done dd92bb7 but the "doc" at the beginning of the file does not tell that these fields are "mandatory", it's more a description of the naming convention.

Copy link
Member

Choose a reason for hiding this comment

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

OK, thanks 👐

<bool name="settings_labs_thread_messages_default">false</bool>
<bool name="settings_labs_new_app_layout_default">true</bool>
<bool name="settings_timeline_show_live_sender_info_visible">true</bool>
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
@@ -33,7 +33,6 @@ interface VectorFeatures {
fun isScreenSharingEnabled(): Boolean
fun isLocationSharingEnabled(): Boolean
fun forceUsageOfOpusEncoder(): Boolean
fun shouldStartDmOnFirstMessage(): Boolean

/**
* This is only to enable if the labs flag should be visible and effective.
@@ -56,7 +55,6 @@ class DefaultVectorFeatures : VectorFeatures {
override fun isScreenSharingEnabled(): Boolean = true
override fun isLocationSharingEnabled() = Config.ENABLE_LOCATION_SHARING
override fun forceUsageOfOpusEncoder(): Boolean = false
override fun shouldStartDmOnFirstMessage(): Boolean = false
override fun isNewAppLayoutFeatureEnabled(): Boolean = true
override fun isNewDeviceManagementEnabled(): Boolean = false
}
Original file line number Diff line number Diff line change
@@ -26,11 +26,11 @@ import im.vector.app.core.di.MavericksAssistedViewModelFactory
import im.vector.app.core.di.hiltMavericksViewModelFactory
import im.vector.app.core.mvrx.runCatchingToAsync
import im.vector.app.core.platform.VectorViewModel
import im.vector.app.features.VectorFeatures
import im.vector.app.features.analytics.AnalyticsTracker
import im.vector.app.features.analytics.plan.CreatedRoom
import im.vector.app.features.raw.wellknown.getElementWellknown
import im.vector.app.features.raw.wellknown.isE2EByDefault
import im.vector.app.features.settings.VectorPreferences
import im.vector.app.features.userdirectory.PendingSelection
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
@@ -45,9 +45,9 @@ import org.matrix.android.sdk.api.session.room.model.create.CreateRoomParams
class CreateDirectRoomViewModel @AssistedInject constructor(
@Assisted initialState: CreateDirectRoomViewState,
private val rawService: RawService,
private val vectorPreferences: VectorPreferences,
val session: Session,
val analyticsTracker: AnalyticsTracker,
val vectorFeatures: VectorFeatures
) :
VectorViewModel<CreateDirectRoomViewState, CreateDirectRoomAction, CreateDirectRoomViewEvents>(initialState) {

@@ -124,7 +124,7 @@ class CreateDirectRoomViewModel @AssistedInject constructor(
}

val result = runCatchingToAsync {
if (vectorFeatures.shouldStartDmOnFirstMessage()) {
if (vectorPreferences.isDeferredDmEnabled()) {
Copy link
Member

Choose a reason for hiding this comment

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

Just a remark as I am seeing this now: can you check that analytics will be sent when the room will be created?
If the feature is disabled, it's sent in the else block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good remark, done fa8b56b
I did not find a better place to add it

session.roomService().createLocalRoom(roomParams)
} else {
analyticsTracker.capture(CreatedRoom(isDM = roomParams.isDirect.orFalse()))
Original file line number Diff line number Diff line change
@@ -16,11 +16,11 @@

package im.vector.app.features.createdirect

import im.vector.app.features.VectorFeatures
import im.vector.app.features.analytics.AnalyticsTracker
import im.vector.app.features.analytics.plan.CreatedRoom
import im.vector.app.features.raw.wellknown.getElementWellknown
import im.vector.app.features.raw.wellknown.isE2EByDefault
import im.vector.app.features.settings.VectorPreferences
import org.matrix.android.sdk.api.extensions.orFalse
import org.matrix.android.sdk.api.extensions.tryOrNull
import org.matrix.android.sdk.api.raw.RawService
@@ -32,7 +32,7 @@ class DirectRoomHelper @Inject constructor(
private val rawService: RawService,
private val session: Session,
private val analyticsTracker: AnalyticsTracker,
private val vectorFeatures: VectorFeatures,
private val vectorPreferences: VectorPreferences,
) {

suspend fun ensureDMExists(userId: String): String {
@@ -50,7 +50,7 @@ class DirectRoomHelper @Inject constructor(
setDirectMessage()
enableEncryptionIfInvitedUsersSupportIt = adminE2EByDefault
}
roomId = if (vectorFeatures.shouldStartDmOnFirstMessage()) {
roomId = if (vectorPreferences.isDeferredDmEnabled()) {
session.roomService().createLocalRoom(roomParams)
} else {
analyticsTracker.capture(CreatedRoom(isDM = roomParams.isDirect.orFalse()))
Original file line number Diff line number Diff line change
@@ -39,6 +39,7 @@ import im.vector.app.core.utils.BehaviorDataSource
import im.vector.app.features.analytics.AnalyticsTracker
import im.vector.app.features.analytics.DecryptionFailureTracker
import im.vector.app.features.analytics.extensions.toAnalyticsJoinedRoom
import im.vector.app.features.analytics.plan.CreatedRoom
import im.vector.app.features.analytics.plan.JoinedRoom
import im.vector.app.features.call.conference.ConferenceEvent
import im.vector.app.features.call.conference.JitsiActiveConferenceHolder
@@ -78,6 +79,7 @@ import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import org.matrix.android.sdk.api.MatrixPatterns
import org.matrix.android.sdk.api.extensions.orFalse
import org.matrix.android.sdk.api.extensions.tryOrNull
import org.matrix.android.sdk.api.query.QueryStringValue
import org.matrix.android.sdk.api.raw.RawService
@@ -1247,8 +1249,12 @@ class TimelineViewModel @AssistedInject constructor(
LocalRoomCreationState.FAILURE -> {
_viewEvents.post(RoomDetailViewEvents.HideWaitingView)
}
LocalRoomCreationState.CREATED ->
_viewEvents.post(RoomDetailViewEvents.OpenRoom(room.localRoomSummary()?.replacementRoomId!!, true))
LocalRoomCreationState.CREATED -> {
room.localRoomSummary()?.let {
analyticsTracker.capture(CreatedRoom(isDM = it.roomSummary?.isDirect.orFalse()))
_viewEvents.post(RoomDetailViewEvents.OpenRoom(it.replacementRoomId!!, true))
}
}
}
}
.launchIn(viewModelScope)
Original file line number Diff line number Diff line change
@@ -66,6 +66,7 @@ class VectorPreferences @Inject constructor(
const val SETTINGS_BACKGROUND_SYNC_DIVIDER_PREFERENCE_KEY = "SETTINGS_BACKGROUND_SYNC_DIVIDER_PREFERENCE_KEY"
const val SETTINGS_LABS_PREFERENCE_KEY = "SETTINGS_LABS_PREFERENCE_KEY"
const val SETTINGS_LABS_NEW_APP_LAYOUT_KEY = "SETTINGS_LABS_NEW_APP_LAYOUT_KEY"
const val SETTINGS_LABS_DEFERRED_DM_KEY = "SETTINGS_LABS_DEFERRED_DM_KEY"
const val SETTINGS_CRYPTOGRAPHY_PREFERENCE_KEY = "SETTINGS_CRYPTOGRAPHY_PREFERENCE_KEY"
const val SETTINGS_CRYPTOGRAPHY_DIVIDER_PREFERENCE_KEY = "SETTINGS_CRYPTOGRAPHY_DIVIDER_PREFERENCE_KEY"
const val SETTINGS_CRYPTOGRAPHY_MANAGE_PREFERENCE_KEY = "SETTINGS_CRYPTOGRAPHY_MANAGE_PREFERENCE_KEY"
@@ -1162,6 +1163,13 @@ class VectorPreferences @Inject constructor(
defaultPrefs.getBoolean(SETTINGS_LABS_NEW_APP_LAYOUT_KEY, getDefault(R.bool.settings_labs_new_app_layout_default))
}

/**
* Indicates whether or not deferred DMs are enabled.
*/
fun isDeferredDmEnabled(): Boolean {
return defaultPrefs.getBoolean(SETTINGS_LABS_DEFERRED_DM_KEY, getDefault(R.bool.settings_labs_deferred_dm_default))
}

fun showLiveSenderInfo(): Boolean {
return defaultPrefs.getBoolean(SETTINGS_TIMELINE_SHOW_LIVE_SENDER_INFO, getDefault(R.bool.settings_timeline_show_live_sender_info_default))
}
9 changes: 8 additions & 1 deletion vector/src/main/res/xml/vector_settings_labs.xml
Original file line number Diff line number Diff line change
@@ -47,8 +47,8 @@

<im.vector.app.core.preference.VectorSwitchPreference
android:defaultValue="false"
android:persistent="false"
android:key="SETTINGS_LABS_MSC3061_SHARE_KEYS_HISTORY"
android:persistent="false"
android:summary="@string/labs_enable_msc3061_share_history_desc"
android:title="@string/labs_enable_msc3061_share_history" />

@@ -89,4 +89,11 @@
android:summary="@string/labs_enable_new_app_layout_summary"
android:title="@string/labs_enable_new_app_layout_title" />

<im.vector.app.core.preference.VectorSwitchPreference
android:defaultValue="@bool/settings_labs_deferred_dm_default"
android:key="SETTINGS_LABS_DEFERRED_DM_KEY"
android:summary="@string/labs_enable_deferred_dm_summary"
android:title="@string/labs_enable_deferred_dm_title"
app:isPreferenceVisible="@bool/settings_labs_deferred_dm_visible" />

</androidx.preference.PreferenceScreen>