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.

Sign up for GitHub

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

Display warning dialog for current() predicate #2524

Merged
merged 1 commit into from
Sep 11, 2018
Merged

Display warning dialog for current() predicate #2524

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
92 changes: 72 additions & 20 deletions collect_app/src/main/java/org/odk/collect/android/widgets/WidgetFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@

package org.odk.collect.android.widgets;

import android.app.AlertDialog;
import android.content.Context;
import android.content.DialogInterface;
import android.content.Intent;

import com.google.android.gms.analytics.HitBuilders;

Expand All @@ -23,6 +26,8 @@
import org.javarosa.core.model.QuestionDef;
import org.javarosa.form.api.FormEntryPrompt;
import org.javarosa.xpath.expr.XPathExpression;
import org.odk.collect.android.R;
import org.odk.collect.android.activities.WebViewActivity;
import org.odk.collect.android.application.Collect;
import org.odk.collect.android.logic.FormController;
import org.odk.collect.android.utilities.FileUtils;
Expand Down Expand Up @@ -210,7 +215,9 @@ public static QuestionWidget createWidgetFromPrompt(FormEntryPrompt fep, Context
questionWidget = new SelectOneWidget(context, fep, appearance.contains("quick"));
}

logChoiceFilterAnalytics(fep.getQuestion());
if (logChoiceFilterAnalytics(fep.getQuestion())) {
showCurrentPredicateAlert(context);
}

break;
case Constants.CONTROL_SELECT_MULTI:
Expand Down Expand Up @@ -247,6 +254,11 @@ public static QuestionWidget createWidgetFromPrompt(FormEntryPrompt fep, Context
} else {
questionWidget = new SelectMultiWidget(context, fep);
}

if (logChoiceFilterAnalytics(fep.getQuestion())) {
showCurrentPredicateAlert(context);
}

break;
case Constants.CONTROL_RANK:
questionWidget = new RankingWidget(context, fep);
Expand All @@ -266,23 +278,23 @@ public static QuestionWidget createWidgetFromPrompt(FormEntryPrompt fep, Context
questionWidget = new StringWidget(context, fep, readOnlyOverride);
break;
}

logChoiceFilterAnalytics(fep.getQuestion());

break;
default:
questionWidget = new StringWidget(context, fep, readOnlyOverride);
break;
}

return questionWidget;
}

/**
* Log analytics event each time a question with a choice filter is accessed, identifying
* choice filters with relative expressions. This will inform communication around the fix
* for a long-standing bug in JavaRosa: https://github.com/opendatakit/javarosa/issues/293
*
* @return True if a predicate with current() was found, false otherwise
*/
private static void logChoiceFilterAnalytics(QuestionDef question) {
private static boolean logChoiceFilterAnalytics(QuestionDef question) {
ItemsetBinding itemsetBinding = question.getDynamicChoices();

if (itemsetBinding != null && itemsetBinding.nodesetRef != null) {
Expand All @@ -291,20 +303,6 @@ private static void logChoiceFilterAnalytics(QuestionDef question) {
List<XPathExpression> predicates = itemsetBinding.nodesetRef.getPredicate(level);

if (predicates != null) {
// Log a hash of the form title and id joined by a space. This
// will allow us to know a rough count of unique forms that use
// current() in a predicate without compromising user privacy.
String formIdentifier = "";
FormController formController = Collect.getInstance().getFormController();
if (formController != null) {
String formID = formController.getFormDef().getMainInstance()
.getRoot().getAttributeValue("", "id");
formIdentifier = formController.getFormTitle() + " " + formID;
}

String formIdentifierHash = FileUtils.getMd5Hash(
new ByteArrayInputStream(formIdentifier.getBytes()));

for (XPathExpression predicate : predicates) {
String actionName = predicate.toString().contains("current") ?
"CurrentPredicate" : "NonCurrentPredicate";
Expand All @@ -313,13 +311,67 @@ private static void logChoiceFilterAnalytics(QuestionDef question) {
.send(new HitBuilders.EventBuilder()
.setCategory("Itemset")
.setAction(actionName)
.setLabel(formIdentifierHash)
.setLabel(getFormIdentifierHash())
.build());

if (predicate.toString().contains("current")) {
return true;
}
}
}
}
}
}
return false;
}

/**
* Gets a unique, privacy-preserving identifier for the current form.
*
* @return hash of the form title, a space, the form ID
*/
private static String getFormIdentifierHash() {
String formIdentifier = "";
FormController formController = Collect.getInstance().getFormController();
if (formController != null) {
String formID = formController.getFormDef().getMainInstance()
.getRoot().getAttributeValue("", "id");
formIdentifier = formController.getFormTitle() + " " + formID;
}

return FileUtils.getMd5Hash(
new ByteArrayInputStream(formIdentifier.getBytes()));
}

/**
* Show an alert explaining the upcoming change in current() predicates.
*/
private static void showCurrentPredicateAlert(Context context) {
AlertDialog.Builder builder = new AlertDialog.Builder(context);
builder.setTitle(R.string.current_predicate_warning_title);
builder.setMessage(R.string.current_predicate_warning);

DialogInterface.OnClickListener forumClickListener = (dialog, id) -> {
Intent intent = new Intent(context, WebViewActivity.class);
intent.putExtra("url", "https://forum.opendatakit.org/t/15122");
context.startActivity(intent);

Collect.getInstance().getDefaultTracker()
.send(new HitBuilders.EventBuilder()
.setCategory("Itemset")
.setAction("CurrentChangeViewed")
.setLabel(getFormIdentifierHash())
.build());
};

builder.setPositiveButton(R.string.current_predicate_forum, forumClickListener);

DialogInterface.OnClickListener okClickListener = (dialog, id) -> {
dialog.dismiss();
};

builder.setNegativeButton(R.string.current_predicate_continue, okClickListener);
AlertDialog dialog = builder.create();
dialog.show();
}
}
6 changes: 6 additions & 0 deletions collect_app/src/main/res/values/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -654,4 +654,10 @@
<string name="missing_google_account_dialog_title">Google account not set</string>
<string name="missing_google_account_dialog_desc">To submit forms via Google Sheets you have to set up your Google account in the Server settings.</string>
<string name="sms_invalid_phone_number_description">Set a destination phone number for SMS submissions or change your transport type.</string>

<!-- Text for temporary notification of upcoming change. Please do not translate "current()". -->
<string name="current_predicate_warning">This question will not be compatible with future versions of ODK Collect because it uses current() in a choice filter. \n\nPlease make sure the person who made this form reads the details below and takes action.</string>
<string name="current_predicate_warning_title">Action needed</string>
<string name="current_predicate_forum">Read details</string>
<string name="current_predicate_continue">Continue</string>
</resources>