Resolving quality issues

lti_consumer/lti_1p1/consumer.py

@@ -68,10 +68,10 @@ def parse_result_json(json_str):
     """
     try:
         json_obj = json.loads(json_str)
-    except (ValueError, TypeError):
+    except (ValueError, TypeError) as error:
         msg = f"Supplied JSON string in request body could not be decoded: {json_str}"
         log.error("[LTI] %s", msg)
-        raise Lti1p1Error(msg)
+        raise Lti1p1Error(msg) from error
 
     # The JSON object must be a dict. If a non-empty list is passed in,
     # use the first element, but only if it is a dict
@@ -112,7 +112,7 @@ def parse_result_json(json_str):
         except (TypeError, ValueError) as err:
             msg = "Could not convert resultScore to float: {}".format(str(err))
             log.error("[LTI] %s", msg)
-            raise Lti1p1Error(msg)
+            raise Lti1p1Error(msg) from err
 
     return score, json_obj.get('comment', "")
 
@@ -387,4 +387,4 @@ def verify_result_headers(self, request, verify_content_type=True):
             return verify_oauth_body_signature(request, self.oauth_secret, outcome_service_url)
         except (ValueError, Lti1p1Error) as err:
             log.error("[LTI]: v2.0 result service -- OAuth body verification failed: %s", str(err))
-            raise Lti1p1Error(str(err))
+            raise Lti1p1Error(str(err)) from err

lti_consumer/lti_1p1/contrib/tests/test_django.py

@@ -2,8 +2,8 @@
 Unit tests for lti_consumer.lti module
 """
 
-from django.test.testcases import TestCase
 from unittest.mock import Mock, patch, ANY
+from django.test.testcases import TestCase
 
 from lti_consumer.lti_1p1.contrib.django import lti_embed
 

lti_consumer/lti_1p1/oauth.py

@@ -56,8 +56,8 @@ def get_oauth_request_signature(key, secret, url, headers, body):
             body=body,
             headers=headers
         )
-    except ValueError:  # Scheme not in url.
-        raise Lti1p1Error("Failed to sign oauth request")
+    except ValueError as err:  # Scheme not in url.
+        raise Lti1p1Error("Failed to sign oauth request") from err
 
     return headers['Authorization']
 

lti_consumer/lti_1p3/consumer.py

@@ -404,8 +404,8 @@ def _validate_preflight_response(self, response):
             assert response.get("state")
             assert response.get("client_id") == self.client_id
             assert response.get("redirect_uri") == self.launch_url
-        except AssertionError:
-            raise exceptions.PreflightRequestValidationFailure()
+        except AssertionError as err:
+            raise exceptions.PreflightRequestValidationFailure() from err
 
     def check_token(self, token, allowed_scopes=None):
         """

lti_consumer/lti_1p3/extensions/rest_framework/authentication.py

@@ -52,18 +52,18 @@ def authenticate(self, request):
         try:
             lti_configuration = LtiConfiguration.objects.get(pk=lti_config_id)
             lti_consumer = lti_configuration.get_lti_consumer()
-        except Exception:
+        except Exception as err:
             msg = _('LTI configuration not found.')
-            raise exceptions.AuthenticationFailed(msg)
+            raise exceptions.AuthenticationFailed(msg) from err
 
         # Verify token validity
         # This doesn't validate specific permissions, just checks if the token
         # is valid or not.
         try:
             lti_consumer.check_token(auth[1])
-        except Exception:
+        except Exception as err:
             msg = _('Invalid token signature.')
-            raise exceptions.AuthenticationFailed(msg)
+            raise exceptions.AuthenticationFailed(msg) from err
 
         # Passing parameters back to the view through the request in order
         # to avoid implementing a separate authentication backend or

lti_consumer/lti_1p3/extensions/rest_framework/serializers.py

@@ -31,8 +31,8 @@ def to_internal_value(self, data):
         """
         try:
             return UsageKey.from_string(data)
-        except InvalidKeyError:
-            raise serializers.ValidationError(f"Invalid usage key: {data}")
+        except InvalidKeyError as err:
+            raise serializers.ValidationError(f"Invalid usage key: {data}") from err
 
 
 class LtiAgsLineItemSerializer(serializers.ModelSerializer):

