Skip to content
Closed
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
1c8fd30
Extend DraftEntity to be able to handle multiple message types.
Oct 13, 2021
3cc57f2
Fix draft entity migration.
Oct 14, 2021
eaf063c
Save draft with message type.
Oct 18, 2021
e97c065
Render draft message.
Oct 21, 2021
a459692
Do not create redundant temp files.
Oct 21, 2021
dfd6884
Changelog added.
Oct 21, 2021
e5865bf
Code review fixes.
Oct 25, 2021
bfdacfe
removing unused import
ouchadam Nov 9, 2021
2d81260
using sealed interface to avoid duplicated constructor calls
ouchadam Nov 9, 2021
a345f26
handling voice drafts via the message composer view model
ouchadam Nov 19, 2021
95e5c12
adding dedicated migration for the draft message type
ouchadam Nov 19, 2021
5687c36
classifying the draft mode as the voice recorder being active to avoi…
ouchadam Nov 22, 2021
6e8c545
fixing #3833, it's caused by us always using the same file name when …
ouchadam Nov 22, 2021
e790091
fixing line length
ouchadam Nov 22, 2021
b9b51af
adding changelog entry
ouchadam Nov 22, 2021
00b954f
removing duplicated migration due to rebase
ouchadam Nov 22, 2021
cd4e991
removing redundant exists() checks, mkdirs already does this
ouchadam Nov 22, 2021
4b67a6d
removing redundant state update, the state already updates when the r…
ouchadam Nov 22, 2021
7655c25
removing unused base property and fixing sealed class casing
ouchadam Nov 22, 2021
94161b2
removing unused message types
ouchadam Nov 22, 2021
517a0d6
using dedicated Voice types instead of message sub type
ouchadam Nov 22, 2021
c11bc2d
replacing flaky post check with a doOnLayout, this allows us to only …
ouchadam Nov 23, 2021
60ed9ec
replacing message type entity change with a dedicated voice draft and…
ouchadam Nov 23, 2021
bf968cc
merging the playback and draft states together, they're the same user…
ouchadam Nov 23, 2021
f360021
replacing cancelled state with None, lifts the vibration up to the fr…
ouchadam Nov 23, 2021
bb2239c
renaming recording ui states to better align to the user facing states
ouchadam Nov 23, 2021
c2ec157
using random file name for the voice message files and using the cont…
ouchadam Nov 23, 2021
be103fc
inlining single use playback views
ouchadam Nov 23, 2021
ff2dc71
making the playback tracker a singleton to avoid using the factory wo…
ouchadam Nov 23, 2021
9939298
parsing the file path from the content uri instead of assuming the fi…
ouchadam Nov 23, 2021
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/3833.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixing queued voice message failing to send or retry
1 change: 1 addition & 0 deletions changelog.d/3922.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Voice messages: Persist drafts of voice messages when navigating between rooms
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import androidx.exifinterface.media.ExifInterface
import com.squareup.moshi.JsonClass
import kotlinx.parcelize.Parcelize
import org.matrix.android.sdk.api.util.MimeTypes.normalizeMimeType
import org.matrix.android.sdk.internal.di.MoshiProvider

@Parcelize
@JsonClass(generateAdapter = true)
Expand All @@ -48,4 +49,14 @@ data class ContentAttachmentData(
}

fun getSafeMimeType() = mimeType?.normalizeMimeType()

fun toJsonString(): String {
return MoshiProvider.providesMoshi().adapter(ContentAttachmentData::class.java).toJson(this)
}

