From b353fcab1e80233aac41f16d2cabbff13e44bb60 Mon Sep 17 00:00:00 2001 From: Mark Walker Date: Thu, 26 Oct 2023 11:48:36 +0100 Subject: [PATCH 1/4] ci: Enable mypy hook --- .pre-commit-config.yaml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index c96bdf6d..ad06dee0 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -44,7 +44,7 @@ repos: - id: end-of-file-fixer - id: trailing-whitespace -# - repo: https://github.com/pre-commit/mirrors-mypy -# rev: v1.3.0 -# hooks: -# - id: mypy + - repo: https://github.com/pre-commit/mirrors-mypy + rev: v1.3.0 + hooks: + - id: mypy From 7abf719f7fdb826c9d08a0946f8316b341b7059b Mon Sep 17 00:00:00 2001 From: Mark Walker Date: Thu, 26 Oct 2023 13:25:41 +0100 Subject: [PATCH 2/4] chore: Type hint all the things --- .github/PULL_REQUEST_TEMPLATE.md | 2 +- aldryn_config.py | 20 ++++++----- pyproject.toml | 3 ++ src/djangocms_snippet/admin.py | 2 +- src/djangocms_snippet/cms_plugins.py | 11 +++++- .../migrations/0003_auto_data_fill_slug.py | 11 +++--- src/djangocms_snippet/models.py | 4 +-- .../templatetags/snippet_tags.py | 28 ++++++++------- tests/settings.py | 25 +++++++------ tests/test_migrations.py | 17 +++++---- tests/test_models.py | 1 - tests/test_plugins.py | 34 +++++++++++++----- tests/test_templatetags.py | 36 ++++++++----------- 13 files changed, 114 insertions(+), 80 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index dc334a20..8f0dc786 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -26,5 +26,5 @@ Use 'x' to check each item: [x] I have ... * [ ] I have opened this pull request against ``master`` * [ ] I have added or modified the tests when changing logic * [ ] I have followed [the conventional commits guidelines](https://www.conventionalcommits.org/) to add meaningful information into the changelog -* [ ] I have read the [contribution guidelines ](https://github.com/django-cms/django-cms/blob/develop/CONTRIBUTING.rst) and I have joined #workgroup-pr-review on +* [ ] I have read the [contribution guidelines ](https://github.com/django-cms/django-cms/blob/develop/CONTRIBUTING.rst) and I have joined #workgroup-pr-review on [Slack](https://www.django-cms.org/slack) to find a “pr review buddy” who is going to review my pull request. diff --git a/aldryn_config.py b/aldryn_config.py index 3e532b2c..f8a06fe8 100644 --- a/aldryn_config.py +++ b/aldryn_config.py @@ -1,3 +1,5 @@ +from typing import Any + from aldryn_client import forms @@ -11,16 +13,18 @@ class Form(forms.BaseForm): required=False, ) enable_search = forms.CheckboxField( - 'Enable snippet content to be searchable.', + "Enable snippet content to be searchable.", required=False, initial=False, ) - def to_settings(self, data, settings): - if data['editor_theme']: - settings['DJANGOCMS_SNIPPET_THEME'] = data['editor_theme'] - if data['editor_mode']: - settings['DJANGOCMS_SNIPPET_MODE'] = data['editor_mode'] - if data['enable_search']: - settings['DJANGOCMS_SNIPPET_SEARCH'] = data['enable_search'] + def to_settings( + self, data: dict[str, Any], settings: dict[str, Any] + ) -> dict[str, Any]: + if data["editor_theme"]: + settings["DJANGOCMS_SNIPPET_THEME"] = data["editor_theme"] + if data["editor_mode"]: + settings["DJANGOCMS_SNIPPET_MODE"] = data["editor_mode"] + if data["enable_search"]: + settings["DJANGOCMS_SNIPPET_SEARCH"] = data["enable_search"] return settings diff --git a/pyproject.toml b/pyproject.toml index fc015bfc..abbd33f5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -135,6 +135,9 @@ ignore = [ "__init__.py" = [ "F401" # unused-import ] +"snippet_tags.py" = [ + "FBT001" # Boolean positional arg in function definition +] [tool.ruff.isort] combine-as-imports = true diff --git a/src/djangocms_snippet/admin.py b/src/djangocms_snippet/admin.py index 5c34d7a7..d8e95a84 100644 --- a/src/djangocms_snippet/admin.py +++ b/src/djangocms_snippet/admin.py @@ -19,7 +19,7 @@ class Media: list_display = ("slug", "name") search_fields: ClassVar[list[str]] = ["slug", "name"] - prepopulated_fields: ClassVar[dict[str, list[str]]] = {"slug": ("name",)} + prepopulated_fields: ClassVar[dict[str, tuple[str]]] = {"slug": ("name",)} change_form_template = "djangocms_snippet/admin/change_form.html" text_area_attrs: ClassVar[dict[str, Any]] = { "rows": 20, diff --git a/src/djangocms_snippet/cms_plugins.py b/src/djangocms_snippet/cms_plugins.py index e6b62b11..b110a346 100644 --- a/src/djangocms_snippet/cms_plugins.py +++ b/src/djangocms_snippet/cms_plugins.py @@ -1,7 +1,11 @@ +from typing import Any + +from cms.models import Placeholder from cms.plugin_base import CMSPluginBase from cms.plugin_pool import plugin_pool from django import template from django.conf import settings +from django.template.context import BaseContext from django.utils.html import escape from django.utils.safestring import mark_safe from django.utils.translation import gettext_lazy as _ @@ -19,7 +23,12 @@ class SnippetPlugin(CMSPluginBase): text_editor_preview = False cache = CACHE_ENABLED - def render(self, context, instance, placeholder): + def render( + self, + context: BaseContext, + instance: SnippetPtr, + placeholder: Placeholder, + ) -> dict[str, Any]: try: if instance.snippet.template: context = context.flatten() diff --git a/src/djangocms_snippet/migrations/0003_auto_data_fill_slug.py b/src/djangocms_snippet/migrations/0003_auto_data_fill_slug.py index 3ecde907..080b4a6a 100644 --- a/src/djangocms_snippet/migrations/0003_auto_data_fill_slug.py +++ b/src/djangocms_snippet/migrations/0003_auto_data_fill_slug.py @@ -1,19 +1,22 @@ from collections import Counter +import typing -from django.db import migrations, models +from django.db import migrations +from django.db.backends.base.schema import BaseDatabaseSchemaEditor +from django.db.migrations.state import StateApps from django.utils.text import slugify -def auto_fill_slugs(apps, schema_editor): +def auto_fill_slugs(apps: StateApps, schema_editor: BaseDatabaseSchemaEditor) -> None: """ Go through every snippet to fill them a slug if not any """ Snippet = apps.get_model("djangocms_snippet", "Snippet") - SlugCounter = Counter() + SlugCounter: typing.Counter[str] = Counter() for snippet_item in Snippet.objects.all(): # pragma: no cover if not snippet_item.slug: snippet_item.slug = slugify(snippet_item.name) - # Avoid duplicate slug, adding slug occurence count to the slug + # Avoid duplicate slug, adding slug occurrence count to the slug if snippet_item.slug in SlugCounter: snippet_item.slug = f"{snippet_item.slug}-{str(SlugCounter[snippet_item.slug])}" SlugCounter[snippet_item.slug] += 1 diff --git a/src/djangocms_snippet/models.py b/src/djangocms_snippet/models.py index 692f1bad..6d1fbf19 100644 --- a/src/djangocms_snippet/models.py +++ b/src/djangocms_snippet/models.py @@ -49,7 +49,7 @@ class Meta: verbose_name = _("Snippet") verbose_name_plural = _("Snippets") - def __str__(self): + def __str__(self) -> str: return self.name @@ -77,6 +77,6 @@ class Meta: verbose_name = _("Snippet Ptr") verbose_name_plural = _("Snippet Ptrs") - def __str__(self): + def __str__(self) -> str: # Return the referenced snippet's name rather than the default (ID #) return self.snippet.name diff --git a/src/djangocms_snippet/templatetags/snippet_tags.py b/src/djangocms_snippet/templatetags/snippet_tags.py index a73870d6..5bb506b0 100644 --- a/src/djangocms_snippet/templatetags/snippet_tags.py +++ b/src/djangocms_snippet/templatetags/snippet_tags.py @@ -1,9 +1,13 @@ """ Snippet template tags """ +from collections.abc import Generator from contextlib import contextmanager +from typing import Any from django import template +from django.template.base import Parser, Token +from django.template.context import BaseContext from django.utils.html import escape from django.utils.safestring import mark_safe from django.utils.translation import gettext_lazy as _ @@ -16,9 +20,11 @@ @contextmanager -def exceptionless(truth): - # Accepts one truth parameter, when 'False' normal behavior - # when 'True' any expection will be suppressed +def exceptionless(truth: bool) -> Generator[None, None, None]: + """ + Accepts one truth parameter, when 'False' normal behavior + when 'True' any exception will be suppressed + """ try: yield except Exception: @@ -26,7 +32,7 @@ def exceptionless(truth): # WARNING: suppressing exception pass else: - # Reraising exception + # Re-raising exception raise @@ -35,7 +41,7 @@ class SnippetFragment(template.Node): Get a snippet HTML fragment """ - def __init__(self, snippet_id_varname, *args): + def __init__(self, snippet_id_varname: str, *args: Any): """ :type insert_instance_varname: string or object ``django.db.models.Model`` @@ -53,7 +59,7 @@ def __init__(self, snippet_id_varname, *args): self.parse_until = True self.nodelist = args[1] - def render(self, context): + def render(self, context: BaseContext) -> str: """ :type context: dict :param context: Context tag object @@ -75,11 +81,9 @@ def render(self, context): self.get_content_render(context, snippet_instance) ) - # Rely on the fact that manager something went wrong - # render the fallback template - return self.nodelist.render(context) - - def get_content_render(self, context, instance): + def get_content_render( + self, context: BaseContext, instance: Snippet + ) -> str: """ Render the snippet HTML, using a template if defined in its instance """ @@ -113,7 +117,7 @@ def get_content_render(self, context, instance): @register.tag(name="snippet_fragment") -def do_snippet_fragment(parser, token): +def do_snippet_fragment(parser: Parser, token: Token) -> SnippetFragment: """ Display a snippet HTML fragment diff --git a/tests/settings.py b/tests/settings.py index 5656cebe..763998f2 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -1,23 +1,26 @@ #!/usr/bin/env python HELPER_SETTINGS = { - 'INSTALLED_APPS': [ - 'tests.utils', + "INSTALLED_APPS": [ + "tests.utils", ], - 'CMS_LANGUAGES': { - 1: [{ - 'code': 'en', - 'name': 'English', - }] + "CMS_LANGUAGES": { + 1: [ + { + "code": "en", + "name": "English", + } + ] }, - 'LANGUAGE_CODE': 'en', - 'ALLOWED_HOSTS': ['localhost'], + "LANGUAGE_CODE": "en", + "ALLOWED_HOSTS": ["localhost"], } def run(): from app_helper import runner - runner.cms('djangocms_snippet') + runner.cms("djangocms_snippet") -if __name__ == '__main__': + +if __name__ == "__main__": run() diff --git a/tests/test_migrations.py b/tests/test_migrations.py index 29c4a94b..fe27c860 100644 --- a/tests/test_migrations.py +++ b/tests/test_migrations.py @@ -7,24 +7,23 @@ class MigrationTestCase(TestCase): - @override_settings(MIGRATION_MODULES={}) def test_for_missing_migrations(self): output = StringIO() options = { - 'interactive': False, - 'dry_run': True, - 'stdout': output, - 'check_changes': True, + "interactive": False, + "dry_run": True, + "stdout": output, + "check_changes": True, } try: - call_command('makemigrations', **options) + call_command("makemigrations", **options) except SystemExit as e: status_code = str(e) else: # the "no changes" exit code is 0 - status_code = '0' + status_code = "0" - if status_code == '1': - self.fail('There are missing migrations:\n {}'.format(output.getvalue())) + if status_code == "1": + self.fail(f"There are missing migrations:\n {output.getvalue()}") diff --git a/tests/test_models.py b/tests/test_models.py index 077e0010..127a0a0d 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -4,7 +4,6 @@ class SnippetModelTestCase(TestCase): - def setUp(self): pass diff --git a/tests/test_plugins.py b/tests/test_plugins.py index 6e4e6767..58d92af9 100644 --- a/tests/test_plugins.py +++ b/tests/test_plugins.py @@ -5,7 +5,6 @@ class SnippetPluginsTestCase(CMSTestCase): - def setUp(self): self.language = "en" self.home = create_page( @@ -29,7 +28,9 @@ def tearDown(self): self.superuser.delete() def test_html_rendering(self): - request_url = self.page.get_absolute_url(self.language) + "?toolbar_off=true" + request_url = ( + self.page.get_absolute_url(self.language) + "?toolbar_off=true" + ) snippet = Snippet.objects.create( name="plugin_snippet", html="

