Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 585: Ensure that users cannot be added to "New" / "Inactive" / "Archived" projects #645

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
15 changes: 14 additions & 1 deletion coldfront/core/project/templates/project/project_detail.html
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,21 @@ <h2 class="d-inline"><i class="fas fa-cubes"></i></a> <a name="manage-project">M
<div class="card-body">
{% if project.status.name != 'Archived' %}
{% if is_allowed_to_update_project %}
<a class="btn btn-success" href="{% url 'project-add-users-search' project.pk %}" role="button"><i class="fas fa-user-plus" aria-hidden="true"></i> Add Users</a>
{% if project.status.name == 'Active' %}
<a
class="btn btn-success"
href="{% url 'project-add-users-search' project.pk %}"
role="button">
<i class="fas fa-user-plus" aria-hidden="true"></i>
Add Users
</a>
{% else %}
<button type="button" class="btn btn-success" disabled>
<i class="fas fa-user-plus" aria-hidden="true"></i>
Add Users
</button>
{% endif %}
{% endif %}
<a class="btn btn-success" href="{% url 'project-review-join-requests' project.pk %}" role="button">
<i class="fas fa-user-plus" aria-hidden="true"></i>
Review Join Requests
Expand Down
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
from coldfront.core.project.models import Project
from coldfront.core.project.models import ProjectStatusChoice
from coldfront.core.utils.tests.test_base import TestBase

from django.urls import reverse
from django.contrib.messages import get_messages


class TestProjectAddUsersSearchResultsView(TestBase):
"""A class for testing ProjectAddUsersSearchResultsView."""

def setUp(self):
"""Set up test data."""
super().setUp()
self.create_test_user()
self.sign_user_access_agreement(self.user)
self.client.login(username=self.user.username, password=self.password)

def test_error_raised_for_non_active_project(self):
"""Test that an error and error message appear when trying to add users
to 'Archived', 'New', or 'Inactive' Projects and doesn't appear for
'Active' Projects"""
statuses = ['Active', 'Inactive', 'Archived', 'New']
projects = []
for status in statuses:
name = f'{status.lower()}_project'
status_obj = ProjectStatusChoice.objects.get(name=status)
projects.append(Project.objects.create(
name=name, title=name, status=status_obj))

for i in range(1, len(projects)):
url = reverse('project-add-users-search-results',
kwargs={'pk': projects[i].pk})
response = self.client.get(url)
messages = list(get_messages(response.wsgi_request))
self.assertEqual(len(messages), i)
self.assertEqual(str(messages[i - 1]),
f'You cannot add users to a project with status {statuses[i]}')

Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
from coldfront.core.project.models import Project
from coldfront.core.project.models import ProjectStatusChoice
from coldfront.core.utils.tests.test_base import TestBase

from django.urls import reverse
from django.contrib.messages import get_messages


class TestProjectAddUsersSearchView(TestBase):
"""A class for testing ProjectAddUsersSearchView."""

def setUp(self):
"""Set up test data."""
super().setUp()
self.create_test_user()
self.sign_user_access_agreement(self.user)
self.client.login(username=self.user.username, password=self.password)

def test_error_raised_for_non_active_project(self):
"""TODO"""
statuses = ['Active', 'Inactive', 'Archived', 'New']
projects = []
for status in statuses:
name = f'{status.lower()}_project'
status_obj = ProjectStatusChoice.objects.get(name=status)
projects.append(Project.objects.create(
name=name, title=name, status=status_obj))

for i in range(1, len(projects)):
url = reverse('project-add-users-search',
kwargs={'pk': projects[i].pk})
response = self.client.get(url)
messages = list(get_messages(response.wsgi_request))
self.assertEqual(len(messages), i)
self.assertEqual(str(messages[i - 1]),
f'You cannot add users to a project with status {statuses[i]}')
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
from coldfront.core.project.models import Project
from coldfront.core.project.models import ProjectStatusChoice
from coldfront.core.utils.tests.test_base import TestBase

