Jlc/certificates metrics #96

johanseto · 2023-09-12T19:24:35Z

Description

Add certificates key for the metric backend.
This add a certificates key for each course metric with this shape for course endpoint

{
            "certificates": {
                "verified": {"downloadable": 0, "notpassing": 0},
                "honor": {"downloadable": 0, "notpassing": 0},
                "audit": {"downloadable": 0, "notpassing": 0},
                "professional": {"downloadable": 0, "notpassing": 0},
                "no-id-professional": {"downloadable": 5, "notpassing": 4},
                "masters": {"downloadable": 0, "notpassing": 0},
                "executive-education": {"downloadable": 0, "notpassing": 0},
                "paid-executive-education": {"downloadable": 0, "notpassing": 0},
                "paid-bootcamp": {"downloadable": 0, "notpassing": 0},
                "total": 9,
            }
}

For the tenant endpoint only the total certificates of the tenant is shown:

{
    "learners": 1,
    "courses": 3,
    "instructors": 2,
    "components": {
        "discussion": 0,
        "drag-and-drop-v2": 0,
        "html": 133,
        "openassessment": 0,
        "problem": 49,
        "video": 0
    },
    "certiticates": 12
}

Testing instructions

Clone this branch and check the behavior of the stats endpoints.

/eox-nelp/api/stats/v1/tenant/
/eox-nelp/api/stats/v1/courses/
/eox-nelp/api/stats/v1/courses/course-v1:edX+cd202+2023

After

Additional information

Jira story:
https://edunext.atlassian.net/browse/FUTUREX-489

Checklist for Merge

Tested in a remote environment
Updated documentation
Rebased master/main
Squashed commits

andrey-canon · 2023-09-12T19:46:02Z

eox_nelp/stats/metrics.py

+                "executive-education": {...}
+                "paid-executive-education":{...},
+                "paid-bootcamp": {...},
+                "total": 0


since you are splitting the certificates based on their status the total metric should reflect also that division

Could you explain this one a little more, I mean I am setting the total without the division.
So the division would reflect something like the total:

"total" : { "no-id-professional": 5, "audit": 5 }

Is that what you say?
Because if is that I prefer to set the total of each one inside each key?? But I didn't find it necessary due you only have to sum the field of each one

I didn't say mode

"total": { "downloadable": 5, "notpassing": 4, },

I didn't say mode

"total": { "downloadable": 5, "notpassing": 4, },

@andrey-canon what about this approach ?

did you remove the modes ?

andrey-canon · 2023-09-12T19:49:17Z

eox_nelp/migrations/0001_initial.py

@@ -95,6 +95,26 @@ class Migration(migrations.Migration):
                ('org', models.CharField(blank=True, max_length=500, null=True))
            ],
        ),
