mattdrayer/cypress-rebase-api: Fixed broken api_manager tests

* Also fixed failing gradebook test
* Addressed OpenCraft PR feedback (Round 1)
* Relocate User API url declarations

lms/djangoapps/api_manager/courses/views.py

@@ -5,7 +5,7 @@ import logging
 import itertools
 from lxml import etree
 from StringIO import StringIO
-from dateutil.relativedelta import relativedelta
+from datetime import timedelta
 
 from django.conf import settings
 from django.contrib.auth.models import Group, User
@@ -1062,7 +1062,7 @@ class CoursesUsersList(SecureAPIView):
             return Response({}, status=status.HTTP_404_NOT_FOUND)
         course_key = get_course_key(course_id)
         # Get a list of all enrolled students
-        users = CourseEnrollment.users_enrolled_in(course_key)
+        users = CourseEnrollment.objects.users_enrolled_in(course_key)
         upper_bound = getattr(settings, 'API_LOOKUP_UPPER_BOUND', 100)
         if orgs:
             orgs = orgs.split(",")[:upper_bound]
@@ -1315,7 +1315,7 @@ class CourseContentUsersList(SecureAPIView):
 
         lookup_group_ids = relationships.values_list('group_profile', flat=True)
         users = User.objects.filter(groups__id__in=lookup_group_ids)
-        enrolled_users = CourseEnrollment.users_enrolled_in(course_key).filter(groups__id__in=lookup_group_ids)
+        enrolled_users = CourseEnrollment.objects.users_enrolled_in(course_key).filter(groups__id__in=lookup_group_ids)
         if enrolled in ['True', 'true']:
             queryset = enrolled_users
         else:
@@ -1446,7 +1446,6 @@ class CoursesMetricsGradesList(SecureListAPIView):
                                                    user__courseenrollment__is_active=True,
                                                    user__courseenrollment__course_id__exact=course_key)\
             .exclude(user__in=exclude_users)
-
         upper_bound = getattr(settings, 'API_LOOKUP_UPPER_BOUND', 200)
         user_ids = self.request.QUERY_PARAMS.get('user_id', None)
         if user_ids:
@@ -1515,7 +1514,7 @@ class CoursesMetrics(SecureAPIView):
         course_descriptor, course_key, course_content = get_course(request, request.user, course_id)
         slash_course_id = get_course_key(course_id, slashseparated=True)
         exclude_users = get_aggregate_exclusion_user_ids(course_key)
-        users_enrolled_qs = CourseEnrollment.users_enrolled_in(course_key).exclude(id__in=exclude_users)
+        users_enrolled_qs = CourseEnrollment.objects.users_enrolled_in(course_key).exclude(id__in=exclude_users)
         organization = request.QUERY_PARAMS.get('organization', None)
         org_ids = None
         if organization:
@@ -1615,23 +1614,36 @@ class CoursesTimeSeriesMetrics(SecureAPIView):
         total_enrolled = enrolled_qs.filter(created__lt=start_dt).count()
         total_started_count = users_started_qs.filter(created__lt=start_dt).aggregate(Count('user', distinct=True))
         total_started = total_started_count['user__count'] or 0
-        enrolled_series = get_time_series_data(enrolled_qs, start_dt, end_dt, interval=interval,
-                                               date_field='created', aggregate=Count('id'))
-        started_series = get_time_series_data(users_started_qs, start_dt, end_dt, interval=interval,
-                                              date_field='created', date_field_model=StudentProgress,
-                                              aggregate=Count('user'))
-        completed_series = get_time_series_data(grades_complete_qs, start_dt, end_dt, interval=interval,
-                                                date_field='modified', date_field_model=StudentGradebook,
-                                                aggregate=Count('id'))
-        modules_completed_series = get_time_series_data(modules_completed_qs, start_dt, end_dt, interval=interval,
-                                                        date_field='created', date_field_model=CourseModuleCompletion,
-                                                        aggregate=Count('id'))
+        enrolled_series = get_time_series_data(
+            enrolled_qs, start_dt, end_dt, interval=interval,
+            date_field='created', date_field_model=CourseEnrollment,
+            aggregate=Count('id')
+        )
+        started_series = get_time_series_data(
+            users_started_qs, start_dt, end_dt, interval=interval,
+            date_field='created', date_field_model=StudentProgress,
+            aggregate=Count('user')
+        )
+        completed_series = get_time_series_data(
+            grades_complete_qs, start_dt, end_dt, interval=interval,
+            date_field='modified', date_field_model=StudentGradebook,
+            aggregate=Count('id')
+        )
+        modules_completed_series = get_time_series_data(
+            modules_completed_qs, start_dt, end_dt, interval=interval,
+            date_field='created', date_field_model=CourseModuleCompletion,
+            aggregate=Count('id')
+        )
+
         # active users are those who accessed course in last 24 hours
