Skip to content

Commit

Permalink
fixed cohort test by using hasattr instead of .get()
Browse files Browse the repository at this point in the history
  • Loading branch information
dsjen committed Aug 29, 2016
1 parent df8bf42 commit 510c2a7
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 26 deletions.
40 changes: 24 additions & 16 deletions analytics_data_api/v0/serializers.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
from collections import OrderedDict
from urlparse import urljoin
from django.conf import settings
from rest_framework import pagination, serializers
from rest_framework.response import Response

from analytics_data_api.constants import (
engagement_events,
enrollment_modes,
learner,
)
from analytics_data_api.v0 import models

Expand Down Expand Up @@ -344,7 +347,7 @@ class LearnerSerializer(serializers.Serializer):
segments = serializers.SerializerMethodField()
engagements = serializers.SerializerMethodField()
enrollment_date = serializers.DateTimeField(format=settings.DATE_FORMAT)
cohort = serializers.CharField()
cohort = serializers.SerializerMethodField()

def get_segments(self, obj):
# using hasattr() instead because DocType.get() is overloaded and makes a request
Expand All @@ -355,7 +358,11 @@ def get_segments(self, obj):
return []

def get_cohort(self, obj):
return obj.get('cohort', None)
if hasattr(obj, 'cohort') and len(obj.cohort) > 0:
# json parsing will fail unless in unicode
return obj.cohort
else:
return None

def get_account_url(self, obj):
if settings.LMS_USER_ACCOUNT_BASE_URL:
Expand Down Expand Up @@ -388,20 +395,21 @@ class EdxPaginationSerializer(pagination.PageNumberPagination):
"""
Adds values to the response according to edX REST API Conventions.
"""
num_pages = serializers.ReadOnlyField(source='paginator.num_pages')


class ElasticsearchDSLSearchSerializer(EdxPaginationSerializer):
def __init__(self, *args, **kwargs):
"""Make sure that the elasticsearch query is executed."""
# Because the elasticsearch-dsl search object has a different
# API from the queryset object that's expected by the django
# Paginator object, we have to manually execute the query.
# Note that the `kwargs['instance']` is the Page object, and
# `kwargs['instance'].object_list` is actually an
# elasticsearch-dsl search object.
kwargs['instance'].object_list = kwargs['instance'].object_list.execute()
super(ElasticsearchDSLSearchSerializer, self).__init__(*args, **kwargs)
page_size_query_param = 'page_size'
page_size = learner.LEARNER_API_DEFAULT_LIST_PAGE_SIZE
max_page_size = 100 # TODO -- tweak during load testing

def get_paginated_response(self, data):
# The output is more readable with num_pages included not at the end, but
# inefficient to insert into an OrderedDict, so the response is copied from
# rest_framework.pagination with the addition of "num_pages".
return Response(OrderedDict([
('count', self.page.paginator.count),
('num_pages', self.page.paginator.num_pages),
('next', self.get_next_link()),
('previous', self.get_previous_link()),
('results', data)
]))


# pylint: disable=abstract-method
Expand Down
2 changes: 1 addition & 1 deletion analytics_data_api/v0/tests/views/test_learners.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ def assert_learners_returned(self, response, expected_learners):
returned.
"""
self.assertEqual(response.status_code, 200)
returned_learners = json.loads(response.content)
returned_learners = json.loads(response.content)['results']
if expected_learners is None:
self.assertEqual(returned_learners, list())
else:
Expand Down
14 changes: 5 additions & 9 deletions analytics_data_api/v0/views/learners.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@

from rest_framework import generics, status

from analytics_data_api.constants import (
learner
)
from analytics_data_api.v0.exceptions import (
LearnerEngagementTimelineNotFoundError,
LearnerNotFoundError,
Expand All @@ -21,7 +18,7 @@
)
from analytics_data_api.v0.serializers import (
CourseLearnerMetadataSerializer,
ElasticsearchDSLSearchSerializer,
EdxPaginationSerializer,
EngagementDaySerializer,
LastUpdatedSerializer,
LearnerSerializer,
Expand Down Expand Up @@ -187,9 +184,7 @@ class LearnerListView(LastUpdateMixin, CourseViewMixin, generics.ListAPIView):
"""
serializer_class = LearnerSerializer
pagination_serializer_class = ElasticsearchDSLSearchSerializer
paginate_by_param = 'page_size'
paginate_by = learner.LEARNER_API_DEFAULT_LIST_PAGE_SIZE
pagination_class = EdxPaginationSerializer
max_paginate_by = 100 # TODO -- tweak during load testing

def _validate_query_params(self):
Expand Down Expand Up @@ -222,8 +217,9 @@ def list(self, request, *args, **kwargs):
"""
response = super(LearnerListView, self).list(request, args, kwargs)
last_updated = self.get_last_updated()
for result in response.data:
result.update(last_updated)
if response.data['results'] is not None:
for result in response.data['results']:
result.update(last_updated)
return response

def get_queryset(self):
Expand Down

0 comments on commit 510c2a7

Please sign in to comment.