From 513486cfb5d446f098b1c97d16932e51e316d1a7 Mon Sep 17 00:00:00 2001 From: Ben Komalo Date: Wed, 12 Oct 2011 16:24:24 -0700 Subject: [PATCH] Fix per account starred mailboxes. We always switched to combined view when we tapped "Starred" in a mailbox list. This was confusing since tapping to show the mailbox list will take you to the mailbox list for the combined view, even if there's only one account on the device! Other confusing things happened like showing coloured account chips, even if there's only one account. This fixes it so that we pass the account ID when building the selection. Lots of wiring to make it happen, but very little change overall. Bug: 5388326 Change-Id: I1fe52f211cceca0c1b26581e200f3c7adcd0734a --- .../emailcommon/provider/EmailContent.java | 15 ++++++-- .../email/activity/MailboxListFragment.java | 8 ++++- .../email/activity/MessageListFragment.java | 9 ++--- .../email/activity/MessageOrderManager.java | 36 +++++++++++-------- .../email/activity/MessagesAdapter.java | 12 ++++--- .../email/activity/UIControllerBase.java | 7 ++-- .../email/activity/UIControllerOnePane.java | 5 ++- .../email/activity/UIControllerTwoPane.java | 5 ++- .../activity/MessageOrderManagerTest.java | 17 +++++---- .../android/email/provider/ProviderTests.java | 32 ----------------- 10 files changed, 73 insertions(+), 73 deletions(-) diff --git a/emailcommon/src/com/android/emailcommon/provider/EmailContent.java b/emailcommon/src/com/android/emailcommon/provider/EmailContent.java index 90a86fd65..45876731b 100644 --- a/emailcommon/src/com/android/emailcommon/provider/EmailContent.java +++ b/emailcommon/src/com/android/emailcommon/provider/EmailContent.java @@ -965,7 +965,8 @@ public abstract class EmailContent { * * Accesses the detabase to determine the mailbox type. DO NOT CALL FROM UI THREAD. */ - public static String buildMessageListSelection(Context context, long mailboxId) { + public static String buildMessageListSelection( + Context context, long accountId, long mailboxId) { if (mailboxId == Mailbox.QUERY_ALL_INBOXES) { return Message.ALL_INBOX_SELECTION; @@ -979,8 +980,18 @@ public abstract class EmailContent { if (mailboxId == Mailbox.QUERY_ALL_UNREAD) { return Message.ALL_UNREAD_SELECTION; } + // TODO: we only support per-account starred mailbox right now, but presumably, we + // can surface the same thing for unread. if (mailboxId == Mailbox.QUERY_ALL_FAVORITES) { - return Message.ALL_FAVORITE_SELECTION; + if (accountId == Account.ACCOUNT_ID_COMBINED_VIEW) { + return Message.ALL_FAVORITE_SELECTION; + } + + final StringBuilder selection = new StringBuilder(); + selection.append(MessageColumns.ACCOUNT_KEY).append('=').append(accountId) + .append(" AND ") + .append(Message.ALL_FAVORITE_SELECTION); + return selection.toString(); } // Now it's a regular mailbox. diff --git a/src/com/android/email/activity/MailboxListFragment.java b/src/com/android/email/activity/MailboxListFragment.java index 34001c8a9..e9750e829 100644 --- a/src/com/android/email/activity/MailboxListFragment.java +++ b/src/com/android/email/activity/MailboxListFragment.java @@ -865,13 +865,19 @@ public class MailboxListFragment extends ListFragment implements OnItemClickList // the current loader and make the mListAdapter lose the cursor. // Note, don't just use getAccountId(). A mailbox may tied to a different account ID // from getAccountId(). (Currently "Starred" does so.) - final long accountId = mListAdapter.getAccountId(position); + long accountId = mListAdapter.getAccountId(position); boolean nestedNavigation = false; if (((MailboxListItem) view).isNavigable() && (id != mParentMailboxId)) { // Drill-in. Selected one will be the next parent, and it'll also be highlighted. startLoading(id, id); nestedNavigation = true; } + if (accountId == Account.ACCOUNT_ID_COMBINED_VIEW) { + // Virtual mailboxes, such as "Starred", will have a "combined view" ID. However, + // we really want to relay the current active account, so that + // things like per-account starred mailboxes work as expected. + accountId = getAccountId(); + } mCallback.onMailboxSelected(accountId, id, nestedNavigation); } } diff --git a/src/com/android/email/activity/MessageListFragment.java b/src/com/android/email/activity/MessageListFragment.java index bffa8bb65..c203c5d2e 100644 --- a/src/com/android/email/activity/MessageListFragment.java +++ b/src/com/android/email/activity/MessageListFragment.java @@ -278,7 +278,7 @@ public class MessageListFragment extends ListFragment * @return true if the mailbox is a combined mailbox. Safe to call even before onCreate. */ public boolean isCombinedMailbox() { - return getMailboxId() < 0; + return getAccountId() == Account.ACCOUNT_ID_COMBINED_VIEW; } public MessageListContext getListContext() { @@ -1229,7 +1229,7 @@ public class MessageListFragment extends ListFragment // Start loading... final LoaderManager lm = getLoaderManager(); - lm.initLoader(LOADER_ID_MESSAGES_LOADER, null, new MessagesLoaderCallback()); + lm.initLoader(LOADER_ID_MESSAGES_LOADER, null, LOADER_CALLBACKS); } /** Timeout to show a warning, since some IMAP searches could take a long time. */ @@ -1254,7 +1254,8 @@ public class MessageListFragment extends ListFragment /** * Loader callbacks for message list. */ - private class MessagesLoaderCallback implements LoaderManager.LoaderCallbacks { + private final LoaderManager.LoaderCallbacks LOADER_CALLBACKS = + new LoaderManager.LoaderCallbacks() { @Override public Loader onCreateLoader(int id, Bundle args) { final MessageListContext listContext = getListContext(); @@ -1386,7 +1387,7 @@ public class MessageListFragment extends ListFragment mSearchedMailbox = null; mCountTotalAccounts = 0; } - } + }; /** * Show/hide the "selection" action mode, according to the number of selected messages and diff --git a/src/com/android/email/activity/MessageOrderManager.java b/src/com/android/email/activity/MessageOrderManager.java index 6e5cacbc6..fffcf7c3b 100644 --- a/src/com/android/email/activity/MessageOrderManager.java +++ b/src/com/android/email/activity/MessageOrderManager.java @@ -16,6 +16,14 @@ package com.android.email.activity; +import android.content.ContentResolver; +import android.content.Context; +import android.database.ContentObserver; +import android.database.Cursor; +import android.os.Handler; + +import com.android.email.MessageListContext; +import com.android.email.activity.MessageOrderManager.Callback; import com.android.emailcommon.provider.EmailContent; import com.android.emailcommon.provider.EmailContent.Message; import com.android.emailcommon.provider.Mailbox; @@ -25,12 +33,6 @@ import com.android.emailcommon.utility.Utility; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; -import android.content.ContentResolver; -import android.content.Context; -import android.database.ContentObserver; -import android.database.Cursor; -import android.os.Handler; - /** * Used by {@link MessageView} to determine the message-id of the previous/next messages. * @@ -55,7 +57,7 @@ public class MessageOrderManager { private final Context mContext; private final ContentResolver mContentResolver; - private final long mMailboxId; + private final MessageListContext mListContext; private final ContentObserver mObserver; private final Callback mCallback; private final DelayedOperations mDelayedOperations; @@ -121,18 +123,18 @@ public class MessageOrderManager { } } - public MessageOrderManager(Context context, long mailboxId, Callback callback) { - this(context, mailboxId, callback, new DelayedOperations(Utility.getMainThreadHandler())); + public MessageOrderManager(Context context, MessageListContext listContext, Callback callback) { + this(context, listContext, callback, new DelayedOperations(Utility.getMainThreadHandler())); } @VisibleForTesting - MessageOrderManager(Context context, long mailboxId, Callback callback, + MessageOrderManager(Context context, MessageListContext listContext, Callback callback, DelayedOperations delayedOperations) { - Preconditions.checkArgument(mailboxId != Mailbox.NO_MAILBOX); + Preconditions.checkArgument(listContext.getMailboxId() != Mailbox.NO_MAILBOX); mContext = context.getApplicationContext(); mContentResolver = mContext.getContentResolver(); mDelayedOperations = delayedOperations; - mMailboxId = mailboxId; + mListContext = listContext; mCallback = new PostingCallback(callback); mObserver = new ContentObserver(getHandlerForContentObserver()) { @Override public void onChange(boolean selfChange) { @@ -145,8 +147,12 @@ public class MessageOrderManager { startTask(); } + public MessageListContext getListContext() { + return mListContext; + } + public long getMailboxId() { - return mMailboxId; + return mListContext.getMailboxId(); } /** @@ -346,7 +352,9 @@ public class MessageOrderManager { */ private Cursor openNewCursor() { final Cursor cursor = mContentResolver.query(EmailContent.Message.CONTENT_URI, - EmailContent.ID_PROJECTION, Message.buildMessageListSelection(mContext, mMailboxId), + EmailContent.ID_PROJECTION, + Message.buildMessageListSelection( + mContext, mListContext.mAccountId, mListContext.getMailboxId()), null, EmailContent.MessageColumns.TIMESTAMP + " DESC"); return cursor; } diff --git a/src/com/android/email/activity/MessagesAdapter.java b/src/com/android/email/activity/MessagesAdapter.java index 0c059c1d0..2e8b2ebc6 100644 --- a/src/com/android/email/activity/MessagesAdapter.java +++ b/src/com/android/email/activity/MessagesAdapter.java @@ -275,26 +275,28 @@ import java.util.Set; } return listContext.isSearch() ? new SearchCursorLoader(context, listContext) - : new MessagesCursorLoader(context, listContext.getMailboxId()); + : new MessagesCursorLoader(context, listContext); } private static class MessagesCursorLoader extends ThrottlingCursorLoader { protected final Context mContext; + private final long mAccountId; private final long mMailboxId; - public MessagesCursorLoader(Context context, long mailboxId) { + public MessagesCursorLoader(Context context, MessageListContext listContext) { // Initialize with no where clause. We'll set it later. super(context, EmailContent.Message.CONTENT_URI, MESSAGE_PROJECTION, null, null, EmailContent.MessageColumns.TIMESTAMP + " DESC"); mContext = context; - mMailboxId = mailboxId; + mAccountId = listContext.mAccountId; + mMailboxId = listContext.getMailboxId(); } @Override public Cursor loadInBackground() { // Build the where cause (which can't be done on the UI thread.) - setSelection(Message.buildMessageListSelection(mContext, mMailboxId)); + setSelection(Message.buildMessageListSelection(mContext, mAccountId, mMailboxId)); // Then do a query to get the cursor return loadExtras(super.loadInBackground()); } @@ -376,7 +378,7 @@ import java.util.Set; private Mailbox mSearchedMailbox = null; public SearchCursorLoader(Context context, MessageListContext listContext) { - super(context, listContext.getMailboxId()); + super(context, listContext); Preconditions.checkArgument(listContext.isSearch()); mListContext = listContext; } diff --git a/src/com/android/email/activity/UIControllerBase.java b/src/com/android/email/activity/UIControllerBase.java index 6f9d7cc0b..e827c246e 100644 --- a/src/com/android/email/activity/UIControllerBase.java +++ b/src/com/android/email/activity/UIControllerBase.java @@ -984,11 +984,10 @@ abstract class UIControllerBase implements MailboxListFragment.Callback, } Preconditions.checkNotNull(mListContext); - final long mailboxId = mListContext.getMailboxId(); - if (mOrderManager == null || mOrderManager.getMailboxId() != mailboxId) { + if (mOrderManager == null || !mOrderManager.getListContext().equals(mListContext)) { stopMessageOrderManager(); - mOrderManager = - new MessageOrderManager(mActivity, mailboxId, mMessageOrderManagerCallback); + mOrderManager = new MessageOrderManager( + mActivity, mListContext, mMessageOrderManagerCallback); } mOrderManager.moveTo(getMessageId()); updateNavigationArrows(); diff --git a/src/com/android/email/activity/UIControllerOnePane.java b/src/com/android/email/activity/UIControllerOnePane.java index b06cbe474..b69f92731 100644 --- a/src/com/android/email/activity/UIControllerOnePane.java +++ b/src/com/android/email/activity/UIControllerOnePane.java @@ -601,7 +601,10 @@ class UIControllerOnePane extends UIControllerBase { // Refreshable only when an actual account is selected, and message view isn't shown. // (i.e. only available on the mailbox list or the message view, but not on the combined // one) - return isActualAccountSelected() && !isMessageViewInstalled(); + if (!isActualAccountSelected() || isMessageViewInstalled()) { + return false; + } + return isMailboxListInstalled() || (mListContext.getMailboxId() > 0); } @Override diff --git a/src/com/android/email/activity/UIControllerTwoPane.java b/src/com/android/email/activity/UIControllerTwoPane.java index 6327f8f89..5940818fc 100644 --- a/src/com/android/email/activity/UIControllerTwoPane.java +++ b/src/com/android/email/activity/UIControllerTwoPane.java @@ -287,9 +287,8 @@ class UIControllerTwoPane extends UIControllerBase implements ThreePaneLayout.Ca */ @Override protected boolean isRefreshEnabled() { - // - Don't show for combined inboxes, but - // - Show even for non-refreshable mailboxes, in which case we refresh the mailbox list - return getActualAccountId() != Account.NO_ACCOUNT; + return getActualAccountId() != Account.NO_ACCOUNT + && (mListContext.getMailboxId() > 0); } diff --git a/tests/src/com/android/email/activity/MessageOrderManagerTest.java b/tests/src/com/android/email/activity/MessageOrderManagerTest.java index 3bbc1f73a..da78627c2 100644 --- a/tests/src/com/android/email/activity/MessageOrderManagerTest.java +++ b/tests/src/com/android/email/activity/MessageOrderManagerTest.java @@ -16,10 +16,6 @@ package com.android.email.activity; -import com.android.email.provider.EmailProvider; -import com.android.emailcommon.provider.EmailContent; -import com.android.emailcommon.utility.DelayedOperations; - import android.content.Context; import android.database.AbstractCursor; import android.database.Cursor; @@ -27,6 +23,11 @@ import android.os.Handler; import android.test.ProviderTestCase2; import android.test.suitebuilder.annotation.SmallTest; +import com.android.email.MessageListContext; +import com.android.email.provider.EmailProvider; +import com.android.emailcommon.provider.EmailContent; +import com.android.emailcommon.utility.DelayedOperations; + import junit.framework.Assert; @SmallTest @@ -256,7 +257,8 @@ public class MessageOrderManagerTest extends ProviderTestCase2 { public void testWithActualClass() { // There are not many things we can test synchronously. // Just open & close just to make sure it won't crash. - MessageOrderManager mom = new MessageOrderManager(getContext(), 1, new MyCallback()); + MessageOrderManager mom = new MessageOrderManager( + getContext(), MessageListContext.forMailbox(1, 1), new MyCallback()); mom.moveTo(123); mom.close(); } @@ -307,7 +309,8 @@ public class MessageOrderManagerTest extends ProviderTestCase2 { public boolean mStartQueryCalled; public MessageOrderManagerForTest(Context context, long mailboxId, Callback callback) { - super(context, mailboxId, callback, new NonDelayedOperations()); + super(context, MessageListContext.forMailbox(1, mailboxId), + callback, new NonDelayedOperations()); } @Override void startQuery() { @@ -345,7 +348,7 @@ public class MessageOrderManagerTest extends ProviderTestCase2 { } private static class MyCursor extends AbstractCursor { - private long[] mList; + private final long[] mList; public MyCursor(long... idList) { mList = (idList == null) ? new long[0] : idList; diff --git a/tests/src/com/android/email/provider/ProviderTests.java b/tests/src/com/android/email/provider/ProviderTests.java index 8c2473c2d..ebf324399 100644 --- a/tests/src/com/android/email/provider/ProviderTests.java +++ b/tests/src/com/android/email/provider/ProviderTests.java @@ -2298,38 +2298,6 @@ public class ProviderTests extends ProviderTestCase2 { assertEquals(b34, testMailbox); } - public void testBuildMessageListSelection() { - final Context c = mMockContext; - - assertEquals(Message.ALL_INBOX_SELECTION, Message.buildMessageListSelection(c, - Mailbox.QUERY_ALL_INBOXES)); - - assertEquals(Message.ALL_DRAFT_SELECTION, Message.buildMessageListSelection(c, - Mailbox.QUERY_ALL_DRAFTS)); - - assertEquals(Message.ALL_OUTBOX_SELECTION, Message.buildMessageListSelection(c, - Mailbox.QUERY_ALL_OUTBOX)); - - assertEquals(Message.ALL_UNREAD_SELECTION, Message.buildMessageListSelection(c, - Mailbox.QUERY_ALL_UNREAD)); - - assertEquals(Message.ALL_FAVORITE_SELECTION, Message.buildMessageListSelection(c, - Mailbox.QUERY_ALL_FAVORITES)); - - final Account account = ProviderTestUtils.setupAccount("1", true, mMockContext); - final Mailbox in = ProviderTestUtils.setupMailbox("i", account.mId, true, c, - Mailbox.TYPE_INBOX); - final Mailbox out = ProviderTestUtils.setupMailbox("o", account.mId, true, c, - Mailbox.TYPE_OUTBOX); - - assertEquals(Message.MAILBOX_KEY + "=" + in.mId + " AND " + Message.FLAG_LOADED_SELECTION, - Message.buildMessageListSelection(c, in.mId)); - - // No LOADED check for outboxes. - assertEquals(Message.MAILBOX_KEY + "=" + out.mId, - Message.buildMessageListSelection(c, out.mId)); - } - /** * Determine whether a list of AccountManager accounts includes a given EmailProvider account * @param amAccountList a list of AccountManager accounts