From 630985b3cc69c46991472de3fb61d98bbb4bfbdc Mon Sep 17 00:00:00 2001 From: Joel Barciauskas Date: Mon, 6 Mar 2017 10:41:39 -0500 Subject: [PATCH 01/51] Wrap all newrelic dependencies in a check to see if the module is loaded --- .../newrelic_custom_metrics/middleware.py | 11 ++- common/lib/xmodule/xmodule/seq_module.py | 12 +++- lms/djangoapps/courseware/module_render.py | 15 +++-- lms/djangoapps/courseware/views/index.py | 12 +++- lms/djangoapps/discussion/views.py | 67 +++++++++++-------- lms/djangoapps/grades/tasks.py | 14 ++-- .../djangoapps/contentserver/middleware.py | 40 ++++++----- 7 files changed, 112 insertions(+), 59 deletions(-) diff --git a/common/djangoapps/newrelic_custom_metrics/middleware.py b/common/djangoapps/newrelic_custom_metrics/middleware.py index 3af8cb5ef1..3d6bcd89f9 100644 --- a/common/djangoapps/newrelic_custom_metrics/middleware.py +++ b/common/djangoapps/newrelic_custom_metrics/middleware.py @@ -6,7 +6,14 @@ This middleware will only call on the newrelic agent if there are any metrics to report for this request, so it will not incur any processing overhead for request handlers which do not record custom metrics. """ -import newrelic.agent +import logging +log = logging.getLogger(__name__) +try: + import newrelic.agent +except ImportError: + log.warning("Unable to load NewRelic agent module") + newrelic = None # pylint: disable=invalid-name + import request_cache REQUEST_CACHE_KEY = 'newrelic_custom_metrics' @@ -40,6 +47,8 @@ class NewRelicCustomMetrics(object): """ Report the collected custom metrics to New Relic. """ + if not newrelic: + return metrics_cache = cls._get_metrics_cache() for metric_name, metric_value in metrics_cache.iteritems(): newrelic.agent.add_custom_parameter(metric_name, metric_value) diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index 8233f2a669..ecda5cc450 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -14,7 +14,6 @@ from lxml import etree from xblock.core import XBlock from xblock.fields import Integer, Scope, Boolean, String from xblock.fragment import Fragment -import newrelic.agent from .exceptions import NotFoundError from .fields import Date @@ -25,6 +24,11 @@ from .xml_module import XmlDescriptor log = logging.getLogger(__name__) +try: + import newrelic.agent +except ImportError: + newrelic = None # pylint: disable=invalid-name + # HACK: This shouldn't be hard-coded to two types # OBSOLETE: This obsoletes 'type' class_priority = ['video', 'problem'] @@ -385,6 +389,8 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule): """ Capture basic information about this sequence in New Relic. """ + if not newrelic: + return newrelic.agent.add_custom_parameter('seq.block_id', unicode(self.location)) newrelic.agent.add_custom_parameter('seq.display_name', self.display_name or '') newrelic.agent.add_custom_parameter('seq.position', self.position) @@ -396,6 +402,8 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule): the sequence as a whole. We send this information to New Relic so that we can do better performance analysis of courseware. """ + if not newrelic: + return # Basic count of the number of Units (a.k.a. VerticalBlocks) we have in # this learning sequence newrelic.agent.add_custom_parameter('seq.num_units', len(display_items)) @@ -414,6 +422,8 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule): """ Capture information about the current selected Unit within the Sequence. """ + if not newrelic: + return # Positions are stored with indexing starting at 1. If we get into a # weird state where the saved position is out of bounds (e.g. the # content was changed), avoid going into any details about this unit. diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 0cef0cbfd2..beb4dd0ab0 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -8,7 +8,6 @@ import logging from collections import OrderedDict from functools import partial -import newrelic.agent from capa.xqueue_interface import XQueueInterface from django.conf import settings from django.contrib.auth.models import User @@ -82,6 +81,10 @@ from .field_overrides import OverrideFieldData log = logging.getLogger(__name__) +try: + import newrelic.agent +except ImportError: + newrelic = None # pylint: disable=invalid-name if settings.XQUEUE_INTERFACE.get('basic_auth') is not None: REQUESTS_AUTH = HTTPBasicAuth(*settings.XQUEUE_INTERFACE['basic_auth']) @@ -966,9 +969,10 @@ def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, course except InvalidKeyError: raise Http404 - # Gather metrics for New Relic so we can slice data in New Relic Insights - newrelic.agent.add_custom_parameter('course_id', unicode(course_key)) - newrelic.agent.add_custom_parameter('org', unicode(course_key.org)) + if newrelic: + # Gather metrics for New Relic so we can slice data in New Relic Insights + newrelic.agent.add_custom_parameter('course_id', unicode(course_key)) + newrelic.agent.add_custom_parameter('org', unicode(course_key.org)) with modulestore().bulk_operations(course_key): instance, tracking_context = get_module_by_usage_id(request, course_id, usage_id, course=course) @@ -978,7 +982,8 @@ def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, course # "handler" in those cases is always just "xmodule_handler". nr_tx_name = "{}.{}".format(instance.__class__.__name__, handler) nr_tx_name += "/{}".format(suffix) if (suffix and handler == "xmodule_handler") else "" - newrelic.agent.set_transaction_name(nr_tx_name, group="Python/XBlock/Handler") + if newrelic: + newrelic.agent.set_transaction_name(nr_tx_name, group="Python/XBlock/Handler") tracking_context_name = 'module_callback_handler' req = django_to_webob_request(request) diff --git a/lms/djangoapps/courseware/views/index.py b/lms/djangoapps/courseware/views/index.py index e07f055760..3a93d4eb87 100644 --- a/lms/djangoapps/courseware/views/index.py +++ b/lms/djangoapps/courseware/views/index.py @@ -19,7 +19,14 @@ from django.shortcuts import redirect from courseware.url_helpers import get_redirect_url_for_global_staff from edxmako.shortcuts import render_to_response, render_to_string import logging -import newrelic.agent + +log = logging.getLogger("edx.courseware.views.index") + +try: + import newrelic.agent +except ImportError: + newrelic = None # pylint: disable=invalid-name + import urllib from xblock.fragment import Fragment @@ -54,7 +61,6 @@ from ..module_render import toc_for_course, get_module_for_descriptor from .views import get_current_child, registered_for_course -log = logging.getLogger("edx.courseware.views.index") TEMPLATE_IMPORTS = {'urllib': urllib} CONTENT_DEPTH = 2 @@ -174,6 +180,8 @@ class CoursewareIndex(View): """ Initialize metrics for New Relic so we can slice data in New Relic Insights """ + if not newrelic: + return newrelic.agent.add_custom_parameter('course_id', unicode(self.course_key)) newrelic.agent.add_custom_parameter('org', unicode(self.course_key.org)) diff --git a/lms/djangoapps/discussion/views.py b/lms/djangoapps/discussion/views.py index db5df8d243..6822503a91 100644 --- a/lms/djangoapps/discussion/views.py +++ b/lms/djangoapps/discussion/views.py @@ -17,7 +17,13 @@ from django.shortcuts import render_to_response from django.template.loader import render_to_string from django.utils.translation import get_language_bidi from django.views.decorators.http import require_GET -import newrelic.agent + +log = logging.getLogger("edx.discussions") +try: + import newrelic.agent +except ImportError: + newrelic = None # pylint: disable=invalid-name + from rest_framework import status from web_fragments.fragment import Fragment @@ -50,13 +56,27 @@ import lms.lib.comment_client as cc from opaque_keys.edx.keys import CourseKey +from contextlib import contextmanager + THREADS_PER_PAGE = 20 INLINE_THREADS_PER_PAGE = 20 PAGES_NEARBY_DELTA = 2 -log = logging.getLogger("edx.discussions") -@newrelic.agent.function_trace() +@contextmanager +def newrelic_function_trace(function_name): + """ + A wrapper context manager newrelic.agent.FunctionTrace to no-op if the + newrelic package is not installed + """ + if newrelic: + nr_transaction = newrelic.agent.current_transaction() + with newrelic.agent.FunctionTrace(nr_transaction, function_name): + yield + else: + yield + + def make_course_settings(course, user): """ Generate a JSON-serializable model for course settings, which will be used to initialize a @@ -71,7 +91,6 @@ def make_course_settings(course, user): } -@newrelic.agent.function_trace() def get_threads(request, course, user_info, discussion_id=None, per_page=THREADS_PER_PAGE): """ This may raise an appropriate subclass of cc.utils.CommentClientError @@ -184,7 +203,6 @@ def inline_discussion(request, course_key, discussion_id): """ Renders JSON for DiscussionModules """ - nr_transaction = newrelic.agent.current_transaction() course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=True) cc_user = cc.User.from_django_user(request.user) @@ -195,12 +213,14 @@ def inline_discussion(request, course_key, discussion_id): except ValueError: return HttpResponseServerError("Invalid group_id") - with newrelic.agent.FunctionTrace(nr_transaction, "get_metadata_for_threads"): + with newrelic_function_trace("get_metadata_for_threads"): annotated_content_info = utils.get_metadata_for_threads(course_key, threads, request.user, user_info) + is_staff = has_permission(request.user, 'openclose_thread', course.id) threads = [utils.prepare_content(thread, course_key, is_staff) for thread in threads] - with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"): + with newrelic_function_trace("add_courseware_context"): add_courseware_context(threads, course, request.user) + return utils.JsonResponse({ 'is_commentable_cohorted': is_commentable_cohorted(course_key, discussion_id), 'discussion_data': threads, @@ -219,8 +239,6 @@ def forum_form_discussion(request, course_key): """ Renders the main Discussion page, potentially filtered by a search query """ - nr_transaction = newrelic.agent.current_transaction() - course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=True) if request.is_ajax(): user = cc.User.from_django_user(request.user) @@ -235,10 +253,10 @@ def forum_form_discussion(request, course_key): except ValueError: return HttpResponseServerError("Invalid group_id") - with newrelic.agent.FunctionTrace(nr_transaction, "get_metadata_for_threads"): + with newrelic_function_trace("get_metadata_for_threads"): annotated_content_info = utils.get_metadata_for_threads(course_key, threads, request.user, user_info) - with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"): + with newrelic_function_trace("add_courseware_context"): add_courseware_context(threads, course, request.user) return utils.JsonResponse({ @@ -265,8 +283,6 @@ def single_thread(request, course_key, discussion_id, thread_id): Depending on the HTTP headers, we'll adjust our response accordingly. """ - nr_transaction = newrelic.agent.current_transaction() - course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=True) if request.is_ajax(): @@ -278,7 +294,7 @@ def single_thread(request, course_key, discussion_id, thread_id): if not thread: raise Http404 - with newrelic.agent.FunctionTrace(nr_transaction, "get_annotated_content_infos"): + with newrelic_function_trace("get_annotated_content_infos"): annotated_content_info = utils.get_annotated_content_infos( course_key, thread, @@ -287,7 +303,7 @@ def single_thread(request, course_key, discussion_id, thread_id): ) content = utils.prepare_content(thread.to_dict(), course_key, is_staff) - with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"): + with newrelic_function_trace("add_courseware_context"): add_courseware_context([content], course, request.user) return utils.JsonResponse({ @@ -372,7 +388,6 @@ def _create_discussion_board_context(request, course_key, discussion_id=None, th """ Returns the template context for rendering the discussion board. """ - nr_transaction = newrelic.agent.current_transaction() context = _create_base_discussion_view_context(request, course_key) course = context['course'] course_settings = context['course_settings'] @@ -401,13 +416,13 @@ def _create_discussion_board_context(request, course_key, discussion_id=None, th is_staff = has_permission(user, 'openclose_thread', course.id) threads = [utils.prepare_content(thread, course_key, is_staff) for thread in threads] - with newrelic.agent.FunctionTrace(nr_transaction, "get_metadata_for_threads"): + with newrelic_function_trace("get_metadata_for_threads"): annotated_content_info = utils.get_metadata_for_threads(course_key, threads, user, user_info) - with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"): + with newrelic_function_trace("add_courseware_context"): add_courseware_context(threads, course, user) - with newrelic.agent.FunctionTrace(nr_transaction, "get_cohort_info"): + with newrelic_function_trace("get_cohort_info"): user_cohort_id = get_cohort_id(user, course_key) context.update({ @@ -435,9 +450,6 @@ def user_profile(request, course_key, user_id): Renders a response to display the user profile page (shown after clicking on a post author's username). """ - - nr_transaction = newrelic.agent.current_transaction() - user = cc.User.from_django_user(request.user) course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=True) @@ -466,13 +478,13 @@ def user_profile(request, course_key, user_id): query_params['page'] = page query_params['num_pages'] = num_pages - with newrelic.agent.FunctionTrace(nr_transaction, "get_metadata_for_threads"): + with newrelic_function_trace("get_metadata_for_threads"): user_info = cc.User.from_django_user(request.user).to_dict() annotated_content_info = utils.get_metadata_for_threads(course_key, threads, request.user, user_info) is_staff = has_permission(request.user, 'openclose_thread', course.id) threads = [utils.prepare_content(thread, course_key, is_staff) for thread in threads] - with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"): + with newrelic_function_trace("add_courseware_context"): add_courseware_context(threads, course, request.user) if request.is_ajax(): return utils.JsonResponse({ @@ -486,7 +498,7 @@ def user_profile(request, course_key, user_id): course_id=course.id ).order_by("name").values_list("name", flat=True).distinct() - with newrelic.agent.FunctionTrace(nr_transaction, "get_cohort_info"): + with newrelic_function_trace("get_cohort_info"): user_cohort_id = get_cohort_id(request.user, course_key) context = _create_base_discussion_view_context(request, course_key) @@ -514,9 +526,6 @@ def followed_threads(request, course_key, user_id): """ Ajax-only endpoint retrieving the threads followed by a specific user. """ - - nr_transaction = newrelic.agent.current_transaction() - course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=True) try: profiled_user = cc.User(id=user_id, course_id=course_key) @@ -557,7 +566,7 @@ def followed_threads(request, course_key, user_id): query_params['num_pages'] = paginated_results.num_pages user_info = cc.User.from_django_user(request.user).to_dict() - with newrelic.agent.FunctionTrace(nr_transaction, "get_metadata_for_threads"): + with newrelic_function_trace("get_metadata_for_threads"): annotated_content_info = utils.get_metadata_for_threads( course_key, paginated_results.collection, diff --git a/lms/djangoapps/grades/tasks.py b/lms/djangoapps/grades/tasks.py index 0359deed1d..b4c51fd6b8 100644 --- a/lms/djangoapps/grades/tasks.py +++ b/lms/djangoapps/grades/tasks.py @@ -9,7 +9,12 @@ from django.contrib.auth.models import User from django.core.exceptions import ValidationError from django.db.utils import DatabaseError from logging import getLogger -import newrelic.agent + +log = getLogger(__name__) +try: + import newrelic.agent +except ImportError: + newrelic = None # pylint: disable=invalid-name from celery_utils.logged_task import LoggedTask from celery_utils.persist_on_failure import PersistOnFailureTask @@ -31,8 +36,6 @@ from .new.subsection_grade import SubsectionGradeFactory from .signals.signals import SUBSECTION_SCORE_CHANGED from .transformer import GradesTransformer -log = getLogger(__name__) - class DatabaseNotReadyError(IOError): """ @@ -96,8 +99,9 @@ def _recalculate_subsection_grade(self, **kwargs): scored_block_usage_key = UsageKey.from_string(kwargs['usage_id']).replace(course_key=course_key) - newrelic.agent.add_custom_parameter('course_id', unicode(course_key)) - newrelic.agent.add_custom_parameter('usage_id', unicode(scored_block_usage_key)) + if newrelic: + newrelic.agent.add_custom_parameter('course_id', unicode(course_key)) + newrelic.agent.add_custom_parameter('usage_id', unicode(scored_block_usage_key)) # The request cache is not maintained on celery workers, # where this code runs. So we take the values from the diff --git a/openedx/core/djangoapps/contentserver/middleware.py b/openedx/core/djangoapps/contentserver/middleware.py index 9fb41c08b5..8f6fe3eea8 100644 --- a/openedx/core/djangoapps/contentserver/middleware.py +++ b/openedx/core/djangoapps/contentserver/middleware.py @@ -4,7 +4,11 @@ Middleware to serve assets. import logging import datetime -import newrelic.agent +log = logging.getLogger(__name__) +try: + import newrelic.agent +except ImportError: + newrelic = None # pylint: disable=invalid-name from django.http import ( HttpResponse, HttpResponseNotModified, HttpResponseForbidden, HttpResponseBadRequest, HttpResponseNotFound, HttpResponsePermanentRedirect) @@ -25,7 +29,6 @@ from .models import CourseAssetCacheTtlConfig, CdnUserAgentsConfig # TODO: Soon as we have a reasonable way to serialize/deserialize AssetKeys, we need # to change this file so instead of using course_id_partial, we're just using asset keys -log = logging.getLogger(__name__) HTTP_DATE_FORMAT = "%a, %d %b %Y %H:%M:%S GMT" @@ -86,17 +89,18 @@ class StaticContentServer(object): if safe_course_key.run is None: safe_course_key = safe_course_key.replace(run='only') - newrelic.agent.add_custom_parameter('course_id', safe_course_key) - newrelic.agent.add_custom_parameter('org', loc.org) - newrelic.agent.add_custom_parameter('contentserver.path', loc.path) + if newrelic: + newrelic.agent.add_custom_parameter('course_id', safe_course_key) + newrelic.agent.add_custom_parameter('org', loc.org) + newrelic.agent.add_custom_parameter('contentserver.path', loc.path) - # Figure out if this is a CDN using us as the origin. - is_from_cdn = StaticContentServer.is_cdn_request(request) - newrelic.agent.add_custom_parameter('contentserver.from_cdn', is_from_cdn) + # Figure out if this is a CDN using us as the origin. + is_from_cdn = StaticContentServer.is_cdn_request(request) + newrelic.agent.add_custom_parameter('contentserver.from_cdn', is_from_cdn) - # Check if this content is locked or not. - locked = self.is_content_locked(content) - newrelic.agent.add_custom_parameter('contentserver.locked', locked) + # Check if this content is locked or not. + locked = self.is_content_locked(content) + newrelic.agent.add_custom_parameter('contentserver.locked', locked) # Check that user has access to the content. if not self.is_user_authorized(request, content, loc): @@ -153,7 +157,8 @@ class StaticContentServer(object): response['Content-Length'] = str(last - first + 1) response.status_code = 206 # Partial Content - newrelic.agent.add_custom_parameter('contentserver.ranged', True) + if newrelic: + newrelic.agent.add_custom_parameter('contentserver.ranged', True) else: log.warning( u"Cannot satisfy ranges in Range header: %s for content: %s", header_value, unicode(loc) @@ -165,8 +170,9 @@ class StaticContentServer(object): response = HttpResponse(content.stream_data()) response['Content-Length'] = content.length - newrelic.agent.add_custom_parameter('contentserver.content_len', content.length) - newrelic.agent.add_custom_parameter('contentserver.content_type', content.content_type) + if newrelic: + newrelic.agent.add_custom_parameter('contentserver.content_len', content.length) + newrelic.agent.add_custom_parameter('contentserver.content_type', content.content_type) # "Accept-Ranges: bytes" tells the user that only "bytes" ranges are allowed response['Accept-Ranges'] = 'bytes' @@ -194,12 +200,14 @@ class StaticContentServer(object): # indicate there should be no caching whatsoever. cache_ttl = CourseAssetCacheTtlConfig.get_cache_ttl() if cache_ttl > 0 and not is_locked: - newrelic.agent.add_custom_parameter('contentserver.cacheable', True) + if newrelic: + newrelic.agent.add_custom_parameter('contentserver.cacheable', True) response['Expires'] = StaticContentServer.get_expiration_value(datetime.datetime.utcnow(), cache_ttl) response['Cache-Control'] = "public, max-age={ttl}, s-maxage={ttl}".format(ttl=cache_ttl) elif is_locked: - newrelic.agent.add_custom_parameter('contentserver.cacheable', False) + if newrelic: + newrelic.agent.add_custom_parameter('contentserver.cacheable', False) response['Cache-Control'] = "private, no-cache, no-store" From cfb032a50c16ce7b784c43c1fa158afe041ad81d Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Tue, 14 Mar 2017 19:35:23 -0400 Subject: [PATCH 02/51] Fix grading for Gated Subsections TNL-5955 --- .../transformers/tests/test_milestones.py | 21 +- lms/djangoapps/gating/api.py | 141 ++++++------- lms/djangoapps/gating/signals.py | 24 +-- lms/djangoapps/gating/tests/test_api.py | 108 ++-------- .../gating/tests/test_integration.py | 192 +++++++++++++++++ lms/djangoapps/gating/tests/test_signals.py | 40 ++-- lms/djangoapps/grades/module_grades.py | 96 --------- lms/djangoapps/grades/tests/test_grades.py | 196 ------------------ .../mobile_api/tests/test_milestones.py | 47 +++-- 9 files changed, 339 insertions(+), 526 deletions(-) create mode 100644 lms/djangoapps/gating/tests/test_integration.py delete mode 100644 lms/djangoapps/grades/module_grades.py diff --git a/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py b/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py index b11b683e59..d3c26ac31a 100644 --- a/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py +++ b/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py @@ -52,7 +52,8 @@ class MilestonesTransformerTestCase(CourseStructureTestCase, MilestonesTestCaseM 'course', 'A', 'B', 'C', 'ProctoredExam', 'D', 'E', 'PracticeExam', 'F', 'G', 'H', 'I', 'TimedExam', 'J', 'K' ) - # The special exams (proctored, practice, timed) should never be visible to students + # The special exams (proctored, practice, timed) are not visible to + # students via the Courses API. ALL_BLOCKS_EXCEPT_SPECIAL = ('course', 'A', 'B', 'C', 'H', 'I') def get_course_hierarchy(self): @@ -133,18 +134,16 @@ class MilestonesTransformerTestCase(CourseStructureTestCase, MilestonesTestCaseM ( 'H', 'A', - 'B', ('course', 'A', 'B', 'C',) ), ( 'H', 'ProctoredExam', - 'D', ('course', 'A', 'B', 'C'), ), ) @ddt.unpack - def test_gated(self, gated_block_ref, gating_block_ref, gating_block_child, expected_blocks_before_completion): + def test_gated(self, gated_block_ref, gating_block_ref, expected_blocks_before_completion): """ First, checks that a student cannot see the gated block when it is gated by the gating block and no attempt has been made to complete the gating block. @@ -164,17 +163,15 @@ class MilestonesTransformerTestCase(CourseStructureTestCase, MilestonesTestCaseM # clear the request cache to simulate a new request self.clear_caches() - # mock the api that the lms gating api calls to get the score for each block to always return 1 (ie 100%) - with patch('gating.api.get_module_score', Mock(return_value=1)): - - # this call triggers reevaluation of prerequisites fulfilled by the parent of the - # block passed in, so we pass in a child of the gating block + # this call triggers reevaluation of prerequisites fulfilled by the gating block. + with patch('gating.api._get_subsection_percentage', Mock(return_value=100)): lms_gating_api.evaluate_prerequisite( self.course, - self.blocks[gating_block_child], - self.user.id, + Mock(location=self.blocks[gating_block_ref].location), + self.user, ) - with self.assertNumQueries(5): + + with self.assertNumQueries(6): self.get_blocks_and_check_against_expected(self.user, self.ALL_BLOCKS_EXCEPT_SPECIAL) def test_staff_access(self): diff --git a/lms/djangoapps/gating/api.py b/lms/djangoapps/gating/api.py index 0f2a8ea55b..14bf7677d2 100644 --- a/lms/djangoapps/gating/api.py +++ b/lms/djangoapps/gating/api.py @@ -2,105 +2,87 @@ API for the gating djangoapp """ from collections import defaultdict -from django.contrib.auth.models import User from django.test.client import RequestFactory import json import logging +from lms.djangoapps.courseware.entrance_exams import get_entrance_exam_score from openedx.core.lib.gating import api as gating_api from opaque_keys.edx.keys import UsageKey -from lms.djangoapps.courseware.entrance_exams import get_entrance_exam_score -from lms.djangoapps.grades.module_grades import get_module_score +from xmodule.modulestore.django import modulestore from util import milestones_helpers log = logging.getLogger(__name__) -def _get_xblock_parent(xblock, category=None): - """ - Returns the parent of the given XBlock. If an optional category is supplied, - traverses the ancestors of the XBlock and returns the first with the - given category. - - Arguments: - xblock (XBlock): Get the parent of this XBlock - category (str): Find an ancestor with this category (e.g. sequential) - """ - parent = xblock.get_parent() - if parent and category: - if parent.category == category: - return parent - else: - return _get_xblock_parent(parent, category) - return parent - - @gating_api.gating_enabled(default=False) -def evaluate_prerequisite(course, block, user_id): +def evaluate_prerequisite(course, subsection_grade, user): """ - Finds the parent subsection of the content in the course and evaluates - any milestone relationships attached to that subsection. If the calculated - grade of the prerequisite subsection meets the minimum score required by - dependent subsections, the related milestone will be fulfilled for the user. - - Arguments: - course (CourseModule): The course - prereq_content_key (UsageKey): The prerequisite content usage key - user_id (int): ID of User for which evaluation should occur - - Returns: - None + Evaluates any gating milestone relationships attached to the given + subsection. If the subsection_grade meets the minimum score required + by dependent subsections, the related milestone will be marked + fulfilled for the user. """ - sequential = _get_xblock_parent(block, 'sequential') - if sequential: - prereq_milestone = gating_api.get_gating_milestone( - course.id, - sequential.location.for_branch(None), - 'fulfills' - ) - if prereq_milestone: - gated_content_milestones = defaultdict(list) - for milestone in gating_api.find_gating_milestones(course.id, None, 'requires'): - gated_content_milestones[milestone['id']].append(milestone) + prereq_milestone = gating_api.get_gating_milestone(course.id, subsection_grade.location, 'fulfills') + if prereq_milestone: + gated_content_milestones = defaultdict(list) + for milestone in gating_api.find_gating_milestones(course.id, content_key=None, relationship='requires'): + gated_content_milestones[milestone['id']].append(milestone) - gated_content = gated_content_milestones.get(prereq_milestone['id']) - if gated_content: - user = User.objects.get(id=user_id) - score = get_module_score(user, course, sequential) * 100 - for milestone in gated_content: - # Default minimum score to 100 - min_score = 100 - requirements = milestone.get('requirements') - if requirements: - try: - min_score = int(requirements.get('min_score')) - except (ValueError, TypeError): - log.warning( - 'Failed to find minimum score for gating milestone %s, defaulting to 100', - json.dumps(milestone) - ) - - if score >= min_score: - milestones_helpers.add_user_milestone({'id': user_id}, prereq_milestone) - else: - milestones_helpers.remove_user_milestone({'id': user_id}, prereq_milestone) + gated_content = gated_content_milestones.get(prereq_milestone['id']) + if gated_content: + for milestone in gated_content: + min_percentage = _get_minimum_required_percentage(milestone) + subsection_percentage = _get_subsection_percentage(subsection_grade) + if subsection_percentage >= min_percentage: + milestones_helpers.add_user_milestone({'id': user.id}, prereq_milestone) + else: + milestones_helpers.remove_user_milestone({'id': user.id}, prereq_milestone) -def evaluate_entrance_exam(course, block, user_id): +def _get_minimum_required_percentage(milestone): """ - Update milestone fulfillments for the specified content module + Returns the minimum percentage requirement for the given milestone. """ - # Fulfillment Use Case: Entrance Exam - # If this module is part of an entrance exam, we'll need to see if the student - # has reached the point at which they can collect the associated milestone - if milestones_helpers.is_entrance_exams_enabled(): - entrance_exam_enabled = getattr(course, 'entrance_exam_enabled', False) - in_entrance_exam = getattr(block, 'in_entrance_exam', False) - if entrance_exam_enabled and in_entrance_exam: + # Default minimum score to 100 + min_score = 100 + requirements = milestone.get('requirements') + if requirements: + try: + min_score = int(requirements.get('min_score')) + except (ValueError, TypeError): + log.warning( + 'Failed to find minimum score for gating milestone %s, defaulting to 100', + json.dumps(milestone) + ) + return min_score + + +def _get_subsection_percentage(subsection_grade): + """ + Returns the percentage value of the given subsection_grade. + """ + if subsection_grade.graded_total.possible: + return float(subsection_grade.graded_total.earned) / float(subsection_grade.graded_total.possible) * 100.0 + else: + return 0 + + +def evaluate_entrance_exam(course, subsection_grade, user): + """ + Evaluates any entrance exam milestone relationships attached + to the given subsection. If the subsection_grade meets the + minimum score required, the dependent milestone will be marked + fulfilled for the user. + """ + if milestones_helpers.is_entrance_exams_enabled() and getattr(course, 'entrance_exam_enabled', False): + subsection = modulestore().get_item(subsection_grade.location) + in_entrance_exam = getattr(subsection, 'in_entrance_exam', False) + if in_entrance_exam: # We don't have access to the true request object in this context, but we can use a mock request = RequestFactory().request() - request.user = User.objects.get(id=user_id) + request.user = user exam_pct = get_entrance_exam_score(request, course) if exam_pct >= course.entrance_exam_minimum_score_pct: exam_key = UsageKey.from_string(course.entrance_exam_id) @@ -110,7 +92,6 @@ def evaluate_entrance_exam(course, block, user_id): exam_key, relationship=relationship_types['FULFILLS'] ) - # Add each milestone to the user's set... - user = {'id': request.user.id} + # Mark each milestone dependent on the entrance exam as fulfilled by the user. for milestone in content_milestones: - milestones_helpers.add_user_milestone(user, milestone) + milestones_helpers.add_user_milestone({'id': request.user.id}, milestone) diff --git a/lms/djangoapps/gating/signals.py b/lms/djangoapps/gating/signals.py index 88bad17149..64a29dedfa 100644 --- a/lms/djangoapps/gating/signals.py +++ b/lms/djangoapps/gating/signals.py @@ -4,25 +4,21 @@ Signal handlers for the gating djangoapp from django.dispatch import receiver from gating import api as gating_api -from lms.djangoapps.grades.signals.signals import PROBLEM_WEIGHTED_SCORE_CHANGED -from opaque_keys.edx.keys import CourseKey, UsageKey -from xmodule.modulestore.django import modulestore +from lms.djangoapps.grades.signals.signals import SUBSECTION_SCORE_CHANGED -@receiver(PROBLEM_WEIGHTED_SCORE_CHANGED) -def handle_score_changed(**kwargs): +@receiver(SUBSECTION_SCORE_CHANGED) +def evaluate_subsection_gated_milestones(**kwargs): """ - Receives the PROBLEM_WEIGHTED_SCORE_CHANGED signal sent by LMS when a student's score has changed - for a given component and triggers the evaluation of any milestone relationships - which are attached to the updated content. + Receives the SUBSECTION_SCORE_CHANGED signal and triggers the + evaluation of any milestone relationships which are attached + to the subsection. Arguments: - kwargs (dict): Contains user ID, course key, and content usage key - + kwargs (dict): Contains user, course, course_structure, subsection_grade Returns: None """ - course = modulestore().get_course(CourseKey.from_string(kwargs.get('course_id'))) - block = modulestore().get_item(UsageKey.from_string(kwargs.get('usage_id'))) - gating_api.evaluate_prerequisite(course, block, kwargs.get('user_id')) - gating_api.evaluate_entrance_exam(course, block, kwargs.get('user_id')) + subsection_grade = kwargs['subsection_grade'] + gating_api.evaluate_prerequisite(kwargs['course'], subsection_grade, kwargs.get('user')) + gating_api.evaluate_entrance_exam(kwargs['course'], subsection_grade, kwargs.get('user')) diff --git a/lms/djangoapps/gating/tests/test_api.py b/lms/djangoapps/gating/tests/test_api.py index b15930de9e..805eb96940 100644 --- a/lms/djangoapps/gating/tests/test_api.py +++ b/lms/djangoapps/gating/tests/test_api.py @@ -1,7 +1,7 @@ """ Unit tests for gating.signals module """ -from mock import patch +from mock import patch, Mock from nose.plugins.attrib import attr from ddt import ddt, data, unpack from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory @@ -11,7 +11,7 @@ from courseware.tests.helpers import LoginEnrollmentTestCase from milestones import api as milestones_api from milestones.tests.utils import MilestonesTestCaseMixin from openedx.core.lib.gating import api as gating_api -from gating.api import _get_xblock_parent, evaluate_prerequisite +from gating.api import evaluate_prerequisite class GatingTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): @@ -48,60 +48,14 @@ class GatingTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): self.seq1 = ItemFactory.create( parent_location=self.chapter1.location, category='sequential', - display_name='untitled sequential 1' + display_name='gating sequential' ) self.seq2 = ItemFactory.create( parent_location=self.chapter1.location, category='sequential', - display_name='untitled sequential 2' + display_name='gated sequential' ) - # create vertical - self.vert1 = ItemFactory.create( - parent_location=self.seq1.location, - category='vertical', - display_name='untitled vertical 1' - ) - - # create problem - self.prob1 = ItemFactory.create( - parent_location=self.vert1.location, - category='problem', - display_name='untitled problem 1' - ) - - # create orphan - self.prob2 = ItemFactory.create( - parent_location=self.course.location, - category='problem', - display_name='untitled problem 2' - ) - - -class TestGetXBlockParent(GatingTestCase): - """ - Tests for the get_xblock_parent function - """ - - def test_get_direct_parent(self): - """ Test test_get_direct_parent """ - - result = _get_xblock_parent(self.vert1) - self.assertEqual(result.location, self.seq1.location) - - def test_get_parent_with_category(self): - """ Test test_get_parent_of_category """ - result = _get_xblock_parent(self.vert1, 'sequential') - self.assertEqual(result.location, self.seq1.location) - result = _get_xblock_parent(self.vert1, 'chapter') - self.assertEqual(result.location, self.chapter1.location) - - def test_get_parent_none(self): - """ Test test_get_parent_none """ - - result = _get_xblock_parent(self.vert1, 'unit') - self.assertIsNone(result) - @attr(shard=3) @ddt @@ -114,62 +68,46 @@ class TestEvaluatePrerequisite(GatingTestCase, MilestonesTestCaseMixin): super(TestEvaluatePrerequisite, self).setUp() self.user_dict = {'id': self.user.id} self.prereq_milestone = None + self.subsection_grade = Mock(location=self.seq1.location) def _setup_gating_milestone(self, min_score): """ Setup a gating milestone for testing """ - gating_api.add_prerequisite(self.course.id, self.seq1.location) gating_api.set_required_content(self.course.id, self.seq2.location, self.seq1.location, min_score) self.prereq_milestone = gating_api.get_gating_milestone(self.course.id, self.seq1.location, 'fulfills') - @patch('gating.api.get_module_score') - @data((.5, True), (1, True), (0, False)) + @patch('gating.api._get_subsection_percentage') + @data((50, True), (100, True), (0, False)) @unpack - def test_min_score_achieved(self, module_score, result, mock_module_score): - """ Test test_min_score_achieved """ - + def test_min_score_achieved(self, module_score, result, mock_score): self._setup_gating_milestone(50) + mock_score.return_value = module_score - mock_module_score.return_value = module_score - evaluate_prerequisite(self.course, self.prob1, self.user.id) + evaluate_prerequisite(self.course, self.subsection_grade, self.user) self.assertEqual(milestones_api.user_has_milestone(self.user_dict, self.prereq_milestone), result) @patch('gating.api.log.warning') - @patch('gating.api.get_module_score') - @data((.5, False), (1, True)) + @patch('gating.api._get_subsection_percentage') + @data((50, False), (100, True)) @unpack - def test_invalid_min_score(self, module_score, result, mock_module_score, mock_log): - """ Test test_invalid_min_score """ - + def test_invalid_min_score(self, module_score, result, mock_score, mock_log): self._setup_gating_milestone(None) + mock_score.return_value = module_score - mock_module_score.return_value = module_score - evaluate_prerequisite(self.course, self.prob1, self.user.id) + evaluate_prerequisite(self.course, self.subsection_grade, self.user) self.assertEqual(milestones_api.user_has_milestone(self.user_dict, self.prereq_milestone), result) self.assertTrue(mock_log.called) - @patch('gating.api.get_module_score') - def test_orphaned_xblock(self, mock_module_score): - """ Test test_orphaned_xblock """ + @patch('gating.api._get_subsection_percentage') + def test_no_prerequisites(self, mock_score): + evaluate_prerequisite(self.course, self.subsection_grade, self.user) + self.assertFalse(mock_score.called) - evaluate_prerequisite(self.course, self.prob2, self.user.id) - self.assertFalse(mock_module_score.called) - - @patch('gating.api.get_module_score') - def test_no_prerequisites(self, mock_module_score): - """ Test test_no_prerequisites """ - - evaluate_prerequisite(self.course, self.prob1, self.user.id) - self.assertFalse(mock_module_score.called) - - @patch('gating.api.get_module_score') - def test_no_gated_content(self, mock_module_score): - """ Test test_no_gated_content """ - - # Setup gating milestones data + @patch('gating.api._get_subsection_percentage') + def test_no_gated_content(self, mock_score): gating_api.add_prerequisite(self.course.id, self.seq1.location) - evaluate_prerequisite(self.course, self.prob1, self.user.id) - self.assertFalse(mock_module_score.called) + evaluate_prerequisite(self.course, self.subsection_grade, self.user) + self.assertFalse(mock_score.called) diff --git a/lms/djangoapps/gating/tests/test_integration.py b/lms/djangoapps/gating/tests/test_integration.py new file mode 100644 index 0000000000..4bfa8c5df5 --- /dev/null +++ b/lms/djangoapps/gating/tests/test_integration.py @@ -0,0 +1,192 @@ +""" +Integration tests for gated content. +""" +import ddt +from nose.plugins.attrib import attr +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory + +from lms.djangoapps.courseware.access import has_access +from lms.djangoapps.grades.tests.utils import answer_problem +from lms.djangoapps.grades.new.course_grade import CourseGradeFactory +from milestones import api as milestones_api +from milestones.tests.utils import MilestonesTestCaseMixin +from openedx.core.djangolib.testing.utils import get_mock_request +from openedx.core.lib.gating import api as gating_api +from request_cache.middleware import RequestCache +from student.tests.factories import UserFactory +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase + + +@attr(shard=3) +@ddt.ddt +class TestGatedContent(MilestonesTestCaseMixin, SharedModuleStoreTestCase): + """ + Base TestCase class for setting up a basic course structure + and testing the gating feature + """ + @classmethod + def setUpClass(cls): + super(TestGatedContent, cls).setUpClass() + cls.set_up_course() + + def setUp(self): + super(TestGatedContent, self).setUp() + self.setup_gating_milestone(50) + self.non_staff_user = UserFactory() + self.staff_user = UserFactory(is_staff=True, is_superuser=True) + self.request = get_mock_request(self.non_staff_user) + + @classmethod + def set_up_course(cls): + """ + Set up a course for testing gated content. + """ + cls.course = CourseFactory.create( + org='edX', + number='EDX101', + run='EDX101_RUN1', + display_name='edX 101' + ) + with modulestore().bulk_operations(cls.course.id): + cls.course.enable_subsection_gating = True + grading_policy = { + "GRADER": [{ + "type": "Homework", + "min_count": 3, + "drop_count": 0, + "short_label": "HW", + "weight": 1.0 + }] + } + cls.course.grading_policy = grading_policy + cls.course.save() + cls.store.update_item(cls.course, 0) + + # create chapter + cls.chapter1 = ItemFactory.create( + parent_location=cls.course.location, + category='chapter', + display_name='chapter 1' + ) + + # create sequentials + cls.seq1 = ItemFactory.create( + parent_location=cls.chapter1.location, + category='sequential', + display_name='gating sequential 1', + graded=True, + format='Homework', + ) + cls.seq2 = ItemFactory.create( + parent_location=cls.chapter1.location, + category='sequential', + display_name='gated sequential 2', + graded=True, + format='Homework', + ) + cls.seq3 = ItemFactory.create( + parent_location=cls.chapter1.location, + category='sequential', + display_name='sequential 3', + graded=True, + format='Homework', + ) + + # create problem + cls.gating_prob1 = ItemFactory.create( + parent_location=cls.seq1.location, + category='problem', + display_name='gating problem 1', + ) + cls.gated_prob2 = ItemFactory.create( + parent_location=cls.seq2.location, + category='problem', + display_name='gated problem 2', + ) + cls.prob3 = ItemFactory.create( + parent_location=cls.seq3.location, + category='problem', + display_name='problem 3', + ) + + def setup_gating_milestone(self, min_score): + """ + Setup a gating milestone for testing. + Gating content: seq1 (must be fulfilled before access to seq2) + Gated content: seq2 (requires completion of seq1 before access) + """ + gating_api.add_prerequisite(self.course.id, str(self.seq1.location)) + gating_api.set_required_content(self.course.id, str(self.seq2.location), str(self.seq1.location), min_score) + self.prereq_milestone = gating_api.get_gating_milestone(self.course.id, self.seq1.location, 'fulfills') + + def assert_access_to_gated_content(self, user, expected_access): + """ + Verifies access to gated content for the given user is as expected. + """ + # clear the request cache to flush any cached access results + RequestCache.clear_request_cache() + + # access to gating content (seq1) remains constant + self.assertTrue(bool(has_access(user, 'load', self.seq1, self.course.id))) + + # access to gated content (seq2) is as expected + self.assertEquals(bool(has_access(user, 'load', self.seq2, self.course.id)), expected_access) + + def assert_user_has_prereq_milestone(self, user, expected_has_milestone): + """ + Verifies whether or not the user has the prereq milestone + """ + self.assertEquals( + milestones_api.user_has_milestone({'id': user.id}, self.prereq_milestone), + expected_has_milestone, + ) + + def assert_course_grade(self, user, expected_percent): + """ + Verifies the given user's course grade is the expected percentage. + Also verifies the user's grade information contains values for + all problems in the course, whether or not they are currently + gated. + """ + course_grade = CourseGradeFactory().create(user, self.course) + for prob in [self.gating_prob1, self.gated_prob2, self.prob3]: + self.assertIn(prob.location, course_grade.locations_to_scores) + + self.assertEquals(course_grade.percent, expected_percent) + + def test_gated_for_nonstaff(self): + self.assert_user_has_prereq_milestone(self.non_staff_user, expected_has_milestone=False) + self.assert_access_to_gated_content(self.non_staff_user, expected_access=False) + + def test_not_gated_for_staff(self): + self.assert_user_has_prereq_milestone(self.staff_user, expected_has_milestone=False) + self.assert_access_to_gated_content(self.staff_user, expected_access=True) + + def test_gated_content_always_in_grades(self): + # start with a grade from a non-gated subsection + answer_problem(self.course, self.request, self.prob3, 10, 10) + + # verify gated status and overall course grade percentage + self.assert_user_has_prereq_milestone(self.non_staff_user, expected_has_milestone=False) + self.assert_access_to_gated_content(self.non_staff_user, expected_access=False) + self.assert_course_grade(self.non_staff_user, .33) + + # fulfill the gated requirements + answer_problem(self.course, self.request, self.gating_prob1, 10, 10) + + # verify gated status and overall course grade percentage + self.assert_user_has_prereq_milestone(self.non_staff_user, expected_has_milestone=True) + self.assert_access_to_gated_content(self.non_staff_user, expected_access=True) + self.assert_course_grade(self.non_staff_user, .67) + + @ddt.data((1, 1, True), (1, 2, True), (1, 3, False), (0, 1, False)) + @ddt.unpack + def test_ungating_when_fulfilled(self, earned, max_possible, result): + self.assert_user_has_prereq_milestone(self.non_staff_user, expected_has_milestone=False) + self.assert_access_to_gated_content(self.non_staff_user, expected_access=False) + + answer_problem(self.course, self.request, self.gating_prob1, earned, max_possible) + + self.assert_user_has_prereq_milestone(self.non_staff_user, expected_has_milestone=result) + self.assert_access_to_gated_content(self.non_staff_user, expected_access=result) diff --git a/lms/djangoapps/gating/tests/test_signals.py b/lms/djangoapps/gating/tests/test_signals.py index bb3eb41910..f88dbf15ae 100644 --- a/lms/djangoapps/gating/tests/test_signals.py +++ b/lms/djangoapps/gating/tests/test_signals.py @@ -1,14 +1,14 @@ """ Unit tests for gating.signals module """ -from mock import patch +from mock import patch, Mock from student.tests.factories import UserFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.django import modulestore -from gating.signals import handle_score_changed +from gating.signals import evaluate_subsection_gated_milestones class TestHandleScoreChanged(ModuleStoreTestCase): @@ -19,32 +19,26 @@ class TestHandleScoreChanged(ModuleStoreTestCase): super(TestHandleScoreChanged, self).setUp() self.course = CourseFactory.create(org='TestX', number='TS01', run='2016_Q1') self.user = UserFactory.create() - self.test_usage_key = self.course.location + self.subsection_grade = Mock() - @patch('gating.signals.gating_api.evaluate_prerequisite') - def test_gating_enabled(self, mock_evaluate): - """ Test evaluate_prerequisite is called when course.enable_subsection_gating is True """ + @patch('lms.djangoapps.gating.api.gating_api.get_gating_milestone') + def test_gating_enabled(self, mock_gating_milestone): self.course.enable_subsection_gating = True modulestore().update_item(self.course, 0) - handle_score_changed( + evaluate_subsection_gated_milestones( sender=None, - points_possible=1, - points_earned=1, - user_id=self.user.id, - course_id=unicode(self.course.id), - usage_id=unicode(self.test_usage_key) + user=self.user, + course=self.course, + subsection_grade=self.subsection_grade, ) - mock_evaluate.assert_called_with(self.course, self.course, self.user.id) # pylint: disable=no-member + self.assertTrue(mock_gating_milestone.called) - @patch('gating.signals.gating_api.evaluate_prerequisite') - def test_gating_disabled(self, mock_evaluate): - """ Test evaluate_prerequisite is not called when course.enable_subsection_gating is False """ - handle_score_changed( + @patch('lms.djangoapps.gating.api.gating_api.get_gating_milestone') + def test_gating_disabled(self, mock_gating_milestone): + evaluate_subsection_gated_milestones( sender=None, - points_possible=1, - points_earned=1, - user_id=self.user.id, - course_id=unicode(self.course.id), - usage_id=unicode(self.test_usage_key) + user=self.user, + course=self.course, + subsection_grade=self.subsection_grade, ) - mock_evaluate.assert_not_called() + self.assertFalse(mock_gating_milestone.called) diff --git a/lms/djangoapps/grades/module_grades.py b/lms/djangoapps/grades/module_grades.py deleted file mode 100644 index a0e95a57e9..0000000000 --- a/lms/djangoapps/grades/module_grades.py +++ /dev/null @@ -1,96 +0,0 @@ -""" -Functionality for module-level grades. -""" -# TODO The score computation in this file is not accurate -# since it is summing percentages instead of computing a -# final percentage of the individual sums. -# Regardless, this file and its code should be removed soon -# as part of TNL-5062. - -from django.test.client import RequestFactory -from courseware.model_data import FieldDataCache, ScoresClient -from courseware.module_render import get_module_for_descriptor -from opaque_keys.edx.locator import BlockUsageLocator -from util.module_utils import yield_dynamic_descriptor_descendants - - -def _get_mock_request(student): - """ - Make a fake request because grading code expects to be able to look at - the request. We have to attach the correct user to the request before - grading that student. - """ - request = RequestFactory().get('/') - request.user = student - return request - - -def _calculate_score_for_modules(user_id, course, modules): - """ - Calculates the cumulative score (percent) of the given modules - """ - - # removing branch and version from exam modules locator - # otherwise student module would not return scores since module usage keys would not match - modules = [m for m in modules] - locations = [ - BlockUsageLocator( - course_key=course.id, - block_type=module.location.block_type, - block_id=module.location.block_id - ) - if isinstance(module.location, BlockUsageLocator) and module.location.version - else module.location - for module in modules - ] - - scores_client = ScoresClient(course.id, user_id) - scores_client.fetch_scores(locations) - - # Iterate over all of the exam modules to get score percentage of user for each of them - module_percentages = [] - ignore_categories = ['course', 'chapter', 'sequential', 'vertical', 'randomize', 'library_content'] - for index, module in enumerate(modules): - if module.category not in ignore_categories and (module.graded or module.has_score): - module_score = scores_client.get(locations[index]) - if module_score: - correct = module_score.correct or 0 - total = module_score.total or 1 - module_percentages.append(correct / total) - - return sum(module_percentages) / float(len(module_percentages)) if module_percentages else 0 - - -def get_module_score(user, course, module): - """ - Collects all children of the given module and calculates the cumulative - score for this set of modules for the given user. - - Arguments: - user (User): The user - course (CourseModule): The course - module (XBlock): The module - - Returns: - float: The cumulative score - """ - def inner_get_module(descriptor): - """ - Delegate to get_module_for_descriptor - """ - field_data_cache = FieldDataCache([descriptor], course.id, user) - return get_module_for_descriptor( - user, - _get_mock_request(user), - descriptor, - field_data_cache, - course.id, - course=course - ) - - modules = yield_dynamic_descriptor_descendants( - module, - user.id, - inner_get_module - ) - return _calculate_score_for_modules(user.id, course, modules) diff --git a/lms/djangoapps/grades/tests/test_grades.py b/lms/djangoapps/grades/tests/test_grades.py index 1b6975e825..8caecfc573 100644 --- a/lms/djangoapps/grades/tests/test_grades.py +++ b/lms/djangoapps/grades/tests/test_grades.py @@ -8,9 +8,6 @@ from mock import patch from nose.plugins.attrib import attr from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory -from courseware.model_data import set_score -from courseware.tests.helpers import LoginEnrollmentTestCase - from lms.djangoapps.course_blocks.api import get_course_blocks from openedx.core.djangoapps.content.block_structure.factory import BlockStructureFactory from openedx.core.djangolib.testing.utils import get_mock_request @@ -22,7 +19,6 @@ from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from .utils import answer_problem -from ..module_grades import get_module_score from ..new.course_grade import CourseGradeFactory from ..new.subsection_grade import SubsectionGradeFactory @@ -334,195 +330,3 @@ class TestScoreForModule(SharedModuleStoreTestCase): earned, possible = self.course_grade.score_for_module(self.m.location) self.assertEqual(earned, 0) self.assertEqual(possible, 0) - - -class TestGetModuleScore(LoginEnrollmentTestCase, SharedModuleStoreTestCase): - """ - Test get_module_score - """ - @classmethod - def setUpClass(cls): - super(TestGetModuleScore, cls).setUpClass() - cls.course = CourseFactory.create() - with cls.store.bulk_operations(cls.course.id): - cls.chapter = ItemFactory.create( - parent=cls.course, - category="chapter", - display_name="Test Chapter" - ) - cls.seq1 = ItemFactory.create( - parent=cls.chapter, - category='sequential', - display_name="Test Sequential 1", - graded=True - ) - cls.seq2 = ItemFactory.create( - parent=cls.chapter, - category='sequential', - display_name="Test Sequential 2", - graded=True - ) - cls.seq3 = ItemFactory.create( - parent=cls.chapter, - category='sequential', - display_name="Test Sequential 3", - graded=True - ) - cls.vert1 = ItemFactory.create( - parent=cls.seq1, - category='vertical', - display_name='Test Vertical 1' - ) - cls.vert2 = ItemFactory.create( - parent=cls.seq2, - category='vertical', - display_name='Test Vertical 2' - ) - cls.vert3 = ItemFactory.create( - parent=cls.seq3, - category='vertical', - display_name='Test Vertical 3' - ) - cls.randomize = ItemFactory.create( - parent=cls.vert2, - category='randomize', - display_name='Test Randomize' - ) - cls.library_content = ItemFactory.create( - parent=cls.vert3, - category='library_content', - display_name='Test Library Content' - ) - problem_xml = MultipleChoiceResponseXMLFactory().build_xml( - question_text='The correct answer is Choice 3', - choices=[False, False, True, False], - choice_names=['choice_0', 'choice_1', 'choice_2', 'choice_3'] - ) - cls.problem1 = ItemFactory.create( - parent=cls.vert1, - category="problem", - display_name="Test Problem 1", - data=problem_xml - ) - cls.problem2 = ItemFactory.create( - parent=cls.vert1, - category="problem", - display_name="Test Problem 2", - data=problem_xml - ) - cls.problem3 = ItemFactory.create( - parent=cls.randomize, - category="problem", - display_name="Test Problem 3", - data=problem_xml - ) - cls.problem4 = ItemFactory.create( - parent=cls.randomize, - category="problem", - display_name="Test Problem 4", - data=problem_xml - ) - - cls.problem5 = ItemFactory.create( - parent=cls.library_content, - category="problem", - display_name="Test Problem 5", - data=problem_xml - ) - cls.problem6 = ItemFactory.create( - parent=cls.library_content, - category="problem", - display_name="Test Problem 6", - data=problem_xml - ) - - def setUp(self): - """ - Set up test course - """ - super(TestGetModuleScore, self).setUp() - - self.request = get_mock_request(UserFactory()) - self.client.login(username=self.request.user.username, password="test") - CourseEnrollment.enroll(self.request.user, self.course.id) - - self.course_structure = get_course_blocks(self.request.user, self.course.location) - - # warm up the score cache to allow accurate query counts, even if tests are run in random order - get_module_score(self.request.user, self.course, self.seq1) - - def test_subsection_scores(self): - """ - Test test_get_module_score - """ - # One query is for getting the list of disabled XBlocks (which is - # then stored in the request). - with self.assertNumQueries(1): - score = get_module_score(self.request.user, self.course, self.seq1) - new_score = SubsectionGradeFactory(self.request.user, self.course, self.course_structure).create(self.seq1) - self.assertEqual(score, 0) - self.assertEqual(new_score.all_total.earned, 0) - - answer_problem(self.course, self.request, self.problem1) - answer_problem(self.course, self.request, self.problem2) - - with self.assertNumQueries(1): - score = get_module_score(self.request.user, self.course, self.seq1) - new_score = SubsectionGradeFactory(self.request.user, self.course, self.course_structure).create(self.seq1) - self.assertEqual(score, 1.0) - self.assertEqual(new_score.all_total.earned, 2.0) - # These differ because get_module_score normalizes the subsection score - # to 1, which can cause incorrect aggregation behavior that will be - # fixed by TNL-5062. - - answer_problem(self.course, self.request, self.problem1) - answer_problem(self.course, self.request, self.problem2, 0) - - with self.assertNumQueries(1): - score = get_module_score(self.request.user, self.course, self.seq1) - new_score = SubsectionGradeFactory(self.request.user, self.course, self.course_structure).create(self.seq1) - self.assertEqual(score, .5) - self.assertEqual(new_score.all_total.earned, 1.0) - - def test_get_module_score_with_empty_score(self): - """ - Test test_get_module_score_with_empty_score - """ - set_score(self.request.user.id, self.problem1.location, None, None) # pylint: disable=no-member - set_score(self.request.user.id, self.problem2.location, None, None) # pylint: disable=no-member - - with self.assertNumQueries(1): - score = get_module_score(self.request.user, self.course, self.seq1) - self.assertEqual(score, 0) - - answer_problem(self.course, self.request, self.problem1) - - with self.assertNumQueries(1): - score = get_module_score(self.request.user, self.course, self.seq1) - self.assertEqual(score, 0.5) - - answer_problem(self.course, self.request, self.problem2) - - with self.assertNumQueries(1): - score = get_module_score(self.request.user, self.course, self.seq1) - self.assertEqual(score, 1.0) - - def test_get_module_score_with_randomize(self): - """ - Test test_get_module_score_with_randomize - """ - answer_problem(self.course, self.request, self.problem3) - answer_problem(self.course, self.request, self.problem4) - - score = get_module_score(self.request.user, self.course, self.seq2) - self.assertEqual(score, 1.0) - - def test_get_module_score_with_library_content(self): - """ - Test test_get_module_score_with_library_content - """ - answer_problem(self.course, self.request, self.problem5) - answer_problem(self.course, self.request, self.problem6) - - score = get_module_score(self.request.user, self.course, self.seq3) - self.assertEqual(score, 1.0) diff --git a/lms/djangoapps/mobile_api/tests/test_milestones.py b/lms/djangoapps/mobile_api/tests/test_milestones.py index dba2faa5c9..bd200bb97d 100644 --- a/lms/djangoapps/mobile_api/tests/test_milestones.py +++ b/lms/djangoapps/mobile_api/tests/test_milestones.py @@ -11,7 +11,6 @@ from util.milestones_helpers import ( add_prerequisite_course, fulfill_course_milestone, ) -from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory @@ -85,28 +84,36 @@ class MobileAPIMilestonesMixin(object): def _add_entrance_exam(self): """ Sets up entrance exam """ - self.course.entrance_exam_enabled = True + with self.store.bulk_operations(self.course.id): + self.course.entrance_exam_enabled = True - self.entrance_exam = ItemFactory.create( # pylint: disable=attribute-defined-outside-init - parent=self.course, - category="chapter", - display_name="Entrance Exam Chapter", - is_entrance_exam=True, - in_entrance_exam=True - ) - self.problem_1 = ItemFactory.create( # pylint: disable=attribute-defined-outside-init - parent=self.entrance_exam, - category='problem', - display_name="The Only Exam Problem", - graded=True, - in_entrance_exam=True - ) + self.entrance_exam = ItemFactory.create( # pylint: disable=attribute-defined-outside-init + parent=self.course, + category="chapter", + display_name="Entrance Exam Chapter", + is_entrance_exam=True, + in_entrance_exam=True, + ) + self.subsection_1 = ItemFactory.create( # pylint: disable=attribute-defined-outside-init + parent=self.entrance_exam, + category='sequential', + display_name="The Only Exam Sequential", + graded=True, + in_entrance_exam=True, + ) + self.problem_1 = ItemFactory.create( # pylint: disable=attribute-defined-outside-init + parent=self.subsection_1, + category='problem', + display_name="The Only Exam Problem", + graded=True, + in_entrance_exam=True, + ) - add_entrance_exam_milestone(self.course, self.entrance_exam) + add_entrance_exam_milestone(self.course, self.entrance_exam) - self.course.entrance_exam_minimum_score_pct = 0.50 - self.course.entrance_exam_id = unicode(self.entrance_exam.location) - modulestore().update_item(self.course, self.user.id) + self.course.entrance_exam_minimum_score_pct = 0.50 + self.course.entrance_exam_id = unicode(self.entrance_exam.location) + self.store.update_item(self.course, self.user.id) def _add_prerequisite_course(self): """ Helper method to set up the prerequisite course """ From d117222f7c8252648ce02dab8e0cc3f66a742ee3 Mon Sep 17 00:00:00 2001 From: uzairr Date: Thu, 9 Mar 2017 07:25:47 +0000 Subject: [PATCH 03/51] Add timeout in request to xqueue --- common/lib/capa/capa/xqueue_interface.py | 16 ++++++++++++---- .../xmodule/xmodule/tests/test_capa_module.py | 17 +++++++++++++++++ .../tests/test_submitting_problems.py | 2 +- 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/common/lib/capa/capa/xqueue_interface.py b/common/lib/capa/capa/xqueue_interface.py index aa327dc285..fe47e9b597 100644 --- a/common/lib/capa/capa/xqueue_interface.py +++ b/common/lib/capa/capa/xqueue_interface.py @@ -15,6 +15,8 @@ XQUEUE_METRIC_NAME = 'edxapp.xqueue' # Wait time for response from Xqueue. XQUEUE_TIMEOUT = 35 # seconds +CONNECT_TIMEOUT = 3.05 # seconds +READ_TIMEOUT = 10 # seconds def make_hashkey(seed): @@ -134,12 +136,18 @@ class XQueueInterface(object): def _http_post(self, url, data, files=None): try: - r = self.session.post(url, data=data, files=files) + response = self.session.post( + url, data=data, files=files, timeout=(CONNECT_TIMEOUT, READ_TIMEOUT) + ) except requests.exceptions.ConnectionError, err: log.error(err) return (1, 'cannot connect to server') - if r.status_code not in [200]: - return (1, 'unexpected HTTP status code [%d]' % r.status_code) + except requests.exceptions.ReadTimeout, err: + log.error(err) + return (1, 'failed to read from the server') - return parse_xreply(r.text) + if response.status_code not in [200]: + return (1, 'unexpected HTTP status code [%d]' % response.status_code) + + return parse_xreply(response.text) diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index e7653ef81c..3af41a46fc 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -8,6 +8,7 @@ Tests of the Capa XModule import datetime import json import random +import requests import os import textwrap import unittest @@ -242,6 +243,22 @@ class CapaModuleTest(unittest.TestCase): problem = CapaFactory.create() self.assertFalse(problem.answer_available()) + @ddt.data( + (requests.exceptions.ReadTimeout, (1, 'failed to read from the server')), + (requests.exceptions.ConnectionError, (1, 'cannot connect to server')), + ) + @ddt.unpack + def test_xqueue_request_exception(self, exception, result): + """ + Makes sure that platform will raise appropriate exception in case of + connect/read timeout(s) to request to xqueue + """ + xqueue_interface = XQueueInterface("http://example.com/xqueue", Mock()) + with patch.object(xqueue_interface.session, 'post', side_effect=exception): + # pylint: disable = protected-access + response = xqueue_interface._http_post('http://some/fake/url', {}) + self.assertEqual(response, result) + def test_showanswer_attempted(self): problem = CapaFactory.create(showanswer='attempted') self.assertFalse(problem.answer_available()) diff --git a/lms/djangoapps/courseware/tests/test_submitting_problems.py b/lms/djangoapps/courseware/tests/test_submitting_problems.py index 9c7ffc3a6b..c0e2602ed8 100644 --- a/lms/djangoapps/courseware/tests/test_submitting_problems.py +++ b/lms/djangoapps/courseware/tests/test_submitting_problems.py @@ -818,7 +818,7 @@ class ProblemWithUploadedFilesTest(TestSubmittingProblems): self.assertEqual(name, "post") self.assertEqual(len(args), 1) self.assertTrue(args[0].endswith("/submit/")) - self.assertItemsEqual(kwargs.keys(), ["files", "data"]) + self.assertItemsEqual(kwargs.keys(), ["files", "data", "timeout"]) self.assertItemsEqual(kwargs['files'].keys(), filenames.split()) From 9ee79a572913eb527e11fd7e7e90d89e4c19e990 Mon Sep 17 00:00:00 2001 From: "J. Cliff Dyer" Date: Fri, 17 Mar 2017 12:40:19 -0400 Subject: [PATCH 04/51] Ensure ScorableXBlocks have rescore button. --- openedx/core/lib/xblock_utils/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx/core/lib/xblock_utils/__init__.py b/openedx/core/lib/xblock_utils/__init__.py index 4474b832b3..73b46f2b01 100644 --- a/openedx/core/lib/xblock_utils/__init__.py +++ b/openedx/core/lib/xblock_utils/__init__.py @@ -383,7 +383,7 @@ def add_staff_markup(user, has_instructor_access, disable_staff_debug_info, bloc 'is_released': is_released, 'has_instructor_access': has_instructor_access, 'can_reset_attempts': 'attempts' in block.fields, - 'can_rescore_problem': hasattr(block, 'rescore_problem'), + 'can_rescore_problem': any(hasattr(block, rescore) for rescore in ['rescore_problem', 'rescore']), 'disable_staff_debug_info': disable_staff_debug_info, } return wrap_fragment(frag, render_to_string("staff_problem_info.html", staff_context)) From 54d938cab5317c27ee8587fc79287228eba165e6 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Tue, 14 Mar 2017 22:48:39 -0400 Subject: [PATCH 05/51] Fix grading for Entrance Exams TNL-6227 --- common/lib/xmodule/xmodule/course_module.py | 3 + lms/djangoapps/courseware/entrance_exams.py | 108 +----------------- lms/djangoapps/courseware/module_render.py | 6 +- lms/djangoapps/courseware/tabs.py | 4 +- .../courseware/tests/test_entrance_exam.py | 52 +++------ .../tests/test_submitting_problems.py | 12 +- lms/djangoapps/courseware/views/index.py | 30 +++-- lms/djangoapps/courseware/views/views.py | 6 +- lms/djangoapps/gating/api.py | 65 +++++++---- lms/djangoapps/gating/signals.py | 17 ++- lms/djangoapps/grades/new/course_grade.py | 34 ++++-- 11 files changed, 138 insertions(+), 199 deletions(-) diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 95af81a9b9..9c39c7b549 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -675,6 +675,9 @@ class CourseFields(object): scope=Scope.settings, ) + # Note: Although users enter the entrance exam minimum score + # as a percentage value, it is internally converted and stored + # as a decimal value less than 1. entrance_exam_minimum_score_pct = Float( display_name=_("Entrance Exam Minimum Score (%)"), help=_( diff --git a/lms/djangoapps/courseware/entrance_exams.py b/lms/djangoapps/courseware/entrance_exams.py index 3968229fd1..7716d462df 100644 --- a/lms/djangoapps/courseware/entrance_exams.py +++ b/lms/djangoapps/courseware/entrance_exams.py @@ -43,118 +43,16 @@ def user_can_skip_entrance_exam(user, course): return False -def user_has_passed_entrance_exam(request, course): +def user_has_passed_entrance_exam(user, course): """ Checks to see if the user has attained a sufficient score to pass the exam Begin by short-circuiting if the course does not have an entrance exam """ if not course_has_entrance_exam(course): return True - if not request.user.is_authenticated(): + if not user.is_authenticated(): return False - entrance_exam_score = get_entrance_exam_score(request, course) - if entrance_exam_score >= course.entrance_exam_minimum_score_pct: - return True - return False - - -# pylint: disable=invalid-name -def user_must_complete_entrance_exam(request, user, course): - """ - Some courses can be gated on an Entrance Exam, which is a specially-configured chapter module which - presents users with a problem set which they must complete. This particular workflow determines - whether or not the user is allowed to clear the Entrance Exam gate and access the rest of the course. - """ - # First, let's see if the user is allowed to skip - if user_can_skip_entrance_exam(user, course): - return False - # If they can't actually skip the exam, we'll need to see if they've already passed it - if user_has_passed_entrance_exam(request, course): - return False - # Can't skip, haven't passed, must take the exam - return True - - -def _calculate_entrance_exam_score(user, course_descriptor, exam_modules): - """ - Calculates the score (percent) of the entrance exam using the provided modules - """ - student_module_dict = {} - scores_client = ScoresClient(course_descriptor.id, user.id) - # removing branch and version from exam modules locator - # otherwise student module would not return scores since module usage keys would not match - locations = [ - BlockUsageLocator( - course_key=course_descriptor.id, - block_type=exam_module.location.block_type, - block_id=exam_module.location.block_id - ) - if isinstance(exam_module.location, BlockUsageLocator) and exam_module.location.version - else exam_module.location - for exam_module in exam_modules - ] - scores_client.fetch_scores(locations) - - # Iterate over all of the exam modules to get score of user for each of them - for index, exam_module in enumerate(exam_modules): - exam_module_score = scores_client.get(locations[index]) - if exam_module_score: - student_module_dict[unicode(locations[index])] = { - 'grade': exam_module_score.correct, - 'max_grade': exam_module_score.total - } - exam_percentage = 0 - module_percentages = [] - ignore_categories = ['course', 'chapter', 'sequential', 'vertical'] - - for index, module in enumerate(exam_modules): - if module.graded and module.category not in ignore_categories: - module_percentage = 0 - module_location = unicode(locations[index]) - if module_location in student_module_dict and student_module_dict[module_location]['max_grade']: - student_module = student_module_dict[module_location] - module_percentage = student_module['grade'] / student_module['max_grade'] - - module_percentages.append(module_percentage) - if module_percentages: - exam_percentage = sum(module_percentages) / float(len(module_percentages)) - return exam_percentage - - -def get_entrance_exam_score(request, course): - """ - Gather the set of modules which comprise the entrance exam - Note that 'request' may not actually be a genuine request, due to the - circular nature of module_render calling entrance_exams and get_module_for_descriptor - being used here. In some use cases, the caller is actually mocking a request, although - in these scenarios the 'user' child object can be trusted and used as expected. - It's a much larger refactoring job to break this legacy mess apart, unfortunately. - """ - exam_key = UsageKey.from_string(course.entrance_exam_id) - exam_descriptor = modulestore().get_item(exam_key) - - def inner_get_module(descriptor): - """ - Delegate to get_module_for_descriptor (imported here to avoid circular reference) - """ - from courseware.module_render import get_module_for_descriptor - field_data_cache = FieldDataCache([descriptor], course.id, request.user) - return get_module_for_descriptor( - request.user, - request, - descriptor, - field_data_cache, - course.id, - course=course - ) - - exam_module_generators = yield_dynamic_descriptor_descendants( - exam_descriptor, - request.user.id, - inner_get_module - ) - exam_modules = [module for module in exam_module_generators] - return _calculate_entrance_exam_score(request.user, course, exam_modules) + return get_entrance_exam_content(user, course) is None def get_entrance_exam_content(user, course): diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index d541be7dca..a4287935b7 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -33,7 +33,7 @@ from xblock.reference.plugins import FSService import static_replace from courseware.access import has_access, get_user_role from courseware.entrance_exams import ( - user_must_complete_entrance_exam, + user_can_skip_entrance_exam, user_has_passed_entrance_exam ) from courseware.masquerade import ( @@ -164,7 +164,7 @@ def toc_for_course(user, request, course, active_chapter, active_section, field_ required_content = milestones_helpers.get_required_content(course, user) # The user may not actually have to complete the entrance exam, if one is required - if not user_must_complete_entrance_exam(request, user, course): + if user_can_skip_entrance_exam(user, course): required_content = [content for content in required_content if not content == course.entrance_exam_id] previous_of_active_section, next_of_active_section = None, None @@ -990,7 +990,7 @@ def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, course and course \ and getattr(course, 'entrance_exam_enabled', False) \ and getattr(instance, 'in_entrance_exam', False): - ee_data = {'entrance_exam_passed': user_has_passed_entrance_exam(request, course)} + ee_data = {'entrance_exam_passed': user_has_passed_entrance_exam(request.user, course)} resp = append_data_to_webob_response(resp, ee_data) except NoSuchHandlerError: diff --git a/lms/djangoapps/courseware/tabs.py b/lms/djangoapps/courseware/tabs.py index cae742e0f4..2da310d4f3 100644 --- a/lms/djangoapps/courseware/tabs.py +++ b/lms/djangoapps/courseware/tabs.py @@ -6,7 +6,7 @@ from django.conf import settings from django.utils.translation import ugettext as _, ugettext_noop from courseware.access import has_access -from courseware.entrance_exams import user_must_complete_entrance_exam +from courseware.entrance_exams import user_can_skip_entrance_exam from openedx.core.lib.course_tabs import CourseTabPluginManager from student.models import CourseEnrollment from xmodule.tabs import CourseTab, CourseTabList, key_checker @@ -294,7 +294,7 @@ def get_course_tab_list(request, course): # If the user has to take an entrance exam, we'll need to hide away all but the # "Courseware" tab. The tab is then renamed as "Entrance Exam". course_tab_list = [] - must_complete_ee = user_must_complete_entrance_exam(request, user, course) + must_complete_ee = not user_can_skip_entrance_exam(user, course) for tab in xmodule_tab_list: if must_complete_ee: # Hide all of the tabs except for 'Courseware' diff --git a/lms/djangoapps/courseware/tests/test_entrance_exam.py b/lms/djangoapps/courseware/tests/test_entrance_exam.py index 8282c37e2f..bac392115b 100644 --- a/lms/djangoapps/courseware/tests/test_entrance_exam.py +++ b/lms/djangoapps/courseware/tests/test_entrance_exam.py @@ -17,7 +17,6 @@ from courseware.tests.helpers import ( from courseware.entrance_exams import ( course_has_entrance_exam, get_entrance_exam_content, - get_entrance_exam_score, user_can_skip_entrance_exam, user_has_passed_entrance_exam, ) @@ -281,32 +280,14 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase, Milest """ exam_chapter = get_entrance_exam_content(self.request.user, self.course) self.assertEqual(exam_chapter.url_name, self.entrance_exam.url_name) - self.assertFalse(user_has_passed_entrance_exam(self.request, self.course)) + self.assertFalse(user_has_passed_entrance_exam(self.request.user, self.course)) answer_entrance_exam_problem(self.course, self.request, self.problem_1) answer_entrance_exam_problem(self.course, self.request, self.problem_2) exam_chapter = get_entrance_exam_content(self.request.user, self.course) self.assertEqual(exam_chapter, None) - self.assertTrue(user_has_passed_entrance_exam(self.request, self.course)) - - def test_entrance_exam_score(self): - """ - test entrance exam score. we will hit the method get_entrance_exam_score to verify exam score. - """ - # One query is for getting the list of disabled XBlocks (which is - # then stored in the request). - with self.assertNumQueries(1): - exam_score = get_entrance_exam_score(self.request, self.course) - self.assertEqual(exam_score, 0) - - answer_entrance_exam_problem(self.course, self.request, self.problem_1) - answer_entrance_exam_problem(self.course, self.request, self.problem_2) - - with self.assertNumQueries(1): - exam_score = get_entrance_exam_score(self.request, self.course) - # 50 percent exam score should be achieved. - self.assertGreater(exam_score * 100, 50) + self.assertTrue(user_has_passed_entrance_exam(self.request.user, self.course)) def test_entrance_exam_requirement_message(self): """ @@ -332,6 +313,10 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase, Milest minimum_score_pct = 29 self.course.entrance_exam_minimum_score_pct = float(minimum_score_pct) / 100 modulestore().update_item(self.course, self.request.user.id) # pylint: disable=no-member + + # answer the problem so it results in only 20% correct. + answer_entrance_exam_problem(self.course, self.request, self.problem_1, value=1, max_value=5) + url = reverse( 'courseware_section', kwargs={ @@ -342,9 +327,11 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase, Milest ) resp = self.client.get(url) self.assertEqual(resp.status_code, 200) - self.assertIn('To access course materials, you must score {required_score}% or higher'.format( - required_score=minimum_score_pct - ), resp.content) + self.assertIn( + 'To access course materials, you must score {}% or higher'.format(minimum_score_pct), + resp.content + ) + self.assertIn('Your current score is 20%.', resp.content) def test_entrance_exam_requirement_message_hidden(self): """ @@ -388,7 +375,7 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase, Milest resp = self.client.get(url) self.assertNotIn('To access course materials, you must score', resp.content) - self.assertIn('You have passed the entrance exam.', resp.content) + self.assertIn('Your score is 100%. You have passed the entrance exam.', resp.content) self.assertIn('Lesson 1', resp.content) def test_entrance_exam_gating(self): @@ -450,7 +437,6 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase, Milest for toc_section in self.expected_unlocked_toc: self.assertIn(toc_section, unlocked_toc) - @patch('courseware.entrance_exams.user_has_passed_entrance_exam', Mock(return_value=False)) def test_courseware_page_access_without_passing_entrance_exam(self): """ Test courseware access page without passing entrance exam @@ -468,7 +454,6 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase, Milest }) self.assertRedirects(response, expected_url, status_code=302, target_status_code=200) - @patch('courseware.entrance_exams.user_has_passed_entrance_exam', Mock(return_value=False)) def test_courseinfo_page_access_without_passing_entrance_exam(self): """ Test courseware access page without passing entrance exam @@ -481,12 +466,11 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase, Milest exam_url = response.get('Location') self.assertRedirects(response, exam_url) - @patch('courseware.entrance_exams.user_has_passed_entrance_exam', Mock(return_value=True)) + @patch('courseware.entrance_exams.get_entrance_exam_content', Mock(return_value=None)) def test_courseware_page_access_after_passing_entrance_exam(self): """ Test courseware access page after passing entrance exam """ - # Mocking get_required_content with empty list to assume user has passed entrance exam self._assert_chapter_loaded(self.course, self.chapter) @patch('util.milestones_helpers.get_required_content', Mock(return_value=['a value'])) @@ -528,7 +512,7 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase, Milest Test has_passed_entrance_exam method with anonymous user """ self.request.user = self.anonymous_user - self.assertFalse(user_has_passed_entrance_exam(self.request, self.course)) + self.assertFalse(user_has_passed_entrance_exam(self.request.user, self.course)) def test_course_has_entrance_exam_missing_exam_id(self): course = CourseFactory.create( @@ -541,7 +525,7 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase, Milest def test_user_has_passed_entrance_exam_short_circuit_missing_exam(self): course = CourseFactory.create( ) - self.assertTrue(user_has_passed_entrance_exam(self.request, course)) + self.assertTrue(user_has_passed_entrance_exam(self.request.user, course)) @patch.dict("django.conf.settings.FEATURES", {'ENABLE_MASQUERADE': False}) def test_entrance_exam_xblock_response(self): @@ -599,7 +583,7 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase, Milest return toc['chapters'] -def answer_entrance_exam_problem(course, request, problem, user=None): +def answer_entrance_exam_problem(course, request, problem, user=None, value=1, max_value=1): """ Takes a required milestone `problem` in a `course` and fulfills it. @@ -608,11 +592,13 @@ def answer_entrance_exam_problem(course, request, problem, user=None): request (Request): request Object problem (xblock): xblock object, the problem to be fulfilled user (User): User object in case it is different from request.user + value (int): raw_earned value of the problem + max_value (int): raw_possible value of the problem """ if not user: user = request.user - grade_dict = {'value': 1, 'max_value': 1, 'user_id': user.id} + grade_dict = {'value': value, 'max_value': max_value, 'user_id': user.id} field_data_cache = FieldDataCache.cache_for_descriptor_descendents( course.id, user, diff --git a/lms/djangoapps/courseware/tests/test_submitting_problems.py b/lms/djangoapps/courseware/tests/test_submitting_problems.py index 9c7ffc3a6b..1f0feec659 100644 --- a/lms/djangoapps/courseware/tests/test_submitting_problems.py +++ b/lms/djangoapps/courseware/tests/test_submitting_problems.py @@ -296,13 +296,11 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase, Probl """ Returns SubsectionGrade for given url. """ - # list of grade summaries for each section - sections_list = [] - for chapter in self.get_course_grade().chapter_grades: - sections_list.extend(chapter['sections']) - - # get the first section that matches the url (there should only be one) - return next(section for section in sections_list if section.url_name == hw_url_name) + for chapter in self.get_course_grade().chapter_grades.itervalues(): + for section in chapter['sections']: + if section.url_name == hw_url_name: + return section + return None def score_for_hw(self, hw_url_name): """ diff --git a/lms/djangoapps/courseware/views/index.py b/lms/djangoapps/courseware/views/index.py index 40a36cee54..fc9cce8552 100644 --- a/lms/djangoapps/courseware/views/index.py +++ b/lms/djangoapps/courseware/views/index.py @@ -22,7 +22,8 @@ import logging import newrelic.agent import urllib -from xblock.fragment import Fragment +from lms.djangoapps.gating.api import get_entrance_exam_score_ratio, get_entrance_exam_usage_key +from lms.djangoapps.grades.new.course_grade import CourseGradeFactory from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY from openedx.core.djangoapps.user_api.preferences.api import get_user_preference @@ -31,11 +32,12 @@ from shoppingcart.models import CourseRegistrationCode from student.models import CourseEnrollment from student.views import is_course_blocked from student.roles import GlobalStaff +from survey.utils import must_answer_survey from util.enterprise_helpers import get_enterprise_consent_url from util.views import ensure_valid_course_key +from xblock.fragment import Fragment from xmodule.modulestore.django import modulestore from xmodule.x_module import STUDENT_VIEW -from survey.utils import must_answer_survey from ..access import has_access, _adjust_start_date_for_beta_testers from ..access_utils import in_preview_mode @@ -43,9 +45,8 @@ from ..courses import get_studio_url, get_course_with_access from ..entrance_exams import ( course_has_entrance_exam, get_entrance_exam_content, - get_entrance_exam_score, user_has_passed_entrance_exam, - user_must_complete_entrance_exam, + user_can_skip_entrance_exam, ) from ..exceptions import Redirect from ..masquerade import setup_masquerade @@ -276,10 +277,7 @@ class CoursewareIndex(View): """ Check to see if an Entrance Exam is required for the user. """ - if ( - course_has_entrance_exam(self.course) and - user_must_complete_entrance_exam(self.request, self.effective_user, self.course) - ): + if not user_can_skip_entrance_exam(self.effective_user, self.course): exam_chapter = get_entrance_exam_content(self.effective_user, self.course) if exam_chapter and exam_chapter.get_children(): exam_section = exam_chapter.get_children()[0] @@ -428,10 +426,7 @@ class CoursewareIndex(View): ) # entrance exam data - if course_has_entrance_exam(self.course): - if getattr(self.chapter, 'is_entrance_exam', False): - courseware_context['entrance_exam_current_score'] = get_entrance_exam_score(self.request, self.course) - courseware_context['entrance_exam_passed'] = user_has_passed_entrance_exam(self.request, self.course) + self._add_entrance_exam_to_context(courseware_context) # staff masquerading data now = datetime.now(UTC()) @@ -469,6 +464,17 @@ class CoursewareIndex(View): return courseware_context + def _add_entrance_exam_to_context(self, courseware_context): + """ + Adds entrance exam related information to the given context. + """ + if course_has_entrance_exam(self.course) and getattr(self.chapter, 'is_entrance_exam', False): + courseware_context['entrance_exam_passed'] = user_has_passed_entrance_exam(self.effective_user, self.course) + courseware_context['entrance_exam_current_score'] = get_entrance_exam_score_ratio( + CourseGradeFactory().create(self.effective_user, self.course), + get_entrance_exam_usage_key(self.course), + ) + def _create_section_context(self, previous_of_active_section, next_of_active_section): """ Returns and creates the rendering context for the section. diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 6a01baf548..482b6dddd4 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -101,7 +101,7 @@ from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem from xmodule.tabs import CourseTabList from xmodule.x_module import STUDENT_VIEW -from ..entrance_exams import user_must_complete_entrance_exam +from ..entrance_exams import user_can_skip_entrance_exam from ..module_render import get_module_for_descriptor, get_module, get_module_by_usage_id from web_fragments.fragment import Fragment @@ -336,7 +336,7 @@ def course_info(request, course_id): # If the user needs to take an entrance exam to access this course, then we'll need # to send them to that specific course module before allowing them into other areas - if user_must_complete_entrance_exam(request, user, course): + if not user_can_skip_entrance_exam(user, course): return redirect(reverse('courseware', args=[unicode(course.id)])) # check to see if there is a required survey that must be taken before @@ -857,7 +857,7 @@ def _progress(request, course_key, student_id): student = User.objects.prefetch_related("groups").get(id=student.id) course_grade = CourseGradeFactory().create(student, course) - courseware_summary = course_grade.chapter_grades + courseware_summary = course_grade.chapter_grades.values() grade_summary = course_grade.summary studio_url = get_studio_url(course, 'settings/grading') diff --git a/lms/djangoapps/gating/api.py b/lms/djangoapps/gating/api.py index 14bf7677d2..baf6888c08 100644 --- a/lms/djangoapps/gating/api.py +++ b/lms/djangoapps/gating/api.py @@ -2,14 +2,12 @@ API for the gating djangoapp """ from collections import defaultdict -from django.test.client import RequestFactory import json import logging -from lms.djangoapps.courseware.entrance_exams import get_entrance_exam_score +from lms.djangoapps.courseware.entrance_exams import get_entrance_exam_content from openedx.core.lib.gating import api as gating_api from opaque_keys.edx.keys import UsageKey -from xmodule.modulestore.django import modulestore from util import milestones_helpers @@ -53,7 +51,7 @@ def _get_minimum_required_percentage(milestone): min_score = int(requirements.get('min_score')) except (ValueError, TypeError): log.warning( - 'Failed to find minimum score for gating milestone %s, defaulting to 100', + u'Gating: Failed to find minimum score for gating milestone %s, defaulting to 100', json.dumps(milestone) ) return min_score @@ -63,35 +61,56 @@ def _get_subsection_percentage(subsection_grade): """ Returns the percentage value of the given subsection_grade. """ - if subsection_grade.graded_total.possible: - return float(subsection_grade.graded_total.earned) / float(subsection_grade.graded_total.possible) * 100.0 - else: - return 0 + return _calculate_ratio(subsection_grade.graded_total.earned, subsection_grade.graded_total.possible) * 100.0 -def evaluate_entrance_exam(course, subsection_grade, user): +def _calculate_ratio(earned, possible): + """ + Returns the percentage of the given earned and possible values. + """ + return float(earned) / float(possible) if possible else 0.0 + + +def evaluate_entrance_exam(course_grade, user): """ Evaluates any entrance exam milestone relationships attached - to the given subsection. If the subsection_grade meets the - minimum score required, the dependent milestone will be marked + to the given course. If the course_grade meets the + minimum score required, the dependent milestones will be marked fulfilled for the user. """ + course = course_grade.course if milestones_helpers.is_entrance_exams_enabled() and getattr(course, 'entrance_exam_enabled', False): - subsection = modulestore().get_item(subsection_grade.location) - in_entrance_exam = getattr(subsection, 'in_entrance_exam', False) - if in_entrance_exam: - # We don't have access to the true request object in this context, but we can use a mock - request = RequestFactory().request() - request.user = user - exam_pct = get_entrance_exam_score(request, course) - if exam_pct >= course.entrance_exam_minimum_score_pct: - exam_key = UsageKey.from_string(course.entrance_exam_id) + if get_entrance_exam_content(user, course): + exam_chapter_key = get_entrance_exam_usage_key(course) + exam_score_ratio = get_entrance_exam_score_ratio(course_grade, exam_chapter_key) + if exam_score_ratio >= course.entrance_exam_minimum_score_pct: relationship_types = milestones_helpers.get_milestone_relationship_types() content_milestones = milestones_helpers.get_course_content_milestones( course.id, - exam_key, + exam_chapter_key, relationship=relationship_types['FULFILLS'] ) - # Mark each milestone dependent on the entrance exam as fulfilled by the user. + # Mark each entrance exam dependent milestone as fulfilled by the user. for milestone in content_milestones: - milestones_helpers.add_user_milestone({'id': request.user.id}, milestone) + milestones_helpers.add_user_milestone({'id': user.id}, milestone) + + +def get_entrance_exam_usage_key(course): + """ + Returns the UsageKey of the entrance exam for the course. + """ + return UsageKey.from_string(course.entrance_exam_id).replace(course_key=course.id) + + +def get_entrance_exam_score_ratio(course_grade, exam_chapter_key): + """ + Returns the score for the given chapter as a ratio of the + aggregated earned over the possible points, resulting in a + decimal value less than 1. + """ + try: + earned, possible = course_grade.score_for_chapter(exam_chapter_key) + except KeyError: + earned, possible = 0.0, 0.0 + log.warning(u'Gating: Unexpectedly failed to find chapter_grade for %s.', exam_chapter_key) + return _calculate_ratio(earned, possible) diff --git a/lms/djangoapps/gating/signals.py b/lms/djangoapps/gating/signals.py index 64a29dedfa..f05dd33b2d 100644 --- a/lms/djangoapps/gating/signals.py +++ b/lms/djangoapps/gating/signals.py @@ -5,6 +5,7 @@ from django.dispatch import receiver from gating import api as gating_api from lms.djangoapps.grades.signals.signals import SUBSECTION_SCORE_CHANGED +from openedx.core.djangoapps.signals.signals import COURSE_GRADE_CHANGED @receiver(SUBSECTION_SCORE_CHANGED) @@ -21,4 +22,18 @@ def evaluate_subsection_gated_milestones(**kwargs): """ subsection_grade = kwargs['subsection_grade'] gating_api.evaluate_prerequisite(kwargs['course'], subsection_grade, kwargs.get('user')) - gating_api.evaluate_entrance_exam(kwargs['course'], subsection_grade, kwargs.get('user')) + + +@receiver(COURSE_GRADE_CHANGED) +def evaluate_course_gated_milestones(**kwargs): + """ + Receives the COURSE_GRADE_CHANGED signal and triggers the + evaluation of any milestone relationships which are attached + to the course grade. + + Arguments: + kwargs (dict): Contains user, course_grade + Returns: + None + """ + gating_api.evaluate_entrance_exam(kwargs['course_grade'], kwargs.get('user')) diff --git a/lms/djangoapps/grades/new/course_grade.py b/lms/djangoapps/grades/new/course_grade.py index 319a3b8156..e4e9bb7b58 100644 --- a/lms/djangoapps/grades/new/course_grade.py +++ b/lms/djangoapps/grades/new/course_grade.py @@ -49,7 +49,7 @@ class CourseGrade(object): a dict keyed by subsection format types. """ subsections_by_format = defaultdict(OrderedDict) - for chapter in self.chapter_grades: + for chapter in self.chapter_grades.itervalues(): for subsection_grade in chapter['sections']: if subsection_grade.graded: graded_total = subsection_grade.graded_total @@ -63,7 +63,7 @@ class CourseGrade(object): Returns a dict of problem scores keyed by their locations. """ locations_to_scores = {} - for chapter in self.chapter_grades: + for chapter in self.chapter_grades.itervalues(): for subsection_grade in chapter['sections']: locations_to_scores.update(subsection_grade.locations_to_scores) return locations_to_scores @@ -88,10 +88,12 @@ class CourseGrade(object): @lazy def chapter_grades(self): """ - Returns a list of chapters, each containing its subsection grades, - display name, and url name. + Returns a dictionary of dictionaries. + The primary dictionary is keyed by the chapter's usage_key. + The secondary dictionary contains the chapter's + subsection grades, display name, and url name. """ - chapter_grades = [] + chapter_grades = OrderedDict() for chapter_key in self.course_structure.get_children(self.course.location): chapter = self.course_structure[chapter_key] chapter_subsection_grades = [] @@ -101,11 +103,11 @@ class CourseGrade(object): self._subsection_grade_factory.create(self.course_structure[subsection_key], read_only=True) ) - chapter_grades.append({ + chapter_grades[chapter_key] = { 'display_name': block_metadata_utils.display_name_with_default_escaped(chapter), 'url_name': block_metadata_utils.url_name_for_block(chapter), 'sections': chapter_subsection_grades - }) + } return chapter_grades @property @@ -152,7 +154,7 @@ class CourseGrade(object): If read_only is True, doesn't save any updates to the grades. """ - subsections_total = sum(len(chapter['sections']) for chapter in self.chapter_grades) + subsections_total = sum(len(chapter['sections']) for chapter in self.chapter_grades.itervalues()) total_graded_subsections = sum(len(x) for x in self.graded_subsections_by_format.itervalues()) subsections_created = len(self._subsection_grade_factory._unsaved_subsection_grades) # pylint: disable=protected-access @@ -185,6 +187,19 @@ class CourseGrade(object): ) ) + def score_for_chapter(self, chapter_key): + """ + Returns the aggregate weighted score for the given chapter. + Raises: + KeyError if the chapter is not found. + """ + earned, possible = 0.0, 0.0 + chapter_grade = self.chapter_grades[chapter_key] + for section in chapter_grade['sections']: + earned += section.graded_total.earned + possible += section.graded_total.possible + return earned, possible + def score_for_module(self, location): """ Calculate the aggregate weighted score for any location in the course. @@ -199,8 +214,7 @@ class CourseGrade(object): score = self.locations_to_scores[location] return score.earned, score.possible children = self.course_structure.get_children(location) - earned = 0.0 - possible = 0.0 + earned, possible = 0.0, 0.0 for child in children: child_earned, child_possible = self.score_for_module(child) earned += child_earned From f9f5119277de01d6b40e07b0822821be7f753d31 Mon Sep 17 00:00:00 2001 From: Matthew Piatetsky Date: Wed, 15 Mar 2017 09:51:17 -0400 Subject: [PATCH 06/51] Update display of progress on programs dashboard ECOM-6601 --- .../views/program_card_view.js | 5 - .../program_card_view_spec.js | 15 +-- lms/static/sass/elements/_program-card.scss | 102 +++++++++++------- .../learner_dashboard/program_card.underscore | 71 ++++++------ 4 files changed, 109 insertions(+), 84 deletions(-) diff --git a/lms/static/js/learner_dashboard/views/program_card_view.js b/lms/static/js/learner_dashboard/views/program_card_view.js index 05cb63d389..9d27c7a47c 100644 --- a/lms/static/js/learner_dashboard/views/program_card_view.js +++ b/lms/static/js/learner_dashboard/views/program_card_view.js @@ -54,11 +54,6 @@ }, postRender: function() { - // Add describedby to parent only if progess is present - if (this.progressModel) { - this.$el.attr('aria-describedby', 'status-' + this.model.get('uuid')); - } - if (navigator.userAgent.indexOf('MSIE') !== -1 || navigator.appVersion.indexOf('Trident/') > 0) { /* Microsoft Internet Explorer detected in. */ diff --git a/lms/static/js/spec/learner_dashboard/program_card_view_spec.js b/lms/static/js/spec/learner_dashboard/program_card_view_spec.js index 6b4fa1bbb9..66b68bae4d 100644 --- a/lms/static/js/spec/learner_dashboard/program_card_view_spec.js +++ b/lms/static/js/spec/learner_dashboard/program_card_view_spec.js @@ -114,18 +114,13 @@ define([ expect(view.reLoadBannerImage).not.toThrow(message); }); - it('should calculate the correct percentages for progress bars', function() { - expect(view.$('.complete').css('width')).toEqual('40%'); - expect(view.$('.in-progress').css('width')).toEqual('20%'); + it('should show the right number of progress bar segments', function() { + expect(view.$('.progress-bar .completed').length).toEqual(4); + expect(view.$('.progress-bar .enrolled').length).toEqual(2); }); - it('should display the correct completed courses message', function() { - var programProgress = _.findWhere(userProgress, {uuid: 'a87e5eac-3c93-45a1-a8e1-4c79ca8401c8'}), - completed = programProgress.completed, - total = completed + programProgress.in_progress + programProgress.not_started; - - expect(view.$('.certificate-status .status-text').not('.secondary').html()) - .toEqual('You have earned certificates in ' + completed + ' of the ' + total + ' courses so far.'); + it('should display the correct course status numbers', function() { + expect(view.$('.number-circle').text()).toEqual('424'); }); it('should render cards if there is no progressData', function() { diff --git a/lms/static/sass/elements/_program-card.scss b/lms/static/sass/elements/_program-card.scss index 37b551075c..344d1440bd 100644 --- a/lms/static/sass/elements/_program-card.scss +++ b/lms/static/sass/elements/_program-card.scss @@ -105,55 +105,81 @@ .hd-3 { color: palette(grayscale, dark); min-height: ($baseline*3); - line-height: 1; + line-height: 1.15; } - .certificate-status { - margin-bottom: 0; + .status-text { + display: flex; + margin-bottom: 5px; - .status-text { - font-size: font-size(x-small); - color: palette(grayscale, dark); - line-height: 1; + .number-status { + text-align: center; + width: 100%; + float: left; + padding: { + left: 5px; + right: 5px; + bottom: 8px; + } + margin-top: -8px; + font-size: 1em; + font-family: $f-sans-serif; } - .secondary { - @extend %hide-until-focus; + .number-circle { + padding-top: 1px; + border-radius: 50%; + margin-left: auto; + margin-right: auto; + width: 23px; + height: 23px; + color: white; + text-align: center; + font-size: 0.9375em; + font-family: $f-sans-serif; + font-weight: bold; } - .status-text { - &:focus, - &:active { - position: relative; - top: 4px; - width: auto; - height: auto; - margin: 0; - text-decoration: none; + .completed { + background: $blue; + } - & ~ .status-text { - @extend %hide-until-focus; + .enrolled { + background: $green; + } + + .not-enrolled { + background: palette(grayscale, dark); + } + } + + .progress-container { + max-width: inherit; + + .progress-bar { + height: 5px; + display: flex; + width: 100%; + + .item { + width: 100%; + margin-right: 2px; + height: 5px; + + &.completed { + background: $blue; + } + &.enrolled { + background: $green; + } + &.not-enrolled { + background: lightgray; + } + &.not-enrolled:last-of-type { + margin-right: 0; } } } } - .progress { - height: 5px; - background: palette(grayscale, back); - - .bar { - @include float(left); - height: 100%; - position: relative; - - &.complete { - background: palette(success, accent); - } - - &.in-progress { - background: palette(warning, accent); - } - } - } } diff --git a/lms/templates/learner_dashboard/program_card.underscore b/lms/templates/learner_dashboard/program_card.underscore index 5cb4c61849..8002a29deb 100644 --- a/lms/templates/learner_dashboard/program_card.underscore +++ b/lms/templates/learner_dashboard/program_card.underscore @@ -7,39 +7,48 @@ - <% if (progress) { %> -

