Repository: ucfopen/canvasapi

Issue:
Unable to list_enrollment_terms() on 0.8.2
It seems Enrollment Terms in the Canvas API aren't structured like most other Paginated Lists. Specifically, in the example response at https://canvas.instructure.com/doc/api/enrollment_terms.html#method.terms_api.index (and my own testing), the list of term objects are nested under a top-level string key `enrollment_terms` in the JSON response. It seems like other Paginated Lists return a JSON payload where the list object of requested items is the top-level element. This difference seems to cause `paginated_list.py` to fail to parse the Enrollment Terms response in the loop over the JSON payload at https://github.com/ucfopen/canvasapi/blob/v0.8.2/canvasapi/paginated_list.py#L78-L81, since on the first iteration `element` will be the string `enrollment_terms`, which does not have an update() member function.

I'm sorry if I missed something obvious that lets me specify what I want the "root element" of the payload to be when parsing a Paginated List. I poked around a little bit but didn't see any way to mess with the data variable in _get_next_page().

Discussion:

Patch:
diff --git a/canvasapi/account.py b/canvasapi/account.py
index 63f2ca2..3a03734 100644
--- a/canvasapi/account.py
+++ b/canvasapi/account.py
@@ -663,6 +663,7 @@ class Account(CanvasObject):
             'GET',
             'accounts/{}/terms'.format(self.id),
             {'account_id': self.id},
+            _root='enrollment_terms',
             _kwargs=combine_kwargs(**kwargs)
         )
 
diff --git a/canvasapi/paginated_list.py b/canvasapi/paginated_list.py
index 8717902..56732fb 100644
--- a/canvasapi/paginated_list.py
+++ b/canvasapi/paginated_list.py
@@ -10,30 +10,31 @@ class PaginatedList(object):
 
     def __init__(
         self, content_class, requester, request_method, first_url, extra_attribs=None,
-            **kwargs):
+            _root=None, **kwargs):
 
-        self.__elements = list()
+        self._elements = list()
 
-        self.__requester = requester
-        self.__content_class = content_class
-        self.__first_url = first_url
-        self.__first_params = kwargs or {}
-        self.__first_params['per_page'] = kwargs.get('per_page', 100)
-        self.__next_url = first_url
-        self.__next_params = self.__first_params
-        self.__extra_attribs = extra_attribs or {}
-        self.__request_method = request_method
+        self._requester = requester
+        self._content_class = content_class
+        self._first_url = first_url
+        self._first_params = kwargs or {}
+        self._first_params['per_page'] = kwargs.get('per_page', 100)
+        self._next_url = first_url
+        self._next_params = self._first_params
+        self._extra_attribs = extra_attribs or {}
+        self._request_method = request_method
+        self._root = _root
 
     def __getitem__(self, index):
         assert isinstance(index, (int, slice))
         if isinstance(index, int):
-            self.__get_up_to_index(index)
-            return self.__elements[index]
+            self._get_up_to_index(index)
+            return self._elements[index]
         else:
             return self._Slice(self, index)
 
     def __iter__(self):
-        for element in self.__elements:
+        for element in self._elements:
             yield element
         while self._has_next():
             new_elements = self._grow()
@@ -41,60 +42,68 @@ class PaginatedList(object):
                 yield element
 
     def __repr__(self):
-        return "<PaginatedList of type {}>".format(self.__content_class.__name__)
+        return "<PaginatedList of type {}>".format(self._content_class.__name__)
 
     def _is_larger_than(self, index):
-        return len(self.__elements) > index or self._has_next()
+        return len(self._elements) > index or self._has_next()
 
-    def __get_up_to_index(self, index):
-        while len(self.__elements) <= index and self._has_next():
+    def _get_up_to_index(self, index):
+        while len(self._elements) <= index and self._has_next():
             self._grow()
 
     def _grow(self):
         new_elements = self._get_next_page()
-        self.__elements += new_elements
+        self._elements += new_elements
         return new_elements
 
     def _has_next(self):
-        return self.__next_url is not None
+        return self._next_url is not None
 
     def _get_next_page(self):
