Skip to content

Commit

Permalink
chore: support django 42 & solve issues in testing
Browse files Browse the repository at this point in the history
* fix: delete deprecated pylint rules
* fix: use pylint messages control
* fix: ignore migrations from quality tests
* fix: solve deprecated methods & apply linter corrections
  • Loading branch information
bra-i-am committed Feb 7, 2024
1 parent 9b6feaa commit a7d944c
Show file tree
Hide file tree
Showing 8 changed files with 87 additions and 133 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ test-python: clean ## Run test suite.
quality: clean ## Run quality test.
$(TOX) pycodestyle ./eox_tagging
$(TOX) pylint ./eox_tagging --rcfile=./setup.cfg
$(TOX) isort --check-only --diff ./eox_tagging
$(TOX) isort --check-only --diff ./eox_tagging --skip ./eox_tagging/migrations

build-docs:
make docs_requirements
Expand Down
38 changes: 29 additions & 9 deletions eox_tagging/api/v1/serializers.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
"""
Serializers for tags and related objects.
"""

from django.core.exceptions import ValidationError
from django.utils.translation import ugettext as _
from django.utils.translation import gettext as _
from rest_framework import serializers

from eox_tagging.api.v1 import fields
Expand All @@ -19,19 +20,36 @@
class TagSerializer(serializers.ModelSerializer):
"""Serializer for tag objects."""

target_id = serializers.CharField(source='target_object', write_only=True)
owner_id = serializers.CharField(source='owner_object', required=False, write_only=True)
owner_type = serializers.CharField(source='owner_object_type', write_only=True, required=False)
target_type = serializers.CharField(source='target_object_type', write_only=True)
target_id = serializers.CharField(source="target_object", write_only=True)
owner_id = serializers.CharField(
source="owner_object", required=False, write_only=True
)
owner_type = serializers.CharField(
source="owner_object_type", write_only=True, required=False
)
target_type = serializers.CharField(source="target_object_type", write_only=True)
meta = serializers.SerializerMethodField()
access = fields.EnumField(enum=AccessLevel, required=False)
status = fields.EnumField(enum=Status, required=False)

class Meta:
"""Meta class."""

model = Tag
fields = ('meta', 'key', 'tag_value', 'tag_type', 'access', 'activation_date', 'expiration_date',
'target_id', 'owner_id', 'owner_type', 'target_type', 'status')
fields = (
"meta",
"key",
"tag_value",
"tag_type",
"access",
"activation_date",
"expiration_date",
"target_id",
"owner_id",
"owner_type",
"target_type",
"status",
)

def get_meta(self, instance):
"""Getter of read-only field that returns technical information."""
Expand Down Expand Up @@ -61,8 +79,10 @@ def create(self, validated_data):

try:
target_object = get_object_from_edxapp(target_type, **data)
except Exception:
raise serializers.ValidationError({"Target": _(f"Error getting {target_type} object.")})
except Exception as exc:
raise serializers.ValidationError(
{"Target": _(f"Error getting {target_type} object.")}
) from exc

if owner_type and owner_type.lower() == "user":
owner_object = self.context.get("request").user
Expand Down
2 changes: 1 addition & 1 deletion eox_tagging/api/v1/test/test_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def decorated_function(*args, **kwargs):
"validate_expiration_date": {"exist": True},
},
])
class TestTagViewSet(TestCase): # pylint: disable=too-many-instance-attributes
class TestTagViewSet(TestCase):
"""Test class for tags viewset."""

patch_permissions = patch(
Expand Down
2 changes: 1 addition & 1 deletion eox_tagging/api/v1/viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ def audit_method(*args, **kwargs): # pylint: disable=unused-argument
],
responses={status.HTTP_404_NOT_FOUND: "Not found"},
)
class TagViewSet(viewsets.ModelViewSet): # pylint: disable=too-many-ancestors
class TagViewSet(viewsets.ModelViewSet):
"""Viewset for listing and creating Tags."""

serializer_class = TagSerializer
Expand Down
4 changes: 2 additions & 2 deletions eox_tagging/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ class Tag(models.Model):

objects = TagQuerySet().as_manager()

class Meta: # pylint: disable=too-few-public-methods
class Meta:
"""Meta class. """
verbose_name = "tag"
verbose_name_plural = "tags"
Expand Down Expand Up @@ -299,7 +299,7 @@ def clean_fields(self): # pylint: disable=arguments-differ
return
self.validator.validate_fields_integrity()

