Skip to content

Commit

Permalink
Better handle activities you can't apply to
Browse files Browse the repository at this point in the history
Instead of a 404, we should just inform users of the endpoints that *do*
support applications.
  • Loading branch information
DavidCain committed Oct 15, 2022
1 parent f221718 commit ac14b3e
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 9 deletions.
4 changes: 2 additions & 2 deletions ws/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,7 @@ class BaseRating(models.Model):
notes = models.TextField(max_length=500, blank=True) # Contingencies, etc.

@property
def activity_enum(self):
def activity_enum(self) -> enums.Activity:
return enums.Activity(self.activity)

def __str__(self):
Expand Down Expand Up @@ -1674,7 +1674,7 @@ def can_reapply(cls, latest_application):
return time_passed > timedelta(days=waiting_period_days)

@property
def activity(self):
def activity(self) -> str:
"""Extract the activity name from the class name/db_name.
Meant to be used by inheriting classes, for example:
Expand Down
28 changes: 28 additions & 0 deletions ws/templates/leaders/apply_any_activity.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
{% extends "base.html" %}
{% load application_tags %}
{% load crispy_forms_tags %}

{% block head_title %}Apply to be a leader{% endblock head_title %}

{% block content %}
{{ block.super }}

<h1>Become a MITOC leader</h1>

<p class="lead">
MITOC's volunteer leaders run trips for the community.
</p>

<p>
MITOCers may apply to become a leader, at the discretion of the activity chair(s).
</p>

Activities which accept leader applications:
<ul>
{% for activity_enum in activities_accepting_applications %}
<li><a href="{% url 'become_leader' activity_enum.value %}">{{ activity_enum.label}}</a></li>
{% endfor %}
</ul>


{% endblock content %}
61 changes: 60 additions & 1 deletion ws/tests/views/test_applications.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
from unittest import mock

from bs4 import BeautifulSoup
from django.contrib import messages
from django.contrib.auth.models import Group
from django.test import Client, TestCase
from freezegun import freeze_time
Expand Down Expand Up @@ -127,7 +130,63 @@ class BikingLeaderApplicationTest(TestCase):
def test_404(self):
self.client.force_login(factories.ParticipantFactory.create().user)
response = self.client.get('/biking/leaders/apply/')
self.assertEqual(response.status_code, 404)
self.assertEqual(response.status_code, 302)
self.assertEqual(response.url, "/leaders/apply/")


class AnyActivityLeaderApplyViewTest(TestCase):
def test_anonymous_user_can_see_page(self):
response = self.client.get('/leaders/apply/')
self.assertEqual(response.status_code, 200)
self.assertIn(
b"MITOC's volunteer leaders run trips for the community",
response.content,
)
# 3 activities (at time of writing) which accept applications
self.assertIn(b"Climbing", response.content)
self.assertIn(b"/climbing/leaders/apply/", response.content)
self.assertIn(b"Hiking", response.content)
self.assertIn(b"/hiking/leaders/apply/", response.content)
self.assertIn(b"Winter School", response.content)
self.assertIn(b"/winter_school/leaders/apply/", response.content)

def test_redirected_from_unknown_activity(self):
self.client.force_login(factories.ParticipantFactory.create().user)
with mock.patch.object(messages, 'error', wraps=messages.error) as msg_error:
response = self.client.get('/raccoon_wrestling/leaders/apply/')
self.assertEqual(response.status_code, 302)
self.assertEqual(response.url, "/leaders/apply/")
msg_error.assert_called_once_with(
mock.ANY, 'raccoon_wrestling is not a known activity.'
)

def test_redirected_from_activity_without_application(self):
self.client.force_login(factories.ParticipantFactory.create().user)
with mock.patch.object(messages, 'error', wraps=messages.error) as msg_error:
response = self.client.get('/cabin/leaders/apply/')
self.assertEqual(response.status_code, 302)
self.assertEqual(response.url, "/leaders/apply/")
msg_error.assert_called_once_with(
mock.ANY, 'Cabin is not accepting leader applications'
)

def test_no_underscores(self):
"""Reproduces a somewhat odd case where stripping underscores led to a 500.
This was because our usual "activity is valid" checks first stripped underscores,
but then `winterschool` was expected to be a valid enum value & was not.
"""
self.client.force_login(factories.ParticipantFactory.create().user)
with mock.patch.object(messages, 'error', wraps=messages.error) as msg_error:
response = self.client.get('/winterschool/leaders/apply/')
self.assertEqual(response.status_code, 302)
self.assertEqual(response.url, "/leaders/apply/")
msg_error.assert_called_once_with(
mock.ANY, 'winterschool is not a known activity.'
)
self.assertIn(
b"/winter_school/leaders/apply/", self.client.get(response.url).content
)


class HikingLeaderApplicationTest(TestCase, Helpers):
Expand Down
4 changes: 2 additions & 2 deletions ws/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,8 @@
path('profile/edit/', views.EditProfileView.as_view(), name='edit_profile'),
path(
'leaders/apply/',
RedirectView.as_view(url='/winter_school/leaders/apply', permanent=True),
name='old_become_leader',
views.AnyActivityLeaderApplyView.as_view(),
name='leaders_apply',
),
path('profile/membership/', views.PayDuesView.as_view(), name='pay_dues'),
path('profile/waiver/', views.SignWaiverView.as_view(), name='initiate_waiver'),
Expand Down
32 changes: 28 additions & 4 deletions ws/views/applications.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@
from django.db.models.functions import Cast, Least
from django.forms.models import model_to_dict
from django.http import Http404
from django.shortcuts import render
from django.shortcuts import redirect, render
from django.urls import reverse, reverse_lazy
from django.utils.decorators import method_decorator
from django.views.generic import CreateView, DetailView, ListView
from django.views.generic import CreateView, DetailView, ListView, TemplateView
from django.views.generic.edit import FormMixin

import ws.utils.perms as perm_utils
Expand Down Expand Up @@ -144,11 +144,35 @@ def get_context_data(self, **kwargs):
@method_decorator(user_info_required)
def dispatch(self, request, *args, **kwargs):
activity = kwargs.get('activity')
if not models.LeaderApplication.can_apply_for_activity(activity):
raise Http404
try:
activity_enum = enums.Activity(activity)
except ValueError: # (Not a valid activity)
messages.error(self.request, f"{activity} is not a known activity.")
return redirect(reverse('leaders_apply'))

if not models.LeaderApplication.can_apply_for_activity(activity_enum):
messages.error(
self.request,
f"{activity_enum.label} is not accepting leader applications",
)
return redirect(reverse('leaders_apply'))

return super().dispatch(request, *args, **kwargs)


class AnyActivityLeaderApplyView(TemplateView):
template_name = 'leaders/apply_any_activity.html'

def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)
context['activities_accepting_applications'] = [
activity_enum
for activity_enum in enums.Activity
if models.LeaderApplication.can_apply_for_activity(activity_enum)
]
return context


# model is a property on LeaderApplicationMixin, but a class attribute on MultipleObjectMixin
class AllLeaderApplicationsView(ApplicationManager, ListView): # type: ignore[misc]
context_object_name = 'leader_applications'
Expand Down

0 comments on commit ac14b3e

Please sign in to comment.