Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Block settings that restart the app when in form entry #6488

Merged
merged 6 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class FillBlankFormWithRepeatGroupTest {
.copyForm("TestRepeat.xml")
.startBlankForm("TestRepeat")
.clickOptionsIcon()
.clickGeneralSettings()
.clickProjectSettings()
.clickOnUserInterface()
.clickNavigation()
.clickUseSwipesAndButtons()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package org.odk.collect.android.feature.formentry
import org.junit.Rule
import org.junit.Test
import org.junit.rules.RuleChain
import org.odk.collect.android.R
import org.odk.collect.android.support.pages.AccessControlPage
import org.odk.collect.android.support.pages.FormEntryPage
import org.odk.collect.android.support.pages.MainMenuPage
Expand Down Expand Up @@ -111,7 +110,7 @@ class FormNavigationTest {

// change settings to 'Horizontal swipes' mode'
.clickOptionsIcon()
.clickGeneralSettings()
.clickProjectSettings()
.clickOnUserInterface()
.clickNavigation()
.clickSwipes()
Expand All @@ -124,7 +123,7 @@ class FormNavigationTest {

// change settings to 'Forward/backward buttons' mode'
.clickOptionsIcon()
.clickGeneralSettings()
.clickProjectSettings()
.clickOnUserInterface()
.clickNavigation()
.clickUseNavigationButtons()
Expand All @@ -138,7 +137,7 @@ class FormNavigationTest {

// change settings to 'Swipes and buttons' mode'
.clickOptionsIcon()
.clickGeneralSettings()
.clickProjectSettings()
.clickOnUserInterface()
.clickNavigation()
.clickUseSwipesAndButtons()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package org.odk.collect.android.feature.formentry
import org.junit.Rule
import org.junit.Test
import org.junit.rules.RuleChain
import org.odk.collect.android.R
import org.odk.collect.android.support.pages.FormEntryPage
import org.odk.collect.android.support.pages.MainMenuPage
import org.odk.collect.android.support.pages.ProjectSettingsPage
Expand Down Expand Up @@ -58,7 +57,7 @@ class FormStylingTest {
.startBlankForm(FORM_NAME)
.assertText("Guidance text")
.clickOptionsIcon()
.clickGeneralSettings()
.clickProjectSettings()
.openFormManagement()
.openShowGuidanceForQuestions()
.clickOnString(org.odk.collect.strings.R.string.guidance_yes_collapsed)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class AuditTest {
.startBlankForm("One Question Audit Track Changes")
.fillOut(FormEntryPage.QuestionAndAnswer("What is your age?", "31"))
.clickOptionsIcon()
.clickGeneralSettings()
.clickProjectSettings()

val auditLog = StorageUtils.getAuditLogForFirstInstance()
assertThat(auditLog[1].get("event"), equalTo("question"))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package org.odk.collect.android.feature.settings

import androidx.test.ext.junit.runners.AndroidJUnit4
import org.junit.Rule
import org.junit.Test
import org.junit.rules.RuleChain
import org.junit.runner.RunWith
import org.odk.collect.android.support.rules.CollectTestRule
import org.odk.collect.android.support.rules.TestRuleChain
import org.odk.collect.strings.R

@RunWith(AndroidJUnit4::class)
class FormEntrySettingsTest {

private val rule = CollectTestRule()

@get:Rule
val chain: RuleChain = TestRuleChain.chain().around(rule)

@Test
fun settingsThatResetAppAreBlocked() {
rule.startAtMainMenu()
.copyForm("one-question.xml")
.startBlankForm("One Question")
.clickOptionsIcon()
.clickProjectSettings()
.assertDisabled(R.string.project_management_section_title)

.clickOnUserInterface()
.assertDisabled(R.string.app_theme)
.assertDisabled(R.string.language)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ public FormEntryPage clickOptionsIcon() {
return clickOptionsIcon(org.odk.collect.strings.R.string.project_settings);
}

public ProjectSettingsPage clickGeneralSettings() {
public ProjectSettingsPage clickProjectSettings() {
onView(withText(getTranslatedString(org.odk.collect.strings.R.string.project_settings))).perform(click());
return new ProjectSettingsPage().assertOnPage();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,6 @@
import org.odk.collect.android.formhierarchy.FormHierarchyActivity;
import org.odk.collect.android.formhierarchy.ViewOnlyFormHierarchyActivity;
import org.odk.collect.android.fragments.MediaLoadingFragment;
import org.odk.collect.android.utilities.ChangeLockProvider;
import org.odk.collect.android.widgets.datetime.pickers.CustomDatePickerDialog;
import org.odk.collect.android.widgets.datetime.pickers.CustomTimePickerDialog;
import org.odk.collect.android.fragments.dialogs.LocationProvidersDisabledDialog;
import org.odk.collect.android.fragments.dialogs.NumberPickerDialog;
import org.odk.collect.android.fragments.dialogs.RankingWidgetDialog;
Expand All @@ -145,7 +142,6 @@
import org.odk.collect.android.listeners.FormLoaderListener;
import org.odk.collect.android.listeners.WidgetValueChangedListener;
import org.odk.collect.android.logic.ImmutableDisplayableQuestion;
import org.odk.collect.android.mainmenu.MainMenuActivity;
import org.odk.collect.android.projects.ProjectsDataService;
import org.odk.collect.android.savepoints.SavepointListener;
import org.odk.collect.android.savepoints.SavepointTask;
Expand All @@ -154,6 +150,7 @@
import org.odk.collect.android.tasks.FormLoaderTask;
import org.odk.collect.android.tasks.SaveFormIndexTask;
import org.odk.collect.android.utilities.ApplicationConstants;
import org.odk.collect.android.utilities.ChangeLockProvider;
import org.odk.collect.android.utilities.ContentUriHelper;
import org.odk.collect.android.utilities.ControllableLifecyleOwner;
import org.odk.collect.android.utilities.ExternalAppIntentProvider;
Expand All @@ -163,8 +160,10 @@
import org.odk.collect.android.utilities.SavepointsRepositoryProvider;
import org.odk.collect.android.utilities.ScreenContext;
import org.odk.collect.android.utilities.SoftKeyboardController;
import org.odk.collect.android.widgets.datetime.DateTimeWidget;
import org.odk.collect.android.widgets.QuestionWidget;
import org.odk.collect.android.widgets.datetime.DateTimeWidget;
import org.odk.collect.android.widgets.datetime.pickers.CustomDatePickerDialog;
import org.odk.collect.android.widgets.datetime.pickers.CustomTimePickerDialog;
import org.odk.collect.android.widgets.interfaces.WidgetDataReceiver;
import org.odk.collect.android.widgets.items.SelectOneFromMapDialogFragment;
import org.odk.collect.android.widgets.range.RangePickerDecimalWidget;
Expand Down Expand Up @@ -1792,23 +1791,12 @@ protected void onResume() {
if (fec != null) {
loadingComplete(formLoaderTask, formLoaderTask.getFormDef(), null);
} else {
DialogFragmentUtils.dismissDialog(FormLoadingDialogFragment.class, getSupportFragmentManager());
FormLoaderTask t = formLoaderTask;
formLoaderTask = null;
t.cancel();
t.destroy();
// there is no formController -- fire MainMenu activity?
Timber.w("Starting MainMenuActivity because formController is null/formLoaderTask not null");
startActivity(new Intent(this, MainMenuActivity.class));
throw new IllegalStateException("Null formController!");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This (and the below case) don't have tests and as far as I'm aware, we don't know how they happen. Crashing means we don't have to deal with form session/lock clean up, and I actually think it's a better experience now that we show the crash message on the next relaunch than mysteriously returning to the Main Menu without warning.

}
}
} else {
if (formController == null && !identityPromptViewModel.requiresIdentityToContinue().getValue()) {
// there is no formController -- fire MainMenu activity?
Timber.w("Starting MainMenuActivity because formController is null/formLoaderTask is null");
startActivity(new Intent(this, MainMenuActivity.class));
exit();
return;
throw new IllegalStateException("Null formController!");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,9 @@ class FormEntryMenuProvider(
showIfNotShowing(RecordingWarningDialogFragment::class.java, activity.supportFragmentManager)
} else {
formEntryViewModel.updateAnswersForScreen(answersProvider.answers, false)
val pref = Intent(activity, ProjectPreferencesActivity::class.java)
activity.startActivityForResult(pref, ApplicationConstants.RequestCodes.CHANGE_SETTINGS)
val intent = Intent(activity, ProjectPreferencesActivity::class.java)
intent.putExtra(ProjectPreferencesActivity.EXTRA_IN_FORM_ENTRY, true)
activity.startActivityForResult(intent, ApplicationConstants.RequestCodes.CHANGE_SETTINGS)
}
true
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import org.odk.collect.android.fragments.dialogs.MovingBackwardsDialog.MovingBac
import org.odk.collect.android.fragments.dialogs.ResetSettingsResultDialog.ResetSettingsResultDialogListener
import org.odk.collect.android.injection.DaggerUtils
import org.odk.collect.android.mainmenu.MainMenuActivity
import org.odk.collect.androidshared.ui.FragmentFactoryBuilder
import org.odk.collect.metadata.PropertyManager
import org.odk.collect.strings.localization.LocalizedActivity
import javax.inject.Inject
Expand All @@ -37,6 +38,12 @@ class ProjectPreferencesActivity :
lateinit var propertyManager: PropertyManager

override fun onCreate(savedInstanceState: Bundle?) {
supportFragmentManager.fragmentFactory = FragmentFactoryBuilder()
.forClass(ProjectPreferencesFragment::class.java) {
ProjectPreferencesFragment(intent.getBooleanExtra(EXTRA_IN_FORM_ENTRY, false))
}
.build()

super.onCreate(savedInstanceState)
setContentView(R.layout.activity_preferences_layout)
DaggerUtils.getComponent(this).inject(this)
Expand Down Expand Up @@ -67,4 +74,8 @@ class ProjectPreferencesActivity :
}

fun isInstanceStateSaved() = isInstanceStateSaved

companion object {
const val EXTRA_IN_FORM_ENTRY = "in_form_entry"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import android.view.MenuInflater
import android.view.MenuItem
import android.view.View
import android.view.ViewGroup
import androidx.core.os.bundleOf
import androidx.preference.Preference
import org.odk.collect.android.R
import org.odk.collect.android.injection.DaggerUtils
Expand All @@ -33,7 +34,7 @@ import org.odk.collect.androidshared.data.Consumable
import org.odk.collect.androidshared.ui.DialogFragmentUtils
import org.odk.collect.androidshared.ui.multiclicksafe.MultiClickGuard

class ProjectPreferencesFragment :
class ProjectPreferencesFragment(private val inFormEntry: Boolean) :
BaseProjectPreferencesFragment(),
Preference.OnPreferenceClickListener {

Expand Down Expand Up @@ -66,16 +67,27 @@ class ProjectPreferencesFragment :
findPreference<Preference>(EXPERIMENTAL_PREFERENCE_KEY)!!.onPreferenceClickListener = this
findPreference<Preference>(UNLOCK_PROTECTED_SETTINGS_PREFERENCE_KEY)!!.onPreferenceClickListener = this
findPreference<Preference>(CHANGE_ADMIN_PASSWORD_PREFERENCE_KEY)!!.onPreferenceClickListener = this
findPreference<Preference>(PROJECT_MANAGEMENT_PREFERENCE_KEY)!!.onPreferenceClickListener = this
findPreference<Preference>(ACCESS_CONTROL_PREFERENCE_KEY)!!.onPreferenceClickListener = this
findPreference<Preference>(PROJECT_MANAGEMENT_PREFERENCE_KEY)!!.also {
it.onPreferenceClickListener = this
if (inFormEntry) {
it.isEnabled = false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could it be made clearer that these settings are disabled? The visual effect feels off - the title isn't grayed out, only the summary, which makes it hard to notice the difference at first glance.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally agree! We can tweak that if we need, so I'd vote we go with this for now and follow up if we're still unhappy with it.

it.setSummary(org.odk.collect.strings.R.string.setting_not_available_during_form_entry)
}
}
}

override fun onPreferenceClick(preference: Preference): Boolean {
if (MultiClickGuard.allowClick(javaClass.name)) {
when (preference.key) {
PROTOCOL_PREFERENCE_KEY -> displayPreferences(ServerPreferencesFragment())
PROJECT_DISPLAY_PREFERENCE_KEY -> displayPreferences(ProjectDisplayPreferencesFragment())
USER_INTERFACE_PREFERENCE_KEY -> displayPreferences(UserInterfacePreferencesFragment())
USER_INTERFACE_PREFERENCE_KEY -> {
val fragment = UserInterfacePreferencesFragment()
fragment.arguments =
grzesiek2010 marked this conversation as resolved.
Show resolved Hide resolved
bundleOf(UserInterfacePreferencesFragment.ARG_IN_FORM_ENTRY to inFormEntry)
displayPreferences(fragment)
}
MAPS_PREFERENCE_KEY -> displayPreferences(MapsPreferencesFragment())
FORM_MANAGEMENT_PREFERENCE_KEY -> displayPreferences(FormManagementPreferencesFragment())
USER_AND_DEVICE_IDENTITY_PREFERENCE_KEY -> displayPreferences(IdentityPreferencesFragment())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,25 +22,38 @@

import android.content.Context;
import android.os.Bundle;

import androidx.preference.ListPreference;

import org.odk.collect.android.R;
import org.odk.collect.android.injection.DaggerUtils;
import org.odk.collect.android.mainmenu.MainMenuActivity;
import org.odk.collect.android.utilities.LocaleHelper;
import org.odk.collect.android.version.VersionInformation;

import java.util.ArrayList;
import java.util.TreeMap;

import javax.inject.Inject;

public class UserInterfacePreferencesFragment extends BaseProjectPreferencesFragment {

public static final String ARG_IN_FORM_ENTRY = "in_form_entry";

@Inject
VersionInformation versionInformation;

private boolean inFormEntry;

@Override
public void onAttach(Context context) {
super.onAttach(context);
DaggerUtils.getComponent(context).inject(this);

Bundle arguments = getArguments();
if (arguments != null) {
inFormEntry = arguments.getBoolean(ARG_IN_FORM_ENTRY);
}
}

@Override
Expand All @@ -58,16 +71,21 @@ private void initThemePrefs() {
final ListPreference pref = findPreference(KEY_APP_THEME);

if (pref != null) {
pref.setSummary(pref.getEntry());
pref.setOnPreferenceChangeListener((preference, newValue) -> {
int index = ((ListPreference) preference).findIndexOfValue(newValue.toString());
String entry = (String) ((ListPreference) preference).getEntries()[index];
if (pref.getEntry() == null || !pref.getEntry().equals(entry)) {
preference.setSummary(entry);
startActivityAndCloseAllOthers(getActivity(), MainMenuActivity.class);
}
return true;
});
if (!inFormEntry) {
pref.setSummary(pref.getEntry());
pref.setOnPreferenceChangeListener((preference, newValue) -> {
int index = ((ListPreference) preference).findIndexOfValue(newValue.toString());
String entry = (String) ((ListPreference) preference).getEntries()[index];
if (pref.getEntry() == null || !pref.getEntry().equals(entry)) {
preference.setSummary(entry);
startActivityAndCloseAllOthers(getActivity(), MainMenuActivity.class);
}
return true;
});
} else {
pref.setEnabled(false);
pref.setSummary(org.odk.collect.strings.R.string.setting_not_available_during_form_entry);
}
}
}

Expand Down Expand Up @@ -103,31 +121,36 @@ private void initLanguagePrefs() {
final ListPreference pref = findPreference(KEY_APP_LANGUAGE);

if (pref != null) {
TreeMap<String, String> languageList = LocaleHelper.languageList();
ArrayList<String> entryValues = new ArrayList<>();
entryValues.add(0, "");
entryValues.addAll(languageList.values());
pref.setEntryValues(entryValues.toArray(new String[0]));
ArrayList<String> entries = new ArrayList<>();
entries.add(0, getActivity().getResources()
.getString(org.odk.collect.strings.R.string.use_device_language));
entries.addAll(languageList.keySet());
pref.setEntries(entries.toArray(new String[0]));
if (pref.getValue() == null) {
//set Default value to "Use phone locale"
pref.setValueIndex(0);
}
pref.setSummary(pref.getEntry());
pref.setOnPreferenceChangeListener((preference, newValue) -> {
int index = ((ListPreference) preference).findIndexOfValue(newValue.toString());
String entry = (String) ((ListPreference) preference).getEntries()[index];
preference.setSummary(entry);
if (!inFormEntry) {
TreeMap<String, String> languageList = LocaleHelper.languageList();
ArrayList<String> entryValues = new ArrayList<>();
entryValues.add(0, "");
entryValues.addAll(languageList.values());
pref.setEntryValues(entryValues.toArray(new String[0]));
ArrayList<String> entries = new ArrayList<>();
entries.add(0, getActivity().getResources()
.getString(org.odk.collect.strings.R.string.use_device_language));
entries.addAll(languageList.keySet());
pref.setEntries(entries.toArray(new String[0]));
if (pref.getValue() == null) {
//set Default value to "Use phone locale"
pref.setValueIndex(0);
}
pref.setSummary(pref.getEntry());
pref.setOnPreferenceChangeListener((preference, newValue) -> {
int index = ((ListPreference) preference).findIndexOfValue(newValue.toString());
String entry = (String) ((ListPreference) preference).getEntries()[index];
preference.setSummary(entry);

settingsProvider.getUnprotectedSettings().save(KEY_APP_LANGUAGE, newValue.toString());
settingsProvider.getUnprotectedSettings().save(KEY_APP_LANGUAGE, newValue.toString());

startActivityAndCloseAllOthers(getActivity(), MainMenuActivity.class);
return true;
});
startActivityAndCloseAllOthers(getActivity(), MainMenuActivity.class);
return true;
});
} else {
pref.setEnabled(false);
pref.setSummary(org.odk.collect.strings.R.string.setting_not_available_during_form_entry);
}
}
}
}
Loading