Remove all bad ideas from SelectedConversations
1. Remove references to AbstractActivityController and
ConversationCursor (yikes!) from the selected conversation set.
2. Create a new interface for delegating all changes to conversation
state.
3. Consolidate FolderChangesCommitListener with interface created above.
Change-Id: Id1d982a19eb2d26c51aec661cee5dc23ef98c110
diff --git a/src/com/android/mail/browse/SelectedConversationsActionMenu.java b/src/com/android/mail/browse/SelectedConversationsActionMenu.java
index 13fd7cb..294418f 100644
--- a/src/com/android/mail/browse/SelectedConversationsActionMenu.java
+++ b/src/com/android/mail/browse/SelectedConversationsActionMenu.java
@@ -36,10 +36,11 @@
import com.android.mail.providers.UIProvider;
import com.android.mail.providers.UIProvider.ConversationColumns;
import com.android.mail.providers.UIProvider.FolderCapabilities;
-import com.android.mail.ui.AbstractActivityController;
import com.android.mail.ui.AnimatedAdapter;
+import com.android.mail.ui.ControllableActivity;
import com.android.mail.ui.ConversationSelectionSet;
import com.android.mail.ui.ConversationSetObserver;
+import com.android.mail.ui.ConversationUpdater;
import com.android.mail.ui.DestructiveAction;
import com.android.mail.ui.FoldersSelectionDialog;
import com.android.mail.ui.RestrictedActivity;
@@ -81,48 +82,28 @@
private Menu mMenu;
private AnimatedAdapter mListAdapter;
- // TODO(viki): Bad idea. This is a relic of the previous method of having a DestructiveAction.
- // A better implementation is not to have clients know about destructive actions but rather
- // request them from the controller directly. Then, you wouldn't need to know when to commit
- // them.
- private AbstractActivityController mController;
- private Account mAccount;
+ /** Object that can update conversation state on our behalf. */
+ private final ConversationUpdater mUpdater;
- protected int mCheckedItem = 0;
+ private final Account mAccount;
- private Folder mFolder;
+ private final Folder mFolder;
- private final ConversationCursor mConversationCursor;
-
- private SwipeableListView mListView;
+ private final SwipeableListView mListView;
public SelectedConversationsActionMenu(RestrictedActivity activity,
- ConversationSelectionSet selectionSet, AnimatedAdapter adapter,
- AbstractActivityController controller, Account account,
+ ConversationSelectionSet selectionSet, AnimatedAdapter adapter, Account account,
Folder folder, SwipeableListView list) {
mActivity = activity;
mSelectionSet = selectionSet;
mListAdapter = adapter;
- mConversationCursor = (ConversationCursor)adapter.getCursor();
- mController = controller;
mAccount = account;
mFolder = folder;
mListView = list;
mContext = mActivity.getActivityContext();
- }
-
- /**
- * Registers a destructive action with the controller and returns it.
- * @param type the resource id of the menu item that corresponds to this action: R.id.delete
- * for example.
- * @return the {@link DestructiveAction} associated with this action.
- */
- // TODO(viki): This is a placeholder during the refactoring. Ideally the controller hands
- // the ID of the action to clients.
- private final DestructiveAction getAction(int type) {
- return mController.getBatchDestruction(type);
+ mUpdater = ((ControllableActivity) mActivity).getConversationUpdater();
}
@Override
@@ -137,10 +118,10 @@
performDestructiveAction(R.id.archive);
break;
case R.id.mute:
- mListAdapter.delete(conversations, getAction(R.id.mute));
+ mListAdapter.delete(conversations, mUpdater.getBatchAction(R.id.mute));
break;
case R.id.report_spam:
- mListAdapter.delete(conversations, getAction(R.id.report_spam));
+ mListAdapter.delete(conversations, mUpdater.getBatchAction(R.id.report_spam));
break;
case R.id.read:
markConversationsRead(true);
@@ -183,7 +164,7 @@
}
}
if (!cantMove) {
- new FoldersSelectionDialog(mContext, acct, mController,
+ new FoldersSelectionDialog(mContext, acct, mUpdater,
mSelectionSet.values(), true).show();
}
break;
@@ -226,7 +207,7 @@
}
private void performDestructiveAction(final int id) {
- final DestructiveAction action = getAction(id);
+ final DestructiveAction action = mUpdater.getBatchAction(id);
final Settings settings = mActivity.getSettings();
final Collection<Conversation> conversations = mSelectionSet.values();
final boolean showDialog = (settings != null
@@ -262,17 +243,14 @@
}
private void markConversationsRead(boolean read) {
- final Collection<Conversation> conversations = mSelectionSet.values();
- mConversationCursor.updateBoolean(mContext, conversations, ConversationColumns.READ, read);
+ mUpdater.updateConversation(mSelectionSet.values(), ConversationColumns.READ, read);
updateSelection();
}
private void markConversationsImportant(boolean important) {
- final Collection<Conversation> conversations = mSelectionSet.values();
final int priority = important ? UIProvider.ConversationPriority.HIGH
: UIProvider.ConversationPriority.LOW;
- mConversationCursor.updateInt(mContext, conversations, ConversationColumns.PRIORITY,
- priority);
+ mUpdater.updateConversation(mSelectionSet.values(), ConversationColumns.PRIORITY, priority);
updateSelection();
}
@@ -282,11 +260,7 @@
* stars from all conversations
*/
private void starConversations(boolean star) {
- final Collection<Conversation> conversations = mSelectionSet.values();
- if (conversations.size() > 0) {
- mConversationCursor.updateBoolean(mContext, conversations, ConversationColumns.STARRED,
- star);
- }
+ mUpdater.updateConversation(mSelectionSet.values(), ConversationColumns.STARRED, star);
updateSelection();
}
diff --git a/src/com/android/mail/providers/UIProvider.java b/src/com/android/mail/providers/UIProvider.java
index d5ef631..c9b1c10 100644
--- a/src/com/android/mail/providers/UIProvider.java
+++ b/src/com/android/mail/providers/UIProvider.java
@@ -740,6 +740,9 @@
public static final int CALENDAR_INVITE = 1<<4;
}
+ /**
+ * Names of columns representing fields in a Conversation.
+ */
public static final class ConversationColumns {
public static final String URI = "conversationUri";
/**
diff --git a/src/com/android/mail/ui/AbstractActivityController.java b/src/com/android/mail/ui/AbstractActivityController.java
index 6de5ce3..0acbc0f 100644
--- a/src/com/android/mail/ui/AbstractActivityController.java
+++ b/src/com/android/mail/ui/AbstractActivityController.java
@@ -53,7 +53,6 @@
import com.android.mail.ConversationListContext;
import com.android.mail.R;
import com.android.mail.browse.ConversationCursor;
-import com.android.mail.browse.ConversationCursor.ConversationListener;
import com.android.mail.browse.ConversationPagerController;
import com.android.mail.browse.SelectedConversationsActionMenu;
import com.android.mail.compose.ComposeActivity;
@@ -96,8 +95,7 @@
* In the Gmail codebase, this was called BaseActivityController
* </p>
*/
-public abstract class AbstractActivityController implements ActivityController,
- ConversationListener, OnScrollListener {
+public abstract class AbstractActivityController implements ActivityController {
// Keys for serialization of various information in Bundles.
/** Tag for {@link #mAccount} */
private static final String SAVED_ACCOUNT = "saved-account";
@@ -617,12 +615,12 @@
break;
}
case R.id.mark_important:
- updateCurrentConversation(ConversationColumns.PRIORITY,
- UIProvider.ConversationPriority.HIGH);
+ updateConversation(Conversation.listOf(mCurrentConversation),
+ ConversationColumns.PRIORITY, UIProvider.ConversationPriority.HIGH);
break;
case R.id.mark_not_important:
- updateCurrentConversation(ConversationColumns.PRIORITY,
- UIProvider.ConversationPriority.LOW);
+ updateConversation(Conversation.listOf(mCurrentConversation),
+ ConversationColumns.PRIORITY, UIProvider.ConversationPriority.LOW);
break;
case R.id.mute:
requestDelete(target, getAction(R.id.mute, target));
@@ -631,7 +629,10 @@
requestDelete(target, getAction(R.id.report_spam, target));
break;
case R.id.inside_conversation_unread:
- updateCurrentConversation(ConversationColumns.READ, false);
+ // TODO(viki): This is strange, and potentially incorrect. READ is an int column
+ // in the provider.
+ updateConversation(Conversation.listOf(mCurrentConversation),
+ ConversationColumns.READ, false);
mViewMode.enterConversationListMode();
break;
case android.R.id.home:
@@ -673,31 +674,23 @@
return handled;
}
- /**
- * Update the specified column name in conversation for a boolean value.
- * @param columnName
- * @param value
- */
- protected void updateCurrentConversation(String columnName, boolean value) {
- mConversationListCursor.updateBoolean(mContext, Conversation.listOf(mCurrentConversation),
- columnName, value);
+ @Override
+ public void updateConversation(Collection <Conversation> target, String columnName,
+ boolean value) {
+ mConversationListCursor.updateBoolean(mContext, target, columnName, value);
refreshConversationList();
}
- /**
- * Update the specified column name in conversation for an integer value.
- * @param columnName
- * @param value
- */
- protected void updateCurrentConversation(String columnName, int value) {
- mConversationListCursor.updateInt(mContext, Conversation.listOf(mCurrentConversation),
- columnName, value);
+ @Override
+ public void updateConversation(Collection <Conversation> target, String columnName, int value) {
+ mConversationListCursor.updateInt(mContext, target, columnName, value);
refreshConversationList();
}
- protected void updateCurrentConversation(String columnName, String value) {
- mConversationListCursor.updateString(mContext, Conversation.listOf(mCurrentConversation),
- columnName, value);
+ @Override
+ public void updateConversation(Collection <Conversation> target, String columnName,
+ String value) {
+ mConversationListCursor.updateString(mContext, target, columnName, value);
refreshConversationList();
}
@@ -1426,7 +1419,7 @@
* clients should only require {@link DestructiveAction}s, not specific implementations of the.
* Only the controllers should know what kind of destructive actions are being created.
*/
- protected class ConversationAction implements DestructiveAction {
+ private class ConversationAction implements DestructiveAction {
/**
* The action to be performed. This is specified as the resource ID of the menu item
* corresponding to this action: R.id.delete, R.id.report_spam, etc.
@@ -1547,7 +1540,7 @@
* @param target the conversations to act upon.
* @return a {@link DestructiveAction} that performs the specified action.
*/
- protected final DestructiveAction getAction(int action, Collection<Conversation> target) {
+ private final DestructiveAction getAction(int action, Collection<Conversation> target) {
final DestructiveAction da = new ConversationAction(action, target, false);
registerDestructiveAction(da);
return da;
@@ -1556,7 +1549,7 @@
// Called from the FolderSelectionDialog after a user is done selecting folders to assign the
// conversations to.
@Override
- public final void onFolderChangesCommit(
+ public final void assignFolder(
Collection<Folder> folders, Collection<Conversation> target, boolean batch) {
final boolean isDestructive = !Folder.containerIncludes(folders, mFolder);
LogUtils.d(LOG_TAG, "onFolderChangesCommit: isDestructive = %b", isDestructive);
@@ -1689,8 +1682,8 @@
return;
}
mCabActionMenu = new SelectedConversationsActionMenu(mActivity, set,
- convList.getAnimatedAdapter(), this,
- mAccount, mFolder, (SwipeableListView) convList.getListView());
+ convList.getAnimatedAdapter(), mAccount, mFolder,
+ (SwipeableListView) convList.getListView());
enableCabMode();
}
@@ -1912,12 +1905,8 @@
return;
}
- /**
- * Get a destructive action for selected conversations.
- * @param action
- * @return
- */
- public final DestructiveAction getBatchDestruction(int action) {
+ @Override
+ public final DestructiveAction getBatchAction(int action) {
final DestructiveAction da = new ConversationAction(action, mSelectedSet.values(), true);
registerDestructiveAction(da);
return da;
diff --git a/src/com/android/mail/ui/ActivityController.java b/src/com/android/mail/ui/ActivityController.java
index 1eef46e..ede3a5e 100644
--- a/src/com/android/mail/ui/ActivityController.java
+++ b/src/com/android/mail/ui/ActivityController.java
@@ -27,13 +27,14 @@
import android.view.Menu;
import android.view.MenuItem;
import android.view.MotionEvent;
+import android.widget.AbsListView.OnScrollListener;
import com.android.mail.ConversationListContext;
+import com.android.mail.browse.ConversationCursor.ConversationListener;
import com.android.mail.providers.Account;
import com.android.mail.providers.Conversation;
import com.android.mail.providers.Folder;
import com.android.mail.providers.Settings;
-import com.android.mail.ui.FoldersSelectionDialog.FolderChangeCommitListener;
import com.android.mail.ui.ViewMode.ModeChangeListener;
/**
@@ -42,10 +43,11 @@
* or respond to user action.
*/
public interface ActivityController extends LayoutListener, SubjectDisplayChanger,
- ModeChangeListener, ConversationListCallbacks, FolderChangeCommitListener,
+ ModeChangeListener, ConversationListCallbacks,
FolderChangeListener, AccountChangeListener, LoaderManager.LoaderCallbacks<Cursor>,
- ConversationSetObserver,
- FolderListFragment.FolderListSelectionListener, HelpCallback, UndoBarView.UndoListener {
+ ConversationSetObserver, ConversationListener, OnScrollListener,
+ FolderListFragment.FolderListSelectionListener, HelpCallback, UndoBarView.UndoListener,
+ ConversationUpdater {
// As far as possible, the methods here that correspond to Activity lifecycle have the same name
// as their counterpart in the Activity lifecycle.
diff --git a/src/com/android/mail/ui/ControllableActivity.java b/src/com/android/mail/ui/ControllableActivity.java
index ce573be..689627b 100644
--- a/src/com/android/mail/ui/ControllableActivity.java
+++ b/src/com/android/mail/ui/ControllableActivity.java
@@ -67,6 +67,11 @@
*/
boolean shouldShowFirstConversation();
+ /**
+ * Get the set of currently selected conversations. This method returns a non-null value.
+ * In case no conversation is currently selected, it returns an empty selection set.
+ * @return
+ */
ConversationSelectionSet getSelectedSet();
/**
@@ -82,4 +87,12 @@
* Get the folder currently being accessed by the activity.
*/
Folder getCurrentFolder();
+
+ /**
+ * Returns an object that can update conversation state. Holding a reference to the
+ * ConversationUpdater is safe since the ConversationUpdater is guaranteed to persist across
+ * changes to the conversation cursor.
+ * @return
+ */
+ ConversationUpdater getConversationUpdater();
}
diff --git a/src/com/android/mail/ui/ConversationUpdater.java b/src/com/android/mail/ui/ConversationUpdater.java
new file mode 100644
index 0000000..60cb4ee
--- /dev/null
+++ b/src/com/android/mail/ui/ConversationUpdater.java
@@ -0,0 +1,77 @@
+/*
+ * Copyright (C) 2012 Google Inc.
+ * Licensed to The Android Open Source Project.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.mail.ui;
+
+import com.android.mail.providers.Conversation;
+import com.android.mail.providers.Folder;
+import com.android.mail.providers.UIProvider;
+
+import java.util.Collection;
+
+/**
+ * Classes that can update conversations implement this interface.
+ */
+public interface ConversationUpdater {
+ /**
+ * Modify the given conversation by changing the column provided here to contain the value
+ * provided. Column names are listed in {@link UIProvider.ConversationColumns}, for example
+ * {@link UIProvider.ConversationColumns#FOLDER_LIST}
+ * @param target
+ * @param columnName
+ * @param value
+ */
+ void updateConversation(Collection <Conversation> target, String columnName, String value);
+
+ /**
+ * Modify the given conversation by changing the column provided here to contain the value
+ * provided. Column names are listed in {@link UIProvider.ConversationColumns}, for example
+ * {@link UIProvider.ConversationColumns#READ}
+ * @param target
+ * @param columnName
+ * @param value
+ */
+ void updateConversation(Collection <Conversation> target, String columnName, int value);
+
+ /**
+ * Modify the given conversation by changing the column provided here to contain the value
+ * provided. Column names are listed in {@link UIProvider.ConversationColumns}, for example
+ * {@link UIProvider.ConversationColumns#HAS_ATTACHMENTS}
+ * @param target
+ * @param columnName
+ * @param value
+ */
+ void updateConversation(Collection <Conversation> target, String columnName, boolean value);
+
+ /**
+ * Get a destructive action for selected conversations. The action corresponds to Menu item
+ * identifiers, for example R.id.unread, or R.id.delete.
+ * @param action
+ * @return
+ */
+ public DestructiveAction getBatchAction(int action);
+
+ /**
+ * Assign the target conversations to the given folders, and remove them from all other
+ * folders that they might be assigned to.
+ * @param folders the folders to assign the conversations to.
+ * @param target the conversations to act upon.
+ * @param batch whether this is a batch operation
+ */
+ public void assignFolder(
+ Collection<Folder> folders, Collection<Conversation> target, boolean batch);
+}
diff --git a/src/com/android/mail/ui/FolderSelectionActivity.java b/src/com/android/mail/ui/FolderSelectionActivity.java
index bcf8c6f..6f38925 100644
--- a/src/com/android/mail/ui/FolderSelectionActivity.java
+++ b/src/com/android/mail/ui/FolderSelectionActivity.java
@@ -319,4 +319,9 @@
public Folder getCurrentFolder() {
return null;
}
+
+ @Override
+ public ConversationUpdater getConversationUpdater() {
+ return null;
+ }
}
diff --git a/src/com/android/mail/ui/FoldersSelectionDialog.java b/src/com/android/mail/ui/FoldersSelectionDialog.java
index b971c4c..21ad75e 100644
--- a/src/com/android/mail/ui/FoldersSelectionDialog.java
+++ b/src/com/android/mail/ui/FoldersSelectionDialog.java
@@ -42,29 +42,16 @@
public class FoldersSelectionDialog implements OnClickListener, OnMultiChoiceClickListener {
private AlertDialog mDialog;
- private FolderChangeCommitListener mCommitListener;
+ private ConversationUpdater mUpdater;
private HashMap<Folder, Boolean> mCheckedState;
private boolean mSingle = false;
private FolderSelectorAdapter mAdapter;
private final Collection<Conversation> mTarget;
private boolean mBatch;
- public interface FolderChangeCommitListener {
- /**
- * Assign the target conversations to the given folders, and remove them from all other
- * folders that they might be assigned to.
- * @param folders the folders to assign the conversations to.
- * @param target the conversations to act upon.
- * @param batch whether this is a batch operation
- */
- public void onFolderChangesCommit(
- Collection<Folder> folders, Collection<Conversation> target, boolean batch);
- }
-
public FoldersSelectionDialog(final Context context, Account account,
- final FolderChangeCommitListener commitListener,
- Collection<Conversation> target, boolean isBatch) {
- mCommitListener = commitListener;
+ final ConversationUpdater updater, Collection<Conversation> target, boolean isBatch) {
+ mUpdater = updater;
mTarget = target;
mBatch = isBatch;
@@ -152,8 +139,8 @@
folders.add(entry.getKey());
}
}
- if (mCommitListener != null) {
- mCommitListener.onFolderChangesCommit(folders, mTarget, mBatch);
+ if (mUpdater != null) {
+ mUpdater.assignFolder(folders, mTarget, mBatch);
}
break;
case DialogInterface.BUTTON_NEGATIVE:
diff --git a/src/com/android/mail/ui/MailActivity.java b/src/com/android/mail/ui/MailActivity.java
index d09bb71..af26817 100644
--- a/src/com/android/mail/ui/MailActivity.java
+++ b/src/com/android/mail/ui/MailActivity.java
@@ -300,4 +300,9 @@
public Folder getCurrentFolder() {
return mController.getFolder();
}
+
+ @Override
+ public ConversationUpdater getConversationUpdater() {
+ return mController;
+ }
}