Batch add and remove group members in group editor

- Create save intent in ContactSaveService to update
the group name, add a list of new members, and remove
a list of existing members

- Add raw contact ID column to GroupMemberLoader so that
the raw contact IDs can be used in ContactSaveService

- Remove old code that would do the database save one at a
time for each contact member when the action was taken
(and remove the step that would load the full contact because
we now only need the raw contact ID)

- TODO: General cleanup to just use raw contact IDs everywhere
and have one Member class (get rid of SuggestedMember)

- TODO: Allow users to add contacts for new groups (still
can only change the name), but this should be straightforward
after this change

Bug: 4961845
Change-Id: I8a2f1086feecbb63dc6eb3d1e985bccabe28b803
diff --git a/src/com/android/contacts/ContactSaveService.java b/src/com/android/contacts/ContactSaveService.java
index 5912225..194d0ba 100644
--- a/src/com/android/contacts/ContactSaveService.java
+++ b/src/com/android/contacts/ContactSaveService.java
@@ -78,8 +78,11 @@
     public static final String ACTION_CREATE_GROUP = "createGroup";
     public static final String ACTION_RENAME_GROUP = "renameGroup";
     public static final String ACTION_DELETE_GROUP = "deleteGroup";
+    public static final String ACTION_UPDATE_GROUP = "updateGroup";
     public static final String EXTRA_GROUP_ID = "groupId";
     public static final String EXTRA_GROUP_LABEL = "groupLabel";
+    public static final String EXTRA_RAW_CONTACTS_TO_ADD = "rawContactsToAdd";
+    public static final String EXTRA_RAW_CONTACTS_TO_REMOVE = "rawContactsToRemove";
 
     public static final String ACTION_SET_STARRED = "setStarred";
     public static final String ACTION_DELETE_CONTACT = "delete";