- <%= interpolate( - ngettext( - '%(count)s course is in progress.', - '%(count)s courses are in progress.', - progress.in_progress - ), - {count: progress.in_progress}, true - ) %> - - <%= interpolate( - ngettext( - '%(count)s course has not been started.', - '%(count)s courses have not been started.', - progress.not_started - ), - {count: progress.not_started}, true - ) %> - - <%= interpolate( - gettext('You have earned certificates in %(completed_courses)s of the %(total_courses)s courses so far.'), - {completed_courses: progress.completed, total_courses: progress.total}, true - ) %> -

- <% } %> <% if (progress) { %> -
-
-
-
-
+
+
+
<%- progress.completed %>
+ + <%- ngettext('Course', 'Courses', progress.completed) %> + + <%- gettext('Completed') %> +
+ +
+
<%- progress.in_progress %>
+ + <%- ngettext('Course', 'Courses', progress.in_progress) %> + + <%- gettext('Enrolled') %> +
+ +
+
<%- progress.not_started %>
+ + <%- ngettext('Course', 'Courses', progress.not_started) %> + + <%- gettext('Not Enrolled') %> +
+
+<% } %> +<% if (progress) { %> +
+
+ <% _.times(progress.completed, function() { %> +
+ <% }) %> + <% _.times(progress.in_progress, function() { %> +
+ <% }) %> + <% _.times(progress.not_started, function() { %> +
+ <% }) %> +
+
<% } %>