companion object {
fun fromJsonString(json: String): ContentAttachmentData? {
return MoshiProvider.providesMoshi().adapter(ContentAttachmentData::class.java).fromJson(json)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,18 @@ package org.matrix.android.sdk.api.session.room.send
* EDIT: draft of an edition of a message
* REPLY: draft of a reply of another message
*/
sealed class UserDraft(open val text: String) {
data class REGULAR(override val text: String) : UserDraft(text)
data class QUOTE(val linkedEventId: String, override val text: String) : UserDraft(text)
data class EDIT(val linkedEventId: String, override val text: String) : UserDraft(text)
data class REPLY(val linkedEventId: String, override val text: String) : UserDraft(text)
sealed interface UserDraft {
val content: String
val messageType: String

data class REGULAR(override val content: String, override val messageType: String) : UserDraft
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm wondering if Voice should be a dedicated draft type rather than a messageType sub type 🤔

Copy link
Member

Choose a reason for hiding this comment

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

I think the code would be clearer with a VOICE data class (they are allcaps...). But I do not know if we will end up with many other cases. @onurays WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it would also mean we can avoid any database entity changes

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch. That make sense and yes we are preserving the draftMode in DraftEntity. So no migration will be needed.

data class QUOTE(val linkedEventId: String, override val content: String, override val messageType: String) : UserDraft
data class EDIT(val linkedEventId: String, override val content: String, override val messageType: String) : UserDraft
data class REPLY(val linkedEventId: String, override val content: String, override val messageType: String) : UserDraft

fun isValid(): Boolean {
return when (this) {
is REGULAR -> text.isNotBlank()
is REGULAR -> content.isNotBlank()
else -> true
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ import org.matrix.android.sdk.api.session.room.model.Membership
import org.matrix.android.sdk.api.session.room.model.RoomJoinRulesContent
import org.matrix.android.sdk.api.session.room.model.VersioningState
import org.matrix.android.sdk.api.session.room.model.create.RoomCreateContent
import org.matrix.android.sdk.api.session.room.model.message.MessageType
import org.matrix.android.sdk.api.session.room.model.tag.RoomTag
import org.matrix.android.sdk.internal.database.model.CurrentStateEventEntityFields
import org.matrix.android.sdk.internal.database.model.DraftEntityFields
import org.matrix.android.sdk.internal.database.model.EditAggregatedSummaryEntityFields
import org.matrix.android.sdk.internal.database.model.EditionOfEventFields
import org.matrix.android.sdk.internal.database.model.EventEntityFields
Expand Down Expand Up @@ -54,7 +56,7 @@ internal class RealmSessionStoreMigration @Inject constructor(
) : RealmMigration {

companion object {
const val SESSION_STORE_SCHEMA_VERSION = 19L
const val SESSION_STORE_SCHEMA_VERSION = 20L
}

/**
Expand Down Expand Up @@ -86,6 +88,7 @@ internal class RealmSessionStoreMigration @Inject constructor(
if (oldVersion <= 16) migrateTo17(realm)
if (oldVersion <= 17) migrateTo18(realm)
if (oldVersion <= 18) migrateTo19(realm)
if (oldVersion <= 19) migrateTo20(realm)
}

private fun migrateTo1(realm: DynamicRealm) {
Expand Down Expand Up @@ -390,4 +393,14 @@ internal class RealmSessionStoreMigration @Inject constructor(
}
}
}

private fun migrateTo20(realm: DynamicRealm) {
Timber.d("Step 19 -> 20")
realm.schema.get("DraftEntity")
?.addField(DraftEntityFields.MESSAGE_TYPE, String::class.java)
?.setRequired(DraftEntityFields.MESSAGE_TYPE, true)
?.transform {
it.setString(DraftEntityFields.MESSAGE_TYPE, MessageType.MSGTYPE_TEXT)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package org.matrix.android.sdk.internal.database.mapper

import org.matrix.android.sdk.api.session.room.model.message.MessageType
import org.matrix.android.sdk.api.session.room.send.UserDraft
import org.matrix.android.sdk.internal.database.model.DraftEntity

Expand All @@ -26,20 +27,24 @@ internal object DraftMapper {

fun map(entity: DraftEntity): UserDraft {
return when (entity.draftMode) {
DraftEntity.MODE_REGULAR -> UserDraft.REGULAR(entity.content)
DraftEntity.MODE_EDIT -> UserDraft.EDIT(entity.linkedEventId, entity.content)
DraftEntity.MODE_QUOTE -> UserDraft.QUOTE(entity.linkedEventId, entity.content)
DraftEntity.MODE_REPLY -> UserDraft.REPLY(entity.linkedEventId, entity.content)
DraftEntity.MODE_REGULAR -> UserDraft.REGULAR(entity.content, entity.messageType)
DraftEntity.MODE_EDIT -> UserDraft.EDIT(entity.linkedEventId, entity.content, entity.messageType)
DraftEntity.MODE_QUOTE -> UserDraft.QUOTE(entity.linkedEventId, entity.content, entity.messageType)
DraftEntity.MODE_REPLY -> UserDraft.REPLY(entity.linkedEventId, entity.content, entity.messageType)
else -> null
} ?: UserDraft.REGULAR("")
} ?: UserDraft.REGULAR("", MessageType.MSGTYPE_TEXT)
}

fun map(domain: UserDraft): DraftEntity {
return when (domain) {
is UserDraft.REGULAR -> DraftEntity(content = domain.text, draftMode = DraftEntity.MODE_REGULAR, linkedEventId = "")
is UserDraft.EDIT -> DraftEntity(content = domain.text, draftMode = DraftEntity.MODE_EDIT, linkedEventId = domain.linkedEventId)
is UserDraft.QUOTE -> DraftEntity(content = domain.text, draftMode = DraftEntity.MODE_QUOTE, linkedEventId = domain.linkedEventId)
is UserDraft.REPLY -> DraftEntity(content = domain.text, draftMode = DraftEntity.MODE_REPLY, linkedEventId = domain.linkedEventId)
is UserDraft.REGULAR -> DraftEntity(content = domain.content,
draftMode = DraftEntity.MODE_REGULAR,
linkedEventId = "",
messageType = domain.messageType
)
is UserDraft.EDIT -> DraftEntity(content = domain.content, draftMode = DraftEntity.MODE_EDIT, linkedEventId = domain.linkedEventId)
is UserDraft.QUOTE -> DraftEntity(content = domain.content, draftMode = DraftEntity.MODE_QUOTE, linkedEventId = domain.linkedEventId)
is UserDraft.REPLY -> DraftEntity(content = domain.content, draftMode = DraftEntity.MODE_REPLY, linkedEventId = domain.linkedEventId)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@
package org.matrix.android.sdk.internal.database.model

import io.realm.RealmObject
import org.matrix.android.sdk.api.session.room.model.message.MessageType

internal open class DraftEntity(var content: String = "",
var draftMode: String = MODE_REGULAR,
var linkedEventId: String = ""
var linkedEventId: String = "",
var messageType: String = MessageType.MSGTYPE_TEXT

) : RealmObject() {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ import org.matrix.android.sdk.api.session.room.model.message.MessageFormat
import org.matrix.android.sdk.api.session.room.model.message.MessageImageInfoContent
import org.matrix.android.sdk.api.session.room.model.message.MessageStickerContent
import org.matrix.android.sdk.api.session.room.model.message.MessageTextContent
import org.matrix.android.sdk.api.session.room.model.message.MessageType
import org.matrix.android.sdk.api.session.room.model.message.MessageVerificationRequestContent
import org.matrix.android.sdk.api.session.room.model.message.MessageVideoContent
import org.matrix.android.sdk.api.session.room.model.message.MessageWithAttachmentContent
Expand Down Expand Up @@ -392,7 +393,13 @@ class RoomDetailFragment @Inject constructor(
return@onEach
}
when (mode) {
is SendMode.REGULAR -> renderRegularMode(mode.text)
is SendMode.REGULAR -> {
if (mode.messageType == MessageType.MSGTYPE_AUDIO) {
Copy link
Member

Choose a reason for hiding this comment

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

This is where the code is not typed enough IMO, MSGTYPE_AUDIO is not necessarily a voice message. But for draft this is the only possible case 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.

I think this is a sign to move away from the MSGTYPE and have a separate Voice send mode

renderVoiceMessageMode(mode.text)
} else {
renderRegularMode(mode.text)
}
}
is SendMode.EDIT -> renderSpecialMode(mode.timelineEvent, R.drawable.ic_edit, R.string.edit, mode.text)
is SendMode.QUOTE -> renderSpecialMode(mode.timelineEvent, R.drawable.ic_quote, R.string.quote, mode.text)
is SendMode.REPLY -> renderSpecialMode(mode.timelineEvent, R.drawable.ic_reply, R.string.reply, mode.text)
Expand Down Expand Up @@ -471,6 +478,13 @@ class RoomDetailFragment @Inject constructor(
}
}

private fun renderVoiceMessageMode(content: String) {
ContentAttachmentData.fromJsonString(content)?.let { audioAttachmentData ->
views.voiceMessageRecorderView.isVisible = true
messageComposerViewModel.handle(MessageComposerAction.InitializeVoiceRecorder(audioAttachmentData))
}
}

private fun handleSendButtonVisibilityChanged(event: MessageComposerViewEvents.AnimateSendButtonVisibility) {
if (event.isVisible) {
views.voiceMessageRecorderView.isVisible = false
Expand Down Expand Up @@ -1044,10 +1058,10 @@ class RoomDetailFragment @Inject constructor(
.show()
}

private fun renderRegularMode(text: String) {
private fun renderRegularMode(content: String) {
autoCompleter.exitSpecialMode()
views.composerLayout.collapse()
views.composerLayout.setTextIfDifferent(text)
views.composerLayout.setTextIfDifferent(content)
views.composerLayout.views.sendButton.contentDescription = getString(R.string.send)
}

Expand Down Expand Up @@ -1131,14 +1145,8 @@ class RoomDetailFragment @Inject constructor(

override fun onPause() {
super.onPause()

notificationDrawerManager.setCurrentRoom(null)

messageComposerViewModel.handle(MessageComposerAction.SaveDraft(views.composerLayout.text.toString()))

// We should improve the UX to support going into playback mode when paused and delete the media when the view is destroyed.
messageComposerViewModel.handle(MessageComposerAction.EndAllVoiceActions(deleteRecord = false))
views.voiceMessageRecorderView.render(RecordingUiState.None)
messageComposerViewModel.handle(MessageComposerAction.OnEntersBackground(views.composerLayout.text.toString()))
}

private val attachmentFileActivityResultLauncher = registerStartForActivityResult {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,21 @@ package im.vector.app.features.home.room.detail.composer

import im.vector.app.core.platform.VectorViewModelAction
import im.vector.app.features.home.room.detail.composer.voice.VoiceMessageRecorderView
import org.matrix.android.sdk.api.session.content.ContentAttachmentData
import org.matrix.android.sdk.api.session.room.model.message.MessageAudioContent

sealed class MessageComposerAction : VectorViewModelAction {
data class SaveDraft(val draft: String) : MessageComposerAction()
data class SendMessage(val text: CharSequence, val autoMarkdown: Boolean) : MessageComposerAction()
data class EnterEditMode(val eventId: String, val text: String) : MessageComposerAction()
data class EnterQuoteMode(val eventId: String, val text: String) : MessageComposerAction()
data class EnterReplyMode(val eventId: String, val text: String) : MessageComposerAction()
data class EnterRegularMode(val text: String, val fromSharing: Boolean) : MessageComposerAction()
data class UserIsTyping(val isTyping: Boolean) : MessageComposerAction()
data class OnTextChanged(val text: CharSequence) : MessageComposerAction()
data class OnEntersBackground(val composerText: String) : MessageComposerAction()

// Voice Message
data class InitializeVoiceRecorder(val attachmentData: ContentAttachmentData) : MessageComposerAction()
data class OnVoiceRecordingUiStateChanged(val uiState: VoiceMessageRecorderView.RecordingUiState) : MessageComposerAction()
object StartRecordingVoiceMessage : MessageComposerAction()
data class EndRecordingVoiceMessage(val isCancelled: Boolean) : MessageComposerAction()
Expand Down
Loading