fix: ooni/probe#2659 (comment) (#660)

Fixes  ooni/probe#2659 (comment)

## Proposed Changes

- Update `ReadMorePlugin` to take a `maxLength` parameter with default
value `150`.
- Update `OverviewActivity` to allow modification of all automated
update items.
- Update `RunTestsActivity` to save preferences only when the tests are
run.

app/src/main/java/org/openobservatory/ooniprobe/activity/OverviewActivity.java

@@ -3,6 +3,7 @@
 import static org.openobservatory.ooniprobe.activity.overview.OverviewViewModel.SELECT_ALL;
 import static org.openobservatory.ooniprobe.activity.overview.OverviewViewModel.SELECT_NONE;
 import static org.openobservatory.ooniprobe.activity.overview.OverviewViewModel.SELECT_SOME;
+import static org.openobservatory.ooniprobe.common.PreferenceManagerExtensionKt.resolveStatus;
 
 import android.content.Context;
 import android.content.Intent;
@@ -32,6 +33,7 @@
 import org.openobservatory.ooniprobe.model.database.Result;
 
 import java.util.Locale;
+import java.util.Objects;
 
 import javax.inject.Inject;
 
@@ -70,8 +72,10 @@ protected void onCreate(@Nullable Bundle savedInstanceState) {
  setThemeColor(ContextCompat.getColor(this, descriptor.getColor()));
  binding.icon.setImageResource(descriptor.getDisplayIcon(this));
  binding.customUrl.setVisibility(descriptor.getName().equals(OONITests.WEBSITES.getLabel()) ? View.VISIBLE : View.GONE);
- Markwon markwon = Markwon.builder(this).usePlugin(new ReadMorePlugin(getString(R.string.OONIRun_ReadMore), getString(R.string.OONIRun_ReadLess))).build();
- if (descriptor.getName().equals(OONITests.EXPERIMENTAL.name())) {
+ Markwon markwon = Markwon.builder(this)
+ .usePlugin(new ReadMorePlugin(getString(R.string.OONIRun_ReadMore), getString(R.string.OONIRun_ReadLess), 400))
+ .build();
+ if (Objects.equals(descriptor.getName(), OONITests.EXPERIMENTAL.name())) {
  markwon.setMarkdown(binding.desc, descriptor.getDescription());
  if (TextUtilsCompat.getLayoutDirectionFromLocale(Locale.getDefault()) == ViewCompat.LAYOUT_DIRECTION_RTL) {
  binding.desc.setTextDirection(View.TEXT_DIRECTION_RTL);
@@ -107,14 +111,17 @@ protected void onCreate(@Nullable Bundle savedInstanceState) {
  }
  });
 
- if (adapter.isSelectedAllItems()) {
- binding.switchTests.setCheckedState(MaterialCheckBox.STATE_CHECKED);
- } else if (adapter.isNotSelectedAnyGroupItem()) {
- binding.switchTests.setCheckedState(MaterialCheckBox.STATE_UNCHECKED);
+ if (descriptor.getName().equals(OONITests.EXPERIMENTAL.getLabel())) {
+ binding.switchTests.setChecked(resolveStatus(preferenceManager, descriptor.getName(), descriptor.preferencePrefix(), true));
  } else {
- binding.switchTests.setCheckedState(MaterialCheckBox.STATE_INDETERMINATE);
+ if (adapter.isSelectedAllItems()) {
+ binding.switchTests.setCheckedState(MaterialCheckBox.STATE_CHECKED);
+ } else if (adapter.isNotSelectedAnyGroupItem()) {
+ binding.switchTests.setCheckedState(MaterialCheckBox.STATE_UNCHECKED);
+ } else {
+ binding.switchTests.setCheckedState(MaterialCheckBox.STATE_INDETERMINATE);
+ }
  }
- binding.switchTests.setEnabled(descriptor.hasPreferencePrefix());
  // Expand all groups
  for (int i = 0; i < adapter.getGroupCount(); i++) {
  binding.expandableListView.expandGroup(i);

app/src/main/java/org/openobservatory/ooniprobe/activity/overview/OverviewTestsExpandableListViewAdapter.kt

@@ -45,6 +45,7 @@ class OverviewTestsExpandableListViewAdapter(
  when (viewModel.descriptor.value?.name) {
  OONITests.EXPERIMENTAL.label -> {
  view.findViewById<TextView>(R.id.group_name).text = groupItem.name
+ view.findViewById<ImageView>(R.id.group_icon).visibility = View.GONE
  }
 
  else -> {
@@ -96,12 +97,21 @@ class OverviewTestsExpandableListViewAdapter(
  }
 
  groupCheckBox.isChecked = groupItem.selected
- // Disable experimental or webconnectivity test
+
+ /**
+ * Hide checkbox for experimental tests.
+ * Experimental tests are not configurable in the settings.
+ * Tests in this category are not permanent and are subject to change.
+ * The checkbox is hidden to prevent the user from mistakenly thinking they can be configured.
+ */
  viewModel.descriptor.value?.run {
- groupCheckBox.isEnabled = hasPreferencePrefix()
+ if (name == OONITests.EXPERIMENTAL.label) {
+ groupCheckBox.visibility = View.GONE
+ } else {
+ groupCheckBox.visibility = View.VISIBLE
+ }
  }
 
-
  if (groupItem.inputs?.isNotEmpty() == true) {
  if (isExpanded) {
  groupIndicator.setImageResource(R.drawable.expand_less)

app/src/main/java/org/openobservatory/ooniprobe/activity/overview/OverviewViewModel.kt

@@ -3,8 +3,8 @@ package org.openobservatory.ooniprobe.activity.overview
 import androidx.lifecycle.MutableLiveData
 import androidx.lifecycle.ViewModel
 import org.openobservatory.engine.BaseNettest
-import org.openobservatory.ooniprobe.activity.runtests.RunTestsViewModel
 import org.openobservatory.ooniprobe.common.OONIDescriptor
+import org.openobservatory.ooniprobe.common.OONITests
 import org.openobservatory.ooniprobe.common.PreferenceManager
 import org.openobservatory.ooniprobe.common.disableTest
 import org.openobservatory.ooniprobe.common.enableTest
@@ -28,22 +28,50 @@ class OverviewViewModel() : ViewModel() {
  val selectedAllBtnStatus: MutableLiveData<String> = MutableLiveData()
 
  init {
- selectedAllBtnStatus.postValue(RunTestsViewModel.SELECT_SOME)
+ selectedAllBtnStatus.postValue(SELECT_SOME)
  }
 
+
+ /**
+ * Set the status of the selected all button.
+ * This method will also update the preference of the tests based on the selected status.
+ * For experimental tests, a single button is used to enable/disable all tests.
+ * @param selectedStatus the status of the selected all button
+ */
  fun setSelectedAllBtnStatus(selectedStatus: String) {
  selectedAllBtnStatus.postValue(selectedStatus)
- when (selectedStatus) {
- SELECT_ALL -> {
- descriptor.value?.nettests?.forEach {
- enableTest(it.name)
+ descriptor.value?.let { desc ->
+
+ when (selectedStatus) {
+ SELECT_ALL -> {
+ when (desc.name) {
+ OONITests.EXPERIMENTAL.label -> {
+ enableTest(name = desc.name)
+ }
+
+ else -> {
+ descriptor.value?.nettests?.forEach {
+ enableTest(name = it.name)
+ }
+ }
+ }
  }
- }
 
- SELECT_NONE -> {
- descriptor.value?.nettests?.forEach {
- disableTest(it.name)
+ SELECT_NONE -> {
+ when (desc.name) {
+ OONITests.EXPERIMENTAL.label -> {
+ disableTest(name = desc.name)
+ }
+
+ else -> {
+ descriptor.value?.nettests?.forEach {
+ disableTest(name = it.name)
+ }
+ }
+ }
  }
+
+ else -> {}
  }
  }
  }

app/src/main/java/org/openobservatory/ooniprobe/activity/runtests/RunTestsActivity.kt

@@ -18,7 +18,10 @@ import org.openobservatory.ooniprobe.activity.runtests.adapter.RunTestsExpandabl
 import org.openobservatory.ooniprobe.activity.runtests.models.ChildItem
 import org.openobservatory.ooniprobe.activity.runtests.models.GroupItem
 import org.openobservatory.ooniprobe.common.OONIDescriptor
+import org.openobservatory.ooniprobe.common.OONITests
 import org.openobservatory.ooniprobe.common.PreferenceManager
+import org.openobservatory.ooniprobe.common.enableTest
+import org.openobservatory.ooniprobe.common.disableTest
 import org.openobservatory.ooniprobe.databinding.ActivityRunTestsBinding
 import java.io.Serializable
 import javax.inject.Inject
@@ -104,6 +107,7 @@ class RunTestsActivity : AbstractActivity() {
  private fun onMenuItemClickListener(menuItem: MenuItem): Boolean {
  return when (menuItem.itemId) {
  R.id.runButton -> {
+ updatePreferences()
  val selectedChildItems: List<String> = getChildItemsSelectedIdList()
  if (selectedChildItems.isNotEmpty()) {
  val testSuitesToRun = getGroupItemsAtLeastOneChildEnabled().map { groupItem ->
@@ -124,6 +128,31 @@ class RunTestsActivity : AbstractActivity() {
  }
  }
 
+ /**
+ * Update the preferences based on the selected tests.
+ * This method is used to update the preferences when the user has selected the tests that they want to run.
+ */
+ private fun updatePreferences() {
+ for (i in 0 until adapter.groupCount) {
+ val group = adapter.getGroup(i)
+ when (group.name) {
+ OONITests.EXPERIMENTAL.label -> {
+ val testNames = OONITests.EXPERIMENTAL.nettests.map { it.name };
+ when(group.nettests.filter { testNames.contains(it.name) }.map { it.selected }.all { it }) {
+ true -> preferenceManager.enableTest(OONITests.EXPERIMENTAL.label)
+ false -> preferenceManager.disableTest(OONITests.EXPERIMENTAL.label)
+ }
+ }
+ else -> group.nettests.forEach { nettest ->
+ when(nettest.selected) {
+ true -> preferenceManager.enableTest(nettest.name)
+ false -> preferenceManager.disableTest(nettest.name)
+ }
+ }
+ }
+ }
+ }
+
  private fun selectAllBtnStatusObserver(selectAllBtnStatus: String?) {
  if (!TextUtils.isEmpty(selectAllBtnStatus)) {
  when (selectAllBtnStatus) {

app/src/main/java/org/openobservatory/ooniprobe/activity/runtests/RunTestsViewModel.kt

@@ -25,41 +25,6 @@ class RunTestsViewModel() : ViewModel() {
 
  fun setSelectedAllBtnStatus(selectedStatus: String) {
  selectedAllBtnStatus.postValue(selectedStatus)
- when (selectedStatus) {
- SELECT_ALL -> {
- OONITests.INSTANT_MESSAGING.nettests.forEach {
- enableTest(it.name)
- }
- OONITests.CIRCUMVENTION.nettests.forEach {
- enableTest(it.name)
- }
- OONITests.PERFORMANCE.nettests.forEach {
- enableTest(it.name)
- }
- enableTest(OONITests.EXPERIMENTAL.label)
- }
-
- SELECT_NONE -> {
- OONITests.INSTANT_MESSAGING.nettests.forEach {
- disableTest(it.name)
- }
- OONITests.CIRCUMVENTION.nettests.forEach {
- disableTest(it.name)
- }
- OONITests.PERFORMANCE.nettests.forEach {
- disableTest(it.name)
- }
- disableTest(OONITests.EXPERIMENTAL.label)
- }
- }
- }
-
- fun disableTest(name: String) {
- preferenceManager.disableTest(name)
- }
-
- fun enableTest(name: String) {
- preferenceManager.enableTest(name)
  }
 
  companion object {

app/src/main/java/org/openobservatory/ooniprobe/activity/runtests/adapter/RunTestsExpandableListViewAdapter.kt

@@ -103,22 +103,11 @@ class RunTestsExpandableListViewAdapter(
  if (groupItem.selected) {
  if (isSelectAllChildItems(groupItem.nettests)) {
  groupSelectionIndicator.setImageResource(R.drawable.check_box)
- // NOTE: This is the only place where OONITests.EXPERIMENTAL.label is used.
- // This doesn't follow the normal rule where the component tests make up the suite.
- if (groupItem.name == OONITests.EXPERIMENTAL.label) {
- viewModel.enableTest(OONITests.EXPERIMENTAL.label)
- }
  } else {
  groupSelectionIndicator.setImageResource(R.drawable.check_box_outline_blank)
- if (groupItem.name == OONITests.EXPERIMENTAL.label) {
- viewModel.disableTest(OONITests.EXPERIMENTAL.label)
- }
  }
  } else {
  groupSelectionIndicator.setImageResource(R.drawable.check_box_outline_blank)
- if (groupItem.name == OONITests.EXPERIMENTAL.label) {
- viewModel.disableTest(OONITests.EXPERIMENTAL.label)
- }
  }
  groupSelectionIndicator.setOnClickListener {
  if (groupItem.selected && isSelectAllChildItems(groupItem.nettests)) {
@@ -193,7 +182,6 @@ class RunTestsExpandableListViewAdapter(
  setOnClickListener {
  if (childItem.selected) {
  childItem.selected = false
- viewModel.disableTest(childItem.name)
  if (isNotSelectedAnyChildItems(groupItem.nettests)) {
  groupItem.selected = false
  }
@@ -204,7 +192,6 @@ class RunTestsExpandableListViewAdapter(
  }
  } else {
  childItem.selected = true
- viewModel.enableTest(childItem.name)
  groupItem.selected = true
  if (isSelectedAllItems(groupedListData)) {
  viewModel.setSelectedAllBtnStatus(SELECT_ALL)

app/src/main/java/org/openobservatory/ooniprobe/common/OONIDescriptor.kt

@@ -70,7 +70,6 @@ open class OONIDescriptor<T : BaseNettest>(
  TestGroupItem(
  selected = when (name) {
  OONITests.EXPERIMENTAL.label -> preferenceManager.isExperimentalOn
- OONITests.WEBSITES.label -> preferenceManager.countEnabledCategory() > 0
  else -> preferenceManager.resolveStatus(
  name = it.name,
  prefix = preferencePrefix(),
@@ -345,6 +344,81 @@ enum class OONITests(
  }
 }
 
+fun autoRunTests(context: Context, preferenceManager: PreferenceManager): List<DynamicTestSuite> {
+
+ return ooniDescriptors(context).filter { ooniDescriptor ->
+ when (ooniDescriptor.name) {
+ OONITests.EXPERIMENTAL.label -> preferenceManager.resolveStatus(
+ name = ooniDescriptor.name,
+ prefix = ooniDescriptor.preferencePrefix(),
+ autoRun = true
+ )
+
+ else -> ooniDescriptor.nettests.any { nettest ->
+ preferenceManager.resolveStatus(
+ name = nettest.name,
+ prefix = ooniDescriptor.preferencePrefix(),
+ autoRun = true
+ )
+ }
+ }
+ }.map { ooniDescriptor ->
+ when (ooniDescriptor.name) {
+ OONITests.EXPERIMENTAL.label -> DynamicTestSuite(
+ name = ooniDescriptor.name,
+ title = ooniDescriptor.title,
+ shortDescription = ooniDescriptor.shortDescription,
+ description = ooniDescriptor.description,
+ icon = ooniDescriptor.getDisplayIcon(context),
+ icon_24 = ooniDescriptor.getDisplayIcon(context),
+ color = ooniDescriptor.color,
+ animation = ooniDescriptor.animation,
+ dataUsage = ooniDescriptor.dataUsage,
+ nettest = (ooniDescriptor.nettests).run {
+ this + (ooniDescriptor.longRunningTests?.filter { nettest ->
+ preferenceManager.resolveStatus(
+ name = nettest.name,
+ prefix = ooniDescriptor.preferencePrefix(),
+ autoRun = true
+ )
+ } ?: listOf())
+ }
+ ).apply {
+ autoRun = true
+ }
+
+ else -> DynamicTestSuite(
+ name = ooniDescriptor.name,
+ title = ooniDescriptor.title,
+ shortDescription = ooniDescriptor.shortDescription,
+ description = ooniDescriptor.description,
+ icon = ooniDescriptor.getDisplayIcon(context),
+ icon_24 = ooniDescriptor.getDisplayIcon(context),
+ color = ooniDescriptor.color,
+ animation = ooniDescriptor.animation,
+ dataUsage = ooniDescriptor.dataUsage,
+ nettest = (ooniDescriptor.nettests).filter { nettest ->
+ preferenceManager.resolveStatus(
+ name = nettest.name,
+ prefix = ooniDescriptor.preferencePrefix(),
+ autoRun = true
+ )
+ }.run {
+ this + (ooniDescriptor.longRunningTests?.filter { nettest ->
+ preferenceManager.resolveStatus(
+ name = nettest.name,
+ prefix = ooniDescriptor.preferencePrefix(),
+ autoRun = true
+ )
+ } ?: listOf())
+ }
+ ).apply {
+ autoRun = true
+ }
+ }
+ }
+}
+
 /**
  * Creates a list of [OONIDescriptor] representing the OONI tests.
  *