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

Feature flags base #4626

Merged
merged 8 commits into from
Dec 3, 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: 1 addition & 0 deletions changelog.d/4626.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Introducing feature flagging to the login and notification settings flows
2 changes: 1 addition & 1 deletion tools/check/forbidden_strings_in_code.txt
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ Formatter\.formatShortFileSize===1
# android\.text\.TextUtils

### This is not a rule, but a warning: the number of "enum class" has changed. For Json classes, it is mandatory that they have `@JsonClass(generateAdapter = false)`. If the enum is not used as a Json class, change the value in file forbidden_strings_in_code.txt
enum class===108
enum class===110

### Do not import temporary legacy classes
import org.matrix.android.sdk.internal.legacy.riot===3
Expand Down
12 changes: 2 additions & 10 deletions vector/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -140,16 +140,8 @@ android {
buildConfigField "String", "BUILD_NUMBER", "\"${buildNumber}\""
resValue "string", "build_number", "\"${buildNumber}\""

// The two booleans must not have the same value. We need two values for the manifest
// LoginFlowV2 is disabled to be merged on develop (changelog: Improve login/register flow (#1410, #2585, #3172))
resValue "bool", "useLoginV1", "true"
resValue "bool", "useLoginV2", "false"

// NotificationSettingsV2 is disabled. To be released in conjunction with iOS/Web
def useNotificationSettingsV2 = true
buildConfigField "Boolean", "USE_NOTIFICATION_SETTINGS_V2", "${useNotificationSettingsV2}"
resValue "bool", "useNotificationSettingsV1", "${!useNotificationSettingsV2}"
resValue "bool", "useNotificationSettingsV2", "${useNotificationSettingsV2}"
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 @@ -21,22 +21,27 @@ 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() {
if (BuildConfig.USE_NOTIFICATION_SETTINGS_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()
} else {
clickOn(R.string.settings_notification_advanced)
pressBack()
when (BuildConfig.NOTIFICATION_SETTINGS_VERSION!!) {
Copy link
Member

Choose a reason for hiding this comment

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

Why not using a DefaultVectorFeatures instance here? It would prevent the usage of !! (I think)

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 point! I wasn't sure how the injections would work in the SanityTests, I would invest more time but this is removed in #4627

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_noisy_notifications_preferences)
TODO Cannot go back
Expand Down
30 changes: 9 additions & 21 deletions vector/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,11 @@

<activity android:name=".features.home.HomeActivity" />

<!-- exported="true" is required to handle android.intent.action.VIEW for URL redirection-->
<activity
android:name=".features.login.LoginActivity"
android:enabled="@bool/useLoginV1"
android:name=".features.login.SSORedirectRouterActivity"
android:exported="true"
android:launchMode="singleTask"
android:windowSoftInputMode="adjustResize">
android:theme="@style/Theme.Vector.Black.Transparent">

<!-- Add intent filter to handle redirection URL after SSO login in external browser -->
<intent-filter>
<action android:name="android.intent.action.VIEW" />
Expand All @@ -133,25 +131,15 @@
</intent-filter>
</activity>

<!-- exported="true" is required to handle android.intent.action.VIEW for URL redirection-->
<activity
android:name=".features.login2.LoginActivity2"
android:enabled="@bool/useLoginV2"
android:exported="true"
android:name=".features.login.LoginActivity"
android:launchMode="singleTask"
android:windowSoftInputMode="adjustResize">
<!-- Add intent filter to handle redirection URL after SSO login in external browser -->
<intent-filter>
<action android:name="android.intent.action.VIEW" />

<category android:name="android.intent.category.DEFAULT" />
<category android:name="android.intent.category.BROWSABLE" />
android:windowSoftInputMode="adjustResize" />

<data
android:host="connect"
android:scheme="element" />
</intent-filter>
</activity>
<activity
android:name=".features.login2.LoginActivity2"
android:launchMode="singleTask"
android:windowSoftInputMode="adjustResize" />

<!-- Add tools:ignore="Instantiatable" for the error reported only by Buildkite :/ -->
<activity
Expand Down
7 changes: 7 additions & 0 deletions vector/src/main/java/im/vector/app/core/di/SingletonModule.kt
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ import im.vector.app.core.error.DefaultErrorFormatter
import im.vector.app.core.error.ErrorFormatter
import im.vector.app.core.time.Clock
import im.vector.app.core.time.DefaultClock
import im.vector.app.features.DefaultVectorFeatures
import im.vector.app.features.VectorFeatures
import im.vector.app.features.invite.AutoAcceptInvites
import im.vector.app.features.invite.CompileTimeAutoAcceptInvites
import im.vector.app.features.navigation.DefaultNavigator
Expand Down Expand Up @@ -133,4 +135,9 @@ object VectorStaticModule {
fun providesCoroutineDispatchers(): CoroutineDispatchers {
return CoroutineDispatchers(io = Dispatchers.IO, computation = Dispatchers.Default)
}

@Provides
fun providesFeatures(): VectorFeatures {
return DefaultVectorFeatures()
}
}
19 changes: 6 additions & 13 deletions vector/src/main/java/im/vector/app/features/MainActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ import im.vector.app.features.pin.UnlockedActivity
import im.vector.app.features.popup.PopupAlertManager
import im.vector.app.features.settings.VectorPreferences
import im.vector.app.features.signout.hard.SignedOutActivity
import im.vector.app.features.signout.soft.SoftLogoutActivity
import im.vector.app.features.signout.soft.SoftLogoutActivity2
import im.vector.app.features.themes.ActivityOtherThemes
import im.vector.app.features.ui.UiStateRepository
import kotlinx.coroutines.Dispatchers
Expand Down Expand Up @@ -223,9 +221,11 @@ class MainActivity : VectorBaseActivity<ActivityMainBinding>(), UnlockedActivity
navigator.openLogin(this, null)
null
}
args.isSoftLogout ->
args.isSoftLogout -> {
// The homeserver has invalidated the token, with a soft logout
getSoftLogoutActivityIntent()
navigator.softLogout(this)
null
}
args.isUserLoggedOut ->
// the homeserver has invalidated the token (password changed, device deleted, other security reasons)
SignedOutActivity.newIntent(this)
Expand All @@ -236,7 +236,8 @@ class MainActivity : VectorBaseActivity<ActivityMainBinding>(), UnlockedActivity
HomeActivity.newIntent(this)
} else {
// The token is still invalid
getSoftLogoutActivityIntent()
navigator.softLogout(this)
null
}
else -> {
// First start, or no active session
Expand All @@ -247,12 +248,4 @@ class MainActivity : VectorBaseActivity<ActivityMainBinding>(), UnlockedActivity
intent?.let { startActivity(it) }
finish()
}

private fun getSoftLogoutActivityIntent(): Intent {
return if (resources.getBoolean(R.bool.useLoginV2)) {
SoftLogoutActivity2.newIntent(this)
} else {
SoftLogoutActivity.newIntent(this)
}
}
}
40 changes: 40 additions & 0 deletions vector/src/main/java/im/vector/app/features/VectorFeatures.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright (c) 2021 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