-        response = self.__requester.request(
-            self.__request_method,
-            self.__next_url,
-            **self.__next_params
+        response = self._requester.request(
+            self._request_method,
+            self._next_url,
+            **self._next_params
         )
         data = response.json()
-        self.__next_url = None
+        self._next_url = None
 
         next_link = response.links.get('next')
-        regex = r'{}(.*)'.format(re.escape(self.__requester.base_url))
+        regex = r'{}(.*)'.format(re.escape(self._requester.base_url))
 
-        self.__next_url = re.search(regex, next_link['url']).group(1) if next_link else None
+        self._next_url = re.search(regex, next_link['url']).group(1) if next_link else None
 
-        self.__next_params = {}
+        self._next_params = {}
 
         content = []
+
+        if self._root:
+            try:
+                data = data[self._root]
+            except KeyError:
+                # TODO: Fix this message to make more sense to an end user.
+                raise ValueError("Invalid root value specified.")
+
         for element in data:
             if element is not None:
-                element.update(self.__extra_attribs)
-                content.append(self.__content_class(self.__requester, element))
+                element.update(self._extra_attribs)
+                content.append(self._content_class(self._requester, element))
 
         return content
 
     class _Slice(object):
         def __init__(self, the_list, the_slice):
-            self.__list = the_list
-            self.__start = the_slice.start or 0
-            self.__stop = the_slice.stop
-            self.__step = the_slice.step or 1
+            self._list = the_list
+            self._start = the_slice.start or 0
+            self._stop = the_slice.stop
+            self._step = the_slice.step or 1
 
         def __iter__(self):
-            index = self.__start
-            while not self.__finished(index):
-                if self.__list._is_larger_than(index):
-                    yield self.__list[index]
-                    index += self.__step
-
-        def __finished(self, index):
-            return self.__stop is not None and index >= self.__stop
+            index = self._start
+            while not self._finished(index):
+                if self._list._is_larger_than(index):
+                    yield self._list[index]
+                    index += self._step
+
+        def _finished(self, index):
+            return self._stop is not None and index >= self._stop


Test patch:
diff --git a/tests/fixtures/account.json b/tests/fixtures/account.json
index bf80693..e12a3e6 100644
--- a/tests/fixtures/account.json
+++ b/tests/fixtures/account.json
@@ -291,7 +291,8 @@
 	"list_enrollment_terms": {
 		"method": "GET",
 		"endpoint": "accounts/1/terms",
-		"data": [
+		"data": {
+			"enrollment_terms": [
 			{
 				"id": 1,
 				"name": "Enrollment Term 1"
@@ -299,8 +300,8 @@
 			{
 				"id": 2,
 				"name": "Enrollment Term 2"
-			}
-		],
+			}]
+		},
 		"status_code": 200
 	},
 	"list_groups_context": {
diff --git a/tests/test_paginated_list.py b/tests/test_paginated_list.py
index b7c2a1d..8fd6521 100644
--- a/tests/test_paginated_list.py
+++ b/tests/test_paginated_list.py
@@ -4,6 +4,7 @@ import unittest
 import requests_mock
 
 from canvasapi import Canvas
+from canvasapi.enrollment_term import EnrollmentTerm
 from canvasapi.paginated_list import PaginatedList
 from canvasapi.user import User
 from tests import settings
@@ -205,3 +206,30 @@ class TestPaginatedList(unittest.TestCase):
             'six_objects_three_pages'
         )
         self.assertEqual(pag_list.__repr__(), '<PaginatedList of type User>')
+
+    def test_root_element_incorrect(self, m):
+        register_uris({'account': ['list_enrollment_terms']}, m)
+
+        pag_list = PaginatedList(
+            EnrollmentTerm,
+            self.requester,
+            'GET',
+            'accounts/1/terms',
+            _root='wrong'
+        )
+
+        with self.assertRaises(ValueError):
+            pag_list[0]
+
+    def test_root_element(self, m):
+        register_uris({'account': ['list_enrollment_terms']}, m)
+
+        pag_list = PaginatedList(
+            EnrollmentTerm,
+            self.requester,
+            'GET',
+            'accounts/1/terms',
+            _root='enrollment_terms'
+        )
+
+        self.assertIsInstance(pag_list[0], EnrollmentTerm)