Skip to content

Commit

Permalink
perf: add compatibility with Quince release (#102)
Browse files Browse the repository at this point in the history
BREAKING CHANGE: add compatibility with Quince release

* chore: update main requirements & add django 4.2 support

* chore: support django 42 & solve issues in testing
* fix: delete deprecated pylint rules
* fix: use pylint messages control
* fix: ignore migrations from quality tests
* fix: solve deprecated methods & apply linter corrections

* chore: update github-actions requirements

* docs: update compatibility notes

* fix: add required blank line after table
  • Loading branch information
bra-i-am authored Feb 13, 2024
1 parent ba004ef commit cff9359
Show file tree
Hide file tree
Showing 22 changed files with 172 additions and 248 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/bumpversion.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ jobs:
previous_tag: ${{ steps.tag_version.outputs.previous_tag }}
bump_commit_sha: ${{ steps.bumpversion.outputs.commit_hash }}
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
with:
token: ${{ secrets.DEDALO_PAT }}
- name: Get next version
Expand All @@ -23,7 +23,7 @@ jobs:
default_prerelease_bump: false
dry_run: true
- name: Set up Python 3.8
uses: actions/setup-python@v4
uses: actions/setup-python@v5
with:
python-version: "3.8"
- name: Create bumpversion
Expand Down Expand Up @@ -53,7 +53,7 @@ jobs:
tag: ${{ steps.tag_version.outputs.new_tag }}
changelog: ${{ steps.tag_version.outputs.changelog }}
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
with:
token: ${{ secrets.DEDALO_PAT }}
- name: Create tag
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/commitlint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@ jobs:
- uses: actions/checkout@v2
with:
fetch-depth: 0
- uses: wagoid/commitlint-github-action@v5.4.4
- uses: wagoid/commitlint-github-action@v5
3 changes: 1 addition & 2 deletions .github/workflows/labeler.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,4 @@ jobs:
This PR exceeds the recommended size of 1000 lines.
Please make sure you are NOT addressing multiple issues with one PR.
Note this PR might be rejected due to its size.
github_api_url: 'api.github.com'
files_to_ignore: ''
github_api_url: 'api.github.com'
2 changes: 1 addition & 1 deletion .github/workflows/python-publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
- name: Set up Python
uses: actions/setup-python@v3
with:
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ jobs:
strategy:
max-parallel: 2
matrix:
python-version: ["3.8"]
django: ["32"]
python-version: ["3.8", "3.10", "3.11"]
django: ["32", "42"]
steps:
- name: Checkout
uses: actions/checkout@v4
Expand All @@ -25,7 +25,7 @@ jobs:
restore-keys: |
${{ runner.os }}-pip-
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v4
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}

Expand Down
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
4 changes: 3 additions & 1 deletion README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ Compatibility Notes
+-------------------+----------------+
| Palm | >= 6.0 |
+-------------------+----------------+
| Quince | >= 7.0 |
+-------------------+----------------+

The following changes to the plugin settings are necessary. If the release you are looking for is
not listed, then the accumulation of changes from previous releases is enough.
Expand All @@ -91,7 +93,7 @@ not listed, then the accumulation of changes from previous releases is enough.
EOX_TAGGING_GET_COURSE_OVERVIEW: "eox_tagging.edxapp_wrappers.backends.course_overview_i_v1"
EOX_TAGGING_BEARER_AUTHENTICATION: "eox_tagging.edxapp_wrappers.backends.bearer_authentication_i_v1"
**Koa, Lilac, Maple, Nutmeg, Olive**
**Koa, Lilac, Maple, Nutmeg, Olive, Palm and Quince**

.. code-block:: yaml
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 cff9359

Please sign in to comment.