From 4627d10e8471d8897dde32387128fcd9d1b07299 Mon Sep 17 00:00:00 2001
From: Jonathan Weth <git@jonathanweth.de>
Date: Mon, 28 Dec 2020 11:56:38 +0100
Subject: [PATCH] Load data checks from related models instead of using a
custom registry
---
aleksis/core/apps.py | 15 ++++++++-
aleksis/core/data_checks.py | 51 ++++++++++++++++++-------------
aleksis/core/models.py | 6 ++--
aleksis/core/util/core_helpers.py | 1 +
aleksis/core/views.py | 4 +--
5 files changed, 50 insertions(+), 27 deletions(-)
diff --git a/aleksis/core/apps.py b/aleksis/core/apps.py
index bdfe3e3cb..75feb14c3 100644
--- a/aleksis/core/apps.py
+++ b/aleksis/core/apps.py
@@ -1,6 +1,7 @@
from typing import Any, List, Optional, Tuple
import django.apps
+from django.apps import apps
from django.conf import settings
from django.db import OperationalError, ProgrammingError
from django.http import HttpRequest
@@ -41,7 +42,7 @@ class CoreConfig(AppConfig):
super().ready()
# Autodiscover various modules defined by AlekSIS
- autodiscover_modules("form_extensions", "model_extensions", "checks", "data_checks")
+ autodiscover_modules("form_extensions", "model_extensions", "checks")
sitepreferencemodel = self.get_model("SitePreferenceModel")
personpreferencemodel = self.get_model("PersonPreferenceModel")
@@ -53,10 +54,22 @@ class CoreConfig(AppConfig):
self._refresh_authentication_backends()
+ self._load_data_checks()
+
from .health_checks import DataChecksHealthCheckBackend
plugin_dir.register(DataChecksHealthCheckBackend)
+ @classmethod
+ def _load_data_checks(cls):
+ """Get all data checks from all loaded models."""
+ from aleksis.core.data_checks import DataCheckRegistry
+
+ data_checks = []
+ for model in apps.get_models():
+ data_checks += getattr(model, "data_checks", [])
+ DataCheckRegistry.data_checks = data_checks
+
@classmethod
def _refresh_authentication_backends(cls):
"""Refresh config list of enabled authentication backends."""
diff --git a/aleksis/core/data_checks.py b/aleksis/core/data_checks.py
index 192a5fc65..cb15f6671 100644
--- a/aleksis/core/data_checks.py
+++ b/aleksis/core/data_checks.py
@@ -2,6 +2,7 @@ import logging
from django.contrib.contenttypes.models import ContentType
from django.db.models.aggregates import Count
+from django.utils.decorators import classproperty
from django.utils.translation import gettext as _
import reversion
@@ -47,6 +48,7 @@ class SolveOption:
class IgnoreSolveOption(SolveOption):
"""Mark the object with data issues as solved."""
+
name = "ignore"
verbose_name = _("Ignore problem")
@@ -70,12 +72,14 @@ class DataCheck:
Example:
+ ``data_checks.py``
+ ******************
+
.. code-block:: python
from aleksis.core.data_checks import DataCheck, DATA_CHECK_REGISTRY
from django.utils.translation import gettext as _
- @DATA_CHECK_REGISTRY.register
class ExampleDataCheck(DataCheck):
name = "example" # has to be unique
verbose_name = _("Ensure that there are no examples.")
@@ -94,6 +98,19 @@ class DataCheck:
for example in wrong_examples:
cls.register_result(example)
+ ``models.py``
+ *************
+
+ .. code-block:: python
+
+ from .data_checks import ExampleDataCheck
+
+ # ...
+
+ class ExampleModel(Model):
+ data_checks = [ExampleDataCheck]
+
+
Solve options are used in order to give the data admin typical solutions to this specific issue.
They are defined by inheriting from SolveOption.
More information about defining solve options can be find there.
@@ -107,10 +124,9 @@ class DataCheck:
your code should find all objects with issues and should register
them in the result database using the class method ``register_result``.
- Data checks have to be registered in the central registry.
- This can be done by decorating the class with
- ``@DATA_CHECK_REGISTRY.register`` or adding it later
- by ``DATA_CHECK_REGISTRY.register(<YourCheck>DataCheck)``.
+ Data checks have to be registered in their corresponding model.
+ This can be done by adding a list ``data_checks``
+ containing the data check classes.
Executing data checks
---------------------
@@ -180,34 +196,27 @@ class DataCheck:
class DataCheckRegistry:
"""Create central registry for all data checks in AlekSIS."""
- def __init__(self):
- self.data_checks = []
- self.data_checks_by_name = {}
- self.data_checks_choices = []
+ data_checks = []
- def register(self, check: DataCheck):
- """Add a new data check to the registry."""
- self.data_checks.append(check)
- self.data_checks_by_name[check.name] = check
- self.data_checks_choices.append((check.name, check.verbose_name))
- return check
+ @classproperty
+ def data_checks_by_name(cls):
+ return {check.name: check for check in cls.data_checks}
-
-DATA_CHECK_REGISTRY = DataCheckRegistry()
+ @classproperty
+ def data_checks_choices(cls):
+ return [(check.name, check.verbose_name) for check in cls.data_checks]
@celery_optional
def check_data():
"""Execute all registered data checks and send email if activated."""
- for check in DATA_CHECK_REGISTRY.data_checks:
+ for check in DataCheckRegistry.data_checks:
logging.info(f"Run check: {check.verbose_name}")
check.check_data()
if get_site_preferences()["general__data_checks_send_emails"]:
send_emails_for_data_checks()
- return True
-
def send_emails_for_data_checks():
"""Notify one or more recipients about new problems with data.
@@ -224,7 +233,7 @@ def send_emails_for_data_checks():
results_with_checks = []
for result in results_by_check:
results_with_checks.append(
- (DATA_CHECK_REGISTRY.data_checks_by_name[result["check"]], result["count"])
+ (DataCheckRegistry.data_checks_by_name[result["check"]], result["count"])
)
recipient_list = [
diff --git a/aleksis/core/models.py b/aleksis/core/models.py
index f82ea02ce..a5fc33a70 100644
--- a/aleksis/core/models.py
+++ b/aleksis/core/models.py
@@ -26,7 +26,7 @@ from model_utils.models import TimeStampedModel
from phonenumber_field.modelfields import PhoneNumberField
from polymorphic.models import PolymorphicModel
-from aleksis.core.data_checks import DATA_CHECK_REGISTRY, DataCheck
+from aleksis.core.data_checks import DataCheck, DataCheckRegistry
from .managers import (
CurrentSiteManagerWithoutMigrations,
@@ -851,7 +851,7 @@ class DataCheckResult(ExtensibleModel):
check = models.CharField(
max_length=255,
verbose_name=_("Related data check task"),
- choices=DATA_CHECK_REGISTRY.data_checks_choices,
+ choices=DataCheckRegistry.data_checks_choices,
)
content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE)
@@ -863,7 +863,7 @@ class DataCheckResult(ExtensibleModel):
@property
def related_check(self) -> DataCheck:
- return DATA_CHECK_REGISTRY.data_checks_by_name[self.check]
+ return DataCheckRegistry.data_checks_by_name[self.check]
def solve(self, solve_option: str = "default"):
self.related_check.solve(self, solve_option)
diff --git a/aleksis/core/util/core_helpers.py b/aleksis/core/util/core_helpers.py
index bef457fee..b3df40881 100644
--- a/aleksis/core/util/core_helpers.py
+++ b/aleksis/core/util/core_helpers.py
@@ -206,6 +206,7 @@ def celery_optional(orig: Callable) -> Callable:
task.delay(*args, **kwargs)
else:
orig(*args, **kwargs)
+ return True
return wrapped
diff --git a/aleksis/core/views.py b/aleksis/core/views.py
index 6cdbba24b..0e29253fa 100644
--- a/aleksis/core/views.py
+++ b/aleksis/core/views.py
@@ -29,7 +29,7 @@ from reversion import set_user
from reversion.views import RevisionMixin
from rules.contrib.views import PermissionRequiredMixin, permission_required
-from aleksis.core.data_checks import DATA_CHECK_REGISTRY, check_data
+from aleksis.core.data_checks import DataCheckRegistry, check_data
from .filters import GroupFilter, PersonFilter
from .forms import (
@@ -720,7 +720,7 @@ class DataCheckView(PermissionRequiredMixin, ListView):
def get_context_data(self, **kwargs: Any) -> Dict[str, Any]:
context = super().get_context_data(**kwargs)
- context["registered_checks"] = DATA_CHECK_REGISTRY.data_checks
+ context["registered_checks"] = DataCheckRegistry.data_checks
return context
--
GitLab