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

Refactor user edx data fetching #1916

Merged
merged 1 commit into from Dec 2, 2016
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 5 additions & 12 deletions dashboard/api.py
Expand Up @@ -9,7 +9,7 @@
import pytz

from courses.models import Program
from dashboard.api_edx_cache import CachedEdxUserData
from dashboard.api_edx_cache import CachedEdxDataApi, CachedEdxUserData
from dashboard.utils import MMTrack

log = logging.getLogger(__name__)
Expand Down Expand Up @@ -120,15 +120,10 @@ def get_user_program_info(user, edx_client):
"""
# update cache
# NOTE: this part can be moved to an asynchronous task
for cache_type in CachedEdxUserData.SUPPORTED_CACHES:
CachedEdxUserData.update_cache_if_expired(user, edx_client, cache_type)
for cache_type in CachedEdxDataApi.SUPPORTED_CACHES:
CachedEdxDataApi.update_cache_if_expired(user, edx_client, cache_type)

# get enrollments for the student
enrollments = CachedEdxUserData.get_cached_edx_data(user, CachedEdxUserData.ENROLLMENT)
# get certificates for the student
certificates = CachedEdxUserData.get_cached_edx_data(user, CachedEdxUserData.CERTIFICATE)
# get current_grades for the student
current_grades = CachedEdxUserData.get_cached_edx_data(user, CachedEdxUserData.CURRENT_GRADE)
edx_user_data = CachedEdxUserData(user)

response_data = []
all_programs = (
Expand All @@ -138,9 +133,7 @@ def get_user_program_info(user, edx_client):
mmtrack_info = MMTrack(
user,
program,
enrollments,
current_grades,
certificates
edx_user_data
)
response_data.append(get_info_for_program(mmtrack_info))
return response_data
Expand Down
39 changes: 39 additions & 0 deletions dashboard/api_edx_cache.py
Expand Up @@ -16,6 +16,45 @@


class CachedEdxUserData:
"""Represents all edx data related to a User"""
# pylint: disable=too-many-instance-attributes

enrollments = None
certificates = None
current_grades = None
raw_enrollments = None
raw_certificates = None
raw_current_grades = None

def __init__(self, user, program=None, include_raw_data=False):
"""
Fetches the given User's edx data and sets object properties

