Move create and update logging into the save method + log invitations…

… created via bulk upload + log changes to tableau related fields + minor changes

corehq/apps/api/resources/v1_0.py

@@ -22,7 +22,6 @@
     Invitation,
 )
 from corehq.apps.users.model_log import InviteModelAction
-from corehq.apps.users.util import log_invitation_change
 from corehq.apps.users.views import InviteWebUserView
 from corehq.apps.reports.util import (
     get_tableau_group_ids_by_names,
@@ -143,31 +142,30 @@ def obj_create(self, bundle, **kwargs):
             'custom_user_data': spec.new_or_existing_user_data or {},
             'invited_by': bundle.request.couch_user.user_id,
             'invited_on': datetime.utcnow(),
+            'tableau_role': spec.tableau_role,
+            'tableau_group_ids': tableau_group_ids,
         }
         invite_params = {
             'role': role_id,
             'primary_location_id': primary_loc_id,
             'profile': profile,
-            'tableau_role': spec.tableau_role,
-            'tableau_group_ids': tableau_group_ids,
         }
         invite_params.update(initial_fields)
         invite = Invitation.objects.create(**invite_params)
         invite.assigned_locations.set(assigned_locs)
 
         # Log invite creation
-        primary_loc = SQLLocation.active_objects.get(location_id=primary_loc_id)
-        user = CouchUser.get_by_username(spec.email.lower())
-        changes = InviteWebUserView.format_changes(domain, spec.role, profile, assigned_locs, primary_loc, None)
+        primary_loc = None
+        if primary_loc_id:
+            primary_loc = SQLLocation.objects.get(location_id=primary_loc_id)
+        changes = InviteWebUserView.format_changes(domain,
+                                                   {'role_name': spec.role,
+                                                    'profile': profile,
+                                                    'assigned_locations': assigned_locs,
+                                                    'primary_location': primary_loc})
         changes.update(initial_fields)
-        log_invitation_change(
-            domain=domain,
-            changed_by=bundle.request.couch_user.user_id,
-            changed_via=INVITATION_CHANGE_VIA_API,
-            action=InviteModelAction.CREATE,
-            invite=invite,
-            user_id=user.user_id if user else None,
-            changes=changes
-        )
+        invite.save(logging_values={"changed_by": bundle.request.couch_user.user_id,
+                                    "changed_via": INVITATION_CHANGE_VIA_API,
+                                    "action": InviteModelAction.CREATE, "changes": changes})
         bundle.obj = invite
         return bundle

corehq/apps/user_importer/importer.py

@@ -50,7 +50,8 @@
     UserRole,
     InvitationStatus
 )
-from corehq.const import USER_CHANGE_VIA_BULK_IMPORTER
+from corehq.apps.users.model_log import InviteModelAction
+from corehq.const import USER_CHANGE_VIA_BULK_IMPORTER, INVITATION_CHANGE_VIA_BULK_IMPORTER
 from corehq.toggles import DOMAIN_PERMISSIONS_MIRROR, TABLEAU_USER_SYNCING
 from corehq.apps.sms.util import validate_phone_number
 
@@ -328,29 +329,49 @@ def get_location_from_site_code(site_code, location_cache):
 def create_or_update_web_user_invite(email, domain, role_qualified_id, upload_user, primary_location_id=None,
                                     assigned_location_ids=None, profile=None, tableau_role=None,
                                     tableau_group_ids=None, user_change_logger=None, send_email=True):
+    from corehq.apps.users.views import InviteWebUserView
+
     if assigned_location_ids is None:
         assigned_location_ids = []