@@ -170,6 +173,8 @@
             renameGroup(intent);
         } else if (ACTION_DELETE_GROUP.equals(action)) {
             deleteGroup(intent);
+        } else if (ACTION_UPDATE_GROUP.equals(action)) {
+            updateGroup(intent);
         } else if (ACTION_SET_STARRED.equals(action)) {
             setStarred(intent);
         } else if (ACTION_SET_SUPER_PRIMARY.equals(action)) {
@@ -471,6 +476,125 @@
     }
 
     /**
+     * Creates an intent that can be sent to this service to rename a group as
+     * well as add and remove members from the group.
+     *
+     * @param context of the application
+     * @param groupId of the group that should be modified
+     * @param newLabel is the updated name of the group (can be null if the name
+     *            should not be updated)
+     * @param rawContactsToAdd is an array of raw contact IDs for contacts that
+     *            should be added to the group
+     * @param rawContactsToRemove is an array of raw contact IDs for contacts
+     *            that should be removed from the group
+     * @param callbackActivity is the activity to send the callback intent to
+     * @param callbackAction is the intent action for the callback intent
+     */
+    public static Intent createGroupUpdateIntent(Context context, long groupId, String newLabel,
+            long[] rawContactsToAdd, long[] rawContactsToRemove,
+            Class<?> callbackActivity, String callbackAction) {
+        Intent serviceIntent = new Intent(context, ContactSaveService.class);
+        serviceIntent.setAction(ContactSaveService.ACTION_UPDATE_GROUP);
+        serviceIntent.putExtra(ContactSaveService.EXTRA_GROUP_ID, groupId);
+        serviceIntent.putExtra(ContactSaveService.EXTRA_GROUP_LABEL, newLabel);
+        serviceIntent.putExtra(ContactSaveService.EXTRA_RAW_CONTACTS_TO_ADD, rawContactsToAdd);
+        serviceIntent.putExtra(ContactSaveService.EXTRA_RAW_CONTACTS_TO_REMOVE,
+                rawContactsToRemove);
+
+        // Callback intent will be invoked by the service once the group is updated
+        Intent callbackIntent = new Intent(context, callbackActivity);
+        callbackIntent.setAction(callbackAction);
+        serviceIntent.putExtra(ContactSaveService.EXTRA_CALLBACK_INTENT, callbackIntent);
+
+        return serviceIntent;
+    }
+
+    private void updateGroup(Intent intent) {
+        long groupId = intent.getLongExtra(EXTRA_GROUP_ID, -1);
+        String label = intent.getStringExtra(EXTRA_GROUP_LABEL);
+        long[] rawContactsToAdd = intent.getLongArrayExtra(EXTRA_RAW_CONTACTS_TO_ADD);
+        long[] rawContactsToRemove = intent.getLongArrayExtra(EXTRA_RAW_CONTACTS_TO_REMOVE);
+
+        if (groupId == -1) {
+            Log.e(TAG, "Invalid arguments for updateGroup request");
+            return;
+        }
+
+        final ContentResolver resolver = getContentResolver();
+        final Uri groupUri = ContentUris.withAppendedId(Groups.CONTENT_URI, groupId);
+
+        // Update group name if necessary
+        if (label != null) {
+            ContentValues values = new ContentValues();
+            values.put(Groups.TITLE, label);
+            getContentResolver().update(groupUri, values, null, null);
+        }
+
+        // Add new group members
+        for (long rawContactId : rawContactsToAdd) {
+            try {
+                final ArrayList<ContentProviderOperation> rawContactOperations =
+                        new ArrayList<ContentProviderOperation>();
+
+                // Build an assert operation to ensure the contact is not already in the group
+                final ContentProviderOperation.Builder assertBuilder = ContentProviderOperation
+                        .newAssertQuery(Data.CONTENT_URI);
+                assertBuilder.withSelection(Data.RAW_CONTACT_ID + "=? AND " +
+                        Data.MIMETYPE + "=? AND " + GroupMembership.GROUP_ROW_ID + "=?",
+                        new String[] { String.valueOf(rawContactId),
+                        GroupMembership.CONTENT_ITEM_TYPE, String.valueOf(groupId)});
+                assertBuilder.withExpectedCount(0);
+                rawContactOperations.add(assertBuilder.build());
+
+                // Build an insert operation to add the contact to the group
+                final ContentProviderOperation.Builder insertBuilder = ContentProviderOperation
+                        .newInsert(Data.CONTENT_URI);
+                insertBuilder.withValue(Data.RAW_CONTACT_ID, rawContactId);
+                insertBuilder.withValue(Data.MIMETYPE, GroupMembership.CONTENT_ITEM_TYPE);
+                insertBuilder.withValue(GroupMembership.GROUP_ROW_ID, groupId);
+                rawContactOperations.add(insertBuilder.build());
+
+                if (DEBUG) {
+                    for (ContentProviderOperation operation : rawContactOperations) {
+                        Log.v(TAG, operation.toString());
+                    }
+                }
+
+                // Apply batch
+                ContentProviderResult[] results = null;
+                if (!rawContactOperations.isEmpty()) {
+                    results = resolver.applyBatch(ContactsContract.AUTHORITY, rawContactOperations);
+                }
+            } catch (RemoteException e) {
+                // Something went wrong, bail without success
+                Log.e(TAG, "Problem persisting user edits for raw contact ID " +
+                        String.valueOf(rawContactId), e);
+            } catch (OperationApplicationException e) {
+                // The assert could have failed because the contact is already in the group,
+                // just continue to the next contact
+                Log.w(TAG, "Assert failed in adding raw contact ID " +
+                        String.valueOf(rawContactId) + ". Already exists in group " +
+                        String.valueOf(groupId), e);
+            }
+        }
+
+        // Remove group members
+        for (long rawContactId : rawContactsToRemove) {
+            // Apply the delete operation on the data row for the given raw contact's
+            // membership in the given group. If no contact matches the provided selection, then
+            // nothing will be done. Just continue to the next contact.
+            getContentResolver().delete(Data.CONTENT_URI, Data.RAW_CONTACT_ID + "=? AND " +
+                    Data.MIMETYPE + "=? AND " + GroupMembership.GROUP_ROW_ID + "=?",
+                    new String[] { String.valueOf(rawContactId),
+                    GroupMembership.CONTENT_ITEM_TYPE, String.valueOf(groupId)});
+        }
+
+        Intent callbackIntent = intent.getParcelableExtra(EXTRA_CALLBACK_INTENT);
+        callbackIntent.setData(groupUri);
+        deliverCallback(callbackIntent);
+    }
+
+    /**
      * Creates an intent that can be sent to this service to star or un-star a contact.
      */
     public static Intent createSetStarredIntent(Context context, Uri contactUri, boolean value) {
diff --git a/src/com/android/contacts/GroupMemberLoader.java b/src/com/android/contacts/GroupMemberLoader.java
index 8f08ccd..13edff0 100644
--- a/src/com/android/contacts/GroupMemberLoader.java
+++ b/src/com/android/contacts/GroupMemberLoader.java
@@ -40,33 +40,35 @@
     private final String[] PROJECTION_DATA = new String[] {
         // TODO: Pull Projection_data out into util class
         Data.CONTACT_ID,                        // 0
-        Data.DISPLAY_NAME_PRIMARY,              // 1
-        Data.DISPLAY_NAME_ALTERNATIVE,          // 2
-        Data.SORT_KEY_PRIMARY,                  // 3
-        Data.STARRED,                           // 4
-        Data.CONTACT_PRESENCE,                  // 5
-        Data.CONTACT_CHAT_CAPABILITY,           // 6
-        Data.PHOTO_ID,                          // 7
-        Data.PHOTO_THUMBNAIL_URI,               // 8
-        Data.LOOKUP_KEY,                        // 9
-        Data.PHONETIC_NAME,                     // 10
-        Data.HAS_PHONE_NUMBER,                  // 11
+        Data.RAW_CONTACT_ID,                    // 1
+        Data.DISPLAY_NAME_PRIMARY,              // 2
+        Data.DISPLAY_NAME_ALTERNATIVE,          // 3
+        Data.SORT_KEY_PRIMARY,                  // 4
+        Data.STARRED,                           // 5
+        Data.CONTACT_PRESENCE,                  // 6
+        Data.CONTACT_CHAT_CAPABILITY,           // 7
+        Data.PHOTO_ID,                          // 8
+        Data.PHOTO_THUMBNAIL_URI,               // 9
+        Data.LOOKUP_KEY,                        // 10
+        Data.PHONETIC_NAME,                     // 11
+        Data.HAS_PHONE_NUMBER,                  // 12
     };
 
     private final long mGroupId;
 
     public static final int CONTACT_ID_COLUMN_INDEX = 0;
-    public static final int CONTACT_DISPLAY_NAME_PRIMARY_COLUMN_INDEX = 1;
-    public static final int CONTACT_DISPLAY_NAME_ALTERNATIVE_COLUMN_INDEX = 2;
-    public static final int CONTACT_SORT_KEY_PRIMARY_COLUMN_INDEX = 3;
-    public static final int CONTACT_STARRED_COLUMN_INDEX = 4;
-    public static final int CONTACT_PRESENCE_STATUS_COLUMN_INDEX = 5;
-    public static final int CONTACT_CHAT_CAPABILITY_COLUMN_INDEX = 6;
-    public static final int CONTACT_PHOTO_ID_COLUMN_INDEX = 7;
-    public static final int CONTACT_PHOTO_URI_COLUMN_INDEX = 8;
-    public static final int CONTACT_LOOKUP_KEY_COLUMN_INDEX = 9;
-    public static final int CONTACT_PHONETIC_NAME_COLUMN_INDEX = 10;
-    public static final int CONTACT_HAS_PHONE_COLUMN_INDEX = 11;
+    public static final int RAW_CONTACT_ID_COLUMN_INDEX = 1;
+    public static final int CONTACT_DISPLAY_NAME_PRIMARY_COLUMN_INDEX = 2;
+    public static final int CONTACT_DISPLAY_NAME_ALTERNATIVE_COLUMN_INDEX = 3;
+    public static final int CONTACT_SORT_KEY_PRIMARY_COLUMN_INDEX = 4;
+    public static final int CONTACT_STARRED_COLUMN_INDEX = 5;
+    public static final int CONTACT_PRESENCE_STATUS_COLUMN_INDEX = 6;
+    public static final int CONTACT_CHAT_CAPABILITY_COLUMN_INDEX = 7;
+    public static final int CONTACT_PHOTO_ID_COLUMN_INDEX = 8;
+    public static final int CONTACT_PHOTO_URI_COLUMN_INDEX = 9;
+    public static final int CONTACT_LOOKUP_KEY_COLUMN_INDEX = 10;
+    public static final int CONTACT_PHONETIC_NAME_COLUMN_INDEX = 11;
+    public static final int CONTACT_HAS_PHONE_COLUMN_INDEX = 12;
 
     public GroupMemberLoader(Context context, long groupId) {
         super(context);
diff --git a/src/com/android/contacts/activities/GroupEditorActivity.java b/src/com/android/contacts/activities/GroupEditorActivity.java
index e71d314..fa41051 100644
--- a/src/com/android/contacts/activities/GroupEditorActivity.java
+++ b/src/com/android/contacts/activities/GroupEditorActivity.java
@@ -110,10 +110,6 @@
             mFragment.onSaveCompleted(true,
                     intent.getIntExtra(GroupEditorFragment.SAVE_MODE_EXTRA_KEY, SaveMode.CLOSE),
                     intent.getData());
-        } else if (ACTION_ADD_MEMBER_COMPLETED.equals(action)) {
-            mFragment.finishAddMember(intent.getData());
-        } else if (ACTION_REMOVE_MEMBER_COMPLETED.equals(action)) {
-            mFragment.finishRemoveMember(intent.getData());
         }
     }
 
