Skip to content

Commit

Permalink
Merge pull request #304 from fasrc/cp_usesignals
Browse files Browse the repository at this point in the history 
Use signals
  • Loading branch information
@claire-peters
claire-peters authored Sep 18, 2024
2 parents 616d0ce + d683899 commit faa494f
Show file tree
Hide file tree
Showing 6 changed files with 177 additions and 109 deletions.
15 changes: 10 additions & 5 deletions coldfront/core/project/signals.py
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
import django.dispatch


project_create = django.dispatch.Signal()
#providing_args=["project_title"]
project_post_create = django.dispatch.Signal()
#providing_args=["project_obj"]

project_make_projectuser = django.dispatch.Signal()
#providing_args=["user_name", "group_name"]

project_preremove_projectuser = django.dispatch.Signal()
#providing_args=["user_name", "group_name"]

project_activate_user = django.dispatch.Signal()
#providing_args=["project_user_pk"]
project_remove_user = django.dispatch.Signal()
#providing_args=["project_user_pk"]
project_filter_users_to_remove = django.dispatch.Signal()
#providing_args=["project_user_list"]
# return tuple of (no_removal, can_remove)
56 changes: 52 additions & 4 deletions coldfront/core/project/tests/test_views.py
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import logging

from django.test import TestCase, tag
from django.test import TestCase, tag, override_settings
from django.urls import reverse
from unittest.mock import patch

from coldfront.core.test_helpers import utils
from coldfront.core.test_helpers.factories import (
Expand All @@ -12,7 +13,11 @@
ProjectStatusChoiceFactory,
ProjectAttributeTypeFactory,
)
from coldfront.core.project.models import Project, ProjectUserStatusChoice
from coldfront.core.project.models import (
Project, ProjectUser,
ProjectUserRoleChoice,
ProjectUserStatusChoice
)

logging.disable(logging.CRITICAL)

Expand Down Expand Up @@ -161,7 +166,6 @@ def test_project_attribute_create_post_required_values(self):

def test_project_attribute_create_value_type_match(self):
"""ProjectAttributeCreate correctly flags value-type mismatch"""

self.client.force_login(self.admin_user,
backend='django.contrib.auth.backends.ModelBackend')
# test that value must be numeric if proj_attr_type is string
Expand Down Expand Up @@ -351,7 +355,7 @@ def test_projectnotecreateview_access(self):
self.project_access_tstbase(self.url)


class ProjectAddUsersSearchView(ProjectViewTestBase):
class ProjectAddUsersSearchViewTest(ProjectViewTestBase):
"""Tests for ProjectAddUsersSearchView"""
def setUp(self):
"""set up users and project for testing"""
Expand All @@ -365,6 +369,50 @@ def test_projectadduserssearchview_access(self):
utils.test_user_cannot_access(self, self.proj_datamanager, self.url)# data manager cannot access
utils.test_user_cannot_access(self, self.proj_allocation_user, self.url)# user cannot access

class ProjectAddUsersViewTest(ProjectViewTestBase):
"""Tests for ProjectAddUsersView"""
def setUp(self):
"""set up users and project for testing"""
self.url = reverse('project-add-users', kwargs={'pk': self.project.pk})
self.form_data = {
'q': self.nonproj_allocation_user.username,
'search_by': 'username_only',
'userform-TOTAL_FORMS': '1',
'userform-INITIAL_FORMS': '0',
'userform-MIN_NUM_FORMS': '0',
'userform-MAX_NUM_FORMS': '1',
'userform-0-selected': 'on',
'userform-0-role': ProjectUserRoleChoice.objects.get(name='User').pk,
'allocationform-allocation': [self.proj_allocation.pk]
}

@override_settings(PLUGIN_LDAP=True)
def test_projectaddusers_ldapsignalfail_messages(self):
"""Test the messages displayed when the add user signal fails"""
self.client.force_login(self.pi_user)

@patch('coldfront.core.project.signals.project_make_projectuser.send')
def test_projectaddusers_form_validation(self, mock_signal):
"""Test that the formset and allocation form are validated correctly"""
self.client.force_login(self.proj_accessmanager)
mock_signal.return_value = None
# Prepare form data for adding a user
response = self.client.post(self.url, data=self.form_data)
self.assertEqual(response.url, reverse('project-detail', kwargs={'pk': self.project.pk}))
self.assertEqual(response.status_code, 302)
# Check that user was added
self.assertTrue(ProjectUser.objects.filter(project=self.project, user=self.nonproj_allocation_user).exists())

@patch('coldfront.core.project.signals.project_make_projectuser.send')
def test_projectaddusers_signal_fail(self, mock_signal):
"""Test that the add users form fails when the signal sent to LDAP fails"""
self.client.force_login(self.proj_accessmanager)
mock_signal.side_effect = Exception("LDAP error occurred")
# Prepare form data for adding a user
response = self.client.post(self.url, data=self.form_data, follow=True)
self.assertContains(response, 'LDAP error occurred')
self.assertContains(response, 'Added 0 users')