Hello World

", @@ -52,7 +53,9 @@ def test_html_rendering(self): self.assertIn(b"

Hello World

", response.content) def test_failing_html_rendering(self): - request_url = self.page.get_absolute_url(self.language) + "?toolbar_off=true" + request_url = ( + self.page.get_absolute_url(self.language) + "?toolbar_off=true" + ) snippet = Snippet.objects.create( name="plugin_snippet", html="{% import weirdness %}", @@ -70,10 +73,14 @@ def test_failing_html_rendering(self): response = self.client.get(request_url) self.assertContains(response, "Invalid block tag on line 1") - self.assertContains(response, "Did you forget to register or load this tag?") + self.assertContains( + response, "Did you forget to register or load this tag?" + ) def test_template_rendering(self): - request_url = self.page.get_absolute_url(self.language) + "?toolbar_off=true" + request_url = ( + self.page.get_absolute_url(self.language) + "?toolbar_off=true" + ) template = "snippet.html" snippet = Snippet.objects.create( name="plugin_snippet", @@ -94,13 +101,22 @@ def test_template_rendering(self): with self.login_user_context(self.superuser): response = self.client.get(request_url) - self.assertNotIn("Template {} does not exist".format(template).encode(), response.content) - self.assertNotIn(b"context must be a dict rather than Context", response.content) - self.assertNotIn(b"context must be a dict rather than PluginContext", response.content) + self.assertNotIn( + f"Template {template} does not exist".encode(), response.content + ) + self.assertNotIn( + b"context must be a dict rather than Context", response.content + ) + self.assertNotIn( + b"context must be a dict rather than PluginContext", + response.content, + ) self.assertContains(response, "

