From 3881e2986c36c232583fbf408b5877966846c2c7 Mon Sep 17 00:00:00 2001 From: Raff Tsai Date: Sun, 17 Feb 2019 01:22:39 +0800 Subject: [PATCH] Do not log Contextual card display when card is dismissed If the db change comes from dismiss card uri. We don't need to log card display again. Fixes: 121196921 Test: Robolectric Change-Id: I4e222187fafa8325e803fa6ee17ebb0b51fb8cb2 --- .../contextualcards/CardContentProvider.java | 8 +++++- .../contextualcards/CardDatabaseHelper.java | 4 +-- .../contextualcards/ContextualCardLoader.java | 20 ++++++++++----- .../slices/SliceContextualCardRenderer.java | 3 ++- .../ContextualCardLoaderTest.java | 25 +++++++++++++++++++ .../SliceContextualCardControllerTest.java | 6 ++--- 6 files changed, 53 insertions(+), 13 deletions(-) diff --git a/src/com/android/settings/homepage/contextualcards/CardContentProvider.java b/src/com/android/settings/homepage/contextualcards/CardContentProvider.java index e7ede14d663..a9a832d6640 100644 --- a/src/com/android/settings/homepage/contextualcards/CardContentProvider.java +++ b/src/com/android/settings/homepage/contextualcards/CardContentProvider.java @@ -39,12 +39,18 @@ public class CardContentProvider extends ContentProvider { public static final String CARD_AUTHORITY = "com.android.settings.homepage.CardContentProvider"; - public static final Uri URI = new Uri.Builder() + public static final Uri REFRESH_CARD_URI = new Uri.Builder() .scheme(ContentResolver.SCHEME_CONTENT) .authority(CardContentProvider.CARD_AUTHORITY) .appendPath(CardDatabaseHelper.CARD_TABLE) .build(); + public static final Uri DELETE_CARD_URI = new Uri.Builder() + .scheme(ContentResolver.SCHEME_CONTENT) + .authority(CardContentProvider.CARD_AUTHORITY) + .appendPath(CardDatabaseHelper.CardColumns.CARD_DISMISSED) + .build(); + private static final String TAG = "CardContentProvider"; /** URI matcher for ContentProvider queries. */ private static final UriMatcher URI_MATCHER = new UriMatcher(UriMatcher.NO_MATCH); diff --git a/src/com/android/settings/homepage/contextualcards/CardDatabaseHelper.java b/src/com/android/settings/homepage/contextualcards/CardDatabaseHelper.java index b9bab21386b..39c48c1e847 100644 --- a/src/com/android/settings/homepage/contextualcards/CardDatabaseHelper.java +++ b/src/com/android/settings/homepage/contextualcards/CardDatabaseHelper.java @@ -208,7 +208,7 @@ public class CardDatabaseHelper extends SQLiteOpenHelper { * Mark a specific ContextualCard with dismissal flag in the database to indicate that the * card has been dismissed. * - * @param context Context + * @param context Context * @param cardName The card name of the ContextualCard which is dismissed by user. * @return The number of rows updated */ @@ -220,7 +220,7 @@ public class CardDatabaseHelper extends SQLiteOpenHelper { final String[] selectionArgs = {cardName}; final int rowsUpdated = database.update(CARD_TABLE, values, selection, selectionArgs); database.close(); - context.getContentResolver().notifyChange(CardContentProvider.URI, null); + context.getContentResolver().notifyChange(CardContentProvider.DELETE_CARD_URI, null); return rowsUpdated; } } diff --git a/src/com/android/settings/homepage/contextualcards/ContextualCardLoader.java b/src/com/android/settings/homepage/contextualcards/ContextualCardLoader.java index d6ea6ca668a..ea6ac43c693 100644 --- a/src/com/android/settings/homepage/contextualcards/ContextualCardLoader.java +++ b/src/com/android/settings/homepage/contextualcards/ContextualCardLoader.java @@ -59,13 +59,16 @@ public class ContextualCardLoader extends AsyncLoaderCompat private final ContentObserver mObserver = new ContentObserver( new Handler(Looper.getMainLooper())) { @Override - public void onChange(boolean selfChange) { + public void onChange(boolean selfChange, Uri uri) { if (isStarted()) { + mNotifyUri = uri; forceLoad(); } } }; + @VisibleForTesting + Uri mNotifyUri; private Context mContext; ContextualCardLoader(Context context) { @@ -77,7 +80,10 @@ public class ContextualCardLoader extends AsyncLoaderCompat @Override protected void onStartLoading() { super.onStartLoading(); - mContext.getContentResolver().registerContentObserver(CardContentProvider.URI, + mNotifyUri = null; + mContext.getContentResolver().registerContentObserver(CardContentProvider.REFRESH_CARD_URI, + false /*notifyForDescendants*/, mObserver); + mContext.getContentResolver().registerContentObserver(CardContentProvider.DELETE_CARD_URI, false /*notifyForDescendants*/, mObserver); } @@ -156,10 +162,12 @@ public class ContextualCardLoader extends AsyncLoaderCompat // Two large cards return visibleCards; } finally { - //TODO(b/121196921): Should not call this if user click dismiss - final ContextualCardFeatureProvider contextualCardFeatureProvider = - FeatureFactory.getFactory(mContext).getContextualCardFeatureProvider(mContext); - contextualCardFeatureProvider.logContextualCardDisplay(visibleCards, hiddenCards); + if (!CardContentProvider.DELETE_CARD_URI.equals(mNotifyUri)) { + final ContextualCardFeatureProvider contextualCardFeatureProvider = + FeatureFactory.getFactory(mContext) + .getContextualCardFeatureProvider(mContext); + contextualCardFeatureProvider.logContextualCardDisplay(visibleCards, hiddenCards); + } } } diff --git a/src/com/android/settings/homepage/contextualcards/slices/SliceContextualCardRenderer.java b/src/com/android/settings/homepage/contextualcards/slices/SliceContextualCardRenderer.java index 2d40efe4f63..48e9f1e7266 100644 --- a/src/com/android/settings/homepage/contextualcards/slices/SliceContextualCardRenderer.java +++ b/src/com/android/settings/homepage/contextualcards/slices/SliceContextualCardRenderer.java @@ -118,7 +118,8 @@ public class SliceContextualCardRenderer implements ContextualCardRenderer, Life sliceLiveData.observe(mLifecycleOwner, slice -> { if (slice == null) { Log.w(TAG, "Slice is null"); - mContext.getContentResolver().notifyChange(CardContentProvider.URI, null); + mContext.getContentResolver().notifyChange(CardContentProvider.REFRESH_CARD_URI, + null); return; } else { //TODO(b/120629936): Take this out once blank card issue is fixed. diff --git a/tests/robotests/src/com/android/settings/homepage/contextualcards/ContextualCardLoaderTest.java b/tests/robotests/src/com/android/settings/homepage/contextualcards/ContextualCardLoaderTest.java index 18aa1c4720b..8b04ef3fed6 100644 --- a/tests/robotests/src/com/android/settings/homepage/contextualcards/ContextualCardLoaderTest.java +++ b/tests/robotests/src/com/android/settings/homepage/contextualcards/ContextualCardLoaderTest.java @@ -24,7 +24,9 @@ import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; import android.content.Context; import android.net.Uri; @@ -33,6 +35,7 @@ import androidx.slice.Slice; import com.android.settings.R; import com.android.settings.slices.CustomSliceRegistry; +import com.android.settings.testutils.FakeFeatureFactory; import org.junit.Before; import org.junit.Test; @@ -52,6 +55,7 @@ public class ContextualCardLoaderTest { private Context mContext; private ContextualCardLoader mContextualCardLoader; private EligibleCardChecker mEligibleCardChecker; + private FakeFeatureFactory mFakeFeatureFactory; @Before public void setUp() { @@ -59,6 +63,7 @@ public class ContextualCardLoaderTest { mContextualCardLoader = spy(new ContextualCardLoader(mContext)); mEligibleCardChecker = spy(new EligibleCardChecker(mContext, getContextualCard(TEST_SLICE_URI))); + mFakeFeatureFactory = FakeFeatureFactory.setupForTest(); } @Test @@ -158,6 +163,26 @@ public class ContextualCardLoaderTest { assertThat(mContextualCardLoader.loadInBackground()).isEmpty(); } + @Test + public void getDisplayableCards_refreshCardUri_shouldLogContextualCardDisplay() { + mContextualCardLoader.mNotifyUri = CardContentProvider.REFRESH_CARD_URI; + + mContextualCardLoader.getDisplayableCards(new ArrayList()); + + verify(mFakeFeatureFactory.mContextualCardFeatureProvider).logContextualCardDisplay( + any(List.class), any(List.class)); + } + + @Test + public void getDisplayableCards_deleteCardUri_shouldNotLogContextualCardDisplay() { + mContextualCardLoader.mNotifyUri = CardContentProvider.DELETE_CARD_URI; + + mContextualCardLoader.getDisplayableCards(new ArrayList()); + + verify(mFakeFeatureFactory.mContextualCardFeatureProvider, never()) + .logContextualCardDisplay(any(List.class), any(List.class)); + } + private ContextualCard getContextualCard(String sliceUri) { return new ContextualCard.Builder() .setName("test_card") diff --git a/tests/robotests/src/com/android/settings/homepage/contextualcards/slices/SliceContextualCardControllerTest.java b/tests/robotests/src/com/android/settings/homepage/contextualcards/slices/SliceContextualCardControllerTest.java index 7e1a32c81f4..e97e01e9701 100644 --- a/tests/robotests/src/com/android/settings/homepage/contextualcards/slices/SliceContextualCardControllerTest.java +++ b/tests/robotests/src/com/android/settings/homepage/contextualcards/slices/SliceContextualCardControllerTest.java @@ -75,7 +75,7 @@ public class SliceContextualCardControllerTest { @Test public void onDismissed_cardShouldBeMarkedAsDismissed() { - final Uri providerUri = CardContentProvider.URI; + final Uri providerUri = CardContentProvider.REFRESH_CARD_URI; mResolver.insert(providerUri, generateOneRow()); doNothing().when(mController).showFeedbackDialog(any(ContextualCard.class)); @@ -96,7 +96,7 @@ public class SliceContextualCardControllerTest { @Test public void onDismissed_noFeedbackEmail_shouldNotShowFeedbackDialog() { - mResolver.insert(CardContentProvider.URI, generateOneRow()); + mResolver.insert(CardContentProvider.REFRESH_CARD_URI, generateOneRow()); final ContextualCardsFragment fragment = FragmentController.of(new ContextualCardsFragment()).create().get(); final ShadowActivity shadowActivity = Shadows.shadowOf(fragment.getActivity()); @@ -109,7 +109,7 @@ public class SliceContextualCardControllerTest { @Test @Config(qualifiers = "mcc999") public void onDismissed_hasFeedbackEmail_shouldShowFeedbackDialog() { - mResolver.insert(CardContentProvider.URI, generateOneRow()); + mResolver.insert(CardContentProvider.REFRESH_CARD_URI, generateOneRow()); final ContextualCardsFragment fragment = FragmentController.of(new ContextualCardsFragment()).create().get(); final ShadowActivity shadowActivity = Shadows.shadowOf(fragment.getActivity());