Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove notifier_api app DEPR-111 (#25464)

Browse Source 
Part of the notifier service deprecation (DEPR-106).

Also removed pdfminer from the package uninstall list, since we no longer install the package it conflicts with either.
This commit is contained in:
Jeremy Bowman
2020-10-29 11:17:23 -04:00
committed by GitHub
parent f90035dc48
commit efc04e3399
9 changed files with 1 additions and 406 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
lms/djangoapps/discussion/README.rst
View File

@@ -12,7 +12,7 @@ Discussions related functionality is scattered across a number of places and sho
* ``openedx/core/djangoapps/django_comment_common``
* ``openedx/core/lib/xblock_builtin/xblock_discussion``
Ideally, what we want in the long term is for all of this extracted into a new repository that holds the code for both the inline discussion XBlock as well as all the Django apps. Use of the notifier API should be replaced with edx-ace.
Ideally, what we want in the long term is for all of this extracted into a new repository that holds the code for both the inline discussion XBlock as well as all the Django apps.
That being said, it's not clear what the path forward for this app is. Forum functionality is something that has not been actively worked on for a while, and it's been suggested that we should remove this app altogether in favor of having better integration with other more established forum software. This decision is usually complicated by the desire to have tight integration with courseware and access to data for analytics.

1
lms/djangoapps/discussion/notifier_api/README.rst
View File

@@ -1 +0,0 @@
See ``lms/djangoapps/discussion/README.rst``

0
lms/djangoapps/discussion/notifier_api/__init__.py
View File

70
lms/djangoapps/discussion/notifier_api/serializers.py
View File

@@ -1,70 +0,0 @@
import six
from django.contrib.auth.models import User
from django.http import Http404
from rest_framework import serializers
from lms.djangoapps.discussion.notification_prefs import NOTIFICATION_PREF_KEY
from openedx.core.djangoapps.course_groups.cohorts import is_course_cohorted
from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY
class NotifierUserSerializer(serializers.ModelSerializer):
"""
A serializer containing all information about a user needed by the notifier
(namely the user's name, email address, notification and language
preferences, and course enrollment and cohort information).
Because these pieces of information reside in different tables, this is
designed to work well with prefetch_related and select_related, which
require the use of all() instead of get() or filter(). The following fields
should be prefetched on the user objects being serialized:
* profile
* preferences
* courseenrollment_set
* course_groups
* roles__permissions
"""
name = serializers.SerializerMethodField()
preferences = serializers.SerializerMethodField()
course_info = serializers.SerializerMethodField()
def get_name(self, user):
return user.profile.name
def get_preferences(self, user):
return {
pref.key: pref.value
for pref
in user.preferences.all()
if pref.key in [LANGUAGE_KEY, NOTIFICATION_PREF_KEY]
}
def get_course_info(self, user):
cohort_id_map = {
cohort.course_id: cohort.id
for cohort in user.course_groups.all()
}
see_all_cohorts_set = {
role.course_id
for role in user.roles.all()
for perm in role.permissions.all() if perm.name == "see_all_cohorts"
}
ret = {}
for enrollment in user.courseenrollment_set.all():
if enrollment.is_active:
try:
ret[six.text_type(enrollment.course_id)] = {
"cohort_id": cohort_id_map.get(enrollment.course_id),
"see_all_cohorts": (
enrollment.course_id in see_all_cohorts_set or
not is_course_cohorted(enrollment.course_id)
),
}
except Http404: # is_course_cohorted raises this if course does not exist
pass
return ret
class Meta(object):
model = User
fields = ("id", "email", "name", "preferences", "course_info")
read_only_fields = ("id", "email")

259
lms/djangoapps/discussion/notifier_api/tests.py
View File

@@ -1,259 +0,0 @@
# pylint: disable=missing-docstring
import itertools
import ddt
import six
from six.moves import range
from django.conf import settings
from django.test.client import RequestFactory
from django.test.utils import override_settings
from opaque_keys.edx.locator import CourseLocator
from lms.djangoapps.discussion.notification_prefs import NOTIFICATION_PREF_KEY
from lms.djangoapps.discussion.notifier_api.views import NotifierUsersViewSet
from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory
from openedx.core.djangoapps.django_comment_common.models import Permission, Role
from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY
from openedx.core.djangoapps.user_api.models import UserPreference
from openedx.core.djangoapps.user_api.tests.factories import UserPreferenceFactory
from student.models import CourseEnrollment
from student.tests.factories import CourseEnrollmentFactory, UserFactory
from util.testing import UrlResetMixin
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
@ddt.ddt
@override_settings(EDX_API_KEY="test_api_key")
class NotifierUsersViewSetTest(UrlResetMixin, ModuleStoreTestCase):
def setUp(self):
super(NotifierUsersViewSetTest, self).setUp()
self.courses = []
self.cohorts = []
self.user = UserFactory()
self.notification_pref = UserPreferenceFactory(
user=self.user,
key=NOTIFICATION_PREF_KEY,
value="notification pref test value"
)
self.list_view = NotifierUsersViewSet.as_view({"get": "list"})
self.detail_view = NotifierUsersViewSet.as_view({"get": "retrieve"})
def _set_up_course(self, is_course_cohorted, is_user_cohorted, is_moderator):
cohort_config = {"cohorted": True} if is_course_cohorted else {}
course = CourseFactory(
number=("TestCourse{}".format(len(self.courses))),
cohort_config=cohort_config
)
self.courses.append(course)
CourseEnrollmentFactory(user=self.user, course_id=course.id)
if is_user_cohorted:
cohort = CohortFactory.create(
name="Test Cohort",
course_id=course.id,
users=[self.user]
)
self.cohorts.append(cohort)
if is_moderator:
moderator_perm, _ = Permission.objects.get_or_create(name="see_all_cohorts")
moderator_role = Role.objects.create(name="Moderator", course_id=course.id)
moderator_role.permissions.add(moderator_perm)
self.user.roles.add(moderator_role)
def _assert_basic_user_info_correct(self, user, result_user):
self.assertEqual(result_user["id"], user.id)
self.assertEqual(result_user["email"], user.email)
self.assertEqual(result_user["name"], user.profile.name)
def test_without_api_key(self):
request = RequestFactory().get("dummy")
for view in [self.list_view, self.detail_view]:
response = view(request)
self.assertEqual(response.status_code, 403)
# Detail view tests
def _make_detail_request(self):
request = RequestFactory().get("dummy", HTTP_X_EDX_API_KEY=settings.EDX_API_KEY)
return self.detail_view(
request,
**{NotifierUsersViewSet.lookup_field: str(self.user.id)}
)
def _get_detail(self):
response = self._make_detail_request()
self.assertEqual(response.status_code, 200)
self.assertEqual(
set(response.data.keys()),
{"id", "email", "name", "preferences", "course_info"}
)
return response.data
def test_detail_invalid_user(self):
UserPreference.objects.all().delete()
response = self._make_detail_request()
self.assertEqual(response.status_code, 404)
def test_basic_user_info(self):
result = self._get_detail()
self._assert_basic_user_info_correct(self.user, result)
def test_course_info(self):
expected_course_info = {}
for is_course_cohorted, is_user_cohorted, is_moderator in (
itertools.product([True, False], [True, False], [True, False])
):
self._set_up_course(is_course_cohorted, is_user_cohorted, is_moderator)
expected_course_info[six.text_type(self.courses[-1].id)] = {
"cohort_id": self.cohorts[-1].id if is_user_cohorted else None,
"see_all_cohorts": is_moderator or not is_course_cohorted
}
result = self._get_detail()
self.assertEqual(result["course_info"], expected_course_info)
def test_course_info_unenrolled(self):
self._set_up_course(False, False, False)
course_id = self.courses[0].id
CourseEnrollment.unenroll(self.user, course_id)
result = self._get_detail()
self.assertNotIn(six.text_type(course_id), result["course_info"])
def test_course_info_no_enrollments(self):
result = self._get_detail()
self.assertEqual(result["course_info"], {})
def test_course_info_non_existent_course_enrollment(self):
CourseEnrollmentFactory(
user=self.user,
course_id=CourseLocator(org="dummy", course="dummy", run="non_existent")
)
result = self._get_detail()
self.assertEqual(result["course_info"], {})
def test_preferences(self):
lang_pref = UserPreferenceFactory(
user=self.user,
key=LANGUAGE_KEY,
value="language pref test value"
)
UserPreferenceFactory(user=self.user, key="non_included_key")
result = self._get_detail()
self.assertEqual(
result["preferences"],
{
NOTIFICATION_PREF_KEY: self.notification_pref.value,
LANGUAGE_KEY: lang_pref.value,
}
)
# List view tests
def _make_list_request(self, page, page_size):
request = RequestFactory().get(
"dummy",
{"page": page, "page_size": page_size},
HTTP_X_EDX_API_KEY=settings.EDX_API_KEY
)
return self.list_view(request)
def _get_list(self, page=1, page_size=''):
response = self._make_list_request(page, page_size)
self.assertEqual(response.status_code, 200)
self.assertEqual(
set(response.data.keys()),
{"count", "next", "previous", "results"}
)
return response.data["results"]
def test_no_users(self):
UserPreference.objects.all().delete()
results = self._get_list()
self.assertEqual(len(results), 0)
def test_multiple_users(self):
other_user = UserFactory()
other_notification_pref = UserPreferenceFactory(
user=other_user,
key=NOTIFICATION_PREF_KEY,
value="other value"
)
self._set_up_course(is_course_cohorted=True, is_user_cohorted=True, is_moderator=False)
self._set_up_course(is_course_cohorted=False, is_user_cohorted=False, is_moderator=False)
# Users have different sets of enrollments
CourseEnrollmentFactory(user=other_user, course_id=self.courses[0].id)
result_map = {result["id"]: result for result in self._get_list()}
self.assertEqual(set(result_map.keys()), {self.user.id, other_user.id})
for user in [self.user, other_user]:
self._assert_basic_user_info_correct(user, result_map[user.id])
self.assertEqual(
result_map[self.user.id]["preferences"],
{NOTIFICATION_PREF_KEY: self.notification_pref.value}
)
self.assertEqual(
result_map[other_user.id]["preferences"],
{NOTIFICATION_PREF_KEY: other_notification_pref.value}
)
self.assertEqual(
result_map[self.user.id]["course_info"],
{
six.text_type(self.courses[0].id): {
"cohort_id": self.cohorts[0].id,
"see_all_cohorts": False,
},
six.text_type(self.courses[1].id): {
"cohort_id": None,
"see_all_cohorts": True,
},
}
)
self.assertEqual(
result_map[other_user.id]["course_info"],
{
six.text_type(self.courses[0].id): {
"cohort_id": None,
"see_all_cohorts": False,
},
}
)
@ddt.data(
3, # Factor of num of results
5, # Non-factor of num of results
12, # Num of results
15 # More than num of results
)
def test_pagination(self, page_size):
num_users = 12
users = [self.user]
while len(users) < num_users:
new_user = UserFactory()
users.append(new_user)
UserPreferenceFactory(user=new_user, key=NOTIFICATION_PREF_KEY)
num_pages = (num_users - 1) // page_size + 1
result_list = []
for i in range(1, num_pages + 1):
result_list.extend(self._get_list(page=i, page_size=page_size))
result_map = {result["id"]: result for result in result_list}
self.assertEqual(len(result_list), num_users)
for user in users:
self._assert_basic_user_info_correct(user, result_map[user.id])
self.assertEqual(
self._make_list_request(page=(num_pages + 1), page_size=page_size).status_code,
404
)
def test_db_access(self):
for _ in range(10):
new_user = UserFactory()
UserPreferenceFactory(user=new_user, key=NOTIFICATION_PREF_KEY)
# The number of queries is one for the users plus one for each prefetch
# in NotifierUsersViewSet (roles__permissions does one for each table).
with self.assertNumQueries(6):
self._get_list()

16
lms/djangoapps/discussion/notifier_api/urls.py
View File

@@ -1,16 +0,0 @@
"""
URLs for the notifier api app
"""
from django.conf.urls import include, url
from rest_framework import routers
from lms.djangoapps.discussion.notifier_api.views import NotifierUsersViewSet
notifier_api_router = routers.DefaultRouter()
notifier_api_router.register(r'users', NotifierUsersViewSet, basename="notifier_users")
urlpatterns = [
url(r'^v1/', include(notifier_api_router.urls)),
]

55
lms/djangoapps/discussion/notifier_api/views.py
View File

@@ -1,55 +0,0 @@
"""
Django views for the Notifier.
"""
from django.contrib.auth.models import User
from rest_framework import pagination
from rest_framework.response import Response
from rest_framework.viewsets import ReadOnlyModelViewSet
from lms.djangoapps.discussion.notification_prefs import NOTIFICATION_PREF_KEY
from lms.djangoapps.discussion.notifier_api.serializers import NotifierUserSerializer
from openedx.core.lib.api.permissions import ApiKeyHeaderPermission
class NotifierPaginator(pagination.PageNumberPagination):
"""
Paginator for the notifier API.
"""
page_size = 10
page_size_query_param = "page_size"
def get_paginated_response(self, data):
"""
Construct a response with pagination information.
"""
return Response({
'next': self.get_next_link(),
'previous': self.get_previous_link(),
'count': self.page.paginator.count,
'results': data
})
class NotifierUsersViewSet(ReadOnlyModelViewSet):
"""
An endpoint that the notifier can use to retrieve users who have enabled
daily forum digests, including all information that the notifier needs about
such users.
"""
permission_classes = (ApiKeyHeaderPermission,)
serializer_class = NotifierUserSerializer
pagination_class = NotifierPaginator
# See NotifierUserSerializer for notes about related tables
queryset = User.objects.filter(
preferences__key=NOTIFICATION_PREF_KEY
).select_related(
"profile"
).prefetch_related(
"preferences",
"courseenrollment_set",
"course_groups",
"roles__permissions"
)

3
lms/urls.py
View File

@@ -108,9 +108,6 @@ urlpatterns = [
url(r'^heartbeat', include('openedx.core.djangoapps.heartbeat.urls')),
url(r'^notifier_api/', include('lms.djangoapps.discussion.notifier_api.urls')),
url(r'^/api/notifier/', include('lms.djangoapps.discussion.notifier_api.urls')),
url(r'^i18n/', include('django.conf.urls.i18n')),
# Enrollment API RESTful endpoints

1
pavelib/prereqs.py
View File

@@ -197,7 +197,6 @@ PACKAGES_TO_UNINSTALL = [
"i18n-tools", # Because now it's called edx-i18n-tools
"moto", # Because we no longer use it and it conflicts with recent jsondiff versions
"python-saml", # Because python3-saml shares the same directory name
"pdfminer", # Replaced by pdfminer.six, which shares the same directory name
"pytest-faulthandler", # Because it was bundled into pytest
"djangorestframework-jwt", # Because now its called drf-jwt.
]