-        start_dt = start_dt + relativedelta(hours=-24)
-        end_dt = end_dt + relativedelta(hours=-24)
-        active_users_series = get_time_series_data(active_users_qs, start_dt, end_dt, interval=interval,
-                                                   date_field='modified', date_field_model=StudentModule,
-                                                   aggregate=Count('student', distinct=True))
+        start_dt = start_dt - timedelta(hours=24)
+        end_dt = end_dt - timedelta(hours=24)
+        active_users_series = get_time_series_data(
+            active_users_qs, start_dt, end_dt, interval=interval,
+            date_field='modified', date_field_model=StudentModule,
+            aggregate=Count('student', distinct=True)
+        )
+
         not_started_series = []
         for enrolled, started in zip(enrolled_series, started_series):
             not_started_series.append((started[0], (total_enrolled + enrolled[1]) - (total_started + started[1])))
@@ -1646,6 +1658,7 @@ class CoursesTimeSeriesMetrics(SecureAPIView):
             'users_enrolled': enrolled_series,
             'active_users': active_users_series
         }
+
         return Response(data, status=status.HTTP_200_OK)
 
 
@@ -1740,7 +1753,7 @@ class CoursesMetricsCompletionsLeadersList(SecureAPIView):
                 completion_percentage = min(100 * (user_completions/total_possible_completions), 100)
             data['completions'] = completion_percentage
 
-        total_users_qs = CourseEnrollment.users_enrolled_in(course_key).exclude(id__in=exclude_users)
+        total_users_qs = CourseEnrollment.objects.users_enrolled_in(course_key).exclude(id__in=exclude_users)
         if orgs_filter:
             total_users_qs = total_users_qs.filter(organizations__in=orgs_filter)
         total_users = total_users_qs.count()
@@ -1809,7 +1822,7 @@ class CoursesMetricsSocial(SecureListAPIView):
             for user_id in exclude_users:
                 if str(user_id) in data:
                     del data[str(user_id)]
-            enrollment_qs = CourseEnrollment.users_enrolled_in(course_key).filter(is_active=True)\
+            enrollment_qs = CourseEnrollment.objects.users_enrolled_in(course_key).filter(is_active=True)\
                 .exclude(id__in=exclude_users)
             actual_data = {}
             if organization:
@@ -1829,7 +1842,6 @@ class CoursesMetricsSocial(SecureListAPIView):
                 "err_msg": str(e)
             }
             http_status = status.HTTP_500_INTERNAL_SERVER_ERROR
-
         return Response(data, http_status)
 
 
@@ -1853,7 +1865,7 @@ class CoursesMetricsCities(SecureListAPIView):
             raise Http404
         course_key = get_course_key(course_id)
         exclude_users = get_aggregate_exclusion_user_ids(course_key)
-        queryset = CourseEnrollment.users_enrolled_in(course_key).exclude(id__in=exclude_users)
+        queryset = CourseEnrollment.objects.users_enrolled_in(course_key).exclude(id__in=exclude_users)
         if city:
             city = city.split(',')[:upper_bound]
             q_list = [Q(profile__city__iexact=item.strip()) for item in city]

lms/djangoapps/api_manager/management/commands/tests/test_migrate_courseids.py

