Skip to content
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: statistics API #4

Merged
merged 1 commit into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 5 additions & 21 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -1,27 +1,11 @@
name: Python CI

on:
push:
branches: [main]
issue_comment:
types: [created]
pull_request:
branches:
- '*'

jobs:
check_comment:
runs-on: ubuntu-latest
if: github.event.issue.pull_request != '' # Only runs if the comment is on a PR
outputs:
should_run: ${{ steps.comment_check.outputs.should_run }}
steps:
- name: Check for 'run tests' comment
id: comment_check
uses: actions/github-script@v5
with:
github-token: ${{secrets.GITHUB_TOKEN}}
script: |
const isRunTestsComment = '${{ github.event.comment.body }}'.trim() === 'run tests';
core.setOutput('should_run', isRunTestsComment ? 'true' : 'false');

run_tests:
name: tests
runs-on: ${{ matrix.os }}
Expand All @@ -32,9 +16,9 @@ jobs:
toxenv: [quality, django32]

steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
- name: setup python
uses: actions/setup-python@v2
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}

Expand Down
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,6 @@ docs/futurex_openedx_extensions.*.rst
# Private requirements
requirements/private.in
requirements/private.txt

# temporary tests migration files
/test_utils/edx_platform_mocks/fake_models/migrations/
2 changes: 1 addition & 1 deletion futurex_openedx_extensions/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
"""One-line description for README and other doc files."""

__version__ = '0.1.0'
__version__ = '0.1.1'
Empty file.
4 changes: 2 additions & 2 deletions futurex_openedx_extensions/dashboard/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
class DashboardConfig(AppConfig):
"""Configuration for the dashboard Django application"""

name = 'dashboard'
name = 'futurex_openedx_extensions.dashboard'

plugin_app = {
'settings_config': {
Expand All @@ -23,7 +23,7 @@ class DashboardConfig(AppConfig):
},
'url_config': {
'lms.djangoapp': {
'namespace': 'dashboard',
'namespace': 'fx_dashboard',
},
},
}
Empty file.
81 changes: 81 additions & 0 deletions futurex_openedx_extensions/dashboard/details/learners.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
"""Learners details collectors"""
from __future__ import annotations

from typing import List

from common.djangoapps.student.models import CourseAccessRole, UserSignupSource
from django.contrib.auth import get_user_model
from django.db.models import Count, Exists, OuterRef, Q, Subquery
from django.db.models.query import QuerySet
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview

from futurex_openedx_extensions.helpers.tenants import get_course_org_filter_list, get_tenant_site


def get_learners_queryset(tenant_ids: List, search_text: str = None) -> QuerySet:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This query is a bit slow on production. Any idea how can we make it faster?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the issue is not this particular query. It's the other helper queries, you'll find a few methods with TODO to add cache. These will increase the performance significantly as far as I can see

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if caching didn't fix it, we might need to do something with UserSignupSource. We need it to record logins, not just signups. So we can use it to directly filter the user by site.

Currently, there are too many records for users with enrollments in a site, but without a record in UserSignupSource

"""
Get the learners queryset for the given tenant IDs and search text.

:param tenant_ids: List of tenant IDs to get the learners for
:type tenant_ids: List
:param search_text: Search text to filter the learners by
:type search_text: str
"""
course_org_filter_list = get_course_org_filter_list(tenant_ids)['course_org_filter_list']
tenant_sites = []
for tenant_id in tenant_ids:
if site := get_tenant_site(tenant_id):
tenant_sites.append(site)

queryset = get_user_model().objects.filter(
is_superuser=False,
is_staff=False,
is_active=True,
)
search_text = (search_text or '').strip()
if search_text:
queryset = queryset.filter(
Q(username__icontains=search_text) |
Q(email__icontains=search_text) |
Q(profile__name__icontains=search_text)
)

queryset = queryset.annotate(
courses_count=Count(
'courseenrollment',
filter=(
Q(courseenrollment__course__org__in=course_org_filter_list) &
~Exists(
CourseAccessRole.objects.filter(
user_id=OuterRef('id'),
org=OuterRef('courseenrollment__course__org')
)
)
),
distinct=True
)
).annotate(
certificates_count=Count(
'generatedcertificate',
filter=(
Q(generatedcertificate__course_id__in=Subquery(
CourseOverview.objects.filter(
org__in=course_org_filter_list
).values_list('id', flat=True)
)) &
Q(generatedcertificate__status='downloadable')
),
distinct=True
)
).annotate(
has_site_login=Exists(
UserSignupSource.objects.filter(
user_id=OuterRef('id'),
site__in=tenant_sites
)
)
).filter(
Q(courses_count__gt=0) | Q(has_site_login=True)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this is the case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need the list of users related to the tenant. So, the user must be enrolled in one of the tenant's courses, or have a site login in the tenant's site

).select_related('profile').order_by('id')

