diff --git a/django_datawatch/admin.py b/django_datawatch/admin.py index cc4579a..68d8f48 100644 --- a/django_datawatch/admin.py +++ b/django_datawatch/admin.py @@ -1,15 +1,23 @@ from django.contrib import admin -from django_datawatch.models import Result, CheckExecution, ResultStatusHistory +from django_datawatch.models import Result, CheckExecution, ResultAssignedGroup, ResultAssignedUser, ResultStatusHistory +class ResultAssignedGroupInline(admin.TabularInline): + model = ResultAssignedGroup + extra = 0 + +class ResultAssignedUserInline(admin.TabularInline): + model = ResultAssignedUser + extra = 0 + @admin.register(Result) class CheckAdmin(admin.ModelAdmin): list_display = ('slug', 'identifier', 'status') readonly_fields = ('created', 'modified') search_fields = ('slug', 'identifier', 'payload_description') - list_filter = ('status', 'slug', 'assigned_to_group') - + list_filter = ('status', 'slug', 'assigned_groups') + inlines = [ResultAssignedGroupInline, ResultAssignedUserInline] @admin.register(CheckExecution) class CheckExecutionAdmin(admin.ModelAdmin): diff --git a/django_datawatch/base.py b/django_datawatch/base.py index 13fa706..79baef8 100644 --- a/django_datawatch/base.py +++ b/django_datawatch/base.py @@ -1,7 +1,10 @@ import logging from contextlib import contextmanager +from typing import Optional, List from django import forms +from django.contrib.auth.models import AbstractUser, Group +from django.db import transaction from django.utils import timezone from django_datawatch.models import Result, CheckExecution, ResultStatusHistory @@ -24,7 +27,6 @@ def track_status_history(slug, identifier, new_status): class DatawatchCheckSkipException(Exception): pass - class BaseCheckForm(forms.Form): def save(self, instance): instance.config = self.cleaned_data @@ -60,8 +62,8 @@ class BaseCheck(object): Any check should inherits from `BaseCheck` and should implements `.generate(self)` and `.check(self, payload)` methods. - Optionally, you can implements `.get_assigned_user(self, payload)` (resp. `.get_assigned_group(self, payload)`) - to define to which user (resp. group) the system had to assign the check result. + Optionally, you can implements `.get_assigned_users(self, payload)` (resp. `.get_assigned_groups(self, payload)`) + to define to which user(s) (resp. group(s)) the system had to assign the check result. """ config_form = None @@ -131,17 +133,28 @@ def get_form_class(self): def save(self, payload, status, data=None, unacknowledge=False): # build default data - defaults = dict(status=status, data=data, assigned_to_user=self.get_assigned_user(payload, status), - assigned_to_group=self.get_assigned_group(payload, status), - payload_description=self.get_payload_description(payload)) + defaults = dict(status=status, data=data, payload_description=self.get_payload_description(payload)) + if unacknowledge: defaults.update(dict(acknowledged_by=None, acknowledged_at=None, acknowledged_until=None)) - with track_status_history(self.slug, self.get_identifier(payload), status): - # save the check - dataset, created = Result.objects.update_or_create( - slug=self.slug, identifier=self.get_identifier(payload), - defaults=defaults) + with transaction.atomic(): + with track_status_history(self.slug, self.get_identifier(payload), status): + # save the check + dataset, created = Result.objects.update_or_create( + slug=self.slug, identifier=self.get_identifier(payload), + defaults=defaults) + + # set assigned users and groups + if groups := self.get_assigned_groups(payload, status): + dataset.assigned_groups.set(groups) + else: + dataset.assigned_groups.clear() + + if users := self.get_assigned_users(payload, status): + dataset.assigned_users.set(users) + else: + dataset.assigned_users.clear() return dataset @@ -179,10 +192,10 @@ def get_payload_description(self, payload): def format_result_data(self, result): return '' - def get_assigned_user(self, payload, result): + def get_assigned_users(self, payload, result) -> Optional[List[AbstractUser]]: return None - def get_assigned_group(self, payload, result): + def get_assigned_groups(self, payload, result) -> Optional[List[Group]]: return None def get_context_data(self, result): diff --git a/django_datawatch/locale/de/LC_MESSAGES/django.mo b/django_datawatch/locale/de/LC_MESSAGES/django.mo index 8a63145..a6aefbf 100644 Binary files a/django_datawatch/locale/de/LC_MESSAGES/django.mo and b/django_datawatch/locale/de/LC_MESSAGES/django.mo differ diff --git a/django_datawatch/locale/de/LC_MESSAGES/django.po b/django_datawatch/locale/de/LC_MESSAGES/django.po index 72c31e6..da7fa20 100644 --- a/django_datawatch/locale/de/LC_MESSAGES/django.po +++ b/django_datawatch/locale/de/LC_MESSAGES/django.po @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: PACKAGE VERSION\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2021-09-14 10:02+0000\n" +"POT-Creation-Date: 2024-01-16 01:07+0000\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: FULL NAME \n" "Language-Team: LANGUAGE \n" @@ -72,6 +72,12 @@ msgstr "Ausgeblendet bis" msgid "Acknowledge reason" msgstr "Ausblendungsgrund" +msgid "Assigned users" +msgstr "Zugewiesene Benutzer" + +msgid "Assigned groups" +msgstr "Zugewiesene Gruppen" + msgid "Result" msgstr "Ergebnis" @@ -84,6 +90,27 @@ msgstr "Zu Status" msgid "Result status history" msgstr "Ergebnis-Status-Historie" +msgid "Group" +msgstr "Gruppe" + +msgid "Result assigned group" +msgstr "Zugewiesene Gruppe" + +msgid "Result assigned groups" +msgstr "Zugewiesene Gruppen" + +msgid "Group must be unique across the result" +msgstr "Gruppen müssen je Ergebnis eindeutig sein" + +msgid "Result assigned user" +msgstr "Zugewiesener Benutzer" + +msgid "Result assigned users" +msgstr "Zugewiesene Benutzer" + +msgid "User must be unique across the result" +msgstr "Benutzer müssen je Ergebnis eindeutig sein" + msgid "Check module slug" msgstr "Modul-Bezeichner" @@ -136,6 +163,9 @@ msgstr "Nicht zugewiesen" msgid "Config form class" msgstr "Formular-Klasse für Konfiguration" +msgid "Config" +msgstr "Konfiguration" + msgid "Run every" msgstr "Periodisches Ausführen" @@ -152,12 +182,6 @@ msgstr "Maximale Ausblendezeit" msgid "%(days)s day(s)" msgstr "%(days)s Tag(e)" -msgid "Assigned group" -msgstr "Zugewiesene Gruppe" - -msgid "Assigned user" -msgstr "Zugewiesener Benutzer" - #, python-format msgid "Acknowledged until %(date)s by %(user)s" msgstr "Ausgeblendet bis %(date)s durch %(user)s" diff --git a/django_datawatch/migrations/0004_resultassigneduser_resultassignedgroup_and_more.py b/django_datawatch/migrations/0004_resultassigneduser_resultassignedgroup_and_more.py new file mode 100644 index 0000000..a8be63b --- /dev/null +++ b/django_datawatch/migrations/0004_resultassigneduser_resultassignedgroup_and_more.py @@ -0,0 +1,59 @@ +# Generated by Django 4.2.9 on 2024-01-16 00:05 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('auth', '0012_alter_user_first_name_max_length'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('django_datawatch', '0003_resultstatushistory'), + ] + + operations = [ + migrations.CreateModel( + name='ResultAssignedUser', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('result', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='django_datawatch.result', verbose_name='Result')), + ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL, verbose_name='User')), + ], + options={ + 'verbose_name': 'Result assigned user', + 'verbose_name_plural': 'Result assigned users', + }, + ), + migrations.CreateModel( + name='ResultAssignedGroup', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('group', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='auth.group', verbose_name='Group')), + ('result', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='django_datawatch.result', verbose_name='Result')), + ], + options={ + 'verbose_name': 'Result assigned group', + 'verbose_name_plural': 'Result assigned groups', + }, + ), + migrations.AddField( + model_name='result', + name='assigned_groups', + field=models.ManyToManyField(blank=True, related_name='assigned_groups', through='django_datawatch.ResultAssignedGroup', to='auth.group', verbose_name='Assigned groups'), + ), + migrations.AddField( + model_name='result', + name='assigned_users', + field=models.ManyToManyField(blank=True, related_name='assigned_results', through='django_datawatch.ResultAssignedUser', to=settings.AUTH_USER_MODEL, verbose_name='Assigned users'), + ), + migrations.AddConstraint( + model_name='resultassigneduser', + constraint=models.UniqueConstraint(fields=('result', 'user'), name='unique_result_assigned_user'), + ), + migrations.AddConstraint( + model_name='resultassignedgroup', + constraint=models.UniqueConstraint(fields=('result', 'group'), name='unique_result_assigned_group'), + ), + ] diff --git a/django_datawatch/migrations/0005_migrate_new_assigned_fields.py b/django_datawatch/migrations/0005_migrate_new_assigned_fields.py new file mode 100644 index 0000000..39d5c41 --- /dev/null +++ b/django_datawatch/migrations/0005_migrate_new_assigned_fields.py @@ -0,0 +1,47 @@ +# Generated by Django 4.2.9 on 2024-01-16 00:06 + +from django.db import migrations + +from django.core.paginator import Paginator + +def migrate_new_assigned_fields(apps, schema_editor): + Result = apps.get_model('django_datawatch', 'Result') + ResultAssignedGroup = apps.get_model('django_datawatch', 'ResultAssignedGroup') + ResultAssignedUser = apps.get_model('django_datawatch', 'ResultAssignedUser') + + # Paginate the queryset to avoid memory issues + paginator = Paginator( + Result.objects.order_by('pk').only('assigned_to_user', 'assigned_to_group'), + 5000, + ) + + for page_number in paginator.page_range: + page = paginator.page(page_number) + group_instances = [] + user_instances = [] + + for result in page.object_list: + if result.assigned_to_group: + group_instances.append(ResultAssignedGroup( + result_id=result.pk, + group_id=result.assigned_to_group.pk, + )) + if result.assigned_to_user: + user_instances.append(ResultAssignedUser( + result_id=result.pk, + user_id=result.assigned_to_user.pk, + )) + + ResultAssignedGroup.objects.bulk_create(group_instances) + ResultAssignedUser.objects.bulk_create(user_instances) + + +class Migration(migrations.Migration): + + dependencies = [ + ('django_datawatch', '0004_resultassigneduser_resultassignedgroup_and_more'), + ] + + operations = [ + migrations.RunPython(migrate_new_assigned_fields), + ] diff --git a/django_datawatch/migrations/0006_remove_result_assigned_to_group_and_more.py b/django_datawatch/migrations/0006_remove_result_assigned_to_group_and_more.py new file mode 100644 index 0000000..a0e1100 --- /dev/null +++ b/django_datawatch/migrations/0006_remove_result_assigned_to_group_and_more.py @@ -0,0 +1,21 @@ +# Generated by Django 4.2.9 on 2024-01-16 00:07 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('django_datawatch', '0005_migrate_new_assigned_fields'), + ] + + operations = [ + migrations.RemoveField( + model_name='result', + name='assigned_to_group', + ), + migrations.RemoveField( + model_name='result', + name='assigned_to_user', + ), + ] diff --git a/django_datawatch/models.py b/django_datawatch/models.py index d75756f..96214ac 100644 --- a/django_datawatch/models.py +++ b/django_datawatch/models.py @@ -4,6 +4,7 @@ from django.utils import timezone from django.conf import settings from django.db import models +from django.core.exceptions import ValidationError from django.utils.functional import cached_property from django.utils.translation import gettext_lazy as _ from django_extensions.db.fields.json import JSONField @@ -42,9 +43,9 @@ class Result(TimeStampedModel): acknowledged_until = models.DateTimeField(null=True, blank=True, verbose_name=_('Acknowledged until')) acknowledged_reason = models.TextField(blank=True, verbose_name=_('Acknowledge reason')) - assigned_to_user = models.ForeignKey(to=settings.AUTH_USER_MODEL, null=True, blank=True, - related_name='assigned_to_user', on_delete=models.SET_NULL) - assigned_to_group = models.ForeignKey(to='auth.Group', null=True, blank=True, on_delete=models.SET_NULL) + assigned_users = models.ManyToManyField(to=settings.AUTH_USER_MODEL, through='ResultAssignedUser', related_name='assigned_results', blank=True, verbose_name=_('Assigned users')) + assigned_groups = models.ManyToManyField(to='auth.Group', through='ResultAssignedGroup', related_name='assigned_groups', blank=True, + verbose_name=_('Assigned groups')) objects = ResultQuerySet.as_manager() @@ -116,6 +117,42 @@ class Meta: verbose_name_plural = _('Result status history') +class ResultAssignedGroup(models.Model): + result = models.ForeignKey(Result, on_delete=models.CASCADE, verbose_name=_('Result')) + group = models.ForeignKey(to='auth.Group', verbose_name=_('Group'), on_delete=models.CASCADE) + + class Meta: + verbose_name = _('Result assigned group') + verbose_name_plural = _('Result assigned groups') + constraints = [models.UniqueConstraint(fields=['result', 'group'], name='unique_result_assigned_group')] + + def validate_unique(self, exclude=None): + if ( + ResultAssignedGroup.objects.filter(result_id=self.result_id, group_id=self.group_id) + .exclude(pk=self.pk) + .exists() + ): + raise ValidationError({"group": _("Group must be unique across the result")}) + super().validate_unique(exclude) + +class ResultAssignedUser(models.Model): + result = models.ForeignKey(Result, on_delete=models.CASCADE, verbose_name=_('Result')) + user = models.ForeignKey(to=settings.AUTH_USER_MODEL, verbose_name=_('User'), on_delete=models.CASCADE) + + class Meta: + verbose_name = _('Result assigned user') + verbose_name_plural = _('Result assigned users') + constraints = [models.UniqueConstraint(fields=['result', 'user'], name='unique_result_assigned_user')] + + def validate_unique(self, exclude=None): + if ( + ResultAssignedUser.objects.filter(result_id=self.result_id, user_id=self.user_id) + .exclude(pk=self.pk) + .exists() + ): + raise ValidationError({"user": _("User must be unique across the result")}) + super().validate_unique(exclude) + class CheckExecution(models.Model): slug = models.TextField(verbose_name=_('Check module slug'), unique=True) last_run = models.DateTimeField(verbose_name=_('Last run')) diff --git a/django_datawatch/querysets.py b/django_datawatch/querysets.py index 8af5ed3..1eb9bd8 100644 --- a/django_datawatch/querysets.py +++ b/django_datawatch/querysets.py @@ -9,8 +9,12 @@ class ResultQuerySet(models.QuerySet): def for_user(self, user): - return self.filter(Q(assigned_to_group__isnull=True) | Q(assigned_to_group__in=user.groups.all()), - Q(assigned_to_user__isnull=True) | Q(assigned_to_user=user)) + user_groups = user.groups.all() + return self.filter( + Q(assigned_users=user) + | Q(assigned_groups__in=user_groups) + | Q(assigned_users__isnull=True, assigned_groups__isnull=True) + ).distinct() def failed(self): return self.exclude(status__in=(self.model.STATUS.unknown, self.model.STATUS.ok)) diff --git a/django_datawatch/templates/django_datawatch/detail.html b/django_datawatch/templates/django_datawatch/detail.html index a739450..90c84bb 100644 --- a/django_datawatch/templates/django_datawatch/detail.html +++ b/django_datawatch/templates/django_datawatch/detail.html @@ -98,20 +98,20 @@

{% trans "Superuser debug info" %}

{% endwith %} - {% trans "Assigned group" %} + {% trans "Assigned groups" %} - {% if result.assigned_to_group %} - {{ result.assigned_to_group }} + {% if result.assigned_groups.all %} + {{ result.assigned_groups.all|join:', ' }} {% else %} {% trans "Not set" %} {% endif %} - {% trans "Assigned user" %} + {% trans "Assigned users" %} - {% if result.assigned_to_user %} - {{ result.assigned_to_user }} + {% if result.assigned_users.all %} + {{ result.assigned_users.all|join:', ' }} {% else %} {% trans "Not set" %} {% endif %} diff --git a/django_datawatch/tests/test_base_check.py b/django_datawatch/tests/test_base_check.py index 0074153..9b7c291 100644 --- a/django_datawatch/tests/test_base_check.py +++ b/django_datawatch/tests/test_base_check.py @@ -1,8 +1,13 @@ +from unittest import mock + +from django.contrib.auth import get_user_model +from django.contrib.auth.models import Group from django.test.testcases import TestCase from django_datawatch.base import BaseCheck from django_datawatch.models import Result, ResultStatusHistory +User = get_user_model() class BaseCheckTestCase(TestCase): def setUp(self) -> None: @@ -54,3 +59,49 @@ def test_save_tracks_different_status_changes(self): latest_history = history_qs.latest('created') self.assertEqual(latest_history.to_status, result2.status) self.assertEqual(latest_history.to_status, result2.status) + + def test_save_sets_groups_and_users(self): + groups = [Group.objects.create(name=f'group_{i}') for i in range(3)] + users = [User.objects.create_user(**{User.USERNAME_FIELD: f'user_{i}'}) for i in range(3)] + + self.check.get_assigned_groups = mock.Mock(return_value=groups) + self.check.get_assigned_users = mock.Mock(return_value=users) + + result = self.check.save(self.fake_payload, Result.STATUS.ok) + + self.assertEqual(result.assigned_groups.count(), 3) + for group in groups: + self.assertTrue(group in result.assigned_groups.all()) + self.assertEqual(result.assigned_users.count(), 3) + for user in users: + self.assertTrue(user in result.assigned_users.all()) + + # Update with no assigned users/groups + self.check.get_assigned_groups = mock.Mock(return_value=None) + self.check.get_assigned_users = mock.Mock(return_value=None) + result = self.check.save(self.fake_payload, Result.STATUS.ok) + + self.assertEqual(result.assigned_groups.count(), 0) + self.assertEqual(result.assigned_users.count(), 0) + + def test_unique_groups(self): + group = Group.objects.create(name='group') + self.check.get_assigned_groups = mock.Mock(return_value=[group, group]) + + result = self.check.save(self.fake_payload, Result.STATUS.ok) + + # Result should be created with only one group + self.assertEqual(Result.objects.count(), 1) + self.assertEqual(result.assigned_groups.count(), 1) + self.assertEqual(result.assigned_groups.first(), group) + + def test_unique_users(self): + user = User.objects.create_user(**{User.USERNAME_FIELD: 'test_user'}) + self.check.get_assigned_users = mock.Mock(return_value=[user, user]) + + result = self.check.save(self.fake_payload, Result.STATUS.ok) + + # Result should be created with only one user + self.assertEqual(Result.objects.count(), 1) + self.assertEqual(result.assigned_users.count(), 1) + self.assertEqual(result.assigned_users.first(), user) diff --git a/django_datawatch/tests/test_post_delete.py b/django_datawatch/tests/test_post_delete.py index d24faf3..7f86311 100644 --- a/django_datawatch/tests/test_post_delete.py +++ b/django_datawatch/tests/test_post_delete.py @@ -39,13 +39,12 @@ def test_update_related_calls_backend(self, mock_get_checks_for_model): mock_get_checks_for_model.return_value = [CheckPostDelete] # mock the manager - manager = mock.Mock(spec=ResultQuerySet) - Result.objects = manager - manager.using.return_value = manager - manager_filtered = mock.Mock(spec=ResultQuerySet) - manager.filter.return_value = manager_filtered - - # test if delete has been called - datawatch.delete_results(sender=Result, instance=Result(pk=1)) - manager.filter.assert_called_with(slug=CheckPostDelete().slug, identifier=1) - manager_filtered.delete.assert_called_with() + with mock.patch.object(Result, "objects", spec=Result.objects) as mock_manager: + mock_manager.using.return_value = mock_manager + manager_filtered = mock.Mock(spec=ResultQuerySet) + mock_manager.filter.return_value = manager_filtered + + # test if delete has been called + datawatch.delete_results(sender=Result, instance=Result(pk=1)) + mock_manager.filter.assert_called_with(slug=CheckPostDelete().slug, identifier=1) + manager_filtered.delete.assert_called_with() diff --git a/django_datawatch/tests/test_querysets.py b/django_datawatch/tests/test_querysets.py new file mode 100644 index 0000000..9a08d64 --- /dev/null +++ b/django_datawatch/tests/test_querysets.py @@ -0,0 +1,106 @@ +from typing import Optional +from django.contrib.auth import get_user_model +from django.contrib.auth.models import Group +from django.test.testcases import TestCase + +from django_datawatch.models import Result + +User = get_user_model() + + +class ResultQuerySetTestCase(TestCase): + def _make_users(self, usernames: list): + users = [] + for username in usernames: + users.append(User(**{User.USERNAME_FIELD: username})) + return User.objects.bulk_create(users) + + def _make_groups(self, names: list): + groups = [] + for name in names: + groups.append(Group(name=name)) + return Group.objects.bulk_create(groups) + + def _make_result( + self, slug, users: Optional[list] = None, groups: Optional[list] = None + ): + result = Result.objects.create(slug=slug, identifier="test_identifier") + if users: + result.assigned_users.set(users) + if groups: + result.assigned_groups.set(groups) + return result + + def _assert_for_user_results(self, user, expected_results): + queryset = Result.objects.for_user(user).order_by("pk") + self.assertEqual(queryset.count(), len(expected_results)) + self.assertEqual(list(queryset), sorted(expected_results, key=lambda x: x.pk)) + + def test_for_user(self): + groups = self._make_groups(["g1", "g2", "g3"]) + users = self._make_users(["u1", "u2", "u3"]) + + group_with_user = groups[1] + group_without_users = groups[2] + group_with_users = groups[0] + + user_with_group = users[0] + user_without_group = users[1] + user_with_groups = users[2] + + user_with_group.groups.set([group_with_users]) + user_with_groups.groups.set([group_with_users, group_with_user]) + + # No group and no user (u1, u2, u3) + res_no_user_no_group = self._make_result("test1") + # One group but no user (u3) + res_with_group_no_user = self._make_result("test2", groups=[group_with_user]) + # One user but no group (u1) + res_with_user_no_group = self._make_result("test3", users=[user_with_group]) + # Multiple groups but no user (u3) + res_with_groups_no_user = self._make_result( + "test4", groups=[group_with_user, group_without_users] + ) + # Multiple users but no groups (u1, u3) + res_with_users_no_group = self._make_result( + "test5", users=[user_with_group, user_with_groups] + ) + # One group and one user (u2, u3) + res_with_group_and_user = self._make_result( + "test6", users=[user_without_group], groups=[group_with_user] + ) + # Multiple groups and one user (u2, u3) + res_with_groups_and_user = self._make_result( + "test7", + users=[user_without_group], + groups=[group_with_user, group_without_users], + ) + # One group and multiple users (u1, u3) + res_with_group_and_users = self._make_result( + "test8", users=[user_with_group, user_with_groups], groups=[group_with_user] + ) + self._assert_for_user_results( + user_with_group, + [ + res_no_user_no_group, + res_with_user_no_group, + res_with_users_no_group, + res_with_group_and_users, + ], + ) + self._assert_for_user_results( + user_without_group, + [res_no_user_no_group, res_with_group_and_user, res_with_groups_and_user], + ) + self._assert_for_user_results( + user_with_groups, + [ + res_no_user_no_group, + res_with_group_no_user, + res_with_groups_no_user, + res_with_users_no_group, + res_with_group_and_user, + res_with_groups_and_user, + res_with_group_and_users, + ], + )