from django.urls import reverse
from django.contrib.messages import get_messages


class TestProjectAddUsersView(TestBase):
"""A class for testing ProjectAddUsersView."""

def setUp(self):
"""Set up test data."""
super().setUp()
self.create_test_user()
self.sign_user_access_agreement(self.user)
self.client.login(username=self.user.username, password=self.password)
self.user.is_superuser = True
self.user.save()

def test_error_raised_for_non_active_project(self):
"""Test that an error and error message appear when trying to add users
to 'Archived', 'New', or 'Inactive' Projects and doesn't appear for
'Active' Projects"""
statuses = ['Active', 'Inactive', 'Archived', 'New']
projects = []
for status in statuses:
name = f'{status.lower()}_project'
status_obj = ProjectStatusChoice.objects.get(name=status)
projects.append(Project.objects.create(
name=name, title=name, status=status_obj))

# Check that for the active project there is no error message.
url = reverse('project-add-users', kwargs={'pk': projects[0].pk})
response = self.client.get(url)
messages = list(get_messages(response.wsgi_request))
self.assertEqual(len(messages), 0)

for i in range(1, len(projects)):
url = reverse('project-add-users', kwargs={'pk': projects[i].pk})
response = self.client.get(url)
messages = list(get_messages(response.wsgi_request))
self.assertEqual(len(messages), i)
self.assertEqual(str(messages[i - 1]),
f'You cannot add users to a project with status {statuses[i]}')

Original file line number Diff line number Diff line change
Expand Up @@ -26,23 +26,22 @@ def project_join_list_url(**parameters):
given URL parameters."""
return f'{reverse("project-join-list")}?{urlencode(parameters)}'

def test_inactive_projects_not_included(self):
"""Test that Projects with the 'Inactive' status are not
included in the list."""
active_name = 'active_project'
active_status = ProjectStatusChoice.objects.get(name='Active')
Project.objects.create(
name=active_name, title=active_name, status=active_status)
inactive_name = 'inactive_project'
inactive_status = ProjectStatusChoice.objects.get(name='Inactive')
Project.objects.create(
name=inactive_name, title=inactive_name, status=inactive_status)
def test_inactive_archived_new_projects_not_included(self):
"""Test that Projects with the 'Inactive', 'Archived', or 'New' status
are not included in the list."""
statuses = ['Active', 'Inactive', 'Archived', 'New']
for status in statuses:
name = f'{status.lower()}_project'
status_obj = ProjectStatusChoice.objects.get(name=status)
Project.objects.create(
name=name, title=name, status=status_obj)

url = self.project_join_list_url()
response = self.client.get(url)

self.assertContains(response, active_name)
self.assertNotContains(response, inactive_name)
self.assertContains(response, f'{statuses[0].lower()}_project')
for i in range(1, len(statuses)):
self.assertNotContains(response, f'{statuses[i].lower()}_project')

def create_join_request(self, user, project, host_user=None):
"""Creates a join request for a certain project. Returns the response"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,25 +23,28 @@ def project_join_url(pk):
key."""
return reverse('project-join', kwargs={'pk': pk})

def test_inactive_projects_cannot_be_joined(self):
"""Test that Projects with the 'Inactive' status cannot be
joined."""
name = 'inactive_project'
inactive_status = ProjectStatusChoice.objects.get(name='Inactive')
project = Project.objects.create(
name=name, title=name, status=inactive_status)

url = self.project_join_url(project.pk)
data = {
'reason': 'This is a test reason for joining the project.',
}
response = self.client.post(url, data)

expected = f'Project {name} is inactive, and may not be joined.'
messages = list(response.context['messages'])
self.assertEqual(len(messages), 1)
actual = messages[0].message
self.assertEqual(expected, actual)
def test_new_archived_inactive_projects_not_joinable(self):
"""Test that Projects with the 'Inactive', 'Archived', or 'New' status
cannot be joined."""
statuses = ['Inactive', 'Archived', 'New']
for status in statuses:
name = f'{status.lower()}_project'
status_obj = ProjectStatusChoice.objects.get(name=status)
project = Project.objects.create(
name=name, title=name, status=status_obj)

url = self.project_join_url(project.pk)
data = {
'reason': 'This is a test reason for joining the project.',
}
response = self.client.post(url, data)

expected = \
f'Project {name} is {status}, and may not be joined.'
messages = list(response.context['messages'])
self.assertEqual(len(messages), 1)
actual = messages[0].message
self.assertEqual(expected, actual)

def test_host_user(self):
"""Test that ProjectJoinView sets the host user in
Expand Down
16 changes: 10 additions & 6 deletions coldfront/core/project/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -712,13 +712,15 @@ def test_func(self):

