From 3ac53d20e9d7fe01a2698cd5092f155f60e410a6 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 10 Dec 2019 01:05:20 +0100 Subject: [PATCH] Bugfix: react several times with the same reaction was possible (was a TODO). --- .../room/model/relation/RelationService.kt | 1 + .../room/relation/DefaultRelationService.kt | 31 ++++++++++++++++--- .../home/room/detail/RoomDetailFragment.kt | 6 +--- .../reactions/EmojiReactionPickerActivity.kt | 10 ++++-- 4 files changed, 36 insertions(+), 12 deletions(-) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/model/relation/RelationService.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/model/relation/RelationService.kt index 385699b4db..b3dd1c6f22 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/model/relation/RelationService.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/model/relation/RelationService.kt @@ -50,6 +50,7 @@ interface RelationService { /** * Sends a reaction (emoji) to the targetedEvent. + * It has no effect if the user has already added the same reaction to the event. * @param targetEventId the id of the event being reacted * @param reaction the reaction (preferably emoji) */ diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/relation/DefaultRelationService.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/relation/DefaultRelationService.kt index db3b6100a0..8731045e14 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/relation/DefaultRelationService.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/relation/DefaultRelationService.kt @@ -30,10 +30,13 @@ import im.vector.matrix.android.api.session.room.model.message.MessageType import im.vector.matrix.android.api.session.room.model.relation.RelationService import im.vector.matrix.android.api.session.room.timeline.TimelineEvent import im.vector.matrix.android.api.util.Cancelable +import im.vector.matrix.android.api.util.NoOpCancellable import im.vector.matrix.android.api.util.Optional import im.vector.matrix.android.api.util.toOptional +import im.vector.matrix.android.internal.database.mapper.TimelineEventMapper import im.vector.matrix.android.internal.database.mapper.asDomain import im.vector.matrix.android.internal.database.model.EventAnnotationsSummaryEntity +import im.vector.matrix.android.internal.database.model.TimelineEventEntity import im.vector.matrix.android.internal.database.query.where import im.vector.matrix.android.internal.di.UserId import im.vector.matrix.android.internal.session.room.send.EncryptEventWorker @@ -44,6 +47,7 @@ import im.vector.matrix.android.internal.session.room.timeline.TimelineSendEvent import im.vector.matrix.android.internal.task.TaskExecutor import im.vector.matrix.android.internal.task.configureWith import im.vector.matrix.android.internal.util.CancelableWork +import im.vector.matrix.android.internal.util.fetchCopyMap import im.vector.matrix.android.internal.worker.WorkerParamsFactory import timber.log.Timber @@ -54,6 +58,7 @@ internal class DefaultRelationService @AssistedInject constructor(@Assisted priv private val cryptoService: CryptoService, private val findReactionEventForUndoTask: FindReactionEventForUndoTask, private val fetchEditHistoryTask: FetchEditHistoryTask, + private val timelineEventMapper: TimelineEventMapper, private val monarchy: Monarchy, private val taskExecutor: TaskExecutor) : RelationService { @@ -64,11 +69,27 @@ internal class DefaultRelationService @AssistedInject constructor(@Assisted priv } override fun sendReaction(targetEventId: String, reaction: String): Cancelable { - val event = eventFactory.createReactionEvent(roomId, targetEventId, reaction) - .also { saveLocalEcho(it) } - val sendRelationWork = createSendEventWork(event, true) - TimelineSendEventWorkCommon.postWork(context, roomId, sendRelationWork) - return CancelableWork(context, sendRelationWork.id) + return if (monarchy + .fetchCopyMap( + { realm -> + TimelineEventEntity.where(realm, roomId, targetEventId).findFirst() + }, + { entity, _ -> + timelineEventMapper.map(entity) + }) + ?.annotations + ?.reactionsSummary + .orEmpty() + .none { it.addedByMe && it.key == reaction }) { + val event = eventFactory.createReactionEvent(roomId, targetEventId, reaction) + .also { saveLocalEcho(it) } + val sendRelationWork = createSendEventWork(event, true) + TimelineSendEventWorkCommon.postWork(context, roomId, sendRelationWork) + CancelableWork(context, sendRelationWork.id) + } else { + Timber.w("Reaction already added") + NoOpCancellable + } } override fun undoReaction(targetEventId: String, reaction: String): Cancelable { diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailFragment.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailFragment.kt index e1bb24eb8f..80f54a9c1f 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailFragment.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailFragment.kt @@ -450,11 +450,7 @@ class RoomDetailFragment @Inject constructor( if (!hasBeenHandled && resultCode == RESULT_OK && data != null) { when (requestCode) { REACTION_SELECT_REQUEST_CODE -> { - val eventId = data.getStringExtra(EmojiReactionPickerActivity.EXTRA_EVENT_ID) - ?: return - val reaction = data.getStringExtra(EmojiReactionPickerActivity.EXTRA_REACTION_RESULT) - ?: return - // TODO check if already reacted with that? + val (eventId, reaction) = EmojiReactionPickerActivity.getOutput(data) ?: return roomDetailViewModel.handle(RoomDetailAction.SendReaction(eventId, reaction)) } } diff --git a/vector/src/main/java/im/vector/riotx/features/reactions/EmojiReactionPickerActivity.kt b/vector/src/main/java/im/vector/riotx/features/reactions/EmojiReactionPickerActivity.kt index 96536e1f16..562ad6f5b2 100644 --- a/vector/src/main/java/im/vector/riotx/features/reactions/EmojiReactionPickerActivity.kt +++ b/vector/src/main/java/im/vector/riotx/features/reactions/EmojiReactionPickerActivity.kt @@ -203,13 +203,19 @@ class EmojiReactionPickerActivity : VectorBaseActivity(), companion object { - const val EXTRA_EVENT_ID = "EXTRA_EVENT_ID" - const val EXTRA_REACTION_RESULT = "EXTRA_REACTION_RESULT" + private const val EXTRA_EVENT_ID = "EXTRA_EVENT_ID" + private const val EXTRA_REACTION_RESULT = "EXTRA_REACTION_RESULT" fun intent(context: Context, eventId: String): Intent { val intent = Intent(context, EmojiReactionPickerActivity::class.java) intent.putExtra(EXTRA_EVENT_ID, eventId) return intent } + + fun getOutput(data: Intent): Pair? { + val eventId = data.getStringExtra(EXTRA_EVENT_ID) ?: return null + val reaction = data.getStringExtra(EXTRA_REACTION_RESULT) ?: return null + return eventId to reaction + } } }