Merge "Do not log Contextual card display when card is dismissed"

This commit is contained in:
Raff Tsai
2019-02-27 23:20:28 +00:00
committed by Android (Google) Code Review
6 changed files with 53 additions and 13 deletions

View File

@@ -39,12 +39,18 @@ public class CardContentProvider extends ContentProvider {
public static final String CARD_AUTHORITY = "com.android.settings.homepage.CardContentProvider"; 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) .scheme(ContentResolver.SCHEME_CONTENT)
.authority(CardContentProvider.CARD_AUTHORITY) .authority(CardContentProvider.CARD_AUTHORITY)
.appendPath(CardDatabaseHelper.CARD_TABLE) .appendPath(CardDatabaseHelper.CARD_TABLE)
.build(); .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"; private static final String TAG = "CardContentProvider";
/** URI matcher for ContentProvider queries. */ /** URI matcher for ContentProvider queries. */
private static final UriMatcher URI_MATCHER = new UriMatcher(UriMatcher.NO_MATCH); private static final UriMatcher URI_MATCHER = new UriMatcher(UriMatcher.NO_MATCH);

View File

@@ -208,7 +208,7 @@ public class CardDatabaseHelper extends SQLiteOpenHelper {
* Mark a specific ContextualCard with dismissal flag in the database to indicate that the * Mark a specific ContextualCard with dismissal flag in the database to indicate that the
* card has been dismissed. * card has been dismissed.
* *
* @param context Context * @param context Context
* @param cardName The card name of the ContextualCard which is dismissed by user. * @param cardName The card name of the ContextualCard which is dismissed by user.
* @return The number of rows updated * @return The number of rows updated
*/ */
@@ -220,7 +220,7 @@ public class CardDatabaseHelper extends SQLiteOpenHelper {
final String[] selectionArgs = {cardName}; final String[] selectionArgs = {cardName};
final int rowsUpdated = database.update(CARD_TABLE, values, selection, selectionArgs); final int rowsUpdated = database.update(CARD_TABLE, values, selection, selectionArgs);
database.close(); database.close();
context.getContentResolver().notifyChange(CardContentProvider.URI, null); context.getContentResolver().notifyChange(CardContentProvider.DELETE_CARD_URI, null);
return rowsUpdated; return rowsUpdated;
} }
} }

View File

@@ -59,13 +59,16 @@ public class ContextualCardLoader extends AsyncLoaderCompat<List<ContextualCard>
private final ContentObserver mObserver = new ContentObserver( private final ContentObserver mObserver = new ContentObserver(
new Handler(Looper.getMainLooper())) { new Handler(Looper.getMainLooper())) {
@Override @Override
public void onChange(boolean selfChange) { public void onChange(boolean selfChange, Uri uri) {
if (isStarted()) { if (isStarted()) {
mNotifyUri = uri;
forceLoad(); forceLoad();
} }
} }
}; };
@VisibleForTesting
Uri mNotifyUri;
private Context mContext; private Context mContext;
ContextualCardLoader(Context context) { ContextualCardLoader(Context context) {
@@ -77,7 +80,10 @@ public class ContextualCardLoader extends AsyncLoaderCompat<List<ContextualCard>
@Override @Override
protected void onStartLoading() { protected void onStartLoading() {
super.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); false /*notifyForDescendants*/, mObserver);
} }
@@ -156,10 +162,12 @@ public class ContextualCardLoader extends AsyncLoaderCompat<List<ContextualCard>
// Two large cards // Two large cards
return visibleCards; return visibleCards;
} finally { } finally {
//TODO(b/121196921): Should not call this if user click dismiss if (!CardContentProvider.DELETE_CARD_URI.equals(mNotifyUri)) {
final ContextualCardFeatureProvider contextualCardFeatureProvider = final ContextualCardFeatureProvider contextualCardFeatureProvider =
FeatureFactory.getFactory(mContext).getContextualCardFeatureProvider(mContext); FeatureFactory.getFactory(mContext)
contextualCardFeatureProvider.logContextualCardDisplay(visibleCards, hiddenCards); .getContextualCardFeatureProvider(mContext);
contextualCardFeatureProvider.logContextualCardDisplay(visibleCards, hiddenCards);
}
} }
} }

View File

