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 9, 2025
1 parent 14e4b24 commit 53e73c5
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 30 deletions.
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 Count, OuterRef, Subquery, Q, Value, BooleanField
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 get_base_queryset_courses, check_staff_exist_queryset

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
51 changes: 47 additions & 4 deletions tests/test_dashboard/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,50 @@ 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 +152,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 53e73c5

Please sign in to comment.