+        migrations.CreateModel(


why are you creating a a backend if you are creating the model ?

Because the backend allows me to load the class that passes the data to the test db.

eox-nelp/eox_nelp/stats/tests/tests_metrics.py

Line 273 in f398520

GeneratedCertificate.objects.get_or_create(**{

eox-nelp/eox_nelp/edxapp_wrapper/test_backends/certificates_m_v1.py

Lines 32 to 58 in f398520

def get_generated_certificate():

"""Return test model.

Returns:

Generated Certificates dummy model.

"""

generated_certificate_fields = {

"user": models.ForeignKey(User, on_delete=models.CASCADE),

"course_id": CourseKeyField(max_length=255, blank=True, default=None),

"verify_uuid": models.CharField(max_length=32, blank=True, default='', db_index=True),

"grade": models.CharField(max_length=5, blank=True, default=''),

"key": models.CharField(max_length=32, blank=True, default=''),

"distinction": models.BooleanField(default=False),

"status": models.CharField(max_length=32, default='unavailable'),

"mode": models.CharField(max_length=32, choices=MODES, default=MODES.honor),

"name": models.CharField(blank=True, max_length=255),

"created_date": models.DateTimeField(auto_now_add=True),

"modified_date": models.DateTimeField(auto_now=True),

"download_uuid": models.CharField(max_length=32, blank=True, default=''),

"download_url": models.CharField(max_length=128, blank=True, default=''),

"error_reason": models.CharField(max_length=512, blank=True, default=''),

# not model fields :

"MODES": MODES,

}

return create_test_model(

"GeneratedCertificate", "eox_nelp", __package__, generated_certificate_fields

)

generally we don't duplicate models from edx-platform, why this case is so especial ?

Actually is because in the test I use the model to generate the certificates in test db. So the mainly reason is that, as you can see there course_overview also was recreated, but with less fields

eox-nelp/eox_nelp/migrations/0001_initial.py

Lines 83 to 97 in f398520

migrations.CreateModel(

name='CourseOverview',

fields=[

(

'id',

opaque_keys.edx.django.models.CourseKeyField(

db_index=True,

primary_key=True,

max_length=255,

verbose_name='ID',

),

),

('org', models.CharField(blank=True, max_length=500, null=True))

],

),

The reasons is that I want to test against like "real certs data"...

So in the test, as you can see I don't mock any function of the certificates because the django test db has the model and data(GeneratedCertficates). So functions like GeneratedCertficates.objects... work and return data from test db.

eox-nelp/eox_nelp/stats/tests/tests_metrics.py

Lines 273 to 296 in f398520

GeneratedCertificate.objects.get_or_create(**{

'user': user,

'course_id': CourseKey.from_string("course-v1:test+Cx105+2022_T4"),

'verify_uuid': 'ddad6d87c5084a3facfd7925b0b2b9a3',

'download_uuid': '',

'download_url': '',

'grade': '71.0',

'key': '807e31d92ab6aeaab514d7669bb2b014',

'distinction': False,

'status': 'downloadable',

'mode': 'no-id-professional',

'name': 'Peter Park',

'error_reason': '',

})

GeneratedCertificate.objects.get_or_create(**{

'user': user2,

'course_id': CourseKey.from_string("course-v1:test+Cx105+2022_T4"),

'verify_uuid': 'ddad6d87c5084a3facfd7925b0b2b9a3',

'download_uuid': '',

'download_url': '',

'grade': '59.0',

'key': '807e31d92ab6aeaab514d7669bb2b014',

'distinction': False,

'status': 'notpassing',

Course overview was created because another model use that as foreign key, there was no another option

IMO there are another alternatives like Mock that could fit here without adding fields that you even don't use

Actually, I started trying to use the mock approach, therefore as I used different Django queryset attributes and mixed them, I found that I have to do a lot of different mocks approaches and also with sense. So I stated to be a little mixed, so finally, I preferred to load the model and get data as the django queryset would work.

eox-nelp/eox_nelp/stats/tests/tests_metrics.py

Lines 257 to 269 in f398520

# this block set the CourseEnrollment mock and its returned values.

filter_result = CourseEnrollment.objects.filter.return_value

values_result = filter_result.values.return_value

distinct_result = values_result.distinct.return_value

distinct_result.count.return_value = self.expected_returned_enrollments

# this block set the CourseAccessRole mock and its returned values.

filter_result = CourseAccessRole.objects.filter.return_value

values_result = filter_result.values.return_value

distinct_result = values_result.distinct.return_value

distinct_result.count.return_value = self.expected_returned_roles

# this block set the GeneratedCertificates mock and its returned values.

Also I set another fields, maybe extra but that could be useful if others created a certificate in the django db test model.

Actually all was loaded from here. Practically I have the same model that the openedx
https://github.com/openedx/edx-platform/blob/master/lms/djangoapps/certificates/models.py#L224-L240

That's the point we can not copy all the models that openedx use

style: isort and pylint pass chore: set tests checkpoint with cert backend style: pylint and isort chore: rename correct cert_status chore: add new dosctrings feat: add test for course certificates metric fx test: add test for view including total certs chore: rename `not_passing` refactor: pr recommendations for total count refactor: use GeneratedCertificates main fields PR recomendations: This ensure I create a django test model that only use the fields that test or the implementation use.\

github-actions bot added test size/m m lines label labels Sep 12, 2023

johanseto assigned andrey-canon Sep 12, 2023

johanseto requested a review from andrey-canon September 12, 2023 19:24

andrey-canon reviewed Sep 12, 2023

View reviewed changes

andrey-canon approved these changes Sep 13, 2023

View reviewed changes

johanseto force-pushed the jlc/certificates-metrics branch from 6ed2d7d to 87f4d83 Compare September 13, 2023 21:24

johanseto merged commit f19260e into master Sep 13, 2023
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jlc/certificates metrics #96

Jlc/certificates metrics #96

johanseto commented Sep 12, 2023

andrey-canon Sep 12, 2023 •

edited

Loading

johanseto Sep 12, 2023

andrey-canon Sep 12, 2023

johanseto Sep 12, 2023

andrey-canon Sep 12, 2023

andrey-canon Sep 12, 2023 •

edited

Loading

johanseto Sep 12, 2023

andrey-canon Sep 12, 2023

johanseto Sep 12, 2023 •

edited

Loading

johanseto Sep 12, 2023 •

edited

Loading

andrey-canon Sep 12, 2023

johanseto Sep 12, 2023

andrey-canon Sep 12, 2023

	def get_generated_certificate():
	"""Return test model.
	Returns:
	Generated Certificates dummy model.
	"""
	generated_certificate_fields = {
	"user": models.ForeignKey(User, on_delete=models.CASCADE),
	"course_id": CourseKeyField(max_length=255, blank=True, default=None),
	"verify_uuid": models.CharField(max_length=32, blank=True, default='', db_index=True),
	"grade": models.CharField(max_length=5, blank=True, default=''),
	"key": models.CharField(max_length=32, blank=True, default=''),
	"distinction": models.BooleanField(default=False),
	"status": models.CharField(max_length=32, default='unavailable'),
	"mode": models.CharField(max_length=32, choices=MODES, default=MODES.honor),
	"name": models.CharField(blank=True, max_length=255),
	"created_date": models.DateTimeField(auto_now_add=True),
	"modified_date": models.DateTimeField(auto_now=True),
	"download_uuid": models.CharField(max_length=32, blank=True, default=''),
	"download_url": models.CharField(max_length=128, blank=True, default=''),
	"error_reason": models.CharField(max_length=512, blank=True, default=''),
	# not model fields :
	"MODES": MODES,
	}

	return create_test_model(
	"GeneratedCertificate", "eox_nelp", __package__, generated_certificate_fields
	)

	migrations.CreateModel(
	name='CourseOverview',
	fields=[
	(
	'id',
	opaque_keys.edx.django.models.CourseKeyField(
	db_index=True,
	primary_key=True,
	max_length=255,
	verbose_name='ID',
	),
	),
	('org', models.CharField(blank=True, max_length=500, null=True))
	],
	),

	GeneratedCertificate.objects.get_or_create(**{
	'user': user,
	'course_id': CourseKey.from_string("course-v1:test+Cx105+2022_T4"),
	'verify_uuid': 'ddad6d87c5084a3facfd7925b0b2b9a3',
	'download_uuid': '',
	'download_url': '',
	'grade': '71.0',
	'key': '807e31d92ab6aeaab514d7669bb2b014',
	'distinction': False,
	'status': 'downloadable',
	'mode': 'no-id-professional',
	'name': 'Peter Park',
	'error_reason': '',
	})
	GeneratedCertificate.objects.get_or_create(**{
	'user': user2,
	'course_id': CourseKey.from_string("course-v1:test+Cx105+2022_T4"),
	'verify_uuid': 'ddad6d87c5084a3facfd7925b0b2b9a3',
	'download_uuid': '',
	'download_url': '',
	'grade': '59.0',
	'key': '807e31d92ab6aeaab514d7669bb2b014',
	'distinction': False,
	'status': 'notpassing',

	# this block set the CourseEnrollment mock and its returned values.
	filter_result = CourseEnrollment.objects.filter.return_value
	values_result = filter_result.values.return_value
	distinct_result = values_result.distinct.return_value
	distinct_result.count.return_value = self.expected_returned_enrollments

	# this block set the CourseAccessRole mock and its returned values.
	filter_result = CourseAccessRole.objects.filter.return_value
	values_result = filter_result.values.return_value
	distinct_result = values_result.distinct.return_value
	distinct_result.count.return_value = self.expected_returned_roles

	# this block set the GeneratedCertificates mock and its returned values.

Jlc/certificates metrics #96

Jlc/certificates metrics #96

Conversation

johanseto commented Sep 12, 2023

Description

Testing instructions

After

Additional information

Checklist for Merge

andrey-canon Sep 12, 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

andrey-canon Sep 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johanseto Sep 12, 2023 • edited Loading

Choose a reason for hiding this comment

johanseto Sep 12, 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

andrey-canon Sep 12, 2023 •

edited

Loading

andrey-canon Sep 12, 2023 •

edited

Loading

johanseto Sep 12, 2023 •

edited

Loading

johanseto Sep 12, 2023 •

edited

Loading