Hello World Template

") def test_failing_template_rendering(self): - request_url = self.page.get_absolute_url(self.language) + "?toolbar_off=true" + request_url = ( + self.page.get_absolute_url(self.language) + "?toolbar_off=true" + ) template = "some_template" snippet = Snippet.objects.create( name="plugin_snippet", diff --git a/tests/test_templatetags.py b/tests/test_templatetags.py index b1bb45b5..344d74f9 100644 --- a/tests/test_templatetags.py +++ b/tests/test_templatetags.py @@ -7,7 +7,6 @@ class SnippetTemplateTagTestCase(TestCase): - def test_html_rendered(self): snippet = Snippet.objects.create( name="test snippet", @@ -20,17 +19,15 @@ def test_html_rendered(self): context = Context({"title": "world"}) template_to_render = Template( - '{% load snippet_tags %}' - '{% snippet_fragment "test_snippet" %}' + "{% load snippet_tags %}" '{% snippet_fragment "test_snippet" %}' ) rendered_template = template_to_render.render(context) - self.assertInHTML('

hello world

', rendered_template) + self.assertInHTML("

hello world

", rendered_template) # test html errors context = Context({"title": "world"}) template_to_render = Template( - '{% load snippet_tags %}' - '{% snippet_fragment "test_snippet_2" %}' + "{% load snippet_tags %}" '{% snippet_fragment "test_snippet_2" %}' ) with self.assertRaises(ObjectDoesNotExist): # Snippet matching query does not exist. @@ -50,29 +47,27 @@ def test_template_rendered(self): # use a string to identify context = Context({}) template_to_render = Template( - '{% load snippet_tags %}' - '{% snippet_fragment "test_snippet" %}' + "{% load snippet_tags %}" '{% snippet_fragment "test_snippet" %}' ) rendered_template = template_to_render.render(context) - self.assertInHTML('