Args:
user (User): a User object
program (Program): an optional Program to filter on
include_raw_data (bool): Set to True if raw edx data is needed (in addition to actual edX objects)
"""
self.user = user
self.program = program
self.fetch_and_set_edx_data()
if include_raw_data:
self.fetch_and_set_raw_data()

def fetch_and_set_edx_data(self):
"""Fetches edx data and sets object properties"""
self.enrollments = models.CachedEnrollment.get_edx_data(self.user, program=self.program)
self.certificates = models.CachedCertificate.get_edx_data(self.user, program=self.program)
self.current_grades = models.CachedCurrentGrade.get_edx_data(self.user, program=self.program)
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the purpose of having the filter by program here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

program defaults to none, and the cached model takes care of the logic for applying the program filter if it's specified. this is a way to reuse the logic for fetching and edx-serializing our cached data. if the program isn't passed into this class, the filter won't be applied

Copy link
Contributor

Choose a reason for hiding this comment

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

do we ever call CachedCurrentGrade.get_edx_data with a program specified?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes. when this class is instantiated from user search serialization: https://github.com/mitodl/micromasters/pull/1916/files#diff-5e24ceac8b57391a6805615b314c3da8R34

program is passed in, and that gets passed along to CachedCurrentGrade.get_edx_data in this method

Copy link
Contributor

Choose a reason for hiding this comment

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

ok


def fetch_and_set_raw_data(self):
"""Fetches raw cached data and sets object properties"""
self.raw_enrollments = list(models.CachedEnrollment.data_qset(self.user, program=self.program))
self.raw_certificates = list(models.CachedCertificate.data_qset(self.user, program=self.program))
self.raw_current_grades = list(models.CachedCurrentGrade.data_qset(self.user, program=self.program))


class CachedEdxDataApi:
"""
Class to handle the retrieval and update of the users' cached edX information
"""
Expand Down
152 changes: 106 additions & 46 deletions dashboard/api_edx_cache_test.py

Large diffs are not rendered by default.

13 changes: 5 additions & 8 deletions dashboard/api_test.py
Expand Up @@ -16,7 +16,7 @@
api,
models,
)
from dashboard.api_edx_cache import CachedEdxUserData
from dashboard.api_edx_cache import CachedEdxDataApi
from dashboard.factories import CachedEnrollmentFactory, CachedCurrentGradeFactory, UserCacheRefreshTimeFactory
from dashboard.utils import MMTrack
from micromasters.factories import UserFactory
Expand Down Expand Up @@ -750,17 +750,14 @@ def setUp(self):
self.expected_programs = [self.program_non_fin_aid, self.program_fin_aid]
self.edx_client = MagicMock()

@patch('dashboard.api_edx_cache.CachedEdxUserData.update_cache_if_expired', new_callable=MagicMock)
@patch('dashboard.api_edx_cache.CachedEdxUserData.get_cached_edx_data', new_callable=MagicMock)
def test_format(self, mock_cache_get, mock_cache_refresh):
@patch('dashboard.api_edx_cache.CachedEdxDataApi.update_cache_if_expired', new_callable=MagicMock)
def test_format(self, mock_cache_refresh):
"""Test that get_user_program_info fetches edx data and returns a list of Program data"""
result = api.get_user_program_info(self.user, self.edx_client)

assert mock_cache_refresh.call_count == len(CachedEdxUserData.SUPPORTED_CACHES)
assert mock_cache_get.call_count == len(CachedEdxUserData.SUPPORTED_CACHES)
for cache_type in CachedEdxUserData.SUPPORTED_CACHES:
assert mock_cache_refresh.call_count == len(CachedEdxDataApi.SUPPORTED_CACHES)
for cache_type in CachedEdxDataApi.SUPPORTED_CACHES:
mock_cache_refresh.assert_any_call(self.user, self.edx_client, cache_type)
mock_cache_get.assert_any_call(self.user, cache_type)

assert len(result) == 2
for i in range(2):
Expand Down
97 changes: 42 additions & 55 deletions dashboard/models.py
Expand Up @@ -36,49 +36,35 @@ class Meta:
abstract = True

@classmethod
def active_qset_all(cls, user):
def user_qset(cls, user, program=None):
"""
Returns a queryset for the active records associated with a User

Args:
user (User): an User object
program (Program): optional Program to filter on

Returns:
QuerySet: a queryset of all the elements for the provided user
"""
return cls.objects.filter(user=user)
query_params = dict(user=user)
if program is not None:
query_params.update(dict(course_run__course__program=program))
return cls.objects.filter(**query_params)

@classmethod
def active_qset_program(cls, user, program):
def data_qset(cls, user, program=None):
"""
Returns a queryset for the active records associated with a User/Program pair
Returns a queryset containing only the data property for the active records associated with a User

Args:
user (User): an User object
program (courses.models.Program): a program
program (Program): optional Program to filter on

Returns:
QuerySet: a queryset of all the elements for the provided user and program
QuerySet: a flattened list queryset of 'data' values
"""
return cls.objects.filter(
user=user,
course_run__course__program=program
)

@classmethod
def active_data(cls, user, program):
"""
Returns a list containing the 'data' property of every active record associated
with a User/Program pair

Args:
user (User): an User object
program (courses.models.Program): a program

Returns:
QuerySet: a queryset of all the data fields for the provided user and program
"""
return cls.active_qset_program(user, program).values_list('data', flat=True).all()
return cls.user_qset(user, program=program).values_list('data', flat=True)

@classmethod
def active_course_ids(cls, user):
Expand All @@ -91,7 +77,7 @@ def active_course_ids(cls, user):
Returns:
list: a list of all the course key fields for the provided user
"""
return list(cls.active_qset_all(user).values_list('course_run__edx_course_key', flat=True).all())
return list(cls.user_qset(user).values_list('course_run__edx_course_key', flat=True).all())

