Added weight validations and test cases, split long length test into …

…sub funtions

edx_sga/sga.py

@@ -316,7 +316,6 @@ def none_to_empty(x):
     @XBlock.json_handler
     def save_sga(self, data, suffix=''):
         self.display_name = data.get('display_name', self.display_name)
-        self.weight = data.get('weight', self.weight)
 
         # Validate points before saving
         points = data.get('points', self.points)
@@ -330,6 +329,21 @@ def save_sga(self, data, suffix=''):
             raise JsonHandlerError(400, 'Points must be a positive integer')
         self.points = points
 
+        # Validate weight before saving
+        weight = data.get('weight', self.weight)
+        # Check that weight is a float.
+        if weight:
+            try:
+                weight = float(weight)
+            except ValueError:
+                raise JsonHandlerError(400, 'Weight must be a decimal number')
+            # Check that we are positive
+            if weight < 0:
+                raise JsonHandlerError(
+                    400, 'Weight must be a positive decimal number'
+                )
+        self.weight = weight
+
     @XBlock.handler
     def upload_assignment(self, request, suffix=''):
         require(self.upload_allowed())

edx_sga/tests.py

@@ -245,6 +245,38 @@ def test_studio_view(self, Fragment, render_template):
             "StaffGradedAssignmentXBlock")
 
     def test_save_sga(self):
+        def weights_positive_float_test():
+            orig_weight = 11.0
+
+            # Test negative weight doesn't work
+            block.save_sga(mock.Mock(method="POST", body=json.dumps({
+                "display_name": "Test Block",
+                "points": '100',
+                "weight": -10.0})))
+            self.assertEqual(block.weight, orig_weight)
+
+            # Test string weight doesn't work
+            block.save_sga(mock.Mock(method="POST", body=json.dumps({
+                "display_name": "Test Block",
+                "points": '100',
+                "weight": "a"})))
+            self.assertEqual(block.weight, orig_weight)
+
+        def point_positive_int_test():
+            # Test negative doesn't work
+            block.save_sga(mock.Mock(method="POST", body=json.dumps({
+                "display_name": "Test Block",
+                "points": '-10',
+                "weight": 11})))
+            self.assertEqual(block.points, orig_score)
+
+            # Test float doesn't work
+            block.save_sga(mock.Mock(method="POST", body=json.dumps({
+                "display_name": "Test Block",
+                "points": '24.5',
+                "weight": 11})))
+            self.assertEqual(block.points, orig_score)
+
         orig_score = 23
         block = self.make_one()
         block.save_sga(mock.Mock(body='{}'))
@@ -259,19 +291,8 @@ def test_save_sga(self):
         self.assertEqual(block.points, orig_score)
         self.assertEqual(block.weight, 11)
 
-        # Test negative doesn't work
-        block.save_sga(mock.Mock(method="POST", body=json.dumps({
-            "display_name": "Test Block",
-            "points": '-10',
-            "weight": 11})))
-        self.assertEqual(block.points, orig_score)
-
-        # Test float doesn't work
-        block.save_sga(mock.Mock(method="POST", body=json.dumps({
-            "display_name": "Test Block",
-            "points": '24.5',
-            "weight": 11})))
-        self.assertEqual(block.points, orig_score)
+        point_positive_int_test()
+        weights_positive_float_test()
 
     def test_upload_download_assignment(self):
         path = pkg_resources.resource_filename(__package__, 'tests.py')