Restrict certificate endpoint to return certificates.
Do not return certificates if there is no active certificate configuration. PROD-1200
This commit is contained in:
@@ -7,6 +7,7 @@ from itertools import product
|
||||
|
||||
import ddt
|
||||
import six
|
||||
from django.conf import settings
|
||||
from django.urls import reverse
|
||||
from django.utils import timezone
|
||||
from freezegun import freeze_time
|
||||
@@ -43,6 +44,13 @@ class CertificatesDetailRestApiTest(AuthAndScopesTestMixin, SharedModuleStoreTes
|
||||
number='verified',
|
||||
display_name='Verified Course'
|
||||
)
|
||||
CourseOverviewFactory.create(
|
||||
id=cls.course.id,
|
||||
display_org_with_default='edx',
|
||||
display_name='Verified Course',
|
||||
cert_html_view_enabled=True,
|
||||
self_paced=True,
|
||||
)
|
||||
|
||||
def setUp(self):
|
||||
freezer = freeze_time(self.now)
|
||||
@@ -51,7 +59,7 @@ class CertificatesDetailRestApiTest(AuthAndScopesTestMixin, SharedModuleStoreTes
|
||||
|
||||
super(CertificatesDetailRestApiTest, self).setUp()
|
||||
|
||||
GeneratedCertificateFactory.create(
|
||||
self.cert = GeneratedCertificateFactory.create(
|
||||
user=self.student,
|
||||
course_id=self.course.id,
|
||||
status=CertificateStatuses.downloadable,
|
||||
@@ -102,6 +110,25 @@ class CertificatesDetailRestApiTest(AuthAndScopesTestMixin, SharedModuleStoreTes
|
||||
'no_certificate_for_user',
|
||||
)
|
||||
|
||||
def test_no_certificate_configuration(self):
|
||||
"""
|
||||
Verify that certificate is not returned if there is no active
|
||||
certificate configuration.
|
||||
"""
|
||||
self.cert.download_url = ''
|
||||
self.cert.save()
|
||||
resp = self.get_response(
|
||||
AuthType.session,
|
||||
requesting_user=self.student,
|
||||
requested_user=self.student,
|
||||
)
|
||||
self.assertEqual(resp.status_code, status.HTTP_404_NOT_FOUND)
|
||||
self.assertIn('error_code', resp.data)
|
||||
self.assertEqual(
|
||||
resp.data['error_code'],
|
||||
'no_certificate_configuration_for_course',
|
||||
)
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
class CertificatesListRestApiTest(AuthAndScopesTestMixin, SharedModuleStoreTestCase, APITestCase):
|
||||
@@ -135,7 +162,7 @@ class CertificatesListRestApiTest(AuthAndScopesTestMixin, SharedModuleStoreTestC
|
||||
|
||||
super(CertificatesListRestApiTest, self).setUp()
|
||||
|
||||
GeneratedCertificateFactory.create(
|
||||
self.cert = GeneratedCertificateFactory.create(
|
||||
user=self.student,
|
||||
course_id=self.course.id,
|
||||
status=CertificateStatuses.downloadable,
|
||||
@@ -155,7 +182,7 @@ class CertificatesListRestApiTest(AuthAndScopesTestMixin, SharedModuleStoreTestC
|
||||
}
|
||||
)
|
||||
|
||||
def assert_success_response_for_student(self, response):
|
||||
def assert_success_response_for_student(self, response, download_url='www.google.com'):
|
||||
""" This method is required by AuthAndScopesTestMixin. """
|
||||
self.assertEqual(
|
||||
response.data,
|
||||
@@ -169,7 +196,7 @@ class CertificatesListRestApiTest(AuthAndScopesTestMixin, SharedModuleStoreTestC
|
||||
'modified_date': self.now,
|
||||
'status': CertificateStatuses.downloadable,
|
||||
'is_passing': True,
|
||||
'download_url': 'www.google.com',
|
||||
'download_url': download_url,
|
||||
'grade': '0.88',
|
||||
}]
|
||||
)
|
||||
@@ -328,3 +355,32 @@ class CertificatesListRestApiTest(AuthAndScopesTestMixin, SharedModuleStoreTestC
|
||||
)
|
||||
self.assertEqual(resp.status_code, status.HTTP_200_OK)
|
||||
self.assertEqual(len(resp.data), 2)
|
||||
|
||||
@patch.dict(settings.FEATURES, {'CERTIFICATES_HTML_VIEW': True})
|
||||
def test_with_no_certificate_configuration(self):
|
||||
"""
|
||||
Verify that certificates are not returned until there is an active
|
||||
certificate configuration.
|
||||
"""
|
||||
self.cert.download_url = ''
|
||||
self.cert.save()
|
||||
|
||||
response = self.get_response(
|
||||
AuthType.jwt,
|
||||
requesting_user=self.student,
|
||||
requested_user=self.student,
|
||||
)
|
||||
self.assertEqual(response.status_code, status.HTTP_200_OK)
|
||||
self.assertEqual(response.data, [])
|
||||
|
||||
self.course_overview.has_any_active_web_certificate = True
|
||||
self.course_overview.save()
|
||||
|
||||
response = self.get_response(
|
||||
AuthType.jwt,
|
||||
requesting_user=self.student,
|
||||
requested_user=self.student,
|
||||
)
|
||||
kwargs = {"user_id": str(self.student.id), "course_id": six.text_type(self.course.id)}
|
||||
expected_download_url = reverse('certificates:html_view', kwargs=kwargs)
|
||||
self.assert_success_response_for_student(response, download_url=expected_download_url)
|
||||
|
||||
@@ -120,6 +120,15 @@ class CertificatesDetailView(GenericAPIView):
|
||||
status=404,
|
||||
data={'error_code': 'no_certificate_for_user'}
|
||||
)
|
||||
|
||||
course_overview = CourseOverview.get_from_id(course_id)
|
||||
# return 404 if it's not a PDF certificates and there is no active certificate configuration.
|
||||
if not user_cert['is_pdf_certificate'] and not course_overview.has_any_active_web_certificate:
|
||||
return Response(
|
||||
status=404,
|
||||
data={'error_code': 'no_certificate_configuration_for_course'}
|
||||
)
|
||||
|
||||
return Response(
|
||||
{
|
||||
"username": user_cert.get('username'),
|
||||
@@ -265,9 +274,12 @@ class CertificatesListView(GenericAPIView):
|
||||
).items():
|
||||
if certificates_viewable_for_course(course_overview):
|
||||
course_certificate = passing_certificates[course_key]
|
||||
course_certificate['course_display_name'] = course_overview.display_name_with_default
|
||||
course_certificate['course_organization'] = course_overview.display_org_with_default
|
||||
viewable_certificates.append(course_certificate)
|
||||
# add certificate into viewable certificate list only if it's a PDF certificate
|
||||
# or there is an active certificate configuration.
|
||||
if course_certificate['is_pdf_certificate'] or course_overview.has_any_active_web_certificate:
|
||||
course_certificate['course_display_name'] = course_overview.display_name_with_default
|
||||
course_certificate['course_organization'] = course_overview.display_org_with_default
|
||||
viewable_certificates.append(course_certificate)
|
||||
|
||||
viewable_certificates.sort(key=lambda certificate: certificate['created'])
|
||||
return viewable_certificates
|
||||
|
||||
@@ -274,7 +274,7 @@ class LearnerProfileViewTest(SiteMixin, UrlResetMixin, ModuleStoreTestCase):
|
||||
@mock.patch.dict(settings.FEATURES, {'CERTIFICATES_HTML_VIEW': True})
|
||||
def test_certificate_visibility_with_no_cert_config(self):
|
||||
"""
|
||||
Verify that certificates are not displayed until there is no active
|
||||
Verify that certificates are not displayed until there is an active
|
||||
certificate configuration.
|
||||
"""
|
||||
# Add new certificate
|
||||
|
||||
Reference in New Issue
Block a user