@classmethod
def delete_all_but(cls, user, course_ids_list):
Expand All @@ -105,16 +91,23 @@ def delete_all_but(cls, user, course_ids_list):
Returns:
None
"""
cls.active_qset_all(user).exclude(course_run__edx_course_key__in=course_ids_list).delete()
cls.user_qset(user).exclude(course_run__edx_course_key__in=course_ids_list).delete()

@classmethod
def get_edx_data(cls, user):
@staticmethod
def deserialize_edx_data(data_iter):
"""
Method to retrieve the cached date and encapsulate in specific edx-api-client classes.
Needs to be implemented in a per cache type bases
Instantiates an edX object with some iterable of raw data.
Must be implemented by specific subclasses.
"""
raise NotImplementedError

@classmethod
def get_edx_data(cls, user, program=None):
"""
Retrieves the cached data and encapsulates in specific edx-api-client classes.
"""
return cls.deserialize_edx_data(cls.data_qset(user, program=program))

def __str__(self):
"""
String representation of the model object
Expand All @@ -129,60 +122,54 @@ class CachedEnrollment(CachedEdxInfoModel):
"""
Model for user enrollment data from edX
"""

@classmethod
def get_edx_data(cls, user):
@staticmethod
def deserialize_edx_data(data_iter):
"""
Implementation for enrollments.
Deserializes raw enrollment data

Args:
user (User): an User object
data_iter (iterable): Some iterable of raw data

Returns:
Enrollments: an enrollments object for the user
Returns: Enrollments: an edX Enrollments object
"""
return Enrollments(
[enrollment.data for enrollment in cls.active_qset_all(user)]
)
return Enrollments(data_iter)


class CachedCertificate(CachedEdxInfoModel):
"""
Model for certificate data from edX
"""
@classmethod
def get_edx_data(cls, user):
@staticmethod
def deserialize_edx_data(data_iter):
"""
Implementation for certificates.
Deserializes raw certificate data

Args:
user (User): an User object
data_iter (iterable): Some iterable of raw data

Returns:
Certificates: a certificates object for the user
Returns: Certificates: an edX Certificates object
"""
return Certificates([
Certificate(certificate.data) for certificate in cls.active_qset_all(user)
Certificate(data) for data in data_iter
])


class CachedCurrentGrade(CachedEdxInfoModel):
"""
Model for current grade data from edX
"""
@classmethod
def get_edx_data(cls, user):
@staticmethod
def deserialize_edx_data(data_iter):
"""
Implementation for current grades.
Deserializes raw current grade data

Args:
user (User): an User object
data_iter (iterable): Some iterable of raw data

Returns:
CurrentGrades: a current grades object for the user
Returns: CachedCurrentGrade: an edX CachedCurrentGrade object
"""
return CurrentGrades([
CurrentGrade(grade.data) for grade in cls.active_qset_all(user)
CurrentGrade(data) for data in data_iter
])


Expand Down
20 changes: 6 additions & 14 deletions dashboard/serializers.py
Expand Up @@ -3,11 +3,7 @@
"""
from decimal import Decimal

from edx_api.certificates.models import Certificate, Certificates
from edx_api.grades.models import CurrentGrade, CurrentGrades
from edx_api.enrollments.models import Enrollments

from dashboard.models import CachedEnrollment, CachedCertificate, CachedCurrentGrade
from dashboard.api_edx_cache import CachedEdxUserData
from dashboard.utils import MMTrack
from roles.models import (
NON_LEARNERS,
Expand Down Expand Up @@ -35,22 +31,18 @@ def serialize(cls, program_enrollment):
"""
user = program_enrollment.user
program = program_enrollment.program
enrollments = list(CachedEnrollment.active_data(user, program))
certificates = list(CachedCertificate.active_data(user, program))
current_grades = list(CachedCurrentGrade.active_data(user, program))
edx_user_data = CachedEdxUserData(user, program=program, include_raw_data=True)

