fix: run-test preferences and color inconsitencies (#675)

## Proposed Changes - Check if `worker` has started before actually displaying progress. - Add color to `AddDescriptorActivity` icon - End `OoniRunV2Activity` when descriptor cannot be fetched. - Update `InstalledDescriptor` to use preference prefix properly.
ooni · Mar 16, 2024 · 73e8ea0 · 73e8ea0
1 parent 969788d
commit 73e8ea0
Show file tree

Hide file tree

Showing 10 changed files with 36 additions and 33 deletions.
diff --git a/app/src/main/java/org/openobservatory/ooniprobe/activity/MainActivity.java b/app/src/main/java/org/openobservatory/ooniprobe/activity/MainActivity.java
@@ -1,6 +1,7 @@
 package org.openobservatory.ooniprobe.activity;
 
 import static org.openobservatory.ooniprobe.common.service.RunTestService.CHANNEL_ID;
+import static org.openobservatory.ooniprobe.common.worker.UpdateDescriptorsWorkerKt.PROGRESS;
 
 import android.Manifest;
 import android.content.Context;
@@ -29,15 +30,12 @@
 import androidx.work.WorkManager;
 
 import com.google.android.material.snackbar.Snackbar;
-import com.google.common.base.Optional;
 
 import org.openobservatory.ooniprobe.R;
-import org.openobservatory.ooniprobe.activity.adddescriptor.AddDescriptorActivity;
 import org.openobservatory.ooniprobe.activity.reviewdescriptorupdates.ReviewDescriptorUpdatesActivity;
 import org.openobservatory.ooniprobe.common.Application;
 import org.openobservatory.ooniprobe.common.NotificationUtility;
 import org.openobservatory.ooniprobe.common.PreferenceManager;
-import org.openobservatory.ooniprobe.common.TaskExecutor;
 import org.openobservatory.ooniprobe.common.TestDescriptorManager;
 import org.openobservatory.ooniprobe.common.ThirdPartyServices;
 import org.openobservatory.ooniprobe.common.service.ServiceUtil;
@@ -51,14 +49,12 @@
 import org.openobservatory.ooniprobe.fragment.dynamicprogressbar.OONIRunDynamicProgressBar;
 import org.openobservatory.ooniprobe.fragment.dynamicprogressbar.OnActionListener;
 import org.openobservatory.ooniprobe.fragment.dynamicprogressbar.ProgressType;
-import org.openobservatory.ooniprobe.model.database.TestDescriptor;
 
 import java.io.Serializable;
 import java.util.concurrent.TimeUnit;
 
 import javax.inject.Inject;
 
-import kotlin.Unit;
 import localhost.toolkit.app.fragment.ConfirmDialogFragment;
 
 public class MainActivity extends ReviewUpdatesAbstractActivity implements ConfirmDialogFragment.OnConfirmedListener {
@@ -217,7 +213,9 @@ public void fetchManualUpdate() {
      */
     private void onManualUpdatesFetchComplete(WorkInfo workInfo) {
         if (workInfo != null) {
-            binding.reviewUpdateNotificationFragment.setVisibility(View.VISIBLE);
+            if (workInfo.getProgress().getInt(PROGRESS,-1) >= 0) {
+                binding.reviewUpdateNotificationFragment.setVisibility(View.VISIBLE);
+            }
             switch (workInfo.getState()) {
                 case SUCCEEDED -> {
                     String descriptor = workInfo.getOutputData().getString(ManualUpdateDescriptorsWorker.KEY_UPDATED_DESCRIPTORS);

diff --git a/...c/main/java/org/openobservatory/ooniprobe/activity/adddescriptor/AddDescriptorActivity.kt b/...c/main/java/org/openobservatory/ooniprobe/activity/adddescriptor/AddDescriptorActivity.kt
@@ -16,9 +16,6 @@ import androidx.databinding.BindingAdapter
 import androidx.lifecycle.ViewModel
 import androidx.lifecycle.ViewModelProvider
 import io.noties.markwon.Markwon
-import org.openobservatory.engine.BaseNettest
-import org.openobservatory.engine.OONIRunDescriptor
-import org.openobservatory.engine.OONIRunNettest
 import org.openobservatory.ooniprobe.R
 import org.openobservatory.ooniprobe.activity.AbstractActivity
 import org.openobservatory.ooniprobe.activity.MainActivity
@@ -85,17 +82,19 @@ class AddDescriptorActivity : AbstractActivity() {
          * @param iconName is the name of the drawable resource
          */
         @JvmStatic
-        @BindingAdapter(value = ["resource"])
-        fun setImageViewResource(imageView: ImageView, iconName: String?) {
-            /* TODO(aanorbel): Update to parse the icon name and set the correct icon.
-            * Remember to ignore icons generated when generated doing this.*/
+        @BindingAdapter(value = ["resource","color"])
+        fun setImageViewResource(imageView: ImageView, iconName: String?, color: Int?) {
             imageView.setImageResource(
                 imageView.context.resources.getIdentifier(
                     StringUtils.camelToSnake(
                         iconName
                     ), "drawable", imageView.context.packageName
                 )
-            )
+            ).apply {
+                color?.let {
+                    imageView.setColorFilter(it)
+                }
+            }
         }
 
     }

diff --git a/.../main/java/org/openobservatory/ooniprobe/activity/adddescriptor/AddDescriptorViewModel.kt b/.../main/java/org/openobservatory/ooniprobe/activity/adddescriptor/AddDescriptorViewModel.kt
@@ -1,11 +1,11 @@
 package org.openobservatory.ooniprobe.activity.adddescriptor
 
+import android.graphics.Color
+import androidx.annotation.ColorInt
 import androidx.lifecycle.MutableLiveData
 import androidx.lifecycle.ViewModel
 import com.google.android.material.checkbox.MaterialCheckBox
 import com.google.android.material.checkbox.MaterialCheckBox.CheckedState
-import org.openobservatory.engine.OONIRunDescriptor
-import org.openobservatory.engine.OONIRunNettest
 import org.openobservatory.ooniprobe.activity.adddescriptor.adapter.GroupedItem
 import org.openobservatory.ooniprobe.common.LocaleUtils
 import org.openobservatory.ooniprobe.common.TestDescriptorManager
@@ -38,6 +38,10 @@ class AddDescriptorViewModel constructor(
         this.descriptor.value = descriptor
     }
 
+    @ColorInt
+    fun getColor(): Int {
+        return Color.parseColor( descriptor.value?.color ?: "#495057")
+    }
     /**
      * This method is used to get the name of the descriptor.
      * Used by the UI during data binding.

diff --git a/app/src/main/java/org/openobservatory/ooniprobe/activity/oonirun/OoniRunV2Activity.kt b/app/src/main/java/org/openobservatory/ooniprobe/activity/oonirun/OoniRunV2Activity.kt
@@ -134,7 +134,7 @@ class OoniRunV2Activity : AbstractActivity() {
         descriptorResponse?.let {
             startActivity(AddDescriptorActivity.newIntent(this, descriptorResponse))
         } ?: run {
-            Toast.makeText(this, getString(R.string.Modal_Error), Toast.LENGTH_LONG).show()
+            finishWithError()
         }
     }
 

diff --git a/.../openobservatory/ooniprobe/activity/runtests/adapter/RunTestsExpandableListViewAdapter.kt b/.../openobservatory/ooniprobe/activity/runtests/adapter/RunTestsExpandableListViewAdapter.kt
@@ -8,12 +8,11 @@ import android.widget.ImageView
 import android.widget.TextView
 import org.openobservatory.ooniprobe.R
 import org.openobservatory.ooniprobe.activity.runtests.RunTestsViewModel
-import org.openobservatory.ooniprobe.activity.runtests.RunTestsViewModel.Companion.SELECT_NONE
 import org.openobservatory.ooniprobe.activity.runtests.RunTestsViewModel.Companion.SELECT_ALL
+import org.openobservatory.ooniprobe.activity.runtests.RunTestsViewModel.Companion.SELECT_NONE
 import org.openobservatory.ooniprobe.activity.runtests.RunTestsViewModel.Companion.SELECT_SOME
 import org.openobservatory.ooniprobe.activity.runtests.models.ChildItem
 import org.openobservatory.ooniprobe.activity.runtests.models.GroupItem
-import org.openobservatory.ooniprobe.common.OONITests
 import org.openobservatory.ooniprobe.test.test.AbstractTest
 
 
@@ -83,7 +82,9 @@ class RunTestsExpandableListViewAdapter(
 			convertView ?: LayoutInflater.from(parent.context).inflate(R.layout.run_tests_group_list_item, parent, false)
 		val groupItem = getGroup(groupPosition)
 		convertView.findViewById<TextView>(R.id.group_name).text = groupItem.title
-		convertView.findViewById<ImageView>(R.id.group_icon).setImageResource(groupItem.getDisplayIcon(parent.context))
+		val icon = convertView.findViewById<ImageView>(R.id.group_icon)
+		icon.setImageResource(groupItem.getDisplayIcon(parent.context))
+		icon.setColorFilter(groupItem.color)
 		val groupIndicator = convertView.findViewById<ImageView>(R.id.group_indicator)
 		val groupSelectionIndicator = convertView.findViewById<ImageView>(R.id.group_select_indicator)
 		val selectedAllBtnStatus = viewModel.selectedAllBtnStatus.getValue()

diff --git a/app/src/main/java/org/openobservatory/ooniprobe/adapters/DashboardAdapter.kt b/app/src/main/java/org/openobservatory/ooniprobe/adapters/DashboardAdapter.kt
@@ -62,6 +62,7 @@ class DashboardAdapter(
                         icon.setImageResource(item.getDisplayIcon(holder.itemView.context)).also {
                             if (item is InstalledDescriptor){
                                 icon.setColorFilter(item.color)
+                                holder.setIsRecyclable(false)
                             }
                         }
                     }

diff --git a/app/src/main/java/org/openobservatory/ooniprobe/common/OONIDescriptor.kt b/app/src/main/java/org/openobservatory/ooniprobe/common/OONIDescriptor.kt
@@ -136,7 +136,7 @@ abstract class AbstractDescriptor<T : BaseNettest>(
                     )
                 } ?: listOf())
             },
-            descriptor = descriptor
+            descriptor = this.descriptor
         )
     }
 
@@ -171,8 +171,8 @@ abstract class AbstractDescriptor<T : BaseNettest>(
      * @return String representing the preference prefix.
      */
     fun preferencePrefix(): String {
-        return when (descriptor?.runId != null) {
-            true -> descriptor?.preferencePrefix() ?: ""
+        return when (this.descriptor?.runId != null) {
+            true -> this.descriptor?.preferencePrefix() ?: ""
             else -> ""
         }
     }

diff --git a/app/src/main/java/org/openobservatory/ooniprobe/common/worker/UpdateDescriptorsWorker.kt b/app/src/main/java/org/openobservatory/ooniprobe/common/worker/UpdateDescriptorsWorker.kt
@@ -87,11 +87,10 @@ class ManualUpdateDescriptorsWorker(
     workerParams: WorkerParameters
 ) : Worker(context, workerParams) {
 
-    init {
+    override fun doWork(): Result {
+
         setProgressAsync(Data.Builder().putInt(PROGRESS, 0).build())
-    }
 
-    override fun doWork(): Result {
         val app = applicationContext.applicationContext as Application
         app.serviceComponent.inject(d)
 

diff --git a/app/src/main/java/org/openobservatory/ooniprobe/model/database/TestDescriptor.kt b/app/src/main/java/org/openobservatory/ooniprobe/model/database/TestDescriptor.kt
@@ -17,8 +17,8 @@ import org.openobservatory.ooniprobe.activity.runtests.models.ChildItem
 import org.openobservatory.ooniprobe.activity.runtests.models.GroupItem
 import org.openobservatory.ooniprobe.common.AbstractDescriptor
 import org.openobservatory.ooniprobe.common.AppDatabase
-import org.openobservatory.ooniprobe.common.OONITests
 import org.openobservatory.ooniprobe.common.PreferenceManager
+import org.openobservatory.ooniprobe.common.resolveStatus
 import java.io.Serializable
 import java.util.Date
 import com.raizlabs.android.dbflow.annotation.TypeConverter as TypeConverterAnnotation
@@ -124,13 +124,13 @@ class InstalledDescriptor(
             dataUsage = this.dataUsage,
             nettests = this.nettests.map { nettest ->
                 ChildItem(
-                    selected = when (this.name == OONITests.EXPERIMENTAL.label) {
-                        true -> preferenceManager.isExperimentalOn
-                        false -> preferenceManager.resolveStatus(nettest.name)
-                    }, name = nettest.name, inputs = nettest.inputs
+                    selected = preferenceManager.resolveStatus(
+                        name = nettest.name,
+                        prefix = preferencePrefix(),
+                    ), name = nettest.name, inputs = nettest.inputs
                 )
             },
-            descriptor = testDescriptor
+            descriptor = this.testDescriptor
         )
     }
 

diff --git a/app/src/main/res/layout/activity_add_descriptor.xml b/app/src/main/res/layout/activity_add_descriptor.xml
@@ -51,7 +51,8 @@
                     app:layout_constraintStart_toStartOf="parent"
                     app:layout_constraintTop_toTopOf="parent"
                     app:resource="@{viewmodel.descriptor.icon}"
-                    tools:src="@drawable/settings_icon" />
+                    tools:src="@drawable/settings_icon"
+                    app:color="@{viewmodel.color}" />
 
                 <TextView
                     android:id="@+id/title"