@@ -65,14 +65,14 @@ class MigrateCourseIdsTests(ModuleStoreTestCase):
         Test the data migration
         """
         # Set up the data to be migrated
-        user = User.objects.create(email='testuser@edx.org', username='testuser', password='testpassword', is_active=True)
+        user = User.objects.create(email='testuser@edx.org', username='testuser_tmc', password='testpassword', is_active=True)
         group = Group.objects.create(name='Test Group')
         group_profile = api_models.GroupProfile.objects.create(group=group)
         course_group = api_models.CourseGroupRelationship.objects.create(course_id=self.old_style_course_id, group=group)
         course_content_group = api_models.CourseContentGroupRelationship.objects.create(course_id=self.old_style_course_id, content_id=self.old_style_content_id, group_profile=group_profile)
         course_module_completion = CourseModuleCompletion.objects.create(user=user, course_id=self.old_style_course_id, content_id=self.old_style_content_id)
 
-        user2 = User.objects.create(email='testuser2@edx.org', username='testuser2', password='testpassword2', is_active=True)
+        user2 = User.objects.create(email='testuser2@edx.org', username='testuser_tmc_2', password='testpassword2', is_active=True)
         group2 = Group.objects.create(name='Test Group2')
         group_profile2 = api_models.GroupProfile.objects.create(group=group2)
         course_group2 = api_models.CourseGroupRelationship.objects.create(course_id=self.new_style_course_id2, group=group2)

lms/djangoapps/api_manager/management/commands/tests/test_migrate_courseids_v2.py

@@ -43,14 +43,14 @@ class MigrateCourseIdsTests(ModuleStoreTestCase):
         Test the data migration
         """
         # Set up the data to be migrated
-        user = User.objects.create(email='testuser@edx.org', username='testuser', password='testpassword', is_active=True)
+        user = User.objects.create(email='testuser@edx.org', username='testuser_tmcv2', password='testpassword', is_active=True)
         group = Group.objects.create(name='Test Group')
         group_profile = api_models.GroupProfile.objects.create(group=group)
         course_group = api_models.CourseGroupRelationship.objects.create(course_id=self.bad_style_course_id, group=group)
         course_content_group = api_models.CourseContentGroupRelationship.objects.create(course_id=self.bad_style_course_id, content_id=self.bad_style_content_id, group_profile=group_profile)
         course_module_completion = CourseModuleCompletion.objects.create(user=user, course_id=self.bad_style_course_id, content_id=self.bad_style_content_id)
 
-        user2 = User.objects.create(email='testuser2@edx.org', username='testuser2', password='testpassword2', is_active=True)
+        user2 = User.objects.create(email='testuser2@edx.org', username='testuser_tmcv2_2', password='testpassword2', is_active=True)
         group2 = Group.objects.create(name='Test Group2')
         group_profile2 = api_models.GroupProfile.objects.create(group=group2)
         course_group2 = api_models.CourseGroupRelationship.objects.create(course_id=self.bad_style_course_id2, group=group2)

lms/djangoapps/api_manager/management/commands/tests/test_migrate_stage_prefix.py

@@ -37,10 +37,10 @@ class MigrateCourseIdsTests(ModuleStoreTestCase):
         Test the data migration
         """
         # Set up the data to be migrated
-        user = User.objects.create(email='testuser@edx.org', username='testuser', password='testpassword', is_active=True)
+        user = User.objects.create(email='testuser@edx.org', username='testuser_tmsp', password='testpassword', is_active=True)
         course_module_completion = CourseModuleCompletion.objects.create(user=user, course_id=self.good_style_course_id, content_id=self.good_style_content_id, stage=self.bad_style_stage)
 
-        user2 = User.objects.create(email='testuser2@edx.org', username='testuser2', password='testpassword2', is_active=True)
+        user2 = User.objects.create(email='testuser2@edx.org', username='testuser_tmsp_2', password='testpassword2', is_active=True)
         course_module_completion2 = CourseModuleCompletion.objects.create(user=user2, course_id=self.good_style_course_id2, content_id=self.good_style_content_id2, stage=self.bad_style_stage2)
 
         # Run the data migration

lms/djangoapps/api_manager/test_receivers.py

@@ -40,7 +40,7 @@ class ApiManagerReceiversTests(ModuleStoreTestCase):
             display_name="Overview"
         )
 
-        self.user = User.objects.create(email='testuser@edx.org', username='testuser', password='testpassword', is_active=True)
+        self.user = User.objects.create(email='testuser@edx.org', username='testuser_amrt', password='testpassword', is_active=True)
 
     def test_receiver_on_course_deleted(self):
         """

lms/djangoapps/api_manager/users/tests.py