mmtrack = MMTrack(
user,
program,
Enrollments(enrollments),
CurrentGrades([CurrentGrade(grade) for grade in current_grades]),
Certificates([Certificate(cert) for cert in certificates])
edx_user_data
)
return {
'id': program.id,
'enrollments': enrollments,
'certificates': certificates,
'current_grades': current_grades,
'enrollments': edx_user_data.raw_enrollments,
'certificates': edx_user_data.raw_certificates,
'current_grades': edx_user_data.raw_current_grades,
'grade_average': cls.calculate_final_grade_average(mmtrack),
'is_learner': cls.is_learner(user, program),
'email_optin': user.profile.email_optin,
Expand Down
22 changes: 11 additions & 11 deletions dashboard/serializers_test.py
Expand Up @@ -137,9 +137,9 @@ def test_full_program_user_serialization(self):
program = self.program_enrollment.program
assert UserProgramSearchSerializer.serialize(self.program_enrollment) == {
'id': program.id,
'enrollments': list(CachedEnrollment.active_data(self.user, program)),
'certificates': list(CachedCertificate.active_data(self.user, program)),
'current_grades': list(CachedCurrentGrade.active_data(self.user, program)),
'enrollments': list(CachedEnrollment.data_qset(self.user, program=program)),
'certificates': list(CachedCertificate.data_qset(self.user, program=program)),
'current_grades': list(CachedCurrentGrade.data_qset(self.user, program=program)),
'grade_average': 75,
'is_learner': True,
'email_optin': True
Expand All @@ -155,9 +155,9 @@ def test_full_program_user_serialization_email_optin_changes(self, email_optin_f
program = self.program_enrollment.program
assert UserProgramSearchSerializer.serialize(self.program_enrollment) == {
'id': program.id,
'enrollments': list(CachedEnrollment.active_data(self.user, program)),
'certificates': list(CachedCertificate.active_data(self.user, program)),
'current_grades': list(CachedCurrentGrade.active_data(self.user, program)),
'enrollments': list(CachedEnrollment.data_qset(self.user, program=program)),
'certificates': list(CachedCertificate.data_qset(self.user, program=program)),
'current_grades': list(CachedCurrentGrade.data_qset(self.user, program=program)),
'grade_average': 75,
'is_learner': True,
'email_optin': email_optin_flag
Expand All @@ -174,9 +174,9 @@ def test_full_program_user_serialization_financial_aid(self):
self.profile.refresh_from_db()
expected_result = {
'id': self.fa_program.id,
'enrollments': list(CachedEnrollment.active_data(self.user, self.fa_program)),
'enrollments': list(CachedEnrollment.data_qset(self.user, program=self.fa_program)),
'certificates': [],
'current_grades': list(CachedCurrentGrade.active_data(self.user, self.fa_program)),
'current_grades': list(CachedCurrentGrade.data_qset(self.user, program=self.fa_program)),
'grade_average': 95,
'is_learner': True,
'email_optin': True
Expand All @@ -198,9 +198,9 @@ def test_full_program_user_serialization_staff(self):

assert UserProgramSearchSerializer.serialize(self.program_enrollment) == {
'id': program.id,
'enrollments': list(CachedEnrollment.active_data(self.user, program)),
'certificates': list(CachedCertificate.active_data(self.user, program)),
'current_grades': list(CachedCurrentGrade.active_data(self.user, program)),
'enrollments': list(CachedEnrollment.data_qset(self.user, program=program)),
'certificates': list(CachedCertificate.data_qset(self.user, program=program)),
'current_grades': list(CachedCurrentGrade.data_qset(self.user, program=program)),
'grade_average': 75,
'is_learner': False,
'email_optin': True
Expand Down