From b525259bec985588de42b78c1efbbe6a76e481b6 Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Tue, 22 Feb 2022 17:18:09 +0100 Subject: [PATCH 01/26] Adding changelog entry --- changelog.d/5005.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5005.feature diff --git a/changelog.d/5005.feature b/changelog.d/5005.feature new file mode 100644 index 0000000000..2db648cd6a --- /dev/null +++ b/changelog.d/5005.feature @@ -0,0 +1 @@ +Add possibility to save image in Gallery + reorder choices in message context menu From 3384a0caa076b304ab4d57697817485823f1b16f Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Tue, 22 Feb 2022 17:41:23 +0100 Subject: [PATCH 02/26] TODO --- .../room/detail/timeline/action/MessageActionsViewModel.kt | 1 + .../app/features/media/VectorAttachmentViewerActivity.kt | 3 +++ 2 files changed, 4 insertions(+) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt index 745cb0c731..763720faa3 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt @@ -251,6 +251,7 @@ class MessageActionsViewModel @AssistedInject constructor(@Assisted val msgType = messageContent?.msgType return arrayListOf().apply { + // TODO need to check all possible items and confirm order when { timelineEvent.root.sendState.hasFailed() -> { addActionsForFailedState(timelineEvent, actionPermissions, messageContent, msgType) diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt index 103511bad5..6c32961ce0 100644 --- a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt @@ -249,7 +249,10 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), BaseAttachmen handle(AttachmentCommands.SeekTo(percent)) } + // TODO add save feature for image => check it works for video as well, + // check if it is already possible to save from menu with long press on video override fun onShareTapped() { + // TODO move the retrieve of the file into ViewModel and use a ViewEvent to call shareMedia lifecycleScope.launch(Dispatchers.IO) { val file = currentSourceProvider?.getFileForSharing(currentPosition) ?: return@launch From 6899b5b637940298fd610b41c57daa15c089e8e6 Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Wed, 23 Feb 2022 11:32:57 +0100 Subject: [PATCH 03/26] Creating dedicated attachment interaction listener --- .../media/AttachmentInteractionListener.kt | 24 +++++ .../features/media/AttachmentOverlayView.kt | 31 ++++--- .../features/media/BaseAttachmentProvider.kt | 22 +---- .../media/VectorAttachmentViewerActivity.kt | 88 ++++++++++++------- 4 files changed, 105 insertions(+), 60 deletions(-) create mode 100644 vector/src/main/java/im/vector/app/features/media/AttachmentInteractionListener.kt diff --git a/vector/src/main/java/im/vector/app/features/media/AttachmentInteractionListener.kt b/vector/src/main/java/im/vector/app/features/media/AttachmentInteractionListener.kt new file mode 100644 index 0000000000..3a8d9b49a6 --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/media/AttachmentInteractionListener.kt @@ -0,0 +1,24 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.media + +interface AttachmentInteractionListener { + fun onDismissTapped() + fun onShareTapped() + fun onPlayPause(play: Boolean) + fun videoSeekTo(percent: Int) +} diff --git a/vector/src/main/java/im/vector/app/features/media/AttachmentOverlayView.kt b/vector/src/main/java/im/vector/app/features/media/AttachmentOverlayView.kt index f79fb03898..47dbae863b 100644 --- a/vector/src/main/java/im/vector/app/features/media/AttachmentOverlayView.kt +++ b/vector/src/main/java/im/vector/app/features/media/AttachmentOverlayView.kt @@ -30,35 +30,38 @@ class AttachmentOverlayView @JvmOverloads constructor( context: Context, attrs: AttributeSet? = null, defStyleAttr: Int = 0 ) : ConstraintLayout(context, attrs, defStyleAttr), AttachmentEventListener { - var onShareCallback: (() -> Unit)? = null - var onBack: (() -> Unit)? = null - var onPlayPause: ((play: Boolean) -> Unit)? = null - var videoSeekTo: ((progress: Int) -> Unit)? = null + /* ========================================================================================== + * Fields + * ========================================================================================== */ + var interactionListener: AttachmentInteractionListener? = null val views: MergeImageAttachmentOverlayBinding - var isPlaying = false + private var isPlaying = false + private var suspendSeekBarUpdate = false - var suspendSeekBarUpdate = false + /* ========================================================================================== + * Init + * ========================================================================================== */ init { inflate(context, R.layout.merge_image_attachment_overlay, this) views = MergeImageAttachmentOverlayBinding.bind(this) setBackgroundColor(Color.TRANSPARENT) views.overlayBackButton.setOnClickListener { - onBack?.invoke() + interactionListener?.onDismissTapped() } views.overlayShareButton.setOnClickListener { - onShareCallback?.invoke() + interactionListener?.onShareTapped() } views.overlayPlayPauseButton.setOnClickListener { - onPlayPause?.invoke(!isPlaying) + interactionListener?.onPlayPause(!isPlaying) } views.overlaySeekBar.setOnSeekBarChangeListener(object : SeekBar.OnSeekBarChangeListener { override fun onProgressChanged(seekBar: SeekBar?, progress: Int, fromUser: Boolean) { if (fromUser) { - videoSeekTo?.invoke(progress) + interactionListener?.videoSeekTo(progress) } } @@ -72,11 +75,19 @@ class AttachmentOverlayView @JvmOverloads constructor( }) } + /* ========================================================================================== + * Public API + * ========================================================================================== */ + fun updateWith(counter: String, senderInfo: String) { views.overlayCounterText.text = counter views.overlayInfoText.text = senderInfo } + /* ========================================================================================== + * Specialization + * ========================================================================================== */ + override fun onEvent(event: AttachmentEvents) { when (event) { is AttachmentEvents.VideoEvent -> { diff --git a/vector/src/main/java/im/vector/app/features/media/BaseAttachmentProvider.kt b/vector/src/main/java/im/vector/app/features/media/BaseAttachmentProvider.kt index ca469bfbcb..4039ea112b 100644 --- a/vector/src/main/java/im/vector/app/features/media/BaseAttachmentProvider.kt +++ b/vector/src/main/java/im/vector/app/features/media/BaseAttachmentProvider.kt @@ -49,14 +49,7 @@ abstract class BaseAttachmentProvider( private val stringProvider: StringProvider ) : AttachmentSourceProvider { - interface InteractionListener { - fun onDismissTapped() - fun onShareTapped() - fun onPlayPause(play: Boolean) - fun videoSeekTo(percent: Int) - } - - var interactionListener: InteractionListener? = null + var interactionListener: AttachmentInteractionListener? = null private var overlayView: AttachmentOverlayView? = null @@ -68,18 +61,7 @@ abstract class BaseAttachmentProvider( if (position == -1) return null if (overlayView == null) { overlayView = AttachmentOverlayView(context) - overlayView?.onBack = { - interactionListener?.onDismissTapped() - } - overlayView?.onShareCallback = { - interactionListener?.onShareTapped() - } - overlayView?.onPlayPause = { play -> - interactionListener?.onPlayPause(play) - } - overlayView?.videoSeekTo = { percent -> - interactionListener?.videoSeekTo(percent) - } + overlayView?.interactionListener = interactionListener } val timelineEvent = getTimelineEventAtPosition(position) diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt index 6c32961ce0..0e8405d0a1 100644 --- a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt @@ -47,7 +47,11 @@ import timber.log.Timber import javax.inject.Inject @AndroidEntryPoint -class VectorAttachmentViewerActivity : AttachmentViewerActivity(), BaseAttachmentProvider.InteractionListener { +class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInteractionListener { + + /* ========================================================================================== + * Arguments + * ========================================================================================== */ @Parcelize data class Args( @@ -56,6 +60,10 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), BaseAttachmen val sharedTransitionName: String? ) : Parcelable + /* ========================================================================================== + * Dependencies + * ========================================================================================== */ + @Inject lateinit var sessionHolder: ActiveSessionHolder @Inject @@ -63,11 +71,18 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), BaseAttachmen @Inject lateinit var imageContentRenderer: ImageContentRenderer + /* ========================================================================================== + * Fields + * ========================================================================================== */ + private var initialIndex = 0 private var isAnimatingOut = false - private var currentSourceProvider: BaseAttachmentProvider<*>? = null + /* ========================================================================================== + * Lifecycle + * ========================================================================================== */ + override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) Timber.i("onCreate Activity ${javaClass.simpleName}") @@ -140,12 +155,6 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), BaseAttachmen Timber.i("onPause Activity ${javaClass.simpleName}") } - private fun getOtherThemes() = ActivityOtherThemes.VectorAttachmentsPreview - - override fun shouldAnimateDismiss(): Boolean { - return currentPosition != initialIndex - } - override fun onBackPressed() { if (currentPosition == initialIndex) { // show back the transition view @@ -156,6 +165,14 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), BaseAttachmen super.onBackPressed() } + /* ========================================================================================== + * Specialization + * ========================================================================================== */ + + override fun shouldAnimateDismiss(): Boolean { + return currentPosition != initialIndex + } + override fun animateClose() { if (currentPosition == initialIndex) { // show back the transition view @@ -166,9 +183,11 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), BaseAttachmen ActivityCompat.finishAfterTransition(this) } - // ========================================================================================== - // PRIVATE METHODS - // ========================================================================================== + /* ========================================================================================== + * Private methods + * ========================================================================================== */ + + private fun getOtherThemes() = ActivityOtherThemes.VectorAttachmentsPreview /** * Try and add a [Transition.TransitionListener] to the entering shared element @@ -218,24 +237,9 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), BaseAttachmen }) } - companion object { - const val EXTRA_ARGS = "EXTRA_ARGS" - const val EXTRA_IMAGE_DATA = "EXTRA_IMAGE_DATA" - const val EXTRA_IN_MEMORY_DATA = "EXTRA_IN_MEMORY_DATA" - - fun newIntent(context: Context, - mediaData: AttachmentData, - roomId: String?, - eventId: String, - inMemoryData: List, - sharedTransitionName: String?) = Intent(context, VectorAttachmentViewerActivity::class.java).also { - it.putExtra(EXTRA_ARGS, Args(roomId, eventId, sharedTransitionName)) - it.putExtra(EXTRA_IMAGE_DATA, mediaData) - if (inMemoryData.isNotEmpty()) { - it.putParcelableArrayListExtra(EXTRA_IN_MEMORY_DATA, ArrayList(inMemoryData)) - } - } - } + /* ========================================================================================== + * Specialization AttachmentInteractionListener + * ========================================================================================== */ override fun onDismissTapped() { animateClose() @@ -252,7 +256,8 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), BaseAttachmen // TODO add save feature for image => check it works for video as well, // check if it is already possible to save from menu with long press on video override fun onShareTapped() { - // TODO move the retrieve of the file into ViewModel and use a ViewEvent to call shareMedia + // TODO the opening of share bottom sheet is extremely slow + // move the retrieve of the file into ViewModel and use a ViewEvent to call shareMedia lifecycleScope.launch(Dispatchers.IO) { val file = currentSourceProvider?.getFileForSharing(currentPosition) ?: return@launch @@ -265,4 +270,27 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), BaseAttachmen } } } + + /* ========================================================================================== + * COMPANION + * ========================================================================================== */ + + companion object { + private const val EXTRA_ARGS = "EXTRA_ARGS" + private const val EXTRA_IMAGE_DATA = "EXTRA_IMAGE_DATA" + private const val EXTRA_IN_MEMORY_DATA = "EXTRA_IN_MEMORY_DATA" + + fun newIntent(context: Context, + mediaData: AttachmentData, + roomId: String?, + eventId: String, + inMemoryData: List, + sharedTransitionName: String?) = Intent(context, VectorAttachmentViewerActivity::class.java).also { + it.putExtra(EXTRA_ARGS, Args(roomId, eventId, sharedTransitionName)) + it.putExtra(EXTRA_IMAGE_DATA, mediaData) + if (inMemoryData.isNotEmpty()) { + it.putParcelableArrayListExtra(EXTRA_IN_MEMORY_DATA, ArrayList(inMemoryData)) + } + } + } } From 042c57f9b8301b0715b80a9e04a4b3f831748072 Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Wed, 23 Feb 2022 11:34:32 +0100 Subject: [PATCH 04/26] Renaming some methods to be more concise --- .../app/features/media/AttachmentInteractionListener.kt | 4 ++-- .../im/vector/app/features/media/AttachmentOverlayView.kt | 4 ++-- .../app/features/media/VectorAttachmentViewerActivity.kt | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/media/AttachmentInteractionListener.kt b/vector/src/main/java/im/vector/app/features/media/AttachmentInteractionListener.kt index 3a8d9b49a6..e8f00427f1 100644 --- a/vector/src/main/java/im/vector/app/features/media/AttachmentInteractionListener.kt +++ b/vector/src/main/java/im/vector/app/features/media/AttachmentInteractionListener.kt @@ -17,8 +17,8 @@ package im.vector.app.features.media interface AttachmentInteractionListener { - fun onDismissTapped() - fun onShareTapped() + fun onDismiss() + fun onShare() fun onPlayPause(play: Boolean) fun videoSeekTo(percent: Int) } diff --git a/vector/src/main/java/im/vector/app/features/media/AttachmentOverlayView.kt b/vector/src/main/java/im/vector/app/features/media/AttachmentOverlayView.kt index 47dbae863b..44e736d07a 100644 --- a/vector/src/main/java/im/vector/app/features/media/AttachmentOverlayView.kt +++ b/vector/src/main/java/im/vector/app/features/media/AttachmentOverlayView.kt @@ -49,10 +49,10 @@ class AttachmentOverlayView @JvmOverloads constructor( views = MergeImageAttachmentOverlayBinding.bind(this) setBackgroundColor(Color.TRANSPARENT) views.overlayBackButton.setOnClickListener { - interactionListener?.onDismissTapped() + interactionListener?.onDismiss() } views.overlayShareButton.setOnClickListener { - interactionListener?.onShareTapped() + interactionListener?.onShare() } views.overlayPlayPauseButton.setOnClickListener { interactionListener?.onPlayPause(!isPlaying) diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt index 0e8405d0a1..3e8f48df98 100644 --- a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt @@ -241,7 +241,7 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt * Specialization AttachmentInteractionListener * ========================================================================================== */ - override fun onDismissTapped() { + override fun onDismiss() { animateClose() } @@ -255,7 +255,7 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt // TODO add save feature for image => check it works for video as well, // check if it is already possible to save from menu with long press on video - override fun onShareTapped() { + override fun onShare() { // TODO the opening of share bottom sheet is extremely slow // move the retrieve of the file into ViewModel and use a ViewEvent to call shareMedia lifecycleScope.launch(Dispatchers.IO) { From 0169396d7d91c6a349b654608718bd20db5c8d72 Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Wed, 23 Feb 2022 11:39:04 +0100 Subject: [PATCH 05/26] Adding save icon into viewer --- .../layout/merge_image_attachment_overlay.xml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/vector/src/main/res/layout/merge_image_attachment_overlay.xml b/vector/src/main/res/layout/merge_image_attachment_overlay.xml index d8e2142f87..1a5c6d8bf4 100644 --- a/vector/src/main/res/layout/merge_image_attachment_overlay.xml +++ b/vector/src/main/res/layout/merge_image_attachment_overlay.xml @@ -67,6 +67,23 @@ app:layout_constraintTop_toBottomOf="@id/overlayCounterText" tools:text="Bill 29 Jun at 19:42" /> + + Date: Wed, 23 Feb 2022 14:08:02 +0100 Subject: [PATCH 06/26] Draft --- .../AttachmentViewerViewModel.kt | 22 +++++++++++++++++++ .../action/MessageActionsViewModel.kt | 2 ++ 2 files changed, 24 insertions(+) create mode 100644 library/attachment-viewer/src/main/java/im/vector/lib/attachmentviewer/AttachmentViewerViewModel.kt diff --git a/library/attachment-viewer/src/main/java/im/vector/lib/attachmentviewer/AttachmentViewerViewModel.kt b/library/attachment-viewer/src/main/java/im/vector/lib/attachmentviewer/AttachmentViewerViewModel.kt new file mode 100644 index 0000000000..fbb46d4348 --- /dev/null +++ b/library/attachment-viewer/src/main/java/im/vector/lib/attachmentviewer/AttachmentViewerViewModel.kt @@ -0,0 +1,22 @@ +/* + * Copyright (c) 2020 New Vector Ltd + * Copyright (C) 2018 stfalcon.com + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.lib.attachmentviewer + +class AttachmentViewerViewModel { + +} diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt index 763720faa3..13ae8098c3 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt @@ -554,6 +554,7 @@ class MessageActionsViewModel @AssistedInject constructor(@Assisted } } + // TODO need to add this check into media viewer? private fun canShare(msgType: String?): Boolean { return when (msgType) { MessageType.MSGTYPE_TEXT, @@ -568,6 +569,7 @@ class MessageActionsViewModel @AssistedInject constructor(@Assisted } } + // TODO need to add this into Viewer screen to check if it is saveable? => create extension on MessageType? private fun canSave(msgType: String?): Boolean { return when (msgType) { MessageType.MSGTYPE_IMAGE, From f64268efdb13f90314c2b1fa605de4566c9b0286 Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Wed, 23 Feb 2022 16:27:18 +0100 Subject: [PATCH 07/26] Adding download media use case --- .../home/room/detail/TimelineFragment.kt | 1 + .../action/MessageActionsViewModel.kt | 4 +- .../media/VectorAttachmentViewerActivity.kt | 7 +++ .../domain/usecase/DownloadMediaUseCase.kt | 51 +++++++++++++++++++ 4 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 vector/src/main/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCase.kt diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt index 1a40018526..9ca2e28646 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt @@ -2111,6 +2111,7 @@ class TimelineFragment @Inject constructor( } } + // TODO mutualize permission checking creating activity extension or delegation interface? private fun onSaveActionClicked(action: EventSharedAction.Save) { if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q && !checkPermissions(PERMISSIONS_FOR_WRITING_FILES, requireActivity(), saveActionActivityResultLauncher)) { diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt index 13ae8098c3..6acaedfd3c 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt @@ -251,7 +251,7 @@ class MessageActionsViewModel @AssistedInject constructor(@Assisted val msgType = messageContent?.msgType return arrayListOf().apply { - // TODO need to check all possible items and confirm order + // TODO need to check all possible items and confirm order changes to apply when { timelineEvent.root.sendState.hasFailed() -> { addActionsForFailedState(timelineEvent, actionPermissions, messageContent, msgType) @@ -569,7 +569,7 @@ class MessageActionsViewModel @AssistedInject constructor(@Assisted } } - // TODO need to add this into Viewer screen to check if it is saveable? => create extension on MessageType? + // TODO need to add this into Viewer screen to check if it is saveable? => create extension on MessageType or useCase? private fun canSave(msgType: String?): Boolean { return when (msgType) { MessageType.MSGTYPE_IMAGE, diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt index 3e8f48df98..61110e9d81 100644 --- a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt @@ -271,6 +271,13 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt } } + // TODO create suspendable usecase: downloadMediaUseCase + // create interface for base use case + // create ViewModel with action downloadAction, events downloading, downloaded, error + // call usecase using viewModel + // launch coroutine in Activity using repeatOnLifeCycle extension + // add unit tests for usecase? + /* ========================================================================================== * COMPANION * ========================================================================================== */ diff --git a/vector/src/main/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCase.kt b/vector/src/main/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCase.kt new file mode 100644 index 0000000000..e7b7d02f01 --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCase.kt @@ -0,0 +1,51 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.media.domain.usecase + +import android.content.Context +import androidx.core.net.toUri +import dagger.hilt.android.qualifiers.ApplicationContext +import im.vector.app.core.intent.getMimeTypeFromUri +import im.vector.app.core.utils.saveMedia +import im.vector.app.features.notifications.NotificationUtils +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import java.io.File +import javax.inject.Inject + +class DownloadMediaUseCase @Inject constructor( + @ApplicationContext private val appContext: Context, + private val notificationUtils: NotificationUtils +) { + + /* ========================================================================================== + * Public API + * ========================================================================================== */ + + // TODO find a way to provide Dispatchers via Interface to be able to unit tests + suspend fun execute(file: File): Result = withContext(Dispatchers.IO) { + runCatching { + saveMedia( + context = appContext, + file = file, + title = file.name, + mediaMimeType = getMimeTypeFromUri(appContext, file.toUri()), + notificationUtils = notificationUtils + ) + } + } +} From 38236e781569adcc61f9d9c6275592e42f238bdb Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Wed, 23 Feb 2022 17:23:04 +0100 Subject: [PATCH 08/26] OnDownload callback --- .../media/AttachmentInteractionListener.kt | 1 + .../features/media/AttachmentOverlayView.kt | 3 +++ .../media/VectorAttachmentViewerActivity.kt | 23 +++++++++++-------- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/media/AttachmentInteractionListener.kt b/vector/src/main/java/im/vector/app/features/media/AttachmentInteractionListener.kt index e8f00427f1..b0cb913596 100644 --- a/vector/src/main/java/im/vector/app/features/media/AttachmentInteractionListener.kt +++ b/vector/src/main/java/im/vector/app/features/media/AttachmentInteractionListener.kt @@ -19,6 +19,7 @@ package im.vector.app.features.media interface AttachmentInteractionListener { fun onDismiss() fun onShare() + fun onDownload() fun onPlayPause(play: Boolean) fun videoSeekTo(percent: Int) } diff --git a/vector/src/main/java/im/vector/app/features/media/AttachmentOverlayView.kt b/vector/src/main/java/im/vector/app/features/media/AttachmentOverlayView.kt index 44e736d07a..0b3407c5a4 100644 --- a/vector/src/main/java/im/vector/app/features/media/AttachmentOverlayView.kt +++ b/vector/src/main/java/im/vector/app/features/media/AttachmentOverlayView.kt @@ -54,6 +54,9 @@ class AttachmentOverlayView @JvmOverloads constructor( views.overlayShareButton.setOnClickListener { interactionListener?.onShare() } + views.overlayDownloadButton.setOnClickListener { + interactionListener?.onDownload() + } views.overlayPlayPauseButton.setOnClickListener { interactionListener?.onPlayPause(!isPlaying) } diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt index 61110e9d81..3dae1f5ed2 100644 --- a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt @@ -253,11 +253,9 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt handle(AttachmentCommands.SeekTo(percent)) } - // TODO add save feature for image => check it works for video as well, - // check if it is already possible to save from menu with long press on video override fun onShare() { - // TODO the opening of share bottom sheet is extremely slow - // move the retrieve of the file into ViewModel and use a ViewEvent to call shareMedia + // TODO + // use repeatOnLifecycle extension lifecycleScope.launch(Dispatchers.IO) { val file = currentSourceProvider?.getFileForSharing(currentPosition) ?: return@launch @@ -271,12 +269,17 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt } } - // TODO create suspendable usecase: downloadMediaUseCase - // create interface for base use case - // create ViewModel with action downloadAction, events downloading, downloaded, error - // call usecase using viewModel - // launch coroutine in Activity using repeatOnLifeCycle extension - // add unit tests for usecase? + // TODO add save feature for image => check it works for video as well, + // check if it is already possible to save from menu with long press on video + override fun onDownload() { + // TODO + // create interface for base use case + // create ViewModel with action downloadAction, events downloading, downloaded, error + // call usecase using viewModel + // launch coroutine in Activity using repeatOnLifeCycle extension + // add unit tests for usecase? + TODO("Not yet implemented") + } /* ========================================================================================== * COMPANION From 374ac45505d71e69cab507599f9fa77d0b349311 Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Wed, 23 Feb 2022 17:23:34 +0100 Subject: [PATCH 09/26] Interface for UseCase --- .../media/VectorAttachmentViewerActivity.kt | 1 - .../domain/usecase/DownloadMediaUseCase.kt | 10 ++++----- .../domain/usecase/VectorInOutUseCase.kt | 22 +++++++++++++++++++ 3 files changed, 27 insertions(+), 6 deletions(-) create mode 100644 vector/src/main/java/im/vector/app/features/media/domain/usecase/VectorInOutUseCase.kt diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt index 3dae1f5ed2..2c51bf0947 100644 --- a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt @@ -273,7 +273,6 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt // check if it is already possible to save from menu with long press on video override fun onDownload() { // TODO - // create interface for base use case // create ViewModel with action downloadAction, events downloading, downloaded, error // call usecase using viewModel // launch coroutine in Activity using repeatOnLifeCycle extension diff --git a/vector/src/main/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCase.kt b/vector/src/main/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCase.kt index e7b7d02f01..f493058281 100644 --- a/vector/src/main/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCase.kt @@ -30,20 +30,20 @@ import javax.inject.Inject class DownloadMediaUseCase @Inject constructor( @ApplicationContext private val appContext: Context, private val notificationUtils: NotificationUtils -) { +) : VectorInOutUseCase { /* ========================================================================================== * Public API * ========================================================================================== */ // TODO find a way to provide Dispatchers via Interface to be able to unit tests - suspend fun execute(file: File): Result = withContext(Dispatchers.IO) { + override suspend fun execute(input: File): Result = withContext(Dispatchers.IO) { runCatching { saveMedia( context = appContext, - file = file, - title = file.name, - mediaMimeType = getMimeTypeFromUri(appContext, file.toUri()), + file = input, + title = input.name, + mediaMimeType = getMimeTypeFromUri(appContext, input.toUri()), notificationUtils = notificationUtils ) } diff --git a/vector/src/main/java/im/vector/app/features/media/domain/usecase/VectorInOutUseCase.kt b/vector/src/main/java/im/vector/app/features/media/domain/usecase/VectorInOutUseCase.kt new file mode 100644 index 0000000000..d7c7585250 --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/media/domain/usecase/VectorInOutUseCase.kt @@ -0,0 +1,22 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.media.domain.usecase + +// TODO move into Core packages +interface VectorInOutUseCase { + suspend fun execute(input: T): Result +} From c6c46375d648c463c7e259b353555070b4e35387 Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Wed, 23 Feb 2022 17:54:21 +0100 Subject: [PATCH 10/26] Creating a ViewModel --- .../media/VectorAttachmentViewerAction.kt | 24 ++++++++++ .../media/VectorAttachmentViewerActivity.kt | 7 ++- .../media/VectorAttachmentViewerViewEvents.kt | 9 ++-- .../media/VectorAttachmentViewerViewModel.kt | 47 +++++++++++++++++++ 4 files changed, 82 insertions(+), 5 deletions(-) create mode 100644 vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerAction.kt rename library/attachment-viewer/src/main/java/im/vector/lib/attachmentviewer/AttachmentViewerViewModel.kt => vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewEvents.kt (63%) create mode 100644 vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerAction.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerAction.kt new file mode 100644 index 0000000000..8c9ca1e1ad --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerAction.kt @@ -0,0 +1,24 @@ +/* + * Copyright 2019 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.media + +import im.vector.app.core.platform.VectorViewModelAction +import java.io.File + +sealed class VectorAttachmentViewerAction : VectorViewModelAction { + data class DownloadMedia(val file: File) : VectorAttachmentViewerAction() +} diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt index 2c51bf0947..b37a0dd361 100644 --- a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt @@ -273,9 +273,12 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt // check if it is already possible to save from menu with long press on video override fun onDownload() { // TODO - // create ViewModel with action downloadAction, events downloading, downloaded, error - // call usecase using viewModel + // call usecase using viewModel into handle method // launch coroutine in Activity using repeatOnLifeCycle extension + // call ViewModel.handle inside this method + // show message on error event: see TimelineFragment + // check write file permissions: see TimelineFragment + // should we check if media is saveable? // add unit tests for usecase? TODO("Not yet implemented") } diff --git a/library/attachment-viewer/src/main/java/im/vector/lib/attachmentviewer/AttachmentViewerViewModel.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewEvents.kt similarity index 63% rename from library/attachment-viewer/src/main/java/im/vector/lib/attachmentviewer/AttachmentViewerViewModel.kt rename to vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewEvents.kt index fbb46d4348..8d0c376dd5 100644 --- a/library/attachment-viewer/src/main/java/im/vector/lib/attachmentviewer/AttachmentViewerViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewEvents.kt @@ -1,6 +1,5 @@ /* * Copyright (c) 2020 New Vector Ltd - * Copyright (C) 2018 stfalcon.com * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,8 +14,12 @@ * limitations under the License. */ -package im.vector.lib.attachmentviewer +package im.vector.app.features.media -class AttachmentViewerViewModel { +import im.vector.app.core.platform.VectorViewEvents +sealed class VectorAttachmentViewerViewEvents : VectorViewEvents { + object DownloadingMedia : VectorAttachmentViewerViewEvents() + object MediaDownloaded : VectorAttachmentViewerViewEvents() + object ErrorDownloadingMedia : VectorAttachmentViewerViewEvents() } diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt new file mode 100644 index 0000000000..e6d86d2e7c --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt @@ -0,0 +1,47 @@ +/* + * Copyright (c) 2020 New Vector Ltd + * Copyright (C) 2018 stfalcon.com + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.media + +import dagger.assisted.Assisted +import dagger.assisted.AssistedFactory +import dagger.assisted.AssistedInject +import im.vector.app.core.di.MavericksAssistedViewModelFactory +import im.vector.app.core.platform.VectorDummyViewState +import im.vector.app.core.platform.VectorViewModel + +class VectorAttachmentViewerViewModel @AssistedInject constructor( + @Assisted initialState: VectorDummyViewState +) : VectorViewModel(initialState) { + + /* ========================================================================================== + * Factory + * ========================================================================================== */ + + @AssistedFactory + interface Factory : MavericksAssistedViewModelFactory { + override fun create(initialState: VectorDummyViewState): VectorAttachmentViewerViewModel + } + + /* ========================================================================================== + * Specialization + * ========================================================================================== */ + + override fun handle(action: VectorAttachmentViewerAction) { + TODO("Not yet implemented") + } +} From b17ce12c3d5fde64dcdad1673efe7e7aba4ab491 Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Thu, 24 Feb 2022 09:31:26 +0100 Subject: [PATCH 11/26] Calling use case inside ViewModel --- .../media/VectorAttachmentViewerActivity.kt | 4 ++-- .../media/VectorAttachmentViewerViewEvents.kt | 1 - .../media/VectorAttachmentViewerViewModel.kt | 23 +++++++++++++++++-- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt index b37a0dd361..c1a38d35b6 100644 --- a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt @@ -269,8 +269,6 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt } } - // TODO add save feature for image => check it works for video as well, - // check if it is already possible to save from menu with long press on video override fun onDownload() { // TODO // call usecase using viewModel into handle method @@ -279,6 +277,8 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt // show message on error event: see TimelineFragment // check write file permissions: see TimelineFragment // should we check if media is saveable? + // check if it is already possible to save from menu with long press on video + // check if it works for video or other media type as well // add unit tests for usecase? TODO("Not yet implemented") } diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewEvents.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewEvents.kt index 8d0c376dd5..2b3266ee7e 100644 --- a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewEvents.kt +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewEvents.kt @@ -20,6 +20,5 @@ import im.vector.app.core.platform.VectorViewEvents sealed class VectorAttachmentViewerViewEvents : VectorViewEvents { object DownloadingMedia : VectorAttachmentViewerViewEvents() - object MediaDownloaded : VectorAttachmentViewerViewEvents() object ErrorDownloadingMedia : VectorAttachmentViewerViewEvents() } diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt index e6d86d2e7c..039977d573 100644 --- a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt @@ -23,9 +23,12 @@ import dagger.assisted.AssistedInject import im.vector.app.core.di.MavericksAssistedViewModelFactory import im.vector.app.core.platform.VectorDummyViewState import im.vector.app.core.platform.VectorViewModel +import im.vector.app.features.media.domain.usecase.DownloadMediaUseCase +import kotlinx.coroutines.launch class VectorAttachmentViewerViewModel @AssistedInject constructor( - @Assisted initialState: VectorDummyViewState + @Assisted initialState: VectorDummyViewState, + private val downloadMediaUseCase: DownloadMediaUseCase ) : VectorViewModel(initialState) { /* ========================================================================================== @@ -42,6 +45,22 @@ class VectorAttachmentViewerViewModel @AssistedInject constructor( * ========================================================================================== */ override fun handle(action: VectorAttachmentViewerAction) { - TODO("Not yet implemented") + when (action) { + is VectorAttachmentViewerAction.DownloadMedia -> handleDownloadAction(action) + } + } + + /* ========================================================================================== + * Private methods + * ========================================================================================== */ + + private fun handleDownloadAction(action: VectorAttachmentViewerAction.DownloadMedia) { + viewModelScope.launch { + _viewEvents.post(VectorAttachmentViewerViewEvents.DownloadingMedia) + + // Success event is handled via a notification inside use case + downloadMediaUseCase.execute(action.file) + .onFailure { _viewEvents.post(VectorAttachmentViewerViewEvents.ErrorDownloadingMedia) } + } } } From 7d7b1f305e43985b19ce4b6af9b2c0c4a4067624 Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Thu, 24 Feb 2022 09:41:32 +0100 Subject: [PATCH 12/26] Calling ViewModel inside Fragment --- .../media/VectorAttachmentViewerActivity.kt | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt index c1a38d35b6..abcab12bd7 100644 --- a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt @@ -30,6 +30,7 @@ import androidx.core.view.isInvisible import androidx.core.view.isVisible import androidx.lifecycle.lifecycleScope import androidx.transition.Transition +import com.airbnb.mvrx.viewModel import dagger.hilt.android.AndroidEntryPoint import im.vector.app.R import im.vector.app.core.di.ActiveSessionHolder @@ -66,8 +67,10 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt @Inject lateinit var sessionHolder: ActiveSessionHolder + @Inject lateinit var dataSourceFactory: AttachmentProviderFactory + @Inject lateinit var imageContentRenderer: ImageContentRenderer @@ -75,6 +78,7 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt * Fields * ========================================================================================== */ + private val viewModel: VectorAttachmentViewerViewModel by viewModel() private var initialIndex = 0 private var isAnimatingOut = false private var currentSourceProvider: BaseAttachmentProvider<*>? = null @@ -254,8 +258,6 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt } override fun onShare() { - // TODO - // use repeatOnLifecycle extension lifecycleScope.launch(Dispatchers.IO) { val file = currentSourceProvider?.getFileForSharing(currentPosition) ?: return@launch @@ -271,16 +273,17 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt override fun onDownload() { // TODO - // call usecase using viewModel into handle method - // launch coroutine in Activity using repeatOnLifeCycle extension - // call ViewModel.handle inside this method + // handle viewEvents // show message on error event: see TimelineFragment // check write file permissions: see TimelineFragment // should we check if media is saveable? // check if it is already possible to save from menu with long press on video // check if it works for video or other media type as well // add unit tests for usecase? - TODO("Not yet implemented") + lifecycleScope.launch(Dispatchers.IO) { + val file = currentSourceProvider?.getFileForSharing(currentPosition) ?: return@launch + viewModel.handle(VectorAttachmentViewerAction.DownloadMedia(file)) + } } /* ========================================================================================== From 73ac3f3fda96502533930f190a2acd9972bfa331 Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Thu, 24 Feb 2022 11:12:38 +0100 Subject: [PATCH 13/26] Fixing DI + observing events --- .../app/core/di/MavericksViewModelModule.kt | 6 +++++ .../media/VectorAttachmentViewerActivity.kt | 24 +++++++++++++++++-- .../media/VectorAttachmentViewerViewEvents.kt | 2 +- .../media/VectorAttachmentViewerViewModel.kt | 6 ++++- 4 files changed, 34 insertions(+), 4 deletions(-) diff --git a/vector/src/main/java/im/vector/app/core/di/MavericksViewModelModule.kt b/vector/src/main/java/im/vector/app/core/di/MavericksViewModelModule.kt index 2cd7136ffc..33afcf1dfb 100644 --- a/vector/src/main/java/im/vector/app/core/di/MavericksViewModelModule.kt +++ b/vector/src/main/java/im/vector/app/core/di/MavericksViewModelModule.kt @@ -58,6 +58,7 @@ import im.vector.app.features.login.LoginViewModel import im.vector.app.features.login2.LoginViewModel2 import im.vector.app.features.login2.created.AccountCreatedViewModel import im.vector.app.features.matrixto.MatrixToBottomSheetViewModel +import im.vector.app.features.media.VectorAttachmentViewerViewModel import im.vector.app.features.onboarding.OnboardingViewModel import im.vector.app.features.poll.create.CreatePollViewModel import im.vector.app.features.qrcode.QrCodeScannerViewModel @@ -594,4 +595,9 @@ interface MavericksViewModelModule { @IntoMap @MavericksViewModelKey(LocationSharingViewModel::class) fun createLocationSharingViewModelFactory(factory: LocationSharingViewModel.Factory): MavericksAssistedViewModelFactory<*, *> + + @Binds + @IntoMap + @MavericksViewModelKey(VectorAttachmentViewerViewModel::class) + fun vectorAttachmentViewerViewModelFactory(factory: VectorAttachmentViewerViewModel.Factory): MavericksAssistedViewModelFactory<*, *> } diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt index abcab12bd7..f051c63828 100644 --- a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt @@ -41,6 +41,8 @@ import im.vector.app.features.themes.ThemeUtils import im.vector.lib.attachmentviewer.AttachmentCommands import im.vector.lib.attachmentviewer.AttachmentViewerActivity import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import kotlinx.parcelize.Parcelize @@ -147,6 +149,8 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt window.statusBarColor = ContextCompat.getColor(this, R.color.black_alpha) window.navigationBarColor = ContextCompat.getColor(this, R.color.black_alpha) + + observeViewEvents() } override fun onResume() { @@ -241,6 +245,23 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt }) } + private fun observeViewEvents() { + viewModel.viewEvents + .stream() + .onEach(::handleViewEvents) + .launchIn(lifecycleScope) + } + + private fun handleViewEvents(event: VectorAttachmentViewerViewEvents) { + when (event) { + is VectorAttachmentViewerViewEvents.DownloadingMedia -> Unit // TODO show loader? + is VectorAttachmentViewerViewEvents.ErrorDownloadingMedia -> { + // TODO show snackbar + Timber.e("failure saving file: ${event.error}") + } + } + } + /* ========================================================================================== * Specialization AttachmentInteractionListener * ========================================================================================== */ @@ -273,13 +294,12 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt override fun onDownload() { // TODO - // handle viewEvents // show message on error event: see TimelineFragment // check write file permissions: see TimelineFragment // should we check if media is saveable? // check if it is already possible to save from menu with long press on video // check if it works for video or other media type as well - // add unit tests for usecase? + // add unit tests for usecase? what is the used mock library? lifecycleScope.launch(Dispatchers.IO) { val file = currentSourceProvider?.getFileForSharing(currentPosition) ?: return@launch viewModel.handle(VectorAttachmentViewerAction.DownloadMedia(file)) diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewEvents.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewEvents.kt index 2b3266ee7e..6516da9dcc 100644 --- a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewEvents.kt +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewEvents.kt @@ -20,5 +20,5 @@ import im.vector.app.core.platform.VectorViewEvents sealed class VectorAttachmentViewerViewEvents : VectorViewEvents { object DownloadingMedia : VectorAttachmentViewerViewEvents() - object ErrorDownloadingMedia : VectorAttachmentViewerViewEvents() + data class ErrorDownloadingMedia(val error: Throwable) : VectorAttachmentViewerViewEvents() } diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt index 039977d573..9e311c6a1f 100644 --- a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt @@ -17,10 +17,12 @@ package im.vector.app.features.media +import com.airbnb.mvrx.MavericksViewModelFactory import dagger.assisted.Assisted import dagger.assisted.AssistedFactory import dagger.assisted.AssistedInject import im.vector.app.core.di.MavericksAssistedViewModelFactory +import im.vector.app.core.di.hiltMavericksViewModelFactory import im.vector.app.core.platform.VectorDummyViewState import im.vector.app.core.platform.VectorViewModel import im.vector.app.features.media.domain.usecase.DownloadMediaUseCase @@ -40,6 +42,8 @@ class VectorAttachmentViewerViewModel @AssistedInject constructor( override fun create(initialState: VectorDummyViewState): VectorAttachmentViewerViewModel } + companion object : MavericksViewModelFactory by hiltMavericksViewModelFactory() + /* ========================================================================================== * Specialization * ========================================================================================== */ @@ -60,7 +64,7 @@ class VectorAttachmentViewerViewModel @AssistedInject constructor( // Success event is handled via a notification inside use case downloadMediaUseCase.execute(action.file) - .onFailure { _viewEvents.post(VectorAttachmentViewerViewEvents.ErrorDownloadingMedia) } + .onFailure { _viewEvents.post(VectorAttachmentViewerViewEvents.ErrorDownloadingMedia(it)) } } } } From cdb1a96664200fce5b77a4c69ccd0d9d0e9dfb2a Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Thu, 24 Feb 2022 15:41:23 +0100 Subject: [PATCH 14/26] Removing TODOs --- .../home/room/detail/timeline/action/MessageActionsViewModel.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt index 6acaedfd3c..8ca3fca9b9 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt @@ -554,7 +554,6 @@ class MessageActionsViewModel @AssistedInject constructor(@Assisted } } - // TODO need to add this check into media viewer? private fun canShare(msgType: String?): Boolean { return when (msgType) { MessageType.MSGTYPE_TEXT, @@ -569,7 +568,6 @@ class MessageActionsViewModel @AssistedInject constructor(@Assisted } } - // TODO need to add this into Viewer screen to check if it is saveable? => create extension on MessageType or useCase? private fun canSave(msgType: String?): Boolean { return when (msgType) { MessageType.MSGTYPE_IMAGE, From 4c09fb747b3e1df54c895d13880623191b0a5630 Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Thu, 24 Feb 2022 15:47:57 +0100 Subject: [PATCH 15/26] Moving base use case interface into core package --- .../usecase/VectorBaseInOutUseCase.kt} | 5 ++--- .../features/media/domain/usecase/DownloadMediaUseCase.kt | 5 +++-- 2 files changed, 5 insertions(+), 5 deletions(-) rename vector/src/main/java/im/vector/app/{features/media/domain/usecase/VectorInOutUseCase.kt => core/usecase/VectorBaseInOutUseCase.kt} (84%) diff --git a/vector/src/main/java/im/vector/app/features/media/domain/usecase/VectorInOutUseCase.kt b/vector/src/main/java/im/vector/app/core/usecase/VectorBaseInOutUseCase.kt similarity index 84% rename from vector/src/main/java/im/vector/app/features/media/domain/usecase/VectorInOutUseCase.kt rename to vector/src/main/java/im/vector/app/core/usecase/VectorBaseInOutUseCase.kt index d7c7585250..277da6794a 100644 --- a/vector/src/main/java/im/vector/app/features/media/domain/usecase/VectorInOutUseCase.kt +++ b/vector/src/main/java/im/vector/app/core/usecase/VectorBaseInOutUseCase.kt @@ -14,9 +14,8 @@ * limitations under the License. */ -package im.vector.app.features.media.domain.usecase +package im.vector.app.core.usecase -// TODO move into Core packages -interface VectorInOutUseCase { +interface VectorBaseInOutUseCase { suspend fun execute(input: T): Result } diff --git a/vector/src/main/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCase.kt b/vector/src/main/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCase.kt index f493058281..6da4e5b419 100644 --- a/vector/src/main/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCase.kt @@ -20,6 +20,7 @@ import android.content.Context import androidx.core.net.toUri import dagger.hilt.android.qualifiers.ApplicationContext import im.vector.app.core.intent.getMimeTypeFromUri +import im.vector.app.core.usecase.VectorBaseInOutUseCase import im.vector.app.core.utils.saveMedia import im.vector.app.features.notifications.NotificationUtils import kotlinx.coroutines.Dispatchers @@ -30,13 +31,13 @@ import javax.inject.Inject class DownloadMediaUseCase @Inject constructor( @ApplicationContext private val appContext: Context, private val notificationUtils: NotificationUtils -) : VectorInOutUseCase { +) : VectorBaseInOutUseCase { /* ========================================================================================== * Public API * ========================================================================================== */ - // TODO find a way to provide Dispatchers via Interface to be able to unit tests + // TODO declare Dispatcher via an Interface provider to be able to unit tests override suspend fun execute(input: File): Result = withContext(Dispatchers.IO) { runCatching { saveMedia( From 882b14356914363b14c0c4a89ba57e2cc79a5be6 Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Thu, 24 Feb 2022 16:15:53 +0100 Subject: [PATCH 16/26] Permission and error handling --- .../AttachmentViewerActivity.kt | 11 +++-- .../home/room/detail/TimelineFragment.kt | 1 - .../media/VectorAttachmentViewerActivity.kt | 48 +++++++++++++++---- .../media/VectorAttachmentViewerViewEvents.kt | 1 - .../media/VectorAttachmentViewerViewModel.kt | 10 ++-- 5 files changed, 53 insertions(+), 18 deletions(-) diff --git a/library/attachment-viewer/src/main/java/im/vector/lib/attachmentviewer/AttachmentViewerActivity.kt b/library/attachment-viewer/src/main/java/im/vector/lib/attachmentviewer/AttachmentViewerActivity.kt index 573138bf5c..21af114c26 100644 --- a/library/attachment-viewer/src/main/java/im/vector/lib/attachmentviewer/AttachmentViewerActivity.kt +++ b/library/attachment-viewer/src/main/java/im/vector/lib/attachmentviewer/AttachmentViewerActivity.kt @@ -45,6 +45,8 @@ import kotlin.math.abs abstract class AttachmentViewerActivity : AppCompatActivity(), AttachmentEventListener { + protected val rootView: View + get() = views.rootContainer protected val pager2: ViewPager2 get() = views.attachmentPager protected val imageTransitionView: ImageView @@ -298,10 +300,11 @@ abstract class AttachmentViewerActivity : AppCompatActivity(), AttachmentEventLi private fun createSwipeToDismissHandler(): SwipeToDismissHandler = SwipeToDismissHandler( - swipeView = views.dismissContainer, - shouldAnimateDismiss = { shouldAnimateDismiss() }, - onDismiss = { animateClose() }, - onSwipeViewMove = ::handleSwipeViewMove) + swipeView = views.dismissContainer, + shouldAnimateDismiss = { shouldAnimateDismiss() }, + onDismiss = { animateClose() }, + onSwipeViewMove = ::handleSwipeViewMove + ) private fun createSwipeDirectionDetector() = SwipeDirectionDetector(this) { swipeDirection = it } diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt index 9ca2e28646..1a40018526 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt @@ -2111,7 +2111,6 @@ class TimelineFragment @Inject constructor( } } - // TODO mutualize permission checking creating activity extension or delegation interface? private fun onSaveActionClicked(action: EventSharedAction.Save) { if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q && !checkPermissions(PERMISSIONS_FOR_WRITING_FILES, requireActivity(), saveActionActivityResultLauncher)) { diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt index f051c63828..a52e0f433b 100644 --- a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt @@ -17,6 +17,7 @@ package im.vector.app.features.media import android.content.Context import android.content.Intent +import android.os.Build import android.os.Bundle import android.os.Parcelable import android.view.View @@ -34,7 +35,13 @@ import com.airbnb.mvrx.viewModel import dagger.hilt.android.AndroidEntryPoint import im.vector.app.R import im.vector.app.core.di.ActiveSessionHolder +import im.vector.app.core.extensions.singletonEntryPoint import im.vector.app.core.intent.getMimeTypeFromUri +import im.vector.app.core.platform.showOptimizedSnackbar +import im.vector.app.core.utils.PERMISSIONS_FOR_WRITING_FILES +import im.vector.app.core.utils.checkPermissions +import im.vector.app.core.utils.onPermissionDeniedDialog +import im.vector.app.core.utils.registerForPermissionsResult import im.vector.app.core.utils.shareMedia import im.vector.app.features.themes.ActivityOtherThemes import im.vector.app.features.themes.ThemeUtils @@ -81,9 +88,20 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt * ========================================================================================== */ private val viewModel: VectorAttachmentViewerViewModel by viewModel() + private val errorFormatter by lazy(LazyThreadSafetyMode.NONE) { singletonEntryPoint().errorFormatter() } private var initialIndex = 0 private var isAnimatingOut = false private var currentSourceProvider: BaseAttachmentProvider<*>? = null + private val downloadActionResultLauncher = registerForPermissionsResult { allGranted, deniedPermanently -> + if (allGranted) { + viewModel.pendingAction?.let { + viewModel.handle(it) + } + } else if (deniedPermanently) { + onPermissionDeniedDialog(R.string.denied_permission_generic) + } + viewModel.pendingAction = null + } /* ========================================================================================== * Lifecycle @@ -254,14 +272,18 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt private fun handleViewEvents(event: VectorAttachmentViewerViewEvents) { when (event) { - is VectorAttachmentViewerViewEvents.DownloadingMedia -> Unit // TODO show loader? - is VectorAttachmentViewerViewEvents.ErrorDownloadingMedia -> { - // TODO show snackbar - Timber.e("failure saving file: ${event.error}") - } + is VectorAttachmentViewerViewEvents.ErrorDownloadingMedia -> showSnackBarError(event.error) } } + private fun showSnackBarError(error: Throwable) { + rootView.showOptimizedSnackbar(errorFormatter.toHumanReadable(error)) + } + + private fun hasWritePermission() = + Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q || + checkPermissions(PERMISSIONS_FOR_WRITING_FILES, this, downloadActionResultLauncher) + /* ========================================================================================== * Specialization AttachmentInteractionListener * ========================================================================================== */ @@ -294,15 +316,23 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt override fun onDownload() { // TODO - // show message on error event: see TimelineFragment - // check write file permissions: see TimelineFragment - // should we check if media is saveable? + // test snackbar error in OnCreate() + // test write permission checking with Android 9 // check if it is already possible to save from menu with long press on video // check if it works for video or other media type as well + // reorder action for a message according to issue requirements // add unit tests for usecase? what is the used mock library? lifecycleScope.launch(Dispatchers.IO) { + val hasWritePermission = withContext(Dispatchers.Main) { + hasWritePermission() + } + val file = currentSourceProvider?.getFileForSharing(currentPosition) ?: return@launch - viewModel.handle(VectorAttachmentViewerAction.DownloadMedia(file)) + if (hasWritePermission) { + viewModel.handle(VectorAttachmentViewerAction.DownloadMedia(file)) + } else { + viewModel.pendingAction = VectorAttachmentViewerAction.DownloadMedia(file) + } } } diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewEvents.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewEvents.kt index 6516da9dcc..29181ee31f 100644 --- a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewEvents.kt +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewEvents.kt @@ -19,6 +19,5 @@ package im.vector.app.features.media import im.vector.app.core.platform.VectorViewEvents sealed class VectorAttachmentViewerViewEvents : VectorViewEvents { - object DownloadingMedia : VectorAttachmentViewerViewEvents() data class ErrorDownloadingMedia(val error: Throwable) : VectorAttachmentViewerViewEvents() } diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt index 9e311c6a1f..b2bbc79e4f 100644 --- a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt @@ -44,6 +44,12 @@ class VectorAttachmentViewerViewModel @AssistedInject constructor( companion object : MavericksViewModelFactory by hiltMavericksViewModelFactory() + /* ========================================================================================== + * Public Api + * ========================================================================================== */ + + var pendingAction: VectorAttachmentViewerAction? = null + /* ========================================================================================== * Specialization * ========================================================================================== */ @@ -60,9 +66,7 @@ class VectorAttachmentViewerViewModel @AssistedInject constructor( private fun handleDownloadAction(action: VectorAttachmentViewerAction.DownloadMedia) { viewModelScope.launch { - _viewEvents.post(VectorAttachmentViewerViewEvents.DownloadingMedia) - - // Success event is handled via a notification inside use case + // Success event is handled via a notification inside the use case downloadMediaUseCase.execute(action.file) .onFailure { _viewEvents.post(VectorAttachmentViewerViewEvents.ErrorDownloadingMedia(it)) } } From 4260d2f155bfc0c6ffb3bf4480989b5d445eefe2 Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Thu, 24 Feb 2022 16:17:42 +0100 Subject: [PATCH 17/26] Updating Changelog entry --- changelog.d/5005.feature | 2 +- .../app/features/media/VectorAttachmentViewerActivity.kt | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/changelog.d/5005.feature b/changelog.d/5005.feature index 2db648cd6a..ce3b2ad1f9 100644 --- a/changelog.d/5005.feature +++ b/changelog.d/5005.feature @@ -1 +1 @@ -Add possibility to save image in Gallery + reorder choices in message context menu +Add possibility to save media from Gallery + reorder choices in message context menu diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt index a52e0f433b..7f9037e31e 100644 --- a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt @@ -316,10 +316,6 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt override fun onDownload() { // TODO - // test snackbar error in OnCreate() - // test write permission checking with Android 9 - // check if it is already possible to save from menu with long press on video - // check if it works for video or other media type as well // reorder action for a message according to issue requirements // add unit tests for usecase? what is the used mock library? lifecycleScope.launch(Dispatchers.IO) { From a583db43c40e181e4dd9b44595f0c3b5cf008507 Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Thu, 24 Feb 2022 17:04:13 +0100 Subject: [PATCH 18/26] Updating TODOs --- .../room/detail/timeline/action/MessageActionsViewModel.kt | 1 + .../app/features/media/VectorAttachmentViewerActivity.kt | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt index 8ca3fca9b9..ea802554d2 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt @@ -252,6 +252,7 @@ class MessageActionsViewModel @AssistedInject constructor(@Assisted return arrayListOf().apply { // TODO need to check all possible items and confirm order changes to apply + // reorder actions according to issue requirements: check if same order in Web when { timelineEvent.root.sendState.hasFailed() -> { addActionsForFailedState(timelineEvent, actionPermissions, messageContent, msgType) diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt index 7f9037e31e..72089d0f39 100644 --- a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt @@ -315,9 +315,6 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt } override fun onDownload() { - // TODO - // reorder action for a message according to issue requirements - // add unit tests for usecase? what is the used mock library? lifecycleScope.launch(Dispatchers.IO) { val hasWritePermission = withContext(Dispatchers.Main) { hasWritePermission() From 157feb1e4ca3cf3e16f4e4ebff6586ee747f0d7e Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Thu, 24 Feb 2022 17:38:30 +0100 Subject: [PATCH 19/26] Updating order of message actions --- .../action/MessageActionsViewModel.kt | 42 +++++++++---------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt index ea802554d2..5575d9b7f6 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt @@ -251,8 +251,6 @@ class MessageActionsViewModel @AssistedInject constructor(@Assisted val msgType = messageContent?.msgType return arrayListOf().apply { - // TODO need to check all possible items and confirm order changes to apply - // reorder actions according to issue requirements: check if same order in Web when { timelineEvent.root.sendState.hasFailed() -> { addActionsForFailedState(timelineEvent, actionPermissions, messageContent, msgType) @@ -345,24 +343,6 @@ class MessageActionsViewModel @AssistedInject constructor(@Assisted add(EventSharedAction.Edit(eventId, timelineEvent.root.getClearType())) } - if (canRedact(timelineEvent, actionPermissions)) { - if (timelineEvent.root.getClearType() == EventType.POLL_START) { - add(EventSharedAction.Redact( - eventId, - askForReason = informationData.senderId != session.myUserId, - dialogTitleRes = R.string.delete_poll_dialog_title, - dialogDescriptionRes = R.string.delete_poll_dialog_content - )) - } else { - add(EventSharedAction.Redact( - eventId, - askForReason = informationData.senderId != session.myUserId, - dialogTitleRes = R.string.delete_event_dialog_title, - dialogDescriptionRes = R.string.delete_event_dialog_content - )) - } - } - if (canCopy(msgType)) { // TODO copy images? html? see ClipBoard add(EventSharedAction.Copy(messageContent!!.body)) @@ -384,12 +364,30 @@ class MessageActionsViewModel @AssistedInject constructor(@Assisted add(EventSharedAction.ViewEditHistory(informationData)) } + if (canSave(msgType) && messageContent is MessageWithAttachmentContent) { + add(EventSharedAction.Save(timelineEvent.eventId, messageContent)) + } + if (canShare(msgType)) { add(EventSharedAction.Share(timelineEvent.eventId, messageContent!!)) } - if (canSave(msgType) && messageContent is MessageWithAttachmentContent) { - add(EventSharedAction.Save(timelineEvent.eventId, messageContent)) + if (canRedact(timelineEvent, actionPermissions)) { + if (timelineEvent.root.getClearType() == EventType.POLL_START) { + add(EventSharedAction.Redact( + eventId, + askForReason = informationData.senderId != session.myUserId, + dialogTitleRes = R.string.delete_poll_dialog_title, + dialogDescriptionRes = R.string.delete_poll_dialog_content + )) + } else { + add(EventSharedAction.Redact( + eventId, + askForReason = informationData.senderId != session.myUserId, + dialogTitleRes = R.string.delete_event_dialog_title, + dialogDescriptionRes = R.string.delete_event_dialog_content + )) + } } } From aea78b70f1bc7cae9646ccaae9d1e84c3026341a Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Thu, 24 Feb 2022 18:08:42 +0100 Subject: [PATCH 20/26] Changing usage of viewModelScope to Session scope --- .../app/features/media/VectorAttachmentViewerViewModel.kt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt index b2bbc79e4f..9c40cd7610 100644 --- a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt @@ -26,10 +26,13 @@ import im.vector.app.core.di.hiltMavericksViewModelFactory import im.vector.app.core.platform.VectorDummyViewState import im.vector.app.core.platform.VectorViewModel import im.vector.app.features.media.domain.usecase.DownloadMediaUseCase +import im.vector.app.features.session.coroutineScope import kotlinx.coroutines.launch +import org.matrix.android.sdk.api.session.Session class VectorAttachmentViewerViewModel @AssistedInject constructor( @Assisted initialState: VectorDummyViewState, + private val session: Session, private val downloadMediaUseCase: DownloadMediaUseCase ) : VectorViewModel(initialState) { @@ -65,7 +68,8 @@ class VectorAttachmentViewerViewModel @AssistedInject constructor( * ========================================================================================== */ private fun handleDownloadAction(action: VectorAttachmentViewerAction.DownloadMedia) { - viewModelScope.launch { + // launch in the coroutine scope session to avoid binding the coroutine to the lifecycle of the VM + session.coroutineScope.launch { // Success event is handled via a notification inside the use case downloadMediaUseCase.execute(action.file) .onFailure { _viewEvents.post(VectorAttachmentViewerViewEvents.ErrorDownloadingMedia(it)) } From 4e4702cad8930db8574114a7ba769204844e3aeb Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Fri, 25 Feb 2022 10:56:38 +0100 Subject: [PATCH 21/26] Fixing date of file creation --- .../vector/app/features/media/VectorAttachmentViewerAction.kt | 4 ++-- .../app/features/media/VectorAttachmentViewerViewEvents.kt | 2 +- .../app/features/media/VectorAttachmentViewerViewModel.kt | 3 +-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerAction.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerAction.kt index 8c9ca1e1ad..5af3cd193a 100644 --- a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerAction.kt +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerAction.kt @@ -1,11 +1,11 @@ /* - * Copyright 2019 New Vector Ltd + * Copyright (c) 2022 New Vector Ltd * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewEvents.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewEvents.kt index 29181ee31f..e46ee02155 100644 --- a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewEvents.kt +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewEvents.kt @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020 New Vector Ltd + * Copyright (c) 2022 New Vector Ltd * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt index 9c40cd7610..caabcc1de3 100644 --- a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt @@ -1,6 +1,5 @@ /* - * Copyright (c) 2020 New Vector Ltd - * Copyright (C) 2018 stfalcon.com + * Copyright (c) 2022 New Vector Ltd * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. From 6230dfc641e81dc2039df189354c7cc5a5d60a05 Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Fri, 25 Feb 2022 12:01:06 +0100 Subject: [PATCH 22/26] Removing section bloc comments --- .../features/media/AttachmentOverlayView.kt | 16 ---------- .../media/VectorAttachmentViewerActivity.kt | 32 ------------------- .../media/VectorAttachmentViewerViewModel.kt | 16 ---------- .../domain/usecase/DownloadMediaUseCase.kt | 13 ++++---- 4 files changed, 7 insertions(+), 70 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/media/AttachmentOverlayView.kt b/vector/src/main/java/im/vector/app/features/media/AttachmentOverlayView.kt index 0b3407c5a4..58d10d2f2d 100644 --- a/vector/src/main/java/im/vector/app/features/media/AttachmentOverlayView.kt +++ b/vector/src/main/java/im/vector/app/features/media/AttachmentOverlayView.kt @@ -30,20 +30,12 @@ class AttachmentOverlayView @JvmOverloads constructor( context: Context, attrs: AttributeSet? = null, defStyleAttr: Int = 0 ) : ConstraintLayout(context, attrs, defStyleAttr), AttachmentEventListener { - /* ========================================================================================== - * Fields - * ========================================================================================== */ - var interactionListener: AttachmentInteractionListener? = null val views: MergeImageAttachmentOverlayBinding private var isPlaying = false private var suspendSeekBarUpdate = false - /* ========================================================================================== - * Init - * ========================================================================================== */ - init { inflate(context, R.layout.merge_image_attachment_overlay, this) views = MergeImageAttachmentOverlayBinding.bind(this) @@ -78,19 +70,11 @@ class AttachmentOverlayView @JvmOverloads constructor( }) } - /* ========================================================================================== - * Public API - * ========================================================================================== */ - fun updateWith(counter: String, senderInfo: String) { views.overlayCounterText.text = counter views.overlayInfoText.text = senderInfo } - /* ========================================================================================== - * Specialization - * ========================================================================================== */ - override fun onEvent(event: AttachmentEvents) { when (event) { is AttachmentEvents.VideoEvent -> { diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt index 72089d0f39..d8c2b83f9b 100644 --- a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerActivity.kt @@ -59,10 +59,6 @@ import javax.inject.Inject @AndroidEntryPoint class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInteractionListener { - /* ========================================================================================== - * Arguments - * ========================================================================================== */ - @Parcelize data class Args( val roomId: String?, @@ -70,10 +66,6 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt val sharedTransitionName: String? ) : Parcelable - /* ========================================================================================== - * Dependencies - * ========================================================================================== */ - @Inject lateinit var sessionHolder: ActiveSessionHolder @@ -83,10 +75,6 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt @Inject lateinit var imageContentRenderer: ImageContentRenderer - /* ========================================================================================== - * Fields - * ========================================================================================== */ - private val viewModel: VectorAttachmentViewerViewModel by viewModel() private val errorFormatter by lazy(LazyThreadSafetyMode.NONE) { singletonEntryPoint().errorFormatter() } private var initialIndex = 0 @@ -103,10 +91,6 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt viewModel.pendingAction = null } - /* ========================================================================================== - * Lifecycle - * ========================================================================================== */ - override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) Timber.i("onCreate Activity ${javaClass.simpleName}") @@ -191,10 +175,6 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt super.onBackPressed() } - /* ========================================================================================== - * Specialization - * ========================================================================================== */ - override fun shouldAnimateDismiss(): Boolean { return currentPosition != initialIndex } @@ -209,10 +189,6 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt ActivityCompat.finishAfterTransition(this) } - /* ========================================================================================== - * Private methods - * ========================================================================================== */ - private fun getOtherThemes() = ActivityOtherThemes.VectorAttachmentsPreview /** @@ -284,10 +260,6 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q || checkPermissions(PERMISSIONS_FOR_WRITING_FILES, this, downloadActionResultLauncher) - /* ========================================================================================== - * Specialization AttachmentInteractionListener - * ========================================================================================== */ - override fun onDismiss() { animateClose() } @@ -329,10 +301,6 @@ class VectorAttachmentViewerActivity : AttachmentViewerActivity(), AttachmentInt } } - /* ========================================================================================== - * COMPANION - * ========================================================================================== */ - companion object { private const val EXTRA_ARGS = "EXTRA_ARGS" private const val EXTRA_IMAGE_DATA = "EXTRA_IMAGE_DATA" diff --git a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt index caabcc1de3..807c69caff 100644 --- a/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/media/VectorAttachmentViewerViewModel.kt @@ -35,10 +35,6 @@ class VectorAttachmentViewerViewModel @AssistedInject constructor( private val downloadMediaUseCase: DownloadMediaUseCase ) : VectorViewModel(initialState) { - /* ========================================================================================== - * Factory - * ========================================================================================== */ - @AssistedFactory interface Factory : MavericksAssistedViewModelFactory { override fun create(initialState: VectorDummyViewState): VectorAttachmentViewerViewModel @@ -46,26 +42,14 @@ class VectorAttachmentViewerViewModel @AssistedInject constructor( companion object : MavericksViewModelFactory by hiltMavericksViewModelFactory() - /* ========================================================================================== - * Public Api - * ========================================================================================== */ - var pendingAction: VectorAttachmentViewerAction? = null - /* ========================================================================================== - * Specialization - * ========================================================================================== */ - override fun handle(action: VectorAttachmentViewerAction) { when (action) { is VectorAttachmentViewerAction.DownloadMedia -> handleDownloadAction(action) } } - /* ========================================================================================== - * Private methods - * ========================================================================================== */ - private fun handleDownloadAction(action: VectorAttachmentViewerAction.DownloadMedia) { // launch in the coroutine scope session to avoid binding the coroutine to the lifecycle of the VM session.coroutineScope.launch { diff --git a/vector/src/main/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCase.kt b/vector/src/main/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCase.kt index 6da4e5b419..aca5661a8f 100644 --- a/vector/src/main/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCase.kt @@ -23,22 +23,23 @@ import im.vector.app.core.intent.getMimeTypeFromUri import im.vector.app.core.usecase.VectorBaseInOutUseCase import im.vector.app.core.utils.saveMedia import im.vector.app.features.notifications.NotificationUtils -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext +import org.matrix.android.sdk.api.MatrixCoroutineDispatchers import java.io.File import javax.inject.Inject class DownloadMediaUseCase @Inject constructor( @ApplicationContext private val appContext: Context, + private val coroutineDispatchers: MatrixCoroutineDispatchers, private val notificationUtils: NotificationUtils ) : VectorBaseInOutUseCase { - /* ========================================================================================== - * Public API - * ========================================================================================== */ + // TODO + // what about UseCase Interface enforcing single type input? => no interface + // add unit tests + // PR to template structure of a class for discussion - // TODO declare Dispatcher via an Interface provider to be able to unit tests - override suspend fun execute(input: File): Result = withContext(Dispatchers.IO) { + override suspend fun execute(input: File): Result = withContext(coroutineDispatchers.io) { runCatching { saveMedia( context = appContext, From cb27608c754ede6c8649d0d7aefecf2a6b75f9de Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Fri, 25 Feb 2022 14:11:26 +0100 Subject: [PATCH 23/26] Removing base use case interface --- .../core/usecase/VectorBaseInOutUseCase.kt | 21 ------------------- .../domain/usecase/DownloadMediaUseCase.kt | 11 +++------- 2 files changed, 3 insertions(+), 29 deletions(-) delete mode 100644 vector/src/main/java/im/vector/app/core/usecase/VectorBaseInOutUseCase.kt diff --git a/vector/src/main/java/im/vector/app/core/usecase/VectorBaseInOutUseCase.kt b/vector/src/main/java/im/vector/app/core/usecase/VectorBaseInOutUseCase.kt deleted file mode 100644 index 277da6794a..0000000000 --- a/vector/src/main/java/im/vector/app/core/usecase/VectorBaseInOutUseCase.kt +++ /dev/null @@ -1,21 +0,0 @@ -/* - * Copyright (c) 2022 New Vector Ltd - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package im.vector.app.core.usecase - -interface VectorBaseInOutUseCase { - suspend fun execute(input: T): Result -} diff --git a/vector/src/main/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCase.kt b/vector/src/main/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCase.kt index aca5661a8f..0f44b02143 100644 --- a/vector/src/main/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCase.kt @@ -20,7 +20,6 @@ import android.content.Context import androidx.core.net.toUri import dagger.hilt.android.qualifiers.ApplicationContext import im.vector.app.core.intent.getMimeTypeFromUri -import im.vector.app.core.usecase.VectorBaseInOutUseCase import im.vector.app.core.utils.saveMedia import im.vector.app.features.notifications.NotificationUtils import kotlinx.coroutines.withContext @@ -32,14 +31,10 @@ class DownloadMediaUseCase @Inject constructor( @ApplicationContext private val appContext: Context, private val coroutineDispatchers: MatrixCoroutineDispatchers, private val notificationUtils: NotificationUtils -) : VectorBaseInOutUseCase { +) { - // TODO - // what about UseCase Interface enforcing single type input? => no interface - // add unit tests - // PR to template structure of a class for discussion - - override suspend fun execute(input: File): Result = withContext(coroutineDispatchers.io) { + // TODO add unit tests: https://github.com/vector-im/element-android/tree/develop/vector/src/test/java/im/vector/app/test + suspend fun execute(input: File): Result = withContext(coroutineDispatchers.io) { runCatching { saveMedia( context = appContext, From 56c6301151deaf59d858d84cf4f2f47d9bc6549e Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Fri, 25 Feb 2022 16:39:26 +0100 Subject: [PATCH 24/26] Adding unit tests --- .../domain/usecase/DownloadMediaUseCase.kt | 7 +- .../usecase/DownloadMediaUseCaseTest.kt | 136 ++++++++++++++++++ 2 files changed, 139 insertions(+), 4 deletions(-) create mode 100644 vector/src/test/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCaseTest.kt diff --git a/vector/src/main/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCase.kt b/vector/src/main/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCase.kt index 0f44b02143..b0401ccd30 100644 --- a/vector/src/main/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCase.kt @@ -23,18 +23,17 @@ import im.vector.app.core.intent.getMimeTypeFromUri import im.vector.app.core.utils.saveMedia import im.vector.app.features.notifications.NotificationUtils import kotlinx.coroutines.withContext -import org.matrix.android.sdk.api.MatrixCoroutineDispatchers +import org.matrix.android.sdk.api.session.Session import java.io.File import javax.inject.Inject class DownloadMediaUseCase @Inject constructor( @ApplicationContext private val appContext: Context, - private val coroutineDispatchers: MatrixCoroutineDispatchers, + private val session: Session, private val notificationUtils: NotificationUtils ) { - // TODO add unit tests: https://github.com/vector-im/element-android/tree/develop/vector/src/test/java/im/vector/app/test - suspend fun execute(input: File): Result = withContext(coroutineDispatchers.io) { + suspend fun execute(input: File): Result = withContext(session.coroutineDispatchers.io) { runCatching { saveMedia( context = appContext, diff --git a/vector/src/test/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCaseTest.kt b/vector/src/test/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCaseTest.kt new file mode 100644 index 0000000000..4ad8ca7fce --- /dev/null +++ b/vector/src/test/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCaseTest.kt @@ -0,0 +1,136 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.media.domain.usecase + +import android.content.Context +import android.net.Uri +import androidx.core.net.toUri +import com.airbnb.mvrx.test.MvRxTestRule +import im.vector.app.core.intent.getMimeTypeFromUri +import im.vector.app.core.utils.saveMedia +import im.vector.app.features.notifications.NotificationUtils +import im.vector.app.test.fakes.FakeSession +import io.mockk.MockKAnnotations +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.every +import io.mockk.impl.annotations.MockK +import io.mockk.impl.annotations.OverrideMockKs +import io.mockk.just +import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.runs +import io.mockk.unmockkStatic +import io.mockk.verify +import io.mockk.verifyAll +import kotlinx.coroutines.test.runBlockingTest +import org.junit.After +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import java.io.File + +class DownloadMediaUseCaseTest { + + @get:Rule + val mvRxTestRule = MvRxTestRule() + + @MockK + lateinit var appContext: Context + + private val session = FakeSession() + + @MockK + lateinit var notificationUtils: NotificationUtils + + @OverrideMockKs + lateinit var downloadMediaUseCase: DownloadMediaUseCase + + @Before + fun setUp() { + MockKAnnotations.init(this) + mockkStatic("im.vector.app.core.utils.ExternalApplicationsUtilKt") + mockkStatic("im.vector.app.core.intent.VectorMimeTypeKt") + mockkStatic(Uri::class) + } + + @After + fun tearDown() { + unmockkStatic("im.vector.app.core.utils.ExternalApplicationsUtilKt") + unmockkStatic("im.vector.app.core.intent.VectorMimeTypeKt") + unmockkStatic(Uri::class) + } + + @Test + fun `given a file when calling execute then save the file in local with success`() = runBlockingTest { + //Given + val file = mockk() + val uri = mockk() + val mimeType = "mimeType" + val name = "name" + every { file.name } returns name + every { file.toUri() } returns uri + every { getMimeTypeFromUri(appContext, uri) } returns mimeType + coEvery { saveMedia(any(), any(), any(), any(), any()) } just runs + + //When + val result = downloadMediaUseCase.execute(file) + + //Then + assert(result.isSuccess) + verifyAll { + file.name + file.toUri() + } + verify { + getMimeTypeFromUri(appContext, uri) + } + coVerify { + saveMedia(appContext, file, name, mimeType, notificationUtils) + } + } + + @Test + fun `given a file when calling execute then save the file in local with error`() = runBlockingTest { + //Given + val file = mockk() + val uri = mockk() + val mimeType = "mimeType" + val name = "name" + val error = Throwable() + every { file.name } returns name + every { file.toUri() } returns uri + every { getMimeTypeFromUri(appContext, uri) } returns mimeType + coEvery { saveMedia(any(), any(), any(), any(), any()) } throws error + + //When + val result = downloadMediaUseCase.execute(file) + + //Then + assert(result.isFailure && result.exceptionOrNull() == error) + verifyAll { + file.name + file.toUri() + } + verify { + getMimeTypeFromUri(appContext, uri) + } + coVerify { + saveMedia(appContext, file, name, mimeType, notificationUtils) + } + } +} From 0170171caa0e1f4ac023b2628d0bbd18a4efe910 Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Mon, 28 Feb 2022 10:28:45 +0100 Subject: [PATCH 25/26] Adding missing spaces after comments --- .../media/domain/usecase/DownloadMediaUseCaseTest.kt | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/vector/src/test/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCaseTest.kt b/vector/src/test/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCaseTest.kt index 4ad8ca7fce..44c0d2cfd0 100644 --- a/vector/src/test/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCaseTest.kt +++ b/vector/src/test/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCaseTest.kt @@ -77,7 +77,7 @@ class DownloadMediaUseCaseTest { @Test fun `given a file when calling execute then save the file in local with success`() = runBlockingTest { - //Given + // Given val file = mockk() val uri = mockk() val mimeType = "mimeType" @@ -87,10 +87,10 @@ class DownloadMediaUseCaseTest { every { getMimeTypeFromUri(appContext, uri) } returns mimeType coEvery { saveMedia(any(), any(), any(), any(), any()) } just runs - //When + // When val result = downloadMediaUseCase.execute(file) - //Then + // Then assert(result.isSuccess) verifyAll { file.name @@ -106,7 +106,7 @@ class DownloadMediaUseCaseTest { @Test fun `given a file when calling execute then save the file in local with error`() = runBlockingTest { - //Given + // Given val file = mockk() val uri = mockk() val mimeType = "mimeType" @@ -117,10 +117,10 @@ class DownloadMediaUseCaseTest { every { getMimeTypeFromUri(appContext, uri) } returns mimeType coEvery { saveMedia(any(), any(), any(), any(), any()) } throws error - //When + // When val result = downloadMediaUseCase.execute(file) - //Then + // Then assert(result.isFailure && result.exceptionOrNull() == error) verifyAll { file.name From 562780a169c4150b297838b2c4736b61be8594bc Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Mon, 28 Feb 2022 11:55:14 +0100 Subject: [PATCH 26/26] Adding a FakeFile class for unit tests --- .../usecase/DownloadMediaUseCaseTest.kt | 37 +++++++------- .../java/im/vector/app/test/fakes/FakeFile.kt | 49 +++++++++++++++++++ 2 files changed, 67 insertions(+), 19 deletions(-) create mode 100644 vector/src/test/java/im/vector/app/test/fakes/FakeFile.kt diff --git a/vector/src/test/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCaseTest.kt b/vector/src/test/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCaseTest.kt index 44c0d2cfd0..2fa8c7d5f7 100644 --- a/vector/src/test/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCaseTest.kt +++ b/vector/src/test/java/im/vector/app/features/media/domain/usecase/DownloadMediaUseCaseTest.kt @@ -23,6 +23,7 @@ import com.airbnb.mvrx.test.MvRxTestRule import im.vector.app.core.intent.getMimeTypeFromUri import im.vector.app.core.utils.saveMedia import im.vector.app.features.notifications.NotificationUtils +import im.vector.app.test.fakes.FakeFile import im.vector.app.test.fakes.FakeSession import io.mockk.MockKAnnotations import io.mockk.coEvery @@ -42,7 +43,6 @@ import org.junit.After import org.junit.Before import org.junit.Rule import org.junit.Test -import java.io.File class DownloadMediaUseCaseTest { @@ -57,6 +57,8 @@ class DownloadMediaUseCaseTest { @MockK lateinit var notificationUtils: NotificationUtils + private val file = FakeFile() + @OverrideMockKs lateinit var downloadMediaUseCase: DownloadMediaUseCase @@ -65,72 +67,69 @@ class DownloadMediaUseCaseTest { MockKAnnotations.init(this) mockkStatic("im.vector.app.core.utils.ExternalApplicationsUtilKt") mockkStatic("im.vector.app.core.intent.VectorMimeTypeKt") - mockkStatic(Uri::class) } @After fun tearDown() { unmockkStatic("im.vector.app.core.utils.ExternalApplicationsUtilKt") unmockkStatic("im.vector.app.core.intent.VectorMimeTypeKt") - unmockkStatic(Uri::class) + file.tearDown() } @Test fun `given a file when calling execute then save the file in local with success`() = runBlockingTest { // Given - val file = mockk() val uri = mockk() val mimeType = "mimeType" - val name = "name" - every { file.name } returns name - every { file.toUri() } returns uri + val name = "filename" every { getMimeTypeFromUri(appContext, uri) } returns mimeType + file.givenName(name) + file.givenUri(uri) coEvery { saveMedia(any(), any(), any(), any(), any()) } just runs // When - val result = downloadMediaUseCase.execute(file) + val result = downloadMediaUseCase.execute(file.instance) // Then assert(result.isSuccess) verifyAll { - file.name - file.toUri() + file.instance.name + file.instance.toUri() } verify { getMimeTypeFromUri(appContext, uri) } coVerify { - saveMedia(appContext, file, name, mimeType, notificationUtils) + saveMedia(appContext, file.instance, name, mimeType, notificationUtils) } } @Test fun `given a file when calling execute then save the file in local with error`() = runBlockingTest { // Given - val file = mockk() val uri = mockk() val mimeType = "mimeType" - val name = "name" + val name = "filename" val error = Throwable() - every { file.name } returns name - every { file.toUri() } returns uri + file.givenName(name) + file.givenUri(uri) every { getMimeTypeFromUri(appContext, uri) } returns mimeType coEvery { saveMedia(any(), any(), any(), any(), any()) } throws error // When - val result = downloadMediaUseCase.execute(file) + val result = downloadMediaUseCase.execute(file.instance) // Then assert(result.isFailure && result.exceptionOrNull() == error) verifyAll { - file.name - file.toUri() + file.instance.name + file.instance.toUri() } verify { getMimeTypeFromUri(appContext, uri) } coVerify { - saveMedia(appContext, file, name, mimeType, notificationUtils) + saveMedia(appContext, file.instance, name, mimeType, notificationUtils) } } } diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeFile.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeFile.kt new file mode 100644 index 0000000000..652d3f93fd --- /dev/null +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeFile.kt @@ -0,0 +1,49 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.test.fakes + +import android.net.Uri +import androidx.core.net.toUri +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.unmockkStatic +import java.io.File + +class FakeFile { + + val instance = mockk() + + init { + mockkStatic(Uri::class) + } + + /** + * To be called after tests. + */ + fun tearDown() { + unmockkStatic(Uri::class) + } + + fun givenName(name: String) { + every { instance.name } returns name + } + + fun givenUri(uri: Uri) { + every { instance.toUri() } returns uri + } +}