From d531dc3058c52b1e3ee2ee53eb8d63d6177d9f91 Mon Sep 17 00:00:00 2001 From: Makoto Onuki Date: Thu, 19 May 2011 11:16:34 -0700 Subject: [PATCH] Fix opening Starred mailbox. We need to pass around the account ID with onMailboxSelected too. (It's kinda sad it wouldn't have happened if we had par-account starred mailbox.) Also made sure MailboxListItem.mMailboxId now really contains only a mailbox ID. Before this chage, we stored an account ID to this for an account row on the combined mailbox. Bug 4452811 Change-Id: I732fd8eb18f787f4a700a45a40768f96e3bb8751 --- .../activity/MailboxFragmentAdapter.java | 37 ++++++++++++------- .../email/activity/MailboxListFragment.java | 15 ++++++-- .../email/activity/MailboxListItem.java | 14 ++++++- .../email/activity/MailboxesAdapter.java | 29 +++++++++++++-- .../email/activity/UIControllerOnePane.java | 4 +- .../email/activity/UIControllerTwoPane.java | 35 ++++++++---------- ...t.java => MailboxFragmentAdapterTest.java} | 11 ++++-- 7 files changed, 98 insertions(+), 47 deletions(-) rename tests/src/com/android/email/activity/{MailboxesAdapterTest.java => MailboxFragmentAdapterTest.java} (93%) diff --git a/src/com/android/email/activity/MailboxFragmentAdapter.java b/src/com/android/email/activity/MailboxFragmentAdapter.java index bb55830e3..e826c7d9f 100644 --- a/src/com/android/email/activity/MailboxFragmentAdapter.java +++ b/src/com/android/email/activity/MailboxFragmentAdapter.java @@ -68,14 +68,16 @@ import android.widget.TextView; final boolean isAccount = isAccountRow(cursor); final int type = cursor.getInt(COLUMN_TYPE); final long id = cursor.getLong(COLUMN_ID); + final long accountId = cursor.getLong(COLUMN_ACCOUNT_ID); final int flags = cursor.getInt(COLUMN_FLAGS); final int rowType = cursor.getInt(COLUMN_ROW_TYPE); final boolean hasVisibleChildren = (flags & Mailbox.FLAG_HAS_CHILDREN) != 0 && (flags & Mailbox.FLAG_CHILDREN_VISIBLE) != 0; MailboxListItem listItem = (MailboxListItem)view; - listItem.mMailboxId = id; + listItem.mMailboxId = isAccountRow(cursor) ? Mailbox.NO_MAILBOX : id; listItem.mMailboxType = type; + listItem.mAccountId = accountId; listItem.mIsValidDropTarget = (id >= 0) && !Utility.arrayContains(Mailbox.INVALID_DROP_TARGETS, type) && (flags & Mailbox.FLAG_ACCEPTS_MOVED_MAIL) != 0; @@ -111,7 +113,7 @@ import android.widget.TextView; final ImageView mailboxExpandedIcon = (ImageView) view.findViewById(R.id.folder_expanded_icon); - switch (cursor.getInt(COLUMN_ROW_TYPE)) { + switch (rowType) { case ROW_TYPE_SUBMAILBOX: if (hasVisibleChildren) { mailboxExpandedIcon.setVisibility(View.VISIBLE); @@ -129,7 +131,7 @@ import android.widget.TextView; folderIcon.setVisibility(View.GONE); break; case ROW_TYPE_MAILBOX: - default: + default: // Includes ROW_TYPE_ACCOUNT if (hasVisibleChildren) { mailboxExpandedIcon.setVisibility(View.VISIBLE); mailboxExpandedIcon.setImageResource( @@ -184,7 +186,8 @@ import android.widget.TextView; * Adds a new row into the given cursor. */ private static void addMailboxRow(MatrixCursor cursor, long mailboxId, String displayName, - int mailboxType, int unreadCount, int messageCount, int rowType, int flags) { + int mailboxType, int unreadCount, int messageCount, int rowType, int flags, + long accountId) { long listId = mailboxId; if (mailboxId < 0) { listId = Long.MAX_VALUE + mailboxId; // IDs for the list view must be positive @@ -198,16 +201,18 @@ import android.widget.TextView; row.add(messageCount); row.add(rowType); row.add(flags); + row.add(accountId); } - private static void addSummaryMailboxRow(MatrixCursor cursor, long id, int mailboxType, + private static void addCombinedMailboxRow(MatrixCursor cursor, long id, int mailboxType, int count, boolean showAlways) { if (id >= 0) { throw new IllegalArgumentException(); // Must be QUERY_ALL_*, which are all negative } if (showAlways || (count > 0)) { addMailboxRow( - cursor, id, "", mailboxType, count, count, ROW_TYPE_MAILBOX, Mailbox.FLAG_NONE); + cursor, id, "", mailboxType, count, count, ROW_TYPE_MAILBOX, Mailbox.FLAG_NONE, + Account.ACCOUNT_ID_COMBINED_VIEW); } } @@ -260,7 +265,7 @@ import android.widget.TextView; if (accountStarredCount > 0) { final MatrixCursor starredCursor = new MatrixCursor(getProjection()); final int totalStarredCount = Message.getFavoriteMessageCount(mContext); - addSummaryMailboxRow(starredCursor, Mailbox.QUERY_ALL_FAVORITES, Mailbox.TYPE_MAIL, + addCombinedMailboxRow(starredCursor, Mailbox.QUERY_ALL_FAVORITES, Mailbox.TYPE_MAIL, totalStarredCount, true); return Utility.CloseTraceCursorWrapper.get( new MergeCursor(new Cursor[] { starredCursor, childMailboxCursor })); @@ -290,8 +295,11 @@ import android.widget.TextView; @Override public Cursor loadInBackground() { final Cursor accounts = super.loadInBackground(); - final MatrixCursor combinedWithAccounts = getCursor(mContext, accounts); + // Build combined mailbox rows. + final MatrixCursor combinedWithAccounts = buildCombinedMailboxes(mContext, accounts); + + // Add account rows. accounts.moveToPosition(-1); while (accounts.moveToNext()) { final long accountId = accounts.getLong(COLUMN_ACCOUND_ID); @@ -299,28 +307,29 @@ import android.widget.TextView; final int unreadCount = Mailbox.getUnreadCountByAccountAndMailboxType( mContext, accountId, Mailbox.TYPE_INBOX); addMailboxRow(combinedWithAccounts, accountId, accountName, Mailbox.TYPE_NONE, - unreadCount, unreadCount, ROW_TYPE_ACCOUNT, Mailbox.FLAG_NONE); + unreadCount, unreadCount, ROW_TYPE_ACCOUNT, Mailbox.FLAG_NONE, + accountId); } return Utility.CloseTraceCursorWrapper.get(combinedWithAccounts); } - /*package*/ static MatrixCursor getCursor(Context context, + /*package*/ static MatrixCursor buildCombinedMailboxes(Context context, Cursor innerCursor) { MatrixCursor cursor = new ClosingMatrixCursor(PROJECTION, innerCursor); // Combined inbox -- show unread count - addSummaryMailboxRow(cursor, Mailbox.QUERY_ALL_INBOXES, Mailbox.TYPE_INBOX, + addCombinedMailboxRow(cursor, Mailbox.QUERY_ALL_INBOXES, Mailbox.TYPE_INBOX, Mailbox.getUnreadCountByMailboxType(context, Mailbox.TYPE_INBOX), true); // Favorite (starred) -- show # of favorites - addSummaryMailboxRow(cursor, Mailbox.QUERY_ALL_FAVORITES, Mailbox.TYPE_MAIL, + addCombinedMailboxRow(cursor, Mailbox.QUERY_ALL_FAVORITES, Mailbox.TYPE_MAIL, Message.getFavoriteMessageCount(context), false); // Drafts -- show # of drafts - addSummaryMailboxRow(cursor, Mailbox.QUERY_ALL_DRAFTS, Mailbox.TYPE_DRAFTS, + addCombinedMailboxRow(cursor, Mailbox.QUERY_ALL_DRAFTS, Mailbox.TYPE_DRAFTS, Mailbox.getMessageCountByMailboxType(context, Mailbox.TYPE_DRAFTS), false); // Outbox -- # of outstanding messages - addSummaryMailboxRow(cursor, Mailbox.QUERY_ALL_OUTBOX, Mailbox.TYPE_OUTBOX, + addCombinedMailboxRow(cursor, Mailbox.QUERY_ALL_OUTBOX, Mailbox.TYPE_OUTBOX, Mailbox.getMessageCountByMailboxType(context, Mailbox.TYPE_OUTBOX), false); return cursor; diff --git a/src/com/android/email/activity/MailboxListFragment.java b/src/com/android/email/activity/MailboxListFragment.java index 2cac522ca..ac3200f35 100644 --- a/src/com/android/email/activity/MailboxListFragment.java +++ b/src/com/android/email/activity/MailboxListFragment.java @@ -137,12 +137,15 @@ public class MailboxListFragment extends ListFragment implements OnItemClickList /** * Called when any mailbox (even a combined mailbox) is selected. * + * @param accountId + * The ID of the owner account of the selected mailbox. + * Or {@link Account#ACCOUNT_ID_COMBINED_VIEW} if it's a combined mailbox. * @param mailboxId * The ID of the selected mailbox. This may be real mailbox ID [e.g. a number > 0], * or a combined mailbox ID [e.g. {@link Mailbox#QUERY_ALL_INBOXES}]. * @param navigate navigate to the mailbox. */ - public void onMailboxSelected(long mailboxId, boolean navigate); + public void onMailboxSelected(long accountId, long mailboxId, boolean navigate); /** * Called when a mailbox is selected during D&D. @@ -158,13 +161,18 @@ public class MailboxListFragment extends ListFragment implements OnItemClickList * * Note the reason why it's separated from onMailboxSelected is because this needs to be * reported when the unread count changes without changing the current mailbox. + * + * @param mailboxId ID for the selected mailbox. It'll never be of a combined mailbox, + * and the owner account ID is always the same as + * {@link MailboxListFragment#getAccountId()}. */ public void onCurrentMailboxUpdated(long mailboxId, String mailboxName, int unreadCount); } private static class EmptyCallback implements Callback { public static final Callback INSTANCE = new EmptyCallback(); - @Override public void onMailboxSelected(long mailboxId, boolean navigate) { } + @Override public void onMailboxSelected(long accountId, long mailboxId, boolean navigate) { + } @Override public void onMailboxSelectedForDnD(long mailboxId) { } @Override public void onAccountSelected(long accountId) { } @Override public void onCurrentMailboxUpdated(long mailboxId, String mailboxName, @@ -496,7 +504,8 @@ public class MailboxListFragment extends ListFragment implements OnItemClickList } else { // STOPSHIP On phone, we need a way to open a message list without navigating to the // mailbox. - mCallback.onMailboxSelected(id, isNavigable(id)); + mCallback.onMailboxSelected(mListAdapter.getAccountId(position), id, + isNavigable(id)); } } diff --git a/src/com/android/email/activity/MailboxListItem.java b/src/com/android/email/activity/MailboxListItem.java index 2f9599a7e..1b0ca6ff0 100644 --- a/src/com/android/email/activity/MailboxListItem.java +++ b/src/com/android/email/activity/MailboxListItem.java @@ -17,6 +17,7 @@ package com.android.email.activity; import com.android.email.R; +import com.android.emailcommon.provider.EmailContent.Account; import com.android.emailcommon.provider.Mailbox; import android.content.Context; @@ -33,6 +34,15 @@ public class MailboxListItem extends RelativeLayout { private static Integer sTextPrimaryColor; private static Integer sTextSecondaryColor; + /** + * Owner account ID for the mailbox, {@link Account#ACCOUNT_ID_COMBINED_VIEW} for a combined + * mailbox, or the ID for the current account, if it's an account row. + */ + public long mAccountId; + + /** + * ID for the current mailbox, or {@link Mailbox#NO_MAILBOX} if it's an account row. + */ public long mMailboxId; public Integer mMailboxType; /** If {@code true} this item can be used as a drop target. Otherwise, drop is prohibited. */ @@ -77,8 +87,8 @@ public class MailboxListItem extends RelativeLayout { * not forbidden by the system (see {@link Mailbox#INVALID_DROP_TARGETS}) will return * {@code true}. */ - public boolean isDropTarget(long itemMailbox) { - return mIsValidDropTarget && (itemMailbox != mMailboxId); + public boolean isDropTarget(long itemMailboxId) { + return mIsValidDropTarget && (itemMailboxId != mMailboxId); } /** diff --git a/src/com/android/email/activity/MailboxesAdapter.java b/src/com/android/email/activity/MailboxesAdapter.java index 17ab79793..2f10dc71f 100644 --- a/src/com/android/email/activity/MailboxesAdapter.java +++ b/src/com/android/email/activity/MailboxesAdapter.java @@ -18,8 +18,10 @@ package com.android.email.activity; import com.android.email.FolderProperties; import com.android.email.ResourceHelper; +import com.android.emailcommon.provider.EmailContent.Account; import com.android.emailcommon.provider.EmailContent.MailboxColumns; import com.android.emailcommon.provider.Mailbox; +import com.google.common.annotations.VisibleForTesting; import android.content.Context; import android.database.Cursor; @@ -80,7 +82,7 @@ import android.widget.CursorAdapter; MailboxColumns.ID + " AS org_mailbox_id", MailboxColumns.DISPLAY_NAME, MailboxColumns.TYPE, MailboxColumns.UNREAD_COUNT, MailboxColumns.MESSAGE_COUNT, ROW_TYPE_MAILBOX + " AS row_type", - MailboxColumns.FLAGS }; + MailboxColumns.FLAGS, MailboxColumns.ACCOUNT_KEY }; // STOPSHIP May need to adjust sub-folder projection depending upon final UX /** * Projection used to retrieve immediate children for a mailbox. The columns need to @@ -91,14 +93,18 @@ import android.widget.CursorAdapter; MailboxColumns.ID + " AS org_mailbox_id", MailboxColumns.DISPLAY_NAME, MailboxColumns.TYPE, MailboxColumns.UNREAD_COUNT, MailboxColumns.MESSAGE_COUNT, ROW_TYPE_SUBMAILBOX + " AS row_type", - MailboxColumns.FLAGS }; + MailboxColumns.FLAGS, MailboxColumns.ACCOUNT_KEY }; /*package*/ static final String[] CURMAILBOX_PROJECTION = new String[] { MailboxColumns.ID, MailboxColumns.ID + " AS org_mailbox_id", MailboxColumns.DISPLAY_NAME, MailboxColumns.TYPE, MailboxColumns.UNREAD_COUNT, MailboxColumns.MESSAGE_COUNT, ROW_TYPE_CURMAILBOX + " AS row_type", - MailboxColumns.FLAGS }; + MailboxColumns.FLAGS, MailboxColumns.ACCOUNT_KEY }; // Column 0 is only for ListView; we don't use it in our code. + /** + * ID for the current row. Normally it's the ID for the current mailbox, but if it's an account + * row on the combined view, it's the ID for the account. + */ /*package*/ static final int COLUMN_ID = 1; /*package*/ static final int COLUMN_DISPLAY_NAME = 2; /*package*/ static final int COLUMN_TYPE = 3; @@ -106,6 +112,12 @@ import android.widget.CursorAdapter; /*package*/ static final int COLUMN_MESSAGE_COUNT = 5; /*package*/ static final int COLUMN_ROW_TYPE = 6; /*package*/ static final int COLUMN_FLAGS = 7; + /** + * ID for the owner account of the mailbox. If it's a combined mailbox, it's + * {@link Account#ACCOUNT_ID_COMBINED_VIEW}. If it's an account row on the combined view, + * it's the ID for the account. + */ + /*package*/ static final int COLUMN_ACCOUNT_ID = 8; /** All mailboxes for the account */ /*package*/ static final String ALL_MAILBOX_SELECTION = MailboxColumns.ACCOUNT_KEY + "=?" + @@ -222,6 +234,12 @@ import android.widget.CursorAdapter; return c.getLong(COLUMN_ID); } + /** @see #COLUMN_ACCOUNT_ID */ + public long getAccountId(int position) { + Cursor c = (Cursor) getItem(position); + return c.getLong(COLUMN_ACCOUNT_ID); + } + /** * Turn on and off list updates; during a drag operation, we do NOT want to the list of * mailboxes to update, as this would be visually jarring @@ -261,4 +279,9 @@ import android.widget.CursorAdapter; /*package*/ static int getUnreadCountForTest(Cursor cursor) { return cursor.getInt(COLUMN_UNREAD_COUNT); } + + @VisibleForTesting + static long getAccountId(Cursor cursor) { + return cursor.getLong(COLUMN_ACCOUNT_ID); + } } \ No newline at end of file diff --git a/src/com/android/email/activity/UIControllerOnePane.java b/src/com/android/email/activity/UIControllerOnePane.java index 26ae5442b..7ea5e6710 100644 --- a/src/com/android/email/activity/UIControllerOnePane.java +++ b/src/com/android/email/activity/UIControllerOnePane.java @@ -74,8 +74,8 @@ class UIControllerOnePane extends UIControllerBase { } @Override - public void onMailboxSelected(long mailboxId, boolean navigate) { - openMailbox(getUIAccountId(), mailboxId); + public void onMailboxSelected(long accountId, long mailboxId, boolean navigate) { + openMailbox(accountId, mailboxId); } @Override diff --git a/src/com/android/email/activity/UIControllerTwoPane.java b/src/com/android/email/activity/UIControllerTwoPane.java index 2734dec15..dafb886af 100644 --- a/src/com/android/email/activity/UIControllerTwoPane.java +++ b/src/com/android/email/activity/UIControllerTwoPane.java @@ -136,7 +136,7 @@ class UIControllerTwoPane extends UIControllerBase implements if (Logging.DEBUG_LIFECYCLE && Email.DEBUG) { Log.d(Logging.LOG_TAG, this + " onMailboxFound()"); } - updateMessageList(mailboxId, true); + updateMessageList(accountId, mailboxId, true); } // MailboxFinder$Callback @@ -169,19 +169,18 @@ class UIControllerTwoPane extends UIControllerBase implements // MailboxListFragment$Callback @Override - public void onMailboxSelected(long mailboxId, boolean navigate) { - final long accountId = getUIAccountId(); - if (mailboxId == Mailbox.NO_MAILBOX) { - // reload the top-level message list. Always implies navigate. - openAccount(accountId); - } else if (navigate) { + public void onMailboxSelected(long accountId, long mailboxId, boolean navigate) { + if ((accountId == Account.NO_ACCOUNT) || (mailboxId == Mailbox.NO_MAILBOX)) { + throw new IllegalArgumentException(); // Shouldn't happen. + } + if (navigate) { if (mailboxId != getMailboxListMailboxId()) { // Don't navigate to the same mailbox id twice in a row openMailbox(accountId, mailboxId); } } else { // Regular case -- just open the mailbox on the message list. - updateMessageList(mailboxId, true); + updateMessageList(accountId, mailboxId, true); } } @@ -610,7 +609,7 @@ class UIControllerTwoPane extends UIControllerBase implements // Show the appropriate message list if (accountId == Account.ACCOUNT_ID_COMBINED_VIEW) { // When opening the Combined view, the right pane will be "combined inbox". - updateMessageList(Mailbox.QUERY_ALL_INBOXES, true); + updateMessageList(accountId, Mailbox.QUERY_ALL_INBOXES, true); } else { // Try to find the inbox for the account closeMailboxFinder(); @@ -621,13 +620,13 @@ class UIControllerTwoPane extends UIControllerBase implements } else if (messageId == Message.NO_MESSAGE) { // STOPSHIP Use the appropriate parent mailbox ID updateMailboxList(accountId, mailboxId, true); - updateMessageList(mailboxId, true); + updateMessageList(accountId, mailboxId, true); mThreePane.showLeftPane(); } else { // STOPSHIP Use the appropriate parent mailbox ID updateMailboxList(accountId, mailboxId, true); - updateMessageList(mailboxId, true); + updateMessageList(accountId, mailboxId, true); updateMessageView(messageId); mThreePane.showRightPane(); @@ -699,18 +698,16 @@ class UIControllerTwoPane extends UIControllerBase implements } /** - * Selects the specified mailbox and optionally loads a message within it. If a message is - * not loaded, a list of the messages contained within the mailbox is shown. Otherwise the - * given message is shown. If navigateToMailbox is true, the - * mailbox is navigated to and any contained mailboxes are shown. + * Show the message list fragment for the given mailbox. * - * @param mailboxId ID of the mailbox to load. Must never be 0 or - * {@link Mailbox#NO_MAILBOX}. + * @param accountId ID of the owner account for the mailbox. Must never be + * {@link Account#NO_ACCOUNT}. + * @param mailboxId ID of the mailbox to load. Must never be {@link Mailbox#NO_MAILBOX}. * @param clearDependentPane if true, the message view will be cleared * * STOPSHIP Need to stop mailbox finder if it's still running */ - private void updateMessageList(long mailboxId, boolean clearDependentPane) { + private void updateMessageList(long accountId, long mailboxId, boolean clearDependentPane) { if (Logging.DEBUG_LIFECYCLE && Email.DEBUG) { Log.d(Logging.LOG_TAG, this + " updateMessageList mMailboxId=" + mailboxId); } @@ -724,7 +721,7 @@ class UIControllerTwoPane extends UIControllerBase implements if (mailboxId != getMessageListMailboxId()) { uninstallMessageListFragment(ft); ft.add(mThreePane.getMiddlePaneId(), MessageListFragment.newInstance( - getUIAccountId(), mailboxId)); + accountId, mailboxId)); } if (clearDependentPane) { uninstallMessageViewFragment(ft); diff --git a/tests/src/com/android/email/activity/MailboxesAdapterTest.java b/tests/src/com/android/email/activity/MailboxFragmentAdapterTest.java similarity index 93% rename from tests/src/com/android/email/activity/MailboxesAdapterTest.java rename to tests/src/com/android/email/activity/MailboxFragmentAdapterTest.java index 6242a7dd9..2d48e5c01 100644 --- a/tests/src/com/android/email/activity/MailboxesAdapterTest.java +++ b/tests/src/com/android/email/activity/MailboxFragmentAdapterTest.java @@ -29,11 +29,11 @@ import android.test.ProviderTestCase2; import junit.framework.Assert; -public class MailboxesAdapterTest extends ProviderTestCase2 { +public class MailboxFragmentAdapterTest extends ProviderTestCase2 { private Context mMockContext; - public MailboxesAdapterTest() { + public MailboxFragmentAdapterTest() { super(EmailProvider.class, EmailContent.AUTHORITY); } @@ -43,7 +43,7 @@ public class MailboxesAdapterTest extends ProviderTestCase2 { mMockContext = getMockContext(); } - public void testAddSummaryMailboxRow() { + public void testBuildCombinedMailboxes() { final Context c = mMockContext; // Prepare test data @@ -79,7 +79,8 @@ public class MailboxesAdapterTest extends ProviderTestCase2 { createMessage(c, b2t, true, true, Message.FLAG_LOADED_UNLOADED); // Kick the method - Cursor cursor = MailboxFragmentAdapter.CombinedMailboxLoader.getCursor(c, null); + Cursor cursor = MailboxFragmentAdapter.CombinedMailboxLoader.buildCombinedMailboxes(c, + null); // Check the result assertEquals(4, cursor.getCount()); @@ -118,5 +119,7 @@ public class MailboxesAdapterTest extends ProviderTestCase2 { Assert.assertEquals(type, MailboxesAdapter.getTypeForTest(cursor)); Assert.assertEquals(count, MailboxesAdapter.getMessageCountForTest(cursor)); Assert.assertEquals(count, MailboxesAdapter.getUnreadCountForTest(cursor)); + Assert.assertEquals(Account.ACCOUNT_ID_COMBINED_VIEW, + MailboxesAdapter.getAccountId(cursor)); } }