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

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

Merged
merged 5 commits into from
Sep 21, 2022
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e9d809d
Move and enable deferred DMs into labs settings
Sep 19, 2022
3786bd9
changelog
Sep 20, 2022
dd92bb7
Add visibility setting field for lab setting
Sep 21, 2022
fa8b56b
Restore tracking for deferred DMs
Sep 21, 2022
c252f6e
Update lab setting wording following design review
Sep 21, 2022
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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
5 changes: 0 additions & 5 deletions vector-app/src/debug/java/im/vector/app/features/debug/features/DebugFeaturesStateFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 0 additions & 3 deletions vector-app/src/debug/java/im/vector/app/features/debug/features/DebugVectorFeatures.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
2 changes: 2 additions & 0 deletions vector-config/src/main/res/values/config-settings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 0 additions & 2 deletions vector/src/main/java/im/vector/app/features/VectorFeatures.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
}
6 changes: 3 additions & 3 deletions vector/src/main/java/im/vector/app/features/createdirect/CreateDirectRoomViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {

Expand Down Expand Up @@ -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()))
Expand Down
6 changes: 3 additions & 3 deletions vector/src/main/java/im/vector/app/features/createdirect/DirectRoomHelper.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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()))
Expand Down
10 changes: 8 additions & 2 deletions vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 8 additions & 0 deletions vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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))
}
Expand Down
9 changes: 8 additions & 1 deletion vector/src/main/res/xml/vector_settings_labs.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" />

Expand Down Expand Up @@ -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>