diff --git a/src/com/android/contacts/group/GroupEditorFragment.java b/src/com/android/contacts/group/GroupEditorFragment.java
index 429bb58..76f48a3 100644
--- a/src/com/android/contacts/group/GroupEditorFragment.java
+++ b/src/com/android/contacts/group/GroupEditorFragment.java
@@ -16,7 +16,6 @@
 
 package com.android.contacts.group;
 
-import com.android.contacts.ContactLoader;
 import com.android.contacts.ContactPhotoManager;
 import com.android.contacts.ContactSaveService;
 import com.android.contacts.GroupMemberLoader;
@@ -28,11 +27,6 @@
 import com.android.contacts.group.SuggestedMemberListAdapter.SuggestedMember;
 import com.android.contacts.model.AccountType;
 import com.android.contacts.model.AccountTypeManager;
-import com.android.contacts.model.DataKind;
-import com.android.contacts.model.EntityDelta;
-import com.android.contacts.model.EntityDelta.ValuesDelta;
-import com.android.contacts.model.EntityDeltaList;
-import com.android.contacts.model.EntityModifier;
 import com.android.internal.util.Objects;
 
 import android.accounts.Account;
@@ -54,9 +48,7 @@
 import android.net.Uri;
 import android.os.Bundle;
 import android.provider.ContactsContract.Contacts;
-import android.provider.ContactsContract.CommonDataKinds.GroupMembership;
 import android.provider.ContactsContract.Intents;
-import android.provider.ContactsContract.RawContacts;
 import android.text.TextUtils;
 import android.util.Log;
 import android.view.LayoutInflater;