@@ -3,7 +3,7 @@
 
 """
 Run these tests @ Devstack:
-    rake fasttest_lms[common/djangoapps/api_manager/tests/test_user_views.py]
+    paver test_system -s lms --fasttest --fail_fast --verbose --test_id=lms/djangoapps/api_manager/users
 """
 from datetime import datetime
 from dateutil.relativedelta import relativedelta
@@ -95,7 +95,7 @@ class UsersApiTests(ModuleStoreTestCase):
         return module
 
     def setUp(self):
-        super(UsersApiTests, self).setUp()
+        super(UsersApiTests, self).setUp(create_user=False)
         self.test_server_prefix = 'https://testserver'
         self.test_username = str(uuid.uuid4())
         self.test_password = 'Test.Me64!'
@@ -221,9 +221,9 @@ class UsersApiTests(ModuleStoreTestCase):
         self.assertEqual(response.status_code, 200)
 
         # ID 1 is a Robot Test (results[0]) so the users list starts with ID 2 (results[1])
-        self.assertEqual(response.data['results'][1]['is_staff'], False)        
-        self.assertEqual(response.data['results'][3]['is_staff'], False)        
-        self.assertEqual(response.data['results'][5]['is_staff'], False)        
+        self.assertEqual(response.data['results'][1]['is_staff'], False)
+        self.assertEqual(response.data['results'][3]['is_staff'], False)
+        self.assertEqual(response.data['results'][5]['is_staff'], False)
         self.assertTrue(response.data['results'][2]['is_staff'])
         self.assertTrue(response.data['results'][4]['is_staff'])
         self.assertTrue(response.data['results'][6]['is_staff'])
@@ -311,8 +311,8 @@ class UsersApiTests(ModuleStoreTestCase):
         # create a 7 new users
         for i in xrange(1, 8):
             data = {
-                'email': 'test{}@example.com'.format(i),
-                'username': 'test_user{}'.format(i),
+                'email': 'test_orgfilter{}@example.com'.format(i),
+                'username': 'test_user_orgfilter{}'.format(i),
                 'password': self.test_password,
                 'first_name': 'John{}'.format(i),
                 'last_name': 'Doe{}'.format(i)
@@ -393,7 +393,7 @@ class UsersApiTests(ModuleStoreTestCase):
         confirm_uri = self.test_server_prefix + \
             test_uri + '/' + str(response.data['id'])
         user = User.objects.get(id=response.data['id'])
-        self.assertIsNotNone(UserPreference.get_preference(user, NOTIFICATION_PREF_KEY))
+        self.assertIsNotNone(UserPreference.get_value(user, NOTIFICATION_PREF_KEY))
 
     def test_user_detail_get(self):
         test_uri = self.users_base_uri
@@ -1452,7 +1452,6 @@ class UsersApiTests(ModuleStoreTestCase):
         module.system.publish(module, 'grade', grade_dict)
 
         test_uri = '{}/{}/courses/{}/grades'.format(self.users_base_uri, user_id, unicode(course.id))
-
         response = self.do_get(test_uri)
         self.assertEqual(response.status_code, 200)
 
@@ -1476,17 +1475,16 @@ class UsersApiTests(ModuleStoreTestCase):
         grading_policy = response.data['grading_policy']
         self.assertGreater(len(grading_policy['GRADER']), 0)
         self.assertIsNotNone(grading_policy['GRADE_CUTOFFS'])
-
-        self.assertEqual(response.data['current_grade'], 0.74)
-        self.assertEqual(response.data['proforma_grade'], 0.9174999999999999)
+        self.assertAlmostEqual(response.data['current_grade'], 0.74, 1)
+        self.assertAlmostEqual(response.data['proforma_grade'], 0.9375, 1)
 
         test_uri = '{}/{}/courses/grades'.format(self.users_base_uri, user_id)
 
         response = self.do_get(test_uri)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data[0]['course_id'], unicode(course.id))
-        self.assertEqual(response.data[0]['current_grade'], 0.74)
-        self.assertEqual(response.data[0]['proforma_grade'], 0.9174999999999999)
+        self.assertAlmostEqual(response.data[0]['current_grade'], 0.74, 1)
+        self.assertAlmostEqual(response.data[0]['proforma_grade'], 0.9375, 1)
         self.assertEqual(response.data[0]['complete_status'], False)
 
     def is_user_profile_created_updated(self, response, data):