Hello World Template

', rendered_template) + self.assertInHTML("

Hello World Template

", rendered_template) # use an id to identify context = Context({}) template_to_render = Template( - '{% load snippet_tags %}' - '{% snippet_fragment 1 %}' + "{% load snippet_tags %}" "{% snippet_fragment 1 %}" ) rendered_template = template_to_render.render(context) - self.assertInHTML('

Hello World Template

', rendered_template) + self.assertInHTML("

Hello World Template

", rendered_template) # tests "or" functionality context = Context({}) template_to_render = Template( - '{% load snippet_tags %}' + "{% load snippet_tags %}" '{% snippet_fragment "test_snippet_1" or %}

hello world

{% endsnippet_fragment %}' ) rendered_template = template_to_render.render(context) - self.assertInHTML('

hello world

', rendered_template) + self.assertInHTML("

hello world

", rendered_template) def test_template_errors(self): template = "does_not_exist.html" @@ -87,16 +82,16 @@ def test_template_errors(self): context = Context({}) template_to_render = Template( - '{% load snippet_tags %}' - '{% snippet_fragment "test_snippet" %}' + "{% load snippet_tags %}" '{% snippet_fragment "test_snippet" %}' ) rendered_template = template_to_render.render(context) - self.assertIn('Template does_not_exist.html does not exist.', rendered_template) + self.assertIn( + "Template does_not_exist.html does not exist.", rendered_template + ) context = Context({}) template_to_render = Template( - '{% load snippet_tags %}' - '{% snippet_fragment "test_snippet_1" %}' + "{% load snippet_tags %}" '{% snippet_fragment "test_snippet_1" %}' ) with self.assertRaises(ObjectDoesNotExist): # Snippet object does not exist @@ -106,6 +101,5 @@ def test_template_errors(self): with self.assertRaises(TemplateSyntaxError): # You need to specify at least a "snippet" ID, slug or instance template_to_render = Template( - '{% load snippet_tags %}' - '{% snippet_fragment %}' + "{% load snippet_tags %}" "{% snippet_fragment %}" ) From 436b13ed5c1d43440fce1cfabc646b116203a411 Mon Sep 17 00:00:00 2001 From: Mark Walker Date: Thu, 26 Oct 2023 13:51:01 +0100 Subject: [PATCH 3/4] docs: Update buttons --- README.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.rst b/README.rst index 73a61098..215adb29 100644 --- a/README.rst +++ b/README.rst @@ -123,11 +123,11 @@ You can run tests by executing:: .. |pypi| image:: https://badge.fury.io/py/djangocms-snippet.svg :target: http://badge.fury.io/py/djangocms-snippet -.. |coverage| image:: https://codecov.io/gh/django-cms/djangocms-snippet/branch/master/graph/badge.svg +.. |coverage| image:: https://codecov.io/gh/django-cms/djangocms-snippet/graph/badge.svg?token=Ztf8ph4OTk :target: https://codecov.io/gh/django-cms/djangocms-snippet -.. |python| image:: https://img.shields.io/badge/python-3.5+-blue.svg +.. |python| image:: https://img.shields.io/badge/python-3.9+-blue.svg :target: https://pypi.org/project/djangocms-snippet/ -.. |django| image:: https://img.shields.io/badge/django-2.2,%203.0,%203.1-blue.svg +.. |django| image:: https://img.shields.io/badge/django-3.2,%204.2-blue.svg :target: https://www.djangoproject.com/ -.. |djangocms| image:: https://img.shields.io/badge/django%20CMS-3.7%2B-blue.svg +.. |djangocms| image:: https://img.shields.io/badge/django%20CMS-3.10%2B-blue.svg :target: https://www.django-cms.org/ From dbf8a0af8a5fe1ad689f6a4870fe1aaf6910187b Mon Sep 17 00:00:00 2001 From: Mark Walker Date: Thu, 26 Oct 2023 19:29:43 +0100 Subject: [PATCH 4/4] test: Fix tests --- .gitignore | 1 + src/djangocms_snippet/templatetags/snippet_tags.py | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 560c3a09..9fa1d001 100644 --- a/.gitignore +++ b/.gitignore @@ -26,3 +26,4 @@ npm-debug.log package-lock.json local.sqlite +.coverage* diff --git a/src/djangocms_snippet/templatetags/snippet_tags.py b/src/djangocms_snippet/templatetags/snippet_tags.py index 5bb506b0..119b5685 100644 --- a/src/djangocms_snippet/templatetags/snippet_tags.py +++ b/src/djangocms_snippet/templatetags/snippet_tags.py @@ -69,6 +69,8 @@ def render(self, context: BaseContext) -> str: """ # Default assume this is directly an instance snippet_instance = self.snippet_id_varname.resolve(context) + + response = self.nodelist.render(context) # Assume this is slug with exceptionless(self.parse_until): if isinstance(snippet_instance, str): @@ -77,10 +79,12 @@ def render(self, context: BaseContext) -> str: elif isinstance(snippet_instance, int): # pragma: no cover snippet_instance = Snippet.objects.get(pk=snippet_instance) - return mark_safe( + response = mark_safe( self.get_content_render(context, snippet_instance) ) + return response + def get_content_render( self, context: BaseContext, instance: Snippet ) -> str: