give create_api_access_request mgmt cmd an option to disconnect signals.
This commit is contained in:
committed by
Alex Dusenbery
parent
b92c30de10
commit
df3d6d160f
@@ -1,17 +1,48 @@
|
||||
""" Management command to create an ApiAccessRequest for given users """
|
||||
from __future__ import unicode_literals
|
||||
|
||||
import logging
|
||||
from django.core.management.base import BaseCommand, CommandError
|
||||
from contextlib import contextmanager
|
||||
|
||||
from django.contrib.auth.models import User
|
||||
from django.contrib.sites.models import Site
|
||||
from django.core.management.base import BaseCommand, CommandError
|
||||
from django.db.models.signals import post_save, pre_save
|
||||
from six import text_type
|
||||
|
||||
from openedx.core.djangoapps.api_admin.models import (
|
||||
ApiAccessConfig,
|
||||
ApiAccessRequest,
|
||||
send_decision_email,
|
||||
send_request_email
|
||||
)
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
@contextmanager
|
||||
def disconnect_request_email_signals():
|
||||
"""
|
||||
Context manager to be used for temporarily disconnecting the `send_request_email`
|
||||
and `send_decision_email` pre/post_save signal receivers from the `ApiAccessRequest` model.
|
||||
"""
|
||||
post_save.disconnect(
|
||||
send_request_email, sender=ApiAccessRequest, dispatch_uid="api_access_request_post_save_email"
|
||||
)
|
||||
pre_save.disconnect(
|
||||
send_decision_email, sender=ApiAccessRequest, dispatch_uid="api_access_request_pre_save_email"
|
||||
)
|
||||
try:
|
||||
yield
|
||||
finally:
|
||||
post_save.connect(
|
||||
send_request_email, sender=ApiAccessRequest, dispatch_uid="api_access_request_post_save_email"
|
||||
)
|
||||
pre_save.connect(
|
||||
send_decision_email, sender=ApiAccessRequest, dispatch_uid="api_access_request_pre_save_email"
|
||||
)
|
||||
|
||||
|
||||
class Command(BaseCommand):
|
||||
"""
|
||||
Create an ApiAccessRequest for the given user
|
||||
@@ -31,6 +62,11 @@ class Command(BaseCommand):
|
||||
action='store_true',
|
||||
help='Create ApiAccessConfig if it does not exist'
|
||||
)
|
||||
parser.add_argument(
|
||||
'--disconnect-signals',
|
||||
action='store_true',
|
||||
help='Disconnect the Django signal receivers that send emails when ApiAccessRequest records are saved'
|
||||
)
|
||||
parser.add_argument(
|
||||
'--status',
|
||||
choices=[choice[0] for choice in ApiAccessRequest.STATUS_CHOICES],
|
||||
@@ -49,6 +85,13 @@ class Command(BaseCommand):
|
||||
)
|
||||
|
||||
def handle(self, *args, **options):
|
||||
if options.get('disconnect_signals'):
|
||||
with disconnect_request_email_signals():
|
||||
self._handle(*args, **options)
|
||||
else:
|
||||
self._handle(*args, **options)
|
||||
|
||||
def _handle(self, *args, **options): # pylint: disable=unused-argument
|
||||
if options.get('create_config'):
|
||||
self.create_api_access_config()
|
||||
user = self.get_user(options.get('username'))
|
||||
@@ -63,7 +106,7 @@ class Command(BaseCommand):
|
||||
try:
|
||||
return User.objects.get(username=username)
|
||||
except User.DoesNotExist:
|
||||
raise CommandError(u'User {} not found'.format(username))
|
||||
raise CommandError('User {} not found'.format(username))
|
||||
|
||||
def create_api_access_request(self, user, status, reason, website):
|
||||
"""
|
||||
@@ -77,14 +120,23 @@ class Command(BaseCommand):
|
||||
reason=reason,
|
||||
site=Site.objects.get_current(),
|
||||
)
|
||||
except OSError as e:
|
||||
# Ignore a specific error that occurs in the downstream `send_request_email` receiver.
|
||||
# see https://openedx.atlassian.net/browse/EDUCATOR-4478
|
||||
error_msg = text_type(e)
|
||||
if 'Permission denied' in error_msg and 'mako_lms' in error_msg:
|
||||
logger.warning('Error sending email about access request: {}'.format(error_msg))
|
||||
else:
|
||||
raise CommandError(error_msg)
|
||||
except Exception as e:
|
||||
msg = u'Unable to create ApiAccessRequest for {}. Exception is {}: {}'.format(
|
||||
msg = 'Unable to create ApiAccessRequest for {}. Exception is {}: {}'.format(
|
||||
user.username,
|
||||
type(e).__name__,
|
||||
e
|
||||
)
|
||||
raise CommandError(msg)
|
||||
logger.info(u'Created ApiAccessRequest for user {}'.format(user.username))
|
||||
|
||||
logger.info('Created ApiAccessRequest for user {}'.format(user.username))
|
||||
|
||||
def create_api_access_config(self):
|
||||
"""
|
||||
@@ -93,9 +145,10 @@ class Command(BaseCommand):
|
||||
try:
|
||||
_, created = ApiAccessConfig.objects.get_or_create(enabled=True)
|
||||
except Exception as e:
|
||||
msg = u'Unable to create ApiAccessConfig. Exception is {}: {}'.format(type(e).__name__, e)
|
||||
msg = 'Unable to create ApiAccessConfig. Exception is {}: {}'.format(type(e).__name__, e)
|
||||
raise CommandError(msg)
|
||||
|
||||
if created:
|
||||
logger.info(u'Created ApiAccessConfig')
|
||||
logger.info('Created ApiAccessConfig')
|
||||
else:
|
||||
logger.info(u'ApiAccessConfig already exists')
|
||||
logger.info('ApiAccessConfig already exists')
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
from mock import patch
|
||||
import unittest
|
||||
|
||||
import ddt
|
||||
from django.conf import settings
|
||||
@@ -6,13 +6,10 @@ from django.contrib.sites.models import Site
|
||||
from django.core.management import call_command
|
||||
from django.core.management.base import CommandError
|
||||
from django.test import TestCase
|
||||
import unittest
|
||||
from mock import patch
|
||||
|
||||
from openedx.core.djangoapps.api_admin.management.commands import create_api_access_request
|
||||
from openedx.core.djangoapps.api_admin.models import (
|
||||
ApiAccessConfig,
|
||||
ApiAccessRequest,
|
||||
)
|
||||
from openedx.core.djangoapps.api_admin.models import ApiAccessConfig, ApiAccessRequest
|
||||
from student.tests.factories import UserFactory
|
||||
|
||||
|
||||
@@ -43,6 +40,20 @@ class TestCreateApiAccessRequest(TestCase):
|
||||
call_command(self.command, self.user.username, create_config=create_config)
|
||||
self.assert_models_exist(True, create_config)
|
||||
|
||||
@patch('openedx.core.djangoapps.api_admin.models._send_new_pending_email')
|
||||
def test_create_api_access_request_signals_disconnected(self, mock_send_new_pending_email):
|
||||
self.assert_models_exist(False, False)
|
||||
call_command(self.command, self.user.username, create_config=True, disconnect_signals=True)
|
||||
self.assert_models_exist(True, True)
|
||||
self.assertFalse(mock_send_new_pending_email.called)
|
||||
|
||||
@patch('openedx.core.djangoapps.api_admin.models._send_new_pending_email')
|
||||
def test_create_api_access_request_signals_connected(self, mock_send_new_pending_email):
|
||||
self.assert_models_exist(False, False)
|
||||
call_command(self.command, self.user.username, create_config=True, disconnect_signals=False)
|
||||
self.assert_models_exist(True, True)
|
||||
self.assertTrue(mock_send_new_pending_email.called)
|
||||
|
||||
def test_config_already_exists(self):
|
||||
ApiAccessConfig.objects.create(enabled=True)
|
||||
self.assert_models_exist(False, True)
|
||||
@@ -64,6 +75,34 @@ class TestCreateApiAccessRequest(TestCase):
|
||||
|
||||
self.assert_models_exist(False, False)
|
||||
|
||||
@patch('openedx.core.djangoapps.api_admin.models.send_request_email')
|
||||
def test_api_request_permission_denied_error(self, mocked_method):
|
||||
"""
|
||||
When a Permission denied OSError with 'mako_lms' in the message occurs in the post_save receiver,
|
||||
the models should still be created and the command should finish without raising.
|
||||
"""
|
||||
mocked_method.side_effect = OSError('Permission denied: something something in /tmp/mako_lms')
|
||||
|
||||
self.assert_models_exist(False, False)
|
||||
|
||||
call_command(self.command, self.user.username, create_config=True)
|
||||
|
||||
self.assert_models_exist(True, True)
|
||||
|
||||
@patch('openedx.core.djangoapps.api_admin.models.ApiAccessRequest.objects.create')
|
||||
def test_api_request_other_oserrors_raise(self, mocked_method):
|
||||
"""
|
||||
When some other Permission denied OSError occurs, we should still raise.
|
||||
"""
|
||||
mocked_method.side_effect = OSError('out of disk space')
|
||||
|
||||
self.assert_models_exist(False, False)
|
||||
|
||||
with self.assertRaisesRegex(CommandError, 'out of disk space'):
|
||||
call_command(self.command, self.user.username)
|
||||
|
||||
self.assert_models_exist(False, False)
|
||||
|
||||
@patch('openedx.core.djangoapps.api_admin.models.ApiAccessConfig.objects.get_or_create')
|
||||
def test_api_config_error(self, mocked_method):
|
||||
mocked_method.side_effect = Exception()
|
||||
|
||||
Reference in New Issue
Block a user