From fe47ed4eddbdc0cb2600dfcb41c868ae3e70a178 Mon Sep 17 00:00:00 2001 From: Sam Bosley Date: Fri, 27 Apr 2012 13:08:15 -0700 Subject: [PATCH] Added documentation and comments --- .../astrid/activity/TaskListActivity.java | 2 ++ .../com/todoroo/astrid/dao/ABTestEventDao.java | 16 ++++++++++++++++ .../com/todoroo/astrid/data/ABTestEvent.java | 5 +++++ .../todoroo/astrid/service/StartupService.java | 3 ++- .../astrid/service/abtesting/ABChooser.java | 7 ++----- .../abtesting/ABTestEventReportingService.java | 13 +++++++++++++ .../service/abtesting/ABTestInvoker.java | 18 ++++++++++++++++++ .../astrid/service/abtesting/ABTests.java | 11 +++++------ 8 files changed, 63 insertions(+), 12 deletions(-) diff --git a/astrid/src/com/todoroo/astrid/activity/TaskListActivity.java b/astrid/src/com/todoroo/astrid/activity/TaskListActivity.java index 0e9f4b692..7b7c4be03 100644 --- a/astrid/src/com/todoroo/astrid/activity/TaskListActivity.java +++ b/astrid/src/com/todoroo/astrid/activity/TaskListActivity.java @@ -205,6 +205,8 @@ public class TaskListActivity extends AstridActivity implements MainMenuListener trackActivitySource(); } + // Have to call this here because sometimes StartupService + // isn't called (i.e. if the app was silently alive in the background) abTestEventReportingService.trackUserRetention(); } diff --git a/astrid/src/com/todoroo/astrid/dao/ABTestEventDao.java b/astrid/src/com/todoroo/astrid/dao/ABTestEventDao.java index aa860ff13..7eeaaff31 100644 --- a/astrid/src/com/todoroo/astrid/dao/ABTestEventDao.java +++ b/astrid/src/com/todoroo/astrid/dao/ABTestEventDao.java @@ -21,6 +21,14 @@ public class ABTestEventDao extends DatabaseDao { setDatabase(database); } + /** + * Creates the baseline +0 days event (i.e. the first time the user + * launches the app containing this test) + * @param testName - the name of the test + * @param testVariant - which option was chosen for this user + * @param newUser - are they a new user? + * @param activeUser - are they an activated user? + */ public void createInitialTestEvent(String testName, String testVariant, boolean newUser, boolean activeUser) { ABTestEvent event = new ABTestEvent(); @@ -36,6 +44,9 @@ public class ABTestEventDao extends DatabaseDao { /** * Only public for unit testing--don't use unless you really mean it! + * + * Creates data points for the specified test name, creating one data point + * for each time interval that hasn't yet been recorded up to the specified one * @param testName * @param timeInterval */ @@ -72,6 +83,11 @@ public class ABTestEventDao extends DatabaseDao { return; } + /** + * For each baseline data point that exists in the database, check the current + * time against the time that baseline was recorded and report the appropriate + * +n days events. Called on startup. + */ public void createRelativeDateEvents() { TodorooCursor allEvents = query(Query.select(ABTestEvent.TEST_NAME, ABTestEvent.DATE_RECORDED) .where(ABTestEvent.TIME_INTERVAL.eq(ABTestEvent.TIME_INTERVAL_0))); diff --git a/astrid/src/com/todoroo/astrid/data/ABTestEvent.java b/astrid/src/com/todoroo/astrid/data/ABTestEvent.java index 4644d7c49..6cd0c6d05 100644 --- a/astrid/src/com/todoroo/astrid/data/ABTestEvent.java +++ b/astrid/src/com/todoroo/astrid/data/ABTestEvent.java @@ -12,6 +12,11 @@ import com.todoroo.andlib.data.Table; import com.todoroo.andlib.data.TodorooCursor; import com.todoroo.astrid.api.AstridApiConstants; +/** + * Model for data points used for tracking user retention in AB tests + * @author Sam + * + */ @SuppressWarnings("nls") public class ABTestEvent extends AbstractModel { diff --git a/astrid/src/com/todoroo/astrid/service/StartupService.java b/astrid/src/com/todoroo/astrid/service/StartupService.java index 052d1fb6c..77c1eae61 100644 --- a/astrid/src/com/todoroo/astrid/service/StartupService.java +++ b/astrid/src/com/todoroo/astrid/service/StartupService.java @@ -175,11 +175,12 @@ public class StartupService { upgradeService.performSecondaryUpgrade(context); - // perform startup activities in a background thread final int finalLatestVersion = latestSetVersion; + // For any uninitialized ab test, make sure an option is chosen abChooser.makeChoicesForAllTests(latestSetVersion == 0, taskService.getUserActivationStatus()); + // perform startup activities in a background thread new Thread(new Runnable() { public void run() { // start widget updating alarm diff --git a/astrid/src/com/todoroo/astrid/service/abtesting/ABChooser.java b/astrid/src/com/todoroo/astrid/service/abtesting/ABChooser.java index 08a3c6bfb..00ec3350a 100644 --- a/astrid/src/com/todoroo/astrid/service/abtesting/ABChooser.java +++ b/astrid/src/com/todoroo/astrid/service/abtesting/ABChooser.java @@ -43,11 +43,8 @@ public class ABChooser { } /** - * Retrieves the choice for the specified feature if already made, - * or chooses one randomly from the distribution of feature probabilities - * if not. - * - * The statistics session needs to be open here in order to collect statistics + * If a choice/variant has not yet been selected for the specified test, + * make one and create the initial +0 analytics data point * * @param testKey - the preference key string of the option (defined in ABTests) */ diff --git a/astrid/src/com/todoroo/astrid/service/abtesting/ABTestEventReportingService.java b/astrid/src/com/todoroo/astrid/service/abtesting/ABTestEventReportingService.java index 9b82e5cce..426ff19eb 100644 --- a/astrid/src/com/todoroo/astrid/service/abtesting/ABTestEventReportingService.java +++ b/astrid/src/com/todoroo/astrid/service/abtesting/ABTestEventReportingService.java @@ -17,6 +17,14 @@ import com.todoroo.astrid.dao.ABTestEventDao; import com.todoroo.astrid.data.ABTestEvent; import com.todoroo.astrid.service.StatisticsService; +/** + * Service to manage the reporting of launch events for AB testing. + * On startup, queries the ABTestEvent database for unreported data + * points, merges them into the expected JSONArray format, and + * pushes them to the server. + * @author Sam + * + */ @SuppressWarnings("nls") public final class ABTestEventReportingService { @@ -37,6 +45,11 @@ public final class ABTestEventReportingService { DependencyInjectionService.getInstance().inject(this); } + /** + * Called on startup from TaskListActivity. Creates any +n days + * launch events that need to be recorded, and pushes all unreported + * data to the server. + */ public void trackUserRetention() { new Thread(new Runnable() { @Override diff --git a/astrid/src/com/todoroo/astrid/service/abtesting/ABTestInvoker.java b/astrid/src/com/todoroo/astrid/service/abtesting/ABTestInvoker.java index 1b56e41b1..b7a048e52 100644 --- a/astrid/src/com/todoroo/astrid/service/abtesting/ABTestInvoker.java +++ b/astrid/src/com/todoroo/astrid/service/abtesting/ABTestInvoker.java @@ -19,6 +19,11 @@ import com.todoroo.andlib.service.Autowired; import com.todoroo.andlib.service.DependencyInjectionService; import com.todoroo.andlib.service.RestClient; +/** + * Invoker for communicating with the Astrid Analytics server + * @author Sam + * + */ @SuppressWarnings("nls") public class ABTestInvoker { @@ -33,6 +38,13 @@ public class ABTestInvoker { DependencyInjectionService.getInstance().inject(this); } + /** + * Posts the payload to the analytics server + * @param payload - JSONArray of data points. Created by the + * helper method in ABTestReportingService + * @return + * @throws IOException + */ public JSONObject post(JSONArray payload) throws IOException { try { HttpEntity postData = createPostData(payload); @@ -51,6 +63,12 @@ public class ABTestInvoker { } + /** + * Converts the JSONArray payload into an HTTPEntity suitable for + * POSTing. + * @param payload + * @return + */ private HttpEntity createPostData(JSONArray payload) { List params = new ArrayList(); params.add(new BasicNameValuePair("apikey", API_KEY)); diff --git a/astrid/src/com/todoroo/astrid/service/abtesting/ABTests.java b/astrid/src/com/todoroo/astrid/service/abtesting/ABTests.java index 17c9f0e86..8d2648721 100644 --- a/astrid/src/com/todoroo/astrid/service/abtesting/ABTests.java +++ b/astrid/src/com/todoroo/astrid/service/abtesting/ABTests.java @@ -93,22 +93,21 @@ public class ABTests { * @param testKey = "" * --This key is used to identify the option in the application and in the preferences * - * @param probs = { int, int, ... } + * @param newUserProbs = { int, int, ... } + * @param existingUserProbs = { int, int, ... } * --The different choices in an option correspond to an index in the probability array. * Probabilities are expressed as integers to easily define relative weights. For example, * the array { 1, 2 } would mean option 0 would happen one time for every two occurrences of option 1 * + * The first array is used for new users and the second is used for existing/upgrading users, + * allowing us to specify different distributions for each group. + * * (optional) * @param descriptions = { "...", "...", ... } * --A string description of each option. Useful for tagging events. The index of * each description should correspond to the events location in the probability array * (i.e. the arrays should be the same length if this one exists) * - * (optional) - * @param relevantEvents = { "...", "...", ... } - * --An arbitrary length list of relevant localytics events. When events are - * tagged from StatisticsService, they will be appended with attributes - * that have that event in this array */ public void addTest(String testKey, int[] newUserProbs, int[] existingUserProbs, String[] descriptions) { ABTestBundle bundle = new ABTestBundle(newUserProbs, existingUserProbs, descriptions);