@@ -112,15 +104,11 @@
     private static final int LOADER_GROUP_METADATA = 1;
     private static final int LOADER_EXISTING_MEMBERS = 2;
     private static final int LOADER_NEW_GROUP_MEMBER = 3;
-    private static final int FULL_LOADER_NEW_GROUP_MEMBER = 4;
 
     public static final String SAVE_MODE_EXTRA_KEY = "saveMode";
 
+    private static final String MEMBER_RAW_CONTACT_ID_KEY = "rawContactId";
     private static final String MEMBER_LOOKUP_URI_KEY = "memberLookupUri";
-    private static final String MEMBER_ACTION_KEY = "memberAction";
-
-    private static final int ADD_MEMBER = 0;
-    private static final int REMOVE_MEMBER = 1;
 
     protected static final String[] PROJECTION_CONTACT = new String[] {
         Contacts._ID,                           // 0
@@ -189,11 +177,13 @@
     private MemberListAdapter mMemberListAdapter;
     private ContactPhotoManager mPhotoManager;
 
-    private Member mMemberToRemove;
-
     private ContentResolver mContentResolver;
     private SuggestedMemberListAdapter mAutoCompleteAdapter;
 
+    private List<Member> mListMembersToAdd = new ArrayList<Member>();
+    private List<Member> mListMembersToRemove = new ArrayList<Member>();
+    private List<Member> mListToDisplay = new ArrayList<Member>();
+
     public GroupEditorFragment() {
     }
 
@@ -236,8 +226,6 @@
             }
             getLoaderManager().initLoader(LOADER_GROUP_METADATA, null,
                     mGroupMetaDataLoaderListener);
-            getLoaderManager().initLoader(LOADER_EXISTING_MEMBERS, null,
-                    mGroupMemberListLoaderListener);
         } else if (Intent.ACTION_INSERT.equals(mAction)) {
             if (mListener != null) {
                 mListener.onTitleLoaded(R.string.editGroup_title_insert);
@@ -362,37 +350,15 @@
         mGroupNameView.setText(mOriginalGroupName);
         mGroupNameView.setFocusable(!mGroupNameIsReadOnly);
         setupAccountHeader();
-
     }
 
-    public void loadMemberToAddToGroup(String contactId) {
+    public void loadMemberToAddToGroup(long rawContactId, String contactId) {
         Bundle args = new Bundle();
+        args.putLong(MEMBER_RAW_CONTACT_ID_KEY, rawContactId);
         args.putString(MEMBER_LOOKUP_URI_KEY, contactId);
-        args.putInt(MEMBER_ACTION_KEY, ADD_MEMBER);
         getLoaderManager().restartLoader(LOADER_NEW_GROUP_MEMBER, args, mContactLoaderListener);
     }
 
-    private void loadMemberToRemoveFromGroup(String lookupUri) {
-        Bundle args = new Bundle();
-        args.putString(MEMBER_LOOKUP_URI_KEY, lookupUri);
-        args.putInt(MEMBER_ACTION_KEY, REMOVE_MEMBER);
-        getLoaderManager().restartLoader(FULL_LOADER_NEW_GROUP_MEMBER, args,
-                mDataLoaderListener);
-    }
-
-    public void finishAddMember(Uri lookupUri) {
-        Toast.makeText(mContext, mContext.getString(R.string.groupMembershipChangeSavedToast),
-                Toast.LENGTH_SHORT).show();
-        getLoaderManager().destroyLoader(FULL_LOADER_NEW_GROUP_MEMBER);
-    }
-
-    public void finishRemoveMember(Uri lookupUri) {
-        Toast.makeText(mContext, mContext.getString(R.string.groupMembershipChangeSavedToast),
-                Toast.LENGTH_SHORT).show();
-        getLoaderManager().destroyLoader(FULL_LOADER_NEW_GROUP_MEMBER);
-        mMemberListAdapter.removeMember(mMemberToRemove);
-    }
-
     public void setListener(Listener value) {
         mListener = value;
     }
@@ -472,13 +438,14 @@
             getLoaderManager().destroyLoader(LOADER_EXISTING_MEMBERS);
         }
 
-        mStatus = Status.SAVING;
-
-        if (!hasChanges()) {
-            onSaveCompleted(false, saveMode, mGroupUri);
+        // If there are no changes, then go straight to onSaveCompleted()
+        if (!hasNameChange() && !hasMembershipChange()) {
+            onSaveCompleted(false, SaveMode.CLOSE, mGroupUri);
             return true;
         }
 
+        mStatus = Status.SAVING;
+
         Activity activity = getActivity();
         // If the activity is not there anymore, then we can't continue with the save process.
         if (activity == null) {
@@ -486,13 +453,29 @@
         }
         Intent saveIntent = null;
         if (mAction == Intent.ACTION_INSERT) {
+            // TODO: Perform similar work to add members at the same time as creating a new group
             saveIntent = ContactSaveService.createNewGroupIntent(activity,
                     new Account(mAccountName, mAccountType), mGroupNameView.getText().toString(),
                     activity.getClass(), GroupEditorActivity.ACTION_SAVE_COMPLETED);
         } else if (mAction == Intent.ACTION_EDIT) {
-            saveIntent = ContactSaveService.createGroupRenameIntent(activity, mGroupId,
-                    mGroupNameView.getText().toString(), activity.getClass(),
-                    GroupEditorActivity.ACTION_SAVE_COMPLETED);
+            // Create array of raw contact IDs for contacts to add to the group
+            int membersToAddCount = mListMembersToAdd.size();
+            long[] membersToAddArray = new long[membersToAddCount];
+            for (int i = 0; i < membersToAddCount; i++) {
+                membersToAddArray[i] = mListMembersToAdd.get(i).getRawContactId();
+            }
+
+            // Create array of raw contact IDs for contacts to add to the group
+            int membersToRemoveCount = mListMembersToRemove.size();
+            long[] membersToRemoveArray = new long[membersToRemoveCount];
+            for (int i = 0; i < membersToRemoveCount; i++) {
+                membersToRemoveArray[i] = mListMembersToRemove.get(i).getRawContactId();
+            }
+
+            // Create the update intent (which includes the updated group name if necessary)
+            saveIntent = ContactSaveService.createGroupUpdateIntent(activity, mGroupId,
+                    getUpdatedName(), membersToAddArray, membersToRemoveArray,
+                    activity.getClass(), GroupEditorActivity.ACTION_SAVE_COMPLETED);
         } else {
             throw new IllegalStateException("Invalid intent action type " + mAction);
         }
@@ -551,9 +534,62 @@
         return !TextUtils.isEmpty(mGroupNameView.getText());
     }
 
-    private boolean hasChanges() {
-        return mGroupNameView.getText() != null &&
-                !mGroupNameView.getText().toString().equals(mOriginalGroupName);
+    private boolean hasNameChange() {
+        return !mGroupNameView.getText().toString().equals(mOriginalGroupName);
+    }
+
+    private boolean hasMembershipChange() {
+        return mListMembersToAdd.size() > 0 || mListMembersToRemove.size() > 0;
+    }
+
+    /**
+     * Returns the group's new name or null if there is no change from the
+     * original name that was loaded for the group.
+     */
+    private String getUpdatedName() {
+        String groupNameFromTextView = mGroupNameView.getText().toString();
+        if (groupNameFromTextView.equals(mOriginalGroupName)) {
+            // No name change, so return null
+            return null;
+        }
+        return groupNameFromTextView;
+    }
+
+    private void addExistingMembers(List<Member> members, List<Long> listContactIds) {
+        mListToDisplay.addAll(members);
+        mMemberListAdapter.notifyDataSetChanged();
+
+        // Update the autocomplete adapter so these contacts don't get suggested
+        mAutoCompleteAdapter.updateExistingMembersList(listContactIds);
+    }
+
+    private void addMember(Member member) {
+        // Update the display list
+        mListMembersToAdd.add(member);
+        mListToDisplay.add(member);
+        mMemberListAdapter.notifyDataSetChanged();
+
+        // Update the autocomplete adapter so the contact doesn't get suggested again
+        mAutoCompleteAdapter.addNewMember(member.getContactId());
+    }
+
+    private void removeMember(Member member) {
+        // If the contact was just added during this session, remove it from the list of
+        // members to add
+        if (mListMembersToAdd.contains(member)) {
+            mListMembersToAdd.remove(member);
+        } else {
+            // Otherwise this contact was already part of the existing list of contacts,
+            // so we need to do a content provider deletion operation
+            mListMembersToRemove.add(member);
+        }
+        // In either case, update the UI so the contact is no longer in the list of
+        // members
+        mListToDisplay.remove(member);
+        mMemberListAdapter.notifyDataSetChanged();
+
+        // Update the autocomplete adapter so the contact can get suggested again
+        mAutoCompleteAdapter.removeMember(member.getContactId());
     }
 
     /**
@@ -571,6 +607,10 @@
         public void onLoadFinished(Loader<Cursor> loader, Cursor data) {
             mStatus = Status.EDITING;
             bindGroupMetaData(data);
+
+            // Load existing members
+            getLoaderManager().initLoader(LOADER_EXISTING_MEMBERS, null,
+                    mGroupMemberListLoaderListener);
         }
 
         @Override
@@ -590,28 +630,27 @@
 
         @Override
         public void onLoadFinished(Loader<Cursor> loader, Cursor data) {
-            List<Member> listMembers = new ArrayList<Member>();
             List<Long> listContactIds = new ArrayList<Long>();
+            List<Member> listExistingMembers = new ArrayList<Member>();
             try {
                 data.moveToPosition(-1);
                 while (data.moveToNext()) {
                     long contactId = data.getLong(GroupMemberLoader.CONTACT_ID_COLUMN_INDEX);
+                    long rawContactId = data.getLong(GroupMemberLoader.RAW_CONTACT_ID_COLUMN_INDEX);
                     String lookupKey = data.getString(
                             GroupMemberLoader.CONTACT_LOOKUP_KEY_COLUMN_INDEX);
                     String displayName = data.getString(
                             GroupMemberLoader.CONTACT_DISPLAY_NAME_PRIMARY_COLUMN_INDEX);
                     String photoUri = data.getString(
                             GroupMemberLoader.CONTACT_PHOTO_URI_COLUMN_INDEX);
-                    listMembers.add(new Member(lookupKey, contactId, displayName, photoUri));
+                    listExistingMembers.add(new Member(rawContactId, lookupKey, contactId,
+                            displayName, photoUri));
                     listContactIds.add(contactId);
                 }
             } finally {
                 data.close();
             }
-            // Update the list of displayed existing members
-            mMemberListAdapter.updateExistingMembersList(listMembers);
 
-            // Setup the group member suggestion adapter
             mAutoCompleteAdapter = new SuggestedMemberListAdapter(getActivity(),
                     android.R.layout.simple_dropdown_item_1line);
             mAutoCompleteAdapter.setContentResolver(mContentResolver);
@@ -622,7 +661,8 @@
                 @Override
                 public void onItemClick(AdapterView<?> parent, View view, int position, long id) {
                     SuggestedMember member = mAutoCompleteAdapter.getItem(position);
-                    loadMemberToAddToGroup(String.valueOf(member.getContactId()));
+                    loadMemberToAddToGroup(member.getRawContactId(),
+                            String.valueOf(member.getContactId()));
 
                     // Update the autocomplete adapter so the contact doesn't get suggested again
                     mAutoCompleteAdapter.addNewMember(member.getContactId());
@@ -632,8 +672,9 @@
                 }
             });
 
-            // Update the autocomplete adapter
-            mAutoCompleteAdapter.updateExistingMembersList(listContactIds);
+            // Update the display list
+            addExistingMembers(listExistingMembers, listContactIds);
+
             // No more updates
             // TODO: move to a runnable
             getLoaderManager().destroyLoader(LOADER_EXISTING_MEMBERS);
@@ -646,15 +687,17 @@
     /**
      * The listener to load a summary of details for a contact.
      */
+    // TODO: Remove this step because showing the aggregate contact can be confusing when the user
+    // just selected a raw contact
     private final LoaderManager.LoaderCallbacks<Cursor> mContactLoaderListener =
             new LoaderCallbacks<Cursor>() {
 
-        private int mMemberAction;
+        private long mRawContactId;
 
         @Override
         public CursorLoader onCreateLoader(int id, Bundle args) {
             String memberId = args.getString(MEMBER_LOOKUP_URI_KEY);
-            mMemberAction = args.getInt(MEMBER_ACTION_KEY);
+            mRawContactId = args.getLong(MEMBER_RAW_CONTACT_ID_KEY);
             return new CursorLoader(mContext, Uri.withAppendedPath(Contacts.CONTENT_URI, memberId),
                     PROJECTION_CONTACT, null, null, null);
         }
@@ -671,7 +714,7 @@
                 String lookupKey = cursor.getString(CONTACT_LOOKUP_KEY_COLUMN_INDEX);
                 String photoUri = cursor.getString(CONTACT_PHOTO_URI_COLUMN_INDEX);
                 getLoaderManager().destroyLoader(LOADER_NEW_GROUP_MEMBER);
-                member = new Member(lookupKey, contactId, displayName, photoUri);
+                member = new Member(mRawContactId, lookupKey, contactId, displayName, photoUri);
             } finally {
                 cursor.close();
             }
@@ -680,26 +723,8 @@
                 return;
             }
 
-            // Don't do anything if the adapter already contains this member
-            // TODO: Come up with a better way to check membership using a DB query
-            if (mMemberListAdapter.contains(member)) {
-                Toast.makeText(getActivity(), getActivity().getString(
-                        R.string.contactAlreadyInGroup), Toast.LENGTH_SHORT).show();
-                return;
-            }
-
             // Otherwise continue adding the member to list of members
-            mMemberListAdapter.addMember(member);
-
-            // Then start loading the full contact so that the change can be saved
-            // TODO: Combine these two loader steps into one. Either we get rid of the first loader
-            // (retrieving summary details) and just use the full contact loader, or find a way
-            // to save changes without loading the full contact
-            Bundle args = new Bundle();
-            args.putString(MEMBER_LOOKUP_URI_KEY, member.getLookupUri().toString());
-            args.putInt(MEMBER_ACTION_KEY, mMemberAction);
-            getLoaderManager().restartLoader(FULL_LOADER_NEW_GROUP_MEMBER, args,
-                    mDataLoaderListener);
+            addMember(member);
         }
 
         @Override
