diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 2c66444ab9..13968d0e98 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -1066,27 +1066,6 @@ def _progress(request, course_key, student_id): # checking certificate generation configuration show_generate_cert_btn = certs_api.cert_generation_enabled(course_key) - credit_course_requirements = None - is_course_credit = settings.FEATURES.get("ENABLE_CREDIT_ELIGIBILITY", False) and is_credit_course(course_key) - if is_course_credit: - requirement_statuses = get_credit_requirement_status(course_key, student.username) - if any(requirement['status'] == 'failed' for requirement in requirement_statuses): - eligibility_status = "not_eligible" - elif is_user_eligible_for_credit(student.username, course_key): - eligibility_status = "eligible" - else: - eligibility_status = "partial_eligible" - - paired_requirements = {} - for requirement in requirement_statuses: - namespace = requirement.pop("namespace") - paired_requirements.setdefault(namespace, []).append(requirement) - - credit_course_requirements = { - 'eligibility_status': eligibility_status, - 'requirements': OrderedDict(sorted(paired_requirements.items(), reverse=True)) - } - context = { 'course': course, 'courseware_summary': courseware_summary, @@ -1096,8 +1075,7 @@ def _progress(request, course_key, student_id): 'student': student, 'passed': is_course_passed(course, grade_summary), 'show_generate_cert_btn': show_generate_cert_btn, - 'credit_course_requirements': credit_course_requirements, - 'is_credit_course': is_course_credit, + 'credit_course_requirements': _credit_course_requirements(course_key, student), } if show_generate_cert_btn: @@ -1124,6 +1102,64 @@ def _progress(request, course_key, student_id): return response +def _credit_course_requirements(course_key, student): + """Return information about which credit requirements a user has satisfied. + + Arguments: + course_key (CourseKey): Identifier for the course. + student (User): Currently logged in user. + + Returns: dict + + """ + # If credit eligibility is not enabled or this is not a credit course, + # short-circuit and return `None`. This indicates that credit requirements + # should NOT be displayed on the progress page. + if not (settings.FEATURES.get("ENABLE_CREDIT_ELIGIBILITY", False) and is_credit_course(course_key)): + return None + + # Retrieve the status of the user for each eligibility requirement in the course. + # For each requirement, the user's status is either "satisfied", "failed", or None. + # In this context, `None` means that we don't know the user's status, either because + # the user hasn't done something (for example, submitting photos for verification) + # or we're waiting on more information (for example, a response from the photo + # verification service). + requirement_statuses = get_credit_requirement_status(course_key, student.username) + + # If the user has been marked as "eligible", then they are *always* eligible + # unless someone manually intervenes. This could lead to some strange behavior + # if the requirements change post-launch. For example, if the user was marked as eligible + # for credit, then a new requirement was added, the user will see that they're eligible + # AND that one of the requirements is still pending. + # We're assuming here that (a) we can mitigate this by properly training course teams, + # and (b) it's a better user experience to allow students who were at one time + # marked as eligible to continue to be eligible. + # If we need to, we can always manually move students back to ineligible by + # deleting CreditEligibility records in the database. + if is_user_eligible_for_credit(student.username, course_key): + eligibility_status = "eligible" + + # If the user has *failed* any requirements (for example, if a photo verification is denied), + # then the user is NOT eligible for credit. + elif any(requirement['status'] == 'failed' for requirement in requirement_statuses): + eligibility_status = "not_eligible" + + # Otherwise, the user may be eligible for credit, but the user has not + # yet completed all the requirements. + else: + eligibility_status = "partial_eligible" + + paired_requirements = {} + for requirement in requirement_statuses: + namespace = requirement.pop("namespace") + paired_requirements.setdefault(namespace, []).append(requirement) + + return { + 'eligibility_status': eligibility_status, + 'requirements': OrderedDict(sorted(paired_requirements.items(), reverse=True)) + } + + @login_required @ensure_valid_course_key def submission_history(request, course_id, student_username, location): diff --git a/lms/djangoapps/verify_student/views.py b/lms/djangoapps/verify_student/views.py index 38b5d5160f..173c7152d2 100644 --- a/lms/djangoapps/verify_student/views.py +++ b/lms/djangoapps/verify_student/views.py @@ -38,7 +38,7 @@ from microsite_configuration import microsite from openedx.core.djangoapps.user_api.accounts import NAME_MIN_LENGTH from openedx.core.djangoapps.user_api.accounts.api import get_account_settings, update_account_settings from openedx.core.djangoapps.user_api.errors import UserNotFound, AccountValidationError -from openedx.core.djangoapps.credit.api import get_credit_requirement, set_credit_requirement_status +from openedx.core.djangoapps.credit.api import set_credit_requirement_status from student.models import CourseEnrollment from shoppingcart.models import Order, CertificateItem from shoppingcart.processors import ( @@ -897,19 +897,19 @@ def _set_user_requirement_status(attempt, namespace, status, reason=None): log.error("Unable to find checkpoint for user with id %d", attempt.user.id) if checkpoint is not None: - course_key = checkpoint.course_id - credit_requirement = get_credit_requirement( - course_key, namespace, checkpoint.checkpoint_location - ) - if credit_requirement is not None: - try: - set_credit_requirement_status( - attempt.user.username, credit_requirement, status, reason - ) - except Exception: # pylint: disable=broad-except - # Catch exception if unable to add credit requirement - # status for user - log.error("Unable to add Credit requirement status for user with id %d", attempt.user.id) + try: + set_credit_requirement_status( + attempt.user.username, + checkpoint.course_id, + namespace, + checkpoint.checkpoint_location, + status=status, + reason=reason, + ) + except Exception: # pylint: disable=broad-except + # Catch exception if unable to add credit requirement + # status for user + log.error("Unable to add Credit requirement status for user with id %d", attempt.user.id) @require_POST diff --git a/lms/templates/courseware/progress.html b/lms/templates/courseware/progress.html index d2dbdafc69..5ceac475e1 100644 --- a/lms/templates/courseware/progress.html +++ b/lms/templates/courseware/progress.html @@ -103,7 +103,7 @@ from django.utils.http import urlquote_plus %endif - % if is_credit_course: + % if credit_course_requirements:
diff --git a/openedx/core/djangoapps/credit/api.py b/openedx/core/djangoapps/credit/api.py index bb567d7ea1..bfe556fb30 100644 --- a/openedx/core/djangoapps/credit/api.py +++ b/openedx/core/djangoapps/credit/api.py @@ -274,12 +274,12 @@ def create_credit_request(course_key, provider_id, username): # Retrieve the final grade from the eligibility table try: - final_grade = CreditRequirementStatus.objects.filter( + final_grade = CreditRequirementStatus.objects.get( username=username, requirement__namespace="grade", requirement__name="grade", status="satisfied" - ).latest().reason["final_grade"] + ).reason["final_grade"] except (CreditRequirementStatus.DoesNotExist, TypeError, KeyError): log.exception( "Could not retrieve final grade from the credit eligibility table " @@ -410,7 +410,7 @@ def get_credit_requests_for_user(username): return CreditRequest.credit_requests_for_user(username) -def get_credit_requirement_status(course_key, username): +def get_credit_requirement_status(course_key, username, namespace=None, name=None): """ Retrieve the user's status for each credit requirement in the course. Args: @@ -447,7 +447,7 @@ def get_credit_requirement_status(course_key, username): Returns: list of requirement statuses """ - requirements = CreditRequirement.get_course_requirements(course_key) + requirements = CreditRequirement.get_course_requirements(course_key, namespace=namespace, name=name) requirement_statuses = CreditRequirementStatus.get_statuses(requirements, username) requirement_statuses = dict((o.requirement, o) for o in requirement_statuses) statuses = [] @@ -511,36 +511,80 @@ def get_credit_requirement(course_key, namespace, name): } if requirement else None -def set_credit_requirement_status(username, requirement, status="satisfied", reason=None): - """Update Credit Requirement Status for given username and requirement - if exists else add new. +def set_credit_requirement_status(username, course_key, req_namespace, req_name, status="satisfied", reason=None): + """ + Update the user's requirement status. + + This will record whether the user satisfied or failed a particular requirement + in a course. If the user has satisfied all requirements, the user will be marked + as eligible for credit in the course. Args: - username(str): Username of the user - requirement(dict): requirement dict - status(str): Status of the requirement - reason(dict): Reason of the status + username (str): Username of the user + course_key (CourseKey): Identifier for the course associated with the requirement. + req_namespace (str): Namespace of the requirement (e.g. "grade" or "reverification") + req_name (str): Name of the requirement (e.g. "grade" or the location of the ICRV XBlock) + + Keyword Arguments: + status (str): Status of the requirement (either "satisfied" or "failed") + reason (dict): Reason of the status Example: >>> set_credit_requirement_status( - "staff", - { - "course_key": "course-v1-edX-DemoX-1T2015" - "namespace": "reverification", - "name": "i4x://edX/DemoX/edx-reverification-block/assessment_uuid", - }, - "satisfied", - {} + "staff", + CourseKey.from_string("course-v1-edX-DemoX-1T2015"), + "reverification", + "i4x://edX/DemoX/edx-reverification-block/assessment_uuid", + status="satisfied", + reason={} ) """ - credit_requirement = CreditRequirement.get_course_requirement( - requirement['course_key'], requirement['namespace'], requirement['name'] - ) + # Check if we're already eligible for credit. + # If so, short-circuit this process. + if CreditEligibility.is_user_eligible_for_credit(course_key, username): + return + + # Retrieve all credit requirements for the course + # We retrieve all of them to avoid making a second query later when + # we need to check whether all requirements have been satisfied. + reqs = CreditRequirement.get_course_requirements(course_key) + + # Find the requirement we're trying to set + req_to_update = next(( + req for req in reqs + if req.namespace == req_namespace + and req.name == req_name + ), None) + + # If we can't find the requirement, then the most likely explanation + # is that there was a lag updating the credit requirements after the course + # was published. We *could* attempt to create the requirement here, + # but that could cause serious performance issues if many users attempt to + # lock the row at the same time. + # Instead, we skip updating the requirement and log an error. + if req_to_update is None: + log.error( + ( + u'Could not update credit requirement in course "%s" ' + u'with namespace "%s" and name "%s" ' + u'because the requirement does not exist. ' + u'The user "%s" should have had his/her status updated to "%s".' + ), + unicode(course_key), req_namespace, req_name, username, status + ) + return + + # Update the requirement status CreditRequirementStatus.add_or_update_requirement_status( - username, credit_requirement, status, reason + username, req_to_update, status=status, reason=reason ) + # If we're marking this requirement as "satisfied", there's a chance + # that the user has met all eligibility requirements. + if status == "satisfied": + CreditEligibility.update_eligibility(reqs, username, course_key) + def _get_requirements_to_disable(old_requirements, new_requirements): """ diff --git a/openedx/core/djangoapps/credit/migrations/0010_add_creditrequirementstatus_history.py b/openedx/core/djangoapps/credit/migrations/0010_add_creditrequirementstatus_history.py new file mode 100644 index 0000000000..777e86782d --- /dev/null +++ b/openedx/core/djangoapps/credit/migrations/0010_add_creditrequirementstatus_history.py @@ -0,0 +1,169 @@ +# -*- coding: utf-8 -*- +from south.utils import datetime_utils as datetime +from south.db import db +from south.v2 import SchemaMigration +from django.db import models + + +class Migration(SchemaMigration): + + def forwards(self, orm): + # Adding model 'HistoricalCreditRequirementStatus' + db.create_table('credit_historicalcreditrequirementstatus', ( + ('id', self.gf('django.db.models.fields.IntegerField')(db_index=True, blank=True)), + ('created', self.gf('model_utils.fields.AutoCreatedField')(default=datetime.datetime.now)), + ('modified', self.gf('model_utils.fields.AutoLastModifiedField')(default=datetime.datetime.now)), + ('username', self.gf('django.db.models.fields.CharField')(max_length=255, db_index=True)), + ('status', self.gf('django.db.models.fields.CharField')(max_length=32)), + ('reason', self.gf('jsonfield.fields.JSONField')(default={})), + ('requirement', self.gf('django.db.models.fields.related.ForeignKey')(blank=True, related_name=u'+', null=True, on_delete=models.DO_NOTHING, to=orm['credit.CreditRequirement'])), + (u'history_id', self.gf('django.db.models.fields.AutoField')(primary_key=True)), + (u'history_date', self.gf('django.db.models.fields.DateTimeField')()), + (u'history_user', self.gf('django.db.models.fields.related.ForeignKey')(related_name=u'+', null=True, on_delete=models.SET_NULL, to=orm['auth.User'])), + (u'history_type', self.gf('django.db.models.fields.CharField')(max_length=1)), + )) + db.send_create_signal('credit', ['HistoricalCreditRequirementStatus']) + + # Adding unique constraint on 'CreditRequirementStatus', fields ['username', 'requirement'] + db.create_unique('credit_creditrequirementstatus', ['username', 'requirement_id']) + + + def backwards(self, orm): + # Removing unique constraint on 'CreditRequirementStatus', fields ['username', 'requirement'] + db.delete_unique('credit_creditrequirementstatus', ['username', 'requirement_id']) + + # Deleting model 'HistoricalCreditRequirementStatus' + db.delete_table('credit_historicalcreditrequirementstatus') + + + models = { + 'auth.group': { + 'Meta': {'object_name': 'Group'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}), + 'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}) + }, + 'auth.permission': { + 'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'}, + 'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '50'}) + }, + 'auth.user': { + 'Meta': {'object_name': 'User'}, + 'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}), + 'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}), + 'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}), + 'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'}) + }, + 'contenttypes.contenttype': { + 'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"}, + 'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '100'}) + }, + 'credit.creditcourse': { + 'Meta': {'object_name': 'CreditCourse'}, + 'course_key': ('xmodule_django.models.CourseKeyField', [], {'unique': 'True', 'max_length': '255', 'db_index': 'True'}), + 'enabled': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'providers': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['credit.CreditProvider']", 'symmetrical': 'False'}) + }, + 'credit.crediteligibility': { + 'Meta': {'unique_together': "(('username', 'course'),)", 'object_name': 'CreditEligibility'}, + 'course': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'eligibilities'", 'to': "orm['credit.CreditCourse']"}), + 'created': ('model_utils.fields.AutoCreatedField', [], {'default': 'datetime.datetime.now'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'modified': ('model_utils.fields.AutoLastModifiedField', [], {'default': 'datetime.datetime.now'}), + 'username': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}) + }, + 'credit.creditprovider': { + 'Meta': {'object_name': 'CreditProvider'}, + 'active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'created': ('model_utils.fields.AutoCreatedField', [], {'default': 'datetime.datetime.now'}), + 'display_name': ('django.db.models.fields.CharField', [], {'max_length': '255'}), + 'eligibility_duration': ('django.db.models.fields.PositiveIntegerField', [], {'default': '31556970'}), + 'enable_integration': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'modified': ('model_utils.fields.AutoLastModifiedField', [], {'default': 'datetime.datetime.now'}), + 'provider_id': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '255'}), + 'provider_url': ('django.db.models.fields.URLField', [], {'default': "''", 'max_length': '200'}) + }, + 'credit.creditrequest': { + 'Meta': {'unique_together': "(('username', 'course', 'provider'),)", 'object_name': 'CreditRequest'}, + 'course': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'credit_requests'", 'to': "orm['credit.CreditCourse']"}), + 'created': ('model_utils.fields.AutoCreatedField', [], {'default': 'datetime.datetime.now'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'modified': ('model_utils.fields.AutoLastModifiedField', [], {'default': 'datetime.datetime.now'}), + 'parameters': ('jsonfield.fields.JSONField', [], {}), + 'provider': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'credit_requests'", 'to': "orm['credit.CreditProvider']"}), + 'status': ('django.db.models.fields.CharField', [], {'default': "'pending'", 'max_length': '255'}), + 'username': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'uuid': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '32', 'db_index': 'True'}) + }, + 'credit.creditrequirement': { + 'Meta': {'unique_together': "(('namespace', 'name', 'course'),)", 'object_name': 'CreditRequirement'}, + 'active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'course': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'credit_requirements'", 'to': "orm['credit.CreditCourse']"}), + 'created': ('model_utils.fields.AutoCreatedField', [], {'default': 'datetime.datetime.now'}), + 'criteria': ('jsonfield.fields.JSONField', [], {}), + 'display_name': ('django.db.models.fields.CharField', [], {'default': "''", 'max_length': '255'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'modified': ('model_utils.fields.AutoLastModifiedField', [], {'default': 'datetime.datetime.now'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '255'}), + 'namespace': ('django.db.models.fields.CharField', [], {'max_length': '255'}) + }, + 'credit.creditrequirementstatus': { + 'Meta': {'unique_together': "(('username', 'requirement'),)", 'object_name': 'CreditRequirementStatus'}, + 'created': ('model_utils.fields.AutoCreatedField', [], {'default': 'datetime.datetime.now'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'modified': ('model_utils.fields.AutoLastModifiedField', [], {'default': 'datetime.datetime.now'}), + 'reason': ('jsonfield.fields.JSONField', [], {'default': '{}'}), + 'requirement': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'statuses'", 'to': "orm['credit.CreditRequirement']"}), + 'status': ('django.db.models.fields.CharField', [], {'max_length': '32'}), + 'username': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}) + }, + 'credit.historicalcreditrequest': { + 'Meta': {'ordering': "(u'-history_date', u'-history_id')", 'object_name': 'HistoricalCreditRequest'}, + 'course': ('django.db.models.fields.related.ForeignKey', [], {'blank': 'True', 'related_name': "u'+'", 'null': 'True', 'on_delete': 'models.DO_NOTHING', 'to': "orm['credit.CreditCourse']"}), + 'created': ('model_utils.fields.AutoCreatedField', [], {'default': 'datetime.datetime.now'}), + u'history_date': ('django.db.models.fields.DateTimeField', [], {}), + u'history_id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + u'history_type': ('django.db.models.fields.CharField', [], {'max_length': '1'}), + u'history_user': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "u'+'", 'null': 'True', 'on_delete': 'models.SET_NULL', 'to': "orm['auth.User']"}), + 'id': ('django.db.models.fields.IntegerField', [], {'db_index': 'True', 'blank': 'True'}), + 'modified': ('model_utils.fields.AutoLastModifiedField', [], {'default': 'datetime.datetime.now'}), + 'parameters': ('jsonfield.fields.JSONField', [], {}), + 'provider': ('django.db.models.fields.related.ForeignKey', [], {'blank': 'True', 'related_name': "u'+'", 'null': 'True', 'on_delete': 'models.DO_NOTHING', 'to': "orm['credit.CreditProvider']"}), + 'status': ('django.db.models.fields.CharField', [], {'default': "'pending'", 'max_length': '255'}), + 'username': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'uuid': ('django.db.models.fields.CharField', [], {'max_length': '32', 'db_index': 'True'}) + }, + 'credit.historicalcreditrequirementstatus': { + 'Meta': {'ordering': "(u'-history_date', u'-history_id')", 'object_name': 'HistoricalCreditRequirementStatus'}, + 'created': ('model_utils.fields.AutoCreatedField', [], {'default': 'datetime.datetime.now'}), + u'history_date': ('django.db.models.fields.DateTimeField', [], {}), + u'history_id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + u'history_type': ('django.db.models.fields.CharField', [], {'max_length': '1'}), + u'history_user': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "u'+'", 'null': 'True', 'on_delete': 'models.SET_NULL', 'to': "orm['auth.User']"}), + 'id': ('django.db.models.fields.IntegerField', [], {'db_index': 'True', 'blank': 'True'}), + 'modified': ('model_utils.fields.AutoLastModifiedField', [], {'default': 'datetime.datetime.now'}), + 'reason': ('jsonfield.fields.JSONField', [], {'default': '{}'}), + 'requirement': ('django.db.models.fields.related.ForeignKey', [], {'blank': 'True', 'related_name': "u'+'", 'null': 'True', 'on_delete': 'models.DO_NOTHING', 'to': "orm['credit.CreditRequirement']"}), + 'status': ('django.db.models.fields.CharField', [], {'max_length': '32'}), + 'username': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}) + } + } + + complete_apps = ['credit'] \ No newline at end of file diff --git a/openedx/core/djangoapps/credit/models.py b/openedx/core/djangoapps/credit/models.py index d107ceee77..4d953f0294 100644 --- a/openedx/core/djangoapps/credit/models.py +++ b/openedx/core/djangoapps/credit/models.py @@ -6,10 +6,10 @@ Credit courses allow students to receive university credit for successful completion of a course on EdX """ +from collections import defaultdict import logging -from django.db import models -from django.db import transaction +from django.db import models, transaction, IntegrityError from django.core.validators import RegexValidator from simple_history.models import HistoricalRecords @@ -168,8 +168,11 @@ class CreditRequirement(TimeStampedModel): course=credit_course, namespace=requirement["namespace"], name=requirement["name"], - display_name=requirement["display_name"], - defaults={"criteria": requirement["criteria"], "active": True} + defaults={ + "display_name": requirement["display_name"], + "criteria": requirement["criteria"], + "active": True + } ) if not created: credit_requirement.criteria = requirement["criteria"] @@ -179,20 +182,29 @@ class CreditRequirement(TimeStampedModel): return credit_requirement, created @classmethod - def get_course_requirements(cls, course_key, namespace=None): + def get_course_requirements(cls, course_key, namespace=None, name=None): """ Get credit requirements of a given course. Args: - course_key(CourseKey): The identifier for a course - namespace(str): Namespace of credit course requirements + course_key (CourseKey): The identifier for a course + + Keyword Arguments + namespace (str): Optionally filter credit requirements by namespace. + name (str): Optionally filter credit requirements by name. Returns: QuerySet of CreditRequirement model + """ requirements = CreditRequirement.objects.filter(course__course_key=course_key, active=True) - if namespace: + + if namespace is not None: requirements = requirements.filter(namespace=namespace) + + if name is not None: + requirements = requirements.filter(name=name) + return requirements @classmethod @@ -261,8 +273,11 @@ class CreditRequirementStatus(TimeStampedModel): # the grade to users later and to send the information to credit providers. reason = JSONField(default={}) + # Maintain a history of requirement status updates for auditing purposes + history = HistoricalRecords() + class Meta(object): # pylint: disable=missing-docstring - get_latest_by = "created" + unique_together = ('username', 'requirement') @classmethod def get_statuses(cls, requirements, username): @@ -324,6 +339,40 @@ class CreditEligibility(TimeStampedModel): """ return cls.objects.filter(username=username).select_related('course').prefetch_related('course__providers') + @classmethod + def update_eligibility(cls, requirements, username, course_key): + """ + Update the user's credit eligibility for a course. + + A user is eligible for credit when the user has satisfied + all requirements for credit in the course. + + Arguments: + requirements (Queryset): Queryset of `CreditRequirement`s to check. + username (str): Identifier of the user being updated. + course_key (CourseKey): Identifier of the course. + + """ + # Check all requirements for the course to determine if the user + # is eligible. We need to check all the *requirements* + # (not just the *statuses*) in case the user doesn't yet have + # a status for a particular requirement. + status_by_req = defaultdict(lambda: False) + for status in CreditRequirementStatus.get_statuses(requirements, username): + status_by_req[status.requirement.id] = status.status + + is_eligible = all(status_by_req[req.id] == "satisfied" for req in requirements) + + # If we're eligible, then mark the user as being eligible for credit. + if is_eligible: + try: + CreditEligibility.objects.create( + username=username, + course=CreditCourse.objects.get(course_key=course_key), + ) + except IntegrityError: + pass + @classmethod def is_user_eligible_for_credit(cls, course_key, username): """Check if the given user is eligible for the provided credit course diff --git a/openedx/core/djangoapps/credit/tests/test_api.py b/openedx/core/djangoapps/credit/tests/test_api.py index dd2f3cbd83..14b5b7acba 100644 --- a/openedx/core/djangoapps/credit/tests/test_api.py +++ b/openedx/core/djangoapps/credit/tests/test_api.py @@ -30,11 +30,6 @@ from openedx.core.djangoapps.credit.models import ( CreditRequirementStatus, CreditEligibility ) -from openedx.core.djangoapps.credit.api import ( - set_credit_requirements, - set_credit_requirement_status, - get_credit_requirement -) from student.models import CourseEnrollment from student.views import _create_credit_availability_message from student.tests.factories import UserFactory @@ -240,7 +235,7 @@ class CreditRequirementApiTests(CreditApiTestBase): } } ] - requirement = get_credit_requirement(self.course_key, "grade", "grade") + requirement = api.get_credit_requirement(self.course_key, "grade", "grade") self.assertIsNone(requirement) expected_requirement = { @@ -252,8 +247,8 @@ class CreditRequirementApiTests(CreditApiTestBase): "min_grade": 0.8 } } - set_credit_requirements(self.course_key, requirements) - requirement = get_credit_requirement(self.course_key, "grade", "grade") + api.set_credit_requirements(self.course_key, requirements) + requirement = api.get_credit_requirement(self.course_key, "grade", "grade") self.assertIsNotNone(requirement) self.assertEqual(requirement, expected_requirement) @@ -276,25 +271,128 @@ class CreditRequirementApiTests(CreditApiTestBase): } ] - set_credit_requirements(self.course_key, requirements) - course_requirements = CreditRequirement.get_course_requirements(self.course_key) + api.set_credit_requirements(self.course_key, requirements) + course_requirements = api.get_credit_requirements(self.course_key) self.assertEqual(len(course_requirements), 2) - requirement = get_credit_requirement(self.course_key, "grade", "grade") - set_credit_requirement_status("staff", requirement, 'satisfied', {}) - course_requirement = CreditRequirement.get_course_requirement( - requirement['course_key'], requirement['namespace'], requirement['name'] - ) - status = CreditRequirementStatus.objects.get(username="staff", requirement=course_requirement) - self.assertEqual(status.requirement.namespace, requirement['namespace']) - self.assertEqual(status.status, "satisfied") + # Initially, the status should be None + req_status = api.get_credit_requirement_status(self.course_key, "staff", namespace="grade", name="grade") + self.assertEqual(req_status[0]["status"], None) - set_credit_requirement_status( - "staff", requirement, 'failed', {'failure_reason': "requirements not satisfied"} + # Set the requirement to "satisfied" and check that it's actually set + api.set_credit_requirement_status("staff", self.course_key, "grade", "grade") + req_status = api.get_credit_requirement_status(self.course_key, "staff", namespace="grade", name="grade") + self.assertEqual(req_status[0]["status"], "satisfied") + + # Set the requirement to "failed" and check that it's actually set + api.set_credit_requirement_status("staff", self.course_key, "grade", "grade", status="failed") + req_status = api.get_credit_requirement_status(self.course_key, "staff", namespace="grade", name="grade") + self.assertEqual(req_status[0]["status"], "failed") + + def test_satisfy_all_requirements(self): + # Configure a course with two credit requirements + self.add_credit_course() + requirements = [ + { + "namespace": "grade", + "name": "grade", + "display_name": "Grade", + "criteria": { + "min_grade": 0.8 + } + }, + { + "namespace": "reverification", + "name": "i4x://edX/DemoX/edx-reverification-block/assessment_uuid", + "display_name": "Assessment 1", + "criteria": {} + } + ] + api.set_credit_requirements(self.course_key, requirements) + + # Satisfy one of the requirements, but not the other + with self.assertNumQueries(7): + api.set_credit_requirement_status( + "bob", + self.course_key, + requirements[0]["namespace"], + requirements[0]["name"] + ) + + # The user should not be eligible (because only one requirement is satisfied) + self.assertFalse(api.is_user_eligible_for_credit("bob", self.course_key)) + + # Satisfy the other requirement + with self.assertNumQueries(10): + api.set_credit_requirement_status( + "bob", + self.course_key, + requirements[1]["namespace"], + requirements[1]["name"] + ) + + # Now the user should be eligible + self.assertTrue(api.is_user_eligible_for_credit("bob", self.course_key)) + + # The user should remain eligible even if the requirement status is later changed + api.set_credit_requirement_status( + "bob", + self.course_key, + requirements[0]["namespace"], + requirements[0]["name"], + status="failed" ) - status = CreditRequirementStatus.objects.get(username="staff", requirement=course_requirement) - self.assertEqual(status.requirement.namespace, requirement['namespace']) - self.assertEqual(status.status, "failed") + self.assertTrue(api.is_user_eligible_for_credit("bob", self.course_key)) + + def test_set_credit_requirement_status_req_not_configured(self): + # Configure a credit course with no requirements + self.add_credit_course() + + # A user satisfies a requirement. This could potentially + # happen if there's a lag when the requirements are updated + # after the course is published. + api.set_credit_requirement_status("bob", self.course_key, "grade", "grade") + + # Since the requirement hasn't been published yet, it won't show + # up in the list of requirements. + req_status = api.get_credit_requirement_status(self.course_key, "bob", namespace="grade", name="grade") + self.assertEqual(req_status, []) + + # Now add the requirements, simulating what happens when a course is published. + requirements = [ + { + "namespace": "grade", + "name": "grade", + "display_name": "Grade", + "criteria": { + "min_grade": 0.8 + } + }, + { + "namespace": "reverification", + "name": "i4x://edX/DemoX/edx-reverification-block/assessment_uuid", + "display_name": "Assessment 1", + "criteria": {} + } + ] + api.set_credit_requirements(self.course_key, requirements) + + # The user should not have satisfied the requirements, since they weren't + # in effect when the user completed the requirement + req_status = api.get_credit_requirement_status(self.course_key, "bob") + self.assertEqual(len(req_status), 2) + self.assertEqual(req_status[0]["status"], None) + self.assertEqual(req_status[0]["status"], None) + + # The user should *not* have satisfied the reverification requirement + req_status = api.get_credit_requirement_status( + self.course_key, + "bob", + namespace=requirements[1]["namespace"], + name=requirements[1]["name"] + ) + self.assertEqual(len(req_status), 1) + self.assertEqual(req_status[0]["status"], None) @ddt.ddt