From 2d369490dd6e5b6cc0214d8bec787e04ce0d26d9 Mon Sep 17 00:00:00 2001
From: adeelehsan <adeel.ehsan@arbisoft.com>
Date: Tue, 7 May 2019 14:32:24 +0500
Subject: [PATCH] Flag added to load limited courses on Dashboard
Dashboard is set to load 250 courses instead of
all the courses. A flag is also added to change
the number the courses to load.
PROD-204
---
.../0022_indexing_in_courseenrollment.py | 19 ++++++++++
common/djangoapps/student/models.py | 18 +++++++--
.../student/tests/test_course_listing.py | 19 +++++++++-
common/djangoapps/student/tests/test_views.py | 17 +++++++++
common/djangoapps/student/views/dashboard.py | 38 +++++++++++++++++--
lms/envs/production.py | 4 ++
lms/envs/test.py | 4 ++
lms/templates/dashboard.html | 8 +++-
themes/edx.org/lms/templates/dashboard.html | 7 ++++
9 files changed, 124 insertions(+), 10 deletions(-)
create mode 100644 common/djangoapps/student/migrations/0022_indexing_in_courseenrollment.py
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 00000000000..8b313ff544f
--- /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 2a5fd003188..0fa401d5a2c 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 90405cb480f..be9b13a8fb6 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 42e34a54e91..8da4372b04c 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 398e3c5e8d1..9a8d844e0a0 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 b07e245ba9f..ef260dad230 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 504cb49d839..2edc04df502 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 a6db6735831..6451c814b09 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:
+ <br/>
+ ${len(course_enrollments)} ${_("results successfully populated,")}
+ <a href="${reverse('dashboard')}?course_limit=None">
+ ${_("Click to load all enrolled courses")}
+ </a>
+ % endif
</ul>
% else:
<div class="empty-dashboard-message">
diff --git a/themes/edx.org/lms/templates/dashboard.html b/themes/edx.org/lms/templates/dashboard.html
index 021ab1f3cbc..c582fc49e93 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:
+ <br/>
+ ${len(course_enrollments)} ${_("results successfully populated,")}
+ <a href="${reverse('dashboard')}?course_limit=None">
+ ${_("Click to load all enrolled courses")}
+ </a>
+ % endif
</ul>
% else:
<section class="empty-dashboard-message">
--
GitLab