From 507ddc2d4a00023201b2d92453d70e2f4a091c49 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Wed, 23 Feb 2022 13:02:45 +0100 Subject: [PATCH 01/59] adds forceLoginFallback debug feature --- .../app/features/debug/features/DebugFeaturesStateFactory.kt | 5 +++++ .../app/features/debug/features/DebugVectorFeatures.kt | 3 +++ .../src/main/java/im/vector/app/features/VectorFeatures.kt | 2 ++ 3 files changed, 10 insertions(+) diff --git a/vector/src/debug/java/im/vector/app/features/debug/features/DebugFeaturesStateFactory.kt b/vector/src/debug/java/im/vector/app/features/debug/features/DebugFeaturesStateFactory.kt index fb803162a7..46f46ad964 100644 --- a/vector/src/debug/java/im/vector/app/features/debug/features/DebugFeaturesStateFactory.kt +++ b/vector/src/debug/java/im/vector/app/features/debug/features/DebugFeaturesStateFactory.kt @@ -48,6 +48,11 @@ class DebugFeaturesStateFactory @Inject constructor( label = "FTUE Use Case", key = DebugFeatureKeys.onboardingUseCase, factory = VectorFeatures::isOnboardingUseCaseEnabled + ), + createBooleanFeature( + label = "Force login fallback", + key = DebugFeatureKeys.forceLoginFallback, + factory = VectorFeatures::isForceLoginFallbackEnabled ) )) } diff --git a/vector/src/debug/java/im/vector/app/features/debug/features/DebugVectorFeatures.kt b/vector/src/debug/java/im/vector/app/features/debug/features/DebugVectorFeatures.kt index 6ca33ca968..396dd02a37 100644 --- a/vector/src/debug/java/im/vector/app/features/debug/features/DebugVectorFeatures.kt +++ b/vector/src/debug/java/im/vector/app/features/debug/features/DebugVectorFeatures.kt @@ -51,6 +51,8 @@ class DebugVectorFeatures( override fun isOnboardingUseCaseEnabled(): Boolean = read(DebugFeatureKeys.onboardingUseCase) ?: vectorFeatures.isOnboardingUseCaseEnabled() + override fun isForceLoginFallbackEnabled(): Boolean = read(DebugFeatureKeys.forceLoginFallback) ?: vectorFeatures.isForceLoginFallbackEnabled() + fun override(value: T?, key: Preferences.Key) = updatePreferences { if (value == null) { it.remove(key) @@ -102,4 +104,5 @@ object DebugFeatureKeys { val onboardingAlreadyHaveAnAccount = booleanPreferencesKey("onboarding-already-have-an-account") val onboardingSplashCarousel = booleanPreferencesKey("onboarding-splash-carousel") val onboardingUseCase = booleanPreferencesKey("onbboarding-splash-carousel") + val forceLoginFallback = booleanPreferencesKey("force-login-fallback") } diff --git a/vector/src/main/java/im/vector/app/features/VectorFeatures.kt b/vector/src/main/java/im/vector/app/features/VectorFeatures.kt index fe8d58fb51..451ecc4722 100644 --- a/vector/src/main/java/im/vector/app/features/VectorFeatures.kt +++ b/vector/src/main/java/im/vector/app/features/VectorFeatures.kt @@ -24,6 +24,7 @@ interface VectorFeatures { fun isOnboardingAlreadyHaveAccountSplashEnabled(): Boolean fun isOnboardingSplashCarouselEnabled(): Boolean fun isOnboardingUseCaseEnabled(): Boolean + fun isForceLoginFallbackEnabled(): Boolean enum class OnboardingVariant { LEGACY, @@ -37,4 +38,5 @@ class DefaultVectorFeatures : VectorFeatures { override fun isOnboardingAlreadyHaveAccountSplashEnabled() = true override fun isOnboardingSplashCarouselEnabled() = true override fun isOnboardingUseCaseEnabled() = true + override fun isForceLoginFallbackEnabled() = false } From 713248805c5c21c402ba9cb867f3c00f9c9dc11f Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Wed, 23 Feb 2022 13:25:16 +0100 Subject: [PATCH 02/59] adds feature flag usage to registration flow result --- .../app/features/onboarding/ftueauth/FtueAuthVariant.kt | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt index 33d57dd95c..e550913e6c 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt @@ -123,8 +123,7 @@ class FtueAuthVariant( private fun handleOnboardingViewEvents(viewEvents: OnboardingViewEvents) { when (viewEvents) { is OnboardingViewEvents.RegistrationFlowResult -> { - // Check that all flows are supported by the application - if (viewEvents.flowResult.missingStages.any { !it.isSupported() }) { + if (registrationShouldFallback(viewEvents)) { // Display a popup to propose use web fallback onRegistrationStageNotSupported() } else { @@ -223,6 +222,12 @@ class FtueAuthVariant( }.exhaustive } + private fun registrationShouldFallback(registrationFlowResult: OnboardingViewEvents.RegistrationFlowResult) = + vectorFeatures.isForceLoginFallbackEnabled() || registrationFlowResult.containsUnsupportedRegistrationFlow() + + private fun OnboardingViewEvents.RegistrationFlowResult.containsUnsupportedRegistrationFlow() = + flowResult.missingStages.any { !it.isSupported() } + private fun updateWithState(viewState: OnboardingViewState) { if (viewState.isUserLogged()) { val intent = HomeActivity.newIntent( From f18808b4d7b689920c8213d819916bbf312736e5 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Wed, 23 Feb 2022 16:53:16 +0100 Subject: [PATCH 03/59] refactors FtueAuthVariant with new feature flag on registration and signin --- .../lib/core/utils/extensions/Global.kt | 20 +++ .../onboarding/ftueauth/FtueAuthVariant.kt | 115 ++++++++++-------- 2 files changed, 83 insertions(+), 52 deletions(-) create mode 100644 library/core-utils/src/main/java/im/vector/lib/core/utils/extensions/Global.kt diff --git a/library/core-utils/src/main/java/im/vector/lib/core/utils/extensions/Global.kt b/library/core-utils/src/main/java/im/vector/lib/core/utils/extensions/Global.kt new file mode 100644 index 0000000000..3b508ab7b2 --- /dev/null +++ b/library/core-utils/src/main/java/im/vector/lib/core/utils/extensions/Global.kt @@ -0,0 +1,20 @@ +/* + * 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.lib.core.utils.extensions + +@Suppress("UNUSED_PARAMETER") +fun doNothing(reason: String = "") = Unit diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt index e550913e6c..d08136eb70 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt @@ -52,6 +52,7 @@ import im.vector.app.features.onboarding.OnboardingViewModel import im.vector.app.features.onboarding.OnboardingViewState import im.vector.app.features.onboarding.ftueauth.terms.FtueAuthTermsFragment import im.vector.app.features.onboarding.ftueauth.terms.FtueAuthTermsFragmentArgument +import im.vector.lib.core.utils.extensions.doNothing import org.matrix.android.sdk.api.auth.registration.FlowResult import org.matrix.android.sdk.api.auth.registration.Stage import org.matrix.android.sdk.api.extensions.tryOrNull @@ -109,7 +110,7 @@ class FtueAuthVariant( } override fun setIsLoading(isLoading: Boolean) { - // do nothing + doNothing() } private fun addFirstFragment() { @@ -134,11 +135,7 @@ class FtueAuthVariant( // First ask for login and password // I add a tag to indicate that this fragment is a registration stage. // This way it will be automatically popped in when starting the next registration stage - activity.addFragmentToBackstack(views.loginFragmentContainer, - FtueAuthLoginFragment::class.java, - tag = FRAGMENT_REGISTRATION_STAGE_TAG, - option = commonOption - ) + openAuthLoginFragmentWithTag(FRAGMENT_REGISTRATION_STAGE_TAG) } } } @@ -228,6 +225,19 @@ class FtueAuthVariant( private fun OnboardingViewEvents.RegistrationFlowResult.containsUnsupportedRegistrationFlow() = flowResult.missingStages.any { !it.isSupported() } + private fun onRegistrationStageNotSupported() { + MaterialAlertDialogBuilder(activity) + .setTitle(R.string.app_name) + .setMessage(activity.getString(R.string.login_registration_not_supported)) + .setPositiveButton(R.string.yes) { _, _ -> + activity.addFragmentToBackstack(views.loginFragmentContainer, + FtueAuthWebFragment::class.java, + option = commonOption) + } + .setNegativeButton(R.string.no, null) + .show() + } + private fun updateWithState(viewState: OnboardingViewState) { if (viewState.isUserLogged()) { val intent = HomeActivity.newIntent( @@ -270,29 +280,56 @@ class FtueAuthVariant( // state.signMode could not be ready yet. So use value from the ViewEvent when (OnboardingViewEvents.signMode) { SignMode.Unknown -> error("Sign mode has to be set before calling this method") - SignMode.SignUp -> { - // This is managed by the OnboardingViewEvents - } - SignMode.SignIn -> { - // It depends on the LoginMode - when (state.loginMode) { - LoginMode.Unknown, - is LoginMode.Sso -> error("Developer error") - is LoginMode.SsoAndPassword, - LoginMode.Password -> activity.addFragmentToBackstack(views.loginFragmentContainer, - FtueAuthLoginFragment::class.java, - tag = FRAGMENT_LOGIN_TAG, - option = commonOption) - LoginMode.Unsupported -> onLoginModeNotSupported(state.loginModeSupportedTypes) - }.exhaustive - } - SignMode.SignInWithMatrixId -> activity.addFragmentToBackstack(views.loginFragmentContainer, - FtueAuthLoginFragment::class.java, - tag = FRAGMENT_LOGIN_TAG, - option = commonOption) + SignMode.SignUp -> doNothing("This is managed by the OnboardingViewEvents") + SignMode.SignIn -> handleSignInSelected(state) + SignMode.SignInWithMatrixId -> handleSignInWithMatrixId(state) }.exhaustive } + private fun handleSignInSelected(state: OnboardingViewState) { + if (vectorFeatures.isForceLoginFallbackEnabled()) + onLoginModeNotSupported(state.loginModeSupportedTypes) + else + disambiguateLoginMode(state) + } + + private fun disambiguateLoginMode(state: OnboardingViewState) = when (state.loginMode) { + LoginMode.Unknown, + is LoginMode.Sso -> error("Developer error") + is LoginMode.SsoAndPassword, + LoginMode.Password -> openAuthLoginFragmentWithTag(FRAGMENT_LOGIN_TAG) + LoginMode.Unsupported -> onLoginModeNotSupported(state.loginModeSupportedTypes) + }.exhaustive + + private fun openAuthLoginFragmentWithTag(tag: String) { + activity.addFragmentToBackstack(views.loginFragmentContainer, + FtueAuthLoginFragment::class.java, + tag = tag, + option = commonOption) + } + + private fun onLoginModeNotSupported(supportedTypes: List) { + MaterialAlertDialogBuilder(activity) + .setTitle(R.string.app_name) + .setMessage(activity.getString(R.string.login_mode_not_supported, supportedTypes.joinToString { "'$it'" })) + .setPositiveButton(R.string.yes) { _, _ -> openAuthWebFragment() } + .setNegativeButton(R.string.no, null) + .show() + } + + private fun handleSignInWithMatrixId(state: OnboardingViewState) { + if (vectorFeatures.isForceLoginFallbackEnabled()) + onLoginModeNotSupported(state.loginModeSupportedTypes) + else + openAuthLoginFragmentWithTag(FRAGMENT_LOGIN_TAG) + } + + private fun openAuthWebFragment() { + activity.addFragmentToBackstack(views.loginFragmentContainer, + FtueAuthWebFragment::class.java, + option = commonOption) + } + /** * Handle the SSO redirection here */ @@ -302,32 +339,6 @@ class FtueAuthVariant( ?.let { onboardingViewModel.handle(OnboardingAction.LoginWithToken(it)) } } - private fun onRegistrationStageNotSupported() { - MaterialAlertDialogBuilder(activity) - .setTitle(R.string.app_name) - .setMessage(activity.getString(R.string.login_registration_not_supported)) - .setPositiveButton(R.string.yes) { _, _ -> - activity.addFragmentToBackstack(views.loginFragmentContainer, - FtueAuthWebFragment::class.java, - option = commonOption) - } - .setNegativeButton(R.string.no, null) - .show() - } - - private fun onLoginModeNotSupported(supportedTypes: List) { - MaterialAlertDialogBuilder(activity) - .setTitle(R.string.app_name) - .setMessage(activity.getString(R.string.login_mode_not_supported, supportedTypes.joinToString { "'$it'" })) - .setPositiveButton(R.string.yes) { _, _ -> - activity.addFragmentToBackstack(views.loginFragmentContainer, - FtueAuthWebFragment::class.java, - option = commonOption) - } - .setNegativeButton(R.string.no, null) - .show() - } - private fun handleRegistrationNavigation(flowResult: FlowResult) { // Complete all mandatory stages first val mandatoryStage = flowResult.missingStages.firstOrNull { it.mandatory } From 137d804cfa88d02e58e7a8dafe1b028eae44ecec Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Wed, 23 Feb 2022 17:26:50 +0100 Subject: [PATCH 04/59] adds changelog file --- changelog.d/5325.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5325.feature diff --git a/changelog.d/5325.feature b/changelog.d/5325.feature new file mode 100644 index 0000000000..23754c790d --- /dev/null +++ b/changelog.d/5325.feature @@ -0,0 +1 @@ +Adds forceLoginFallback feature flag and usages to FTUE login and registration \ No newline at end of file From c89f28ffb201bd32e77d26536549a3f9079846d0 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Wed, 23 Feb 2022 17:43:40 +0100 Subject: [PATCH 05/59] adds missing brackets on multiline if statements --- .../features/onboarding/ftueauth/FtueAuthVariant.kt | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt index 962b40df66..8e168b5d20 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt @@ -286,10 +286,11 @@ class FtueAuthVariant( } private fun handleSignInSelected(state: OnboardingViewState) { - if (vectorFeatures.isForceLoginFallbackEnabled()) + if (vectorFeatures.isForceLoginFallbackEnabled()) { onLoginModeNotSupported(state.loginModeSupportedTypes) - else + } else { disambiguateLoginMode(state) + } } private fun disambiguateLoginMode(state: OnboardingViewState) = when (state.loginMode) { @@ -317,10 +318,11 @@ class FtueAuthVariant( } private fun handleSignInWithMatrixId(state: OnboardingViewState) { - if (vectorFeatures.isForceLoginFallbackEnabled()) + if (vectorFeatures.isForceLoginFallbackEnabled()) { onLoginModeNotSupported(state.loginModeSupportedTypes) - else + } else { openAuthLoginFragmentWithTag(FRAGMENT_LOGIN_TAG) + } } private fun openAuthWebFragment() { From ac4d748c8cefbbb9da1e100b06d9f6a877560f0f Mon Sep 17 00:00:00 2001 From: David Langley Date: Wed, 23 Feb 2022 18:01:22 +0000 Subject: [PATCH 06/59] Add concurrency to integration tests. --- .github/workflows/integration_tests.yml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/.github/workflows/integration_tests.yml b/.github/workflows/integration_tests.yml index cac35fb1fc..3f4fc5f779 100644 --- a/.github/workflows/integration_tests.yml +++ b/.github/workflows/integration_tests.yml @@ -16,6 +16,11 @@ jobs: build-android-test-matrix-sdk: name: Matrix SDK - Build Android Tests runs-on: ubuntu-latest + concurrency: + # When running on develop, use the sha to allow all runs of this workflow to run concurrently. + # Otherwise only allow a single run of this workflow on each branch, automatically cancelling older runs. + group: ${{ github.ref == 'refs/heads/develop' && format('test-matrix-sdk-develop-{0}', github.sha) || format('test-matrix-sdk-{0}', github.ref) }} + cancel-in-progress: true steps: - uses: actions/checkout@v2 - uses: actions/cache@v2 @@ -33,6 +38,11 @@ jobs: build-android-test-app: name: App - Build Android Tests runs-on: ubuntu-latest + concurrency: + # When running on develop, use the sha to allow all runs of this workflow to run concurrently. + # Otherwise only allow a single run of this workflow on each branch, automatically cancelling older runs. + group: ${{ github.ref == 'refs/heads/develop' && format('test-app-develop-{0}', github.sha) || format('test-app-{0}', github.ref) }} + cancel-in-progress: true steps: - uses: actions/checkout@v2 - uses: actions/cache@v2 @@ -50,6 +60,11 @@ jobs: integration-tests: name: Matrix SDK - Running Integration Tests runs-on: ubuntu-latest + concurrency: + # When running on develop, use the sha to allow all runs of this workflow to run concurrently. + # Otherwise only allow a single run of this workflow on each branch, automatically cancelling older runs. + group: ${{ github.ref == 'refs/heads/develop' && format('integration-tests-develop-{0}', github.sha) || format('integration-tests-{0}', github.ref) }} + cancel-in-progress: true strategy: fail-fast: false matrix: From 5bbd6769b9e5740be31fad34b64fdafe915eb3d9 Mon Sep 17 00:00:00 2001 From: David Langley Date: Wed, 23 Feb 2022 18:02:59 +0000 Subject: [PATCH 07/59] noop to test cancel --- .github/workflows/integration_tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/integration_tests.yml b/.github/workflows/integration_tests.yml index 3f4fc5f779..8a08dedb01 100644 --- a/.github/workflows/integration_tests.yml +++ b/.github/workflows/integration_tests.yml @@ -1,3 +1,4 @@ +# no-op name: Integration Tests on: From 639774662a2c03ca661da0e4f65620b291b5248f Mon Sep 17 00:00:00 2001 From: David Langley Date: Wed, 23 Feb 2022 18:11:46 +0000 Subject: [PATCH 08/59] Add concurrency checks for unit tests, build and quality checks. --- .github/workflows/build.yml | 10 ++++++++++ .github/workflows/integration_tests.yml | 1 - .github/workflows/quality.yml | 15 +++++++++++++++ .github/workflows/tests.yml | 5 +++++ 4 files changed, 30 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 91dc6d830b..1de7864a95 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -15,6 +15,11 @@ jobs: debug: name: Build debug APKs (${{ matrix.target }}) runs-on: ubuntu-latest + concurrency: + # When running on develop, use the sha to allow all runs of this workflow to run concurrently. + # Otherwise only allow a single run of this workflow on each branch, automatically cancelling older runs. + group: ${{ github.ref == 'refs/heads/develop' && format('build-debug-develop-{0}', github.sha) || format('build-debug-{0}', github.ref) }} + cancel-in-progress: true if: github.ref != 'refs/heads/main' strategy: fail-fast: false @@ -42,6 +47,11 @@ jobs: release: name: Build unsigned GPlay APKs runs-on: ubuntu-latest + concurrency: + # When running on develop, use the sha to allow all runs of this workflow to run concurrently. + # Otherwise only allow a single run of this workflow on each branch, automatically cancelling older runs. + group: ${{ github.ref == 'refs/heads/develop' && format('build-release-develop-{0}', github.sha) || format('build-release-{0}', github.ref) }} + cancel-in-progress: true if: github.ref == 'refs/heads/main' steps: - uses: actions/checkout@v2 diff --git a/.github/workflows/integration_tests.yml b/.github/workflows/integration_tests.yml index 8a08dedb01..3f4fc5f779 100644 --- a/.github/workflows/integration_tests.yml +++ b/.github/workflows/integration_tests.yml @@ -1,4 +1,3 @@ -# no-op name: Integration Tests on: diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index 69f17a3875..93e29bf394 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -18,6 +18,11 @@ jobs: ktlint: name: Kotlin Linter runs-on: ubuntu-latest + concurrency: + # When running on develop, use the sha to allow all runs of this workflow to run concurrently. + # Otherwise only allow a single run of this workflow on each branch, automatically cancelling older runs. + group: ${{ github.ref == 'refs/heads/develop' && format('ktlint-develop-{0}', github.sha) || format('ktlint-{0}', github.ref) }} + cancel-in-progress: true steps: - uses: actions/checkout@v2 - name: Run ktlint @@ -87,6 +92,11 @@ jobs: android-lint: name: Android Linter runs-on: ubuntu-latest + concurrency: + # When running on develop, use the sha to allow all runs of this workflow to run concurrently. + # Otherwise only allow a single run of this workflow on each branch, automatically cancelling older runs. + group: ${{ github.ref == 'refs/heads/develop' && format('android-lint-develop-{0}', github.sha) || format('android-lint-{0}', github.ref) }} + cancel-in-progress: true steps: - uses: actions/checkout@v2 - uses: actions/cache@v2 @@ -111,6 +121,11 @@ jobs: apk-lint: name: Lint APK (${{ matrix.target }}) runs-on: ubuntu-latest + concurrency: + # When running on develop, use the sha to allow all runs of this workflow to run concurrently. + # Otherwise only allow a single run of this workflow on each branch, automatically cancelling older runs. + group: ${{ github.ref == 'refs/heads/develop' && format('apk-lint-develop-{0}', github.sha) || format('apk-lint-{0}', github.ref) }} + cancel-in-progress: true if: github.ref != 'refs/heads/main' strategy: fail-fast: false diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 50195638de..13eb191680 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -15,6 +15,11 @@ jobs: unit-tests: name: Run Unit Tests runs-on: ubuntu-latest + concurrency: + # When running on develop, use the sha to allow all runs of this workflow to run concurrently. + # Otherwise only allow a single run of this workflow on each branch, automatically cancelling older runs. + group: ${{ github.ref == 'refs/heads/develop' && format('test-unit-develop-{0}', github.sha) || format('test-unit-{0}', github.ref) }} + cancel-in-progress: true steps: - uses: actions/checkout@v2 - uses: actions/cache@v2 From edb92f85c58add11451dff041411a816fd2805fe Mon Sep 17 00:00:00 2001 From: David Langley Date: Wed, 23 Feb 2022 18:12:43 +0000 Subject: [PATCH 09/59] noop to test cancel --- .github/workflows/integration_tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/integration_tests.yml b/.github/workflows/integration_tests.yml index 3f4fc5f779..8a08dedb01 100644 --- a/.github/workflows/integration_tests.yml +++ b/.github/workflows/integration_tests.yml @@ -1,3 +1,4 @@ +# no-op name: Integration Tests on: From 43c125ed32b6aafa67cef948988593bb8c6dd5a3 Mon Sep 17 00:00:00 2001 From: David Langley Date: Wed, 23 Feb 2022 22:16:24 +0000 Subject: [PATCH 10/59] Use matrix.target to fix build group. Use env vars to clean up. --- .github/workflows/build.yml | 13 ++++++------- .github/workflows/integration_tests.yml | 17 +++++++---------- .github/workflows/quality.yml | 18 +++++++++--------- .github/workflows/tests.yml | 9 +++++---- 4 files changed, 27 insertions(+), 30 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 1de7864a95..7935344232 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -10,15 +10,16 @@ env: CI_GRADLE_ARG_PROPERTIES: > -Porg.gradle.jvmargs=-Xmx2g -Porg.gradle.parallel=false - + CI_ANY_MAIN: ${{ github.ref == 'refs/heads/main' && format('main-{0}', github.sha) }} + CI_ANY_DEVELOP: ${{ github.ref == 'refs/heads/develop' && format('develop-{0}', github.sha) }} + CI_ONE_PR: ${{ format('build-debug-{0}-{1}', matrix.target, github.ref) }} + CI_GROUP_SUFFIX: ${{ CI_ANY_MAIN || CI_ANY_DEVELOP || CI_ONE_PR }} jobs: debug: name: Build debug APKs (${{ matrix.target }}) runs-on: ubuntu-latest concurrency: - # When running on develop, use the sha to allow all runs of this workflow to run concurrently. - # Otherwise only allow a single run of this workflow on each branch, automatically cancelling older runs. - group: ${{ github.ref == 'refs/heads/develop' && format('build-debug-develop-{0}', github.sha) || format('build-debug-{0}', github.ref) }} + group: ${{ format('build-debug-{0}-{1}', matrix.target, CI_GROUP_SUFFIX) }} cancel-in-progress: true if: github.ref != 'refs/heads/main' strategy: @@ -48,9 +49,7 @@ jobs: name: Build unsigned GPlay APKs runs-on: ubuntu-latest concurrency: - # When running on develop, use the sha to allow all runs of this workflow to run concurrently. - # Otherwise only allow a single run of this workflow on each branch, automatically cancelling older runs. - group: ${{ github.ref == 'refs/heads/develop' && format('build-release-develop-{0}', github.sha) || format('build-release-{0}', github.ref) }} + group: ${{ format('build-release-{0}-{1}', matrix.target, CI_GROUP_SUFFIX) }} cancel-in-progress: true if: github.ref == 'refs/heads/main' steps: diff --git a/.github/workflows/integration_tests.yml b/.github/workflows/integration_tests.yml index 8a08dedb01..ca024b7be1 100644 --- a/.github/workflows/integration_tests.yml +++ b/.github/workflows/integration_tests.yml @@ -1,4 +1,3 @@ -# no-op name: Integration Tests on: @@ -11,6 +10,10 @@ env: CI_GRADLE_ARG_PROPERTIES: > -Porg.gradle.jvmargs=-Xmx2g -Porg.gradle.parallel=false + CI_ANY_MAIN: ${{ github.ref == 'refs/heads/main' && format('main-{0}', github.sha) }} + CI_ANY_DEVELOP: ${{ github.ref == 'refs/heads/develop' && format('develop-{0}', github.sha) }} + CI_ONE_PR: ${{ format('build-debug-{0}-{1}', matrix.target, github.ref) }} + CI_GROUP_SUFFIX: ${{ CI_ANY_MAIN || CI_ANY_DEVELOP || CI_ONE_PR }} jobs: # Build Android Tests [Matrix SDK] @@ -18,9 +21,7 @@ jobs: name: Matrix SDK - Build Android Tests runs-on: ubuntu-latest concurrency: - # When running on develop, use the sha to allow all runs of this workflow to run concurrently. - # Otherwise only allow a single run of this workflow on each branch, automatically cancelling older runs. - group: ${{ github.ref == 'refs/heads/develop' && format('test-matrix-sdk-develop-{0}', github.sha) || format('test-matrix-sdk-{0}', github.ref) }} + group: ${{ format('test-matrix-sdk-{0}', CI_GROUP_SUFFIX) }} cancel-in-progress: true steps: - uses: actions/checkout@v2 @@ -40,9 +41,7 @@ jobs: name: App - Build Android Tests runs-on: ubuntu-latest concurrency: - # When running on develop, use the sha to allow all runs of this workflow to run concurrently. - # Otherwise only allow a single run of this workflow on each branch, automatically cancelling older runs. - group: ${{ github.ref == 'refs/heads/develop' && format('test-app-develop-{0}', github.sha) || format('test-app-{0}', github.ref) }} + group: ${{ format('test-app-{0}', CI_GROUP_SUFFIX) }} cancel-in-progress: true steps: - uses: actions/checkout@v2 @@ -62,9 +61,7 @@ jobs: name: Matrix SDK - Running Integration Tests runs-on: ubuntu-latest concurrency: - # When running on develop, use the sha to allow all runs of this workflow to run concurrently. - # Otherwise only allow a single run of this workflow on each branch, automatically cancelling older runs. - group: ${{ github.ref == 'refs/heads/develop' && format('integration-tests-develop-{0}', github.sha) || format('integration-tests-{0}', github.ref) }} + group: ${{ format('integration-tests-{0}', CI_GROUP_SUFFIX) }} cancel-in-progress: true strategy: fail-fast: false diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index 93e29bf394..6cfb0d7f05 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -5,6 +5,12 @@ on: push: branches: [ main, develop ] +env: + CI_ANY_MAIN: ${{ github.ref == 'refs/heads/main' && format('main-{0}', github.sha) }} + CI_ANY_DEVELOP: ${{ github.ref == 'refs/heads/develop' && format('develop-{0}', github.sha) }} + CI_ONE_PR: ${{ format('build-debug-{0}-{1}', matrix.target, github.ref) }} + CI_GROUP_SUFFIX: ${{ CI_ANY_MAIN || CI_ANY_DEVELOP || CI_ONE_PR }} + jobs: check: name: Project Check Suite @@ -19,9 +25,7 @@ jobs: name: Kotlin Linter runs-on: ubuntu-latest concurrency: - # When running on develop, use the sha to allow all runs of this workflow to run concurrently. - # Otherwise only allow a single run of this workflow on each branch, automatically cancelling older runs. - group: ${{ github.ref == 'refs/heads/develop' && format('ktlint-develop-{0}', github.sha) || format('ktlint-{0}', github.ref) }} + group: ${{ format('klint-{0}', CI_GROUP_SUFFIX) }} cancel-in-progress: true steps: - uses: actions/checkout@v2 @@ -93,9 +97,7 @@ jobs: name: Android Linter runs-on: ubuntu-latest concurrency: - # When running on develop, use the sha to allow all runs of this workflow to run concurrently. - # Otherwise only allow a single run of this workflow on each branch, automatically cancelling older runs. - group: ${{ github.ref == 'refs/heads/develop' && format('android-lint-develop-{0}', github.sha) || format('android-lint-{0}', github.ref) }} + group: ${{ format('android-lint-{0}', CI_GROUP_SUFFIX) }} cancel-in-progress: true steps: - uses: actions/checkout@v2 @@ -122,9 +124,7 @@ jobs: name: Lint APK (${{ matrix.target }}) runs-on: ubuntu-latest concurrency: - # When running on develop, use the sha to allow all runs of this workflow to run concurrently. - # Otherwise only allow a single run of this workflow on each branch, automatically cancelling older runs. - group: ${{ github.ref == 'refs/heads/develop' && format('apk-lint-develop-{0}', github.sha) || format('apk-lint-{0}', github.ref) }} + group: ${{ format('apk-lint-{0}', CI_GROUP_SUFFIX) }} cancel-in-progress: true if: github.ref != 'refs/heads/main' strategy: diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 13eb191680..2ae26e32dd 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -10,15 +10,16 @@ env: CI_GRADLE_ARG_PROPERTIES: > -Porg.gradle.jvmargs=-Xmx2g -Porg.gradle.parallel=false - + CI_ANY_MAIN: ${{ github.ref == 'refs/heads/main' && format('main-{0}', github.sha) }} + CI_ANY_DEVELOP: ${{ github.ref == 'refs/heads/develop' && format('develop-{0}', github.sha) }} + CI_ONE_PR: ${{ format('build-debug-{0}-{1}', matrix.target, github.ref) }} + CI_GROUP_SUFFIX: ${{ CI_ANY_MAIN || CI_ANY_DEVELOP || CI_ONE_PR }} jobs: unit-tests: name: Run Unit Tests runs-on: ubuntu-latest concurrency: - # When running on develop, use the sha to allow all runs of this workflow to run concurrently. - # Otherwise only allow a single run of this workflow on each branch, automatically cancelling older runs. - group: ${{ github.ref == 'refs/heads/develop' && format('test-unit-develop-{0}', github.sha) || format('test-unit-{0}', github.ref) }} + group: ${{ format('unit-tests-{0}', CI_GROUP_SUFFIX) }} cancel-in-progress: true steps: - uses: actions/checkout@v2 From 8c65285ec55b009f123b67b38dc5509a926114fe Mon Sep 17 00:00:00 2001 From: David Langley Date: Wed, 23 Feb 2022 22:37:19 +0000 Subject: [PATCH 11/59] try multiline expression for group sufficx --- .github/workflows/build.yml | 10 ++++++---- .github/workflows/integration_tests.yml | 13 +++++++------ .github/workflows/quality.yml | 10 ++++++---- .github/workflows/tests.yml | 10 ++++++---- 4 files changed, 25 insertions(+), 18 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 7935344232..d751c10431 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -10,10 +10,12 @@ env: CI_GRADLE_ARG_PROPERTIES: > -Porg.gradle.jvmargs=-Xmx2g -Porg.gradle.parallel=false - CI_ANY_MAIN: ${{ github.ref == 'refs/heads/main' && format('main-{0}', github.sha) }} - CI_ANY_DEVELOP: ${{ github.ref == 'refs/heads/develop' && format('develop-{0}', github.sha) }} - CI_ONE_PR: ${{ format('build-debug-{0}-{1}', matrix.target, github.ref) }} - CI_GROUP_SUFFIX: ${{ CI_ANY_MAIN || CI_ANY_DEVELOP || CI_ONE_PR }} + CI_GROUP_SUFFIX: > + ${{ + github.ref == 'refs/heads/main' && format('main-{0}', github.sha) || + github.ref == 'refs/heads/develop' && format('develop-{0}', github.sha) || + github.ref + }} jobs: debug: name: Build debug APKs (${{ matrix.target }}) diff --git a/.github/workflows/integration_tests.yml b/.github/workflows/integration_tests.yml index ca024b7be1..13d302ae02 100644 --- a/.github/workflows/integration_tests.yml +++ b/.github/workflows/integration_tests.yml @@ -10,18 +10,19 @@ env: CI_GRADLE_ARG_PROPERTIES: > -Porg.gradle.jvmargs=-Xmx2g -Porg.gradle.parallel=false - CI_ANY_MAIN: ${{ github.ref == 'refs/heads/main' && format('main-{0}', github.sha) }} - CI_ANY_DEVELOP: ${{ github.ref == 'refs/heads/develop' && format('develop-{0}', github.sha) }} - CI_ONE_PR: ${{ format('build-debug-{0}-{1}', matrix.target, github.ref) }} - CI_GROUP_SUFFIX: ${{ CI_ANY_MAIN || CI_ANY_DEVELOP || CI_ONE_PR }} - + CI_GROUP_SUFFIX: > + ${{ + github.ref == 'refs/heads/main' && format('main-{0}', github.sha) || + github.ref == 'refs/heads/develop' && format('develop-{0}', github.sha) || + github.ref + }} jobs: # Build Android Tests [Matrix SDK] build-android-test-matrix-sdk: name: Matrix SDK - Build Android Tests runs-on: ubuntu-latest concurrency: - group: ${{ format('test-matrix-sdk-{0}', CI_GROUP_SUFFIX) }} + group: ${{ format('test-matrix-sdk-{0}', env.CI_GROUP_SUFFIX) }} cancel-in-progress: true steps: - uses: actions/checkout@v2 diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index 6cfb0d7f05..b8b35de7a7 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -6,10 +6,12 @@ on: branches: [ main, develop ] env: - CI_ANY_MAIN: ${{ github.ref == 'refs/heads/main' && format('main-{0}', github.sha) }} - CI_ANY_DEVELOP: ${{ github.ref == 'refs/heads/develop' && format('develop-{0}', github.sha) }} - CI_ONE_PR: ${{ format('build-debug-{0}-{1}', matrix.target, github.ref) }} - CI_GROUP_SUFFIX: ${{ CI_ANY_MAIN || CI_ANY_DEVELOP || CI_ONE_PR }} + CI_GROUP_SUFFIX: > + ${{ + github.ref == 'refs/heads/main' && format('main-{0}', github.sha) || + github.ref == 'refs/heads/develop' && format('develop-{0}', github.sha) || + github.ref + }} jobs: check: diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 2ae26e32dd..7a6d19ac92 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -10,10 +10,12 @@ env: CI_GRADLE_ARG_PROPERTIES: > -Porg.gradle.jvmargs=-Xmx2g -Porg.gradle.parallel=false - CI_ANY_MAIN: ${{ github.ref == 'refs/heads/main' && format('main-{0}', github.sha) }} - CI_ANY_DEVELOP: ${{ github.ref == 'refs/heads/develop' && format('develop-{0}', github.sha) }} - CI_ONE_PR: ${{ format('build-debug-{0}-{1}', matrix.target, github.ref) }} - CI_GROUP_SUFFIX: ${{ CI_ANY_MAIN || CI_ANY_DEVELOP || CI_ONE_PR }} + CI_GROUP_SUFFIX: > + ${{ + github.ref == 'refs/heads/main' && format('main-{0}', github.sha) || + github.ref == 'refs/heads/develop' && format('develop-{0}', github.sha) || + github.ref + }} jobs: unit-tests: name: Run Unit Tests From 355931a59233c6f3a35740343bd6b11a605d656d Mon Sep 17 00:00:00 2001 From: David Langley Date: Wed, 23 Feb 2022 22:42:11 +0000 Subject: [PATCH 12/59] add env prefix. --- .github/workflows/build.yml | 4 ++-- .github/workflows/integration_tests.yml | 4 ++-- .github/workflows/quality.yml | 6 +++--- .github/workflows/tests.yml | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d751c10431..c74e0f1ccf 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -21,7 +21,7 @@ jobs: name: Build debug APKs (${{ matrix.target }}) runs-on: ubuntu-latest concurrency: - group: ${{ format('build-debug-{0}-{1}', matrix.target, CI_GROUP_SUFFIX) }} + group: ${{ format('build-debug-{0}-{1}', matrix.target, env.CI_GROUP_SUFFIX) }} cancel-in-progress: true if: github.ref != 'refs/heads/main' strategy: @@ -51,7 +51,7 @@ jobs: name: Build unsigned GPlay APKs runs-on: ubuntu-latest concurrency: - group: ${{ format('build-release-{0}-{1}', matrix.target, CI_GROUP_SUFFIX) }} + group: ${{ format('build-release-{0}-{1}', matrix.target, env.CI_GROUP_SUFFIX) }} cancel-in-progress: true if: github.ref == 'refs/heads/main' steps: diff --git a/.github/workflows/integration_tests.yml b/.github/workflows/integration_tests.yml index 13d302ae02..0306464fe9 100644 --- a/.github/workflows/integration_tests.yml +++ b/.github/workflows/integration_tests.yml @@ -42,7 +42,7 @@ jobs: name: App - Build Android Tests runs-on: ubuntu-latest concurrency: - group: ${{ format('test-app-{0}', CI_GROUP_SUFFIX) }} + group: ${{ format('test-app-{0}', env.CI_GROUP_SUFFIX) }} cancel-in-progress: true steps: - uses: actions/checkout@v2 @@ -62,7 +62,7 @@ jobs: name: Matrix SDK - Running Integration Tests runs-on: ubuntu-latest concurrency: - group: ${{ format('integration-tests-{0}', CI_GROUP_SUFFIX) }} + group: ${{ format('integration-tests-{0}', env.CI_GROUP_SUFFIX) }} cancel-in-progress: true strategy: fail-fast: false diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index b8b35de7a7..cbfe206d2b 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -27,7 +27,7 @@ jobs: name: Kotlin Linter runs-on: ubuntu-latest concurrency: - group: ${{ format('klint-{0}', CI_GROUP_SUFFIX) }} + group: ${{ format('klint-{0}', env.CI_GROUP_SUFFIX) }} cancel-in-progress: true steps: - uses: actions/checkout@v2 @@ -99,7 +99,7 @@ jobs: name: Android Linter runs-on: ubuntu-latest concurrency: - group: ${{ format('android-lint-{0}', CI_GROUP_SUFFIX) }} + group: ${{ format('android-lint-{0}', env.CI_GROUP_SUFFIX) }} cancel-in-progress: true steps: - uses: actions/checkout@v2 @@ -126,7 +126,7 @@ jobs: name: Lint APK (${{ matrix.target }}) runs-on: ubuntu-latest concurrency: - group: ${{ format('apk-lint-{0}', CI_GROUP_SUFFIX) }} + group: ${{ format('apk-lint-{0}', env.CI_GROUP_SUFFIX) }} cancel-in-progress: true if: github.ref != 'refs/heads/main' strategy: diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 7a6d19ac92..4f473567c5 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -21,7 +21,7 @@ jobs: name: Run Unit Tests runs-on: ubuntu-latest concurrency: - group: ${{ format('unit-tests-{0}', CI_GROUP_SUFFIX) }} + group: ${{ format('unit-tests-{0}', env.CI_GROUP_SUFFIX) }} cancel-in-progress: true steps: - uses: actions/checkout@v2 From cf7417d544f7992ffd23c2f437bb5f216a14c8bf Mon Sep 17 00:00:00 2001 From: David Langley Date: Wed, 23 Feb 2022 22:58:51 +0000 Subject: [PATCH 13/59] Have to inline it as can't use env vars in other env vars. --- .github/workflows/build.yml | 15 +++------------ .github/workflows/integration_tests.yml | 12 +++--------- .github/workflows/quality.yml | 18 +++++------------- .github/workflows/tests.yml | 8 +------- 4 files changed, 12 insertions(+), 41 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index c74e0f1ccf..99cfebabe4 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -10,24 +10,18 @@ env: CI_GRADLE_ARG_PROPERTIES: > -Porg.gradle.jvmargs=-Xmx2g -Porg.gradle.parallel=false - CI_GROUP_SUFFIX: > - ${{ - github.ref == 'refs/heads/main' && format('main-{0}', github.sha) || - github.ref == 'refs/heads/develop' && format('develop-{0}', github.sha) || - github.ref - }} jobs: debug: name: Build debug APKs (${{ matrix.target }}) runs-on: ubuntu-latest - concurrency: - group: ${{ format('build-debug-{0}-{1}', matrix.target, env.CI_GROUP_SUFFIX) }} - cancel-in-progress: true if: github.ref != 'refs/heads/main' strategy: fail-fast: false matrix: target: [ Gplay, Fdroid ] + concurrency: + group: ${{ github.ref == 'refs/heads/develop' && format('integration-tests-develop-{0}-{1}', matrix.target, github.sha) || format('build-debug-{0}-{1}', matrix.target, github.ref) }} + cancel-in-progress: true steps: - uses: actions/checkout@v2 - uses: actions/cache@v2 @@ -50,9 +44,6 @@ jobs: release: name: Build unsigned GPlay APKs runs-on: ubuntu-latest - concurrency: - group: ${{ format('build-release-{0}-{1}', matrix.target, env.CI_GROUP_SUFFIX) }} - cancel-in-progress: true if: github.ref == 'refs/heads/main' steps: - uses: actions/checkout@v2 diff --git a/.github/workflows/integration_tests.yml b/.github/workflows/integration_tests.yml index 0306464fe9..4a048118e7 100644 --- a/.github/workflows/integration_tests.yml +++ b/.github/workflows/integration_tests.yml @@ -10,19 +10,13 @@ env: CI_GRADLE_ARG_PROPERTIES: > -Porg.gradle.jvmargs=-Xmx2g -Porg.gradle.parallel=false - CI_GROUP_SUFFIX: > - ${{ - github.ref == 'refs/heads/main' && format('main-{0}', github.sha) || - github.ref == 'refs/heads/develop' && format('develop-{0}', github.sha) || - github.ref - }} jobs: # Build Android Tests [Matrix SDK] build-android-test-matrix-sdk: name: Matrix SDK - Build Android Tests runs-on: ubuntu-latest concurrency: - group: ${{ format('test-matrix-sdk-{0}', env.CI_GROUP_SUFFIX) }} + group: ${{ github.ref == 'refs/heads/main' && format('test-matrix-sdk-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('test-matrix-sdk-develop-{0}', github.sha) || format('test-matrix-sdk-{0}', github.ref) }} cancel-in-progress: true steps: - uses: actions/checkout@v2 @@ -42,7 +36,7 @@ jobs: name: App - Build Android Tests runs-on: ubuntu-latest concurrency: - group: ${{ format('test-app-{0}', env.CI_GROUP_SUFFIX) }} + group: ${{ github.ref == 'refs/heads/main' && format('test-app-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('test-app-develop-{0}', github.sha) || format('test-app-{0}', github.ref) }} cancel-in-progress: true steps: - uses: actions/checkout@v2 @@ -62,7 +56,7 @@ jobs: name: Matrix SDK - Running Integration Tests runs-on: ubuntu-latest concurrency: - group: ${{ format('integration-tests-{0}', env.CI_GROUP_SUFFIX) }} + group: ${{ github.ref == 'refs/heads/main' && format('integration-tests-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('integration-tests-develop-{0}', github.sha) || format('integration-tests-{0}', github.ref) }} cancel-in-progress: true strategy: fail-fast: false diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index cbfe206d2b..e681b850a5 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -5,14 +5,6 @@ on: push: branches: [ main, develop ] -env: - CI_GROUP_SUFFIX: > - ${{ - github.ref == 'refs/heads/main' && format('main-{0}', github.sha) || - github.ref == 'refs/heads/develop' && format('develop-{0}', github.sha) || - github.ref - }} - jobs: check: name: Project Check Suite @@ -27,7 +19,7 @@ jobs: name: Kotlin Linter runs-on: ubuntu-latest concurrency: - group: ${{ format('klint-{0}', env.CI_GROUP_SUFFIX) }} + group: ${{ github.ref == 'refs/heads/main' && format('klint-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('klint-develop-{0}', github.sha) || format('klint-{0}', github.ref) }} cancel-in-progress: true steps: - uses: actions/checkout@v2 @@ -99,7 +91,7 @@ jobs: name: Android Linter runs-on: ubuntu-latest concurrency: - group: ${{ format('android-lint-{0}', env.CI_GROUP_SUFFIX) }} + group: ${{ github.ref == 'refs/heads/main' && format('android-lint-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('android-lint-develop-{0}', github.sha) || format('android-lint-{0}', github.ref) }} cancel-in-progress: true steps: - uses: actions/checkout@v2 @@ -125,14 +117,14 @@ jobs: apk-lint: name: Lint APK (${{ matrix.target }}) runs-on: ubuntu-latest - concurrency: - group: ${{ format('apk-lint-{0}', env.CI_GROUP_SUFFIX) }} - cancel-in-progress: true if: github.ref != 'refs/heads/main' strategy: fail-fast: false matrix: target: [ Gplay, Fdroid ] + concurrency: + group: ${{ github.ref == 'refs/heads/develop' && format('apk-lint-develop-{0}', github.sha) || format('apk-lint-{0}', github.ref) }} + cancel-in-progress: true steps: - uses: actions/checkout@v2 - uses: actions/cache@v2 diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 4f473567c5..143da2f5ae 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -10,18 +10,12 @@ env: CI_GRADLE_ARG_PROPERTIES: > -Porg.gradle.jvmargs=-Xmx2g -Porg.gradle.parallel=false - CI_GROUP_SUFFIX: > - ${{ - github.ref == 'refs/heads/main' && format('main-{0}', github.sha) || - github.ref == 'refs/heads/develop' && format('develop-{0}', github.sha) || - github.ref - }} jobs: unit-tests: name: Run Unit Tests runs-on: ubuntu-latest concurrency: - group: ${{ format('unit-tests-{0}', env.CI_GROUP_SUFFIX) }} + group: ${{ github.ref == 'refs/heads/main' && format('unit-test-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('unit-test-develop-{0}', github.sha) || format('unit-test-{0}', github.ref) }} cancel-in-progress: true steps: - uses: actions/checkout@v2 From 80fd816d6674d4dfbd5080136f13421a57fe1340 Mon Sep 17 00:00:00 2001 From: David Langley Date: Wed, 23 Feb 2022 23:00:22 +0000 Subject: [PATCH 14/59] test cancel with noop --- .github/workflows/tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 143da2f5ae..050376be41 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -1,3 +1,4 @@ +#no-op name: Test on: From d561ad6acd5a04b04534211aad981a46e3b59b55 Mon Sep 17 00:00:00 2001 From: David Langley Date: Wed, 23 Feb 2022 23:04:18 +0000 Subject: [PATCH 15/59] include matrix.target in apk-lint group --- .github/workflows/quality.yml | 2 +- .github/workflows/tests.yml | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index e681b850a5..4e4a534caf 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -123,7 +123,7 @@ jobs: matrix: target: [ Gplay, Fdroid ] concurrency: - group: ${{ github.ref == 'refs/heads/develop' && format('apk-lint-develop-{0}', github.sha) || format('apk-lint-{0}', github.ref) }} + group: ${{ github.ref == 'refs/heads/develop' && format('apk-lint-develop-{0}', github.sha) || format('apk-lint-{0}-{1}', matrix.target, github.ref) }} cancel-in-progress: true steps: - uses: actions/checkout@v2 diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 050376be41..143da2f5ae 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -1,4 +1,3 @@ -#no-op name: Test on: From de7a572329f3c07d4cfe7aea8bd7095653bc7586 Mon Sep 17 00:00:00 2001 From: David Langley Date: Wed, 23 Feb 2022 23:05:34 +0000 Subject: [PATCH 16/59] noop to test cancel --- .github/workflows/integration_tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/integration_tests.yml b/.github/workflows/integration_tests.yml index 4a048118e7..9869eb653b 100644 --- a/.github/workflows/integration_tests.yml +++ b/.github/workflows/integration_tests.yml @@ -1,3 +1,4 @@ +#no-op name: Integration Tests on: From 93c6216269d2bbdd2ecd3d1e248e5e28abf4d2bf Mon Sep 17 00:00:00 2001 From: David Langley Date: Wed, 23 Feb 2022 23:13:30 +0000 Subject: [PATCH 17/59] Add descriptions of concurrencies --- .github/workflows/build.yml | 2 ++ .github/workflows/integration_tests.yml | 4 +++- .github/workflows/quality.yml | 3 +++ .github/workflows/tests.yml | 1 + 4 files changed, 9 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 99cfebabe4..e601a626b2 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -19,6 +19,7 @@ jobs: fail-fast: false matrix: target: [ Gplay, Fdroid ] + # Allow all jobs on develop. Just one per PR. concurrency: group: ${{ github.ref == 'refs/heads/develop' && format('integration-tests-develop-{0}-{1}', matrix.target, github.sha) || format('build-debug-{0}-{1}', matrix.target, github.ref) }} cancel-in-progress: true @@ -45,6 +46,7 @@ jobs: name: Build unsigned GPlay APKs runs-on: ubuntu-latest if: github.ref == 'refs/heads/main' + # Only runs on main, no concurrency. steps: - uses: actions/checkout@v2 - uses: actions/cache@v2 diff --git a/.github/workflows/integration_tests.yml b/.github/workflows/integration_tests.yml index 9869eb653b..fd7a77deb5 100644 --- a/.github/workflows/integration_tests.yml +++ b/.github/workflows/integration_tests.yml @@ -1,4 +1,3 @@ -#no-op name: Integration Tests on: @@ -16,6 +15,7 @@ jobs: build-android-test-matrix-sdk: name: Matrix SDK - Build Android Tests runs-on: ubuntu-latest + # Allow all jobs on main and develop. Just one per PR. concurrency: group: ${{ github.ref == 'refs/heads/main' && format('test-matrix-sdk-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('test-matrix-sdk-develop-{0}', github.sha) || format('test-matrix-sdk-{0}', github.ref) }} cancel-in-progress: true @@ -36,6 +36,7 @@ jobs: build-android-test-app: name: App - Build Android Tests runs-on: ubuntu-latest + # Allow all jobs on main and develop. Just one per PR. concurrency: group: ${{ github.ref == 'refs/heads/main' && format('test-app-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('test-app-develop-{0}', github.sha) || format('test-app-{0}', github.ref) }} cancel-in-progress: true @@ -56,6 +57,7 @@ jobs: integration-tests: name: Matrix SDK - Running Integration Tests runs-on: ubuntu-latest + # Allow all jobs on main and develop. Just one per PR. concurrency: group: ${{ github.ref == 'refs/heads/main' && format('integration-tests-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('integration-tests-develop-{0}', github.sha) || format('integration-tests-{0}', github.ref) }} cancel-in-progress: true diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index 4e4a534caf..dfa7522e75 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -18,6 +18,7 @@ jobs: ktlint: name: Kotlin Linter runs-on: ubuntu-latest + # Allow all jobs on main and develop. Just one per PR. concurrency: group: ${{ github.ref == 'refs/heads/main' && format('klint-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('klint-develop-{0}', github.sha) || format('klint-{0}', github.ref) }} cancel-in-progress: true @@ -90,6 +91,7 @@ jobs: android-lint: name: Android Linter runs-on: ubuntu-latest + # Allow all jobs on main and develop. Just one per PR. concurrency: group: ${{ github.ref == 'refs/heads/main' && format('android-lint-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('android-lint-develop-{0}', github.sha) || format('android-lint-{0}', github.ref) }} cancel-in-progress: true @@ -122,6 +124,7 @@ jobs: fail-fast: false matrix: target: [ Gplay, Fdroid ] + # Allow all jobs on develop. Just one per PR. concurrency: group: ${{ github.ref == 'refs/heads/develop' && format('apk-lint-develop-{0}', github.sha) || format('apk-lint-{0}-{1}', matrix.target, github.ref) }} cancel-in-progress: true diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 143da2f5ae..9c7aefc336 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -14,6 +14,7 @@ jobs: unit-tests: name: Run Unit Tests runs-on: ubuntu-latest + # Allow all jobs on main and develop. Just one per PR. concurrency: group: ${{ github.ref == 'refs/heads/main' && format('unit-test-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('unit-test-develop-{0}', github.sha) || format('unit-test-{0}', github.ref) }} cancel-in-progress: true From d4a423b382dd528c64657678eeee2f43f770c4b6 Mon Sep 17 00:00:00 2001 From: David Langley Date: Thu, 24 Feb 2022 09:30:28 +0000 Subject: [PATCH 18/59] Fix spacing --- .github/workflows/build.yml | 1 + .github/workflows/integration_tests.yml | 1 + .github/workflows/tests.yml | 1 + 3 files changed, 3 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e601a626b2..d1e59e71d5 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -10,6 +10,7 @@ env: CI_GRADLE_ARG_PROPERTIES: > -Porg.gradle.jvmargs=-Xmx2g -Porg.gradle.parallel=false + jobs: debug: name: Build debug APKs (${{ matrix.target }}) diff --git a/.github/workflows/integration_tests.yml b/.github/workflows/integration_tests.yml index fd7a77deb5..3bdb460d8e 100644 --- a/.github/workflows/integration_tests.yml +++ b/.github/workflows/integration_tests.yml @@ -10,6 +10,7 @@ env: CI_GRADLE_ARG_PROPERTIES: > -Porg.gradle.jvmargs=-Xmx2g -Porg.gradle.parallel=false + jobs: # Build Android Tests [Matrix SDK] build-android-test-matrix-sdk: diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 9c7aefc336..8de12ed986 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -10,6 +10,7 @@ env: CI_GRADLE_ARG_PROPERTIES: > -Porg.gradle.jvmargs=-Xmx2g -Porg.gradle.parallel=false + jobs: unit-tests: name: Run Unit Tests From 4ebaa349c31f34275d7967d6cc05636445e252aa Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Thu, 24 Feb 2022 14:45:48 +0100 Subject: [PATCH 19/59] Reverts adding force login fallback flag to debug features --- .../app/features/debug/features/DebugFeaturesStateFactory.kt | 5 ----- .../app/features/debug/features/DebugVectorFeatures.kt | 3 --- .../src/main/java/im/vector/app/features/VectorFeatures.kt | 2 -- 3 files changed, 10 deletions(-) diff --git a/vector/src/debug/java/im/vector/app/features/debug/features/DebugFeaturesStateFactory.kt b/vector/src/debug/java/im/vector/app/features/debug/features/DebugFeaturesStateFactory.kt index 3fbe80fd98..8702c8d966 100644 --- a/vector/src/debug/java/im/vector/app/features/debug/features/DebugFeaturesStateFactory.kt +++ b/vector/src/debug/java/im/vector/app/features/debug/features/DebugFeaturesStateFactory.kt @@ -54,11 +54,6 @@ class DebugFeaturesStateFactory @Inject constructor( key = DebugFeatureKeys.onboardingPersonalize, factory = VectorFeatures::isOnboardingPersonalizeEnabled ), - createBooleanFeature( - label = "Force login fallback", - key = DebugFeatureKeys.forceLoginFallback, - factory = VectorFeatures::isForceLoginFallbackEnabled - ) )) } diff --git a/vector/src/debug/java/im/vector/app/features/debug/features/DebugVectorFeatures.kt b/vector/src/debug/java/im/vector/app/features/debug/features/DebugVectorFeatures.kt index 85d3a501f6..f93e3d96fb 100644 --- a/vector/src/debug/java/im/vector/app/features/debug/features/DebugVectorFeatures.kt +++ b/vector/src/debug/java/im/vector/app/features/debug/features/DebugVectorFeatures.kt @@ -54,8 +54,6 @@ class DebugVectorFeatures( override fun isOnboardingPersonalizeEnabled(): Boolean = read(DebugFeatureKeys.onboardingPersonalize) ?: vectorFeatures.isOnboardingPersonalizeEnabled() - override fun isForceLoginFallbackEnabled(): Boolean = read(DebugFeatureKeys.forceLoginFallback) ?: vectorFeatures.isForceLoginFallbackEnabled() - fun override(value: T?, key: Preferences.Key) = updatePreferences { if (value == null) { it.remove(key) @@ -108,5 +106,4 @@ object DebugFeatureKeys { val onboardingSplashCarousel = booleanPreferencesKey("onboarding-splash-carousel") val onboardingUseCase = booleanPreferencesKey("onbboarding-splash-carousel") val onboardingPersonalize = booleanPreferencesKey("onbboarding-personalize") - val forceLoginFallback = booleanPreferencesKey("force-login-fallback") } diff --git a/vector/src/main/java/im/vector/app/features/VectorFeatures.kt b/vector/src/main/java/im/vector/app/features/VectorFeatures.kt index 7d8d14b3af..a19b3d9026 100644 --- a/vector/src/main/java/im/vector/app/features/VectorFeatures.kt +++ b/vector/src/main/java/im/vector/app/features/VectorFeatures.kt @@ -25,7 +25,6 @@ interface VectorFeatures { fun isOnboardingSplashCarouselEnabled(): Boolean fun isOnboardingUseCaseEnabled(): Boolean fun isOnboardingPersonalizeEnabled(): Boolean - fun isForceLoginFallbackEnabled(): Boolean enum class OnboardingVariant { LEGACY, @@ -40,5 +39,4 @@ class DefaultVectorFeatures : VectorFeatures { override fun isOnboardingSplashCarouselEnabled() = true override fun isOnboardingUseCaseEnabled() = true override fun isOnboardingPersonalizeEnabled() = false - override fun isForceLoginFallbackEnabled() = false } From 2917d4e4d680f590454c153dca5bb154c1a16c32 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Thu, 24 Feb 2022 15:01:18 +0100 Subject: [PATCH 20/59] Adds forceLoginFallback private setting --- .../settings/DebugPrivateSettingsViewActions.kt | 1 + .../settings/DebugPrivateSettingsViewModel.kt | 17 +++++++++++++---- .../settings/DebugPrivateSettingsViewState.kt | 3 ++- .../app/features/settings/VectorDataStore.kt | 13 +++++++++++++ 4 files changed, 29 insertions(+), 5 deletions(-) diff --git a/vector/src/debug/java/im/vector/app/features/debug/settings/DebugPrivateSettingsViewActions.kt b/vector/src/debug/java/im/vector/app/features/debug/settings/DebugPrivateSettingsViewActions.kt index ecbb241387..1c76cf6fb2 100644 --- a/vector/src/debug/java/im/vector/app/features/debug/settings/DebugPrivateSettingsViewActions.kt +++ b/vector/src/debug/java/im/vector/app/features/debug/settings/DebugPrivateSettingsViewActions.kt @@ -20,4 +20,5 @@ import im.vector.app.core.platform.VectorViewModelAction sealed class DebugPrivateSettingsViewActions : VectorViewModelAction { data class SetDialPadVisibility(val force: Boolean) : DebugPrivateSettingsViewActions() + data class SetForceLoginFallbackEnabled(val force: Boolean) : DebugPrivateSettingsViewActions() } diff --git a/vector/src/debug/java/im/vector/app/features/debug/settings/DebugPrivateSettingsViewModel.kt b/vector/src/debug/java/im/vector/app/features/debug/settings/DebugPrivateSettingsViewModel.kt index 624c46556a..7b64349f47 100644 --- a/vector/src/debug/java/im/vector/app/features/debug/settings/DebugPrivateSettingsViewModel.kt +++ b/vector/src/debug/java/im/vector/app/features/debug/settings/DebugPrivateSettingsViewModel.kt @@ -45,15 +45,18 @@ class DebugPrivateSettingsViewModel @AssistedInject constructor( private fun observeVectorDataStore() { vectorDataStore.forceDialPadDisplayFlow.setOnEach { - copy( - dialPadVisible = it - ) + copy(dialPadVisible = it) + } + + vectorDataStore.forceLoginFallbackFlow.setOnEach { + copy(forceLoginFallback = false) } } override fun handle(action: DebugPrivateSettingsViewActions) { when (action) { - is DebugPrivateSettingsViewActions.SetDialPadVisibility -> handleSetDialPadVisibility(action) + is DebugPrivateSettingsViewActions.SetDialPadVisibility -> handleSetDialPadVisibility(action) + is DebugPrivateSettingsViewActions.SetForceLoginFallbackEnabled -> handleSetForceLoginFallbackEnabled(action) } } @@ -62,4 +65,10 @@ class DebugPrivateSettingsViewModel @AssistedInject constructor( vectorDataStore.setForceDialPadDisplay(action.force) } } + + private fun handleSetForceLoginFallbackEnabled(action: DebugPrivateSettingsViewActions.SetForceLoginFallbackEnabled) { + viewModelScope.launch { + vectorDataStore.setForceLoginFallbackFlow(action.force) + } + } } diff --git a/vector/src/debug/java/im/vector/app/features/debug/settings/DebugPrivateSettingsViewState.kt b/vector/src/debug/java/im/vector/app/features/debug/settings/DebugPrivateSettingsViewState.kt index 0ad4b185ec..7fca29af8c 100644 --- a/vector/src/debug/java/im/vector/app/features/debug/settings/DebugPrivateSettingsViewState.kt +++ b/vector/src/debug/java/im/vector/app/features/debug/settings/DebugPrivateSettingsViewState.kt @@ -19,5 +19,6 @@ package im.vector.app.features.debug.settings import com.airbnb.mvrx.MavericksState data class DebugPrivateSettingsViewState( - val dialPadVisible: Boolean = false + val dialPadVisible: Boolean = false, + val forceLoginFallback: Boolean = false, ) : MavericksState diff --git a/vector/src/main/java/im/vector/app/features/settings/VectorDataStore.kt b/vector/src/main/java/im/vector/app/features/settings/VectorDataStore.kt index 6a5ef0ac99..f742d93734 100644 --- a/vector/src/main/java/im/vector/app/features/settings/VectorDataStore.kt +++ b/vector/src/main/java/im/vector/app/features/settings/VectorDataStore.kt @@ -59,4 +59,17 @@ class VectorDataStore @Inject constructor( settings[forceDialPadDisplay] = force } } + + + private val forceLoginFallback = booleanPreferencesKey("force_login_fallback") + + val forceLoginFallbackFlow: Flow = context.dataStore.data.map { preferences -> + preferences[forceLoginFallback].orFalse() + } + + suspend fun setForceLoginFallbackFlow(force: Boolean) { + context.dataStore.edit { settings -> + settings[forceLoginFallback] = force + } + } } From 092761c1187407f9b66d9ad1e10b42de8e0507b9 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Thu, 24 Feb 2022 15:07:10 +0100 Subject: [PATCH 21/59] Adds forceLoginFallback private setting view --- .../features/debug/settings/DebugPrivateSettingsFragment.kt | 3 +++ .../debug/res/layout/fragment_debug_private_settings.xml | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/vector/src/debug/java/im/vector/app/features/debug/settings/DebugPrivateSettingsFragment.kt b/vector/src/debug/java/im/vector/app/features/debug/settings/DebugPrivateSettingsFragment.kt index 808c379354..0738af728b 100644 --- a/vector/src/debug/java/im/vector/app/features/debug/settings/DebugPrivateSettingsFragment.kt +++ b/vector/src/debug/java/im/vector/app/features/debug/settings/DebugPrivateSettingsFragment.kt @@ -43,6 +43,9 @@ class DebugPrivateSettingsFragment : VectorBaseFragment viewModel.handle(DebugPrivateSettingsViewActions.SetDialPadVisibility(isChecked)) } + views.forceLoginFallback.setOnCheckedChangeListener { _, isChecked -> + viewModel.handle(DebugPrivateSettingsViewActions.SetForceLoginFallbackEnabled(isChecked)) + } } override fun invalidate() = withState(viewModel) { diff --git a/vector/src/debug/res/layout/fragment_debug_private_settings.xml b/vector/src/debug/res/layout/fragment_debug_private_settings.xml index b4186e7bba..6760c68169 100644 --- a/vector/src/debug/res/layout/fragment_debug_private_settings.xml +++ b/vector/src/debug/res/layout/fragment_debug_private_settings.xml @@ -25,6 +25,12 @@ android:layout_height="wrap_content" android:text="Force DialPad tab display" /> + + From 981393fefb5d9b4b78689eba2171000834455dc4 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Thu, 24 Feb 2022 15:27:51 +0100 Subject: [PATCH 22/59] Changes copy value of forceLoginFallback to it --- .../features/debug/settings/DebugPrivateSettingsViewModel.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vector/src/debug/java/im/vector/app/features/debug/settings/DebugPrivateSettingsViewModel.kt b/vector/src/debug/java/im/vector/app/features/debug/settings/DebugPrivateSettingsViewModel.kt index 7b64349f47..038b1e6cc7 100644 --- a/vector/src/debug/java/im/vector/app/features/debug/settings/DebugPrivateSettingsViewModel.kt +++ b/vector/src/debug/java/im/vector/app/features/debug/settings/DebugPrivateSettingsViewModel.kt @@ -49,7 +49,7 @@ class DebugPrivateSettingsViewModel @AssistedInject constructor( } vectorDataStore.forceLoginFallbackFlow.setOnEach { - copy(forceLoginFallback = false) + copy(forceLoginFallback = it) } } From 92c6d599846734a10c2b011457952e4502b93f9a Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Thu, 24 Feb 2022 15:28:33 +0100 Subject: [PATCH 23/59] Adds private setting usage to FtueAuthVariant --- .../app/features/onboarding/OnboardingViewModel.kt | 11 ++++++++++- .../app/features/onboarding/OnboardingViewState.kt | 3 ++- .../features/onboarding/ftueauth/FtueAuthVariant.kt | 10 +++++++--- .../vector/app/features/settings/VectorDataStore.kt | 1 - 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt index ca3c3644bd..63f1875235 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt @@ -46,6 +46,7 @@ import im.vector.app.features.login.LoginMode import im.vector.app.features.login.ReAuthHelper import im.vector.app.features.login.ServerType import im.vector.app.features.login.SignMode +import im.vector.app.features.settings.VectorDataStore import kotlinx.coroutines.Job import kotlinx.coroutines.launch import org.matrix.android.sdk.api.MatrixPatterns.getDomain @@ -78,7 +79,8 @@ class OnboardingViewModel @AssistedInject constructor( private val stringProvider: StringProvider, private val homeServerHistoryService: HomeServerHistoryService, private val vectorFeatures: VectorFeatures, - private val analyticsTracker: AnalyticsTracker + private val analyticsTracker: AnalyticsTracker, + private val vectorDataStore: VectorDataStore, ) : VectorViewModel(initialState) { @AssistedFactory @@ -90,6 +92,7 @@ class OnboardingViewModel @AssistedInject constructor( init { getKnownCustomHomeServersUrls() + observeDataStore() } private fun getKnownCustomHomeServersUrls() { @@ -98,6 +101,12 @@ class OnboardingViewModel @AssistedInject constructor( } } + private fun observeDataStore() = viewModelScope.launch { + vectorDataStore.forceLoginFallbackFlow.setOnEach { isForceLoginFallbackEnabled -> + copy(isForceLoginFallbackEnabled = isForceLoginFallbackEnabled) + } + } + // Store the last action, to redo it after user has trusted the untrusted certificate private var lastAction: OnboardingAction? = null private var currentHomeServerConnectionConfig: HomeServerConnectionConfig? = null diff --git a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewState.kt b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewState.kt index 7bad2682a9..39c5094d30 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewState.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewState.kt @@ -62,7 +62,8 @@ data class OnboardingViewState( // Supported types for the login. We cannot use a sealed class for LoginType because it is not serializable @PersistState val loginModeSupportedTypes: List = emptyList(), - val knownCustomHomeServersUrls: List = emptyList() + val knownCustomHomeServersUrls: List = emptyList(), + val isForceLoginFallbackEnabled: Boolean = false, ) : MavericksState { fun isLoading(): Boolean { diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt index 8e168b5d20..c5b65508b4 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt @@ -76,6 +76,8 @@ class FtueAuthVariant( private val popEnterAnim = R.anim.no_anim private val popExitAnim = R.anim.exit_fade_out + private var isForceLoginFallbackEnabled = false + private val topFragment: Fragment? get() = supportFragmentManager.findFragmentById(views.loginFragmentContainer.id) @@ -225,7 +227,7 @@ class FtueAuthVariant( } private fun registrationShouldFallback(registrationFlowResult: OnboardingViewEvents.RegistrationFlowResult) = - vectorFeatures.isForceLoginFallbackEnabled() || registrationFlowResult.containsUnsupportedRegistrationFlow() + isForceLoginFallbackEnabled || registrationFlowResult.containsUnsupportedRegistrationFlow() private fun OnboardingViewEvents.RegistrationFlowResult.containsUnsupportedRegistrationFlow() = flowResult.missingStages.any { !it.isSupported() } @@ -244,6 +246,8 @@ class FtueAuthVariant( } private fun updateWithState(viewState: OnboardingViewState) { + isForceLoginFallbackEnabled = viewState.isForceLoginFallbackEnabled + views.loginLoading.isVisible = if (vectorFeatures.isOnboardingPersonalizeEnabled()) { viewState.isLoading() } else { @@ -286,7 +290,7 @@ class FtueAuthVariant( } private fun handleSignInSelected(state: OnboardingViewState) { - if (vectorFeatures.isForceLoginFallbackEnabled()) { + if (isForceLoginFallbackEnabled) { onLoginModeNotSupported(state.loginModeSupportedTypes) } else { disambiguateLoginMode(state) @@ -318,7 +322,7 @@ class FtueAuthVariant( } private fun handleSignInWithMatrixId(state: OnboardingViewState) { - if (vectorFeatures.isForceLoginFallbackEnabled()) { + if (isForceLoginFallbackEnabled) { onLoginModeNotSupported(state.loginModeSupportedTypes) } else { openAuthLoginFragmentWithTag(FRAGMENT_LOGIN_TAG) diff --git a/vector/src/main/java/im/vector/app/features/settings/VectorDataStore.kt b/vector/src/main/java/im/vector/app/features/settings/VectorDataStore.kt index f742d93734..a7981a8b2a 100644 --- a/vector/src/main/java/im/vector/app/features/settings/VectorDataStore.kt +++ b/vector/src/main/java/im/vector/app/features/settings/VectorDataStore.kt @@ -60,7 +60,6 @@ class VectorDataStore @Inject constructor( } } - private val forceLoginFallback = booleanPreferencesKey("force_login_fallback") val forceLoginFallbackFlow: Flow = context.dataStore.data.map { preferences -> From 3d57d72a7eb97c98407d097d2022acaca9ca0c34 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Thu, 24 Feb 2022 15:33:06 +0100 Subject: [PATCH 24/59] Reorders some functions within FtueAuthVariant --- .../onboarding/ftueauth/FtueAuthVariant.kt | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt index c5b65508b4..2083d936be 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt @@ -112,10 +112,6 @@ class FtueAuthVariant( } } - override fun setIsLoading(isLoading: Boolean) { - doNothing() - } - private fun addFirstFragment() { val splashFragment = when (vectorFeatures.isOnboardingSplashCarouselEnabled()) { true -> FtueAuthSplashCarouselFragment::class.java @@ -124,6 +120,23 @@ class FtueAuthVariant( activity.addFragment(views.loginFragmentContainer, splashFragment) } + private fun updateWithState(viewState: OnboardingViewState) { + isForceLoginFallbackEnabled = viewState.isForceLoginFallbackEnabled + views.loginLoading.isVisible = shouldShowLoading(viewState) + } + + private fun shouldShowLoading(viewState: OnboardingViewState) = + if (vectorFeatures.isOnboardingPersonalizeEnabled()) { + viewState.isLoading() + } else { + // Keep loading when during success because of the delay when switching to the next Activity + viewState.isLoading() || viewState.isAuthTaskCompleted() + } + + override fun setIsLoading(isLoading: Boolean) { + doNothing() + } + private fun handleOnboardingViewEvents(viewEvents: OnboardingViewEvents) { when (viewEvents) { is OnboardingViewEvents.RegistrationFlowResult -> { @@ -245,17 +258,6 @@ class FtueAuthVariant( .show() } - private fun updateWithState(viewState: OnboardingViewState) { - isForceLoginFallbackEnabled = viewState.isForceLoginFallbackEnabled - - views.loginLoading.isVisible = if (vectorFeatures.isOnboardingPersonalizeEnabled()) { - viewState.isLoading() - } else { - // Keep loading when during success because of the delay when switching to the next Activity - viewState.isLoading() || viewState.isAuthTaskCompleted() - } - } - private fun onWebLoginError(onWebLoginError: OnboardingViewEvents.OnWebLoginError) { // Pop the backstack supportFragmentManager.popBackStack(null, FragmentManager.POP_BACK_STACK_INCLUSIVE) From 8d0410d961af3d58af3f268c818a051141b7d2c7 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Thu, 24 Feb 2022 15:38:36 +0100 Subject: [PATCH 25/59] Removes Global doNothing function --- .../lib/core/utils/extensions/Global.kt | 20 ------------------- .../onboarding/ftueauth/FtueAuthVariant.kt | 7 ++----- 2 files changed, 2 insertions(+), 25 deletions(-) delete mode 100644 library/core-utils/src/main/java/im/vector/lib/core/utils/extensions/Global.kt diff --git a/library/core-utils/src/main/java/im/vector/lib/core/utils/extensions/Global.kt b/library/core-utils/src/main/java/im/vector/lib/core/utils/extensions/Global.kt deleted file mode 100644 index 3b508ab7b2..0000000000 --- a/library/core-utils/src/main/java/im/vector/lib/core/utils/extensions/Global.kt +++ /dev/null @@ -1,20 +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.lib.core.utils.extensions - -@Suppress("UNUSED_PARAMETER") -fun doNothing(reason: String = "") = Unit diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt index 2083d936be..2b1ada0906 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt @@ -53,7 +53,6 @@ import im.vector.app.features.onboarding.OnboardingViewModel import im.vector.app.features.onboarding.OnboardingViewState import im.vector.app.features.onboarding.ftueauth.terms.FtueAuthTermsFragment import im.vector.app.features.onboarding.ftueauth.terms.FtueAuthTermsFragmentArgument -import im.vector.lib.core.utils.extensions.doNothing import org.matrix.android.sdk.api.auth.registration.FlowResult import org.matrix.android.sdk.api.auth.registration.Stage import org.matrix.android.sdk.api.extensions.tryOrNull @@ -133,9 +132,7 @@ class FtueAuthVariant( viewState.isLoading() || viewState.isAuthTaskCompleted() } - override fun setIsLoading(isLoading: Boolean) { - doNothing() - } + override fun setIsLoading(isLoading: Boolean) = Unit private fun handleOnboardingViewEvents(viewEvents: OnboardingViewEvents) { when (viewEvents) { @@ -285,7 +282,7 @@ class FtueAuthVariant( // state.signMode could not be ready yet. So use value from the ViewEvent when (OnboardingViewEvents.signMode) { SignMode.Unknown -> error("Sign mode has to be set before calling this method") - SignMode.SignUp -> doNothing("This is managed by the OnboardingViewEvents") + SignMode.SignUp -> Unit // This case is processed in handleOnboardingViewEvents SignMode.SignIn -> handleSignInSelected(state) SignMode.SignInWithMatrixId -> handleSignInWithMatrixId(state) }.exhaustive From 9f0cef7040fa86822e1a0c83c9d4d1f1f2fd958a Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Thu, 24 Feb 2022 15:39:27 +0100 Subject: [PATCH 26/59] Removes unnecessary exhaustive on when statement --- .../vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt index 2b1ada0906..0093cb20ea 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt @@ -302,7 +302,7 @@ class FtueAuthVariant( is LoginMode.SsoAndPassword, LoginMode.Password -> openAuthLoginFragmentWithTag(FRAGMENT_LOGIN_TAG) LoginMode.Unsupported -> onLoginModeNotSupported(state.loginModeSupportedTypes) - }.exhaustive + } private fun openAuthLoginFragmentWithTag(tag: String) { activity.addFragmentToBackstack(views.loginFragmentContainer, From 65242dfb4707ab23402090b10e24ab90e75a1289 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Thu, 24 Feb 2022 16:57:04 +0100 Subject: [PATCH 27/59] Adds missing invalidation step to forceLoginFallback checkbox --- .../app/features/debug/settings/DebugPrivateSettingsFragment.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/vector/src/debug/java/im/vector/app/features/debug/settings/DebugPrivateSettingsFragment.kt b/vector/src/debug/java/im/vector/app/features/debug/settings/DebugPrivateSettingsFragment.kt index 0738af728b..b54d776901 100644 --- a/vector/src/debug/java/im/vector/app/features/debug/settings/DebugPrivateSettingsFragment.kt +++ b/vector/src/debug/java/im/vector/app/features/debug/settings/DebugPrivateSettingsFragment.kt @@ -50,5 +50,6 @@ class DebugPrivateSettingsFragment : VectorBaseFragment Date: Thu, 24 Feb 2022 16:26:22 +0000 Subject: [PATCH 28/59] Fix ktlint typo Co-authored-by: Michael Kaye <1917473+michaelkaye@users.noreply.github.com> --- .github/workflows/quality.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index dfa7522e75..4aaa5b5d30 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -20,7 +20,7 @@ jobs: runs-on: ubuntu-latest # Allow all jobs on main and develop. Just one per PR. concurrency: - group: ${{ github.ref == 'refs/heads/main' && format('klint-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('klint-develop-{0}', github.sha) || format('klint-{0}', github.ref) }} + group: ${{ github.ref == 'refs/heads/main' && format('ktlint-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('ktlint-develop-{0}', github.sha) || format('ktlint-{0}', github.ref) }} cancel-in-progress: true steps: - uses: actions/checkout@v2 From 9832f1e8e8744b1a17f08e7518a9f8684f28683d Mon Sep 17 00:00:00 2001 From: David Langley Date: Thu, 24 Feb 2022 16:28:14 +0000 Subject: [PATCH 29/59] Add matrix.target to apk-lint concurrency group for develop branch. Co-authored-by: Michael Kaye <1917473+michaelkaye@users.noreply.github.com> --- .github/workflows/quality.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index 4aaa5b5d30..02827e7f17 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -126,7 +126,7 @@ jobs: target: [ Gplay, Fdroid ] # Allow all jobs on develop. Just one per PR. concurrency: - group: ${{ github.ref == 'refs/heads/develop' && format('apk-lint-develop-{0}', github.sha) || format('apk-lint-{0}-{1}', matrix.target, github.ref) }} + group: ${{ github.ref == 'refs/heads/develop' && format('apk-lint-develop-{0}-{1}', matrix.target, github.sha) || format('apk-lint-{0}-{1}', matrix.target, github.ref) }} cancel-in-progress: true steps: - uses: actions/checkout@v2 From 79a8652308e45949f469b5227a75c700a6e0d941 Mon Sep 17 00:00:00 2001 From: David Langley Date: Thu, 24 Feb 2022 16:31:17 +0000 Subject: [PATCH 30/59] Fix unit tests concurrency group naming. Co-authored-by: Michael Kaye <1917473+michaelkaye@users.noreply.github.com> --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 8de12ed986..d6e194916b 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -17,7 +17,7 @@ jobs: runs-on: ubuntu-latest # Allow all jobs on main and develop. Just one per PR. concurrency: - group: ${{ github.ref == 'refs/heads/main' && format('unit-test-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('unit-test-develop-{0}', github.sha) || format('unit-test-{0}', github.ref) }} + group: ${{ github.ref == 'refs/heads/main' && format('unit-tests-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('unit-tests-develop-{0}', github.sha) || format('unit-tests-{0}', github.ref) }} cancel-in-progress: true steps: - uses: actions/checkout@v2 From b525259bec985588de42b78c1efbbe6a76e481b6 Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Tue, 22 Feb 2022 17:18:09 +0100 Subject: [PATCH 31/59] 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 32/59] 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 33/59] 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 34/59] 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 35/59] 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 36/59] 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 37/59] 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 38/59] 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 39/59] 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 40/59] 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 41/59] 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 42/59] 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 43/59] 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 44/59] 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 45/59] 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 46/59] 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 47/59] 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 48/59] 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 49/59] 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 7e308d10d8e256892f6f42af9c3868351216f372 Mon Sep 17 00:00:00 2001 From: David Langley Date: Thu, 24 Feb 2022 16:48:30 +0000 Subject: [PATCH 50/59] Add comments where concurrency is not required so that it is considered when making changes. --- .github/workflows/gradle-wrapper-validation.yml | 1 + .github/workflows/sanity_test.yml | 1 + .github/workflows/sync-from-external-sources.yml | 3 +++ 3 files changed, 5 insertions(+) diff --git a/.github/workflows/gradle-wrapper-validation.yml b/.github/workflows/gradle-wrapper-validation.yml index 405a2b3065..ee4a87293f 100644 --- a/.github/workflows/gradle-wrapper-validation.yml +++ b/.github/workflows/gradle-wrapper-validation.yml @@ -5,6 +5,7 @@ jobs: validation: name: "Validation" runs-on: ubuntu-latest + # No concurrency required, this is a prerequisite to other actions and should run every time. steps: - uses: actions/checkout@v2 - uses: gradle/wrapper-validation-action@v1 diff --git a/.github/workflows/sanity_test.yml b/.github/workflows/sanity_test.yml index 483926fa1f..e5122547bf 100644 --- a/.github/workflows/sanity_test.yml +++ b/.github/workflows/sanity_test.yml @@ -19,6 +19,7 @@ jobs: fail-fast: false matrix: api-level: [ 28 ] + # No concurrency required, runs every time on a schedule. steps: - uses: actions/checkout@v2 with: diff --git a/.github/workflows/sync-from-external-sources.yml b/.github/workflows/sync-from-external-sources.yml index 5a5d8152ff..691729617f 100644 --- a/.github/workflows/sync-from-external-sources.yml +++ b/.github/workflows/sync-from-external-sources.yml @@ -9,6 +9,7 @@ jobs: runs-on: ubuntu-latest # Skip in forks if: github.repository == 'vector-im/element-android' + # No concurrency required, runs every time on a schedule. steps: - uses: actions/checkout@v2 - name: Set up Python 3.8 @@ -43,6 +44,7 @@ jobs: runs-on: ubuntu-latest # Skip in forks if: github.repository == 'vector-im/element-android' + # No concurrency required, runs every time on a schedule. steps: - uses: actions/checkout@v2 - name: Set up Python 3.8 @@ -76,6 +78,7 @@ jobs: runs-on: ubuntu-latest # Skip in forks if: github.repository == 'vector-im/element-android' + # No concurrency required, runs every time on a schedule. steps: - uses: actions/checkout@v2 - name: Run analytics import script From aea78b70f1bc7cae9646ccaae9d1e84c3026341a Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Thu, 24 Feb 2022 18:08:42 +0100 Subject: [PATCH 51/59] 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 52/59] 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 53/59] 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 54/59] 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 55/59] 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 56/59] 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 795072312d17428840d15032255ed49ce5a0d56a Mon Sep 17 00:00:00 2001 From: Michael Kaye <1917473+michaelkaye@users.noreply.github.com> Date: Mon, 28 Feb 2022 09:57:01 +0000 Subject: [PATCH 57/59] Remove duplicate sanity_test.yml - now in nightly.yml --- .github/workflows/sanity_test.yml | 84 ------------------------------- 1 file changed, 84 deletions(-) delete mode 100644 .github/workflows/sanity_test.yml diff --git a/.github/workflows/sanity_test.yml b/.github/workflows/sanity_test.yml deleted file mode 100644 index 83ad067446..0000000000 --- a/.github/workflows/sanity_test.yml +++ /dev/null @@ -1,84 +0,0 @@ -name: Sanity Test - -on: - schedule: - # At 20:00 every day UTC - - cron: '0 20 * * *' - -# Enrich gradle.properties for CI/CD -env: - CI_GRADLE_ARG_PROPERTIES: > - -Porg.gradle.jvmargs=-Xmx4g - -Porg.gradle.parallel=false - -jobs: - integration-tests: - name: Sanity Tests (Synapse) - runs-on: macos-latest - strategy: - fail-fast: false - matrix: - api-level: [ 28 ] - steps: - - uses: actions/checkout@v2 - with: - ref: develop - - name: Set up Python 3.8 - uses: actions/setup-python@v2 - with: - python-version: 3.8 - - uses: actions/cache@v2 - with: - path: | - ~/.gradle/caches - ~/.gradle/wrapper - key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle*', '**/gradle-wrapper.properties') }} - restore-keys: | - ${{ runner.os }}-gradle- - - name: Start synapse server - run: | - pip install matrix-synapse - curl -sL https://raw.githubusercontent.com/matrix-org/synapse/develop/demo/start.sh \ - | sed s/127.0.0.1/0.0.0.0/g | sed 's/http:\/\/localhost/http:\/\/10.0.2.2/g' | bash -s -- --no-rate-limit - - uses: actions/setup-java@v2 - with: - distribution: 'adopt' - java-version: '11' - - name: Run sanity tests on API ${{ matrix.api-level }} - uses: reactivecircus/android-emulator-runner@v2 - with: - api-level: ${{ matrix.api-level }} - arch: x86 - profile: Nexus 5X - force-avd-creation: false - emulator-options: -no-snapshot-save -no-window -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none - emulator-build: 7425822 # workaround to emulator bug: https://github.com/ReactiveCircus/android-emulator-runner/issues/160 - script: | - adb root - adb logcat -c - touch emulator.log - chmod 777 emulator.log - adb logcat >> emulator.log & - ./gradlew $CI_GRADLE_ARG_PROPERTIES -PallWarningsAsErrors=false connectedGplayDebugAndroidTest -Pandroid.testInstrumentationRunnerArguments.class=im.vector.app.ui.UiAllScreensSanityTest || (adb pull storage/emulated/0/Pictures/failure_screenshots && exit 1 ) - - name: Upload Test Report Log - uses: actions/upload-artifact@v2 - if: always() - with: - name: sanity-error-results - path: | - emulator.log - failure_screenshots/ - - - notify: - runs-on: ubuntu-latest - needs: integration-tests - if: always() - steps: - - uses: michaelkaye/matrix-hookshot-action@v0.2.0 - with: - github_token: ${{ secrets.GITHUB_TOKEN }} - matrix_access_token: ${{ secrets.ELEMENT_ANDROID_NOTIFICATION_ACCESS_TOKEN }} - matrix_room_id: ${{ secrets.ELEMENT_ANDROID_INTERNAL_ROOM_ID }} - text_template: "Sanity test run: {{#each job_statuses }}{{#with this }}{{#if completed }} {{name}} {{conclusion}} at {{completed_at}} {{html_url}}{{/if}}{{/with}}{{/each}}" - html_template: "CI Sanity test run results: {{#each job_statuses }}{{#with this }}{{#if completed }} {{name}} {{conclusion}} at {{completed_at}} [details]{{/if}}{{/with}}{{/each}}" From 562780a169c4150b297838b2c4736b61be8594bc Mon Sep 17 00:00:00 2001 From: Maxime Naturel Date: Mon, 28 Feb 2022 11:55:14 +0100 Subject: [PATCH 58/59] 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 + } +} From c61af45216bd7609a49c1dbe884dd1e2a8165b48 Mon Sep 17 00:00:00 2001 From: David Langley Date: Mon, 28 Feb 2022 12:31:36 +0000 Subject: [PATCH 59/59] Fix spacing --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d1e59e71d5..1ba71c1f61 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -20,7 +20,7 @@ jobs: fail-fast: false matrix: target: [ Gplay, Fdroid ] - # Allow all jobs on develop. Just one per PR. + # Allow all jobs on develop. Just one per PR. concurrency: group: ${{ github.ref == 'refs/heads/develop' && format('integration-tests-develop-{0}-{1}', matrix.target, github.sha) || format('build-debug-{0}-{1}', matrix.target, github.ref) }} cancel-in-progress: true