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

feat!: Remove allow_certificate checks from course certificates (#27206)

Browse Source 
DEPR-140 MICROBA-985
This commit is contained in:
Christie Rice
2021-04-05 14:00:07 -04:00
committed by GitHub
parent 9d7bd99f8d
commit 1181fb343e
9 changed files with 31 additions and 190 deletions
Download Patch File Download Diff File Expand all files Collapse all files

94
common/djangoapps/student/management/commands/cert_restriction.py
View File

@@ -1,94 +0,0 @@
# lint-amnesty, pylint: disable=missing-module-docstring
import csv
import os
from django.core.management.base import BaseCommand, CommandError
from common.djangoapps.student.models import UserProfile
class Command(BaseCommand): # lint-amnesty, pylint: disable=missing-class-docstring
help = """
Sets or gets certificate restrictions for users
from embargoed countries. (allow_certificate in
userprofile)
CSV should be comma delimited with double quoted entries.
$ ... cert_restriction --import path/to/userlist.csv
Export a list of students who have "allow_certificate" in
userprofile set to True
$ ... cert_restriction --output path/to/export.csv
Enable a single user so she is not on the restricted list
$ ... cert_restriction -e user
Disable a single user so she is on the restricted list
$ ... cert_restriction -d user
"""
def add_arguments(self, parser):
# This command can only take one of these arguments per run, this enforces that.
group = parser.add_mutually_exclusive_group(required=True)
group.add_argument('-i', '--import',
metavar='IMPORT_FILE',
nargs='?',
help='CSV file to import, comma delimitted file with double-quoted entries')
group.add_argument('-o', '--output',
metavar='EXPORT_FILE',
nargs='?',
help='CSV file to export')
group.add_argument('-e', '--enable',
metavar='STUDENT',
nargs='?',
help='Enable a certificate for a single student')
group.add_argument('-d', '--disable',
metavar='STUDENT',
nargs='?',
help='Disable a certificate for a single student')
def handle(self, *args, **options):
if options['output']:
if os.path.exists(options['output']):
raise CommandError("File {} already exists".format(options['output']))
disabled_users = UserProfile.objects.filter(allow_certificate=False)
with open(options['output'], 'w') as csvfile:
csvwriter = csv.writer(csvfile, delimiter=',', quotechar='"', quoting=csv.QUOTE_MINIMAL)
for user in disabled_users:
csvwriter.writerow([user.user.username])
print('{} disabled users written'.format(len(disabled_users)))
elif options['import']:
if not os.path.exists(options['import']):
raise CommandError("File {} does not exist".format(options['import']))
print("Importing students from {}".format(options['import']))
with open(options['import']) as csvfile:
student_list = csv.reader(csvfile, delimiter=',', quotechar='"')
students = [student[0] for student in student_list]
if not students:
raise CommandError("Unable to read student data from {}".format(options['import']))
update_cnt = UserProfile.objects.filter(user__username__in=students).update(allow_certificate=False)
print('{} user(s) disabled out of {} in CSV file'.format(update_cnt, len(students)))
elif options['enable']:
print("Enabling {} for certificate download".format(options['enable']))
cert_allow = UserProfile.objects.get(user__username=options['enable'])
cert_allow.allow_certificate = True
cert_allow.save()
elif options['disable']:
print("Disabling {} for certificate download".format(options['disable']))
cert_allow = UserProfile.objects.get(user__username=options['disable'])
cert_allow.allow_certificate = False
cert_allow.save()

1
common/djangoapps/student/tests/factories.py
View File

@@ -59,7 +59,6 @@ class UserProfileFactory(DjangoModelFactory): # lint-amnesty, pylint: disable=m
gender = 'm'
mailing_address = None
goals = 'Learn a lot'
allow_certificate = True
class RegistrationFactory(DjangoModelFactory): # lint-amnesty, pylint: disable=missing-class-docstring

3
common/test/db_fixtures/certificates_web_view.json
View File

@@ -37,8 +37,7 @@
"fields": {
"user": 99,
"name": "test cert",
"courseware": "course.xml",
"allow_certificate": true
"courseware": "course.xml"
}
},
{

8
import_shims/lms/student/management/commands/cert_restriction.py
View File

@@ -1,8 +0,0 @@
"""Deprecated import support. Auto-generated by import_shims/generate_shims.sh."""
# pylint: disable=redefined-builtin,wrong-import-position,wildcard-import,useless-suppression,line-too-long
from import_shims.warn import warn_deprecated_import
warn_deprecated_import('student.management.commands.cert_restriction', 'common.djangoapps.student.management.commands.cert_restriction')
from common.djangoapps.student.management.commands.cert_restriction import *

8
import_shims/studio/student/management/commands/cert_restriction.py
View File

@@ -1,8 +0,0 @@
"""Deprecated import support. Auto-generated by import_shims/generate_shims.sh."""
# pylint: disable=redefined-builtin,wrong-import-position,wildcard-import,useless-suppression,line-too-long
from import_shims.warn import warn_deprecated_import
warn_deprecated_import('student.management.commands.cert_restriction', 'common.djangoapps.student.management.commands.cert_restriction')
from common.djangoapps.student.management.commands.cert_restriction import *

13
lms/djangoapps/certificates/models.py
View File

@@ -89,9 +89,7 @@ class CertificateWhitelist(models.Model):
"""
Tracks students who are whitelisted, all users
in this table will always qualify for a certificate
regardless of their grade unless they are on the
embargoed country restriction list
(allow_certificate set to False in userprofile).
regardless of their grade.
.. no_pii:
"""
@@ -584,10 +582,9 @@ def certificate_status(generated_certificate):
deleted - The certificate has been deleted.
downloadable - The certificate is available for download.
notpassing - The student was graded but is not passing
restricted - The student is on the restricted embargo list and
should not be issued a certificate. This will
be set if allow_certificate is set to False in
the userprofile table
restricted - The student is on the restricted list. This status was
previously set if allow_certificate was set to False in
the userprofile table.
unverified - The student is in verified enrollment track and
the student did not have their identity verified,
even though they should be eligible for the cert otherwise.
@@ -643,7 +640,7 @@ def certificate_info_for_user(user, course_id, grade, user_is_whitelisted, user_
user_is_verified = grade is not None and mode_is_verified
eligible_for_certificate = 'Y' if (user_is_whitelisted or user_is_verified or certificate_generated) \
and user.profile.allow_certificate else 'N'
else 'N'
if certificate_generated and can_have_certificate:
certificate_is_delivered = 'Y'

24
lms/djangoapps/certificates/queue.py
View File

@@ -104,7 +104,6 @@ class XQueueCertInterface:
requests_auth,
)
self.whitelist = CertificateWhitelist.objects.all()
self.restricted = UserProfile.objects.filter(allow_certificate=False)
self.use_https = True
def regen_cert(self, student, course_id, course=None, forced_grade=None, template_file=None, generate_pdf=True):
@@ -212,9 +211,6 @@ class XQueueCertInterface:
If a student has a passing grade or is in the whitelist
table for the course a request will be made for a new cert.
If a student has allow_certificate set to False in the
userprofile table the status will change to 'restricted'
If a student does not have a passing grade the status
will change to status.notpassing
@@ -402,26 +398,6 @@ class XQueueCertInterface:
)
return cert
# Check to see whether the student is on the the embargoed
# country restricted list. If so, they should not receive a
# certificate -- set their status to restricted and log it.
if self.restricted.filter(user=student).exists():
cert.status = status.restricted
cert.save()
LOGGER.info(
(
"Student %s is in the embargoed country restricted "
"list, so their certificate status has been set to '%s' "
"for the course '%s'. "
"No certificate generation task was sent to the XQueue."
),
student.id,
cert.status,
str(course_id)
)
return cert
if unverified:
cert.status = status.unverified
cert.save()

