From 9ea3ec78287eda3abc358ba39d5c930db1301a10 Mon Sep 17 00:00:00 2001 From: Tyler Hallada Date: Thu, 20 Apr 2017 16:17:45 -0400 Subject: [PATCH 1/5] Add programs endpoint with tests --- analyticsclient/client.py | 2 + analyticsclient/course_summaries.py | 7 +- analyticsclient/programs.py | 40 +++++++++ analyticsclient/tests/__init__.py | 81 +++++++++++++++++++ .../tests/test_course_summaries.py | 79 ++++-------------- analyticsclient/tests/test_programs.py | 9 +++ 6 files changed, 152 insertions(+), 66 deletions(-) create mode 100644 analyticsclient/programs.py create mode 100644 analyticsclient/tests/test_programs.py diff --git a/analyticsclient/client.py b/analyticsclient/client.py index 9846f6f..6808847 100644 --- a/analyticsclient/client.py +++ b/analyticsclient/client.py @@ -6,6 +6,7 @@ from analyticsclient.course import Course from analyticsclient.course_summaries import CourseSummaries +from analyticsclient.programs import Programs from analyticsclient.exceptions import ClientError, InvalidRequestError, NotFoundError, TimeoutError from analyticsclient.module import Module from analyticsclient.status import Status @@ -42,6 +43,7 @@ def __init__(self, base_url, auth_token=None, timeout=0.25): self.status = Status(self) self.course_summaries = lambda: CourseSummaries(self) + self.programs = lambda: Programs(self) self.courses = lambda course_id: Course(self, course_id) self.modules = lambda course_id, module_id: Module(self, course_id, module_id) diff --git a/analyticsclient/course_summaries.py b/analyticsclient/course_summaries.py index 51385dd..648e83a 100644 --- a/analyticsclient/course_summaries.py +++ b/analyticsclient/course_summaries.py @@ -17,16 +17,19 @@ def __init__(self, client): """ self.client = client - def course_summaries(self, course_ids=None, fields=None, data_format=DF.JSON): + def course_summaries(self, course_ids=None, fields=None, exclude=None, programs=None, data_format=DF.JSON): """ Get list of summaries. Arguments: course_ids: Array of course IDs as strings to return. Default is to return all. fields: Array of fields to return. Default is to return all. + exclude: Array of fields to exclude from response. Default is to not exclude any fields. + programs: If included in the query parameters, will include the programs array in the response. """ query_params = {} - for query_arg, data in zip(['course_ids', 'fields'], [course_ids, fields]): + for query_arg, data in zip(['course_ids', 'fields', 'exclude', 'programs'], + [course_ids, fields, exclude, programs]): if data: query_params[query_arg] = ','.join(data) diff --git a/analyticsclient/programs.py b/analyticsclient/programs.py new file mode 100644 index 0000000..eb2fe2b --- /dev/null +++ b/analyticsclient/programs.py @@ -0,0 +1,40 @@ +import urllib + +import analyticsclient.constants.data_format as DF + + +class Programs(object): + """Programs client.""" + + def __init__(self, client): + """ + Initialize the Programs client. + + Arguments: + + client (analyticsclient.client.Client): The client to use to access remote resources. + + """ + self.client = client + + def programs(self, program_ids=None, fields=None, exclude=None, data_format=DF.JSON): + """ + Get list of programs metadata. + + Arguments: + program_ids: Array of program IDs as strings to return. Default is to return all. + fields: Array of fields to return. Default is to return all. + exclude: Array of fields to exclude from response. Default is to not exclude any fields. + """ + query_params = {} + for query_arg, data in zip(['program_ids', 'fields', 'exclude'], + [program_ids, fields, exclude]): + if data: + query_params[query_arg] = ','.join(data) + + path = 'programs/' + querystring = urllib.urlencode(query_params) + if querystring: + path += '?{0}'.format(querystring) + + return self.client.get(path, data_format=data_format) diff --git a/analyticsclient/tests/__init__.py b/analyticsclient/tests/__init__.py index 80b20be..d17dc93 100644 --- a/analyticsclient/tests/__init__.py +++ b/analyticsclient/tests/__init__.py @@ -1,5 +1,8 @@ from unittest import TestCase +import ddt +import httpretty + from analyticsclient.client import Client @@ -22,3 +25,81 @@ def get_api_url(self, path): Complete API URL and path """ return "{0}/{1}".format(self.client.base_url, path) + + +@ddt.ddt +class APIListTestCase(object): + """Base class for API list view tests.""" + + # Override in the subclass: + endpoint = 'list' + + def setUp(self): + """Set up the test case.""" + super(APIListTestCase, self).setUp() + self.base_uri = self.get_api_url('{}/'.format(self.endpoint)) + self.client_class = getattr(self.client, self.endpoint)() + httpretty.enable() + + def verify_last_querystring_equal(self, expected_query): + """Convenience method for asserting the last request was made with the expected query parameters.""" + self.assertDictEqual(httpretty.last_request().querystring, expected_query) + + def expected_query(self, **kwargs): + """Pack the query arguments into expected format for http pretty.""" + query = {} + for field, data in kwargs.items(): + if data is not None: + query[field] = [','.join(data)] + return query + + @httpretty.activate + def kwarg_test(self, **kwargs): + """Construct URL with given query parameters and check if it is what we expect.""" + httpretty.reset() + uri_template = '{uri}?' + for key in kwargs: + uri_template += '%s={%s}' % (key, key) + uri = uri_template.format(uri=self.base_uri, **kwargs) + httpretty.register_uri(httpretty.GET, uri, body='{}') + getattr(self.client_class, self.endpoint)(**kwargs) + self.verify_last_querystring_equal(self.expected_query(**kwargs)) + + def test_all_items_url(self): + """Endpoint can be called without parameters.""" + httpretty.register_uri(httpretty.GET, self.base_uri, body='{}') + getattr(self.client_class, self.endpoint)() + + @ddt.data( + ['edx/demo/course'], + ['edx/demo/course', 'another/demo/course'] + ) + def test_courses_ids(self, course_ids): + """Endpoint can be called with course IDs.""" + self.kwarg_test(course_ids=course_ids) + + @ddt.data( + ['course_id'], + ['course_id', 'enrollment_modes'] + ) + def test_fields(self, fields): + """Endpoint can be called with fields.""" + self.kwarg_test(fields=fields) + + @ddt.data( + ['course_id'], + ['course_id', 'enrollment_modes'] + ) + def test_exclude(self, exclude): + """Endpoint can be called with exclude.""" + self.kwarg_test(exclude=exclude) + + @ddt.data( + (['edx/demo/course'], ['course_id'], ['enrollment_modes']), + (['edx/demo/course', 'another/demo/course'], ['course_id', 'enrollment_modes'], + ['created', 'pacing_type']) + ) + @ddt.unpack + def test_all_parameters(self, course_ids, fields, exclude): + """Endpoint can be called with all parameters.""" + self.kwarg_test(course_ids=course_ids, fields=fields, exclude=exclude) diff --git a/analyticsclient/tests/test_course_summaries.py b/analyticsclient/tests/test_course_summaries.py index 99ce879..2992df6 100644 --- a/analyticsclient/tests/test_course_summaries.py +++ b/analyticsclient/tests/test_course_summaries.py @@ -1,77 +1,28 @@ +# pylint: disable=arguments-differ import ddt -import httpretty - -from analyticsclient.tests import ClientTestCase +from analyticsclient.tests import ClientTestCase, APIListTestCase @ddt.ddt -class CourseSummariesTests(ClientTestCase): - - def setUp(self): - super(CourseSummariesTests, self).setUp() - self.base_uri = self.get_api_url('course_summaries/') - self.course_summaries_client = self.client.course_summaries() - httpretty.enable() - - def verify_last_querystring_equal(self, expected_query): - """ - Convenience method for asserting the last request was made with the - expected query parameters. - """ - self.assertDictEqual(httpretty.last_request().querystring, expected_query) - - def expected_query(self, course_ids=None, fields=None): - """Packs the query arguments into expected format for http pretty.""" - query = {} - for field, data in zip(['course_ids', 'fields'], [course_ids, fields]): - if data is not None: - query[field] = [','.join(data)] - return query +class CourseSummariesTests(APIListTestCase, ClientTestCase): - @httpretty.activate - def test_all_summaries_url(self): - """Course summaries can be called without parameters.""" - httpretty.register_uri(httpretty.GET, self.base_uri, body='{}') - self.course_summaries_client.course_summaries() - - @httpretty.activate - @ddt.data( - ['edx/demo/course'], - ['edx/demo/course', 'another/demo/course'] - ) - def test_courses_ids(self, course_ids): - """Course summaries can be called with course IDs""" - uri_template = '{uri}?course_ids={ids}' - uri = uri_template.format(uri=self.base_uri, ids=course_ids) - httpretty.register_uri(httpretty.GET, uri, body='{}') - self.course_summaries_client.course_summaries(course_ids=course_ids) - self.verify_last_querystring_equal(self.expected_query(course_ids=course_ids)) + endpoint = 'course_summaries' - @httpretty.activate @ddt.data( - ['course_id'], - ['course_id', 'enrollment_modes'] + ['123'], + ['123', '456'] ) - def test_fields(self, fields): - """Course summaries can be called with fields.""" - uri_template = '{uri}?fields={fields}' - uri = uri_template.format(uri=self.base_uri, fields=fields[0]) - httpretty.register_uri(httpretty.GET, uri, body='{}') - self.course_summaries_client.course_summaries(fields=fields) - self.verify_last_querystring_equal(self.expected_query(fields=fields)) + def test_programs(self, programs): + """Course summaries can be called with programs.""" + self.kwarg_test(programs=programs) - @httpretty.activate @ddt.data( - (['edx/demo/course'], ['course_id']), - (['edx/demo/course', 'another/demo/course'], ['course_id', 'enrollment_modes']) + (['edx/demo/course'], ['course_id'], ['enrollment_modes'], ['123']), + (['edx/demo/course', 'another/demo/course'], ['course_id', 'enrollment_modes'], + ['created', 'pacing_type'], ['123', '456']) ) @ddt.unpack - def test_all_parameters(self, course_ids, fields): - """Course summaries can be called with both fields and course IDs.""" - httpretty.reset() - uri_template = '{uri}?course_ids={ids}fields={fields}' - uri = uri_template.format(uri=self.base_uri, ids=course_ids, fields=fields) - httpretty.register_uri(httpretty.GET, uri, body='{}') - self.course_summaries_client.course_summaries(course_ids=course_ids, fields=fields) - self.verify_last_querystring_equal(self.expected_query(course_ids=course_ids, fields=fields)) + def test_all_parameters(self, course_ids, fields, exclude, programs): + """Course summaries can be called with all parameters including programs.""" + self.kwarg_test(course_ids=course_ids, fields=fields, exclude=exclude, programs=programs) diff --git a/analyticsclient/tests/test_programs.py b/analyticsclient/tests/test_programs.py new file mode 100644 index 0000000..27720a4 --- /dev/null +++ b/analyticsclient/tests/test_programs.py @@ -0,0 +1,9 @@ +import ddt + +from analyticsclient.tests import ClientTestCase, APIListTestCase + + +@ddt.ddt +class ProgramsTests(APIListTestCase, ClientTestCase): + + endpoint = 'programs' From e584a3b0c03d62916431113eb3db5871b6073037 Mon Sep 17 00:00:00 2001 From: Tyler Hallada Date: Thu, 20 Apr 2017 16:17:58 -0400 Subject: [PATCH 2/5] Fix pylint warnings --- .pylintrc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.pylintrc b/.pylintrc index 65c64b0..d8b6a05 100644 --- a/.pylintrc +++ b/.pylintrc @@ -129,7 +129,7 @@ generated-members= [BASIC] # Required attributes for module, separated by a comma -required-attributes= +# required-attributes= # List of builtins function names that should not be used, separated by a comma bad-functions=map,filter,apply,input @@ -274,7 +274,7 @@ max-public-methods=20 # List of interface methods to ignore, separated by a comma. This is used for # instance to not check methods defines in Zope's Interface base class. -ignore-iface-methods=isImplementedBy,deferred,extends,names,namesAndDescriptions,queryDescriptionFor,getBases,getDescriptionFor,getDoc,getName,getTaggedValue,getTaggedValueTags,isEqualOrExtendedBy,setTaggedValue,isImplementedByInstancesOf,adaptWith,is_implemented_by +# ignore-iface-methods=isImplementedBy,deferred,extends,names,namesAndDescriptions,queryDescriptionFor,getBases,getDescriptionFor,getDoc,getName,getTaggedValue,getTaggedValueTags,isEqualOrExtendedBy,setTaggedValue,isImplementedByInstancesOf,adaptWith,is_implemented_by # List of method names used to declare (i.e. assign) instance attributes. defining-attr-methods=__init__,__new__,setUp From af5e84376270b6a01f51d7107db5d05bb960945f Mon Sep 17 00:00:00 2001 From: Tyler Hallada Date: Thu, 20 Apr 2017 16:39:41 -0400 Subject: [PATCH 3/5] Add id_field attribute to fix tests --- analyticsclient/tests/__init__.py | 11 ++++++----- analyticsclient/tests/test_course_summaries.py | 1 + analyticsclient/tests/test_programs.py | 1 + 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/analyticsclient/tests/__init__.py b/analyticsclient/tests/__init__.py index d17dc93..9983ea4 100644 --- a/analyticsclient/tests/__init__.py +++ b/analyticsclient/tests/__init__.py @@ -33,6 +33,7 @@ class APIListTestCase(object): # Override in the subclass: endpoint = 'list' + id_field = 'id' def setUp(self): """Set up the test case.""" @@ -74,9 +75,9 @@ def test_all_items_url(self): ['edx/demo/course'], ['edx/demo/course', 'another/demo/course'] ) - def test_courses_ids(self, course_ids): - """Endpoint can be called with course IDs.""" - self.kwarg_test(course_ids=course_ids) + def test_courses_ids(self, ids): + """Endpoint can be called with IDs.""" + self.kwarg_test(**{self.id_field: ids}) @ddt.data( ['course_id'], @@ -100,6 +101,6 @@ def test_exclude(self, exclude): ['created', 'pacing_type']) ) @ddt.unpack - def test_all_parameters(self, course_ids, fields, exclude): + def test_all_parameters(self, ids, fields, exclude): """Endpoint can be called with all parameters.""" - self.kwarg_test(course_ids=course_ids, fields=fields, exclude=exclude) + self.kwarg_test(**{self.id_field: ids, 'fields': fields, 'exclude': exclude}) diff --git a/analyticsclient/tests/test_course_summaries.py b/analyticsclient/tests/test_course_summaries.py index 2992df6..75d2df9 100644 --- a/analyticsclient/tests/test_course_summaries.py +++ b/analyticsclient/tests/test_course_summaries.py @@ -8,6 +8,7 @@ class CourseSummariesTests(APIListTestCase, ClientTestCase): endpoint = 'course_summaries' + id_field = 'course_ids' @ddt.data( ['123'], diff --git a/analyticsclient/tests/test_programs.py b/analyticsclient/tests/test_programs.py index 27720a4..b699a30 100644 --- a/analyticsclient/tests/test_programs.py +++ b/analyticsclient/tests/test_programs.py @@ -7,3 +7,4 @@ class ProgramsTests(APIListTestCase, ClientTestCase): endpoint = 'programs' + id_field = 'program_ids' From 96c858fbbb2b7b85f3529b37f7b7738489e0edbb Mon Sep 17 00:00:00 2001 From: Tyler Hallada Date: Thu, 20 Apr 2017 16:41:13 -0400 Subject: [PATCH 4/5] Bump version of package to 0.11.0 --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 9741531..e7a92fa 100644 --- a/setup.py +++ b/setup.py @@ -2,7 +2,7 @@ setup( name='edx-analytics-data-api-client', - version='0.10.0', + version='0.11.0', packages=['analyticsclient', 'analyticsclient.constants'], url='https://github.com/edx/edx-analytics-data-api-client', description='Client used to access edX analytics data warehouse', From 6f5ec2278bc6d4fa85a76974fed6c766e30f5c55 Mon Sep 17 00:00:00 2001 From: Tyler Hallada Date: Mon, 24 Apr 2017 11:39:54 -0400 Subject: [PATCH 5/5] Address PR comments: alphabetic import order --- analyticsclient/client.py | 2 +- analyticsclient/tests/test_programs.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/analyticsclient/client.py b/analyticsclient/client.py index 6808847..bd8fec4 100644 --- a/analyticsclient/client.py +++ b/analyticsclient/client.py @@ -6,9 +6,9 @@ from analyticsclient.course import Course from analyticsclient.course_summaries import CourseSummaries -from analyticsclient.programs import Programs from analyticsclient.exceptions import ClientError, InvalidRequestError, NotFoundError, TimeoutError from analyticsclient.module import Module +from analyticsclient.programs import Programs from analyticsclient.status import Status diff --git a/analyticsclient/tests/test_programs.py b/analyticsclient/tests/test_programs.py index b699a30..6e62fa2 100644 --- a/analyticsclient/tests/test_programs.py +++ b/analyticsclient/tests/test_programs.py @@ -1,6 +1,6 @@ import ddt -from analyticsclient.tests import ClientTestCase, APIListTestCase +from analyticsclient.tests import APIListTestCase, ClientTestCase @ddt.ddt