feat: add create external certificate task #76

andrey-canon · 2023-08-03T20:24:40Z

Description

https://edunext.atlassian.net/browse/FUTUREX-481

Testing instructions

Before

After

andrey-canon · 2023-08-05T01:24:46Z

@johanv26 this is ready for review

johanseto · 2023-08-03T21:47:32Z

eox_nelp/edxapp_wrapper/backends/student_m_v1.py

+
+def get_student_api():
+    """Allow to get the student_api module from
+    https://github.com/eduNEXT/edunext-platform/blob/ednx-release/mango.master/common/djangoapps/student/models_api.py


student_api is weird because the file name is models_api but there are no classes. May be is my habit to relate models to classes hehehe

If you check the certificate code openedx changes the models_api to student_api, so that's why a used the same openedx convention, however I'm not using this anymore, this belongs to a first version, and now I'm getting the user english name from the certificate data. So I will remove this

johanseto · 2023-08-08T15:15:23Z

eox_nelp/signals/receivers.py

@@ -63,7 +64,7 @@ def create_course_notifications(course_key, **kwargs):  # pylint: disable=unused
    create_course_notifications_task.delay(course_id=str(course_key))


-def certificate_publisher(certificate, **kwargs):  # pylint: disable=unused-argument
+def certificate_publisher(certificate, metadata, **kwargs):  # pylint: disable=unused-argument


Suggested change

def certificate_publisher(certificate, metadata, **kwargs): # pylint: disable=unused-argument

def certificate_publisher(certificate, metadata, *args, **kwargs): # pylint: disable=unused-argument

?

IMO that is not necessary, since the signal doesn't' send any args

johanseto · 2023-08-08T15:18:55Z

eox_nelp/signals/tasks.py

 logger = logging.getLogger(__name__)


-def _user_has_passing_grade(user, course_id):


Ok this also brings some refactor.

eox-nelp/eox_nelp/signals/utils.py

Lines 52 to 63 in 2b62c92

def _user_has_passing_grade(user, course_id):

"""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

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

johanseto · 2023-08-08T15:42:26Z

eox_nelp/signals/tests/test_receivers.py

+            certificate_publisher(self.certificate_data, self.metadata)
+
+        generate_data_mock.assert_called_with(
+            timestamp=self.metadata.time,


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

self.metadata = EventsMetadata(

event_type="org.openedx.learning.certificate.created.v1",

minorversion=0,

)

https://github.com/eduNEXT/openedx-events/blob/main/openedx_events/data.py#L42

But in this case what return Self.metadata.time
That could raise an error?if its optional...

Defaults to current time in UTC.

johanseto · 2023-08-08T15:44:25Z

eox_nelp/signals/tests/test_receivers.py

+
+    @patch("eox_nelp.signals.receivers._generate_external_certificate_data")
+    @patch("eox_nelp.signals.receivers.create_external_certificate")
+    def test_create_call(self, mock_task, generate_data_mock):


Suggested change

def test_create_call(self, mock_task, generate_data_mock):

def test_create_call(self, create_external_certificate_mock, generate_data_mock):

this is not more explicit?

johanseto · 2023-08-08T15:49:04Z

eox_nelp/signals/utils.py

+User = get_user_model()
+
+
+def _generate_external_certificate_data(timestamp, certificate_data):


Suggested change

def _generate_external_certificate_data(timestamp, certificate_data):

def _generate_futurex_external_certificate_data(timestamp, certificate_data):

??
Because we have a function like dispatch futurex progress?

I used futurex because that is the name of the service. In this case I have no idea how the service name's but probably it's not futurex since the base url is different

https://edunext.atlassian.net/browse/FUTUREX-481

This reverts commit 65cad739397645fcb1841d58784d3aaa9359141e.

johanseto

LGTM in my opinion

andrey-canon requested a review from johanseto August 3, 2023 20:24

github-actions bot added the size/s label Aug 3, 2023

johanseto assigned andrey-canon Aug 3, 2023

andrey-canon mentioned this pull request Aug 3, 2023

feat: add certificates api client #75

Merged

andrey-canon force-pushed the and/certifictae_task branch from 23bb9cf to 0ac72bf Compare August 4, 2023 18:51

github-actions bot added the test label Aug 4, 2023

andrey-canon force-pushed the and/certifictae_task branch from 0ac72bf to 9cc81a9 Compare August 4, 2023 22:34

github-actions bot added size/m m lines label and removed size/s labels Aug 4, 2023

andrey-canon changed the base branch from and/add_certificates_api to and/add_basic_authentication August 4, 2023 22:35

andrey-canon force-pushed the and/certifictae_task branch 2 times, most recently from a85da6d to 2b62c92 Compare August 5, 2023 01:09

andrey-canon marked this pull request as ready for review August 5, 2023 01:09

johanseto reviewed Aug 8, 2023

View reviewed changes

andrey-canon changed the base branch from and/add_basic_authentication to master August 8, 2023 18:24

feat: add create external certificate task

2617741

https://edunext.atlassian.net/browse/FUTUREX-481

andrey-canon force-pushed the and/certifictae_task branch from 2b62c92 to 2617741 Compare August 8, 2023 18:25

andrey-canon added 2 commits August 8, 2023 13:26

Revert "Auxiliary commit to revert individual files from 2617741"

fa4f81d

This reverts commit 65cad739397645fcb1841d58784d3aaa9359141e.

refactor: apply pr suggestions

2b28833

andrey-canon requested a review from johanseto August 8, 2023 18:44

johanseto approved these changes Aug 8, 2023

View reviewed changes

andrey-canon merged commit 7c7401a into master Aug 8, 2023
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add create external certificate task #76

feat: add create external certificate task #76

andrey-canon commented Aug 3, 2023

andrey-canon commented Aug 5, 2023

johanseto Aug 3, 2023

andrey-canon Aug 8, 2023

johanseto Aug 8, 2023

andrey-canon Aug 8, 2023

johanseto Aug 8, 2023

andrey-canon Aug 8, 2023

johanseto Aug 8, 2023

andrey-canon Aug 8, 2023

johanseto Aug 8, 2023 •

edited

Loading

andrey-canon Aug 8, 2023

johanseto Aug 8, 2023

johanseto Aug 8, 2023

andrey-canon Aug 8, 2023

johanseto left a comment

	def certificate_publisher(certificate, metadata, **kwargs): # pylint: disable=unused-argument
	def certificate_publisher(certificate, metadata, args, *kwargs): # pylint: disable=unused-argument

		logger = logging.getLogger(__name__)


		def _user_has_passing_grade(user, course_id):

	def _user_has_passing_grade(user, course_id):
	"""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

	self.metadata = EventsMetadata(
	event_type="org.openedx.learning.certificate.created.v1",
	minorversion=0,
	)

	def test_create_call(self, mock_task, generate_data_mock):
	def test_create_call(self, create_external_certificate_mock, generate_data_mock):

		User = get_user_model()


		def _generate_external_certificate_data(timestamp, certificate_data):

	def _generate_external_certificate_data(timestamp, certificate_data):
	def _generate_futurex_external_certificate_data(timestamp, certificate_data):

feat: add create external certificate task #76

feat: add create external certificate task #76

Conversation

andrey-canon commented Aug 3, 2023

Description

Testing instructions

Before

After

andrey-canon commented Aug 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johanseto Aug 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johanseto left a comment

Choose a reason for hiding this comment

johanseto Aug 8, 2023 •

edited

Loading