def dispatch(self, request, *args, **kwargs):
project_obj = get_object_or_404(Project, pk=self.kwargs.get('pk'))
if project_obj.status.name not in ['Active', 'Inactive', 'New', ]:
if project_obj.status.name != 'Active':
messages.error(
request, 'You cannot add users to an archived project.')
request, ('You cannot add users to a project with '
f'status {project_obj.status.name}'))
return HttpResponseRedirect(reverse('project-detail', kwargs={'pk': project_obj.pk}))
else:
return super().dispatch(request, *args, **kwargs)


def get_context_data(self, *args, **kwargs):
context = super().get_context_data(*args, **kwargs)
context['user_search_form'] = UserSearchForm()
Expand All @@ -745,9 +747,10 @@ def test_func(self):

def dispatch(self, request, *args, **kwargs):
project_obj = get_object_or_404(Project, pk=self.kwargs.get('pk'))
if project_obj.status.name not in ['Active', 'Inactive', 'New', ]:
if project_obj.status.name != 'Active':
messages.error(
request, 'You cannot add users to an archived project.')
request, ('You cannot add users to a project with '
f'status {project_obj.status.name}'))
return HttpResponseRedirect(reverse('project-detail', kwargs={'pk': project_obj.pk}))
else:
return super().dispatch(request, *args, **kwargs)
Expand Down Expand Up @@ -835,9 +838,10 @@ def dispatch(self, request, *args, **kwargs):
project_obj = get_object_or_404(Project, pk=self.kwargs.get('pk'))
_redirect = HttpResponseRedirect(
reverse('project-detail', kwargs={'pk': project_obj.pk}))
if project_obj.status.name not in ['Active', 'Inactive', 'New', ]:
if project_obj.status.name != 'Active':
messages.error(
request, 'You cannot add users to an archived project.')
request, ('You cannot add users to a project with '
f'status {project_obj.status.name}'))
return _redirect
elif is_project_billing_id_required_and_missing(project_obj):
message = (
Expand Down
13 changes: 7 additions & 6 deletions coldfront/core/project/views_/join_views/request_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,16 @@ def test_func(self):
self.request, 'You must sign the User Access Agreement before you can join a project.')
return False

inactive_project_status = ProjectStatusChoice.objects.get(
name='Inactive')
if project_obj.status == inactive_project_status:
active_project_status = ProjectStatusChoice.objects.get(
name='Active')
if project_obj.status != active_project_status:
message = (
f'Project {project_obj.name} is inactive, and may not be '
f'joined.')
f'Project {project_obj.name} is {project_obj.status.name}, '
f'and may not be joined.')
messages.error(self.request, message)
return False


if project_users.exists():
project_user = project_users.first()
if project_user.status.name == 'Active':
Expand Down Expand Up @@ -202,7 +203,7 @@ def get_queryset(self):

projects = Project.objects.prefetch_related(
'field_of_science', 'status').filter(
status__name__in=['New', 'Active', ]
status__name__in=['Active', ]
).order_by(order_by)
projects = annotate_queryset_with_cluster_name(projects)

Expand Down
Loading