ci: Switch flake8 and isort for ruff (#329)

* ci: Switch flake8 and isort for ruff

* ci: Add stacklevel to test check on mock assertion.

Author: marksweb

.github/workflows/lint.yml

@@ -7,36 +7,19 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
-  flake8:
-    name: flake8
+  ruff:
+    name: ruff
     runs-on: ubuntu-latest
     steps:
       - name: Checkout
         uses: actions/checkout@v3
       - name: Set up Python
-        uses: actions/setup-python@v3
+        uses: actions/setup-python@v4
         with:
-          python-version: 3.8
-      - name: Install flake8
-        run: pip install --upgrade flake8
-      - name: Run flake8
-        uses: liskin/gh-problem-matcher-wrap@v1
-        with:
-          linters: flake8
-          run: flake8
-
-  isort:
-    runs-on: ubuntu-latest
-    steps:
-      - name: Checkout
-        uses: actions/checkout@v3
-      - name: Set up Python
-        uses: actions/setup-python@v3
-        with:
-          python-version: 3.8
-      - run: python -m pip install isort
-      - name: isort
-        uses: liskin/gh-problem-matcher-wrap@v1
-        with:
-          linters: isort
-          run: isort --check --diff ./
+          python-version: "3.11"
+          cache: 'pip'
+      - run: |
+          python -m pip install --upgrade pip
+          pip install ruff
+      - name: Run Ruff
+        run: ruff djangocms_versioning

.pre-commit-config.yaml

@@ -14,19 +14,8 @@ repos:
       - id: check-merge-conflict
       - id: mixed-line-ending
 
-  - repo: https://github.com/PyCQA/flake8
-    rev: 4.0.1
+  - repo: https://github.com/charliermarsh/ruff-pre-commit
+    rev: "v0.0.264"
     hooks:
-      - id: flake8
-        additional_dependencies:
-          - flake8-broken-line
-          - flake8-bugbear
-          - flake8-builtins
-          - flake8-eradicate
-          - flake8-tidy-imports
-          - pep8-naming
-
-  - repo: https://github.com/pycqa/isort
-    rev: 5.10.1
-    hooks:
-      - id: isort
+      - id: ruff
+        args: [--fix, --exit-non-zero-on-fix]

djangocms_versioning/admin.py

@@ -2,6 +2,10 @@
 from collections import OrderedDict
 from urllib.parse import urlparse
 
+from cms.models import PageContent
+from cms.utils import get_language_from_request
+from cms.utils.conf import get_cms_setting
+from cms.utils.urlutils import add_url_parameters, static_with_version
 from django.contrib import admin, messages
 from django.contrib.admin.options import IncorrectLookupParameters
 from django.contrib.admin.utils import unquote
@@ -19,11 +23,6 @@
 from django.utils.html import format_html, format_html_join
 from django.utils.translation import gettext_lazy as _
 
-from cms.models import PageContent
-from cms.utils import get_language_from_request
-from cms.utils.conf import get_cms_setting
-from cms.utils.urlutils import add_url_parameters, static_with_version
-
 from . import versionables
 from .conf import USERNAME_FIELD
 from .constants import DRAFT, INDICATOR_DESCRIPTIONS, PUBLISHED
@@ -173,16 +172,16 @@ def indicator(obj):
                     "description": INDICATOR_DESCRIPTIONS.get(status, _("Empty")),
                     "menu_template": "admin/cms/page/tree/indicator_menu.html",
                     "menu": json.dumps(render_to_string("admin/cms/page/tree/indicator_menu.html",
-                                                        dict(indicator_menu_items=menu))) if menu else None,
+                                                        {"indicator_menu_items": menu})) if menu else None,
                 }
             )
         indicator.short_description = self.indicator_column_label
         return indicator
 
     def state_indicator(self, obj):
         raise ValueError(
-            "ModelAdmin.display_list contains \"state_indicator\" as a placeholder for status indicators. "
-            "Status indicators, however, are not loaded. If you implement \"get_list_display\" make "
+            'ModelAdmin.display_list contains "state_indicator" as a placeholder for status indicators. '
+            'Status indicators, however, are not loaded. If you implement "get_list_display" make '
             "sure it calls super().get_list_display."
         )  # pragma: no cover
 
@@ -392,8 +391,8 @@ def extend_list_display(self, request, modifier_dict, list_display):
                 list_display[list_display.index(field)] = self._get_field_modifier(request, modifier_dict, field)
                 list_display = tuple(list_display)
                 return list_display
-            except ValueError:
-                raise ImproperlyConfigured("The target field does not exist in this context")
+            except ValueError as err:
+                raise ImproperlyConfigured("The target field does not exist in this context") from err
         return tuple(list_display)
 
     def get_list_display(self, request):
@@ -777,19 +776,19 @@ def archive_view(self, request, object_id):
             return redirect(version_list_url(version.content))
 
         if request.method != "POST":