@@ -707,111 +732,33 @@
     };
 
     /**
-     * The listener for the loader that loads the full details of a contact so that when the data
-     * has arrived, the contact can be added or removed from the group.
-     */
-    private final LoaderManager.LoaderCallbacks<ContactLoader.Result> mDataLoaderListener =
-            new LoaderCallbacks<ContactLoader.Result>() {
-
-        private int mMemberAction;
-
-        @Override
-        public Loader<ContactLoader.Result> onCreateLoader(int id, Bundle args) {
-            mMemberAction = args.getInt(MEMBER_ACTION_KEY);
-            String memberLookupUri = args.getString(MEMBER_LOOKUP_URI_KEY);
-            return new ContactLoader(mContext, Uri.parse(memberLookupUri));
-        }
-
-        @Override
-        public void onLoadFinished(Loader<ContactLoader.Result> loader, ContactLoader.Result data) {
-            if (data == ContactLoader.Result.NOT_FOUND || data == ContactLoader.Result.ERROR) {
-                Log.i(TAG, "Contact was not found");
-                return;
-            }
-            saveChange(data, mMemberAction);
-        }
-
-        public void onLoaderReset(Loader<ContactLoader.Result> loader) {
-        }
-    };
-
-    private void saveChange(ContactLoader.Result data, int action) {
-        EntityDeltaList state = EntityDeltaList.fromIterator(data.getEntities().iterator());
-
-        // We need a raw contact to save this group membership change to, so find the first valid
-        // {@link EntityDelta}.
-        // TODO: Find a better way to do this. This will not work if the group is associated with
-        // the other
-        final AccountTypeManager accountTypes = AccountTypeManager.getInstance(mContext);
-        AccountType type = null;
-        EntityDelta entity = null;
-        int size = state.size();
-        for (int i = 0; i < size; i++) {
-            entity = state.get(i);
-            final ValuesDelta values = entity.getValues();
-            if (!values.isVisible()) continue;
-
-            final String accountName = values.getAsString(RawContacts.ACCOUNT_NAME);
-            final String accountType = values.getAsString(RawContacts.ACCOUNT_TYPE);
-            type = accountTypes.getAccountType(accountType);
-            // If the account name and type match this group's properties and the account type is
-            // not an external type, then use this raw contact
-            if (mAccountName.equals(accountName) && mAccountType.equals(accountType) &&
-                    !type.isExternal()) {
-                break;
-            }
-        }
-
-        Intent intent = null;
-        switch (action) {
-            case ADD_MEMBER:
-                DataKind groupMembershipKind = type.getKindForMimetype(
-                        GroupMembership.CONTENT_ITEM_TYPE);
-                ValuesDelta entry = EntityModifier.insertChild(entity, groupMembershipKind);
-                entry.put(GroupMembership.GROUP_ROW_ID, mGroupId);
-                // Form intent
-                intent = ContactSaveService.createSaveContactIntent(getActivity(), state,
-                        SAVE_MODE_EXTRA_KEY, SaveMode.CLOSE, getActivity().getClass(),
-                        GroupEditorActivity.ACTION_ADD_MEMBER_COMPLETED);
-                break;
-            case REMOVE_MEMBER:
-                // TODO: Check that the contact was in the group in the first place
-                ArrayList<ValuesDelta> entries = entity.getMimeEntries(
-                        GroupMembership.CONTENT_ITEM_TYPE);
-                if (entries != null) {
-                    for (ValuesDelta valuesDeltaEntry : entries) {
-                        if (!valuesDeltaEntry.isDelete()) {
-                            Long groupId = valuesDeltaEntry.getAsLong(GroupMembership.GROUP_ROW_ID);
-                            if (groupId == mGroupId) {
-                                valuesDeltaEntry.markDeleted();
-                            }
-                        }
-                    }
-                }
-                intent = ContactSaveService.createSaveContactIntent(getActivity(), state,
-                        SAVE_MODE_EXTRA_KEY, SaveMode.CLOSE, getActivity().getClass(),
-                        GroupEditorActivity.ACTION_REMOVE_MEMBER_COMPLETED);
-                break;
-            default:
-                throw new IllegalStateException("Invalid action for a group member " + action);
-        }
-        getActivity().startService(intent);
-    }
-
-    /**
      * This represents a single member of the current group.
      */
     public static class Member {
+        // TODO: Switch to just dealing with raw contact IDs everywhere if possible
+        private final long mRawContactId;
+        private final long mContactId;
         private final Uri mLookupUri;
         private final String mDisplayName;
         private final Uri mPhotoUri;
 
-        public Member(String lookupKey, long contactId, String displayName, String photoUri) {
+        public Member(long rawContactId, String lookupKey, long contactId, String displayName,
+                String photoUri) {
+            mRawContactId = rawContactId;
+            mContactId = contactId;
             mLookupUri = Contacts.getLookupUri(contactId, lookupKey);
             mDisplayName = displayName;
             mPhotoUri = (photoUri != null) ? Uri.parse(photoUri) : null;
         }
 
+        public long getRawContactId() {
+            return mRawContactId;
+        }
+
+        public long getContactId() {
+            return mContactId;
+        }
+
         public Uri getLookupUri() {
             return mLookupUri;
         }
@@ -839,34 +786,6 @@
      */
     private final class MemberListAdapter extends BaseAdapter {
 
-        private List<Member> mNewMembersList = new ArrayList<Member>();
-        private List<Member> mTotalList = new ArrayList<Member>();
-
-        public boolean contains(Member member) {
-            return mTotalList.contains(member);
-        }
-
-        public void addMember(Member member) {
-            mNewMembersList.add(member);
-            mTotalList.add(member);
-            notifyDataSetChanged();
-        }
-
-        public void removeMember(Member member) {
-            if (mNewMembersList.contains(member)) {
-                mNewMembersList.remove(member);
-            }
-            mTotalList.remove(member);
-            notifyDataSetChanged();
-        }
-
-        public void updateExistingMembersList(List<Member> existingMembers) {
-            mTotalList.clear();
-            mTotalList.addAll(mNewMembersList);
-            mTotalList.addAll(existingMembers);
-            notifyDataSetChanged();
-        }
-
         @Override
         public View getView(int position, View convertView, ViewGroup parent) {
             View result;
@@ -887,14 +806,7 @@
             deleteButton.setOnClickListener(new OnClickListener() {
                 @Override
                 public void onClick(View v) {
-                    loadMemberToRemoveFromGroup(member.getLookupUri().toString());
-                    // TODO: This is a hack to save the reference to the member that should be
-                    // removed. This won't work if the user tries to remove multiple times in a row
-                    // and reference is outdated. We actually need a hash map of member URIs to the
-                    // actual Member object. Before dealing with hash map though, hopefully we can
-                    // figure out how to batch save membership changes, which would eliminate the
-                    // need for this variable.
-                    mMemberToRemove = member;
+                    removeMember(member);
                 }
             });
 
@@ -904,37 +816,17 @@
 
         @Override
         public int getCount() {
-            return mTotalList.size();
+            return mListToDisplay.size();
         }
 
         @Override
         public Member getItem(int position) {
-            return mTotalList.get(position);
-        }
-
-        @Override
-        public int getItemViewType(int position) {
-            return 0;
-        }
-
-        @Override
-        public int getViewTypeCount() {
-            return 1;
+            return mListToDisplay.get(position);
         }
 
         @Override
         public long getItemId(int position) {
-            return -1;
-        }
-
-        @Override
-        public boolean areAllItemsEnabled() {
-            return false;
-        }
-
-        @Override
-        public boolean isEnabled(int position) {
-            return false;
+            return position;
         }
     }
 }
diff --git a/src/com/android/contacts/group/SuggestedMemberListAdapter.java b/src/com/android/contacts/group/SuggestedMemberListAdapter.java
index dd74df5..5abb5fb 100644
--- a/src/com/android/contacts/group/SuggestedMemberListAdapter.java
+++ b/src/com/android/contacts/group/SuggestedMemberListAdapter.java
@@ -26,7 +26,6 @@
 import android.provider.ContactsContract.CommonDataKinds.Email;
 import android.provider.ContactsContract.CommonDataKinds.Phone;
 import android.provider.ContactsContract.CommonDataKinds.Photo;
-import android.provider.ContactsContract.Contacts;
 import android.provider.ContactsContract.Contacts.Data;
 import android.provider.ContactsContract.RawContacts;
 import android.provider.ContactsContract.RawContactsEntity;
@@ -80,6 +79,8 @@
     private String mAccountType = "";
     private String mAccountName = "";
 
+    // TODO: Make this a Map for better performance when we check if a new contact is in the list
+    // or not
     private List<Long> mExistingMemberContactIds = new ArrayList<Long>();
 
     private static final int SUGGESTIONS_LIMIT = 5;
@@ -186,6 +187,7 @@
             try {
                 cursor.moveToPosition(-1);
                 while (cursor.moveToNext() && suggestionsMap.keySet().size() < SUGGESTIONS_LIMIT) {
+                    long rawContactId = cursor.getLong(RAW_CONTACT_ID_COLUMN_INDEX);
                     long contactId = cursor.getLong(CONTACT_ID_COLUMN_INDEX);
                     // Filter out contacts that have already been added to this group
                     if (mExistingMemberContactIds.contains(contactId)) {
@@ -193,8 +195,8 @@
                     }
                     // Otherwise, add the contact as a suggested new group member
                     String displayName = cursor.getString(DISPLAY_NAME_PRIMARY_COLUMN_INDEX);
-                    long rawContactId = cursor.getLong(RAW_CONTACT_ID_COLUMN_INDEX);
-                    suggestionsMap.put(rawContactId, new SuggestedMember(displayName, contactId));
+                    suggestionsMap.put(rawContactId, new SuggestedMember(rawContactId,
+                            displayName, contactId));
                 }
             } finally {
                 cursor.close();
@@ -285,14 +287,18 @@
     /**
      * This represents a single contact that is a suggestion for the user to add to a group.
      */
+    // TODO: Merge this with the {@link GroupEditorFragment} Member class once we can find the
+    // lookup URI for this contact using the autocomplete filter queries
     public class SuggestedMember {
 
+        private long mRawContactId;
         private long mContactId;
         private String mDisplayName;
         private String mExtraInfo;
         private byte[] mPhoto;
 
-        public SuggestedMember(String displayName, long contactId) {
+        public SuggestedMember(long rawContactId, String displayName, long contactId) {
+            mRawContactId = rawContactId;
             mDisplayName = displayName;
             mContactId = contactId;
         }
@@ -305,6 +311,10 @@
             return mExtraInfo;
         }
 
+        public long getRawContactId() {
+            return mRawContactId;
+        }
+
         public long getContactId() {
             return mContactId;
         }