Skip to content

Commit

Permalink
[#2374] Set employment group default in pre_save not post_save
Browse files Browse the repository at this point in the history
Employments are forced to have a non-null group, though the model does
allow null values, by trying to set a default group in the post_save.
But, this can fail if such an employment already exists, thus creating
an employment with a null group.  Multiple such employments with a null
group can get created, too.

This commit moves the setting of the group to the pre_save signal, thus
preventing an invalid employment from getting created.

Closes #2374, Closes #2122
  • Loading branch information
punchagan committed Nov 23, 2016
1 parent a86aa4b commit b5b256f
Showing 1 changed file with 8 additions and 7 deletions.
15 changes: 8 additions & 7 deletions akvo/rsr/signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,15 +169,23 @@ def act_on_log_entry(sender, **kwargs):
def employment_pre_save(sender, **kwargs):
"""
This signal intends to send a mail to the user when his/her account has been approved.
This signal also sets 'Users' Group for the employment if no group has been set
A mail will be sent when:
- A new employment is created with is_approved = True.
* We assume this happens when an existing user is invited for a new organisation.
- An existing employment is updated from is_approved = False changed to True.
* We assume this happens when an existing user has requested to join an organisation himself.
"""
# FIXME: The actual save may fail. Why are emails being sent pre_save?!
employment = kwargs.get("instance", None)

# Set the group to 'Users' when no group has been specified
if not employment.group:
employment.group = Group.objects.get(name='Users')

try:
obj = sender.objects.get(pk=employment.pk)
except sender.DoesNotExist:
Expand Down Expand Up @@ -218,13 +226,11 @@ def employment_post_save(sender, **kwargs):
- Set User to is_staff (for admin access) when the employment is approved and the Group is set
to 'Project Editors', 'User managers' or 'Admins', or when the user is a superuser or general
admin.
- Set 'Users' Group for the employment if no group has been set
If a new employment is created for an active user of which the employment is not approved yet:
- Inform RSR support users, organisation admins and organisation user managers of the request
"""
# Retrieve all user groups and the employment
users_group = Group.objects.get(name='Users')
project_editors_group = Group.objects.get(name='Project Editors')
user_managers_group = Group.objects.get(name='User Managers')
admins_group = Group.objects.get(name='Admins')
Expand All @@ -239,11 +245,6 @@ def employment_post_save(sender, **kwargs):
user.is_staff = True
user.save()

# Set the group to 'Users' when no group has been specified
if not employment.group:
employment.group = users_group
employment.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:
Expand Down

0 comments on commit b5b256f

Please sign in to comment.