The UI text should be clearer for what happens to status, roles, and mapped fields of users who are updated. It should also say "Update Existing Users" instead of "Replace existing users". Replace implies new accounts with new passwords, $account->data, etc. Update is what is actually happening for existing users. Below is what the patch changes it to:

[X] Update existing users
Existing users will be determined using mappings that are a "unique target". Updated content will behave as follows:

  • Only mapped fields will replace existing user data.
  • "Status" of existing users will not be changed. Only new users will follow "Status" selection below.
  • "Additional roles" will be granted, but existing users roles will not be removed.

To make additional roles behave this way, I added some roles array merging.
I will attach a patch in next comment.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

johnbarclay’s picture

Patch from previous comment. Also its unclear if merging configured drupal roles is the desired behavior. If it isn't, I can take out the drupal role merge and alter the UI text to reflect that.

johnbarclay’s picture

left the 'update_existing' default value out. Here is the fixed patch.

febbraro’s picture

Status: Active » Needs work

I think the patch looks good at first glance, but I would like a unit test to accompany it. I fixed all of the unit tests yesterday so let me know if you can include one in the patch. It will make it quicker for me to review and accept it.

I do realize I am commenting 2 months later :(

dooug’s picture

I tested this patch and it applied cleanly on the latest 7.x-2.x-dev and worked for a simple test case of importing users with roles.

I was going to attempt to learn/write a simpletest unit test but got stuck in a mire of getting simpletest to work on my setup:
(If you want to help me there, that'd be appreciated: #1678514: Unable to use simpletest on a D7 site setup with Aegir due to PHP open_basedir restriction).

This is also related to an issue that adds a mapping target for the user role: #1376774: User Processor - add mapping target for user roles

twistor’s picture

The idea for this patch is right, but adding default roles back onto existing users is behavior that doesn't exist anywhere else in Feeds.

Also, FeedsProcessor provides the update_existing form element already. It would be better to try and update the description there.

wizonesolutions’s picture

Issue summary: View changes

It seems like user language doesn't get updated either.

MegaChriz’s picture

An additional description of what happens when replacing existing users would be useful. Now that #1376774: User Processor - add mapping target for user roles has been committed, additional roles are also added to users that are being updated. When replacing users, all unchecked roles will be revoked.