class ProjectUserDetailViewTest(ProjectViewTestBase):
"""Tests for ProjectUserDetailView"""
Expand Down
136 changes: 63 additions & 73 deletions coldfront/core/project/views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,13 @@
allocation_remove_user,
allocation_activate_user,
)
from coldfront.core.project.signals import project_create, project_post_create
from coldfront.core.project.signals import (
project_filter_users_to_remove,
project_preremove_projectuser,
project_make_projectuser,
project_create,
project_post_create
)
from coldfront.core.grant.models import Grant
from coldfront.core.project.forms import (
ProjectReviewForm,
Expand Down Expand Up @@ -68,9 +74,6 @@
if 'django_q' in settings.INSTALLED_APPS:
from django_q.tasks import Task

if 'coldfront.plugins.ldap' in settings.INSTALLED_APPS:
from coldfront.plugins.ldap.utils import LDAPConn

ALLOCATION_ENABLE_ALLOCATION_RENEWAL = import_from_settings(
'ALLOCATION_ENABLE_ALLOCATION_RENEWAL', True)
ALLOCATION_DEFAULT_ALLOCATION_LENGTH = import_from_settings(
Expand Down Expand Up @@ -609,10 +612,7 @@ def post(self, request, *args, **kwargs):
allocation_form = ProjectAddUsersToAllocationForm(
request.user, project_obj.pk, request.POST, prefix='allocationform'
)

added_users_count = 0
if 'coldfront.plugins.ldap' in settings.INSTALLED_APPS:
ldap_conn = LDAPConn()

if formset.is_valid() and allocation_form.is_valid():
projuserstatus_active = ProjectUserStatusChoice.objects.get(name='Active')
Expand Down Expand Up @@ -649,28 +649,19 @@ def post(self, request, *args, **kwargs):

role_choice = user_form_data.get('role')

if 'coldfront.plugins.ldap' in settings.INSTALLED_APPS:
try:
ldap_conn.add_user_to_group(
user_obj.username, project_obj.title,
)
logger.info(
"P678: Coldfront user %s added AD User for %s to AD Group %s",
self.request.user,
user_obj.username,
project_obj.title,
)
except Exception as e:
error = f"Could not add user {user_obj} to AD Group for {project_obj.title}: {e}\nPlease contact Coldfront administration for further assistance."
logger.error(
"P685: user %s could not add AD user of %s to AD Group of %s: %s",
self.request.user, user_obj, project_obj.title, e
)
errors.append(error)
continue
success_msg = f"User {user_obj} added by {request.user} to AD Group for {project_obj.title}"
logger.info(success_msg)
successes.append(success_msg)
try:
project_make_projectuser.send(
sender=self.__class__,
user_name=user_obj.username, group_name=project_obj.title
)
except Exception as e:
error = f"Could not add user {user_obj} to AD Group for {project_obj.title}: {e}\nPlease contact Coldfront administration for further assistance."
logger.exception('P646: %s', e)
errors.append(error)
continue
success_msg = f"User {user_obj} added by {request.user} to AD Group for {project_obj.title}"
logger.info(success_msg)
successes.append(success_msg)

# Is the user already in the project?
project_obj.projectuser_set.update_or_create(
Expand All @@ -680,7 +671,6 @@ def post(self, request, *args, **kwargs):
'status': projuserstatus_active,
}
)

added_users_count += 1
for allocation in Allocation.objects.filter(
pk__in=allocation_form_data
Expand Down Expand Up @@ -757,18 +747,19 @@ def get_users_to_remove(self, project_obj):
def get(self, request, *args, **kwargs):
pk = self.kwargs.get('pk')
project_obj = get_object_or_404(Project, pk=pk)
users_to_remove = self.get_users_to_remove(project_obj)
users_no_removal = None
users_list = self.get_users_to_remove(project_obj)

# if ldap is activated, prevent selection of users with project corresponding to primary group
if 'coldfront.plugins.ldap' in settings.INSTALLED_APPS:

usernames = [u['username'] for u in users_to_remove]
ldap_conn = LDAPConn()
users_main_group = ldap_conn.users_in_primary_group(
usernames, project_obj.title)
ingroup = lambda u: u['username'] in users_main_group
users_no_removal, users_to_remove = sort_by(users_to_remove, ingroup, how="condition")
signal_response = project_filter_users_to_remove.send(
sender=self.__class__, users_to_remove=users_list, project=project_obj
)
if signal_response:
user_categories = signal_response[0][1]
users_no_removal = user_categories[0]
users_to_remove = user_categories[1]
else:
users_no_removal = []
users_to_remove = users_list

context = {}

Expand All @@ -789,15 +780,14 @@ def post(self, request, *args, **kwargs):

users_to_remove = self.get_users_to_remove(project_obj)
# if ldap is activated, prevent selection of users with project corresponding to primary group
if 'coldfront.plugins.ldap' in settings.INSTALLED_APPS:

usernames = [u['username'] for u in users_to_remove]
ldap_conn = LDAPConn()
users_main_group = ldap_conn.users_in_primary_group(
usernames, project_obj.title)
ingroup = lambda u: u['username'] in users_main_group
users_no_removal, users_to_remove = sort_by(users_to_remove, ingroup, how="condition")

signal_response = project_filter_users_to_remove.send(
sender=self.__class__, users_to_remove=users_to_remove, project=project_obj
)
if signal_response:
user_categories = signal_response[0][1]
users_to_remove = user_categories[1]
else:
users_to_remove = users_to_remove

formset = formset_factory(ProjectRemoveUserForm, max_num=len(users_to_remove))
formset = formset(request.POST, initial=users_to_remove, prefix='userform')
Expand All @@ -822,30 +812,30 @@ def post(self, request, *args, **kwargs):

project_user_obj = project_obj.projectuser_set.get(user=user_obj)

if 'coldfront.plugins.ldap' in settings.INSTALLED_APPS:
try:
ldap_conn.remove_member_from_group(
user_obj.username, project_obj.title,
)
logger.info(
"P835: Coldfront user %s removed AD User for %s from AD Group for %s",
self.request.user,
user_obj.username,
project_obj.title,
)
except Exception as e:
messages.error(
request,
f"could not remove user {user_obj}: {e}"
)
logger.error(
"P846: Coldfront user %s could NOT remove AD User for %s from AD Group for %s: %s",
self.request.user,
user_obj.username,
project_obj.title,
e
)
continue
try:
project_preremove_projectuser.send(
sender=self.__class__,
user_name=user_obj.username, group_name=project_obj.title
)
logger.info(
"P802: Coldfront user %s removed AD User for %s from AD Group for %s",
self.request.user,
user_obj.username,
project_obj.title,
)
except Exception as e:
messages.error(
request,
f"could not remove user {user_obj}: {e}"
)
logger.exception(
"P802: Coldfront user %s could NOT remove AD User for %s from AD Group for %s: %s",
self.request.user,
user_obj.username,
project_obj.title,
e
)
continue

project_user_obj.status = projectuser_status_removed
project_user_obj.save()
Expand Down
35 changes: 18 additions & 17 deletions coldfront/core/resource/management/commands/add_resource_defaults.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,27 +83,28 @@ def handle(self, *args, **options):
('Tier 3', 'Attic Storage - Tape', True, storage_tier, None, 20, True, True),
('holylfs04/tier0', 'Holyoke data center lustre storage', True, storage, 'Tier 0', 1, True, True),
('holylfs05/tier0', 'Holyoke data center lustre storage', True, storage, 'Tier 0', 1, True, True),
('holylfs06/tier0', 'Holyoke data center lustre storage', True, storage, 'Tier 0', 1, True, True),
('nesetape/tier3', 'Cold storage for past projects', True, storage, 'Tier 3', 20, True, True),
('holy-isilon/tier1', 'Tier1 storage with snapshots and disaster recovery copy', True, storage, 'Tier 1', 1, True, True),
('bos-isilon/tier1', 'Tier1 storage for on-campus storage mounting', True, storage, 'Tier 1', 1, True, True),
('holystore01/tier0', 'Luster storage under Tier0', True, storage, 'Tier 0', 1, True, True),
('b-nfs02-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True),
('b-nfs03-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True),
('b-nfs04-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True),
('b-nfs05-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True),
('b-nfs06-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True),
('b-nfs07-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True),
('b-nfs08-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True),
('b-nfs09-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True),
('h-nfs11-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True),
('h-nfs12-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True),
('h-nfs13-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True),
('h-nfs14-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True),
('h-nfs15-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True),
('h-nfs16-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True),
('h-nfs17-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True),
('h-nfs18-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True),
('h-nfs19-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True),
('b-nfs02-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True),
('b-nfs03-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True),
('b-nfs04-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True),
('b-nfs05-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True),
('b-nfs06-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True),
('b-nfs07-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True),
('b-nfs08-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True),
('b-nfs09-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True),
('h-nfs11-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True),
('h-nfs12-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True),
('h-nfs13-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True),
('h-nfs14-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True),
('h-nfs15-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True),
('h-nfs16-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True),
('h-nfs17-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True),
('h-nfs18-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True),
('h-nfs19-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True),
('boslfs02', 'complimentary lab storage', True, storage, 'Tier 0', 1, False, False),
('holylabs', 'complimentary lab storage', True, storage, 'Tier 0', 1, False, False),
):
Expand Down
Loading

0 comments on commit faa494f

Please sign in to comment.