-            context = dict(
-                object_name=version.content,
-                version_number=version.number,
-                object_id=object_id,
-                archive_url=reverse(
+            context = {
+                "object_name": version.content,
+                "version_number": version.number,
+                "object_id": object_id,
+                "archive_url": reverse(
                     "admin:{app}_{model}_archive".format(
                         app=self.model._meta.app_label,
                         model=self.model._meta.model_name,
                     ),
                     args=(version.content.pk,),
                 ),
-                back_url=self.back_link(request, version),
-            )
+                "back_url": self.back_link(request, version),
+            }
             return render(
                 request, "djangocms_versioning/admin/archive_confirmation.html", context
             )
@@ -857,19 +856,19 @@ def unpublish_view(self, request, object_id):
             return redirect(version_list_url(version.content))
 
         if request.method != "POST":
-            context = dict(
-                object_name=version.content,
-                version_number=version.number,
-                object_id=object_id,
-                unpublish_url=reverse(
+            context = {
+                "object_name": version.content,
+                "version_number": version.number,
+                "object_id": object_id,
+                "unpublish_url": reverse(
                     "admin:{app}_{model}_unpublish".format(
                         app=self.model._meta.app_label,
                         model=self.model._meta.model_name,
                     ),
                     args=(version.content.pk,),
                 ),
-                back_url=self.back_link(request, version),
-            )
+                "back_url": self.back_link(request, version),
+            }
             extra_context = OrderedDict(
                 [
                     (key, func(request, version))
@@ -970,20 +969,20 @@ def revert_view(self, request, object_id):
             draft_version = drafts.first()
 
         if request.method != "POST":
-            context = dict(
-                object_name=version.content,
-                version_number=version.number,
-                draft_version=draft_version,
-                object_id=object_id,
-                revert_url=reverse(
+            context = {
+                "object_name": version.content,
+                "version_number": version.number,
+                "draft_version": draft_version,
+                "object_id": object_id,
+                "revert_url": reverse(
                     "admin:{app}_{model}_revert".format(
                         app=self.model._meta.app_label,
                         model=self.model._meta.model_name,
                     ),
                     args=(version.content.pk,),
                 ),
-                back_url=self.back_link(request, version),
-            )
+                "back_url": self.back_link(request, version),
+            }
             return render(
                 request, "djangocms_versioning/admin/revert_confirmation.html", context
             )
@@ -1012,20 +1011,20 @@ def discard_view(self, request, object_id):
             return redirect(version_list_url(version.content))
 
         if request.method != "POST":
-            context = dict(
-                object_name=version.content,
-                version_number=version.number,
-                draft_version=version,
-                object_id=object_id,
-                revert_url=reverse(
+            context = {
+                "object_name": version.content,
+                "version_number": version.number,
+                "draft_version": version,
+                "object_id": object_id,
+                "revert_url": reverse(
                     "admin:{app}_{model}_revert".format(
                         app=self.model._meta.app_label,
                         model=self.model._meta.model_name,
                     ),
                     args=(version.content.pk,),
                 ),
-                back_url=self.back_link(request, version),
-            )
+                "back_url": self.back_link(request, version),
+            }
             return render(
                 request, "djangocms_versioning/admin/discard_confirmation.html", context
             )
@@ -1034,9 +1033,9 @@ def discard_view(self, request, object_id):
         if request.POST.get("discard"):
             ModelClass = version.content.__class__
             deleted = version.delete()
-            if deleted[1]['last']:
-                version_url = get_admin_url(ModelClass, 'changelist')
-                self.message_user(request, _('The last version has been deleted'))
+            if deleted[1]["last"]:
+                version_url = get_admin_url(ModelClass, "changelist")
+                self.message_user(request, _("The last version has been deleted"))
 
         return redirect(version_url)
 
@@ -1116,7 +1115,7 @@ def changelist_view(self, request, extra_context=None):
             # redirect to grouper form when there's no GET parameters
             opts = self.model._meta
             return redirect(
-                reverse("admin:{}_{}_grouper".format(opts.app_label, opts.model_name))
+                reverse(f"admin:{opts.app_label}_{opts.model_name}_grouper")
             )
         extra_context = extra_context or {}
         versionable = versionables.for_content(self.model._source_model)
@@ -1134,7 +1133,7 @@ def changelist_view(self, request, extra_context=None):
 
         if grouper:
             # CAVEAT: as the breadcrumb trails expect a value for latest content in the template
-            extra_context["latest_content"] = ({'pk': None})
+            extra_context["latest_content"] = ({"pk": None})
 
             extra_context.update(
                 grouper=grouper,

djangocms_versioning/apps.py

@@ -6,7 +6,7 @@
 class VersioningConfig(AppConfig):
     name = "djangocms_versioning"
     verbose_name = _("django CMS Versioning")
-    default_auto_field = 'django.db.models.AutoField'
+    default_auto_field = "django.db.models.AutoField"
 
     def ready(self):
         from cms.models import contentmodels, fields
@@ -24,7 +24,7 @@ def ready(self):
 
         # Remove uniqueness constraint from PageContent model to allow for different versions
         pagecontent_unique_together = tuple(
-            set(contentmodels.PageContent._meta.unique_together) - set((("language", "page"),))
+            set(contentmodels.PageContent._meta.unique_together) - {("language", "page")}
         )
         contentmodels.PageContent._meta.unique_together = pagecontent_unique_together
 

djangocms_versioning/cms_config.py

@@ -1,24 +1,27 @@
 import collections
 
+from cms.app_base import CMSAppConfig, CMSAppExtension
+from cms.models import PageContent, Placeholder
+from cms.utils import get_language_from_request
+from cms.utils.i18n import get_language_list, get_language_tuple
+from cms.utils.plugins import copy_plugins_to_placeholder
+from cms.utils.urlutils import admin_reverse
 from django.conf import settings
 from django.contrib.admin.utils import flatten_fieldsets
 from django.core.exceptions import (
     ImproperlyConfigured,
     ObjectDoesNotExist,
     PermissionDenied,
 )
-from django.http import HttpResponse, HttpResponseBadRequest, HttpResponseForbidden
+from django.http import (
+    HttpResponse,
+    HttpResponseBadRequest,
+    HttpResponseForbidden,
+)
 from django.utils.encoding import force_str
 from django.utils.functional import cached_property
 from django.utils.translation import gettext_lazy as _
 
-from cms.app_base import CMSAppConfig, CMSAppExtension
-from cms.models import PageContent, Placeholder
-from cms.utils import get_language_from_request
-from cms.utils.i18n import get_language_list, get_language_tuple
-from cms.utils.plugins import copy_plugins_to_placeholder
-from cms.utils.urlutils import admin_reverse
-
 from . import indicators, versionables
 from .admin import VersioningAdminMixin
 from .constants import INDICATOR_DESCRIPTIONS
@@ -87,7 +90,7 @@ def handle_versioning_setting(self, cms_config):
             registered_so_far = [v.content_model for v in self.versionables]
             if versionable.content_model in registered_so_far:
                 raise ImproperlyConfigured(
-                    "{!r} has already been registered".format(versionable.content_model)
+                    f"{versionable.content_model!r} has already been registered"
                 )
             # Checks passed. Add versionable to our master list
             self.versionables.append(versionable)
@@ -237,8 +240,8 @@ def label_from_instance(obj, language):
     """
     title = obj.get_title(language) or _("No available title")
     path = obj.get_path(language)
-    path = "/{}/".format(path) if path else _("Unpublished")
-    return "{title} ({path})".format(title=title, path=path)
+    path = f"/{path}/" if path else _("Unpublished")
+    return f"{title} ({path})"
 
 
 def on_page_content_publish(version):
@@ -279,7 +282,7 @@ def get_readonly_fields(self, request, obj=None):
             version = Version.objects.get_for_content(obj)
             if not version.check_modify.as_bool(request.user):
                 form = self.get_form_class(request)
-                if getattr(form, "fieldsets"):
+                if form.fieldsets:
                     fields = flatten_fieldsets(form.fieldsets)
                 fields = list(fields)
                 for f_name in ["slug", "overwrite_url"]:
@@ -304,8 +307,8 @@ def get_queryset(self, request):
             # TODO: Improve the grouping filters to use anything defined in the
             #       apps versioning config extra_grouping_fields
             grouping_filters = {}
-            if 'language' in versionable.extra_grouping_fields:
-                grouping_filters['language'] = get_language_from_request(request)
+            if "language" in versionable.extra_grouping_fields:
+                grouping_filters["language"] = get_language_from_request(request)
 
             return queryset.filter(pk__in=versionable.distinct_groupers(**grouping_filters))
         return queryset
@@ -318,7 +321,7 @@ def get_queryset(self, request):
     #           - where it should live going forwards (cms vs versioning)
     #           - A better way of making the feature extensible / modifiable for versioning
     def copy_language(self, request, object_id):
-        target_language = request.POST.get('target_language')
+        target_language = request.POST.get("target_language")
 
         # CAVEAT: Avoiding self.get_object because it sets the page cache,
         #         We don't want a draft showing to a regular site visitor!
@@ -349,7 +352,7 @@ def copy_language(self, request, object_id):
             plugins = placeholder.get_plugins_list(source_page_content.language)
 
             if not target.has_add_plugins_permission(request.user, plugins):
-                return HttpResponseForbidden(force_str(_('You do not have permission to copy these plugins.')))
+                return HttpResponseForbidden(force_str(_("You do not have permission to copy these plugins.")))
             copy_plugins_to_placeholder(plugins, target, language=target_language)
         return HttpResponse("ok")