[#2374] Minor code clean-up

akvo · Nov 24, 2016 · e2ce4f5 · e2ce4f5
1 parent a938cf5
commit e2ce4f5
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 47 deletions.
diff --git a/akvo/rest/views/user.py b/akvo/rest/views/user.py
@@ -100,7 +100,7 @@ def request_organisation(request, pk=None):
     # request.user is the user identified by the auth token
     user = request.user
     # Users themselves are only allowed to request to join an organisation
-    if not user_by_pk == request.user:
+    if not user_by_pk == user:
         raise PermissionDenied()
     request.DATA['user'] = pk
 
@@ -109,14 +109,13 @@ def request_organisation(request, pk=None):
     if serializer.is_valid():
         try:
             organisation = Organisation.objects.get(pk=serializer.data['organisation'])
-            employment = Employment(
+            employment = Employment.objects.create(
                 user=user,
                 organisation=organisation,
                 country=serializer.data['country'],
                 job_title=serializer.data['job_title'],
                 is_approved=False,
             )
-            employment.save()
         except IntegrityError:
             return Response({'detail': _(u'User already linked to this organisation')},
                             status=status.HTTP_409_CONFLICT)

diff --git a/akvo/rsr/signals.py b/akvo/rsr/signals.py
@@ -236,50 +236,52 @@ def employment_post_save(sender, **kwargs):
     admins_group = Group.objects.get(name='Admins')
     employment = kwargs.get("instance", None)
 
-    if employment:
-        user = employment.user
-
-        # Set user to staff when in a certain group
-        if (employment.group in [project_editors_group, user_managers_group, admins_group] and
-                employment.is_approved) or user.is_superuser or user.is_admin:
-            user.is_staff = True
-            user.save()
-
-        # Send an 'Organisation request' mail when an employment has been newly created, the
-        # user is active and the employment has not been approved yet.
-        if kwargs['created'] and user.is_active and not employment.is_approved:
-            organisation = employment.organisation
-
-            # Retrieve all active support users
-            active_support_users = get_user_model().objects.filter(is_active=True, is_support=True)
-
-            # General (support) admins will always be informed
-            admin_users = active_support_users.filter(is_admin=True)
-
-            # As well as organisation (+ content owners) User managers and Admins
-            employer_users = active_support_users.filter(
-                employers__organisation__in=organisation.content_owned_by(),
-                employers__group__in=[user_managers_group, admins_group]
-            )
-
-            notify = active_support_users.filter(
-                Q(pk__in=admin_users.values_list('pk', flat=True)) |
-                Q(pk__in=employer_users.values_list('pk', flat=True))
-            ).exclude(pk=user.pk).distinct()
-
-            rsr_send_mail_to_users(
-                notify,
-                subject='registration/user_organisation_request_subject.txt',
-                message='registration/user_organisation_request_message.txt',
-                subject_context={
-                    'user': user,
-                    'organisation': organisation
-                },
-                msg_context={
-                    'user': user,
-                    'organisation': organisation
-                },
-            )
+    if not employment:
+        return
+
+    user = employment.user
+
+    # Set user to staff when in a certain group
+    if (employment.group in [project_editors_group, user_managers_group, admins_group] and
+            employment.is_approved) or user.is_superuser or user.is_admin:
+        user.is_staff = True
+        user.save()
+
+    # Send an 'Organisation request' mail when an employment has been newly created, the
+    # user is active and the employment has not been approved yet.
+    if kwargs['created'] and user.is_active and not employment.is_approved:
+        organisation = employment.organisation
+
+        # Retrieve all active support users
+        active_support_users = get_user_model().objects.filter(is_active=True, is_support=True)
+
+        # General (support) admins will always be informed
+        admin_users = active_support_users.filter(is_admin=True)
+
+        # As well as organisation (+ content owners) User managers and Admins
+        employer_users = active_support_users.filter(
+            employers__organisation__in=organisation.content_owned_by(),
+            employers__group__in=[user_managers_group, admins_group]
+        )
+
+        notify = active_support_users.filter(
+            Q(pk__in=admin_users.values_list('pk', flat=True)) |
+            Q(pk__in=employer_users.values_list('pk', flat=True))
+        ).exclude(pk=user.pk).distinct()
+
+        rsr_send_mail_to_users(
+            notify,
+            subject='registration/user_organisation_request_subject.txt',
+            message='registration/user_organisation_request_message.txt',
+            subject_context={
+                'user': user,
+                'organisation': organisation
+            },
+            msg_context={
+                'user': user,
+                'organisation': organisation
+            },
+        )
 
 
 def update_project_budget(sender, **kwargs):