From 3e87cb7c7d4a569fe2ae587bc428733ee61cd708 Mon Sep 17 00:00:00 2001 From: Matthew Piatetsky <mpiatetsky@edx.org> Date: Fri, 12 Oct 2018 13:48:57 -0400 Subject: [PATCH] fix expired message on dashboard and some of the tests --- .../tests/test_field_override_performance.py | 54 +++++++++---------- lms/djangoapps/courseware/access.py | 2 +- .../courseware/tests/test_access.py | 14 ++--- .../courseware/tests/test_course_info.py | 4 +- lms/djangoapps/courseware/tests/test_views.py | 12 ++--- lms/djangoapps/discussion/tests/test_views.py | 16 +++--- .../django_comment_client/base/tests.py | 8 +-- .../dashboard/_dashboard_course_listing.html | 27 +++++----- .../features/course_duration_limits/access.py | 11 +++- .../features/course_duration_limits/admin.py | 6 --- .../features/course_duration_limits/apps.py | 3 ++ .../migrations/__init__.py | 0 .../features/course_duration_limits/models.py | 6 --- .../features/course_duration_limits/tests.py | 6 --- .../features/course_duration_limits/views.py | 6 --- .../tests/views/test_course_home.py | 2 +- .../tests/views/test_course_updates.py | 2 +- 17 files changed, 84 insertions(+), 95 deletions(-) delete mode 100644 openedx/features/course_duration_limits/admin.py delete mode 100644 openedx/features/course_duration_limits/migrations/__init__.py delete mode 100644 openedx/features/course_duration_limits/models.py delete mode 100644 openedx/features/course_duration_limits/tests.py delete mode 100644 openedx/features/course_duration_limits/views.py diff --git a/lms/djangoapps/ccx/tests/test_field_override_performance.py b/lms/djangoapps/ccx/tests/test_field_override_performance.py index 672c9bc38ce..1e91d7e15f6 100644 --- a/lms/djangoapps/ccx/tests/test_field_override_performance.py +++ b/lms/djangoapps/ccx/tests/test_field_override_performance.py @@ -235,18 +235,18 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase): # # of sql queries to default, # # of mongo queries, # ) - ('no_overrides', 1, True, False): (18, 1), - ('no_overrides', 2, True, False): (18, 1), - ('no_overrides', 3, True, False): (18, 1), - ('ccx', 1, True, False): (18, 1), - ('ccx', 2, True, False): (18, 1), - ('ccx', 3, True, False): (18, 1), - ('no_overrides', 1, False, False): (18, 1), - ('no_overrides', 2, False, False): (18, 1), - ('no_overrides', 3, False, False): (18, 1), - ('ccx', 1, False, False): (18, 1), - ('ccx', 2, False, False): (18, 1), - ('ccx', 3, False, False): (18, 1), + ('no_overrides', 1, True, False): (20, 1), + ('no_overrides', 2, True, False): (20, 1), + ('no_overrides', 3, True, False): (20, 1), + ('ccx', 1, True, False): (20, 1), + ('ccx', 2, True, False): (20, 1), + ('ccx', 3, True, False): (20, 1), + ('no_overrides', 1, False, False): (20, 1), + ('no_overrides', 2, False, False): (20, 1), + ('no_overrides', 3, False, False): (20, 1), + ('ccx', 1, False, False): (20, 1), + ('ccx', 2, False, False): (20, 1), + ('ccx', 3, False, False): (20, 1), } @@ -258,19 +258,19 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase): __test__ = True TEST_DATA = { - ('no_overrides', 1, True, False): (18, 3), - ('no_overrides', 2, True, False): (18, 3), - ('no_overrides', 3, True, False): (18, 3), - ('ccx', 1, True, False): (18, 3), - ('ccx', 2, True, False): (18, 3), - ('ccx', 3, True, False): (18, 3), - ('ccx', 1, True, True): (19, 3), - ('ccx', 2, True, True): (19, 3), - ('ccx', 3, True, True): (19, 3), - ('no_overrides', 1, False, False): (18, 3), - ('no_overrides', 2, False, False): (18, 3), - ('no_overrides', 3, False, False): (18, 3), - ('ccx', 1, False, False): (18, 3), - ('ccx', 2, False, False): (18, 3), - ('ccx', 3, False, False): (18, 3), + ('no_overrides', 1, True, False): (20, 3), + ('no_overrides', 2, True, False): (20, 3), + ('no_overrides', 3, True, False): (20, 3), + ('ccx', 1, True, False): (20, 3), + ('ccx', 2, True, False): (20, 3), + ('ccx', 3, True, False): (20, 3), + ('ccx', 1, True, True): (21, 3), + ('ccx', 2, True, True): (21, 3), + ('ccx', 3, True, True): (21, 3), + ('no_overrides', 1, False, False): (20, 3), + ('no_overrides', 2, False, False): (20, 3), + ('no_overrides', 3, False, False): (20, 3), + ('ccx', 1, False, False): (20, 3), + ('ccx', 2, False, False): (20, 3), + ('ccx', 3, False, False): (20, 3), } diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 91360ef1804..ae822e836b5 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -11,7 +11,7 @@ Note: The access control logic in this file does NOT check for enrollment in It is a wrapper around has_access that additionally checks for enrollment. """ import logging -from datetime import datetime, timedelta +from datetime import datetime from django.conf import settings from django.contrib.auth.models import AnonymousUser diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index 260101cb740..9020bcaf534 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -836,15 +836,15 @@ class CourseOverviewAccessTestCase(ModuleStoreTestCase): user = getattr(self, user_attr_name) user = User.objects.get(id=user.id) - if (user_attr_name == 'user_staff' and - action == 'see_exists' and - course_attr_name in - ['course_default', 'course_not_started']): + if user_attr_name == 'user_staff' and action == 'see_exists': # checks staff role num_queries = 1 - elif user_attr_name == 'user_normal' and action == 'see_exists' and course_attr_name != 'course_started': - # checks staff role and enrollment data - num_queries = 2 + elif user_attr_name == 'user_normal' and action == 'see_exists': + if course_attr_name == 'course_started': + num_queries = 1 + else: + # checks staff role and enrollment data + num_queries = 2 else: num_queries = 0 diff --git a/lms/djangoapps/courseware/tests/test_course_info.py b/lms/djangoapps/courseware/tests/test_course_info.py index 1d15f3b60fc..8b2ead3fa50 100644 --- a/lms/djangoapps/courseware/tests/test_course_info.py +++ b/lms/djangoapps/courseware/tests/test_course_info.py @@ -431,7 +431,7 @@ class SelfPacedCourseInfoTestCase(LoginEnrollmentTestCase, SharedModuleStoreTest self.assertEqual(resp.status_code, 200) def test_num_queries_instructor_paced(self): - self.fetch_course_info_with_queries(self.instructor_paced_course, 28, 3) + self.fetch_course_info_with_queries(self.instructor_paced_course, 29, 3) def test_num_queries_self_paced(self): - self.fetch_course_info_with_queries(self.self_paced_course, 28, 3) + self.fetch_course_info_with_queries(self.self_paced_course, 29, 3) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index be6d9d92ff3..f86a926430a 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -205,8 +205,8 @@ class IndexQueryTestCase(ModuleStoreTestCase): NUM_PROBLEMS = 20 @ddt.data( - (ModuleStoreEnum.Type.mongo, 10, 147), - (ModuleStoreEnum.Type.split, 4, 147), + (ModuleStoreEnum.Type.mongo, 10, 157), + (ModuleStoreEnum.Type.split, 4, 153), ) @ddt.unpack def test_index_query_counts(self, store_type, expected_mongo_query_count, expected_mysql_query_count): @@ -1430,8 +1430,8 @@ class ProgressPageTests(ProgressPageBaseTests): self.assertContains(resp, u"Download Your Certificate") @ddt.data( - (True, 38), - (False, 37) + (True, 40), + (False, 39) ) @ddt.unpack def test_progress_queries_paced_courses(self, self_paced, query_count): @@ -1442,8 +1442,8 @@ class ProgressPageTests(ProgressPageBaseTests): @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) @ddt.data( - (False, 45, 28), - (True, 37, 24) + (False, 47, 30), + (True, 39, 26) ) @ddt.unpack def test_progress_queries(self, enable_waffle, initial, subsequent): diff --git a/lms/djangoapps/discussion/tests/test_views.py b/lms/djangoapps/discussion/tests/test_views.py index 375fb798000..b6a4c2be336 100644 --- a/lms/djangoapps/discussion/tests/test_views.py +++ b/lms/djangoapps/discussion/tests/test_views.py @@ -430,18 +430,18 @@ class SingleThreadQueryCountTestCase(ForumsEnableMixin, ModuleStoreTestCase): # course is outside the context manager that is verifying the number of queries, # and with split mongo, that method ends up querying disabled_xblocks (which is then # cached and hence not queried as part of call_single_thread). - (ModuleStoreEnum.Type.mongo, False, 1, 5, 2, 17, 5), - (ModuleStoreEnum.Type.mongo, False, 50, 5, 2, 17, 5), + (ModuleStoreEnum.Type.mongo, False, 1, 5, 2, 19, 7), + (ModuleStoreEnum.Type.mongo, False, 50, 5, 2, 19, 7), # split mongo: 3 queries, regardless of thread response size. - (ModuleStoreEnum.Type.split, False, 1, 3, 3, 17, 5), - (ModuleStoreEnum.Type.split, False, 50, 3, 3, 17, 5), + (ModuleStoreEnum.Type.split, False, 1, 3, 3, 19, 7), + (ModuleStoreEnum.Type.split, False, 50, 3, 3, 19, 7), # Enabling Enterprise integration should have no effect on the number of mongo queries made. - (ModuleStoreEnum.Type.mongo, True, 1, 5, 2, 17, 5), - (ModuleStoreEnum.Type.mongo, True, 50, 5, 2, 17, 5), + (ModuleStoreEnum.Type.mongo, True, 1, 5, 2, 19, 7), + (ModuleStoreEnum.Type.mongo, True, 50, 5, 2, 19, 7), # split mongo: 3 queries, regardless of thread response size. - (ModuleStoreEnum.Type.split, True, 1, 3, 3, 17, 5), - (ModuleStoreEnum.Type.split, True, 50, 3, 3, 17, 5), + (ModuleStoreEnum.Type.split, True, 1, 3, 3, 19, 7), + (ModuleStoreEnum.Type.split, True, 50, 3, 3, 19, 7), ) @ddt.unpack def test_number_of_mongo_queries( diff --git a/lms/djangoapps/django_comment_client/base/tests.py b/lms/djangoapps/django_comment_client/base/tests.py index 07d60509ee1..26ae5a59d95 100644 --- a/lms/djangoapps/django_comment_client/base/tests.py +++ b/lms/djangoapps/django_comment_client/base/tests.py @@ -403,8 +403,8 @@ class ViewsQueryCountTestCase( return inner @ddt.data( - (ModuleStoreEnum.Type.mongo, 3, 4, 35), - (ModuleStoreEnum.Type.split, 3, 13, 35), + (ModuleStoreEnum.Type.mongo, 3, 4, 37), + (ModuleStoreEnum.Type.split, 3, 13, 37), ) @ddt.unpack @count_queries @@ -412,8 +412,8 @@ class ViewsQueryCountTestCase( self.create_thread_helper(mock_request) @ddt.data( - (ModuleStoreEnum.Type.mongo, 3, 3, 31), - (ModuleStoreEnum.Type.split, 3, 10, 31), + (ModuleStoreEnum.Type.mongo, 3, 3, 33), + (ModuleStoreEnum.Type.split, 3, 10, 33), ) @ddt.unpack @count_queries diff --git a/lms/templates/dashboard/_dashboard_course_listing.html b/lms/templates/dashboard/_dashboard_course_listing.html index 1b16d0ba339..e1472f3a8fc 100644 --- a/lms/templates/dashboard/_dashboard_course_listing.html +++ b/lms/templates/dashboard/_dashboard_course_listing.html @@ -33,6 +33,8 @@ from util.course import get_link_for_about_page, get_encoded_course_sharing_utm_ if cert_name_long == "": cert_name_long = settings.CERT_NAME_LONG billing_email = settings.PAYMENT_SUPPORT_EMAIL + + is_course_expired = getattr(show_courseware_link, 'error_code') == 'audit_expired' %> <%namespace name='static' file='../static_content.html'/> @@ -65,7 +67,7 @@ from util.course import get_link_for_about_page, get_encoded_course_sharing_utm_ <h2 class="hd hd-2 sr" id="details-heading-${enrollment.course_id}">${_('Course details')}</h2> <div class="wrapper-course-image" aria-hidden="true"> % if show_courseware_link and not is_unfulfilled_entitlement: - % if not is_course_blocked: + % if not is_course_blocked and not is_course_expired: <a href="${course_target}" data-course-key="${enrollment.course_id}" class="cover" tabindex="-1"> <img src="${course_overview.image_urls['small']}" class="course-image" alt="${_('{course_number} {course_name} Home Page').format(course_number=course_overview.number, course_name=course_overview.display_name_with_default)}" /> </a> @@ -92,7 +94,7 @@ from util.course import get_link_for_about_page, get_encoded_course_sharing_utm_ <div class="wrapper-course-details"> <h3 class="course-title" id="course-title-${enrollment.course_id}"> % if show_courseware_link and not is_unfulfilled_entitlement: - % if not is_course_blocked: + % if not is_course_blocked and not is_course_expired: <a data-course-key="${enrollment.course_id}" href="${course_target}">${course_overview.display_name_with_default}</a> % else: <a class="disable-look" data-course-key="${enrollment.course_id}">${course_overview.display_name_with_default}</a> @@ -128,7 +130,14 @@ from util.course import get_link_for_about_page, get_encoded_course_sharing_utm_ %> <span class="info-date-block-container"> - % if is_unfulfilled_entitlement: + % if is_course_expired: + <span class="info-date-block" data-course-key="${enrollment.course_id}"> + ${show_courseware_link.user_message} + <span class="sr"> + ${_('for {course_display_name}').format(course_display_name=course_overview.display_name_with_default)} + </span> + </span> + % elif is_unfulfilled_entitlement: <span class="info-date-block" aria-live="polite"> <span class="icon fa fa-warning" aria-hidden="true"></span> % if not entitlement_expired_at: @@ -167,7 +176,7 @@ from util.course import get_link_for_about_page, get_encoded_course_sharing_utm_ <div class="course-actions"> % if show_courseware_link or is_unfulfilled_entitlement: % if course_overview.has_ended(): - % if not is_course_blocked: + % if not is_course_blocked and not is_course_expired: <a href="${course_target}" class="enter-course archived" data-course-key="${enrollment.course_id}">${_('View Archived Course')}<span class="sr"> ${course_overview.display_name_with_default}</span></a> % else: <a class="enter-course-blocked archived" data-course-key="${enrollment.course_id}">${_('View Archived Course')}<span class="sr"> ${course_overview.display_name_with_default}</span></a> @@ -183,7 +192,7 @@ from util.course import get_link_for_about_page, get_encoded_course_sharing_utm_ ${course_overview.display_name_with_default} </span> </a> - % elif not is_course_blocked: + % elif not is_course_blocked and not is_course_expired: <a href="${course_target}" class="enter-course ${'hidden' if is_unfulfilled_entitlement else ''}" data-course-key="${enrollment.course_id}"> @@ -202,14 +211,6 @@ from util.course import get_link_for_about_page, get_encoded_course_sharing_utm_ </a> % endif % endif - % elif hasattr(show_courseware_link, 'user_message'): - <span class="enter-course-blocked" - data-course-key="${enrollment.course_id}"> - ${show_courseware_link.user_message} - <span class="sr"> - ${_('for {course_display_name}').format(course_display_name=course_overview.display_name_with_default)} - </span> - </span> % endif % if show_courseware_link or course_overview.has_social_sharing_url() or course_overview.has_marketing_url(): diff --git a/openedx/features/course_duration_limits/access.py b/openedx/features/course_duration_limits/access.py index 866da292a95..148d1b7abb7 100644 --- a/openedx/features/course_duration_limits/access.py +++ b/openedx/features/course_duration_limits/access.py @@ -1,3 +1,8 @@ +# -*- coding: utf-8 -*- +""" +Contains code related to computing content gating course duration limits +and course access based on these limits. +""" from datetime import timedelta from django.apps import apps @@ -16,11 +21,15 @@ class AuditExpiredError(AccessError): def __init__(self, user, course, end_date): error_code = "audit_expired" developer_message = "User {} had access to {} until {}".format(user, course, end_date) - user_message = _("Your audit access period has expired.") + # TODO: Translate the end_date + user_message = _("Course access expired on ") + end_date.strftime("%B %d, %Y") super(AuditExpiredError, self).__init__(error_code, developer_message, user_message) def check_course_expired(user, course): + """ + Check if the course expired for the user. + """ # TODO: Only limit audit users # TODO: Limit access to instructor paced courses based on end-date, rather than content availability date CourseEnrollment = apps.get_model('student.CourseEnrollment') diff --git a/openedx/features/course_duration_limits/admin.py b/openedx/features/course_duration_limits/admin.py deleted file mode 100644 index 13be29d96fa..00000000000 --- a/openedx/features/course_duration_limits/admin.py +++ /dev/null @@ -1,6 +0,0 @@ -# -*- coding: utf-8 -*- -from __future__ import unicode_literals - -from django.contrib import admin - -# Register your models here. diff --git a/openedx/features/course_duration_limits/apps.py b/openedx/features/course_duration_limits/apps.py index a0f1f4b2d71..d88cc87209c 100644 --- a/openedx/features/course_duration_limits/apps.py +++ b/openedx/features/course_duration_limits/apps.py @@ -1,3 +1,6 @@ +""" +Course duration limits application configuration +""" # -*- coding: utf-8 -*- from __future__ import unicode_literals diff --git a/openedx/features/course_duration_limits/migrations/__init__.py b/openedx/features/course_duration_limits/migrations/__init__.py deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/openedx/features/course_duration_limits/models.py b/openedx/features/course_duration_limits/models.py deleted file mode 100644 index 1dfab76043e..00000000000 --- a/openedx/features/course_duration_limits/models.py +++ /dev/null @@ -1,6 +0,0 @@ -# -*- coding: utf-8 -*- -from __future__ import unicode_literals - -from django.db import models - -# Create your models here. diff --git a/openedx/features/course_duration_limits/tests.py b/openedx/features/course_duration_limits/tests.py deleted file mode 100644 index 5982e6bcd29..00000000000 --- a/openedx/features/course_duration_limits/tests.py +++ /dev/null @@ -1,6 +0,0 @@ -# -*- coding: utf-8 -*- -from __future__ import unicode_literals - -from django.test import TestCase - -# Create your tests here. diff --git a/openedx/features/course_duration_limits/views.py b/openedx/features/course_duration_limits/views.py deleted file mode 100644 index e784a0bd2ca..00000000000 --- a/openedx/features/course_duration_limits/views.py +++ /dev/null @@ -1,6 +0,0 @@ -# -*- coding: utf-8 -*- -from __future__ import unicode_literals - -from django.shortcuts import render - -# Create your views here. diff --git a/openedx/features/course_experience/tests/views/test_course_home.py b/openedx/features/course_experience/tests/views/test_course_home.py index ad298ca7d06..de380d2968a 100644 --- a/openedx/features/course_experience/tests/views/test_course_home.py +++ b/openedx/features/course_experience/tests/views/test_course_home.py @@ -173,7 +173,7 @@ class TestCourseHomePage(CourseHomePageTestCase): course_home_url(self.course) # Fetch the view and verify the query counts - with self.assertNumQueries(54, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): + with self.assertNumQueries(66, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): with check_mongo_calls(4): url = course_home_url(self.course) self.client.get(url) diff --git a/openedx/features/course_experience/tests/views/test_course_updates.py b/openedx/features/course_experience/tests/views/test_course_updates.py index 5891c4688fa..67c79757e4f 100644 --- a/openedx/features/course_experience/tests/views/test_course_updates.py +++ b/openedx/features/course_experience/tests/views/test_course_updates.py @@ -124,7 +124,7 @@ class TestCourseUpdatesPage(SharedModuleStoreTestCase): course_updates_url(self.course) # Fetch the view and verify that the query counts haven't changed - with self.assertNumQueries(34, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): + with self.assertNumQueries(38, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): with check_mongo_calls(4): url = course_updates_url(self.course) self.client.get(url) -- GitLab