lti_consumer/lti_1p3/key_handlers.py

@@ -55,8 +55,8 @@ def __init__(self, public_key=None, keyset_url=None):
                 # Import Key and save to internal state
                 new_key.load_key(RSA.import_key(raw_key))
                 self.public_key = new_key
-            except ValueError:
-                raise exceptions.InvalidRsaKey()
+            except ValueError as err:
+                raise exceptions.InvalidRsaKey() from err
 
     def _get_keyset(self, kid=None):
         """
@@ -113,12 +113,12 @@ def validate_and_decode(self, token):
             # Else returns decoded message
             return message
 
-        except NoSuitableSigningKeys:
-            raise exceptions.NoSuitableKeys()
-        except BadSyntax:
-            raise exceptions.MalformedJwtToken()
-        except WrongNumberOfParts:
-            raise exceptions.MalformedJwtToken()
+        except NoSuitableSigningKeys as err:
+            raise exceptions.NoSuitableKeys() from err
+        except BadSyntax as err:
+            raise exceptions.MalformedJwtToken() from err
+        except WrongNumberOfParts as err:
+            raise exceptions.MalformedJwtToken() from err
 
 
 class PlatformKeyHandler:
@@ -145,8 +145,8 @@ def __init__(self, key_pem, kid=None):
                     kid=kid,
                     key=RSA.import_key(key_pem)
                 )
-            except ValueError:
-                raise exceptions.InvalidRsaKey()
+            except ValueError as err:
+                raise exceptions.InvalidRsaKey() from err
 
     def encode_and_sign(self, message, expiration=None):
         """
@@ -211,7 +211,7 @@ def validate_and_decode(self, token, iss=None, aud=None):
             # Else return token contents
             return message
 
-        except NoSuitableSigningKeys:
-            raise exceptions.NoSuitableKeys()
-        except BadSyntax:
-            raise exceptions.MalformedJwtToken()
+        except NoSuitableSigningKeys as err:
+            raise exceptions.NoSuitableKeys() from err
+        except BadSyntax as err:
+            raise exceptions.MalformedJwtToken() from err

lti_consumer/lti_1p3/tests/extensions/rest_framework/test_authentication.py

@@ -2,8 +2,8 @@
 Unit tests for LTI 1.3 consumer implementation
 """
 
-import ddt
 from unittest.mock import MagicMock, patch
+import ddt
 
 from Cryptodome.PublicKey import RSA
 from django.test.testcases import TestCase

lti_consumer/lti_1p3/tests/extensions/rest_framework/test_permissions.py

@@ -2,8 +2,8 @@
 Unit tests for LTI 1.3 consumer implementation
 """
 
-import ddt
 from unittest.mock import MagicMock
+import ddt
 
 from Cryptodome.PublicKey import RSA
 from django.test.testcases import TestCase

lti_consumer/lti_1p3/tests/test_consumer.py

@@ -4,9 +4,9 @@
 
 import json
 from urllib.parse import urlparse, parse_qs
+from unittest.mock import patch
 import ddt
 
-from unittest.mock import patch
 from django.test.testcases import TestCase
 
 from Cryptodome.PublicKey import RSA

lti_consumer/lti_1p3/tests/test_key_handlers.py