+    primary_location = SQLLocation.by_location_id(primary_location_id)
+    invite_fields = {
+        'invited_by': upload_user.user_id,
+        'invited_on': datetime.utcnow(),
+        'tableau_role': tableau_role,
+        'tableau_group_ids': tableau_group_ids,
+        'primary_location': primary_location,
+        'role': role_qualified_id,
+        'profile': profile,
+    }
     invite, invite_created = Invitation.objects.update_or_create(
         email=email,
         domain=domain,
         is_accepted=False,
-        defaults={
-            'invited_by': upload_user.user_id,
-            'invited_on': datetime.utcnow(),
-            'tableau_role': tableau_role,
-            'tableau_group_ids': tableau_group_ids,
-            'primary_location': SQLLocation.by_location_id(primary_location_id),
-            'role': role_qualified_id,
-            'profile': profile,
-        },
+        defaults=invite_fields,
     )
     assigned_locations = [SQLLocation.by_location_id(assigned_location_id)
-            for assigned_location_id in assigned_location_ids]
+                          for assigned_location_id in assigned_location_ids]
     invite.assigned_locations.set(assigned_locations)
-    if invite_created and send_email:
-        invite.send_activation_email()
-    if invite_created and user_change_logger:
-        user_change_logger.add_info(UserChangeMessage.invited_to_domain(domain))
+    changes = InviteWebUserView.format_changes(domain,
+                                               {'role_name': role_qualified_id,
+                                                'profile': profile,
+                                                'assigned_locations': assigned_locations,
+                                                'primary_location': primary_location})
+    if invite_created:
+        if send_email:
+            invite.send_activation_email()
+        if user_change_logger:
+            user_change_logger.add_info(UserChangeMessage.invited_to_domain(domain))
+        action = InviteModelAction.CREATE
+        invite_fields.update({'domain': domain, 'email': email})
+        invite_fields.pop('primary_location')
+        invite_fields.pop('role')
+        invite_fields.pop('profile')
+        changes.update(invite_fields)
+    else:
+        action = InviteModelAction.UPDATE
+    invite.save(logging_values={"changed_by": upload_user.user_id, "action": action,
+                                "changed_via": INVITATION_CHANGE_VIA_BULK_IMPORTER, "changes": changes})
 
 
 def find_location_id(location_codes, location_cache):

corehq/apps/users/models.py

@@ -85,7 +85,6 @@
     username_to_user_id,
     bulk_auto_deactivate_commcare_users,
     is_dimagi_email,
-    SYSTEM_USER_ID,
 )
 from corehq.const import INVITATION_CHANGE_VIA_WEB
 from corehq.form_processor.exceptions import CaseNotFound
@@ -2767,16 +2766,24 @@ def __repr__(self):
 
     def save(self, *args, **kwargs):
         self.full_clean()
+        logging_values = kwargs.pop('logging_values', None)
         super().save(*args, **kwargs)
+        if logging_values:
+            user = self.get_invited_user()
+            log_invitation_change(
+                domain=self.domain,
+                changed_by=logging_values.get('changed_by'),
+                changed_via=logging_values.get('changed_via'),
+                action=logging_values.get('action'),
+                invite=self,
+                user_id=user.user_id if user else None,
+                changes=logging_values.get('changes'),
+            )
 
     def delete(self, **kwargs):
         from corehq.apps.users.model_log import InviteModelAction
-        user = CouchUser.get_by_username(self.email)
+        user = self.get_invited_user()
         deleted_by = kwargs.get('deleted_by')
-        if not deleted_by and not settings.UNIT_TESTING:
-            raise ValueError("Missing deleted_by")
-        elif not deleted_by and settings.UNIT_TESTING:
-            deleted_by = SYSTEM_USER_ID
         log_invitation_change(
             domain=self.domain,
             user_id=user.user_id if user else None,
@@ -2834,6 +2841,9 @@ def send_activation_email(self, remaining_days=30):
                                     domain=self.domain,
                                     use_domain_gateway=True)
 
+    def get_invited_user(self):
+        return CouchUser.get_by_username(self.email)
+
     def get_role_name(self):
         if self.role:
             if self.role == 'admin':

corehq/apps/users/util.py

