-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add create external certificate task #76
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -12,31 +12,17 @@ | |||||||||||||||||||||||||
from django.db.models import Q | ||||||||||||||||||||||||||
from eox_core.edxapp_wrapper.courseware import get_courseware_courses | ||||||||||||||||||||||||||
from eox_core.edxapp_wrapper.enrollments import get_enrollment | ||||||||||||||||||||||||||
from eox_core.edxapp_wrapper.grades import get_course_grade_factory | ||||||||||||||||||||||||||
from opaque_keys.edx.keys import CourseKey | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
from eox_nelp.api_clients.certificates import ExternalCertificatesApiClient | ||||||||||||||||||||||||||
from eox_nelp.api_clients.futurex import FuturexApiClient | ||||||||||||||||||||||||||
from eox_nelp.edxapp_wrapper.course_overviews import CourseOverview | ||||||||||||||||||||||||||
from eox_nelp.signals.utils import _user_has_passing_grade | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
courses = get_courseware_courses() | ||||||||||||||||||||||||||
CourseGradeFactory = get_course_grade_factory() | ||||||||||||||||||||||||||
logger = logging.getLogger(__name__) | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
def _user_has_passing_grade(user, course_id): | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok this also brings some refactor. eox-nelp/eox_nelp/signals/utils.py Lines 52 to 63 in 2b62c92
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, Imoved some common method to an different file in order to use them in multiple places, the name starting with _ could be consider as wrong since that have a private connotation, but I didn't make bigger this pr by adding name changes |
||||||||||||||||||||||||||
"""Determines if a user has passed a course based on the grading policies. | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
Args: | ||||||||||||||||||||||||||
user<User>: Instace of Django User model. | ||||||||||||||||||||||||||
course_id<str>: Unique course identifier. | ||||||||||||||||||||||||||
Returns: | ||||||||||||||||||||||||||
course_grade.passed<bool>: True if the user has passed the course, otherwise False | ||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||
course_grade = CourseGradeFactory().read(user, course_key=CourseKey.from_string(course_id)) | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
return course_grade.passed | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
@shared_task | ||||||||||||||||||||||||||
def dispatch_futurex_progress(course_id, user_id, is_complete=None): | ||||||||||||||||||||||||||
"""Dispatch the course progress of a user to Futurex platform. | ||||||||||||||||||||||||||
|
@@ -148,3 +134,17 @@ def _generate_progress_enrollment_data(user, course_id, user_has_passing_grade): | |||||||||||||||||||||||||
progress_enrollment_data, | ||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||
return progress_enrollment_data | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
@shared_task | ||||||||||||||||||||||||||
def create_external_certificate(external_certificate_data): | ||||||||||||||||||||||||||
"""This will create an external NELP certificate base on the input data | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
Args: | ||||||||||||||||||||||||||
timestamp<Datetime>: Date when the certificate was created. | ||||||||||||||||||||||||||
certificate<CertificateData>: This an instance of the class defined in this link | ||||||||||||||||||||||||||
https://github.com/eduNEXT/openedx-events/blob/main/openedx_events/learning/data.py#L100 | ||||||||||||||||||||||||||
and will provide of the user certificate data. | ||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||
api_client = ExternalCertificatesApiClient() | ||||||||||||||||||||||||||
api_client.create_external_certificate(external_certificate_data) |
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -6,11 +6,19 @@ | |||||||||
import unittest | ||||||||||
|
||||||||||
from django.contrib.auth import get_user_model | ||||||||||
from django.test import override_settings | ||||||||||
from mock import patch | ||||||||||
from opaque_keys.edx.keys import CourseKey | ||||||||||
from openedx_events.data import EventsMetadata | ||||||||||
from openedx_events.learning.data import CertificateData, CourseData, UserData, UserPersonalData | ||||||||||
|
||||||||||
from eox_nelp.edxapp_wrapper.test_backends import create_test_model | ||||||||||
from eox_nelp.signals.receivers import block_completion_progress_publisher, course_grade_changed_progress_publisher | ||||||||||
from eox_nelp.signals import receivers | ||||||||||
from eox_nelp.signals.receivers import ( | ||||||||||
block_completion_progress_publisher, | ||||||||||
certificate_publisher, | ||||||||||
course_grade_changed_progress_publisher, | ||||||||||
) | ||||||||||
|
||||||||||
User = get_user_model() | ||||||||||
|
||||||||||
|
@@ -64,3 +72,173 @@ def test_call_dispatch(self, dispatch_mock): | |||||||||
course_id=str(course_key), | ||||||||||
user_id=13, | ||||||||||
) | ||||||||||
|
||||||||||
|
||||||||||
class CertificatePublisherTestCase(unittest.TestCase): | ||||||||||
"""Test class for certificate_publisher.""" | ||||||||||
|
||||||||||
def setUp(self): | ||||||||||
"""Setup common conditions for every test case""" | ||||||||||
self.username = "Harry" | ||||||||||
self.course_key = CourseKey.from_string("course-v1:test+Cx105+2022_T4") | ||||||||||
self.certificate_data = CertificateData( | ||||||||||
user=UserData( | ||||||||||
pii=UserPersonalData( | ||||||||||
username=self.username, | ||||||||||
email="[email protected]", | ||||||||||
name="Harry Potter", | ||||||||||
), | ||||||||||
id=10, | ||||||||||
is_active=True, | ||||||||||
), | ||||||||||
course=CourseData( | ||||||||||
course_key=self.course_key, | ||||||||||
), | ||||||||||
mode="no-id-professional", | ||||||||||
grade=5, | ||||||||||
current_status="downloadable", | ||||||||||
download_url="", | ||||||||||
name="", | ||||||||||
) | ||||||||||
self.metadata = EventsMetadata( | ||||||||||
event_type="org.openedx.learning.certificate.created.v1", | ||||||||||
minorversion=0, | ||||||||||
) | ||||||||||
|
||||||||||
@override_settings(ENABLE_CERTIFICATE_PUBLISHER=False) | ||||||||||
@patch("eox_nelp.signals.receivers.create_external_certificate") | ||||||||||
def test_inactive_behavior(self, create_external_certificate_mock): | ||||||||||
"""Test that the asynchronous task wont' be called when the setting is not active. | ||||||||||
|
||||||||||
Expected behavior: | ||||||||||
- create_external_certificate is not called | ||||||||||
""" | ||||||||||
certificate_publisher(self.certificate_data, self.metadata) | ||||||||||
|
||||||||||
create_external_certificate_mock.delay.assert_not_called() | ||||||||||
|
||||||||||
@patch("eox_nelp.signals.receivers.create_external_certificate") | ||||||||||
def test_invalid_mode(self, create_external_certificate_mock): | ||||||||||
"""Test when the certificate data has an invalid mode. | ||||||||||
|
||||||||||
Expected behavior: | ||||||||||
- create_external_certificate is not called. | ||||||||||
- Invalid error was logged. | ||||||||||
""" | ||||||||||
invalid_mode = "audit" | ||||||||||
log_info = ( | ||||||||||
f"The {invalid_mode} certificate associated with the user <{self.username}> and course <{self.course_key}> " | ||||||||||
"doesn't have a valid mode and therefore its data won't be published." | ||||||||||
) | ||||||||||
|
||||||||||
certificate_data = CertificateData( | ||||||||||
user=UserData( | ||||||||||
pii=UserPersonalData( | ||||||||||
username=self.username, | ||||||||||
email="[email protected]", | ||||||||||
name="Harry Potter", | ||||||||||
), | ||||||||||
id=10, | ||||||||||
is_active=True, | ||||||||||
), | ||||||||||
course=CourseData( | ||||||||||
course_key=self.course_key, | ||||||||||
), | ||||||||||
mode=invalid_mode, | ||||||||||
grade=5, | ||||||||||
current_status="downloadable", | ||||||||||
download_url="", | ||||||||||
name="", | ||||||||||
) | ||||||||||
|
||||||||||
with self.assertLogs(receivers.__name__, level="INFO") as logs: | ||||||||||
certificate_publisher(certificate_data, self.metadata) | ||||||||||
|
||||||||||
create_external_certificate_mock.delay.assert_not_called() | ||||||||||
self.assertEqual(logs.output, [ | ||||||||||
f"INFO:{receivers.__name__}:{log_info}" | ||||||||||
]) | ||||||||||
|
||||||||||
@patch("eox_nelp.signals.receivers._generate_external_certificate_data") | ||||||||||
@patch("eox_nelp.signals.receivers.create_external_certificate") | ||||||||||
def test_create_call(self, create_external_certificate_mock, generate_data_mock): | ||||||||||
"""Test when the certificate mode is valid and the asynchronous task is called | ||||||||||
|
||||||||||
Expected behavior: | ||||||||||
- _generate_external_certificate_data is called with the right parameters. | ||||||||||
- create_external_certificate is called with the _generate_external_certificate_data output. | ||||||||||
- Info was logged. | ||||||||||
""" | ||||||||||
generate_data_mock.return_value = { | ||||||||||
"test": True, | ||||||||||
} | ||||||||||
log_info = ( | ||||||||||
f"The no-id-professional certificate associated with the user <{self.username}> and " | ||||||||||
f"course <{self.course_key}> has been already generated and its data will be sent " | ||||||||||
"to the NELC certificate service." | ||||||||||
) | ||||||||||
|
||||||||||
with self.assertLogs(receivers.__name__, level="INFO") as logs: | ||||||||||
certificate_publisher(self.certificate_data, self.metadata) | ||||||||||
|
||||||||||
generate_data_mock.assert_called_with( | ||||||||||
timestamp=self.metadata.time, | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the time of the object where is generated, or is auto generated? eox-nelp/eox_nelp/signals/tests/test_receivers.py Lines 103 to 106 in 2b62c92
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But in this case what return There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Defaults to current time in UTC. |
||||||||||
certificate_data=self.certificate_data, | ||||||||||
) | ||||||||||
create_external_certificate_mock.delay.assert_called_with( | ||||||||||
external_certificate_data=generate_data_mock() | ||||||||||
) | ||||||||||
self.assertEqual(logs.output, [ | ||||||||||
f"INFO:{receivers.__name__}:{log_info}" | ||||||||||
]) | ||||||||||
|
||||||||||
@override_settings(CERTIFICATE_PUBLISHER_VALID_MODES=["another-mode"]) | ||||||||||
@patch("eox_nelp.signals.receivers._generate_external_certificate_data") | ||||||||||
@patch("eox_nelp.signals.receivers.create_external_certificate") | ||||||||||
def test_alternative_mode(self, create_external_certificate_mock, generate_data_mock): | ||||||||||
"""Test when the certificate data has an alternative mode. | ||||||||||
|
||||||||||
Expected behavior: | ||||||||||
- _generate_external_certificate_data is called with the right parameters. | ||||||||||
- create_external_certificate is called with the _generate_external_certificate_data output. | ||||||||||
- Info was logged. | ||||||||||
""" | ||||||||||
alternative_mode = "another-mode" | ||||||||||
log_info = ( | ||||||||||
f"The {alternative_mode} certificate associated with the user <{self.username}> and " | ||||||||||
f"course <{self.course_key}> has been already generated and its data will be sent " | ||||||||||
"to the NELC certificate service." | ||||||||||
) | ||||||||||
certificate_data = CertificateData( | ||||||||||
user=UserData( | ||||||||||
pii=UserPersonalData( | ||||||||||
username=self.username, | ||||||||||
email="[email protected]", | ||||||||||
name="Harry Potter", | ||||||||||
), | ||||||||||
id=10, | ||||||||||
is_active=True, | ||||||||||
), | ||||||||||
course=CourseData( | ||||||||||
course_key=self.course_key, | ||||||||||
), | ||||||||||
mode=alternative_mode, | ||||||||||
grade=5, | ||||||||||
current_status="downloadable", | ||||||||||
download_url="", | ||||||||||
name="", | ||||||||||
) | ||||||||||
|
||||||||||
with self.assertLogs(receivers.__name__, level="INFO") as logs: | ||||||||||
certificate_publisher(certificate_data, self.metadata) | ||||||||||
|
||||||||||
generate_data_mock.assert_called_with( | ||||||||||
timestamp=self.metadata.time, | ||||||||||
certificate_data=certificate_data, | ||||||||||
) | ||||||||||
create_external_certificate_mock.delay.assert_called_with( | ||||||||||
external_certificate_data=generate_data_mock() | ||||||||||
) | ||||||||||
self.assertEqual(logs.output, [ | ||||||||||
f"INFO:{receivers.__name__}:{log_info}" | ||||||||||
]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO that is not necessary, since the signal doesn't' send any args