Skip to content

Commit

Permalink
Fix include_staff in get_certificates_Count of stats api
Browse files Browse the repository at this point in the history
  • Loading branch information
tehreem-sadat committed Jan 10, 2025
1 parent 14e4b24 commit b2de349
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 32 deletions.
2 changes: 1 addition & 1 deletion futurex_openedx_extensions/dashboard/details/courses.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
from django.db.models.functions import Coalesce
from django.db.models.query import QuerySet
from eox_nelp.course_experience.models import FeedbackCourse
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from lms.djangoapps.certificates.models import GeneratedCertificate
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview

from futurex_openedx_extensions.helpers.querysets import (
check_staff_exist_queryset,
Expand Down
47 changes: 32 additions & 15 deletions futurex_openedx_extensions/dashboard/statistics/certificates.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,22 @@
from typing import Dict

from django.conf import settings
from django.db.models import Count, OuterRef, Subquery
from django.db.models import BooleanField, Count, OuterRef, Q, Subquery, Value
from django.db.models.functions import Lower
from lms.djangoapps.certificates.models import GeneratedCertificate
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview

from futurex_openedx_extensions.helpers.exceptions import FXCodedException, FXExceptionCodes
from futurex_openedx_extensions.helpers.querysets import get_base_queryset_courses
from futurex_openedx_extensions.helpers.querysets import check_staff_exist_queryset, get_base_queryset_courses

log = logging.getLogger(__name__)


def get_certificates_count(
fx_permission_info: dict, visible_courses_filter: bool | None = True, active_courses_filter: bool | None = None
fx_permission_info: dict,
visible_courses_filter: bool | None = True,
active_courses_filter: bool | None = None,
include_staff: bool | None = None
) -> Dict[str, int]:
"""
Get the count of issued certificates in the given tenants. The count is grouped by organization. Certificates
Expand All @@ -32,18 +35,32 @@ def get_certificates_count(
:return: Count of certificates per organization
:rtype: Dict[str, int]
"""
result = list(GeneratedCertificate.objects.filter(
status='downloadable',
course_id__in=get_base_queryset_courses(
fx_permission_info,
visible_filter=visible_courses_filter,
active_filter=active_courses_filter,
),
).annotate(course_org=Subquery(
CourseOverview.objects.filter(
id=OuterRef('course_id')
).values(org_lower_case=Lower('org'))
)).values('course_org').annotate(certificates_count=Count('id')).values_list('course_org', 'certificates_count'))
if include_staff:
is_staff_queryset = Q(Value(False, output_field=BooleanField()))
else:
is_staff_queryset = check_staff_exist_queryset(
ref_user_id='user_id', ref_org='course_org', ref_course_id='course_id',
)

result = list(
GeneratedCertificate.objects.filter(
status='downloadable',
course_id__in=get_base_queryset_courses(
fx_permission_info,
visible_filter=visible_courses_filter,
active_filter=active_courses_filter,
),
)
.annotate(
course_org=Subquery(
CourseOverview.objects.filter(
id=OuterRef('course_id')
).values(org_lower_case=Lower('org'))
)
)
.filter(~is_staff_queryset)
.values('course_org').annotate(certificates_count=Count('id')).values_list('course_org', 'certificates_count')
)

return dict(result)

Expand Down
6 changes: 3 additions & 3 deletions futurex_openedx_extensions/dashboard/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,9 @@ class TotalCountsView(FXViewRoleInfoMixin, APIView):
fx_view_description = 'api/fx/statistics/v1/total_counts/: Get the total count statistics'

@staticmethod
def _get_certificates_count_data(one_tenant_permission_info: dict) -> int:
def _get_certificates_count_data(one_tenant_permission_info: dict, include_staff: bool) -> int:
"""Get the count of certificates for the given tenant"""
collector_result = get_certificates_count(one_tenant_permission_info)
collector_result = get_certificates_count(one_tenant_permission_info, include_staff=include_staff)
return sum(certificate_count for certificate_count in collector_result.values())

@staticmethod
Expand Down Expand Up @@ -153,7 +153,7 @@ def _get_stat_count(self, stat: str, tenant_id: int, include_staff: bool) -> int

one_tenant_permission_info = get_tenant_limited_fx_permission_info(self.fx_permission_info, tenant_id)
if stat == self.STAT_CERTIFICATES:
result = self._get_certificates_count_data(one_tenant_permission_info)
result = self._get_certificates_count_data(one_tenant_permission_info, include_staff=include_staff)

elif stat == self.STAT_COURSES:
result = self._get_courses_count_data(one_tenant_permission_info, visible_filter=True)
Expand Down
16 changes: 8 additions & 8 deletions tests/test_dashboard/test_statistics/test_certificates.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,17 @@

@pytest.mark.django_db
@pytest.mark.parametrize('tenant_ids, expected_result', [
([1], {'org1': 4, 'org2': 10}),
([2], {'org3': 7, 'org8': 2}),
([1], {'org1': 2, 'org2': 9}),
([2], {'org3': 6, 'org8': 2}),
([3], {}),
([4], {}),
([5], {}),
([6], {}),
([7], {'org3': 7}),
([7], {'org3': 6}),
([8], {'org8': 2}),
([1, 2], {'org1': 4, 'org2': 10, 'org3': 7, 'org8': 2}),
([2, 7], {'org3': 7, 'org8': 2}),
([7, 8], {'org3': 7, 'org8': 2}),
([1, 2], {'org1': 2, 'org2': 9, 'org3': 6, 'org8': 2}),
([2, 7], {'org3': 6, 'org8': 2}),
([7, 8], {'org3': 6, 'org8': 2}),
])
def test_get_certificates_count(
base_data, fx_permission_info, tenant_ids, expected_result
Expand All @@ -37,15 +37,15 @@ def test_get_certificates_count(
def test_get_certificates_count_not_downloadable(base_data, fx_permission_info): # pylint: disable=unused-argument
"""Verify get_certificates_count function with empty tenant_ids."""
result = certificates.get_certificates_count(fx_permission_info)
assert result == {'org1': 4, 'org2': 10}, f'Wrong certificates result. expected: {result}'
assert result == {'org1': 2, 'org2': 9}, f'Wrong certificates result. expected: {result}'

some_status_not_downloadable = 'whatever'
GeneratedCertificate.objects.filter(
course_id='course-v1:ORG1+5+5',
user_id=40,
).update(status=some_status_not_downloadable)
result = certificates.get_certificates_count(fx_permission_info)
assert result == {'org1': 3, 'org2': 10}, f'Wrong certificates result. expected: {result}'
assert result == {'org1': 1, 'org2': 9}, f'Wrong certificates result. expected: {result}'


@pytest.mark.django_db
Expand Down
54 changes: 49 additions & 5 deletions tests/test_dashboard/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
IsSystemStaff,
)
from futurex_openedx_extensions.upgrade.models_switch import CourseAccessRole
from tests.base_test_data import expected_statistics
from tests.fixture_helpers import get_all_orgs, get_test_data_dict, get_user1_fx_permission_info
from tests.test_dashboard.test_mixins import MockPatcherMixin

Expand Down Expand Up @@ -92,7 +91,52 @@ def test_all_stats(self):
)
self.assertTrue(isinstance(response, JsonResponse))
self.assertEqual(response.status_code, http_status.HTTP_200_OK)
self.assertDictEqual(json.loads(response.content), expected_statistics)
self.assertDictEqual(json.loads(response.content), {
'1': {
'certificates_count': 11, 'courses_count': 12, 'hidden_courses_count': 0,
'learners_count': 16, 'enrollments_count': 26, 'learning_hours_count': 280
},
'2': {
'certificates_count': 8, 'courses_count': 5, 'hidden_courses_count': 0,
'learners_count': 21, 'enrollments_count': 21, 'learning_hours_count': 180
},
'3': {
'certificates_count': 0, 'courses_count': 1, 'hidden_courses_count': 0,
'learners_count': 6, 'enrollments_count': 4, 'learning_hours_count': 0
},
'7': {
'certificates_count': 6, 'courses_count': 3, 'hidden_courses_count': 0,
'learners_count': 17, 'enrollments_count': 14, 'learning_hours_count': 140
},
'8': {
'certificates_count': 2, 'courses_count': 2, 'hidden_courses_count': 0,
'learners_count': 9, 'enrollments_count': 7, 'learning_hours_count': 40
},
'total_certificates_count': 27, 'total_courses_count': 23, 'total_hidden_courses_count': 0,
'total_learners_count': 69, 'total_enrollments_count': 72, 'total_learning_hours_count': 640,
'total_unique_learners': 37, 'limited_access': False
})

def test_all_stats_with_include_staff(self):
"""Test get method"""
self.login_user(self.staff_user)
response = self.client.get(
self.url + '?stats=certificates,courses,learners,enrollments&include_staff=1'
)
self.assertTrue(isinstance(response, JsonResponse))
self.assertEqual(response.status_code, http_status.HTTP_200_OK)
self.assertDictEqual(json.loads(response.content), {
'1': {'certificates_count': 14, 'courses_count': 12, 'learners_count': 18, 'enrollments_count': 32},
'2': {'certificates_count': 9, 'courses_count': 5, 'learners_count': 26, 'enrollments_count': 25},
'3': {'certificates_count': 0, 'courses_count': 1, 'learners_count': 6, 'enrollments_count': 4},
'7': {'certificates_count': 7, 'courses_count': 3, 'learners_count': 20, 'enrollments_count': 17},
'8': {'certificates_count': 2, 'courses_count': 2, 'learners_count': 10, 'enrollments_count': 8},
'total_certificates_count': 32,
'total_courses_count': 23,
'total_learners_count': 80,
'total_enrollments_count': 86,
'limited_access': False
})

def test_limited_access(self):
"""Test get method with limited access"""
Expand All @@ -109,9 +153,9 @@ def test_selected_tenants(self):
self.assertTrue(isinstance(response, JsonResponse))
self.assertEqual(response.status_code, http_status.HTTP_200_OK)
expected_response = {
'1': {'certificates_count': 14, 'courses_count': 12, 'learners_count': 16},
'2': {'certificates_count': 9, 'courses_count': 5, 'learners_count': 21},
'total_certificates_count': 23,
'1': {'certificates_count': 11, 'courses_count': 12, 'learners_count': 16},
'2': {'certificates_count': 8, 'courses_count': 5, 'learners_count': 21},
'total_certificates_count': 19,
'total_courses_count': 17,
'total_learners_count': 37,
'limited_access': False,
Expand Down

0 comments on commit b2de349

Please sign in to comment.