From 63a675d1b37decc6d5db11117352b42697fc9660 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 20 Jul 2023 22:01:45 +0200 Subject: [PATCH 1/7] Remove feature flag CollapseRoomStateEvents. It was not used anyway. --- .../element/android/libraries/featureflag/api/FeatureFlags.kt | 4 ---- .../featureflag/impl/BuildtimeFeatureFlagProvider.kt | 1 - 2 files changed, 5 deletions(-) diff --git a/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlags.kt b/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlags.kt index 920be09389..073bffe1e6 100644 --- a/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlags.kt +++ b/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlags.kt @@ -22,10 +22,6 @@ enum class FeatureFlags( override val description: String? = null, override val defaultValue: Boolean = true ) : Feature { - CollapseRoomStateEvents( - key = "feature.collapseroomstateevents", - title = "Collapse room state events", - ), ShowStartChatFlow( key = "feature.showstartchatflow", title = "Show start chat flow", diff --git a/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/BuildtimeFeatureFlagProvider.kt b/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/BuildtimeFeatureFlagProvider.kt index ae498e67df..d6fdb1621d 100644 --- a/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/BuildtimeFeatureFlagProvider.kt +++ b/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/BuildtimeFeatureFlagProvider.kt @@ -29,7 +29,6 @@ class BuildtimeFeatureFlagProvider @Inject constructor() : override suspend fun isFeatureEnabled(feature: Feature): Boolean { return if (feature is FeatureFlags) { when (feature) { - FeatureFlags.CollapseRoomStateEvents -> false FeatureFlags.ShowStartChatFlow -> false FeatureFlags.ShowMediaUploadingFlow -> false } From 062e7553db9a52ea803f2eb88f56d032e4c24ab6 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 20 Jul 2023 22:07:32 +0200 Subject: [PATCH 2/7] Remove feature flag ShowStartChatFlow. It was just used in test. --- .../libraries/featureflag/api/FeatureFlags.kt | 4 ---- .../impl/BuildtimeFeatureFlagProvider.kt | 1 - .../impl/DefaultFeatureFlagServiceTest.kt | 22 +++++++++---------- 3 files changed, 11 insertions(+), 16 deletions(-) diff --git a/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlags.kt b/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlags.kt index 073bffe1e6..64b6162053 100644 --- a/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlags.kt +++ b/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlags.kt @@ -22,10 +22,6 @@ enum class FeatureFlags( override val description: String? = null, override val defaultValue: Boolean = true ) : Feature { - ShowStartChatFlow( - key = "feature.showstartchatflow", - title = "Show start chat flow", - ), ShowMediaUploadingFlow( key = "feature.showmediauploadingflow", title = "Show media uploading flow", diff --git a/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/BuildtimeFeatureFlagProvider.kt b/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/BuildtimeFeatureFlagProvider.kt index d6fdb1621d..fe17433f17 100644 --- a/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/BuildtimeFeatureFlagProvider.kt +++ b/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/BuildtimeFeatureFlagProvider.kt @@ -29,7 +29,6 @@ class BuildtimeFeatureFlagProvider @Inject constructor() : override suspend fun isFeatureEnabled(feature: Feature): Boolean { return if (feature is FeatureFlags) { when (feature) { - FeatureFlags.ShowStartChatFlow -> false FeatureFlags.ShowMediaUploadingFlow -> false } } else { diff --git a/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeatureFlagServiceTest.kt b/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeatureFlagServiceTest.kt index 5f7f01423a..0351b20716 100644 --- a/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeatureFlagServiceTest.kt +++ b/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeatureFlagServiceTest.kt @@ -26,14 +26,14 @@ class DefaultFeatureFlagServiceTest { @Test fun `given service without provider when feature is checked then it returns the default value`() = runTest { val featureFlagService = DefaultFeatureFlagService(emptySet()) - val isFeatureEnabled = featureFlagService.isFeatureEnabled(FeatureFlags.ShowStartChatFlow) - assertThat(isFeatureEnabled).isEqualTo(FeatureFlags.ShowStartChatFlow.defaultValue) + val isFeatureEnabled = featureFlagService.isFeatureEnabled(FeatureFlags.ShowMediaUploadingFlow) + assertThat(isFeatureEnabled).isEqualTo(FeatureFlags.ShowMediaUploadingFlow.defaultValue) } @Test fun `given service without provider when set enabled feature is called then it returns false`() = runTest { val featureFlagService = DefaultFeatureFlagService(emptySet()) - val result = featureFlagService.setFeatureEnabled(FeatureFlags.ShowStartChatFlow, true) + val result = featureFlagService.setFeatureEnabled(FeatureFlags.ShowMediaUploadingFlow, true) assertThat(result).isEqualTo(false) } @@ -41,7 +41,7 @@ class DefaultFeatureFlagServiceTest { fun `given service with a runtime provider when set enabled feature is called then it returns true`() = runTest { val featureFlagProvider = FakeRuntimeFeatureFlagProvider(0) val featureFlagService = DefaultFeatureFlagService(setOf(featureFlagProvider)) - val result = featureFlagService.setFeatureEnabled(FeatureFlags.ShowStartChatFlow, true) + val result = featureFlagService.setFeatureEnabled(FeatureFlags.ShowMediaUploadingFlow, true) assertThat(result).isEqualTo(true) } @@ -49,10 +49,10 @@ class DefaultFeatureFlagServiceTest { fun `given service with a runtime provider and feature enabled when feature is checked then it returns the correct value`() = runTest { val featureFlagProvider = FakeRuntimeFeatureFlagProvider(0) val featureFlagService = DefaultFeatureFlagService(setOf(featureFlagProvider)) - featureFlagService.setFeatureEnabled(FeatureFlags.ShowStartChatFlow, true) - assertThat(featureFlagService.isFeatureEnabled(FeatureFlags.ShowStartChatFlow)).isEqualTo(true) - featureFlagService.setFeatureEnabled(FeatureFlags.ShowStartChatFlow, false) - assertThat(featureFlagService.isFeatureEnabled(FeatureFlags.ShowStartChatFlow)).isEqualTo(false) + featureFlagService.setFeatureEnabled(FeatureFlags.ShowMediaUploadingFlow, true) + assertThat(featureFlagService.isFeatureEnabled(FeatureFlags.ShowMediaUploadingFlow)).isEqualTo(true) + featureFlagService.setFeatureEnabled(FeatureFlags.ShowMediaUploadingFlow, false) + assertThat(featureFlagService.isFeatureEnabled(FeatureFlags.ShowMediaUploadingFlow)).isEqualTo(false) } @Test @@ -60,8 +60,8 @@ class DefaultFeatureFlagServiceTest { val lowPriorityfeatureFlagProvider = FakeRuntimeFeatureFlagProvider(LOW_PRIORITY) val highPriorityfeatureFlagProvider = FakeRuntimeFeatureFlagProvider(HIGH_PRIORITY) val featureFlagService = DefaultFeatureFlagService(setOf(lowPriorityfeatureFlagProvider, highPriorityfeatureFlagProvider)) - lowPriorityfeatureFlagProvider.setFeatureEnabled(FeatureFlags.ShowStartChatFlow, false) - highPriorityfeatureFlagProvider.setFeatureEnabled(FeatureFlags.ShowStartChatFlow, true) - assertThat(featureFlagService.isFeatureEnabled(FeatureFlags.ShowStartChatFlow)).isEqualTo(true) + lowPriorityfeatureFlagProvider.setFeatureEnabled(FeatureFlags.ShowMediaUploadingFlow, false) + highPriorityfeatureFlagProvider.setFeatureEnabled(FeatureFlags.ShowMediaUploadingFlow, true) + assertThat(featureFlagService.isFeatureEnabled(FeatureFlags.ShowMediaUploadingFlow)).isEqualTo(true) } } From cd3e6c42e21a80c6fba5640b545938c3fd46da37 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 20 Jul 2023 22:20:08 +0200 Subject: [PATCH 3/7] Replace FeatureFlags.ShowMediaUploadingFlow by FeatureFlags.LocationSharing because it has more chance to be disabled. I do not want to remove all our feature flags... --- .../messagecomposer/AttachmentsBottomSheet.kt | 34 +++++++++++-------- .../MessageComposerPresenter.kt | 22 ++++++------ .../messagecomposer/MessageComposerState.kt | 1 + .../MessageComposerStateProvider.kt | 25 +++++++++----- .../MessageComposerPresenterTest.kt | 2 +- .../libraries/featureflag/api/FeatureFlags.kt | 6 ++-- .../impl/BuildtimeFeatureFlagProvider.kt | 2 +- .../impl/DefaultFeatureFlagServiceTest.kt | 22 ++++++------ 8 files changed, 64 insertions(+), 50 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/AttachmentsBottomSheet.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/AttachmentsBottomSheet.kt index c8324ec677..b554ef98f4 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/AttachmentsBottomSheet.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/AttachmentsBottomSheet.kt @@ -83,7 +83,7 @@ internal fun AttachmentsBottomSheet( onDismissRequest = { isVisible = false } ) { AttachmentSourcePickerMenu( - eventSink = state.eventSink, + state = state, onSendLocationClicked = onSendLocationClicked, ) } @@ -93,7 +93,7 @@ internal fun AttachmentsBottomSheet( @OptIn(ExperimentalMaterialApi::class) @Composable internal fun AttachmentSourcePickerMenu( - eventSink: (MessageComposerEvents) -> Unit, + state: MessageComposerState, onSendLocationClicked: () -> Unit, modifier: Modifier = Modifier, ) { @@ -102,33 +102,35 @@ internal fun AttachmentSourcePickerMenu( // .navigationBarsPadding() - FIXME after https://issuetracker.google.com/issues/275849044 ) { ListItem( - modifier = Modifier.clickable { eventSink(MessageComposerEvents.PickAttachmentSource.FromGallery) }, + modifier = Modifier.clickable { state.eventSink(MessageComposerEvents.PickAttachmentSource.FromGallery) }, icon = { Icon(Icons.Default.Collections, null) }, text = { Text(stringResource(R.string.screen_room_attachment_source_gallery)) }, ) ListItem( - modifier = Modifier.clickable { eventSink(MessageComposerEvents.PickAttachmentSource.FromFiles) }, + modifier = Modifier.clickable { state.eventSink(MessageComposerEvents.PickAttachmentSource.FromFiles) }, icon = { Icon(Icons.Default.AttachFile, null) }, text = { Text(stringResource(R.string.screen_room_attachment_source_files)) }, ) ListItem( - modifier = Modifier.clickable { eventSink(MessageComposerEvents.PickAttachmentSource.PhotoFromCamera) }, + modifier = Modifier.clickable { state.eventSink(MessageComposerEvents.PickAttachmentSource.PhotoFromCamera) }, icon = { Icon(Icons.Default.PhotoCamera, null) }, text = { Text(stringResource(R.string.screen_room_attachment_source_camera_photo)) }, ) ListItem( - modifier = Modifier.clickable { eventSink(MessageComposerEvents.PickAttachmentSource.VideoFromCamera) }, + modifier = Modifier.clickable { state.eventSink(MessageComposerEvents.PickAttachmentSource.VideoFromCamera) }, icon = { Icon(Icons.Default.Videocam, null) }, text = { Text(stringResource(R.string.screen_room_attachment_source_camera_video)) }, ) - ListItem( - modifier = Modifier.clickable { - eventSink(MessageComposerEvents.PickAttachmentSource.Location) - onSendLocationClicked() - }, - icon = { Icon(Icons.Default.LocationOn, null) }, - text = { Text(stringResource(R.string.screen_room_attachment_source_location)) }, - ) + if (state.canShareLocation) { + ListItem( + modifier = Modifier.clickable { + state.eventSink(MessageComposerEvents.PickAttachmentSource.Location) + onSendLocationClicked() + }, + icon = { Icon(Icons.Default.LocationOn, null) }, + text = { Text(stringResource(R.string.screen_room_attachment_source_location)) }, + ) + } } } @@ -136,7 +138,9 @@ internal fun AttachmentSourcePickerMenu( @Composable internal fun AttachmentSourcePickerMenuPreview() = ElementPreview { AttachmentSourcePickerMenu( - eventSink = {}, + state = aMessageComposerState( + canShareLocation = true, + ), onSendLocationClicked = {}, ) } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt index 020236e890..49f66c140c 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt @@ -74,6 +74,11 @@ class MessageComposerPresenter @Inject constructor( mutableStateOf(AttachmentsState.None) } + var canShareLocation = false + LaunchedEffect(Unit) { + canShareLocation = featureFlagService.isFeatureEnabled(FeatureFlags.LocationSharing) + } + val galleryMediaPicker = mediaPickerProvider.registerGalleryPicker { uri, mimeType -> handlePickedMedia(attachmentsState, uri, mimeType) } @@ -140,23 +145,23 @@ class MessageComposerPresenter @Inject constructor( ) ) } - MessageComposerEvents.AddAttachment -> localCoroutineScope.launchIfMediaPickerEnabled { + MessageComposerEvents.AddAttachment -> localCoroutineScope.launch { showAttachmentSourcePicker = true } MessageComposerEvents.DismissAttachmentMenu -> showAttachmentSourcePicker = false - MessageComposerEvents.PickAttachmentSource.FromGallery -> localCoroutineScope.launchIfMediaPickerEnabled { + MessageComposerEvents.PickAttachmentSource.FromGallery -> localCoroutineScope.launch { showAttachmentSourcePicker = false galleryMediaPicker.launch() } - MessageComposerEvents.PickAttachmentSource.FromFiles -> localCoroutineScope.launchIfMediaPickerEnabled { + MessageComposerEvents.PickAttachmentSource.FromFiles -> localCoroutineScope.launch { showAttachmentSourcePicker = false filesPicker.launch() } - MessageComposerEvents.PickAttachmentSource.PhotoFromCamera -> localCoroutineScope.launchIfMediaPickerEnabled { + MessageComposerEvents.PickAttachmentSource.PhotoFromCamera -> localCoroutineScope.launch { showAttachmentSourcePicker = false cameraPhotoPicker.launch() } - MessageComposerEvents.PickAttachmentSource.VideoFromCamera -> localCoroutineScope.launchIfMediaPickerEnabled { + MessageComposerEvents.PickAttachmentSource.VideoFromCamera -> localCoroutineScope.launch { showAttachmentSourcePicker = false cameraVideoPicker.launch() } @@ -173,17 +178,12 @@ class MessageComposerPresenter @Inject constructor( hasFocus = hasFocus.value, mode = messageComposerContext.composerMode, showAttachmentSourcePicker = showAttachmentSourcePicker, + canShareLocation = canShareLocation, attachmentsState = attachmentsState.value, eventSink = ::handleEvents ) } - private fun CoroutineScope.launchIfMediaPickerEnabled(action: suspend () -> Unit) = launch { - if (featureFlagService.isFeatureEnabled(FeatureFlags.ShowMediaUploadingFlow)) { - action() - } - } - private fun CoroutineScope.sendMessage( text: String, updateComposerMode: (newComposerMode: MessageComposerMode) -> Unit, diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerState.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerState.kt index 28ec14ffeb..32faaf9d81 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerState.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerState.kt @@ -28,6 +28,7 @@ data class MessageComposerState( val hasFocus: Boolean, val mode: MessageComposerMode, val showAttachmentSourcePicker: Boolean, + val canShareLocation: Boolean, val attachmentsState: AttachmentsState, val eventSink: (MessageComposerEvents) -> Unit ) { diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerStateProvider.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerStateProvider.kt index 1934154824..a1fbb7ffa0 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerStateProvider.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerStateProvider.kt @@ -26,12 +26,21 @@ open class MessageComposerStateProvider : PreviewParameterProvider false + FeatureFlags.LocationSharing -> false } } else { false diff --git a/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeatureFlagServiceTest.kt b/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeatureFlagServiceTest.kt index 0351b20716..ea9b03acdf 100644 --- a/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeatureFlagServiceTest.kt +++ b/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeatureFlagServiceTest.kt @@ -26,14 +26,14 @@ class DefaultFeatureFlagServiceTest { @Test fun `given service without provider when feature is checked then it returns the default value`() = runTest { val featureFlagService = DefaultFeatureFlagService(emptySet()) - val isFeatureEnabled = featureFlagService.isFeatureEnabled(FeatureFlags.ShowMediaUploadingFlow) - assertThat(isFeatureEnabled).isEqualTo(FeatureFlags.ShowMediaUploadingFlow.defaultValue) + val isFeatureEnabled = featureFlagService.isFeatureEnabled(FeatureFlags.LocationSharing) + assertThat(isFeatureEnabled).isEqualTo(FeatureFlags.LocationSharing.defaultValue) } @Test fun `given service without provider when set enabled feature is called then it returns false`() = runTest { val featureFlagService = DefaultFeatureFlagService(emptySet()) - val result = featureFlagService.setFeatureEnabled(FeatureFlags.ShowMediaUploadingFlow, true) + val result = featureFlagService.setFeatureEnabled(FeatureFlags.LocationSharing, true) assertThat(result).isEqualTo(false) } @@ -41,7 +41,7 @@ class DefaultFeatureFlagServiceTest { fun `given service with a runtime provider when set enabled feature is called then it returns true`() = runTest { val featureFlagProvider = FakeRuntimeFeatureFlagProvider(0) val featureFlagService = DefaultFeatureFlagService(setOf(featureFlagProvider)) - val result = featureFlagService.setFeatureEnabled(FeatureFlags.ShowMediaUploadingFlow, true) + val result = featureFlagService.setFeatureEnabled(FeatureFlags.LocationSharing, true) assertThat(result).isEqualTo(true) } @@ -49,10 +49,10 @@ class DefaultFeatureFlagServiceTest { fun `given service with a runtime provider and feature enabled when feature is checked then it returns the correct value`() = runTest { val featureFlagProvider = FakeRuntimeFeatureFlagProvider(0) val featureFlagService = DefaultFeatureFlagService(setOf(featureFlagProvider)) - featureFlagService.setFeatureEnabled(FeatureFlags.ShowMediaUploadingFlow, true) - assertThat(featureFlagService.isFeatureEnabled(FeatureFlags.ShowMediaUploadingFlow)).isEqualTo(true) - featureFlagService.setFeatureEnabled(FeatureFlags.ShowMediaUploadingFlow, false) - assertThat(featureFlagService.isFeatureEnabled(FeatureFlags.ShowMediaUploadingFlow)).isEqualTo(false) + featureFlagService.setFeatureEnabled(FeatureFlags.LocationSharing, true) + assertThat(featureFlagService.isFeatureEnabled(FeatureFlags.LocationSharing)).isEqualTo(true) + featureFlagService.setFeatureEnabled(FeatureFlags.LocationSharing, false) + assertThat(featureFlagService.isFeatureEnabled(FeatureFlags.LocationSharing)).isEqualTo(false) } @Test @@ -60,8 +60,8 @@ class DefaultFeatureFlagServiceTest { val lowPriorityfeatureFlagProvider = FakeRuntimeFeatureFlagProvider(LOW_PRIORITY) val highPriorityfeatureFlagProvider = FakeRuntimeFeatureFlagProvider(HIGH_PRIORITY) val featureFlagService = DefaultFeatureFlagService(setOf(lowPriorityfeatureFlagProvider, highPriorityfeatureFlagProvider)) - lowPriorityfeatureFlagProvider.setFeatureEnabled(FeatureFlags.ShowMediaUploadingFlow, false) - highPriorityfeatureFlagProvider.setFeatureEnabled(FeatureFlags.ShowMediaUploadingFlow, true) - assertThat(featureFlagService.isFeatureEnabled(FeatureFlags.ShowMediaUploadingFlow)).isEqualTo(true) + lowPriorityfeatureFlagProvider.setFeatureEnabled(FeatureFlags.LocationSharing, false) + highPriorityfeatureFlagProvider.setFeatureEnabled(FeatureFlags.LocationSharing, true) + assertThat(featureFlagService.isFeatureEnabled(FeatureFlags.LocationSharing)).isEqualTo(true) } } From 1c90c32c4e709aba4ebf89ed73f030ca972724e4 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 20 Jul 2023 22:21:31 +0200 Subject: [PATCH 4/7] Enable LocationSharing flag for the release. --- .../libraries/featureflag/impl/BuildtimeFeatureFlagProvider.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/BuildtimeFeatureFlagProvider.kt b/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/BuildtimeFeatureFlagProvider.kt index 05534ad53a..d226885495 100644 --- a/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/BuildtimeFeatureFlagProvider.kt +++ b/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/BuildtimeFeatureFlagProvider.kt @@ -29,7 +29,7 @@ class BuildtimeFeatureFlagProvider @Inject constructor() : override suspend fun isFeatureEnabled(feature: Feature): Boolean { return if (feature is FeatureFlags) { when (feature) { - FeatureFlags.LocationSharing -> false + FeatureFlags.LocationSharing -> true } } else { false From f0d02996dfab08890156ed020c76298cc5dfd084 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 20 Jul 2023 22:23:54 +0200 Subject: [PATCH 5/7] canShareLocation must be a MutableState. --- .../impl/messagecomposer/MessageComposerPresenter.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt index 49f66c140c..ec16d3342d 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt @@ -74,9 +74,9 @@ class MessageComposerPresenter @Inject constructor( mutableStateOf(AttachmentsState.None) } - var canShareLocation = false + val canShareLocation = remember { mutableStateOf(false) } LaunchedEffect(Unit) { - canShareLocation = featureFlagService.isFeatureEnabled(FeatureFlags.LocationSharing) + canShareLocation.value = featureFlagService.isFeatureEnabled(FeatureFlags.LocationSharing) } val galleryMediaPicker = mediaPickerProvider.registerGalleryPicker { uri, mimeType -> @@ -178,7 +178,7 @@ class MessageComposerPresenter @Inject constructor( hasFocus = hasFocus.value, mode = messageComposerContext.composerMode, showAttachmentSourcePicker = showAttachmentSourcePicker, - canShareLocation = canShareLocation, + canShareLocation = canShareLocation.value, attachmentsState = attachmentsState.value, eventSink = ::handleEvents ) From a41fffbe4c6fb171487bf8d823387a6a11486dce Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 20 Jul 2023 23:17:29 +0200 Subject: [PATCH 6/7] Must skip 1 item due to the location feature flag value emitting 1 item. --- .../messages/MessagesPresenterTest.kt | 60 ++++++++----------- .../MessageComposerPresenterTest.kt | 19 ++++++ 2 files changed, 43 insertions(+), 36 deletions(-) diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/MessagesPresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/MessagesPresenterTest.kt index 95848d835f..e0af846656 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/MessagesPresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/MessagesPresenterTest.kt @@ -84,7 +84,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - + skipItems(1) val initialState = awaitItem() assertThat(initialState.roomId).isEqualTo(A_ROOM_ID) } @@ -98,7 +98,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - + skipItems(1) val initialState = awaitItem() initialState.eventSink.invoke(MessagesEvents.ToggleReaction("👍", AN_EVENT_ID)) assertThat(room.myReactions.count()).isEqualTo(1) @@ -119,7 +119,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - + skipItems(1) val initialState = awaitItem() initialState.eventSink.invoke(MessagesEvents.ToggleReaction("👍", AN_EVENT_ID)) assertThat(room.myReactions.count()).isEqualTo(1) @@ -136,7 +136,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - + skipItems(1) val initialState = awaitItem() initialState.eventSink.invoke(MessagesEvents.HandleAction(TimelineItemAction.Forward, aMessageEvent())) assertThat(awaitItem().actionListState.target).isEqualTo(ActionListState.Target.None) @@ -152,7 +152,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - + skipItems(1) val initialState = awaitItem() initialState.eventSink.invoke(MessagesEvents.HandleAction(TimelineItemAction.Copy, event)) assertThat(awaitItem().actionListState.target).isEqualTo(ActionListState.Target.None) @@ -166,10 +166,9 @@ class MessagesPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - + skipItems(1) val initialState = awaitItem() initialState.eventSink.invoke(MessagesEvents.HandleAction(TimelineItemAction.Reply, aMessageEvent())) - val finalState = awaitItem() assertThat(finalState.composerState.mode).isInstanceOf(MessageComposerMode.Reply::class.java) assertThat(awaitItem().actionListState.target).isEqualTo(ActionListState.Target.None) @@ -182,7 +181,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - + skipItems(1) val initialState = awaitItem() initialState.eventSink.invoke(MessagesEvents.HandleAction(TimelineItemAction.Reply, aMessageEvent(eventId = null))) assertThat(awaitItem().actionListState.target).isEqualTo(ActionListState.Target.None) @@ -197,7 +196,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - + skipItems(1) val initialState = awaitItem() val mediaMessage = aMessageEvent( content = TimelineItemImageContent( @@ -214,7 +213,6 @@ class MessagesPresenterTest { ) ) initialState.eventSink.invoke(MessagesEvents.HandleAction(TimelineItemAction.Reply, mediaMessage)) - val finalState = awaitItem() assertThat(finalState.composerState.mode).isInstanceOf(MessageComposerMode.Reply::class.java) val replyMode = finalState.composerState.mode as MessageComposerMode.Reply @@ -229,7 +227,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - + skipItems(1) val initialState = awaitItem() val mediaMessage = aMessageEvent( content = TimelineItemVideoContent( @@ -262,7 +260,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - + skipItems(1) val initialState = awaitItem() val mediaMessage = aMessageEvent( content = TimelineItemFileContent( @@ -275,7 +273,6 @@ class MessagesPresenterTest { ) ) initialState.eventSink.invoke(MessagesEvents.HandleAction(TimelineItemAction.Reply, mediaMessage)) - val finalState = awaitItem() assertThat(finalState.composerState.mode).isInstanceOf(MessageComposerMode.Reply::class.java) val replyMode = finalState.composerState.mode as MessageComposerMode.Reply @@ -290,10 +287,9 @@ class MessagesPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - + skipItems(1) val initialState = awaitItem() initialState.eventSink.invoke(MessagesEvents.HandleAction(TimelineItemAction.Edit, aMessageEvent())) - val finalState = awaitItem() assertThat(finalState.composerState.mode).isInstanceOf(MessageComposerMode.Edit::class.java) assertThat(awaitItem().actionListState.target).isEqualTo(ActionListState.Target.None) @@ -308,7 +304,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - + skipItems(1) val initialState = awaitItem() initialState.eventSink.invoke(MessagesEvents.HandleAction(TimelineItemAction.Redact, aMessageEvent())) assertThat(matrixRoom.redactEventEventIdParam).isEqualTo(AN_EVENT_ID) @@ -323,7 +319,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - + skipItems(1) val initialState = awaitItem() initialState.eventSink.invoke(MessagesEvents.HandleAction(TimelineItemAction.ReportContent, aMessageEvent())) assertThat(awaitItem().actionListState.target).isEqualTo(ActionListState.Target.None) @@ -337,7 +333,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - + skipItems(1) val initialState = awaitItem() initialState.eventSink.invoke(MessagesEvents.Dismiss) assertThat(awaitItem().actionListState.target).isEqualTo(ActionListState.Target.None) @@ -351,7 +347,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - + skipItems(1) val initialState = awaitItem() initialState.eventSink.invoke(MessagesEvents.HandleAction(TimelineItemAction.Developer, aMessageEvent())) assertThat(awaitItem().actionListState.target).isEqualTo(ActionListState.Target.None) @@ -366,17 +362,14 @@ class MessagesPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - + skipItems(1) val initialState = awaitItem() - // Initially the composer doesn't have focus, so we don't show the alert assertThat(initialState.showReinvitePrompt).isFalse() - // When the input field is focused we show the alert initialState.composerState.eventSink(MessageComposerEvents.FocusChanged(true)) val focusedState = awaitItem() assertThat(focusedState.showReinvitePrompt).isTrue() - // If it's dismissed then we stop showing the alert initialState.eventSink(MessagesEvents.InviteDialogDismissed(InviteDialogAction.Cancel)) val dismissedState = awaitItem() @@ -391,10 +384,9 @@ class MessagesPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - + skipItems(1) val initialState = awaitItem() assertThat(initialState.showReinvitePrompt).isFalse() - initialState.composerState.eventSink(MessageComposerEvents.FocusChanged(true)) val focusedState = awaitItem() assertThat(focusedState.showReinvitePrompt).isFalse() @@ -408,10 +400,9 @@ class MessagesPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - + skipItems(1) val initialState = awaitItem() assertThat(initialState.showReinvitePrompt).isFalse() - initialState.composerState.eventSink(MessageComposerEvents.FocusChanged(true)) val focusedState = awaitItem() assertThat(focusedState.showReinvitePrompt).isFalse() @@ -433,14 +424,13 @@ class MessagesPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { + skipItems(1) val initialState = awaitItem() skipItems(1) initialState.eventSink(MessagesEvents.InviteDialogDismissed(InviteDialogAction.Invite)) - skipItems(1) val loadingState = awaitItem() assertThat(loadingState.inviteProgress.isLoading()).isTrue() - val newState = awaitItem() assertThat(newState.inviteProgress.isSuccess()).isTrue() assertThat(room.invitedUserId).isEqualTo(A_SESSION_ID_2) @@ -463,14 +453,13 @@ class MessagesPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { + skipItems(1) val initialState = awaitItem() skipItems(1) initialState.eventSink(MessagesEvents.InviteDialogDismissed(InviteDialogAction.Invite)) - skipItems(1) val loadingState = awaitItem() assertThat(loadingState.inviteProgress.isLoading()).isTrue() - val newState = awaitItem() assertThat(newState.inviteProgress.isSuccess()).isTrue() assertThat(room.invitedUserId).isEqualTo(A_SESSION_ID_2) @@ -485,14 +474,13 @@ class MessagesPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { + skipItems(1) val initialState = awaitItem() skipItems(1) initialState.eventSink(MessagesEvents.InviteDialogDismissed(InviteDialogAction.Invite)) - skipItems(1) val loadingState = awaitItem() assertThat(loadingState.inviteProgress.isLoading()).isTrue() - val newState = awaitItem() assertThat(newState.inviteProgress.isFailure()).isTrue() } @@ -514,13 +502,13 @@ class MessagesPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { + skipItems(1) val initialState = awaitItem() skipItems(1) initialState.eventSink(MessagesEvents.InviteDialogDismissed(InviteDialogAction.Invite)) skipItems(1) val loadingState = awaitItem() assertThat(loadingState.inviteProgress.isLoading()).isTrue() - val newState = awaitItem() assertThat(newState.inviteProgress.isFailure()).isTrue() } @@ -534,7 +522,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - + skipItems(1) assertThat(awaitItem().userHasPermissionToSendMessage).isTrue() } } @@ -549,7 +537,7 @@ class MessagesPresenterTest { }.test { // Default value assertThat(awaitItem().userHasPermissionToSendMessage).isTrue() - skipItems(1) + skipItems(2) assertThat(awaitItem().userHasPermissionToSendMessage).isFalse() } } diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/textcomposer/MessageComposerPresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/textcomposer/MessageComposerPresenterTest.kt index 92daee6dfb..f0871f035a 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/textcomposer/MessageComposerPresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/textcomposer/MessageComposerPresenterTest.kt @@ -81,6 +81,7 @@ class MessageComposerPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { + skipItems(1) val initialState = awaitItem() assertThat(initialState.isFullScreen).isFalse() assertThat(initialState.text).isEqualTo("") @@ -97,6 +98,7 @@ class MessageComposerPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { + skipItems(1) val initialState = awaitItem() initialState.eventSink.invoke(MessageComposerEvents.ToggleFullScreenState) val fullscreenState = awaitItem() @@ -113,6 +115,7 @@ class MessageComposerPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { + skipItems(1) val initialState = awaitItem() initialState.eventSink.invoke(MessageComposerEvents.UpdateText(A_MESSAGE)) val withMessageState = awaitItem() @@ -131,6 +134,7 @@ class MessageComposerPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { + skipItems(1) var state = awaitItem() val mode = anEditMode() state.eventSink.invoke(MessageComposerEvents.SetMode(mode)) @@ -149,6 +153,7 @@ class MessageComposerPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { + skipItems(1) var state = awaitItem() val mode = aReplyMode() state.eventSink.invoke(MessageComposerEvents.SetMode(mode)) @@ -166,6 +171,7 @@ class MessageComposerPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { + skipItems(1) var state = awaitItem() val mode = aQuoteMode() state.eventSink.invoke(MessageComposerEvents.SetMode(mode)) @@ -183,6 +189,7 @@ class MessageComposerPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { + skipItems(1) val initialState = awaitItem() initialState.eventSink.invoke(MessageComposerEvents.UpdateText(A_MESSAGE)) val withMessageState = awaitItem() @@ -205,6 +212,7 @@ class MessageComposerPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { + skipItems(1) val initialState = awaitItem() assertThat(initialState.text).isEqualTo("") val mode = anEditMode() @@ -236,6 +244,7 @@ class MessageComposerPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { + skipItems(1) val initialState = awaitItem() assertThat(initialState.text).isEqualTo("") val mode = anEditMode(eventId = null, transactionId = A_TRANSACTION_ID) @@ -267,6 +276,7 @@ class MessageComposerPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { + skipItems(1) val initialState = awaitItem() assertThat(initialState.text).isEqualTo("") val mode = aReplyMode() @@ -294,6 +304,7 @@ class MessageComposerPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { + skipItems(1) val initialState = awaitItem() assertThat(initialState.showAttachmentSourcePicker).isEqualTo(false) initialState.eventSink(MessageComposerEvents.AddAttachment) @@ -307,6 +318,7 @@ class MessageComposerPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { + skipItems(1) val initialState = awaitItem() initialState.eventSink(MessageComposerEvents.AddAttachment) skipItems(1) @@ -341,6 +353,7 @@ class MessageComposerPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { + skipItems(1) val initialState = awaitItem() initialState.eventSink(MessageComposerEvents.PickAttachmentSource.FromGallery) val previewingState = awaitItem() @@ -375,6 +388,7 @@ class MessageComposerPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { + skipItems(1) val initialState = awaitItem() initialState.eventSink(MessageComposerEvents.PickAttachmentSource.FromGallery) val previewingState = awaitItem() @@ -393,6 +407,7 @@ class MessageComposerPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { + skipItems(1) val initialState = awaitItem() initialState.eventSink(MessageComposerEvents.PickAttachmentSource.FromGallery) // No crashes here, otherwise it fails @@ -413,6 +428,7 @@ class MessageComposerPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { + skipItems(1) val initialState = awaitItem() initialState.eventSink(MessageComposerEvents.PickAttachmentSource.FromFiles) val sendingState = awaitItem() @@ -434,6 +450,7 @@ class MessageComposerPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { + skipItems(1) val initialState = awaitItem() initialState.eventSink(MessageComposerEvents.PickAttachmentSource.PhotoFromCamera) val previewingState = awaitItem() @@ -450,6 +467,7 @@ class MessageComposerPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { + skipItems(1) val initialState = awaitItem() initialState.eventSink(MessageComposerEvents.PickAttachmentSource.VideoFromCamera) val previewingState = awaitItem() @@ -467,6 +485,7 @@ class MessageComposerPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { + skipItems(1) val initialState = awaitItem() initialState.eventSink(MessageComposerEvents.PickAttachmentSource.FromFiles) val sendingState = awaitItem() From e03408a806e1d0ade0391eecbcdca09a72a3afad Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 20 Jul 2023 23:18:18 +0200 Subject: [PATCH 7/7] Test new field `canShareLocation` --- .../messages/textcomposer/MessageComposerPresenterTest.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/textcomposer/MessageComposerPresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/textcomposer/MessageComposerPresenterTest.kt index f0871f035a..5c75c963e5 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/textcomposer/MessageComposerPresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/textcomposer/MessageComposerPresenterTest.kt @@ -87,6 +87,7 @@ class MessageComposerPresenterTest { assertThat(initialState.text).isEqualTo("") assertThat(initialState.mode).isEqualTo(MessageComposerMode.Normal("")) assertThat(initialState.showAttachmentSourcePicker).isFalse() + assertThat(initialState.canShareLocation).isTrue() assertThat(initialState.attachmentsState).isEqualTo(AttachmentsState.None) assertThat(initialState.isSendButtonVisible).isFalse() }