45
lms/djangoapps/certificates/tests/tests.py
View File

@@ -58,19 +58,17 @@ class CertificatesModelTest(ModuleStoreTestCase, MilestonesTestCaseMixin):
@unpack
@data(
{'allow_certificate': False, 'whitelisted': False, 'grade': None, 'output': ['N', 'N', 'N/A']},
{'allow_certificate': True, 'whitelisted': True, 'grade': None, 'output': ['Y', 'N', 'N/A']},
{'allow_certificate': True, 'whitelisted': False, 'grade': 0.9, 'output': ['N', 'N', 'N/A']},
{'allow_certificate': False, 'whitelisted': True, 'grade': 0.8, 'output': ['N', 'N', 'N/A']},
{'allow_certificate': False, 'whitelisted': None, 'grade': 0.8, 'output': ['N', 'N', 'N/A']}
{'whitelisted': False, 'grade': None, 'output': ['N', 'N', 'N/A']},
{'whitelisted': True, 'grade': None, 'output': ['Y', 'N', 'N/A']},
{'whitelisted': False, 'grade': 0.9, 'output': ['N', 'N', 'N/A']},
{'whitelisted': True, 'grade': 0.8, 'output': ['Y', 'N', 'N/A']},
{'whitelisted': None, 'grade': 0.8, 'output': ['N', 'N', 'N/A']}
)
def test_certificate_info_for_user(self, allow_certificate, whitelisted, grade, output):
def test_certificate_info_for_user(self, whitelisted, grade, output):
"""
Verify that certificate_info_for_user works.
"""
student = UserFactory()
student.profile.allow_certificate = allow_certificate
student.profile.save() # pylint: disable=no-member
# for instructor paced course
certificate_info = certificate_info_for_user(
@@ -88,15 +86,15 @@ class CertificatesModelTest(ModuleStoreTestCase, MilestonesTestCaseMixin):
@unpack
@data(
{'allow_certificate': False, 'whitelisted': False, 'grade': None, 'output': ['N', 'Y', 'honor']},
{'allow_certificate': True, 'whitelisted': True, 'grade': None, 'output': ['Y', 'Y', 'honor']},
{'allow_certificate': True, 'whitelisted': False, 'grade': 0.9, 'output': ['Y', 'Y', 'honor']},
{'allow_certificate': False, 'whitelisted': True, 'grade': 0.8, 'output': ['N', 'Y', 'honor']},
{'allow_certificate': False, 'whitelisted': None, 'grade': 0.8, 'output': ['N', 'Y', 'honor']},
{'allow_certificate': True, 'whitelisted': None, 'grade': None, 'output': ['Y', 'Y', 'honor']},
{'allow_certificate': False, 'whitelisted': True, 'grade': None, 'output': ['N', 'Y', 'honor']}
{'whitelisted': False, 'grade': None, 'output': ['Y', 'Y', 'honor']},
{'whitelisted': True, 'grade': None, 'output': ['Y', 'Y', 'honor']},
{'whitelisted': False, 'grade': 0.9, 'output': ['Y', 'Y', 'honor']},
{'whitelisted': True, 'grade': 0.8, 'output': ['Y', 'Y', 'honor']},
{'whitelisted': None, 'grade': 0.8, 'output': ['Y', 'Y', 'honor']},
{'whitelisted': None, 'grade': None, 'output': ['Y', 'Y', 'honor']},
{'whitelisted': True, 'grade': None, 'output': ['Y', 'Y', 'honor']}
)
def test_certificate_info_for_user_when_grade_changes(self, allow_certificate, whitelisted, grade, output):
def test_certificate_info_for_user_when_grade_changes(self, whitelisted, grade, output):
"""
Verify that certificate_info_for_user works as expect in scenario when grading of problems
changes after certificates already generated. In such scenario `Certificate delivered` should not depend
@@ -104,8 +102,6 @@ class CertificatesModelTest(ModuleStoreTestCase, MilestonesTestCaseMixin):
of time.
"""
student = UserFactory()
student.profile.allow_certificate = allow_certificate
student.profile.save() # pylint: disable=no-member
certificate1 = GeneratedCertificateFactory.create(
user=student,
@@ -137,17 +133,16 @@ class CertificatesModelTest(ModuleStoreTestCase, MilestonesTestCaseMixin):
@unpack
@data(
{'allow_certificate': True, 'whitelisted': False, 'grade': 0.8, 'mode': 'audit', 'output': ['N', 'N', 'N/A']},
{'allow_certificate': True, 'whitelisted': True, 'grade': 0.8, 'mode': 'audit', 'output': ['Y', 'N', 'N/A']},
{'allow_certificate': True, 'whitelisted': False, 'grade': 0.8, 'mode': 'verified', 'output': ['Y', 'N', 'N/A']}
{'whitelisted': False, 'grade': 0.8, 'mode': 'audit', 'output': ['N', 'N', 'N/A']},
{'whitelisted': True, 'grade': 0.8, 'mode': 'audit', 'output': ['Y', 'N', 'N/A']},
{'whitelisted': False, 'grade': 0.8, 'mode': 'verified', 'output': ['Y', 'N', 'N/A']}
)
def test_certificate_info_for_user_with_course_modes(self, allow_certificate, whitelisted, grade, mode, output):
def test_certificate_info_for_user_with_course_modes(self, whitelisted, grade, mode, output):
"""
Verify that certificate_info_for_user works with course modes.
"""
user = UserFactory.create()
user.profile.allow_certificate = allow_certificate
user.profile.save()
_ = CourseEnrollment.enroll(user, self.instructor_paced_course.id, mode)
certificate_info = certificate_info_for_user(
user, self.instructor_paced_course.id, grade,
@@ -159,8 +154,6 @@ class CertificatesModelTest(ModuleStoreTestCase, MilestonesTestCaseMixin):
# Create one user with certs and one without
student_no_certs = UserFactory()
student_with_certs = UserFactory()
student_with_certs.profile.allow_certificate = True
student_with_certs.profile.save() # pylint: disable=no-member
# Set up a couple of courses
course_1 = CourseFactory.create()

25
lms/djangoapps/instructor_task/tests/test_tasks_helper.py
View File

@@ -1889,14 +1889,6 @@ class TestGradeReportEnrollmentAndCertificateInfo(TestReportMixin, InstructorTas
data=problem_xml
)
def user_is_embargoed(self, user, is_embargoed):
"""
Set a users embargoed state.
"""
user_profile = UserFactory(username=user.username, email=user.email).profile
user_profile.allow_certificate = not is_embargoed
user_profile.save() # pylint: disable=no-member
def _verify_csv_data(self, username, expected_data):
"""
Verify grade report data.
@@ -1919,7 +1911,6 @@ class TestGradeReportEnrollmentAndCertificateInfo(TestReportMixin, InstructorTas
user_enroll_mode,
has_passed,
whitelisted,
is_embargoed,
verification_status,
certificate_status,
certificate_mode):
@@ -1934,8 +1925,6 @@ class TestGradeReportEnrollmentAndCertificateInfo(TestReportMixin, InstructorTas
CertificateWhitelistFactory.create(user=user, course_id=self.course.id, whitelist=whitelisted)
self.user_is_embargoed(user, is_embargoed)
if user_enroll_mode in CourseMode.VERIFIED_MODES:
SoftwareSecurePhotoVerificationFactory.create(user=user, status=verification_status)
@@ -1950,19 +1939,19 @@ class TestGradeReportEnrollmentAndCertificateInfo(TestReportMixin, InstructorTas
@ddt.data(
(
'verified', False, False, False, 'approved', 'notpassing', 'honor',
'verified', False, False, 'approved', 'notpassing', 'honor',
['verified', 'ID Verified', 'N', 'N', 'N/A']
),
(
'verified', False, True, False, 'approved', 'downloadable', 'verified',
'verified', False, True, 'approved', 'downloadable', 'verified',
['verified', 'ID Verified', 'Y', 'Y', 'verified']
),
(
'honor', True, True, True, 'approved', 'restricted', 'honor',
['honor', 'N/A', 'N', 'N', 'N/A']
'honor', True, True, 'approved', 'restricted', 'honor',
['honor', 'N/A', 'Y', 'N', 'N/A']
),
(
'verified', True, True, False, 'must_retry', 'downloadable', 'honor',
'verified', True, True, 'must_retry', 'downloadable', 'honor',
['verified', 'Not ID Verified', 'Y', 'Y', 'honor']
),
)
@@ -1972,7 +1961,6 @@ class TestGradeReportEnrollmentAndCertificateInfo(TestReportMixin, InstructorTas
user_enroll_mode,
has_passed,
whitelisted,
is_embargoed,
verification_status,
certificate_status,
certificate_mode,
@@ -1983,7 +1971,6 @@ class TestGradeReportEnrollmentAndCertificateInfo(TestReportMixin, InstructorTas
user_enroll_mode,
has_passed,
whitelisted,
is_embargoed,
verification_status,
certificate_status,
certificate_mode
@@ -2034,7 +2021,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase):
'failed': 0,
'skipped': 2
}
with self.assertNumQueries(174):
with self.assertNumQueries(169):
self.assertCertificatesGenerated(task_input, expected_results)
expected_results = {