@@ -118,7 +118,8 @@ public class SliceContextualCardRenderer implements ContextualCardRenderer, Life
sliceLiveData.observe(mLifecycleOwner, slice -> { sliceLiveData.observe(mLifecycleOwner, slice -> {
if (slice == null) { if (slice == null) {
Log.w(TAG, "Slice is null"); Log.w(TAG, "Slice is null");
mContext.getContentResolver().notifyChange(CardContentProvider.URI, null); mContext.getContentResolver().notifyChange(CardContentProvider.REFRESH_CARD_URI,
null);
return; return;
} else { } else {
//TODO(b/120629936): Take this out once blank card issue is fixed. //TODO(b/120629936): Take this out once blank card issue is fixed.

View File

@@ -24,7 +24,9 @@ import static com.google.common.truth.Truth.assertThat;
import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy; import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
import android.content.Context; import android.content.Context;
import android.net.Uri; import android.net.Uri;
@@ -33,6 +35,7 @@ import androidx.slice.Slice;
import com.android.settings.R; import com.android.settings.R;
import com.android.settings.slices.CustomSliceRegistry; import com.android.settings.slices.CustomSliceRegistry;
import com.android.settings.testutils.FakeFeatureFactory;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
@@ -52,6 +55,7 @@ public class ContextualCardLoaderTest {
private Context mContext; private Context mContext;
private ContextualCardLoader mContextualCardLoader; private ContextualCardLoader mContextualCardLoader;
private EligibleCardChecker mEligibleCardChecker; private EligibleCardChecker mEligibleCardChecker;
private FakeFeatureFactory mFakeFeatureFactory;
@Before @Before
public void setUp() { public void setUp() {
@@ -59,6 +63,7 @@ public class ContextualCardLoaderTest {
mContextualCardLoader = spy(new ContextualCardLoader(mContext)); mContextualCardLoader = spy(new ContextualCardLoader(mContext));
mEligibleCardChecker = mEligibleCardChecker =
spy(new EligibleCardChecker(mContext, getContextualCard(TEST_SLICE_URI))); spy(new EligibleCardChecker(mContext, getContextualCard(TEST_SLICE_URI)));
mFakeFeatureFactory = FakeFeatureFactory.setupForTest();
} }
@Test @Test
@@ -158,6 +163,26 @@ public class ContextualCardLoaderTest {
assertThat(mContextualCardLoader.loadInBackground()).isEmpty(); assertThat(mContextualCardLoader.loadInBackground()).isEmpty();
} }
@Test
public void getDisplayableCards_refreshCardUri_shouldLogContextualCardDisplay() {
mContextualCardLoader.mNotifyUri = CardContentProvider.REFRESH_CARD_URI;
mContextualCardLoader.getDisplayableCards(new ArrayList<ContextualCard>());
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<ContextualCard>());
verify(mFakeFeatureFactory.mContextualCardFeatureProvider, never())
.logContextualCardDisplay(any(List.class), any(List.class));
}
private ContextualCard getContextualCard(String sliceUri) { private ContextualCard getContextualCard(String sliceUri) {
return new ContextualCard.Builder() return new ContextualCard.Builder()
.setName("test_card") .setName("test_card")

View File

@@ -75,7 +75,7 @@ public class SliceContextualCardControllerTest {
@Test @Test
public void onDismissed_cardShouldBeMarkedAsDismissed() { public void onDismissed_cardShouldBeMarkedAsDismissed() {
final Uri providerUri = CardContentProvider.URI; final Uri providerUri = CardContentProvider.REFRESH_CARD_URI;
mResolver.insert(providerUri, generateOneRow()); mResolver.insert(providerUri, generateOneRow());
doNothing().when(mController).showFeedbackDialog(any(ContextualCard.class)); doNothing().when(mController).showFeedbackDialog(any(ContextualCard.class));
@@ -96,7 +96,7 @@ public class SliceContextualCardControllerTest {
@Test @Test
public void onDismissed_noFeedbackEmail_shouldNotShowFeedbackDialog() { public void onDismissed_noFeedbackEmail_shouldNotShowFeedbackDialog() {
mResolver.insert(CardContentProvider.URI, generateOneRow()); mResolver.insert(CardContentProvider.REFRESH_CARD_URI, generateOneRow());
final ContextualCardsFragment fragment = final ContextualCardsFragment fragment =
FragmentController.of(new ContextualCardsFragment()).create().get(); FragmentController.of(new ContextualCardsFragment()).create().get();
final ShadowActivity shadowActivity = Shadows.shadowOf(fragment.getActivity()); final ShadowActivity shadowActivity = Shadows.shadowOf(fragment.getActivity());
@@ -109,7 +109,7 @@ public class SliceContextualCardControllerTest {
@Test @Test
@Config(qualifiers = "mcc999") @Config(qualifiers = "mcc999")
public void onDismissed_hasFeedbackEmail_shouldShowFeedbackDialog() { public void onDismissed_hasFeedbackEmail_shouldShowFeedbackDialog() {
mResolver.insert(CardContentProvider.URI, generateOneRow()); mResolver.insert(CardContentProvider.REFRESH_CARD_URI, generateOneRow());
final ContextualCardsFragment fragment = final ContextualCardsFragment fragment =
FragmentController.of(new ContextualCardsFragment()).create().get(); FragmentController.of(new ContextualCardsFragment()).create().get();
final ShadowActivity shadowActivity = Shadows.shadowOf(fragment.getActivity()); final ShadowActivity shadowActivity = Shadows.shadowOf(fragment.getActivity());