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

Merge pull request #20439 from edx/aehsan/prod-204/learners_seeing_504_error

Browse Source 
Flag added to load limited courses on Dashboard
This commit is contained in:
adeelehsan
2019-07-01 13:01:21 +05:00
committed by GitHub
parent a5629bff89 2d369490dd
commit 46ffa0103d
9 changed files with 124 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
common/djangoapps/student/migrations/0022_indexing_in_courseenrollment.py Normal file
View File

@@ -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'),
),
]

18
common/djangoapps/student/models.py
View File

@@ -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.

19
common/djangoapps/student/tests/test_course_listing.py
View File

@@ -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

17
common/djangoapps/student/tests/test_views.py
View File

@@ -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())

38
common/djangoapps/student/views/dashboard.py
View File

@@ -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)

4
lms/envs/production.py
View File

@@ -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)

4
lms/envs/test.py
View File

@@ -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

8
lms/templates/dashboard.html
View File

@@ -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">

7
themes/edx.org/lms/templates/dashboard.html
View File

@@ -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">