Add feature flag to temporary disable sending caption by default in production.

This commit is contained in:
Benoit Marty 2024-11-27 10:54:20 +01:00
parent 8d26af5b26
commit 9d6ea2175f
7 changed files with 112 additions and 11 deletions

View file

@ -37,6 +37,8 @@ import io.element.android.features.messages.impl.timeline.model.event.canBeForwa
import io.element.android.features.messages.impl.timeline.model.event.canReact import io.element.android.features.messages.impl.timeline.model.event.canReact
import io.element.android.libraries.architecture.Presenter import io.element.android.libraries.architecture.Presenter
import io.element.android.libraries.di.RoomScope import io.element.android.libraries.di.RoomScope
import io.element.android.libraries.featureflag.api.FeatureFlagService
import io.element.android.libraries.featureflag.api.FeatureFlags
import io.element.android.libraries.matrix.api.core.EventId import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.MatrixRoom
import io.element.android.libraries.preferences.api.store.AppPreferencesStore import io.element.android.libraries.preferences.api.store.AppPreferencesStore
@ -60,6 +62,7 @@ class DefaultActionListPresenter @AssistedInject constructor(
private val isPinnedMessagesFeatureEnabled: IsPinnedMessagesFeatureEnabled, private val isPinnedMessagesFeatureEnabled: IsPinnedMessagesFeatureEnabled,
private val room: MatrixRoom, private val room: MatrixRoom,
private val userSendFailureFactory: VerifiedUserSendFailureFactory, private val userSendFailureFactory: VerifiedUserSendFailureFactory,
private val featureFlagService: FeatureFlagService,
) : ActionListPresenter { ) : ActionListPresenter {
@AssistedFactory @AssistedFactory
@ContributesBinding(RoomScope::class) @ContributesBinding(RoomScope::class)
@ -134,7 +137,7 @@ class DefaultActionListPresenter @AssistedInject constructor(
} }
} }
private fun buildActions( private suspend fun buildActions(
timelineItem: TimelineItem.Event, timelineItem: TimelineItem.Event,
usersEventPermissions: UserEventPermissions, usersEventPermissions: UserEventPermissions,
isDeveloperModeEnabled: Boolean, isDeveloperModeEnabled: Boolean,
@ -157,7 +160,9 @@ class DefaultActionListPresenter @AssistedInject constructor(
if (timelineItem.content is TimelineItemEventContentWithAttachment) { if (timelineItem.content is TimelineItemEventContentWithAttachment) {
// Caption // Caption
if (timelineItem.content.caption == null) { if (timelineItem.content.caption == null) {
add(TimelineItemAction.AddCaption) if (featureFlagService.isFeatureEnabled(FeatureFlags.MediaCaptionCreation)) {
add(TimelineItemAction.AddCaption)
}
} else { } else {
add(TimelineItemAction.EditCaption) add(TimelineItemAction.EditCaption)
add(TimelineItemAction.RemoveCaption) add(TimelineItemAction.RemoveCaption)

View file

@ -10,6 +10,7 @@ package io.element.android.features.messages.impl.attachments.preview
import androidx.compose.runtime.Composable import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.MutableState import androidx.compose.runtime.MutableState
import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.getValue import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember import androidx.compose.runtime.remember
@ -46,7 +47,7 @@ class AttachmentsPreviewPresenter @AssistedInject constructor(
private val mediaSender: MediaSender, private val mediaSender: MediaSender,
private val permalinkBuilder: PermalinkBuilder, private val permalinkBuilder: PermalinkBuilder,
private val temporaryUriDeleter: TemporaryUriDeleter, private val temporaryUriDeleter: TemporaryUriDeleter,
private val featureFlagsService: FeatureFlagService, private val featureFlagService: FeatureFlagService,
) : Presenter<AttachmentsPreviewState> { ) : Presenter<AttachmentsPreviewState> {
@AssistedFactory @AssistedFactory
interface Factory { interface Factory {
@ -72,6 +73,7 @@ class AttachmentsPreviewPresenter @AssistedInject constructor(
val ongoingSendAttachmentJob = remember { mutableStateOf<Job?>(null) } val ongoingSendAttachmentJob = remember { mutableStateOf<Job?>(null) }
val userSentAttachment = remember { mutableStateOf(false) } val userSentAttachment = remember { mutableStateOf(false) }
val allowCaption by featureFlagService.isFeatureEnabledFlow(FeatureFlags.MediaCaptionCreation).collectAsState(initial = false)
val mediaUploadInfoState = remember { mutableStateOf<AsyncData<MediaUploadInfo>>(AsyncData.Uninitialized) } val mediaUploadInfoState = remember { mutableStateOf<AsyncData<MediaUploadInfo>>(AsyncData.Uninitialized) }
LaunchedEffect(Unit) { LaunchedEffect(Unit) {
@ -112,7 +114,7 @@ class AttachmentsPreviewPresenter @AssistedInject constructor(
fun handleEvents(attachmentsPreviewEvents: AttachmentsPreviewEvents) { fun handleEvents(attachmentsPreviewEvents: AttachmentsPreviewEvents) {
when (attachmentsPreviewEvents) { when (attachmentsPreviewEvents) {
is AttachmentsPreviewEvents.SendAttachment -> coroutineScope.launch { is AttachmentsPreviewEvents.SendAttachment -> coroutineScope.launch {
val useSendQueue = featureFlagsService.isFeatureEnabled(FeatureFlags.MediaUploadOnSendQueue) val useSendQueue = featureFlagService.isFeatureEnabled(FeatureFlags.MediaUploadOnSendQueue)
userSentAttachment.value = true userSentAttachment.value = true
val instantSending = mediaUploadInfoState.value.isReady() && useSendQueue val instantSending = mediaUploadInfoState.value.isReady() && useSendQueue
sendActionState.value = if (instantSending) { sendActionState.value = if (instantSending) {
@ -142,6 +144,7 @@ class AttachmentsPreviewPresenter @AssistedInject constructor(
attachment = attachment, attachment = attachment,
sendActionState = sendActionState.value, sendActionState = sendActionState.value,
textEditorState = textEditorState, textEditorState = textEditorState,
allowCaption = allowCaption,
eventSink = ::handleEvents eventSink = ::handleEvents
) )
} }

View file

@ -15,11 +15,9 @@ data class AttachmentsPreviewState(
val attachment: Attachment, val attachment: Attachment,
val sendActionState: SendActionState, val sendActionState: SendActionState,
val textEditorState: TextEditorState, val textEditorState: TextEditorState,
val allowCaption: Boolean,
val eventSink: (AttachmentsPreviewEvents) -> Unit val eventSink: (AttachmentsPreviewEvents) -> Unit
) { )
// Keep the val to eventually set to false for some mimetypes.
val allowCaption: Boolean = true
}
@Immutable @Immutable
sealed interface SendActionState { sealed interface SendActionState {

View file

@ -29,6 +29,7 @@ open class AttachmentsPreviewStateProvider : PreviewParameterProvider<Attachment
anAttachmentsPreviewState(sendActionState = SendActionState.Sending.Processing), anAttachmentsPreviewState(sendActionState = SendActionState.Sending.Processing),
anAttachmentsPreviewState(sendActionState = SendActionState.Sending.Uploading(0.5f)), anAttachmentsPreviewState(sendActionState = SendActionState.Sending.Uploading(0.5f)),
anAttachmentsPreviewState(sendActionState = SendActionState.Failure(RuntimeException("error"))), anAttachmentsPreviewState(sendActionState = SendActionState.Failure(RuntimeException("error"))),
anAttachmentsPreviewState(allowCaption = false),
) )
} }
@ -36,11 +37,13 @@ fun anAttachmentsPreviewState(
mediaInfo: MediaInfo = anImageMediaInfo(), mediaInfo: MediaInfo = anImageMediaInfo(),
textEditorState: TextEditorState = aTextEditorStateMarkdown(), textEditorState: TextEditorState = aTextEditorStateMarkdown(),
sendActionState: SendActionState = SendActionState.Idle, sendActionState: SendActionState = SendActionState.Idle,
allowCaption: Boolean = true,
) = AttachmentsPreviewState( ) = AttachmentsPreviewState(
attachment = Attachment.Media( attachment = Attachment.Media(
localMedia = LocalMedia("file://path".toUri(), mediaInfo), localMedia = LocalMedia("file://path".toUri(), mediaInfo),
), ),
sendActionState = sendActionState, sendActionState = sendActionState,
textEditorState = textEditorState, textEditorState = textEditorState,
allowCaption = allowCaption,
eventSink = {} eventSink = {}
) )

View file

@ -26,6 +26,8 @@ import io.element.android.features.messages.impl.timeline.model.event.aTimelineI
import io.element.android.features.messages.impl.timeline.model.event.aTimelineItemStateEventContent import io.element.android.features.messages.impl.timeline.model.event.aTimelineItemStateEventContent
import io.element.android.features.messages.impl.timeline.model.event.aTimelineItemVoiceContent import io.element.android.features.messages.impl.timeline.model.event.aTimelineItemVoiceContent
import io.element.android.features.poll.api.pollcontent.aPollAnswerItemList import io.element.android.features.poll.api.pollcontent.aPollAnswerItemList
import io.element.android.libraries.featureflag.api.FeatureFlags
import io.element.android.libraries.featureflag.test.FakeFeatureFlagService
import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.MatrixRoom
import io.element.android.libraries.matrix.api.timeline.item.event.LocalEventSendState import io.element.android.libraries.matrix.api.timeline.item.event.LocalEventSendState
import io.element.android.libraries.matrix.test.AN_EVENT_ID import io.element.android.libraries.matrix.test.AN_EVENT_ID
@ -558,6 +560,57 @@ class ActionListPresenterTest {
} }
} }
@Test
fun `present - compute for a media item - caption disabled`() = runTest {
val presenter = createActionListPresenter(
isDeveloperModeEnabled = true,
isPinFeatureEnabled = true,
allowCaption = false,
)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val initialState = awaitItem()
val messageEvent = aMessageEvent(
isMine = true,
isEditable = true,
content = aTimelineItemImageContent(),
)
initialState.eventSink.invoke(
ActionListEvents.ComputeForMessage(
event = messageEvent,
userEventPermissions = aUserEventPermissions(
canRedactOwn = true,
canRedactOther = false,
canSendMessage = true,
canSendReaction = true,
canPinUnpin = true,
),
)
)
val successState = awaitItem()
assertThat(successState.target).isEqualTo(
ActionListState.Target.Success(
event = messageEvent,
displayEmojiReactions = true,
verifiedUserSendFailure = VerifiedUserSendFailure.None,
actions = persistentListOf(
TimelineItemAction.Reply,
TimelineItemAction.Forward,
// Not here
// TimelineItemAction.AddCaption,
TimelineItemAction.Pin,
TimelineItemAction.CopyLink,
TimelineItemAction.ViewSource,
TimelineItemAction.Redact,
)
)
)
initialState.eventSink.invoke(ActionListEvents.Clear)
assertThat(awaitItem().target).isEqualTo(ActionListState.Target.None)
}
}
@Test @Test
fun `present - compute for a media with caption item`() = runTest { fun `present - compute for a media with caption item`() = runTest {
val presenter = createActionListPresenter(isDeveloperModeEnabled = true, isPinFeatureEnabled = true) val presenter = createActionListPresenter(isDeveloperModeEnabled = true, isPinFeatureEnabled = true)
@ -1151,6 +1204,7 @@ private fun createActionListPresenter(
isDeveloperModeEnabled: Boolean, isDeveloperModeEnabled: Boolean,
isPinFeatureEnabled: Boolean, isPinFeatureEnabled: Boolean,
room: MatrixRoom = FakeMatrixRoom(), room: MatrixRoom = FakeMatrixRoom(),
allowCaption: Boolean = true,
): ActionListPresenter { ): ActionListPresenter {
val preferencesStore = InMemoryAppPreferencesStore(isDeveloperModeEnabled = isDeveloperModeEnabled) val preferencesStore = InMemoryAppPreferencesStore(isDeveloperModeEnabled = isDeveloperModeEnabled)
return DefaultActionListPresenter( return DefaultActionListPresenter(
@ -1158,6 +1212,11 @@ private fun createActionListPresenter(
appPreferencesStore = preferencesStore, appPreferencesStore = preferencesStore,
isPinnedMessagesFeatureEnabled = { isPinFeatureEnabled }, isPinnedMessagesFeatureEnabled = { isPinFeatureEnabled },
room = room, room = room,
userSendFailureFactory = VerifiedUserSendFailureFactory(room) userSendFailureFactory = VerifiedUserSendFailureFactory(room),
featureFlagService = FakeFeatureFlagService(
initialState = mapOf(
FeatureFlags.MediaCaptionCreation.key to allowCaption,
),
)
) )
} }

View file

@ -64,6 +64,28 @@ class AttachmentsPreviewPresenterTest {
private val mockMediaUrl: Uri = mockk("localMediaUri") private val mockMediaUrl: Uri = mockk("localMediaUri")
@Test
fun `present - initial state`() = runTest {
createAttachmentsPreviewPresenter().test {
skipItems(1)
val initialState = awaitItem()
assertThat(initialState.sendActionState).isEqualTo(SendActionState.Idle)
assertThat(initialState.allowCaption).isTrue()
}
}
@Test
fun `present - initial state - caption not allowed`() = runTest {
createAttachmentsPreviewPresenter(
allowCaption = false,
).test {
skipItems(1)
val initialState = awaitItem()
assertThat(initialState.sendActionState).isEqualTo(SendActionState.Idle)
assertThat(initialState.allowCaption).isFalse()
}
}
@Test @Test
fun `present - send media success scenario`() = runTest { fun `present - send media success scenario`() = runTest {
val sendFileResult = lambdaRecorder<File, FileInfo, String?, String?, ProgressCallback?, Result<FakeMediaUploadHandler>> { _, _, _, _, _ -> val sendFileResult = lambdaRecorder<File, FileInfo, String?, String?, ProgressCallback?, Result<FakeMediaUploadHandler>> { _, _, _, _, _ ->
@ -420,6 +442,7 @@ class AttachmentsPreviewPresenterTest {
temporaryUriDeleter: TemporaryUriDeleter = FakeTemporaryUriDeleter(), temporaryUriDeleter: TemporaryUriDeleter = FakeTemporaryUriDeleter(),
onDoneListener: OnDoneListener = OnDoneListener { lambdaError() }, onDoneListener: OnDoneListener = OnDoneListener { lambdaError() },
mediaUploadOnSendQueueEnabled: Boolean = true, mediaUploadOnSendQueueEnabled: Boolean = true,
allowCaption: Boolean = true,
): AttachmentsPreviewPresenter { ): AttachmentsPreviewPresenter {
return AttachmentsPreviewPresenter( return AttachmentsPreviewPresenter(
attachment = aMediaAttachment(localMedia), attachment = aMediaAttachment(localMedia),
@ -427,8 +450,11 @@ class AttachmentsPreviewPresenterTest {
mediaSender = MediaSender(mediaPreProcessor, room, InMemorySessionPreferencesStore()), mediaSender = MediaSender(mediaPreProcessor, room, InMemorySessionPreferencesStore()),
permalinkBuilder = permalinkBuilder, permalinkBuilder = permalinkBuilder,
temporaryUriDeleter = temporaryUriDeleter, temporaryUriDeleter = temporaryUriDeleter,
featureFlagsService = FakeFeatureFlagService( featureFlagService = FakeFeatureFlagService(
initialState = mapOf(FeatureFlags.MediaUploadOnSendQueue.key to mediaUploadOnSendQueueEnabled), initialState = mapOf(
FeatureFlags.MediaUploadOnSendQueue.key to mediaUploadOnSendQueueEnabled,
FeatureFlags.MediaCaptionCreation.key to allowCaption,
),
) )
) )
} }

View file

@ -140,4 +140,11 @@ enum class FeatureFlags(
defaultValue = { buildMeta -> buildMeta.buildType != BuildType.RELEASE }, defaultValue = { buildMeta -> buildMeta.buildType != BuildType.RELEASE },
isFinished = false, isFinished = false,
), ),
MediaCaptionCreation(
key = "feature.media_caption_creation",
title = "Allow creation of media captions",
description = null,
defaultValue = { buildMeta -> buildMeta.buildType != BuildType.RELEASE },
isFinished = false,
),
} }