-
Notifications
You must be signed in to change notification settings - Fork 21
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
Events: auto reject registration applications to event after the event ends #1934
base: dev
Are you sure you want to change the base?
Changes from all commits
6e2a33f
5f3e9a4
1949efb
86812b5
7e550fb
438d1e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
import logging | ||
|
||
from django.core.management.base import BaseCommand | ||
from django.conf import settings | ||
from django.db import transaction | ||
from django.http import HttpRequest | ||
from django.utils import timezone | ||
|
||
from events.models import Event, EventApplication | ||
import notification.utility as notification | ||
|
||
LOGGER = logging.getLogger(__name__) | ||
|
||
AUTO_REJECTION_REASON = 'Event has ended.' | ||
|
||
|
||
class Command(BaseCommand): | ||
def add_arguments(self, parser): | ||
parser.add_argument("-n", "--number", type=int, help="Number of applications to be rejected") | ||
|
||
def handle(self, *args, **options): | ||
""" | ||
Auto reject pending registration applications for events that have ended | ||
""" | ||
if not settings.ENABLE_EVENT_REGISTRATION_AUTO_REJECTION: | ||
LOGGER.info('Auto rejection of event applications is disabled.') | ||
return | ||
|
||
# creating an instance of HttpRequest to be used in the notification utility | ||
request = HttpRequest() | ||
|
||
total_applications_per_event_to_reject = (options['number'] | ||
or settings.DEFAULT_NUMBER_OF_APPLICATIONS_TO_REJECT_PER_EVENT) | ||
past_events = Event.objects.filter( | ||
end_date__lt=timezone.now(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems like it would introduce a lot of redundancy - it will fetch all the events that were finished in the past, even if this command already processed them. Intuitively, this is something that should happen once per event. Fetching events from years ago makes no sense for this command. |
||
applications__status=EventApplication.EventApplicationStatus.WAITLISTED) | ||
|
||
LOGGER.info(f'{past_events.count()} events selected for auto rejection of waitlisted applications.') | ||
|
||
for event in past_events: | ||
applications = event.applications.filter(status=EventApplication.EventApplicationStatus.WAITLISTED)[ | ||
:total_applications_per_event_to_reject | ||
] | ||
Comment on lines
+41
to
+43
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Batching the rejections this way doesn't make a lot of sense to me. Doing a |
||
for application in applications: | ||
with transaction.atomic(): | ||
application.reject(comment_to_applicant=AUTO_REJECTION_REASON) | ||
notification.notify_participant_event_decision( | ||
request=request, | ||
user=application.user, | ||
event=application.event, | ||
decision=EventApplication.EventApplicationStatus.NOT_APPROVED.label, | ||
comment_to_applicant=AUTO_REJECTION_REASON | ||
) | ||
LOGGER.info(f'Application {application.id} for event {event.id} rejected.') |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
import logging | ||
from io import StringIO | ||
|
||
from django.conf import settings | ||
from django.core.management import call_command | ||
from django.utils import timezone | ||
|
||
from events.models import Event, EventApplication | ||
from events.tests_views import TestMixin | ||
|
||
LOGGER = logging.getLogger(__name__) | ||
|
||
|
||
class TestRejectPendingCredentialingApplications(TestMixin): | ||
|
||
def test_rejection(self): | ||
|
||
# get dict of applications to be rejected per event | ||
event_application_dict = {} | ||
past_events = Event.objects.filter( | ||
end_date__lt=timezone.now(), | ||
applications__status=EventApplication.EventApplicationStatus.WAITLISTED) | ||
for event in past_events: | ||
event_application_dict[event] = event.applications.filter( | ||
status=EventApplication.EventApplicationStatus.WAITLISTED | ||
)[:settings.DEFAULT_NUMBER_OF_APPLICATIONS_TO_REJECT_PER_EVENT] | ||
|
||
LOGGER.info(f'Found {len(past_events)} events with waitlisted applications to be rejected.') | ||
|
||
# call the management command to auto reject applications | ||
out = StringIO() | ||
call_command('reject_past_event_applications', | ||
number=settings.DEFAULT_NUMBER_OF_APPLICATIONS_TO_REJECT_PER_EVENT, stdout=out) | ||
|
||
if not settings.ENABLE_EVENT_REGISTRATION_AUTO_REJECTION: | ||
# check if the applications status is unchanged | ||
LOGGER.info('Auto rejection of event applications is disabled.') | ||
LOGGER.info('Checking if the applications status is unchanged.') | ||
for event, applications in event_application_dict.items(): | ||
for application in applications: | ||
application.refresh_from_db() | ||
self.assertEqual(application.status, EventApplication.EventApplicationStatus.WAITLISTED) | ||
return | ||
|
||
# check if the applications are rejected | ||
for event, applications in event_application_dict.items(): | ||
for application in applications: | ||
application.refresh_from_db() | ||
self.assertEqual(application.status, EventApplication.EventApplicationStatus.NOT_APPROVED) | ||
LOGGER.info(f'Application {application.id} for event {event.id} auto rejection confirmed.') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's disabled then, in my opinion, this command shouldn't even be called. Also, there is no point in logging a string every time the cron job is executed.