diff --git a/common/djangoapps/student/migrations/0022_indexing_in_courseenrollment.py b/common/djangoapps/student/migrations/0022_indexing_in_courseenrollment.py new file mode 100644 index 0000000000..8b313ff544 --- /dev/null +++ b/common/djangoapps/student/migrations/0022_indexing_in_courseenrollment.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.20 on 2019-06-24 19:01 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('student', '0021_historicalcourseenrollment'), + ] + + operations = [ + migrations.AddIndex( + model_name='courseenrollment', + index=models.Index(fields=[b'user', b'-created'], name='student_cou_user_id_b19dcd_idx'), + ), + ] diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 2a5fd00318..0fa401d5a2 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -31,7 +31,7 @@ from django.contrib.auth.signals import user_logged_in, user_logged_out from django.core.cache import cache from django.core.exceptions import MultipleObjectsReturned, ObjectDoesNotExist from django.db import IntegrityError, models -from django.db.models import Count, Q +from django.db.models import Count, Q, Index from django.db.models.signals import post_save, pre_save from django.db.utils import ProgrammingError from django.dispatch import receiver @@ -1147,7 +1147,8 @@ class CourseEnrollment(models.Model): MODE_CACHE_NAMESPACE = u'CourseEnrollment.mode_and_active' class Meta(object): - unique_together = (('user', 'course'),) + unique_together = (('user', 'course'), ) + indexes = [Index(fields=['user', '-created'])] ordering = ('user', 'course') def __init__(self, *args, **kwargs): @@ -1610,7 +1611,7 @@ class CourseEnrollment(models.Model): return cls.objects.filter(user=user, is_active=1).select_related('user') @classmethod - def enrollments_for_user_with_overviews_preload(cls, user): # pylint: disable=invalid-name + def enrollments_for_user_with_overviews_preload(cls, user, courses_limit=None): # pylint: disable=invalid-name """ List of user's CourseEnrollments, CourseOverviews preloaded if possible. @@ -1625,7 +1626,12 @@ class CourseEnrollment(models.Model): The name of this method is long, but was the end result of hashing out a number of alternatives, so pylint can stuff it (disable=invalid-name) """ - enrollments = list(cls.enrollments_for_user(user)) + + if courses_limit: + enrollments = cls.enrollments_for_user(user).order_by('-created')[:courses_limit] + else: + enrollments = cls.enrollments_for_user(user) + overviews = CourseOverview.get_from_ids_if_exists( enrollment.course_id for enrollment in enrollments ) @@ -2400,6 +2406,10 @@ def enforce_single_login(sender, request, user, signal, **kwargs): # pylint: class DashboardConfiguration(ConfigurationModel): """ + Note: + This model is deprecated and we should not be adding new content to it. + We will eventually migrate this one entry to a django setting as well. + Dashboard Configuration settings. Includes configuration options for the dashboard, which impact behavior and rendering for the application. diff --git a/common/djangoapps/student/tests/test_course_listing.py b/common/djangoapps/student/tests/test_course_listing.py index 90405cb480..be9b13a8fb 100644 --- a/common/djangoapps/student/tests/test_course_listing.py +++ b/common/djangoapps/student/tests/test_course_listing.py @@ -13,7 +13,7 @@ from django.test.client import Client from milestones.tests.utils import MilestonesTestCaseMixin from openedx.core.djangoapps.content.course_overviews.models import CourseOverview -from student.models import CourseEnrollment +from student.models import CourseEnrollment, DashboardConfiguration from student.roles import GlobalStaff from student.tests.factories import UserFactory from student.views import get_course_enrollments @@ -85,6 +85,23 @@ class TestCourseListing(ModuleStoreTestCase, MilestonesTestCaseMixin): courses_list = list(get_course_enrollments(self.student, None, [])) self.assertEqual(len(courses_list), 0) + @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') + def test_get_limited_number_of_courses_using_config(self): + course_location = self.store.make_course_key('Org0', 'Course0', 'Run0') + self._create_course_with_access_groups(course_location) + + course_location = self.store.make_course_key('Org1', 'Course1', 'Run1') + self._create_course_with_access_groups(course_location) + + # get dashboard + courses_list = list(get_course_enrollments(self.student, None, [])) + self.assertEqual(len(courses_list), 2) + + with self.settings(DASHBOARD_COURSE_LIMIT=1): + course_limit = settings.DASHBOARD_COURSE_LIMIT + courses_list = list(get_course_enrollments(self.student, None, [], course_limit)) + self.assertEqual(len(courses_list), 1) + def test_errored_course_regular_access(self): """ Test the course list for regular staff when get_course returns an ErrorDescriptor diff --git a/common/djangoapps/student/tests/test_views.py b/common/djangoapps/student/tests/test_views.py index 42e34a54e9..8da4372b04 100644 --- a/common/djangoapps/student/tests/test_views.py +++ b/common/djangoapps/student/tests/test_views.py @@ -573,6 +573,23 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, self.assertIn('You are not enrolled in any courses yet.', response.content) self.assertIn(empty_dashboard_message, response.content) + @patch('django.conf.settings.DASHBOARD_COURSE_LIMIT', 1) + def test_course_limit_on_dashboard(self): + course = CourseFactory.create() + CourseEnrollmentFactory( + user=self.user, + course_id=course.id + ) + + course_v1 = CourseFactory.create() + CourseEnrollmentFactory( + user=self.user, + course_id=course_v1.id + ) + + response = self.client.get(reverse('dashboard')) + self.assertIn('1 results successfully populated', response.content) + @staticmethod def _remove_whitespace_from_html_string(html): return ''.join(html.split()) diff --git a/common/djangoapps/student/views/dashboard.py b/common/djangoapps/student/views/dashboard.py index 398e3c5e8d..9a8d844e0a 100644 --- a/common/djangoapps/student/views/dashboard.py +++ b/common/djangoapps/student/views/dashboard.py @@ -198,7 +198,7 @@ def _create_recent_enrollment_message(course_enrollments, course_modes): # pyli ) -def get_course_enrollments(user, org_whitelist, org_blacklist): +def get_course_enrollments(user, org_whitelist, org_blacklist, course_limit=None): """ Given a user, return a filtered set of his or her course enrollments. @@ -206,12 +206,13 @@ def get_course_enrollments(user, org_whitelist, org_blacklist): user (User): the user in question. org_whitelist (list[str]): If not None, ONLY courses of these orgs will be returned. org_blacklist (list[str]): Courses of these orgs will be excluded. + course_limit: Number courses to load in dashboard if set to None then all the courses would be load. Returns: generator[CourseEnrollment]: a sequence of enrollments to be displayed on the user's dashboard. """ - for enrollment in CourseEnrollment.enrollments_for_user_with_overviews_preload(user): + for enrollment in CourseEnrollment.enrollments_for_user_with_overviews_preload(user, course_limit): # If the course is missing or broken, log an error and skip it. course_overview = enrollment.course_overview @@ -526,6 +527,29 @@ def _credit_statuses(user, course_enrollments): return statuses +def show_load_all_courses_link(user, course_limit, course_enrollments): + """ + By default dashboard will show limited courses based on the course limit + set in configuration. + + A link would be provided provided at the bottom to load all the courses if there are any courses. + """ + + if course_limit is None: + return False + + total_enrollments = CourseEnrollment.enrollments_for_user(user).count() + return len(course_enrollments) < total_enrollments + + +def get_dashboard_course_limit(): + """ + get course limit from configuration + """ + course_limit = getattr(settings, 'DASHBOARD_COURSE_LIMIT', None) + return course_limit + + @login_required @ensure_csrf_cookie @add_maintenance_banner @@ -534,7 +558,9 @@ def student_dashboard(request): Provides the LMS dashboard view TODO: This is lms specific and does not belong in common code. - + Note: + To load the all courses set course_limit=None as parameter in GET. If its not None then default course + limit will be used that is set in configuration Arguments: request: The request object. @@ -567,9 +593,12 @@ def student_dashboard(request): 'EMPTY_DASHBOARD_MESSAGE', None ) + disable_course_limit = request and 'course_limit' in request.GET + course_limit = get_dashboard_course_limit() if not disable_course_limit else None + # Get the org whitelist or the org blacklist for the current site site_org_whitelist, site_org_blacklist = get_org_black_and_whitelist_for_site() - course_enrollments = list(get_course_enrollments(user, site_org_whitelist, site_org_blacklist)) + course_enrollments = list(get_course_enrollments(user, site_org_whitelist, site_org_blacklist, course_limit)) # Get the entitlements for the user and a mapping to all available sessions for that entitlement # If an entitlement has no available sessions, pass through a mock course overview object @@ -854,6 +883,7 @@ def student_dashboard(request): 'empty_dashboard_message': empty_dashboard_message, 'recovery_email_message': recovery_email_message, 'recovery_email_activation_message': recovery_email_activation_message, + 'show_load_all_courses_link': show_load_all_courses_link(user, course_limit, course_enrollments), # TODO START: clean up as part of REVEM-199 (START) 'course_info': get_dashboard_course_info(user, course_enrollments), # TODO START: clean up as part of REVEM-199 (END) diff --git a/lms/envs/production.py b/lms/envs/production.py index b07e245ba9..ef260dad23 100644 --- a/lms/envs/production.py +++ b/lms/envs/production.py @@ -1132,3 +1132,7 @@ derive_settings(__name__) ######################### Overriding custom enrollment roles ############### MANUAL_ENROLLMENT_ROLE_CHOICES = ENV_TOKENS.get('MANUAL_ENROLLMENT_ROLE_CHOICES', MANUAL_ENROLLMENT_ROLE_CHOICES) + +########################## limiting dashboard courses ###################### + +DASHBOARD_COURSE_LIMIT = ENV_TOKENS.get('DASHBOARD_COURSE_LIMIT', None) diff --git a/lms/envs/test.py b/lms/envs/test.py index 504cb49d83..2edc04df50 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -650,3 +650,7 @@ PDF_RECEIPT_TAX_ID_LABEL = 'Tax ID' PROFILE_MICROFRONTEND_URL = "http://profile-mfe/abc/" ORDER_HISTORY_MICROFRONTEND_URL = "http://order-history-mfe/" ACCOUNT_MICROFRONTEND_URL = "http://account-mfe/" + +########################## limiting dashboard courses ###################### + +DASHBOARD_COURSE_LIMIT = 250 diff --git a/lms/templates/dashboard.html b/lms/templates/dashboard.html index a6db673583..6451c814b0 100644 --- a/lms/templates/dashboard.html +++ b/lms/templates/dashboard.html @@ -206,7 +206,13 @@ from student.models import CourseEnrollment %> <%include file='dashboard/_dashboard_course_listing.html' args='course_overview=course_overview, course_card_index=dashboard_index, enrollment=enrollment, is_unfulfilled_entitlement=is_unfulfilled_entitlement, is_fulfilled_entitlement=is_fulfilled_entitlement, entitlement=entitlement, entitlement_session=entitlement_session, entitlement_available_sessions=entitlement_available_sessions, entitlement_expiration_date=entitlement_expiration_date, entitlement_expired_at=entitlement_expired_at, show_courseware_link=show_courseware_link, cert_status=cert_status, can_refund_entitlement=can_refund_entitlement, can_unenroll=can_unenroll, credit_status=credit_status, show_email_settings=show_email_settings, course_mode_info=course_mode_info, is_paid_course=is_paid_course, is_course_blocked=is_course_blocked, verification_status=course_verification_status, course_requirements=course_requirements, dashboard_index=dashboard_index, share_settings=share_settings, user=user, related_programs=related_programs, display_course_modes_on_dashboard=display_course_modes_on_dashboard, show_consent_link=show_consent_link, enterprise_customer_name=enterprise_customer_name, resume_button_url=resume_button_url' /> % endfor - + % if show_load_all_courses_link: +
+ ${len(course_enrollments)} ${_("results successfully populated,")} + + ${_("Click to load all enrolled courses")} + + % endif % else:
diff --git a/themes/edx.org/lms/templates/dashboard.html b/themes/edx.org/lms/templates/dashboard.html index 021ab1f3cb..c582fc49e9 100644 --- a/themes/edx.org/lms/templates/dashboard.html +++ b/themes/edx.org/lms/templates/dashboard.html @@ -203,6 +203,13 @@ from student.models import CourseEnrollment %> <%include file='dashboard/_dashboard_course_listing.html' args='course_overview=course_overview, course_card_index=dashboard_index, enrollment=enrollment, is_unfulfilled_entitlement=is_unfulfilled_entitlement, is_fulfilled_entitlement=is_fulfilled_entitlement, entitlement=entitlement, entitlement_session=entitlement_session, entitlement_available_sessions=entitlement_available_sessions, entitlement_expiration_date=entitlement_expiration_date, entitlement_expired_at=entitlement_expired_at, show_courseware_link=show_courseware_link, cert_status=cert_status, can_refund_entitlement=can_refund_entitlement, can_unenroll=can_unenroll, credit_status=credit_status, show_email_settings=show_email_settings, course_mode_info=course_mode_info, is_paid_course=is_paid_course, is_course_blocked=is_course_blocked, verification_status=course_verification_status, course_requirements=course_requirements, dashboard_index=dashboard_index, share_settings=share_settings, user=user, related_programs=related_programs, display_course_modes_on_dashboard=display_course_modes_on_dashboard, show_consent_link=show_consent_link, enterprise_customer_name=enterprise_customer_name, resume_button_url=resume_button_url' /> % endfor + % if show_load_all_courses_link: +
+ ${len(course_enrollments)} ${_("results successfully populated,")} + + ${_("Click to load all enrolled courses")} + + % endif % else: