diff --git a/res/menu/message_list_selection_mode.xml b/res/menu/message_list_selection_mode.xml index 1e2298cfb..d1da8435f 100644 --- a/res/menu/message_list_selection_mode.xml +++ b/res/menu/message_list_selection_mode.xml @@ -15,6 +15,8 @@ --> + Fix All Bugs Cheaper than beer + + + + + Move is not supported on POP3 accounts. + + + Cannot move. Selection contains multiple accounts. + + + Messages in Drafts, Outbox and Sent cannot be moved. diff --git a/src/com/android/email/Utility.java b/src/com/android/email/Utility.java index 540ef3dab..dc8ff669f 100644 --- a/src/com/android/email/Utility.java +++ b/src/com/android/email/Utility.java @@ -64,6 +64,7 @@ import java.nio.CharBuffer; import java.nio.charset.Charset; import java.security.NoSuchAlgorithmException; import java.util.ArrayList; +import java.util.Collection; import java.util.Date; import java.util.GregorianCalendar; import java.util.TimeZone; @@ -1042,4 +1043,15 @@ public class Utility { protected void onFinished() { } } + + public static long[] toPrimitiveLongArray(Collection collection) { + final int size = collection.size(); + final long[] ret = new long[size]; + // Collection doesn't have get(i). (Iterable doesn't have size()) + int i = 0; + for (Long value : collection) { + ret[i++] = value; + } + return ret; + } } diff --git a/src/com/android/email/activity/ActivityHelper.java b/src/com/android/email/activity/ActivityHelper.java index 29b62ae63..7a3345c87 100644 --- a/src/com/android/email/activity/ActivityHelper.java +++ b/src/com/android/email/activity/ActivityHelper.java @@ -36,10 +36,11 @@ import android.provider.Browser; */ public final class ActivityHelper { /** - * Loader IDs have to be unique in a fragment. We reserve ID(s) here for loaders created + * Loader IDs have to be unique in a fragment. We reserve IDs here for loaders created * outside of fragments. */ - public static final int GLOBAL_LOADER_ID_MOVE_TO_DIALOG_LOADER = 1000; + public static final int GLOBAL_LOADER_ID_MOVE_TO_DIALOG_MAILBOX_LOADER = 1000; + public static final int GLOBAL_LOADER_ID_MOVE_TO_DIALOG_MESSAGE_CHECKER = 1001; private ActivityHelper() { } diff --git a/src/com/android/email/activity/MessageListFragment.java b/src/com/android/email/activity/MessageListFragment.java index c6720b6e7..b82e25cbe 100644 --- a/src/com/android/email/activity/MessageListFragment.java +++ b/src/com/android/email/activity/MessageListFragment.java @@ -71,11 +71,13 @@ import java.util.Set; * We run them sequentially. i.e. First starts {@link MailboxAccountLoader}, and when it finishes * starts the other. * - * TODO Add "send all messages" button to outboxes + * TODO Finalize batch move UI. Probably the "move" button should be disabled or hidden when + * the selection contains non-movable messages. But then how does the user know why they can't be + * moved? */ public class MessageListFragment extends ListFragment implements OnItemClickListener, OnItemLongClickListener, MessagesAdapter.Callback, - OnClickListener { + OnClickListener, MoveMessageToDialog.Callback { private static final String BUNDLE_LIST_STATE = "MessageListFragment.state.listState"; private static final int LOADER_ID_MAILBOX_LOADER = 1; @@ -453,6 +455,21 @@ public class MessageListFragment extends ListFragment onMultiDelete(mListAdapter.getSelectedSet()); } + public void onMultiMove() { + long[] messageIds = Utility.toPrimitiveLongArray(mListAdapter.getSelectedSet()); + MoveMessageToDialog dialog = MoveMessageToDialog.newInstance(getActivity(), messageIds, + this); + dialog.show(getFragmentManager(), "dialog"); + } + + @Override + public void onMoveToMailboxSelected(long newMailboxId, long[] messageIds) { + ActivityHelper.moveMessages(getActivity(), newMailboxId, messageIds); + + // Move is async, so we can't refresh now. Instead, just clear the selection. + onDeselectAll(); + } + /** * Refresh the list. NOOP for special mailboxes (e.g. combined inbox). * @@ -1018,6 +1035,9 @@ public class MessageListFragment extends ListFragment case R.id.delete: onMultiDelete(); break; + case R.id.move: + onMultiMove(); + break; } return true; } diff --git a/src/com/android/email/activity/MessageListXL.java b/src/com/android/email/activity/MessageListXL.java index fa1636bd4..6d9f83034 100644 --- a/src/com/android/email/activity/MessageListXL.java +++ b/src/com/android/email/activity/MessageListXL.java @@ -38,7 +38,6 @@ import android.database.Cursor; import android.os.AsyncTask; import android.os.Bundle; import android.util.Log; -import android.view.KeyEvent; import android.view.Menu; import android.view.MenuItem; import android.view.View; @@ -313,13 +312,21 @@ public class MessageListXL extends Activity implements View.OnClickListener, } private void onMoveMessage() { - long accountId = mFragmentManager.getAccountId(); long messageId = mFragmentManager.getMessageId(); - MoveMessageToDialog dialog = MoveMessageToDialog.newInstance(this, accountId, - new long[] {messageId}); + MoveMessageToDialog dialog = MoveMessageToDialog.newInstance(this, new long[] {messageId}, + null); dialog.show(getFragmentManager(), "dialog"); } + @Override + public void onMoveToMailboxSelected(long newMailboxId, long[] messageIds) { + ActivityHelper.moveMessages(this, newMailboxId, messageIds); + if (!moveToOlder()) { + // if this is the last message, move up to message-list. + mFragmentManager.goBackToMailbox(); + } + } + /** * Start {@link MessageOrderManager} if not started, and sync it to the current message. */ @@ -739,14 +746,4 @@ public class MessageListXL extends Activity implements View.OnClickListener, return true; } } - - // TODO It's a temporary implementation. See {@link MoveMessagetoDialog} - @Override - public void onMoveToMailboxSelected(long newMailboxId, long[] messageIds) { - ActivityHelper.moveMessages(this, newMailboxId, messageIds); - if (!moveToOlder()) { - // if this is the last message, move up to message-list. - mFragmentManager.goBackToMailbox(); - } - } } diff --git a/src/com/android/email/activity/MoveMessageToDialog.java b/src/com/android/email/activity/MoveMessageToDialog.java index 6ad275e10..c23abd13e 100644 --- a/src/com/android/email/activity/MoveMessageToDialog.java +++ b/src/com/android/email/activity/MoveMessageToDialog.java @@ -17,41 +17,75 @@ package com.android.email.activity; import com.android.email.R; +import com.android.email.Utility; +import com.android.email.provider.EmailContent.Account; +import com.android.email.provider.EmailContent.Mailbox; +import com.android.email.provider.EmailContent.Message; import android.app.Activity; import android.app.AlertDialog; import android.app.Dialog; import android.app.DialogFragment; +import android.app.Fragment; import android.app.LoaderManager; +import android.content.AsyncTaskLoader; +import android.content.Context; import android.content.DialogInterface; import android.content.Loader; import android.database.Cursor; import android.os.Bundle; +import java.security.InvalidParameterException; + /** * "Move (messages) to" dialog. * * TODO Make callback mechanism better (don't use getActivity--use setTargetFragment instead.) * TODO Fix the text color in mailbox_list_item.xml. * TODO Don't show unread counts. + * TODO The check logic in MessageCheckerCallback is not efficient. It shouldn't restore full + * Message objects. But we don't bother at this point as the UI is still temporary. */ public class MoveMessageToDialog extends DialogFragment implements DialogInterface.OnClickListener { - private static final String BUNDLE_ACCOUNT_ID = "account_id"; private static final String BUNDLE_MESSAGE_IDS = "message_ids"; - private long mAccountId; + + /** Message IDs passed to {@link #newInstance} */ + private long[] mMessageIds; private MailboxesAdapter mAdapter; + /** Account ID is restored by {@link MailboxesLoaderCallbacks} */ + private long mAccountId; + + private boolean mDestroyed; + + /** + * Callback that target fragments, or the owner activity should implement. + */ public interface Callback { public void onMoveToMailboxSelected(long newMailboxId, long[] messageIds); } - public static MoveMessageToDialog newInstance(Activity parent, long accountId, - long[] messageIds) { + /** + * Create and return a new instance. + * + * @param parent owner activity. + * @param messageIds IDs of the messages to be moved. + * @param callbackFragment Fragment that gets a callback. The fragment must implement + * {@link Callback}. If null is passed, then the owner activity is used instead, in which case + * it must implement {@link Callback} instead. + */ + public static MoveMessageToDialog newInstance(Activity parent, + long[] messageIds, Fragment callbackFragment) { + if (messageIds.length == 0) { + throw new InvalidParameterException(); + } MoveMessageToDialog dialog = new MoveMessageToDialog(); Bundle args = new Bundle(); - args.putLong(BUNDLE_ACCOUNT_ID, accountId); args.putLongArray(BUNDLE_MESSAGE_IDS, messageIds); dialog.setArguments(args); + if (callbackFragment != null) { + dialog.setTargetFragment(callbackFragment, 0); + } return dialog; } @@ -59,17 +93,27 @@ public class MoveMessageToDialog extends DialogFragment implements DialogInterfa public void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); mAdapter = new MailboxesAdapter(getActivity().getApplicationContext()); - mAccountId = getArguments().getLong(BUNDLE_ACCOUNT_ID); + mMessageIds = getArguments().getLongArray(BUNDLE_MESSAGE_IDS); setStyle(STYLE_NORMAL, android.R.style.Theme_Light_Holo); } + @Override + public void onDestroy() { + LoaderManager lm = getActivity().getLoaderManager(); + lm.stopLoader(ActivityHelper.GLOBAL_LOADER_ID_MOVE_TO_DIALOG_MESSAGE_CHECKER); + lm.stopLoader(ActivityHelper.GLOBAL_LOADER_ID_MOVE_TO_DIALOG_MAILBOX_LOADER); + mDestroyed = true; + super.onDestroy(); + } + @Override public Dialog onCreateDialog(Bundle savedInstanceState) { final Activity a = getActivity(); final String title = a.getResources().getString(R.string.move_to_folder_dialog_title); - a.getLoaderManager().initLoader(ActivityHelper.GLOBAL_LOADER_ID_MOVE_TO_DIALOG_LOADER, - getArguments(), new MyLoaderCallbacks()); + a.getLoaderManager().initLoader( + ActivityHelper.GLOBAL_LOADER_ID_MOVE_TO_DIALOG_MESSAGE_CHECKER, + null, new MessageCheckerCallback()); return new AlertDialog.Builder(a) .setTitle(title) @@ -80,14 +124,51 @@ public class MoveMessageToDialog extends DialogFragment implements DialogInterfa @Override public void onClick(DialogInterface dialog, int position) { final long mailboxId = mAdapter.getItemId(position); - final long[] massageIds = getArguments().getLongArray(BUNDLE_MESSAGE_IDS); - // TODO Fix it. It's not flexible - ((Callback) getActivity()).onMoveToMailboxSelected(mailboxId, massageIds); + getCallback().onMoveToMailboxSelected(mailboxId, mMessageIds); dismiss(); } - private class MyLoaderCallbacks implements LoaderManager.LoaderCallbacks { + private Callback getCallback() { + Fragment targetFragment = getTargetFragment(); + if (targetFragment != null) { + // If a target is set, it MUST implement Callback. + return (Callback) targetFragment; + } + // If not the parent activity MUST implement Callback. + return (Callback) getActivity(); + } + + /** + * Loader callback for {@link MessageChecker} + */ + private class MessageCheckerCallback implements LoaderManager.LoaderCallbacks { + @Override + public Loader onCreateLoader(int id, Bundle args) { + return new MessageChecker(getActivity(), mMessageIds); + } + + @Override + public void onLoadFinished(Loader loader, Long accountId) { + if (mDestroyed) { + return; + } + // accountId shouldn't be null, but I'm paranoia. + if ((accountId == null) || (accountId == -1)) { + dismiss(); // Some of the messages can't be moved. + return; + } + mAccountId = accountId; + getActivity().getLoaderManager().initLoader( + ActivityHelper.GLOBAL_LOADER_ID_MOVE_TO_DIALOG_MAILBOX_LOADER, + null, new MailboxesLoaderCallbacks()); + } + } + + /** + * Loader callback for destination mailbox list. + */ + private class MailboxesLoaderCallbacks implements LoaderManager.LoaderCallbacks { @Override public Loader onCreateLoader(int id, Bundle args) { return MailboxesAdapter.createLoader(getActivity().getApplicationContext(), mAccountId, @@ -96,7 +177,82 @@ public class MoveMessageToDialog extends DialogFragment implements DialogInterfa @Override public void onLoadFinished(Loader loader, Cursor data) { + if (mDestroyed) { + return; + } mAdapter.changeCursor(data); } } + + /** + * A loader that checks if the messages can be moved, and return the Id of the account that owns + * the messages. (If any of the messages can't be moved, return -1.) + */ + private static class MessageChecker extends AsyncTaskLoader { + private final Activity mActivity; + private final long[] mMessageIds; + + public MessageChecker(Activity activity, long[] messageIds) { + super(activity); + mActivity = activity; + mMessageIds = messageIds; + } + + @Override + public Long loadInBackground() { + final Context c = getContext(); + + long accountId = -1; // -1 == account not found yet. + + for (long messageId : mMessageIds) { + // TODO This shouln't restore a full Message object. + final Message message = Message.restoreMessageWithId(c, messageId); + if (message == null) { + continue; // Skip removed messages. + } + + // First, check account. + if (accountId == -1) { + // First message -- see if the account supports move. + accountId = message.mAccountKey; + if (!Account.supportsMoveMessages(c, accountId)) { + Utility.showToast( + mActivity, R.string.cannot_move_protocol_not_supported_toast); + return -1L; + } + } else { + // Following messages -- have to belong to the same account + if (message.mAccountKey != accountId) { + Utility.showToast(mActivity, R.string.cannot_move_multiple_accounts_toast); + return -1L; + } + } + // Second, check mailbox. + if (!Mailbox.canMoveFrom(c, message.mMailboxKey)) { + Utility.showToast(mActivity, R.string.cannot_move_special_messages); + return -1L; + } + } + + // If all messages have been removed, accountId remains -1, which is what we should + // return here. + return accountId; + } + + @Override + public void startLoading() { + cancelLoad(); + forceLoad(); + } + + @Override + public void stopLoading() { + cancelLoad(); + } + + @Override + public void destroy() { + stopLoading(); + } + } } diff --git a/src/com/android/email/provider/EmailContent.java b/src/com/android/email/provider/EmailContent.java index 50427ba5e..da0e4bd37 100644 --- a/src/com/android/email/provider/EmailContent.java +++ b/src/com/android/email/provider/EmailContent.java @@ -35,6 +35,7 @@ import android.os.RemoteException; import java.io.File; import java.net.URI; import java.net.URISyntaxException; +import java.security.InvalidParameterException; import java.util.ArrayList; import java.util.List; import java.util.UUID; @@ -1286,6 +1287,14 @@ public abstract class EmailContent { return "eas".equals(getProtocol(context)); } + /** + * @return true if the account supports "move messages". + */ + public static boolean supportsMoveMessages(Context context, long accountId) { + String protocol = getProtocol(context, accountId); + return "eas".equals(protocol) || "imap".equals(protocol); + } + /** * Set the account to be the default account. If this is set to "true", when the account * is saved, all other accounts will have the same value set to "false". @@ -2299,6 +2308,8 @@ public abstract class EmailContent { } /** + * @param mailboxId ID of a mailbox. This method accepts magic mailbox IDs, such as + * {@link #QUERY_ALL_INBOXES}. (They're all non-refreshable.) * @return true if a mailbox is refreshable. */ public static boolean isRefreshable(Context context, long mailboxId) { @@ -2313,6 +2324,33 @@ public abstract class EmailContent { } return true; } + + /** + * @param mailboxId ID of a mailbox. This method DOES NOT accept magic mailbox IDs, such as + * {@link #QUERY_ALL_INBOXES} (because only the actual mailbox ID matters here. e.g. + * {@link #QUERY_ALL_FAVORITES} can contain ANY kind of messages), so don't pass a negative + * value. + * @return true if messages in a mailbox can be moved to another mailbox. + * This method only checks the mailbox information. It doesn't check its account/protocol, + * so it may return true even for POP3 mailbox. + */ + public static boolean canMoveFrom(Context context, long mailboxId) { + if (mailboxId < 0) { + throw new InvalidParameterException(); + } + Uri url = ContentUris.withAppendedId(Mailbox.CONTENT_URI, mailboxId); + int type = Utility.getFirstRowInt(context, url, MAILBOX_TYPE_PROJECTION, + null, null, null, MAILBOX_TYPE_TYPE_COLUMN); + switch (type) { + case TYPE_INBOX: + case TYPE_MAIL: + case TYPE_TRASH: + case TYPE_JUNK: + return true; + } + return false; // TYPE_DRAFTS, TYPE_OUTBOX, TYPE_SENT, etc + } + } public interface HostAuthColumns { diff --git a/tests/src/com/android/email/UtilityUnitTests.java b/tests/src/com/android/email/UtilityUnitTests.java index 670420b3f..52db07402 100644 --- a/tests/src/com/android/email/UtilityUnitTests.java +++ b/tests/src/com/android/email/UtilityUnitTests.java @@ -39,6 +39,8 @@ import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.IOException; import java.io.OutputStream; +import java.util.ArrayList; +import java.util.Collection; import java.util.HashSet; import java.util.Locale; import java.util.Set; @@ -479,6 +481,27 @@ public class UtilityUnitTests extends AndroidTestCase { } } + public void testToPrimitiveLongArray() { + assertEquals(0, Utility.toPrimitiveLongArray(createLongCollection()).length); + + final long[] one = Utility.toPrimitiveLongArray(createLongCollection(1)); + assertEquals(1, one.length); + assertEquals(1, one[0]); + + final long[] two = Utility.toPrimitiveLongArray(createLongCollection(3, 4)); + assertEquals(2, two.length); + assertEquals(3, two[0]); + assertEquals(4, two[1]); + } + + private static Collection createLongCollection(long... values) { + ArrayList ret = new ArrayList(); + for (long value : values) { + ret.add(value); + } + return ret; + } + /** * A {@link ListView} used by {@link #testListStateSaver}. */ diff --git a/tests/src/com/android/email/provider/ProviderTests.java b/tests/src/com/android/email/provider/ProviderTests.java index c02d150af..badbb6b74 100644 --- a/tests/src/com/android/email/provider/ProviderTests.java +++ b/tests/src/com/android/email/provider/ProviderTests.java @@ -2037,4 +2037,19 @@ public class ProviderTests extends ProviderTestCase2 { assertFalse(Mailbox.isRefreshable(c, Mailbox.QUERY_ALL_DRAFTS)); assertFalse(Mailbox.isRefreshable(c, Mailbox.QUERY_ALL_INBOXES)); } + + public void testMailboxCanMoveFrom() { + final Context c = mMockContext; + + Account a = ProviderTestUtils.setupAccount("acct1", true, c); + Mailbox bi = ProviderTestUtils.setupMailbox("b1", a.mId, true, c, Mailbox.TYPE_INBOX); + Mailbox bm = ProviderTestUtils.setupMailbox("b1", a.mId, true, c, Mailbox.TYPE_MAIL); + Mailbox bd = ProviderTestUtils.setupMailbox("b1", a.mId, true, c, Mailbox.TYPE_DRAFTS); + Mailbox bo = ProviderTestUtils.setupMailbox("b1", a.mId, true, c, Mailbox.TYPE_OUTBOX); + + assertTrue(Mailbox.canMoveFrom(c, bi.mId)); + assertTrue(Mailbox.canMoveFrom(c, bm.mId)); + assertFalse(Mailbox.canMoveFrom(c, bd.mId)); + assertFalse(Mailbox.canMoveFrom(c, bo.mId)); + } }