@@ -3,9 +3,9 @@
 """
 
 import json
+from unittest.mock import patch
 import ddt
 
-from unittest.mock import patch
 from django.test.testcases import TestCase
 
 from Cryptodome.PublicKey import RSA

lti_consumer/lti_xblock.py

@@ -87,7 +87,6 @@
     lti_1p3_enabled,
 )
 
-
 log = logging.getLogger(__name__)
 
 DOCS_ANCHOR_TAG_OPEN = (
@@ -573,8 +572,8 @@ def editable_fields(self):
         if config_service:
             is_already_sharing_learner_info = self.ask_to_send_email or self.ask_to_send_username
             if not config_service.configuration.lti_access_to_learners_editable(
-                    self.course_id,
-                    is_already_sharing_learner_info,
+                self.course_id,
+                is_already_sharing_learner_info,
             ):
                 editable_fields = tuple(
                     field
@@ -648,10 +647,10 @@ def lti_provider_key_secret(self):
                 if not key:
                     raise ValueError
                 key = ':'.join(key)
-            except ValueError:
+            except ValueError as error:
                 msg = 'Could not parse LTI passport: {lti_passport!r}. Should be "id:key:secret" string.'
                 msg = self.ugettext(msg).format(lti_passport=lti_passport)
-                raise LtiError(msg)
+                raise LtiError(msg) from error
 
             if lti_id == self.lti_id.strip():
                 return key, secret
@@ -783,11 +782,11 @@ def prefixed_custom_parameters(self):
             for custom_parameter in self.custom_parameters:
                 try:
                     param_name, param_value = [p.strip() for p in custom_parameter.split('=', 1)]
-                except ValueError:
+                except ValueError as error:
                     _ = self.runtime.service(self, "i18n").ugettext
                     msg = 'Could not parse custom parameter: {custom_parameter!r}. Should be "x=y" string.'
                     msg = self.ugettext(msg).format(custom_parameter=custom_parameter)
-                    raise LtiError(msg)
+                    raise LtiError(msg) from error
 
                 # LTI specs: 'custom_' should be prepended before each custom parameter, as pointed in link above.
                 if param_name not in LTI_PARAMETERS:
@@ -929,7 +928,7 @@ def student_view(self, context):
         return fragment
 
     @XBlock.handler
-    def lti_launch_handler(self, request, suffix=''):
+    def lti_launch_handler(self, request, suffix=''):  # pylint: disable=W0613
         """
         XBlock handler for launching LTI 1.1 tools.
 
@@ -984,7 +983,7 @@ def lti_launch_handler(self, request, suffix=''):
         return Response(template, content_type='text/html')
 
     @XBlock.handler
-    def lti_1p3_launch_handler(self, request, suffix=''):
+    def lti_1p3_launch_handler(self, request, suffix=''):  # pylint: disable=W0613
         """
         XBlock handler for launching the LTI 1.3 tools.
 
@@ -1010,7 +1009,7 @@ def lti_1p3_launch_handler(self, request, suffix=''):
         return Response(template, content_type='text/html')
 
     @XBlock.handler
-    def lti_1p3_launch_callback(self, request, suffix=''):
+    def lti_1p3_launch_callback(self, request, suffix=''):  # pylint: disable=W0613
         """
         XBlock handler for launching the LTI 1.3 tool.
 
@@ -1071,7 +1070,7 @@ def lti_1p3_launch_callback(self, request, suffix=''):
             return Response(template, status=400, content_type='text/html')
 
     @XBlock.handler
-    def lti_1p3_access_token(self, request, suffix=''):
+    def lti_1p3_access_token(self, request, suffix=''):  # pylint: disable=W0613
         """
         XBlock handler for creating access tokens for the LTI 1.3 tool.
 
@@ -1130,7 +1129,7 @@ def lti_1p3_access_token(self, request, suffix=''):
             )
 
     @XBlock.handler
-    def outcome_service_handler(self, request, suffix=''):
+    def outcome_service_handler(self, request, suffix=''):  # pylint: disable=W0613
         """
         XBlock handler for LTI Outcome Service requests.
 

lti_consumer/models.py

@@ -399,7 +399,7 @@ def clean(self):
         if self.score_given and self.score_maximum is None:
             raise ValidationError({'score_maximum': 'cannot be unset when score_given is set'})
 
-    def save(self, *args, **kwargs):  # pylint: disable=arguments-differ
+    def save(self, *args, **kwargs):  # pylint: disable=W0222
         self.full_clean()
         super().save(*args, **kwargs)
 

lti_consumer/outcomes.py

@@ -46,32 +46,32 @@ def parse_grade_xml_body(body):
         parser = etree.XMLParser(ns_clean=True, recover=True, encoding='utf-8')
         root = etree.fromstring(data, parser=parser)
     except etree.XMLSyntaxError as ex:
