From e08e99aba0716936364d24195011e51af86fe0f6 Mon Sep 17 00:00:00 2001 From: PavloNetrebchuk Date: Thu, 14 Nov 2024 17:02:57 +0200 Subject: [PATCH] refactor: minor rule changes --- .github/workflows/detekt.yml | 33 +++++++++++++++++++ .../java/org/openedx/app/di/ScreenModule.kt | 2 +- config/detekt.yml | 12 ++----- .../java/org/openedx/core/config/Config.kt | 1 + .../openedx/core/domain/model/AppConfig.kt | 10 ++++-- .../core/domain/model/CourseDateBlock.kt | 19 +++++------ .../dialog/appreview/AppReviewManager.kt | 12 +++---- .../org/openedx/core/ui/WebContentScreen.kt | 7 +--- .../unit/html/HtmlUnitFragment.kt | 7 +--- .../detail/CourseDetailsFragment.kt | 7 +--- .../DiscussionCommentsViewModelTest.kt | 1 + .../presentation/edit/EditProfileFragment.kt | 3 +- .../presentation/edit/EditProfileViewModel.kt | 2 ++ .../edit/EditProfileViewModelTest.kt | 16 +++++---- 14 files changed, 74 insertions(+), 58 deletions(-) create mode 100644 .github/workflows/detekt.yml diff --git a/.github/workflows/detekt.yml b/.github/workflows/detekt.yml new file mode 100644 index 000000000..0cee02ffe --- /dev/null +++ b/.github/workflows/detekt.yml @@ -0,0 +1,33 @@ +name: Detekt + +on: + workflow_dispatch: + pull_request: { } + +env: + GRADLE_OPTS: -Dorg.gradle.daemon=false + CI_GRADLE_ARG_PROPERTIES: --stacktrace + +jobs: + linting: + name: Run Detekt + runs-on: ubuntu-latest + + steps: + - name: Checkout Repo + uses: actions/checkout@v4 + + - name: Setup Java + uses: actions/setup-java@v4 + with: + distribution: 'temurin' # See 'Supported distributions' for available options + java-version: '17' + + - name: Run Detekt + run: ./gradlew detektAll + + - name: Upload report + uses: github/codeql-action/upload-sarif@v3 + if: success() || failure() + with: + sarif_file: build/reports/detekt/detekt.sarif diff --git a/app/src/main/java/org/openedx/app/di/ScreenModule.kt b/app/src/main/java/org/openedx/app/di/ScreenModule.kt index 3c31091f8..c134e6de6 100644 --- a/app/src/main/java/org/openedx/app/di/ScreenModule.kt +++ b/app/src/main/java/org/openedx/app/di/ScreenModule.kt @@ -184,7 +184,7 @@ val screenModule = module { profileRouter = get(), ) } - viewModel { (account: Account) -> EditProfileViewModel(get(), get(), get(), get(), account) } + viewModel { (account: Account) -> EditProfileViewModel(get(), get(), get(), get(), get(), account) } viewModel { VideoSettingsViewModel(get(), get(), get(), get()) } viewModel { (qualityType: String) -> VideoQualityViewModel(qualityType, get(), get(), get()) } viewModel { DeleteProfileViewModel(get(), get(), get(), get(), get()) } diff --git a/config/detekt.yml b/config/detekt.yml index 11c63e2fc..37d257629 100644 --- a/config/detekt.yml +++ b/config/detekt.yml @@ -1,5 +1,5 @@ build: - maxIssues: 100 + maxIssues: 0 weights: complexity: 2 LongParameterList: 1 @@ -43,10 +43,6 @@ style: active: true ignoreAnnotated: - 'Preview' - ForbiddenComment: - active: false - SerialVersionUIDInSerializableClass: - active: false complexity: active: true @@ -54,8 +50,6 @@ complexity: active: true ignoreAnnotated: [ 'Composable' ] ignoreFunction: [ 'onCreateView' ] - LargeClass: - active: false LongParameterList: active: true functionThreshold: 15 @@ -64,8 +58,8 @@ complexity: ignoreAnnotated: [ 'Composable' ] TooManyFunctions: active: true - thresholdInClasses: 30 - thresholdInInterfaces: 30 + thresholdInClasses: 21 + thresholdInInterfaces: 20 ignoreAnnotatedFunctions: [ 'Composable' ] ignoreOverridden: true ignorePrivate: true diff --git a/core/src/main/java/org/openedx/core/config/Config.kt b/core/src/main/java/org/openedx/core/config/Config.kt index 53b4c3c09..1b58c7e44 100644 --- a/core/src/main/java/org/openedx/core/config/Config.kt +++ b/core/src/main/java/org/openedx/core/config/Config.kt @@ -8,6 +8,7 @@ import com.google.gson.JsonParser import org.openedx.core.domain.model.AgreementUrls import java.io.InputStreamReader +@Suppress("TooManyFunctions") class Config(context: Context) { private var configProperties: JsonObject = try { diff --git a/core/src/main/java/org/openedx/core/domain/model/AppConfig.kt b/core/src/main/java/org/openedx/core/domain/model/AppConfig.kt index 97750957f..a64e09655 100644 --- a/core/src/main/java/org/openedx/core/domain/model/AppConfig.kt +++ b/core/src/main/java/org/openedx/core/domain/model/AppConfig.kt @@ -1,14 +1,18 @@ package org.openedx.core.domain.model -import java.io.Serializable +import com.google.gson.annotations.SerializedName data class AppConfig( val courseDatesCalendarSync: CourseDatesCalendarSync = CourseDatesCalendarSync(), -) : Serializable +) data class CourseDatesCalendarSync( + @SerializedName("is_enabled") val isEnabled: Boolean = false, + @SerializedName("is_self_paced_enabled") val isSelfPacedEnabled: Boolean = false, + @SerializedName("is_instructor_paced_enabled") val isInstructorPacedEnabled: Boolean = false, + @SerializedName("is_deep_link_enabled") val isDeepLinkEnabled: Boolean = false, -) : Serializable +) diff --git a/core/src/main/java/org/openedx/core/domain/model/CourseDateBlock.kt b/core/src/main/java/org/openedx/core/domain/model/CourseDateBlock.kt index c60dcaf0a..6c2165dca 100644 --- a/core/src/main/java/org/openedx/core/domain/model/CourseDateBlock.kt +++ b/core/src/main/java/org/openedx/core/domain/model/CourseDateBlock.kt @@ -18,17 +18,14 @@ data class CourseDateBlock( val assignmentType: String? = "", ) : Parcelable { fun isCompleted(): Boolean { - return complete || ( - dateType in setOf( - DateType.COURSE_START_DATE, - DateType.COURSE_END_DATE, - DateType.CERTIFICATE_AVAILABLE_DATE, - DateType.VERIFIED_UPGRADE_DEADLINE, - DateType.VERIFICATION_DEADLINE_DATE, - ) && date.before( - Date() - ) - ) + val dateTypeInSet = dateType in setOf( + DateType.COURSE_START_DATE, + DateType.COURSE_END_DATE, + DateType.CERTIFICATE_AVAILABLE_DATE, + DateType.VERIFIED_UPGRADE_DEADLINE, + DateType.VERIFICATION_DEADLINE_DATE + ) + return complete || (dateTypeInSet && date.before(Date())) } override fun equals(other: Any?): Boolean { diff --git a/core/src/main/java/org/openedx/core/presentation/dialog/appreview/AppReviewManager.kt b/core/src/main/java/org/openedx/core/presentation/dialog/appreview/AppReviewManager.kt index d3cc91620..06a2a278a 100644 --- a/core/src/main/java/org/openedx/core/presentation/dialog/appreview/AppReviewManager.kt +++ b/core/src/main/java/org/openedx/core/presentation/dialog/appreview/AppReviewManager.kt @@ -17,13 +17,11 @@ class AppReviewManager( isDialogShowed = true val currentVersionName = reviewPreferences.formatVersionName(appData.versionName) // Check is app wasn't positive rated AND 2 minor OR 1 major app versions passed since the last review - if ( - !reviewPreferences.wasPositiveRated && - ( - currentVersionName.minorVersion - 2 >= reviewPreferences.lastReviewVersion.minorVersion || - currentVersionName.majorVersion - 1 >= reviewPreferences.lastReviewVersion.majorVersion - ) - ) { + val minorVersionPassed = + currentVersionName.minorVersion - 2 >= reviewPreferences.lastReviewVersion.minorVersion + val majorVersionPassed = + currentVersionName.majorVersion - 1 >= reviewPreferences.lastReviewVersion.majorVersion + if (!reviewPreferences.wasPositiveRated && (minorVersionPassed || majorVersionPassed)) { val dialog = RateDialogFragment.newInstance() dialog.show( supportFragmentManager, diff --git a/core/src/main/java/org/openedx/core/ui/WebContentScreen.kt b/core/src/main/java/org/openedx/core/ui/WebContentScreen.kt index 296065e01..70f320368 100644 --- a/core/src/main/java/org/openedx/core/ui/WebContentScreen.kt +++ b/core/src/main/java/org/openedx/core/ui/WebContentScreen.kt @@ -147,12 +147,7 @@ private fun WebViewContent( request: WebResourceRequest? ): Boolean { val clickUrl = request?.url?.toString() ?: "" - return if (clickUrl.isNotEmpty() && - ( - clickUrl.startsWith("http://") || - clickUrl.startsWith("https://") - ) - ) { + return if (clickUrl.isNotEmpty() && clickUrl.startsWith("http")) { context.startActivity(Intent(Intent.ACTION_VIEW, Uri.parse(clickUrl))) true } else if (clickUrl.startsWith("mailto:")) { diff --git a/course/src/main/java/org/openedx/course/presentation/unit/html/HtmlUnitFragment.kt b/course/src/main/java/org/openedx/course/presentation/unit/html/HtmlUnitFragment.kt index 3b9a87d54..607606421 100644 --- a/course/src/main/java/org/openedx/course/presentation/unit/html/HtmlUnitFragment.kt +++ b/course/src/main/java/org/openedx/course/presentation/unit/html/HtmlUnitFragment.kt @@ -320,12 +320,7 @@ private fun HTMLContentView( request: WebResourceRequest? ): Boolean { val clickUrl = request?.url?.toString() ?: "" - return if (clickUrl.isNotEmpty() && - ( - clickUrl.startsWith("http://") || - clickUrl.startsWith("https://") - ) - ) { + return if (clickUrl.isNotEmpty() && clickUrl.startsWith("http")) { context.startActivity(Intent(Intent.ACTION_VIEW, Uri.parse(clickUrl))) true } else if (clickUrl.startsWith("mailto:")) { diff --git a/discovery/src/main/java/org/openedx/discovery/presentation/detail/CourseDetailsFragment.kt b/discovery/src/main/java/org/openedx/discovery/presentation/detail/CourseDetailsFragment.kt index dd3e46863..556f61459 100644 --- a/discovery/src/main/java/org/openedx/discovery/presentation/detail/CourseDetailsFragment.kt +++ b/discovery/src/main/java/org/openedx/discovery/presentation/detail/CourseDetailsFragment.kt @@ -640,12 +640,7 @@ private fun CourseDescription( request: WebResourceRequest? ): Boolean { val clickUrl = request?.url?.toString() ?: "" - return if (clickUrl.isNotEmpty() && - ( - clickUrl.startsWith("http://") || - clickUrl.startsWith("https://") - ) - ) { + return if (clickUrl.isNotEmpty() && clickUrl.startsWith("http")) { context.startActivity( Intent(Intent.ACTION_VIEW, Uri.parse(clickUrl)) ) diff --git a/discussion/src/test/java/org/openedx/discussion/presentation/comments/DiscussionCommentsViewModelTest.kt b/discussion/src/test/java/org/openedx/discussion/presentation/comments/DiscussionCommentsViewModelTest.kt index 8c783a3fd..e9323270e 100644 --- a/discussion/src/test/java/org/openedx/discussion/presentation/comments/DiscussionCommentsViewModelTest.kt +++ b/discussion/src/test/java/org/openedx/discussion/presentation/comments/DiscussionCommentsViewModelTest.kt @@ -40,6 +40,7 @@ import org.openedx.foundation.presentation.UIMessage import org.openedx.foundation.system.ResourceManager import java.net.UnknownHostException +@Suppress("LargeClass") @OptIn(ExperimentalCoroutinesApi::class) class DiscussionCommentsViewModelTest { diff --git a/profile/src/main/java/org/openedx/profile/presentation/edit/EditProfileFragment.kt b/profile/src/main/java/org/openedx/profile/presentation/edit/EditProfileFragment.kt index da19701b4..62727f822 100644 --- a/profile/src/main/java/org/openedx/profile/presentation/edit/EditProfileFragment.kt +++ b/profile/src/main/java/org/openedx/profile/presentation/edit/EditProfileFragment.kt @@ -278,10 +278,9 @@ class EditProfileFragment : Fragment() { fos.write(bitmapData) fos.flush() fos.close() - // TODO: get applicationId instead of packageName return FileProvider.getUriForFile( requireContext(), - requireContext().packageName + ".fileprovider", + viewModel.config.getAppId() + ".fileprovider", newFile )!! } diff --git a/profile/src/main/java/org/openedx/profile/presentation/edit/EditProfileViewModel.kt b/profile/src/main/java/org/openedx/profile/presentation/edit/EditProfileViewModel.kt index 14e812d5e..dd8781cf9 100644 --- a/profile/src/main/java/org/openedx/profile/presentation/edit/EditProfileViewModel.kt +++ b/profile/src/main/java/org/openedx/profile/presentation/edit/EditProfileViewModel.kt @@ -6,6 +6,7 @@ import androidx.lifecycle.MutableLiveData import androidx.lifecycle.viewModelScope import kotlinx.coroutines.launch import org.openedx.core.R +import org.openedx.core.config.Config import org.openedx.foundation.extension.isInternetError import org.openedx.foundation.presentation.BaseViewModel import org.openedx.foundation.presentation.UIMessage @@ -24,6 +25,7 @@ class EditProfileViewModel( private val resourceManager: ResourceManager, private val notifier: ProfileNotifier, private val analytics: ProfileAnalytics, + val config: Config, account: Account, ) : BaseViewModel() { diff --git a/profile/src/test/java/org/openedx/profile/presentation/edit/EditProfileViewModelTest.kt b/profile/src/test/java/org/openedx/profile/presentation/edit/EditProfileViewModelTest.kt index f3655338c..9ea2f1d5f 100644 --- a/profile/src/test/java/org/openedx/profile/presentation/edit/EditProfileViewModelTest.kt +++ b/profile/src/test/java/org/openedx/profile/presentation/edit/EditProfileViewModelTest.kt @@ -20,6 +20,7 @@ import org.junit.Rule import org.junit.Test import org.junit.rules.TestRule import org.openedx.core.R +import org.openedx.core.config.Config import org.openedx.core.domain.model.ProfileImage import org.openedx.foundation.presentation.UIMessage import org.openedx.foundation.system.ResourceManager @@ -43,6 +44,7 @@ class EditProfileViewModelTest { private val interactor = mockk() private val notifier = mockk() private val analytics = mockk() + private val config = mockk() private val account = Account( username = "thom84", @@ -84,7 +86,7 @@ class EditProfileViewModelTest { @Test fun `updateAccount no internet connection`() = runTest { val viewModel = - EditProfileViewModel(interactor, resourceManager, notifier, analytics, account) + EditProfileViewModel(interactor, resourceManager, notifier, analytics, config, account) coEvery { interactor.updateAccount(any()) } throws UnknownHostException() viewModel.updateAccount(emptyMap()) advanceUntilIdle() @@ -99,7 +101,7 @@ class EditProfileViewModelTest { @Test fun `updateAccount unknown exception`() = runTest { val viewModel = - EditProfileViewModel(interactor, resourceManager, notifier, analytics, account) + EditProfileViewModel(interactor, resourceManager, notifier, analytics, config, account) coEvery { interactor.updateAccount(any()) } throws Exception() viewModel.updateAccount(emptyMap()) @@ -115,7 +117,7 @@ class EditProfileViewModelTest { @Test fun `updateAccount success`() = runTest { val viewModel = - EditProfileViewModel(interactor, resourceManager, notifier, analytics, account) + EditProfileViewModel(interactor, resourceManager, notifier, analytics, config, account) coEvery { interactor.updateAccount(any()) } returns account coEvery { notifier.send(any()) } returns Unit every { analytics.logEvent(any(), any()) } returns Unit @@ -132,7 +134,7 @@ class EditProfileViewModelTest { @Test fun `updateAccountAndImage no internet connection`() = runTest { val viewModel = - EditProfileViewModel(interactor, resourceManager, notifier, analytics, account) + EditProfileViewModel(interactor, resourceManager, notifier, analytics, config, account) coEvery { interactor.setProfileImage(any(), any()) } throws UnknownHostException() coEvery { interactor.updateAccount(any()) } returns account coEvery { notifier.send(AccountUpdated()) } returns Unit @@ -152,7 +154,7 @@ class EditProfileViewModelTest { @Test fun `updateAccountAndImage unknown exception`() = runTest { val viewModel = - EditProfileViewModel(interactor, resourceManager, notifier, analytics, account) + EditProfileViewModel(interactor, resourceManager, notifier, analytics, config, account) coEvery { interactor.setProfileImage(any(), any()) } throws Exception() coEvery { interactor.updateAccount(any()) } returns account coEvery { notifier.send(AccountUpdated()) } returns Unit @@ -172,7 +174,7 @@ class EditProfileViewModelTest { @Test fun `updateAccountAndImage success`() = runTest { val viewModel = - EditProfileViewModel(interactor, resourceManager, notifier, analytics, account) + EditProfileViewModel(interactor, resourceManager, notifier, analytics, config, account) coEvery { interactor.setProfileImage(any(), any()) } returns Unit coEvery { interactor.updateAccount(any()) } returns account coEvery { notifier.send(any()) } returns Unit @@ -194,7 +196,7 @@ class EditProfileViewModelTest { @Test fun `setImageUri set new value`() { val viewModel = - EditProfileViewModel(interactor, resourceManager, notifier, analytics, account) + EditProfileViewModel(interactor, resourceManager, notifier, analytics, config, account) viewModel.setImageUri(mockk()) assert(viewModel.selectedImageUri.value != null)