From 25d8049bb2d494dae4d55a30c3f290565b42d779 Mon Sep 17 00:00:00 2001 From: Soroosh Mariooryad Date: Fri, 10 Feb 2017 11:12:31 -0800 Subject: [PATCH] Modifying setting suggestion logging to only log the shown items. Previously if there was three suggestions in the suggestions view, all three would be logged as shown, although by default only two of them are shown and the third one is shown only if the view is expanded. Now, only the actual shown items will be logged. Test: RunSettingsRoboTests Fixes: b/35348496 Change-Id: Ic3af7961b4713f48e63c51ac599cb55bf69975ff --- .../settings/dashboard/DashboardAdapter.java | 62 +++++++++++++++++++ .../settings/dashboard/DashboardSummary.java | 35 +---------- .../dashboard/DashboardAdapterTest.java | 51 +++++++++++++++ 3 files changed, 114 insertions(+), 34 deletions(-) diff --git a/src/com/android/settings/dashboard/DashboardAdapter.java b/src/com/android/settings/dashboard/DashboardAdapter.java index 514eaf2b7a5..26037aba7f8 100644 --- a/src/com/android/settings/dashboard/DashboardAdapter.java +++ b/src/com/android/settings/dashboard/DashboardAdapter.java @@ -49,7 +49,9 @@ import com.android.settingslib.drawer.DashboardCategory; import com.android.settingslib.drawer.Tile; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Set; public class DashboardAdapter extends RecyclerView.Adapter implements SummaryLoader.SummaryConsumer { @@ -57,11 +59,13 @@ public class DashboardAdapter extends RecyclerView.Adapter mSuggestionsShownLogged; private SuggestionParser mSuggestionParser; private boolean mFirstFrameDrawn; @@ -120,6 +124,10 @@ public class DashboardAdapter extends RecyclerView.Adapter(); } mDashboardData = new DashboardData.Builder() @@ -161,6 +169,25 @@ public class DashboardAdapter extends RecyclerView.Adapter shownSuggestions = null; + switch (mDashboardData.getSuggestionMode()) { + case DashboardData.SUGGESTION_MODE_DEFAULT: + shownSuggestions = suggestions.subList(0, + Math.min(suggestions.size(), DashboardData.DEFAULT_SUGGESTION_COUNT)); + break; + case DashboardData.SUGGESTION_MODE_EXPANDED: + shownSuggestions = suggestions; + break; + } + if (shownSuggestions != null) { + for (Tile suggestion : shownSuggestions) { + String suggestionId = getSuggestionIdentifier(mContext, suggestion); + mMetricsFeatureProvider.action( + mContext, MetricsEvent.ACTION_SHOW_SETTINGS_SUGGESTION, + getSuggestionIdentifier(mContext, suggestion)); + mSuggestionsShownLogged.add(getSuggestionIdentifier(mContext, suggestion)); + } + } } public void setCategory(List category) { @@ -234,6 +261,13 @@ public class DashboardAdapter extends RecyclerView.Adapter expandedSuggestions = mDashboardData.getSuggestions().subList( + DashboardData.DEFAULT_SUGGESTION_COUNT, + mDashboardData.getSuggestions().size()); + for (Tile suggestion : expandedSuggestions) { + String suggestionId = + DashboardAdapter.getSuggestionIdentifier(mContext, suggestion); + if (!mSuggestionsShownLogged.contains(suggestionId)) { + mMetricsFeatureProvider.action( + mContext, MetricsEvent.ACTION_SHOW_SETTINGS_SUGGESTION, + suggestionId); + mSuggestionsShownLogged.add(suggestionId); + } + } } else { suggestionMode = DashboardData.SUGGESTION_MODE_COLLAPSED; } @@ -426,6 +487,7 @@ public class DashboardAdapter extends RecyclerView.Adapter(categories)); } outState.putInt(STATE_SUGGESTION_MODE, mDashboardData.getSuggestionMode()); + outState.putStringArrayList(STATE_SUGGESTIONS_SHOWN_LOGGED, mSuggestionsShownLogged); } private static class IconCache { diff --git a/src/com/android/settings/dashboard/DashboardSummary.java b/src/com/android/settings/dashboard/DashboardSummary.java index ce1cf650173..41dfdf7da43 100644 --- a/src/com/android/settings/dashboard/DashboardSummary.java +++ b/src/com/android/settings/dashboard/DashboardSummary.java @@ -60,8 +60,6 @@ public class DashboardSummary extends InstrumentedFragment private static final String SUGGESTIONS = "suggestions"; private static final String EXTRA_SCROLL_POSITION = "scroll_position"; - private static final String EXTRA_SUGGESTION_SHOWN_LOGGED = "suggestions_shown_logged"; - private static final String EXTRA_SUGGESTION_HIDDEN_LOGGED = "suggestions_hidden_logged"; private final Handler mHandler = new Handler(); @@ -73,8 +71,6 @@ public class DashboardSummary extends InstrumentedFragment private SuggestionRanker mSuggestionRanker; private LinearLayoutManager mLayoutManager; private SuggestionsChecks mSuggestionsChecks; - private ArrayList mSuggestionsShownLogged; - private ArrayList mSuggestionsHiddenLogged; private DashboardFeatureProvider mDashboardFeatureProvider; private SuggestionFeatureProvider mSuggestionFeatureProvider; @@ -106,15 +102,6 @@ public class DashboardSummary extends InstrumentedFragment mSuggestionRanker = new SuggestionRanker( new SuggestionFeaturizer(new EventStore(activity))); mSuggestionsChecks = new SuggestionsChecks(getContext()); - if (savedInstanceState == null) { - mSuggestionsShownLogged = new ArrayList<>(); - mSuggestionsHiddenLogged = new ArrayList<>(); - } else { - mSuggestionsShownLogged = - savedInstanceState.getStringArrayList(EXTRA_SUGGESTION_SHOWN_LOGGED); - mSuggestionsHiddenLogged = - savedInstanceState.getStringArrayList(EXTRA_SUGGESTION_HIDDEN_LOGGED); - } if (DEBUG_TIMING) { Log.d(TAG, "onCreate took " + (System.currentTimeMillis() - startTime) + " ms"); @@ -158,18 +145,8 @@ public class DashboardSummary extends InstrumentedFragment mMetricsFeatureProvider.hidden(getContext(), c.getMetricsConstant()); } } - if (mAdapter.getSuggestions() == null) { - return; - } if (!getActivity().isChangingConfigurations()) { - for (Tile suggestion : mAdapter.getSuggestions()) { - String id = DashboardAdapter.getSuggestionIdentifier(getContext(), suggestion); - if (!mSuggestionsHiddenLogged.contains(id)) { - mSuggestionsHiddenLogged.add(id); - mMetricsFeatureProvider.action(getContext(), - MetricsEvent.ACTION_HIDE_SETTINGS_SUGGESTION, id); - } - } + mAdapter.onPause(); } } @@ -200,8 +177,6 @@ public class DashboardSummary extends InstrumentedFragment @Override public void onSaveInstanceState(Bundle outState) { super.onSaveInstanceState(outState); - outState.putStringArrayList(EXTRA_SUGGESTION_HIDDEN_LOGGED, mSuggestionsHiddenLogged); - outState.putStringArrayList(EXTRA_SUGGESTION_SHOWN_LOGGED, mSuggestionsShownLogged); if (mLayoutManager == null) return; outState.putInt(EXTRA_SCROLL_POSITION, mLayoutManager.findFirstVisibleItemPosition()); if (mAdapter != null) { @@ -280,20 +255,12 @@ public class DashboardSummary extends InstrumentedFragment } // TODO: create a Suggestion class to maintain the id and other info mSuggestionRanker.rank(suggestions, suggestionIds); - // TODO: consider showing only top-k (e.g., top-3) } for (int i = 0; i < suggestions.size(); i++) { Tile suggestion = suggestions.get(i); if (mSuggestionsChecks.isSuggestionComplete(suggestion)) { mAdapter.disableSuggestion(suggestion); suggestions.remove(i--); - } else if (context != null) { - String id = DashboardAdapter.getSuggestionIdentifier(context, suggestion); - if (!mSuggestionsShownLogged.contains(id)) { - mSuggestionsShownLogged.add(id); - mMetricsFeatureProvider.action(context, - MetricsEvent.ACTION_SHOW_SETTINGS_SUGGESTION, id); - } } } return suggestions; diff --git a/tests/robotests/src/com/android/settings/dashboard/DashboardAdapterTest.java b/tests/robotests/src/com/android/settings/dashboard/DashboardAdapterTest.java index 187e2debb65..e9851798f25 100644 --- a/tests/robotests/src/com/android/settings/dashboard/DashboardAdapterTest.java +++ b/tests/robotests/src/com/android/settings/dashboard/DashboardAdapterTest.java @@ -15,20 +15,34 @@ */ package com.android.settings.dashboard; +import android.content.ComponentName; import android.content.Context; +import android.content.Intent; +import android.content.res.TypedArray; import android.view.View; +import com.android.internal.logging.nano.MetricsProto.MetricsEvent; import com.android.settings.SettingsRobolectricTestRunner; import com.android.settings.TestConfig; import com.android.settings.core.instrumentation.MetricsFeatureProvider; import com.android.settings.dashboard.conditional.Condition; +import com.android.settingslib.drawer.DashboardCategory; +import com.android.settingslib.drawer.Tile; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.robolectric.annotation.Config; +import java.util.ArrayList; +import java.util.List; + import static com.google.common.truth.Truth.assertThat; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @RunWith(SettingsRobolectricTestRunner.class) @@ -42,6 +56,12 @@ public class DashboardAdapterTest { private Condition mCondition; @Mock private MetricsFeatureProvider mMetricsFeatureProvider; + @Mock + private TypedArray mTypedArray; + @Captor + private ArgumentCaptor mActionCategoryCaptor = ArgumentCaptor.forClass(Integer.class); + @Captor + private ArgumentCaptor mActionPackageCaptor = ArgumentCaptor.forClass(String.class); private DashboardAdapter mDashboardAdapter; @Before @@ -59,4 +79,35 @@ public class DashboardAdapterTest { mDashboardAdapter.setConditions(null); assertThat(mDashboardAdapter.mDashboardData.getExpandedCondition()).isNull(); } + + @Test + public void testSuggestionsLogs() { + when(mTypedArray.getColor(any(int.class), any(int.class))).thenReturn(0); + when(mContext.obtainStyledAttributes(any(int[].class))).thenReturn(mTypedArray); + List suggestions = new ArrayList(); + suggestions.add(makeSuggestion("pkg1", "cls1")); + suggestions.add(makeSuggestion("pkg2", "cls2")); + suggestions.add(makeSuggestion("pkg3", "cls3")); + mDashboardAdapter.setCategoriesAndSuggestions( + new ArrayList(), suggestions); + mDashboardAdapter.onPause(); + verify(mMetricsFeatureProvider, times(4)).action( + any(Context.class), mActionCategoryCaptor.capture(), mActionPackageCaptor.capture()); + String[] expectedPackages = new String[] {"pkg1", "pkg2", "pkg1", "pkg2"}; + Integer[] expectedActions = new Integer[] { + MetricsEvent.ACTION_SHOW_SETTINGS_SUGGESTION, + MetricsEvent.ACTION_SHOW_SETTINGS_SUGGESTION, + MetricsEvent.ACTION_HIDE_SETTINGS_SUGGESTION, + MetricsEvent.ACTION_HIDE_SETTINGS_SUGGESTION}; + assertThat(mActionPackageCaptor.getAllValues().toArray()).isEqualTo(expectedPackages); + assertThat(mActionCategoryCaptor.getAllValues().toArray()).isEqualTo(expectedActions); + } + + private Tile makeSuggestion(String pkgName, String className) { + Tile suggestion = new Tile(); + suggestion.intent = new Intent("action"); + suggestion.intent.setComponent(new ComponentName(pkgName, className)); + return suggestion; + } + }