-        raise LtiError(str(ex) or 'Body is not valid XML')
+        raise LtiError(str(ex) or 'Body is not valid XML') from ex
 
     try:
         imsx_message_identifier = root.xpath("//def:imsx_messageIdentifier", namespaces=namespaces)[0].text or ''
-    except IndexError:
-        raise LtiError('Failed to parse imsx_messageIdentifier from XML request body')
+    except IndexError as error:
+        raise LtiError('Failed to parse imsx_messageIdentifier from XML request body') from error
 
     try:
         body = root.xpath("//def:imsx_POXBody", namespaces=namespaces)[0]
-    except IndexError:
-        raise LtiError('Failed to parse imsx_POXBody from XML request body')
+    except IndexError as error:
+        raise LtiError('Failed to parse imsx_POXBody from XML request body') from error
 
     try:
         action = body.getchildren()[0].tag.replace('{' + lti_spec_namespace + '}', '')
-    except IndexError:
-        raise LtiError('Failed to parse action from XML request body')
+    except IndexError as error:
+        raise LtiError('Failed to parse action from XML request body') from error
 
     try:
         sourced_id = root.xpath("//def:sourcedId", namespaces=namespaces)[0].text
-    except IndexError:
-        raise LtiError('Failed to parse sourcedId from XML request body')
+    except IndexError as error:
+        raise LtiError('Failed to parse sourcedId from XML request body') from error
 
     try:
         score = root.xpath("//def:textString", namespaces=namespaces)[0].text
-    except IndexError:
-        raise LtiError('Failed to parse score textString from XML request body')
+    except IndexError as error:
+        raise LtiError('Failed to parse score textString from XML request body') from error
 
     # Raise exception if score is not float or not in range 0.0-1.0 regarding spec.
     score = float(score)

lti_consumer/plugin/views.py

@@ -161,7 +161,7 @@ def perform_create(self, serializer):
         url_path='results/(?P<user_id>[^/.]+)?',
         renderer_classes=[LineItemResultsRenderer]
     )
-    def results(self, request, user_id=None, **kwargs):
+    def results(self, request, user_id=None, **kwargs):  # pylint: disable=W0613
         """
         Return a Result list for an LtiAgsLineItem
 
@@ -199,7 +199,7 @@ def results(self, request, user_id=None, **kwargs):
         parser_classes=[LineItemScoreParser],
         renderer_classes=[LineItemScoreRenderer]
     )
-    def scores(self, request, *args, **kwargs):
+    def scores(self, request, *args, **kwargs):  # pylint: disable=W0613
         """
         Create a Score record for an LtiAgsLineItem
 

lti_consumer/tests/unit/test_api.py

@@ -1,8 +1,8 @@
 """
 Tests for LTI API.
 """
-from django.test.testcases import TestCase
 from unittest.mock import Mock, patch
+from django.test.testcases import TestCase
 
 from lti_consumer.api import (
     _get_or_create_local_lti_config,

lti_consumer/tests/unit/test_lti_xblock.py

@@ -5,13 +5,13 @@
 from datetime import timedelta
 import json
 import urllib.parse
+from unittest.mock import Mock, PropertyMock, NonCallableMock, patch
 
 import ddt
 from Cryptodome.PublicKey import RSA
 from django.test.testcases import TestCase
 from django.utils import timezone
 from jwkest.jwk import RSAKey
-from unittest.mock import Mock, PropertyMock, NonCallableMock, patch
 
 from lti_consumer.api import get_lti_1p3_launch_info
 from lti_consumer.exceptions import LtiError

lti_consumer/tests/unit/test_models.py

@@ -2,12 +2,12 @@
 Unit tests for LTI models.
 """
 from datetime import timedelta
+from unittest.mock import patch
 from Cryptodome.PublicKey import RSA
 from django.utils import timezone
 from django.test.testcases import TestCase
 
 from jwkest.jwk import RSAKey
-from unittest.mock import patch
 
 from lti_consumer.lti_xblock import LtiConsumerXBlock
 from lti_consumer.models import LtiAgsLineItem, LtiConfiguration, LtiAgsScore