lms/djangoapps/api_manager/users/urls.py

@@ -30,6 +30,7 @@ urlpatterns = patterns(
     url(r'^(?P<user_id>[a-zA-Z0-9]+)/roles/(?P<role>[a-z_]+)/courses/{0}$'.format(COURSE_ID_PATTERN), users_views.UsersRolesCoursesDetail.as_view(), name='users-roles-courses-detail'),
     url(r'^(?P<user_id>[a-zA-Z0-9]+)/roles/*$', users_views.UsersRolesList.as_view(), name='users-roles-list'),
     url(r'^(?P<user_id>[a-zA-Z0-9]+)/workgroups/$', users_views.UsersWorkgroupsList.as_view(), name='users-workgroups-list'),
+    url(r'^(?P<user_id>[a-zA-Z0-9]+)/notifications/(?P<msg_id>[0-9]+)/$', users_views.UsersNotificationsDetail.as_view(), name='users-notifications-detail'),
     url(r'^(?P<user_id>[a-zA-Z0-9]+)$', users_views.UsersDetail.as_view(), name='apimgr-users-detail'),
     url(r'/*$^', users_views.UsersList.as_view(), name='apimgr-users-list'),
 )

lms/djangoapps/api_manager/users/views.py

@@ -19,13 +19,6 @@ from rest_framework.response import Response
 
 from courseware import grades, module_render
 from courseware.model_data import FieldDataCache
-from openedx.core.djangoapps.course_groups.models import CourseUserGroup, CourseCohort
-from openedx.core.djangoapps.course_groups.cohorts import (
-    get_cohort_by_name,
-    add_cohort,
-    add_user_to_cohort,
-    remove_user_from_cohort
-)
 from django_comment_common.models import Role, FORUM_ROLE_MODERATOR
 from gradebook.models import StudentGradebook
 from instructor.access import revoke_access, update_forum_role
@@ -36,10 +29,17 @@ from notification_prefs.views import enable_notifications
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.keys import UsageKey, CourseKey
 from opaque_keys.edx.locations import Location, SlashSeparatedCourseKey
+from openedx.core.djangoapps.course_groups.models import CourseUserGroup, CourseCohort
+from openedx.core.djangoapps.course_groups.cohorts import (
+    get_cohort_by_name,
+    add_cohort,
+    add_user_to_cohort,
+    remove_user_from_cohort
+)
+from openedx.core.djangoapps.user_api.models import UserPreference
 from openedx.core.djangoapps.user_api.preferences.api import set_user_preference
 from student.models import CourseEnrollment, PasswordHistory, UserProfile
 from student.roles import CourseAccessRole, CourseInstructorRole, CourseObserverRole, CourseStaffRole, CourseAssistantRole, UserBasedRole