import im.vector.app.BuildConfig

interface VectorFeatures {

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

enum class LoginVersion {
V1,
V2
}

enum class NotificationSettingsVersion {
V1,
V2
}
}

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 @@ -17,14 +17,14 @@ package im.vector.app.features.home.room.list.actions

import androidx.annotation.StringRes
import com.airbnb.epoxy.TypedEpoxyController
import im.vector.app.BuildConfig
import im.vector.app.R
import im.vector.app.core.epoxy.bottomSheetDividerItem
import im.vector.app.core.epoxy.bottomsheet.bottomSheetActionItem
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 @@ -38,7 +38,8 @@ import javax.inject.Inject
class RoomListQuickActionsEpoxyController @Inject constructor(
private val avatarRenderer: AvatarRenderer,
private val colorProvider: ColorProvider,
private val stringProvider: StringProvider
private val stringProvider: StringProvider,
private val features: VectorFeatures
) : TypedEpoxyController<RoomListQuickActionViewState>() {

var listener: Listener? = null
Expand All @@ -47,7 +48,7 @@ class RoomListQuickActionsEpoxyController @Inject constructor(
val notificationViewState = state.notificationSettingsViewState
val roomSummary = notificationViewState.roomSummary() ?: return
val host = this
val isV2 = BuildConfig.USE_NOTIFICATION_SETTINGS_V2
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

Expand All @@ -73,14 +74,14 @@ class RoomListQuickActionsEpoxyController @Inject constructor(
}

if (isV2) {
notificationViewState.notificationOptions.forEach { notificationState ->
notificationViewState.notificationOptions.forEach { notificationState ->
val title = titleForNotificationState(notificationState)
radioButtonItem {
id(notificationState.name)
titleRes(title)
selected(notificationViewState.notificationStateMapped() == notificationState)
listener {
host.listener?.didSelectRoomNotificationState(notificationState)
host.listener?.didSelectRoomNotificationState(notificationState)
}
}
}
Expand All @@ -102,8 +103,9 @@ class RoomListQuickActionsEpoxyController @Inject constructor(
RoomNotificationState.ALL_MESSAGES_NOISY -> R.string.room_settings_all_messages
RoomNotificationState.MENTIONS_ONLY -> R.string.room_settings_mention_and_keyword_only
RoomNotificationState.MUTE -> R.string.room_settings_none
else -> null
else -> null
}

private fun RoomListQuickActionsSharedAction.toBottomSheetItem(index: Int, roomNotificationState: RoomNotificationState? = null) {
val host = this@RoomListQuickActionsEpoxyController
val selected = when (this) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package im.vector.app.features.login

import android.content.Context
import android.content.Intent
import android.net.Uri
import android.view.View
import android.view.ViewGroup
import androidx.core.view.ViewCompat
Expand Down Expand Up @@ -363,5 +364,11 @@ open class LoginActivity : VectorBaseActivity<ActivityLoginBinding>(), ToolbarCo
putExtra(EXTRA_CONFIG, loginConfig)
}
}

fun redirectIntent(context: Context, data: Uri?): Intent {
return Intent(context, LoginActivity::class.java).apply {
setData(data)
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* Copyright (c) 2021 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.login

import android.os.Bundle
import androidx.appcompat.app.AppCompatActivity
import dagger.hilt.android.AndroidEntryPoint
import im.vector.app.features.navigation.Navigator
import javax.inject.Inject

@AndroidEntryPoint
class SSORedirectRouterActivity : AppCompatActivity() {

@Inject lateinit var navigator: Navigator

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
navigator.loginSSORedirect(this, intent.data)
finish()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package im.vector.app.features.login2

import android.content.Context
import android.content.Intent
import android.net.Uri
import android.view.View
import android.view.ViewGroup
import androidx.core.view.ViewCompat
Expand Down Expand Up @@ -396,5 +397,11 @@ open class LoginActivity2 : VectorBaseActivity<ActivityLoginBinding>(), ToolbarC
putExtra(EXTRA_CONFIG, loginConfig)
}
}

fun redirectIntent(context: Context, data: Uri?): Intent {
return Intent(context, LoginActivity2::class.java).apply {
setData(data)
}
}
}
}
Loading