@@ -419,6 +419,15 @@ def log_invitation_change(domain, changed_by, changed_via, action, invite=None,
     :param changes: dict of all changes. Combines the "changes" and "change_message" field present in UserHistory
     """
     from corehq.apps.users.models import InvitationHistory
+
+    if not changed_by and not settings.UNIT_TESTING:
+        raise ValueError("Missing changed_by")
+    elif not changed_by and settings.UNIT_TESTING:
+        changed_by = SYSTEM_USER_ID
+
+    if not changed_via and settings.UNIT_TESTING:
+        changed_via = "TEST"
+
     return InvitationHistory.objects.create(
         domain=domain,
         user_id=user_id,

corehq/apps/users/views/__init__.py

@@ -116,7 +116,7 @@
     UserRole,
 )
 from corehq.apps.users.model_log import InviteModelAction
-from corehq.apps.users.util import log_user_change, log_invitation_change
+from corehq.apps.users.util import log_user_change
 from corehq.apps.users.views.utils import (
     filter_user_query_by_locations_accessible_to_user,
     get_editable_role_choices, BulkUploadResponseWrapper,
@@ -1250,27 +1250,18 @@ def post(self, request, *args, **kwargs):
             if invitation:
                 create_invitation = False
                 invitation, changed_values = self._get_and_set_changes(invitation, data, profile)
-                changes = self.format_changes(*changed_values)
+                changes = self.format_changes(self.domain, changed_values)
                 user_data = data.get("custom_user_data", {})
                 changed_user_data = {}
                 for key, value in invitation.custom_user_data.items():
                     if key in user_data and user_data[key] != value:
                         changed_user_data[key] = user_data[key]
                 changes.update({"custom_user_data": changed_user_data})
                 invitation.custom_user_data = user_data
-                invitation.tableau_role = data.get("tableau_role", None)
-                invitation.tableau_group_ids = data.get("tableau_group_ids", None)
-                invitation.save()
+                invitation.save(logging_values={"changed_by": request.couch_user.user_id,
+                                                "changed_via": INVITATION_CHANGE_VIA_WEB,
+                                                "action": InviteModelAction.UPDATE, "changes": changes})
                 messages.success(request, "Invite to %s was successfully updated." % data["email"])
-                log_invitation_change(
-                    domain=self.domain,
-                    changed_by=request.couch_user.user_id,
-                    changed_via=INVITATION_CHANGE_VIA_WEB,
-                    action=InviteModelAction.UPDATE,
-                    invite=invitation,
-                    user_id=user.user_id if user else None,
-                    changes=changes
-                )
             elif domain_request is not None:
                 domain_request.is_approved = True
                 domain_request.save()
@@ -1302,25 +1293,22 @@ def post(self, request, *args, **kwargs):
                 data["primary_location"], assigned_locations = self._get_sql_locations(
                     data.pop("primary_location", None), data.pop("assigned_locations", []))
                 invite = Invitation(**data)
-                invite.save()
-                invite.assigned_locations.set(assigned_locations)
-                invite.send_activation_email()
-                changes = self.format_changes(self.domain, data.get("role"), profile, assigned_locations,
-                                              data["primary_location"], data.get("program", None))
+                changes = self.format_changes(self.domain,
+                                              {'role_name': data.get("role"),
+                                               'profile': profile,
+                                               'assigned_locations': assigned_locations,
+                                               'primary_location': data["primary_location"],
+                                               'program_id': data.get("program", None)})
                 for key in changes:
                     if key in data:
                         data.pop(key, None)
                 data.pop("primary_location", None)
                 changes.update(data)
-                log_invitation_change(
-                    domain=self.domain,
-                    user_id=user.user_id if user else None,
-                    changed_by=request.couch_user.user_id,
-                    changed_via=INVITATION_CHANGE_VIA_WEB,
-                    action=InviteModelAction.CREATE,
-                    invite=invite,
-                    changes=changes
-                )
+                invite.save(logging_values={"changed_by": request.couch_user.user_id,
+                                            "changed_via": INVITATION_CHANGE_VIA_WEB,
+                                            "action": InviteModelAction.CREATE, "changes": changes})
+                invite.assigned_locations.set(assigned_locations)
+                invite.send_activation_email()
 
             # Ensure trust is established with Invited User's Identity Provider
             if not IdentityProvider.does_domain_trust_user(self.domain, data["email"]):
@@ -1344,34 +1332,42 @@ def _get_sql_locations(self, primary_location_id, assigned_location_ids):
         return primary_location, assigned_locations
 
     def _get_and_set_changes(self, invite, form_data, profile):
-        change_values = [None] * 5
+        change_values = {}
         role = form_data.get("role")
         if invite.role != role:
-            change_values[0] = role
+            change_values['role_name'] = role
             invite.role = role
         if invite.profile != profile:
-            change_values[1] = profile
+            change_values['profile'] = profile
             invite.profile = profile
         primary_location, assigned_locations = self._get_sql_locations(
             form_data.pop("primary_location", None), form_data.pop("assigned_locations", []))
         previous_locations = [loc for loc in invite.assigned_locations.all()]
         if len(assigned_locations) != len(previous_locations) \
            or set(assigned_locations) != set(previous_locations):
-            change_values[2] = assigned_locations
+            change_values['assigned_locations'] = assigned_locations
             invite.assigned_locations.set(assigned_locations)
         if invite.primary_location != primary_location:
-            change_values[3] = primary_location
+            change_values['primary_location'] = primary_location
             invite.primary_location = primary_location
         if invite.program != form_data.get("program", None):
             program = form_data.get("program", None)
-            change_values[4] = program
+            change_values['program_id'] = program
             invite.program = program
+        if invite.tableau_role != form_data.get("tableau_role", None):
+            tableau_role = form_data.get("program", None)
+            change_values['tableau_role'] = tableau_role
+            invite.tableau_role = tableau_role
+        if invite.tableau_group_ids != form_data.get("tableau_group_ids", None):
+            tableau_group_ids = form_data.get("tableau_group_ids", None)
+            change_values['tableau_group_ids'] = tableau_group_ids
+            invite.program = tableau_group_ids
 
-        return invite, [self.domain] + change_values
+        return invite, change_values
 
     @staticmethod
-    def format_changes(domain, role_name, profile, assigned_locations, primary_location, program_id):
-        changes = {}
+    def format_changes(domain, changed_values):
+        role_name = changed_values.pop("role_name", None)
         if role_name:
             if role_name == "admin":
                 role = StaticRole.domain_admin(domain)
@@ -1381,17 +1377,21 @@ def format_changes(domain, role_name, profile, assigned_locations, primary_locat
                 except UserRole.DoesNotExist:
                     role = None
             if role:
-                changes.update(UserChangeMessage.role_change(role))
+                changed_values.update(UserChangeMessage.role_change(role))
+        profile = changed_values.pop('profile', None)
         if profile:
-            changes.update(UserChangeMessage.profile_info(profile.id, profile.name))
+            changed_values.update(UserChangeMessage.profile_info(profile.id, profile.name))
+        program_id = changed_values.pop('program_id', None)
         if program_id:
-            changes.update(UserChangeMessage.program_change(Program.get(program_id)))
+            changed_values.update(UserChangeMessage.program_change(Program.get(program_id)))
+        assigned_locations = changed_values.pop('assigned_locations', None)
         if assigned_locations:
-            changes.update(UserChangeMessage.assigned_locations_info(assigned_locations))
+            changed_values.update(UserChangeMessage.assigned_locations_info(assigned_locations))
+        primary_location = changed_values.pop('primary_location', None)
         if primary_location:
-            changes.update(UserChangeMessage.primary_location_info(primary_location))
+            changed_values.update(UserChangeMessage.primary_location_info(primary_location))
 
-        return changes
+        return changed_values
 
 
 class BaseUploadUser(BaseUserSettingsView):

corehq/const.py

@@ -40,5 +40,6 @@
 
 INVITATION_CHANGE_VIA_WEB = "invite_web"
 INVITATION_CHANGE_VIA_API = "invite_api"
+INVITATION_CHANGE_VIA_BULK_IMPORTER = "invite_bulk_importer"
 
 LOADTEST_HARD_LIMIT = 500_000  # max cases a loadtest user can sync