def full_clean(self, exclude=None, validate_unique=False):
def full_clean(self, exclude=None, validate_unique=False): # pylint: disable=arguments-differ
"""
Call clean_fields(), clean(), and validate_unique() -not implemented- on the model.
Raise a ValidationError for any errors that occur.
Expand Down
70 changes: 43 additions & 27 deletions eox_tagging/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,10 @@ def __force_configuration_values(self):
if key.startswith(pattern):
try:
self.instance.set_attribute(key.replace(pattern, ""), value)
except Exception:
raise ValidationError(f"EOX_TAGGING | The field {key} with value `{value}` is wrongly configured")
except Exception as exc:
raise ValidationError(
f"EOX_TAGGING | The field {key} with value `{value}` is wrongly configured"
) from exc
del self.current_tag_definitions[key]

def __validate_required(self):
Expand Down Expand Up @@ -126,16 +128,18 @@ def __validate_configuration(self):
for _key in value: # Validations must exist as a class method
try:
getattr(self, f"validate_{_key}")
except AttributeError:
except AttributeError as exc:
raise ValidationError(
f"EOX_TAGGING | The field {key} with value `{_key}` is wrongly configured."
)
) from exc
# Validate key existence
clean_key = re.sub(regex, "", key)
try:
self.instance.get_attribute(clean_key)
except AttributeError:
raise ValidationError(f"EOX_TAGGING | The field `{key}` is wrongly configured.")
except AttributeError as exc:
raise ValidationError(
f"EOX_TAGGING | The field `{key}` is wrongly configured."
) from exc

def __validate_configuration_types(self):
"""Function that validate the correct type for pairs <key, value> in configuration."""
Expand Down Expand Up @@ -170,13 +174,17 @@ def __validate_model(self, field_name):
"""Function that validates the instances in GFK fields calling the integrity validators."""
try:
model_name = self.instance.get_attribute(field_name, name=True)
except AttributeError:
raise ValidationError(f"EOX_TAGGING | The field '{field_name}' is wrongly configured.")
except AttributeError as exc:
raise ValidationError(
f"EOX_TAGGING | The field '{field_name}' is wrongly configured."
) from exc
try:
if model_name:
self.model_validations[model_name](field_name)
except KeyError:
raise ValidationError(f"EOX_TAGGING | Could not find integrity validation for field '{field_name}'")
except KeyError as exc:
raise ValidationError(
f"EOX_TAGGING | Could not find integrity validation for field '{field_name}'"
) from exc

# Integrity validators
def __validate_proxy_model(self, object_name):
Expand All @@ -189,10 +197,10 @@ def __validate_proxy_model(self, object_name):
opaque_key = self.instance.get_attribute(object_name).opaque_key
try:
CourseOverview.get_from_id(opaque_key)
except Exception:
except Exception as exc:
raise ValidationError(
f"EOX_TAGGING | Could not find opaque key: '{opaque_key}' for relation '{object_name}'"
)
) from exc

def __validate_user_integrity(self, object_name):
"""
Expand All @@ -209,9 +217,11 @@ def __validate_user_integrity(self, object_name):
try:
get_edxapp_user(**data)

except Exception:
raise ValidationError(f"EOX_TAGGING | Could not find ID: {self.instance.get_attribute(object_name).id} \
for relation '{object_name}'")
except Exception as exc:
raise ValidationError(
f"EOX_TAGGING | Could not find ID: {self.instance.get_attribute(object_name).id} \
for relation '{object_name}'"
) from exc

def __validate_enrollment_integrity(self, object_name):
"""
Expand All @@ -232,11 +242,11 @@ def __validate_enrollment_integrity(self, object_name):
f"EOX_TAGGING | Enrollment for user '{data['username']}' and courseID '{data['course_id']}' \
does not exist"
)
except Exception:
except Exception as exc:
raise ValidationError(
f"EOX_TAGGING | Error getting enrollment for user '{data['username']}' \
and courseID '{data['course_id']}'"
)
) from exc

def __validate_site_integrity(self, object_name):
"""
Expand All @@ -249,8 +259,10 @@ def __validate_site_integrity(self, object_name):

try:
Site.objects.get(id=site_id)
except ObjectDoesNotExist:
raise ValidationError(f"EOX_TAGGING | Site '{site_id}' does not exist")
except ObjectDoesNotExist as exc:
raise ValidationError(
f"EOX_TAGGING | Site '{site_id}' does not exist"
) from exc

def __validate_certificate_integrity(self, object_name):
"""
Expand All @@ -263,8 +275,10 @@ def __validate_certificate_integrity(self, object_name):

try:
GeneratedCertificate.objects.get(id=certificate_id)
except ObjectDoesNotExist:
raise ValidationError(f"EOX_TAGGING | Certificate '{certificate_id}' does not exist")
except ObjectDoesNotExist as exc:
raise ValidationError(
f"EOX_TAGGING | Certificate '{certificate_id}' does not exist"
) from exc

# Other validations
def validate_no_updating(self):
Expand Down Expand Up @@ -303,9 +317,11 @@ def validate_opaque_key(self, field, value):
opaque_key_to_validate = getattr(all_opaque_keys, value)
# Validation method for OpaqueKey: opaque_key_to_validate
getattr(opaque_key_to_validate, "from_string")(field_value)
except InvalidKeyError:
except InvalidKeyError as exc:
# We don't recognize this key
raise ValidationError(f"The key '{field_value}' for '{field}' is not an opaque key")
raise ValidationError(
f"The key '{field_value}' for '{field}' is not an opaque key"
) from exc

def validate_in(self, field, values):
"""
Expand Down Expand Up @@ -374,11 +390,11 @@ def validate_between(self, field, value):
for datetime_str in value:
try:
datetime_obj.append(datetime.datetime.strptime(datetime_str, DATETIME_FORMAT_VALIDATION))
except TypeError:
except TypeError as exc:
raise ValidationError(
f"EOX_TAGGING | The DateTime field '{datetime_str}' \
must follow the format '{DATETIME_FORMAT_VALIDATION}'."
)
) from exc

if field_value < datetime_obj[0] or field_value > datetime_obj[-1]:
raise ValidationError(
Expand All @@ -396,10 +412,10 @@ def __compare_equal_dates(self, field_value, value):
"""
try:
datetime_str = datetime.datetime.strptime(value, DATETIME_FORMAT_VALIDATION)
except TypeError:
except TypeError as exc:
raise ValidationError(
f"EOX_TAGGING | The DateTime field '{value}' must follow the format '{DATETIME_FORMAT_VALIDATION}'."
)
) from exc

if field_value != datetime_str:
raise ValidationError(f"EOX_TAGGING | The DateTime field '{field_value}' must be equal to '{str(value)}'.")
Expand Down
Loading

0 comments on commit a7d944c

Please sign in to comment.