Skip to content

Commit

Permalink
Merge pull request #3953 from element-hq/feature/bma/mediaCaptionFeat…
Browse files Browse the repository at this point in the history
…ureFlag

Add feature flag to temporary disable sending caption by default in production
  • Loading branch information
bmarty authored Nov 27, 2024
2 parents 6b9eb99 + ae30b9f commit 5610b19
Show file tree
Hide file tree
Showing 8 changed files with 115 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.libraries.architecture.Presenter
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.room.MatrixRoom
import io.element.android.libraries.preferences.api.store.AppPreferencesStore
Expand All @@ -60,6 +62,7 @@ class DefaultActionListPresenter @AssistedInject constructor(
private val isPinnedMessagesFeatureEnabled: IsPinnedMessagesFeatureEnabled,
private val room: MatrixRoom,
private val userSendFailureFactory: VerifiedUserSendFailureFactory,
private val featureFlagService: FeatureFlagService,
) : ActionListPresenter {
@AssistedFactory
@ContributesBinding(RoomScope::class)
Expand Down Expand Up @@ -134,7 +137,7 @@ class DefaultActionListPresenter @AssistedInject constructor(
}
}

private fun buildActions(
private suspend fun buildActions(
timelineItem: TimelineItem.Event,
usersEventPermissions: UserEventPermissions,
isDeveloperModeEnabled: Boolean,
Expand All @@ -157,7 +160,9 @@ class DefaultActionListPresenter @AssistedInject constructor(
if (timelineItem.content is TimelineItemEventContentWithAttachment) {
// Caption
if (timelineItem.content.caption == null) {
add(TimelineItemAction.AddCaption)
if (featureFlagService.isFeatureEnabled(FeatureFlags.MediaCaptionCreation)) {
add(TimelineItemAction.AddCaption)
}
} else {
add(TimelineItemAction.EditCaption)
add(TimelineItemAction.RemoveCaption)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ package io.element.android.features.messages.impl.attachments.preview
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.MutableState
import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
Expand Down Expand Up @@ -46,7 +47,7 @@ class AttachmentsPreviewPresenter @AssistedInject constructor(
private val mediaSender: MediaSender,
private val permalinkBuilder: PermalinkBuilder,
private val temporaryUriDeleter: TemporaryUriDeleter,
private val featureFlagsService: FeatureFlagService,
private val featureFlagService: FeatureFlagService,
) : Presenter<AttachmentsPreviewState> {
@AssistedFactory
interface Factory {
Expand All @@ -72,6 +73,7 @@ class AttachmentsPreviewPresenter @AssistedInject constructor(
val ongoingSendAttachmentJob = remember { mutableStateOf<Job?>(null) }

val userSentAttachment = remember { mutableStateOf(false) }
val allowCaption by featureFlagService.isFeatureEnabledFlow(FeatureFlags.MediaCaptionCreation).collectAsState(initial = false)

val mediaUploadInfoState = remember { mutableStateOf<AsyncData<MediaUploadInfo>>(AsyncData.Uninitialized) }
LaunchedEffect(Unit) {
Expand Down Expand Up @@ -112,7 +114,7 @@ class AttachmentsPreviewPresenter @AssistedInject constructor(
fun handleEvents(attachmentsPreviewEvents: AttachmentsPreviewEvents) {
when (attachmentsPreviewEvents) {
is AttachmentsPreviewEvents.SendAttachment -> coroutineScope.launch {
val useSendQueue = featureFlagsService.isFeatureEnabled(FeatureFlags.MediaUploadOnSendQueue)
val useSendQueue = featureFlagService.isFeatureEnabled(FeatureFlags.MediaUploadOnSendQueue)
userSentAttachment.value = true
val instantSending = mediaUploadInfoState.value.isReady() && useSendQueue
sendActionState.value = if (instantSending) {
Expand Down Expand Up @@ -142,6 +144,7 @@ class AttachmentsPreviewPresenter @AssistedInject constructor(
attachment = attachment,
sendActionState = sendActionState.value,
textEditorState = textEditorState,
allowCaption = allowCaption,
eventSink = ::handleEvents
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,9 @@ data class AttachmentsPreviewState(
val attachment: Attachment,
val sendActionState: SendActionState,
val textEditorState: TextEditorState,
val allowCaption: Boolean,
val eventSink: (AttachmentsPreviewEvents) -> Unit
) {
// Keep the val to eventually set to false for some mimetypes.
val allowCaption: Boolean = true
}
)

@Immutable
sealed interface SendActionState {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,21 @@ open class AttachmentsPreviewStateProvider : PreviewParameterProvider<Attachment
anAttachmentsPreviewState(sendActionState = SendActionState.Sending.Processing),
anAttachmentsPreviewState(sendActionState = SendActionState.Sending.Uploading(0.5f)),
anAttachmentsPreviewState(sendActionState = SendActionState.Failure(RuntimeException("error"))),
anAttachmentsPreviewState(allowCaption = false),
)
}

fun anAttachmentsPreviewState(
mediaInfo: MediaInfo = anImageMediaInfo(),
textEditorState: TextEditorState = aTextEditorStateMarkdown(),
sendActionState: SendActionState = SendActionState.Idle,
allowCaption: Boolean = true,
) = AttachmentsPreviewState(
attachment = Attachment.Media(
localMedia = LocalMedia("file://path".toUri(), mediaInfo),
),
sendActionState = sendActionState,
textEditorState = textEditorState,
allowCaption = allowCaption,
eventSink = {}
)
Original file line number Diff line number Diff line change
Expand Up @@ -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.aTimelineItemVoiceContent
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.timeline.item.event.LocalEventSendState
import io.element.android.libraries.matrix.test.AN_EVENT_ID
Expand Down Expand Up @@ -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
fun `present - compute for a media with caption item`() = runTest {
val presenter = createActionListPresenter(isDeveloperModeEnabled = true, isPinFeatureEnabled = true)
Expand Down Expand Up @@ -1151,13 +1204,19 @@ private fun createActionListPresenter(
isDeveloperModeEnabled: Boolean,
isPinFeatureEnabled: Boolean,
room: MatrixRoom = FakeMatrixRoom(),
allowCaption: Boolean = true,
): ActionListPresenter {
val preferencesStore = InMemoryAppPreferencesStore(isDeveloperModeEnabled = isDeveloperModeEnabled)
return DefaultActionListPresenter(
postProcessor = TimelineItemActionPostProcessor.Default,
appPreferencesStore = preferencesStore,
isPinnedMessagesFeatureEnabled = { isPinFeatureEnabled },
room = room,
userSendFailureFactory = VerifiedUserSendFailureFactory(room)
userSendFailureFactory = VerifiedUserSendFailureFactory(room),
featureFlagService = FakeFeatureFlagService(
initialState = mapOf(
FeatureFlags.MediaCaptionCreation.key to allowCaption,
),
)
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,28 @@ class AttachmentsPreviewPresenterTest {

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
fun `present - send media success scenario`() = runTest {
val sendFileResult = lambdaRecorder<File, FileInfo, String?, String?, ProgressCallback?, Result<FakeMediaUploadHandler>> { _, _, _, _, _ ->
Expand Down Expand Up @@ -420,15 +442,19 @@ class AttachmentsPreviewPresenterTest {
temporaryUriDeleter: TemporaryUriDeleter = FakeTemporaryUriDeleter(),
onDoneListener: OnDoneListener = OnDoneListener { lambdaError() },
mediaUploadOnSendQueueEnabled: Boolean = true,
allowCaption: Boolean = true,
): AttachmentsPreviewPresenter {
return AttachmentsPreviewPresenter(
attachment = aMediaAttachment(localMedia),
onDoneListener = onDoneListener,
mediaSender = MediaSender(mediaPreProcessor, room, InMemorySessionPreferencesStore()),
permalinkBuilder = permalinkBuilder,
temporaryUriDeleter = temporaryUriDeleter,
featureFlagsService = FakeFeatureFlagService(
initialState = mapOf(FeatureFlags.MediaUploadOnSendQueue.key to mediaUploadOnSendQueueEnabled),
featureFlagService = FakeFeatureFlagService(
initialState = mapOf(
FeatureFlags.MediaUploadOnSendQueue.key to mediaUploadOnSendQueueEnabled,
FeatureFlags.MediaCaptionCreation.key to allowCaption,
),
)
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,4 +140,11 @@ enum class FeatureFlags(
defaultValue = { buildMeta -> buildMeta.buildType != BuildType.RELEASE },
isFinished = false,
),
MediaCaptionCreation(
key = "feature.media_caption_creation",
title = "Allow creation of media captions",
description = null,
defaultValue = { buildMeta -> buildMeta.buildType != BuildType.RELEASE },
isFinished = false,
),
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit 5610b19

Please sign in to comment.