refactor: minor rule changes

PavloNetrebchuk · PavloNetrebchuk · commit e08e99aba071 · 2024-11-14T18:23:18.000+02:00
diff --git a/.github/workflows/detekt.yml 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
@@ -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
@@ -1,5 +1,5 @@
 build:
-  maxIssues: 100
+  maxIssues: 0
   weights:
     complexity: 2
     LongParameterList: 1
@@ -43,19 +43,13 @@ style:
     active: true
     ignoreAnnotated:
       - 'Preview'
-  ForbiddenComment:
-    active: false
-  SerialVersionUIDInSerializableClass:
-    active: false
 
 complexity:
   active: true
   LongMethod:
     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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<ProfileInteractor>()
     private val notifier = mockk<ProfileNotifier>()
     private val analytics = mockk<ProfileAnalytics>()
+    private val config = mockk<Config>()
 
     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<AccountUpdated>()) } 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<AccountUpdated>()) } 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)

Original file line number	Diff line number	Diff line change
`@@ -184,7 +184,7 @@ val screenModule = module {`
`184`	`184`	`profileRouter = get(),`
`185`	`185`	`)`
`186`	`186`	`}`
`187`		`- viewModel { (account: Account) -> EditProfileViewModel(get(), get(), get(), get(), account) }`
	`187`	`+ viewModel { (account: Account) -> EditProfileViewModel(get(), get(), get(), get(), get(), account) }`
`188`	`188`	`viewModel { VideoSettingsViewModel(get(), get(), get(), get()) }`
`189`	`189`	`viewModel { (qualityType: String) -> VideoQualityViewModel(qualityType, get(), get(), get()) }`
`190`	`190`	`viewModel { DeleteProfileViewModel(get(), get(), get(), get(), get()) }`