return queryset
3 changes: 0 additions & 3 deletions futurex_openedx_extensions/dashboard/models.py

This file was deleted.

68 changes: 68 additions & 0 deletions futurex_openedx_extensions/dashboard/serializers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
"""Serializers for the dashboard details API."""

from django.contrib.auth import get_user_model
from rest_framework import serializers


class LearnerDetailsSerializer(serializers.ModelSerializer):
"""Serializer for learner details."""
user_id = serializers.SerializerMethodField()
full_name = serializers.SerializerMethodField()
username = serializers.CharField()
email = serializers.EmailField()
mobile_no = serializers.SerializerMethodField()
date_of_birth = serializers.SerializerMethodField()
gender = serializers.SerializerMethodField()
date_joined = serializers.DateTimeField()
last_login = serializers.DateTimeField()
enrolled_courses_count = serializers.SerializerMethodField()
certificates_count = serializers.SerializerMethodField()

class Meta:
model = get_user_model()
fields = [
'user_id',
'full_name',
'username',
'email',
'mobile_no',
'date_of_birth',
'gender',
'date_joined',
'last_login',
'enrolled_courses_count',
'certificates_count',
]

@staticmethod
def _get_profile_field(obj, field_name):
"""Get the profile field value."""
return getattr(obj.profile, field_name) if hasattr(obj, 'profile') and obj.profile else None

def get_user_id(self, obj):
"""Return user ID."""
return obj.id

def get_full_name(self, obj):
"""Return full name."""
return self._get_profile_field(obj, 'name')

def get_mobile_no(self, obj):
"""Return mobile number."""
return self._get_profile_field(obj, 'phone_number')

def get_date_of_birth(self, obj): # pylint: disable=unused-argument
"""Return date of birth."""
return None

def get_gender(self, obj):
"""Return gender."""
return self._get_profile_field(obj, 'gender')

def get_certificates_count(self, obj):
"""Return certificates count."""
return obj.certificates_count

def get_enrolled_courses_count(self, obj):
"""Return enrolled courses count."""
return obj.courses_count
Empty file.
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ def plugin_settings(settings): # pylint: disable=unused-argument
"""
plugin settings
"""
# Nothing to do here yet.
# Nothing to do here yet
Empty file.
36 changes: 36 additions & 0 deletions futurex_openedx_extensions/dashboard/statistics/certificates.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
"""functions for getting statistics about certificates"""
from __future__ import annotations

from typing import Dict, List

from django.db.models import Count, OuterRef, Subquery
from lms.djangoapps.certificates.models import GeneratedCertificate
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview

from futurex_openedx_extensions.helpers.tenants import get_course_org_filter_list


def get_certificates_count(tenant_ids: List[int]) -> Dict[str, int]:
"""
Get the count of issued certificates in the given tenants. The count is grouped by organization. Certificates
for admins, staff, and superusers are also included.

:param tenant_ids: List of tenant IDs to get the count for
:type tenant_ids: List[int]
:return: Count of certificates per organization
:rtype: Dict[str, int]
"""
course_org_filter_list = get_course_org_filter_list(tenant_ids)['course_org_filter_list']

result = list(GeneratedCertificate.objects.filter(
status='downloadable',
course_id__in=CourseOverview.objects.filter(
org__in=course_org_filter_list
),
).annotate(course_org=Subquery(
CourseOverview.objects.filter(
id=OuterRef('course_id')
).values('org')
)).values('course_org').annotate(certificates_count=Count('id')).values_list('course_org', 'certificates_count'))

return dict(result)
41 changes: 41 additions & 0 deletions futurex_openedx_extensions/dashboard/statistics/courses.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
"""functions for getting statistics about courses"""
from __future__ import annotations

from typing import List

from django.db.models import Count, Q
from django.db.models.query import QuerySet
from django.utils.timezone import now
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview

from futurex_openedx_extensions.helpers.tenants import get_course_org_filter_list


def get_courses_count(tenant_ids: List[int], only_active=False, only_visible=False) -> QuerySet:
"""
Get the count of courses in the given tenants

:param tenant_ids: List of tenant IDs to get the count for
:type tenant_ids: List[int]
:param only_active: Whether to only count active courses (according to dates)
:type only_active: bool
:param only_visible: Whether to only count visible courses (according to staff-only visibility)
:type only_visible: bool
:return: QuerySet of courses count per organization
:rtype: QuerySet
"""
course_org_filter_list = get_course_org_filter_list(tenant_ids)['course_org_filter_list']

q_set = CourseOverview.objects.filter(org__in=course_org_filter_list)
if only_active:
q_set = q_set.filter(
Q(start__isnull=True) | Q(start__lte=now()),
).filter(
Q(end__isnull=True) | Q(end__gte=now()),
)
if only_visible:
q_set = q_set.filter(visible_to_staff_only=False)

return q_set.values('org').annotate(
courses_count=Count('id')
).order_by('org')
Loading