-from openedx.core.djangoapps.user_api.models import UserPreference
 from util.bad_request_rate_limiter import BadRequestRateLimiter
 from util.password_policy_validators import (
     validate_password_length, validate_password_complexity,
@@ -57,7 +57,6 @@ from projects.serializers import BasicWorkgroupSerializer
 from .serializers import UserSerializer, UserCountByCitySerializer, UserRolesSerializer
 
 
-
 log = logging.getLogger(__name__)
 AUDIT_LOG = logging.getLogger("audit")
 
@@ -984,7 +983,6 @@ class UsersCoursesGradesDetail(SecureAPIView):
         """
         GET /api/users/{user_id}/courses/{course_id}/grades
         """
-
         # The pre-fetching of groups is done to make auth checks not require an
         # additional DB lookup (this kills the Progress page in particular).
         try:

lms/djangoapps/courseware/grades.py

@@ -317,19 +317,19 @@ def grade_for_percentage(grade_cutoffs, percentage):
 
 
 @transaction.commit_manually
-def progress_summary(student, request, course):
+def progress_summary(student, request, course, locators_as_strings=False):
     """
     Wraps "_progress_summary" with the manual_transaction context manager just
     in case there are unanticipated errors.
     """
     with manual_transaction():
-        return _progress_summary(student, request, course)
+        return _progress_summary(student, request, course, locators_as_strings)
 
 
 # TODO: This method is not very good. It was written in the old course style and
 # then converted over and performance is not good. Once the progress page is redesigned
 # to not have the progress summary this method should be deleted (so it won't be copied).
-def _progress_summary(student, request, course):
+def _progress_summary(student, request, course, locators_as_strings=False):
     """
     Unwrapped version of "progress_summary".
 
@@ -398,13 +398,15 @@ def _progress_summary(student, request, course):
                     if correct is None and total is None:
                         continue
 
+                    module_locator = unicode(module_descriptor.location) if locators_as_strings else module_descriptor.location
+
                     scores.append(
                         Score(
                             correct,
                             total,
                             graded,
                             module_descriptor.display_name_with_default,
-                            module_descriptor.location,
+                            module_locator,
                             due,
                         )
                     )

lms/djangoapps/gradebook/management/commands/tests/test_migrate_gradebook_courseids_v2.py

@@ -38,10 +38,10 @@ class MigrateCourseIdsTests(ModuleStoreTestCase):
         Test the data migration
         """
         # Set up the data to be migrated
-        user = User.objects.create(email='testuser@edx.org', username='testuser', password='testpassword', is_active=True)
+        user = User.objects.create(email='testuser@edx.org', username='testuser_courseids', password='testpassword', is_active=True)
         gradebook_entry = gradebook_models.StudentGradebook.objects.create(user=user, course_id=self.bad_style_course_id, grade=0.85, proforma_grade=0.74)
 
-        user2 = User.objects.create(email='testuser2@edx.org', username='testuser2', password='testpassword2', is_active=True)
+        user2 = User.objects.create(email='testuser2@edx.org', username='testuser_courseids2', password='testpassword2', is_active=True)
         gradebook_entry2 = gradebook_models.StudentGradebook.objects.create(user=user2, course_id=self.bad_style_course_id2, grade=0.95, proforma_grade=0.64)
 
         # Run the data migration

lms/djangoapps/gradebook/receivers.py

@@ -38,7 +38,7 @@ def on_score_changed(sender, **kwargs):
     course_descriptor = get_course(course_key, depth=None)
     request = RequestMockWithoutMiddleware().get('/')
     request.user = user
-    progress_summary = grades.progress_summary(user, request, course_descriptor)
+    progress_summary = grades.progress_summary(user, request, course_descriptor, locators_as_strings=True)
     grade_summary = grades.grade(user, request, course_descriptor)
     grading_policy = course_descriptor.grading_policy
     grade = grade_summary['percent']

lms/urls.py

@@ -60,10 +60,6 @@ urlpatterns = (
 
     url(r'^heartbeat$', include('heartbeat.urls')),
 
-    # Note: these are older versions of the User API that will eventually be
-    # subsumed by api/user listed below.
-    url(r'^user_api/', include('openedx.core.djangoapps.user_api.legacy_urls')),
-
     url(r'^notifier_api/', include('notifier_api.urls')),
 
     url(r'^i18n/', include('django.conf.urls.i18n')),
@@ -80,9 +76,6 @@ urlpatterns = (
     # Course content API
     url(r'^api/course_structure/', include('course_structure_api.urls', namespace='course_structure_api')),
 
-    # User API endpoints
-    url(r'^api/user/', include('openedx.core.djangoapps.user_api.urls')),
-
     # Profile Images API endpoints
     url(r'^api/profile_images/', include('openedx.core.djangoapps.profile_images.urls')),
 
@@ -128,6 +121,19 @@ if settings.FEATURES["API"]:
         url(r'^api/server/', include('api_manager.urls')),
     )
 
+# OPEN EDX USER API
+# mattdrayer: Please note that the user_api declaration must follow
+# the server api declaration.  When declared ahead of the server api
+# the user_api will oddly begin to return server-oriented user URIs
+# At this time I'm not sure why this seems to be a one-way scenario.
+urlpatterns += (
+    url(r'^user_api/', include('openedx.core.djangoapps.user_api.legacy_urls')),
+)
+urlpatterns += (
+    # User API endpoints
+    url(r'^api/user/', include('openedx.core.djangoapps.user_api.urls')),
+)
+
 # if settings.FEATURES.get("MULTIPLE_ENROLLMENT_ROLES"):
 urlpatterns += (
     # TODO Namespace these!