Fix #268 and fix #269: bugs/enhancements related to --strategy push

* Move all push-based group removals to come before group adds, and update the tests to expect that order.
* Make sure we do secondary group adds when creating while syncing; so now it works the same for primaries and secondaries.

Author: adobeDan

tests/rules_test.py

@@ -155,21 +155,21 @@ def mock_load_users_and_groups(groups=None, extended_attributes=None, all_users=
         user = everybody[0]
         commands = tests.helper.create_umapi_commands(user)
         commands.add_user(self.create_user_attributes_for_commands(user, rule_options['update_user_info']))
-        commands.add_groups(set([primary_group_11]))
         commands.remove_groups(set([primary_group_21]))
+        commands.add_groups(set([primary_group_11]))
         expected_primary_commands_list.append(commands)
 
         user = everybody[1]
         commands = tests.helper.create_umapi_commands(user)
         commands.add_user(self.create_user_attributes_for_commands(user, rule_options['update_user_info']))
-        commands.add_groups(set([primary_group_21]))
         commands.remove_groups(set([primary_group_11]))
+        commands.add_groups(set([primary_group_21]))
         expected_primary_commands_list.append(commands)
 
         user = everybody[2]
         commands = tests.helper.create_umapi_commands(user)
-        commands.add_user(self.create_user_attributes_for_commands(user, rule_options['update_user_info']))
         commands.remove_groups(set([primary_group_11, primary_group_21]))
+        commands.add_user(self.create_user_attributes_for_commands(user, rule_options['update_user_info']))
         expected_primary_commands_list.append(commands)
 
         expected_secondary_commands_list = []

user_sync/rules.py

@@ -647,10 +647,10 @@ def add_umapi_user(self, user_key, groups_to_add, umapi_connectors):
         self.action_summary['adobe_users_created'] += 1
         primary_commands.add_user(attributes)
         if manage_groups:
-            primary_commands.add_groups(groups_to_add)
             if doing_push:
                 groups_to_remove = self.get_umapi_info(PRIMARY_UMAPI_NAME).get_mapped_groups() - groups_to_add
                 primary_commands.remove_groups(groups_to_remove)
+            primary_commands.add_groups(groups_to_add)
         umapi_connectors.get_primary_connector().send_commands(primary_commands)
         # add the user to secondaries, maybe with groups
         attributes['option'] = 'ignoreIfAlreadyExists'  # can only update in the owning org
@@ -662,10 +662,11 @@ def add_umapi_user(self, user_key, groups_to_add, umapi_connectors):
                 self.logger.info('Adding directory user to %s with user key: %s', umapi_name, user_key)
                 secondary_commands = self.create_commands_from_directory_user(directory_user, identity_type)
                 secondary_commands.add_user(attributes)
-                if manage_groups and doing_push:
+                if manage_groups:
+                    if doing_push:
+                        groups_to_remove = secondary_umapi_info.get_mapped_groups() - groups_to_add
+                        secondary_commands.remove_groups(groups_to_remove)
                     secondary_commands.add_groups(groups_to_add)
-                    groups_to_remove = secondary_umapi_info.get_mapped_groups() - groups_to_add
-                    secondary_commands.remove_groups(groups_to_remove)
                 umapi_connector.send_commands(secondary_commands)
 
     def update_umapi_user(self, umapi_info, user_key, umapi_connector,