Replace FeatureFlags.ShowMediaUploadingFlow by FeatureFlags.LocationSharing because it has more chance to be disabled.

I do not want to remove all our feature flags...
This commit is contained in:
Benoit Marty
2023-07-20 22:20:08 +02:00
parent 062e7553db
commit cd3e6c42e2
8 changed files with 64 additions and 50 deletions

View File

@@ -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 = {},
)
}

View File

@@ -74,6 +74,11 @@ class MessageComposerPresenter @Inject constructor(
mutableStateOf<AttachmentsState>(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,

View File

@@ -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
) {

View File

@@ -26,12 +26,21 @@ open class MessageComposerStateProvider : PreviewParameterProvider<MessageCompos
)
}
fun aMessageComposerState() = MessageComposerState(
text = "",
isFullScreen = false,
hasFocus = false,
mode = MessageComposerMode.Normal(content = ""),
showAttachmentSourcePicker = false,
attachmentsState = AttachmentsState.None,
eventSink = {}
fun aMessageComposerState(
text: String = "",
isFullScreen: Boolean = false,
hasFocus: Boolean = false,
mode: MessageComposerMode = MessageComposerMode.Normal(content = ""),
showAttachmentSourcePicker: Boolean = false,
canShareLocation: Boolean = true,
attachmentsState: AttachmentsState = AttachmentsState.None,
) = MessageComposerState(
text = text,
isFullScreen = isFullScreen,
hasFocus = hasFocus,
mode = mode,
showAttachmentSourcePicker = showAttachmentSourcePicker,
canShareLocation = canShareLocation,
attachmentsState = attachmentsState,
eventSink = {},
)

View File

@@ -68,7 +68,7 @@ class MessageComposerPresenterTest {
givenResult(mockk()) // Uri is not available in JVM, so the only way to have a non-null Uri is using Mockk
}
private val featureFlagService = FakeFeatureFlagService(
mapOf(FeatureFlags.ShowMediaUploadingFlow.key to true)
mapOf(FeatureFlags.LocationSharing.key to true)
)
private val mediaPreProcessor = FakeMediaPreProcessor()
private val snackbarDispatcher = SnackbarDispatcher()

View File

@@ -22,8 +22,8 @@ enum class FeatureFlags(
override val description: String? = null,
override val defaultValue: Boolean = true
) : Feature {
ShowMediaUploadingFlow(
key = "feature.showmediauploadingflow",
title = "Show media uploading flow",
LocationSharing(
key = "feature.locationsharing",
title = "Allow user to share location",
)
}

View File

@@ -29,7 +29,7 @@ class BuildtimeFeatureFlagProvider @Inject constructor() :
override suspend fun isFeatureEnabled(feature: Feature): Boolean {
return if (feature is FeatureFlags) {
when (feature) {
FeatureFlags.ShowMediaUploadingFlow -> false
FeatureFlags.LocationSharing -> false
}
} else {
false

View File

@@ -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)
}
}