Repository: ucfopen/canvasapi

Issue:
Grouped Responses
The `list_multiple_submissions` methods on `Course` and `Section` objects takes in an optional parameter `grouped` (See [Canvas Docs](https://canvas.instructure.com/doc/api/submissions.html#method.submissions_api.for_students)). If `grouped` is true (note: see #61 for details on boolean weirdness for this particular method), the format Canvas returns is different than what PaginatedList normally expects.

To get around this, we prevent the `grouped` parameter from being passed at all. Ideally, we should see if it's possible to parse this other format and leverage it's benefit of being grouped by user.

Similar issues may exist in other endpoints. It's possible that support for different formats is related to #56.

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index b653134..17c3e42 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -36,6 +36,7 @@
 - Fixed an issue where creating an external tool did not properly send parameters to Canvas. (Thanks, [@altgilbers](https://github.com/altgilbers))
 - Fixed an issue where getting Quiz Submissions would only return up to the first 10 results (Thanks,[@Mike-Nahmias](https://github.com/Mike-Nahmias))
 - Fixed an issue where unhandled 4XX and 5XX HTTP errors would cause a JSONDecodeError
+- Removed a limitation where the parameter `grouped` being passed to `get_multiple_submissions` would be ignored. These methods now return a `GroupedSubmission` object containing multiple `Submission` objects, instead of ignoring. (Thanks, [@bennettscience](https://github.com/bennettscience))
 
 ## [0.12.0] - 2019-04-03
 
diff --git a/canvasapi/course.py b/canvasapi/course.py
index d87df1a..ec1996f 100644
--- a/canvasapi/course.py
+++ b/canvasapi/course.py
@@ -18,9 +18,11 @@ from canvasapi.paginated_list import PaginatedList
 from canvasapi.progress import Progress
 from canvasapi.quiz import QuizExtension
 from canvasapi.tab import Tab
-from canvasapi.submission import Submission
+from canvasapi.submission import GroupedSubmission, Submission
 from canvasapi.upload import Uploader
-from canvasapi.util import combine_kwargs, is_multivalued, file_or_path, obj_or_id
+from canvasapi.util import (
+    combine_kwargs, is_multivalued, file_or_path, obj_or_id, normalize_bool
+)
 from canvasapi.rubric import Rubric
 
 
@@ -1410,17 +1412,21 @@ class Course(CanvasObject):
         :rtype: :class:`canvasapi.paginated_list.PaginatedList` of
             :class:`canvasapi.submission.Submission`
         """
-        if 'grouped' in kwargs:
-            warnings.warn('The `grouped` parameter must be empty. Removing kwarg `grouped`.')
-            del kwargs['grouped']
+
+        is_grouped = kwargs.get("grouped", False)
+
+        if normalize_bool(is_grouped, "grouped"):
+            cls = GroupedSubmission
+        else:
+            cls = Submission
 
         return PaginatedList(
-            Submission,
+            cls,
             self._requester,
             'GET',
             'courses/{}/students/submissions'.format(self.id),
             {'course_id': self.id},
-            _kwargs=combine_kwargs(**kwargs)
+            _kwargs=combine_kwargs(**kwargs),
         )
 
     def get_submission(self, assignment, user, **kwargs):
diff --git a/canvasapi/section.py b/canvasapi/section.py
index d76be75..5486937 100644
--- a/canvasapi/section.py
+++ b/canvasapi/section.py
@@ -6,8 +6,8 @@ from six import python_2_unicode_compatible
 from canvasapi.canvas_object import CanvasObject
 from canvasapi.paginated_list import PaginatedList
 from canvasapi.progress import Progress
-from canvasapi.submission import Submission
-from canvasapi.util import combine_kwargs, obj_or_id
+from canvasapi.submission import GroupedSubmission, Submission
+from canvasapi.util import combine_kwargs, obj_or_id, normalize_bool
 
 warnings.simplefilter('always', DeprecationWarning)
 
@@ -242,17 +242,20 @@ class Section(CanvasObject):
         :rtype: :class:`canvasapi.paginated_list.PaginatedList` of
             :class:`canvasapi.submission.Submission`
         """
-        if 'grouped' in kwargs:
-            warnings.warn('The `grouped` parameter must be empty. Removing kwarg `grouped`.')
-            del kwargs['grouped']
+        is_grouped = kwargs.get("grouped", False)
+
+        if normalize_bool(is_grouped, "grouped"):
+            cls = GroupedSubmission
+        else:
+            cls = Submission
 
         return PaginatedList(
-            Submission,
+            cls,
             self._requester,
             'GET',
             'sections/{}/students/submissions'.format(self.id),
             {'section_id': self.id},
-            _kwargs=combine_kwargs(**kwargs)
+            _kwargs=combine_kwargs(**kwargs),
         )
 
     def get_submission(self, assignment, user, **kwargs):
diff --git a/canvasapi/submission.py b/canvasapi/submission.py
index b1816f3..d36be90 100644
--- a/canvasapi/submission.py
+++ b/canvasapi/submission.py
@@ -188,3 +188,23 @@ class Submission(CanvasObject):
                 }
             )
         return response
+
+
+@python_2_unicode_compatible
+class GroupedSubmission(CanvasObject):
+    def __str__(self):
+        return "{} submission(s) for User #{}".format(
+            len(self.submissions), self.user_id
+        )
+
+    def __init__(self, requester, attributes):
+        try:
+            self.submissions = [
+                Submission(requester, submission)
+                for submission in attributes['submissions']
+            ]
+            del attributes['submissions']
+        except KeyError:
+            self.submissions = list()
+
+        super(GroupedSubmission, self).__init__(requester, attributes)
diff --git a/canvasapi/util.py b/canvasapi/util.py
index 351f2fb..4b71275 100644
--- a/canvasapi/util.py
+++ b/canvasapi/util.py
@@ -128,7 +128,9 @@ def obj_or_id(parameter, param_name, object_types):
                     break
 
         obj_type_list = ",".join([obj_type.__name__ for obj_type in object_types])
-        message = 'Parameter {} must be of type {} or int.'.format(param_name, obj_type_list)
+        message = 'Parameter {} must be of type {} or int.'.format(
+            param_name, obj_type_list
+        )
         raise TypeError(message)
 
 
@@ -168,3 +170,29 @@ def file_or_path(file):
         is_path = True
 
     return file, is_path
+
+
+def normalize_bool(val, param_name):
+    """
+    Normalize boolean-like strings to their corresponding boolean values.
+
+    :param val: Value to normalize. Acceptable values:
+        True, "True", "true", False, "False", "false"
+    :type val: str or bool
+    :param param_name: Name of the parameter being checked
+    :type param_name: str
+
+    :rtype: bool
+    """
+    if isinstance(val, bool):
+        return val
+    elif val in ("True", "true"):
+        return True
+    elif val in ("False", "false"):
+        return False
+    else:
+        raise ValueError(
+            'Parameter `{}` must be True, "True", "true", False, "False", or "false".'.format(
+                param_name
+            )
+        )


Test patch:
diff --git a/tests/fixtures/course.json b/tests/fixtures/course.json
index 1b9b787..8b0df0d 100644
--- a/tests/fixtures/course.json
+++ b/tests/fixtures/course.json
@@ -1431,6 +1431,37 @@
 		],
 		"status_code": 200
 	},
+	"list_multiple_submissions_grouped": {
+		"method": "GET",
+		"endpoint": "courses/1/students/submissions",
+		"data": [
+			{
+				"user_id": 1,
+				"submissions": [
+					{
+						"id": 1,
+						"assignments_id": 1,
+						"user_id": 1,
+						"html_url": "https://example.com/courses/1/assignments/1/submissions/1",
+						"submission_type": "online_upload"
+					}
+				]
+			},
+			{
+				"user_id": 2,
+				"submissions": [
+					{
+						"id": 2,
+						"assignments_id": 1,
+						"user_id": 2,
+						"html_url": "https://example.com/courses/1/assignments/1/submissions/2",
+						"submission_type": "online_upload"
+					}
+				]
+			}
+		],
+		"status_code": 200
+	},
 	"list_gradeable_students": {
 		"method": "GET",
 		"endpoint": "courses/1/assignments/1/gradeable_students",
diff --git a/tests/fixtures/section.json b/tests/fixtures/section.json
index 5c7df15..a4eb936 100644
--- a/tests/fixtures/section.json
+++ b/tests/fixtures/section.json
@@ -148,6 +148,37 @@
 		],
 		"status_code": 200
 	},
+	"list_multiple_submissions_grouped": {
+		"method": "GET",
+		"endpoint": "sections/1/students/submissions",
+		"data": [
+			{
+				"user_id": 1,
+				"submissions": [
+					{
+						"id": 1,
+						"assignments_id": 1,
+						"user_id": 1,
+						"html_url": "https://example.com/sections/1/assignments/1/submissions/1",
+						"submission_type": "online_upload"
+					}
+				]
+			},
+			{
+				"user_id": 2,
+				"submissions": [
+					{
+						"id": 2,
+						"assignments_id": 1,
+						"user_id": 2,
+						"html_url": "https://example.com/sections/1/assignments/1/submissions/2",
+						"submission_type": "online_upload"
+					}
+				]
+			}
+		],
+		"status_code": 200
+	},
 	"get_submission": {
 		"method": "GET",
 		"endpoint": "sections/1/assignments/1/submissions/1",
diff --git a/tests/test_course.py b/tests/test_course.py
index 70fb8df..6263c3a 100644
--- a/tests/test_course.py
+++ b/tests/test_course.py
@@ -32,7 +32,7 @@ from canvasapi.progress import Progress
 from canvasapi.quiz import Quiz, QuizExtension
 from canvasapi.rubric import Rubric
 from canvasapi.section import Section
-from canvasapi.submission import Submission
+from canvasapi.submission import GroupedSubmission, Submission
 from canvasapi.tab import Tab
 from canvasapi.user import User
 from canvasapi.user import UserDisplay
@@ -917,24 +917,29 @@ class TestCourse(unittest.TestCase):
         self.assertEqual(len(submission_list), 2)
         self.assertIsInstance(submission_list[0], Submission)
 
-    def test_get_multiple_submissions_grouped_param(self, m):
+    def test_get_multiple_submissions_grouped_true(self, m):
+        register_uris({'course': ['list_multiple_submissions_grouped']}, m)
+
+        submissions = self.course.get_multiple_submissions(grouped=True)
+        submission_list = [submission for submission in submissions]
+
+        self.assertEqual(len(submission_list), 2)
+        self.assertIsInstance(submission_list[0], GroupedSubmission)
+
+    def test_get_multiple_submissions_grouped_false(self, m):
         register_uris({'course': ['list_multiple_submissions']}, m)
 
-        with warnings.catch_warnings(record=True) as warning_list:
-            warnings.simplefilter('always')
-            submissions = self.course.get_multiple_submissions(grouped=True)
-            submission_list = [submission for submission in submissions]
+        submissions = self.course.get_multiple_submissions(grouped=False)
+        submission_list = [submission for submission in submissions]
 
-            # Ensure using the `grouped` param raises a warning
-            self.assertEqual(len(warning_list), 1)
-            self.assertEqual(warning_list[-1].category, UserWarning)
-            self.assertEqual(
-                text_type(warning_list[-1].message),
-                'The `grouped` parameter must be empty. Removing kwarg `grouped`.'
-            )
+        self.assertEqual(len(submission_list), 2)
+        self.assertIsInstance(submission_list[0], Submission)
 
-            self.assertEqual(len(submission_list), 2)
-            self.assertIsInstance(submission_list[0], Submission)
+    def test_get_multiple_submissions_grouped_invalid(self, m):
+        with self.assertRaises(ValueError) as cm:
+            self.course.get_multiple_submissions(grouped='blargh')
+
+        self.assertIn("Parameter `grouped` must", cm.exception.args[0])
 
     # get_submission()
     def test_get_submission(self, m):
diff --git a/tests/test_section.py b/tests/test_section.py
index ca01c1c..e7dfdfa 100644
--- a/tests/test_section.py
+++ b/tests/test_section.py
@@ -3,7 +3,6 @@ import unittest
 import warnings
 
 import requests_mock
-from six import text_type
 
 from canvasapi import Canvas
 from canvasapi.assignment import AssignmentOverride
@@ -11,7 +10,7 @@ from canvasapi.enrollment import Enrollment
 from canvasapi.exceptions import RequiredFieldMissing
 from canvasapi.progress import Progress
 from canvasapi.section import Section
-from canvasapi.submission import Submission
+from canvasapi.submission import GroupedSubmission, Submission
 from tests import settings
 from tests.util import register_uris
 
@@ -184,24 +183,29 @@ class TestSection(unittest.TestCase):
         self.assertEqual(len(submission_list), 2)
         self.assertIsInstance(submission_list[0], Submission)
 
-    def test_get_multiple_submissions_grouped_param(self, m):
+    def test_get_multiple_submissions_grouped_true(self, m):
+        register_uris({'section': ['list_multiple_submissions_grouped']}, m)
+
+        submissions = self.section.get_multiple_submissions(grouped=True)
+        submission_list = [submission for submission in submissions]
+
+        self.assertEqual(len(submission_list), 2)
+        self.assertIsInstance(submission_list[0], GroupedSubmission)
+
+    def test_get_multiple_submissions_grouped_false(self, m):
         register_uris({'section': ['list_multiple_submissions']}, m)
 
-        with warnings.catch_warnings(record=True) as warning_list:
-            warnings.simplefilter('always')
-            submissions = self.section.get_multiple_submissions(grouped=True)
-            submission_list = [submission for submission in submissions]
+        submissions = self.section.get_multiple_submissions(grouped=False)
+        submission_list = [submission for submission in submissions]
 
-            # Ensure using the `grouped` param raises a warning
-            self.assertEqual(len(warning_list), 1)
-            self.assertEqual(warning_list[-1].category, UserWarning)
-            self.assertEqual(
-                text_type(warning_list[-1].message),
-                'The `grouped` parameter must be empty. Removing kwarg `grouped`.'
-            )
+        self.assertEqual(len(submission_list), 2)
+        self.assertIsInstance(submission_list[0], Submission)
 
-            self.assertEqual(len(submission_list), 2)
-            self.assertIsInstance(submission_list[0], Submission)
+    def test_get_multiple_submissions_grouped_invalid(self, m):
+        with self.assertRaises(ValueError) as cm:
+            self.section.get_multiple_submissions(grouped='blargh')
+
+        self.assertIn("Parameter `grouped` must", cm.exception.args[0])
 
     # get_submission()
     def test_get_submission(self, m):
diff --git a/tests/test_submission.py b/tests/test_submission.py
index ae00970..f3928b1 100644
--- a/tests/test_submission.py
+++ b/tests/test_submission.py
@@ -7,23 +7,25 @@ import requests_mock
 
 from canvasapi import Canvas
 from canvasapi.peer_review import PeerReview
-from canvasapi.submission import Submission
+from canvasapi.submission import GroupedSubmission, Submission
 from tests import settings
 from tests.util import cleanup_file, register_uris
 
 
 @requests_mock.Mocker()
 class TestSubmission(unittest.TestCase):
-
     def setUp(self):
         self.canvas = Canvas(settings.BASE_URL, settings.API_KEY)
 
         with requests_mock.Mocker() as m:
-            register_uris({
-                'course': ['get_by_id', 'get_assignment_by_id'],
-                'section': ['get_by_id'],
-                'submission': ['get_by_id_course', 'get_by_id_section']
-            }, m)
+            register_uris(
+                {
+                    'course': ['get_by_id', 'get_assignment_by_id'],
+                    'section': ['get_by_id'],
+                    'submission': ['get_by_id_course', 'get_by_id_section'],
+                },
+                m,
+            )
 
             with warnings.catch_warnings(record=True) as warning_list:
                 self.course = self.canvas.get_course(1)
@@ -43,9 +45,7 @@ class TestSubmission(unittest.TestCase):
 
     # create_submission_peer_review()
     def test_create_submission_peer_review(self, m):
-        register_uris({'submission': [
-            'create_submission_peer_review',
-        ]}, m)
+        register_uris({'submission': ['create_submission_peer_review']}, m)
 
         created_peer_review = self.submission_course.create_submission_peer_review(1)
 
@@ -54,9 +54,7 @@ class TestSubmission(unittest.TestCase):
 
     # delete_submission_peer_review()
     def test_delete_submission_peer_review(self, m):
-        register_uris({'submission': [
-            'delete_submission_peer_review',
-        ]}, m)
+        register_uris({'submission': ['delete_submission_peer_review']}, m)
 
         deleted_peer_review = self.submission_course.delete_submission_peer_review(1)
 
@@ -65,9 +63,7 @@ class TestSubmission(unittest.TestCase):
 
     # edit()
     def test_edit(self, m):
-        register_uris({
-            'submission': ['edit']
-        }, m)
+        register_uris({'submission': ['edit']}, m)
 
         self.assertFalse(hasattr(self.submission_course, 'excused'))
 
@@ -79,23 +75,21 @@ class TestSubmission(unittest.TestCase):
 
     # get_submission_peer_reviews()
     def test_get_submission_peer_reviews(self, m):
-        register_uris({'submission': [
-            'list_submission_peer_reviews',
-        ]}, m)
+        register_uris({'submission': ['list_submission_peer_reviews']}, m)
 
         submission_peer_reviews = self.submission_course.get_submission_peer_reviews()
-        submission_peer_review_list = [peer_review for peer_review in submission_peer_reviews]
+        submission_peer_review_list = [
+            peer_review for peer_review in submission_peer_reviews
+        ]
 
         self.assertEqual(len(submission_peer_review_list), 2)
         self.assertIsInstance(submission_peer_review_list[0], PeerReview)
 
     # upload_comment()
     def test_upload_comment(self, m):
-        register_uris({'submission': [
-            'upload_comment',
-            'upload_comment_final',
-            'edit',
-        ]}, m)
+        register_uris(
+            {'submission': ['upload_comment', 'upload_comment_final', 'edit']}, m
+        )
 
         filename = 'testfile_submission_{}'.format(uuid.uuid4().hex)
 
@@ -110,11 +104,9 @@ class TestSubmission(unittest.TestCase):
             cleanup_file(filename)
 
     def test_upload_comment_section(self, m):
-        register_uris({'submission': [
-            'upload_comment',
-            'upload_comment_final',
-            'edit',
-        ]}, m)
+        register_uris(
+            {'submission': ['upload_comment', 'upload_comment_final', 'edit']}, m
+        )
 
         filename = 'testfile_submission_{}'.format(uuid.uuid4().hex)
 
@@ -127,3 +119,41 @@ class TestSubmission(unittest.TestCase):
             self.assertIn('url', response[1])
         finally:
             cleanup_file(filename)
+
+
+class TestGroupedSubmission(unittest.TestCase):
+    def setUp(self):
+        self.canvas = Canvas(settings.BASE_URL, settings.API_KEY)
+
+        self.grouped_submission = GroupedSubmission(
+            self.canvas._Canvas__requester,
+            {
+                'user_id': 1,
+                'submissions': [
+                    {
+                        'id': 1,
+                        'assignment_id': 1,
+                        'user_id': 1,
+                        "html_url": "https://example.com/courses/1/assignments/1/submissions/1",
+                        "submission_type": "online_upload",
+                    }
+                ],
+            },
+        )
+
+    # __init__()
+    def test__init__no_submission_key(self):
+        grouped_submission = GroupedSubmission(
+            self.canvas._Canvas__requester, {'user_id': 1}
+        )
+
+        self.assertIsInstance(grouped_submission, GroupedSubmission)
+        self.assertTrue(hasattr(grouped_submission, 'submissions'))
+        self.assertIsInstance(grouped_submission.submissions, list)
+        self.assertEqual(len(grouped_submission.submissions), 0)
+
+    # __str__()
+    def test__str__(self):
+        string = str(self.grouped_submission)
+        self.assertIsInstance(string, str)
+        self.assertEqual(string, '1 submission(s) for User #1')
diff --git a/tests/test_util.py b/tests/test_util.py
index d52eb8e..2c7ffda 100644
--- a/tests/test_util.py
+++ b/tests/test_util.py
@@ -8,7 +8,12 @@ from canvasapi import Canvas
 from canvasapi.course import CourseNickname
 from canvasapi.user import User
 from canvasapi.util import (
-    combine_kwargs, get_institution_url, is_multivalued, obj_or_id, file_or_path
+    combine_kwargs,
+    get_institution_url,
+    is_multivalued,
+    obj_or_id,
+    file_or_path,
+    normalize_bool,
 )
 from itertools import chain
 from six import integer_types, iterkeys, itervalues, iteritems, text_type
@@ -19,7 +24,6 @@ from tests.util import cleanup_file, register_uris
 
 @requests_mock.Mocker()
 class TestUtil(unittest.TestCase):
-
     def setUp(self):
         self.canvas = Canvas(settings.BASE_URL, settings.API_KEY)
 
@@ -79,6 +83,7 @@ class TestUtil(unittest.TestCase):
     def test_is_multivalued_generator_call(self, m):
         def yielder():
             yield 'item'
+
         self.assertTrue(is_multivalued(yielder()))
 
     def test_is_multivalued_chain(self, m):
@@ -536,3 +541,22 @@ class TestUtil(unittest.TestCase):
 
         with self.assertRaises(IOError):
             handler, is_path = file_or_path(filename)
+
+    # normalize_bool()
+    def test_normalize_bool_boolean(self, m):
+        self.assertTrue(normalize_bool(True, "value"))
+        self.assertFalse(normalize_bool(False, "value"))
+
+    def test_normalize_bool_str_lower(self, m):
+        self.assertTrue(normalize_bool("true", "value"))
+        self.assertFalse(normalize_bool("false", "value"))
+
+    def test_normalize_bool_str_upper(self, m):
+        self.assertTrue(normalize_bool("True", "value"))
+        self.assertFalse(normalize_bool("False", "value"))
+
+    def test_normalize_bool_str_invalid(self, m):
+        with self.assertRaises(ValueError) as cm:
+            normalize_bool("invalid", "value")
+
+        self.assertIn("Parameter `value` must", cm.exception.args[0])