From a79c90f08b8d0040c8282fa37c246e91722eb0e7 Mon Sep 17 00:00:00 2001 From: Abdelrahman Kandil Date: Tue, 16 Jan 2024 03:14:05 +0200 Subject: [PATCH 1/5] Add support for multiple users and groups --- .../0005_migrate_new_assigned_fields.py | 52 +++++++++++++++ django_datawatch/admin.py | 14 ++++- django_datawatch/base.py | 35 +++++++---- .../locale/de/LC_MESSAGES/django.mo | Bin 3124 -> 3289 bytes .../locale/de/LC_MESSAGES/django.po | 38 ++++++++--- ...signeduser_resultassignedgroup_and_more.py | 59 ++++++++++++++++++ .../0005_migrate_new_assigned_fields.py | 47 ++++++++++++++ ...emove_result_assigned_to_group_and_more.py | 21 +++++++ django_datawatch/models.py | 43 ++++++++++++- django_datawatch/querysets.py | 6 +- .../templates/django_datawatch/detail.html | 12 ++-- django_datawatch/tests/test_base_check.py | 39 ++++++++++++ 12 files changed, 332 insertions(+), 34 deletions(-) create mode 100644 django_datawatch/0005_migrate_new_assigned_fields.py create mode 100644 django_datawatch/migrations/0004_resultassigneduser_resultassignedgroup_and_more.py create mode 100644 django_datawatch/migrations/0005_migrate_new_assigned_fields.py create mode 100644 django_datawatch/migrations/0006_remove_result_assigned_to_group_and_more.py diff --git a/django_datawatch/0005_migrate_new_assigned_fields.py b/django_datawatch/0005_migrate_new_assigned_fields.py new file mode 100644 index 0000000..722c3a2 --- /dev/null +++ b/django_datawatch/0005_migrate_new_assigned_fields.py @@ -0,0 +1,52 @@ +# Generated by Django 4.2.9 on 2024-01-15 23:41 + +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') + ThroughResultAssignedGroups = Result.assigned_groups.through + ThroughResultAssignedUsers = Result.assigned_users.through + + for result in Result.objects.all(): + result.assigned_groups = result.assigned_groups.all() + result.assigned_users = result.assigned_users.all() + result.save(update_fields=['assigned_groups', 'assigned_users']) + + # Paginate the queryset to avoid memory issues + paginator = Paginator( + Result.objects.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(ThroughResultAssignedGroups( + result_id=result.pk, + group_id=result.assigned_to_group.pk, + )) + if result.assigned_to_user: + user_instances.append(ThroughResultAssignedUsers( + result_id=result.pk, + user_id=result.assigned_to_user.pk, + )) + + ThroughResultAssignedGroups.objects.bulk_create(group_instances) + ThroughResultAssignedUsers.objects.bulk_create(user_instances) + + +class Migration(migrations.Migration): + + dependencies = [ + ('django_datawatch', '0004_result_assigned_groups_result_assigned_users'), + ] + + operations = [ + migrations.RunPython(migrate_new_assigned_fields) + ] diff --git a/django_datawatch/admin.py b/django_datawatch/admin.py index cc4579a..c0e80a3 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', 'assigned_users') + inlines = [ResultAssignedGroupInline, ResultAssignedUserInline] @admin.register(CheckExecution) class CheckExecutionAdmin(admin.ModelAdmin): diff --git a/django_datawatch/base.py b/django_datawatch/base.py index 13fa706..6e32690 100644 --- a/django_datawatch/base.py +++ b/django_datawatch/base.py @@ -2,6 +2,7 @@ from contextlib import contextmanager from django import forms +from django.db import transaction from django.utils import timezone from django_datawatch.models import Result, CheckExecution, ResultStatusHistory @@ -24,7 +25,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 +60,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 +131,26 @@ 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): + assert len(groups) == len(set(g.pk for g in groups)), 'groups must be unique' + dataset.assigned_groups.set(groups) + + if users := self.get_assigned_users(payload, status): + assert len(users) == len(set(u.pk for u in users)), 'users must be unique' + dataset.assigned_users.set(users) return dataset @@ -179,10 +188,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): return None - def get_assigned_group(self, payload, result): + def get_assigned_groups(self, payload, result): 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 8a63145704d0301701fbbd340f17b8adabb67fb6..ff43f28115b53049fa66871eddde42bbb7ef7f4a 100644 GIT binary patch delta 1366 zcmXxkO-PhM9LMol+g;t=EVH$=)?77HOH0e_QiMtGU91S*+eT_jwF~=l$c6S02vH}W ziV!3S!b_l$V9==`sGvymqD0`yBnpHmIz)YcyW{Zu=kq+X^UTcipV6tx`KsKPa`!z$ zIY5MnIhQdf@y;45%B0(v_4pJ6_!2kbdo0IA`}LW2&=10)G%%KF0k3c1W%D)pbkb+~|9*fQ!c{y;6j#s0~1)B@^JTi1@&*nyho zY=Hc$Lmv$qU=X#kA=H9yVkJJr&G;Pk9koSToQkr*tNBUvOd`w<-B7j=`ug3( zPC`Z7p^{S?wE&e?LjMC)w9k79Wu=X1A$Ae_2&K^edoZ=!D}yRZc@v>St8-C9s3;@a zwq`;HZ?!ZOrYM}!Fs(|V(yX$d=peS~!4_qx(}}w0Tq&p96LIc)dIQIjiEH6hI-1U; zobR5j6ZM9I{i)2=v~3Juj-}FxG&$IRF&;~`7Mfd6?;9*opr%>&usk+&wbEvI(ME07|ELP$iYUMtm?tjNk z_}dyHJ0(zw=_XFvIne-vs69Q88h8vj%uRcJ2kFDipgNpIz5fn(<4@E|l#-2}t3ejm z)T6$I7G!SbxV10L{;OfYK6n=OaYRuGT|p(3LJfG!I*l6WDQat8VlB?0`kS}+7g6so zqY}trf45>SlB+pX%=+tvqcjAWp_Z@@D=>zwIEnrE81>>GbTF3>MKi5KZA~k(GwY>} zGp$e~K@P#HLhwOrWg^=DAS2X_Itcm>2G<_85&CWPd+Hm}s;IORnz@P+Qc)uMg;lh_ z{}(0KLTGhW_7hss^iC?xc&+Gz(Ms%FYd|fb%C1$`>t@cFaIm!6{~bgO7l($Ex(PI}Fr15KX)CjWJrIp%rsk%Sv_Ql6XiuZ4#Eg;2lWRWKECMx&lH?79gj z>0V5_-l!Y%mkP2kW(tqwR+^#Ih&vH?JvZSdoo+Xg8lU3*rJ|UBCOqbU4nL1L6RBj( Yb)uKXQ_;(DFYYGC{cQ19|6xhjKcFFff&c&j diff --git a/django_datawatch/locale/de/LC_MESSAGES/django.po b/django_datawatch/locale/de/LC_MESSAGES/django.po index 72c31e6..6ba2635 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 "Zugewiesener 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 "Gruppen" + +msgid "Result assigned group" +msgstr "Zugewiesene Gruppe" + +msgid "Result assigned groups" +msgstr "Zugewiesene Gruppen" + +msgid "Group must be unique across the result" +msgstr "Die Gruppe muss im gesamten Ergebnis eindeutig sein" + +msgid "Result assigned user" +msgstr "Zugewiesener Benutzer" + +msgid "Result assigned users" +msgstr "Zugewiesener Benutzern" + +msgid "User must be unique across the result" +msgstr "Der Benutzer muss im gesamten 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..5583c2b 100644 --- a/django_datawatch/querysets.py +++ b/django_datawatch/querysets.py @@ -9,8 +9,10 @@ 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_groups__in=user_groups) | Q(assigned_users=user), + ) 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..4684040 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,37 @@ 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()) + + def test_unique_groups(self): + group = Group.objects.create(name='group') + self.check.get_assigned_groups = mock.Mock(return_value=[group, group]) + with self.assertRaises(AssertionError): + self.check.save(self.fake_payload, Result.STATUS.ok) + + # No results should be created + self.assertEqual(Result.objects.count(), 0) + + 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]) + with self.assertRaises(AssertionError): + self.check.save(self.fake_payload, Result.STATUS.ok) + + # No results should be created + self.assertEqual(Result.objects.count(), 0) \ No newline at end of file From 8f034a2165e7b8ba2189e420e2b57c4be446a1c0 Mon Sep 17 00:00:00 2001 From: Abdelrahman Kandil Date: Tue, 16 Jan 2024 12:33:52 +0200 Subject: [PATCH 2/5] fix queryset filtering and minor refactoring --- .../0005_migrate_new_assigned_fields.py | 52 ------------------ django_datawatch/admin.py | 2 +- django_datawatch/base.py | 2 - .../locale/de/LC_MESSAGES/django.mo | Bin 3289 -> 3864 bytes .../locale/de/LC_MESSAGES/django.po | 10 ++-- django_datawatch/querysets.py | 3 +- django_datawatch/tests/test_base_check.py | 20 ++++--- django_datawatch/tests/test_post_delete.py | 19 +++---- django_datawatch/tests/test_querysets.py | 28 ++++++++++ 9 files changed, 57 insertions(+), 79 deletions(-) delete mode 100644 django_datawatch/0005_migrate_new_assigned_fields.py create mode 100644 django_datawatch/tests/test_querysets.py diff --git a/django_datawatch/0005_migrate_new_assigned_fields.py b/django_datawatch/0005_migrate_new_assigned_fields.py deleted file mode 100644 index 722c3a2..0000000 --- a/django_datawatch/0005_migrate_new_assigned_fields.py +++ /dev/null @@ -1,52 +0,0 @@ -# Generated by Django 4.2.9 on 2024-01-15 23:41 - -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') - ThroughResultAssignedGroups = Result.assigned_groups.through - ThroughResultAssignedUsers = Result.assigned_users.through - - for result in Result.objects.all(): - result.assigned_groups = result.assigned_groups.all() - result.assigned_users = result.assigned_users.all() - result.save(update_fields=['assigned_groups', 'assigned_users']) - - # Paginate the queryset to avoid memory issues - paginator = Paginator( - Result.objects.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(ThroughResultAssignedGroups( - result_id=result.pk, - group_id=result.assigned_to_group.pk, - )) - if result.assigned_to_user: - user_instances.append(ThroughResultAssignedUsers( - result_id=result.pk, - user_id=result.assigned_to_user.pk, - )) - - ThroughResultAssignedGroups.objects.bulk_create(group_instances) - ThroughResultAssignedUsers.objects.bulk_create(user_instances) - - -class Migration(migrations.Migration): - - dependencies = [ - ('django_datawatch', '0004_result_assigned_groups_result_assigned_users'), - ] - - operations = [ - migrations.RunPython(migrate_new_assigned_fields) - ] diff --git a/django_datawatch/admin.py b/django_datawatch/admin.py index c0e80a3..68d8f48 100644 --- a/django_datawatch/admin.py +++ b/django_datawatch/admin.py @@ -16,7 +16,7 @@ class CheckAdmin(admin.ModelAdmin): list_display = ('slug', 'identifier', 'status') readonly_fields = ('created', 'modified') search_fields = ('slug', 'identifier', 'payload_description') - list_filter = ('status', 'slug', 'assigned_groups', 'assigned_users') + list_filter = ('status', 'slug', 'assigned_groups') inlines = [ResultAssignedGroupInline, ResultAssignedUserInline] @admin.register(CheckExecution) diff --git a/django_datawatch/base.py b/django_datawatch/base.py index 6e32690..55cae59 100644 --- a/django_datawatch/base.py +++ b/django_datawatch/base.py @@ -145,11 +145,9 @@ def save(self, payload, status, data=None, unacknowledge=False): # set assigned users and groups if groups := self.get_assigned_groups(payload, status): - assert len(groups) == len(set(g.pk for g in groups)), 'groups must be unique' dataset.assigned_groups.set(groups) if users := self.get_assigned_users(payload, status): - assert len(users) == len(set(u.pk for u in users)), 'users must be unique' dataset.assigned_users.set(users) return dataset diff --git a/django_datawatch/locale/de/LC_MESSAGES/django.mo b/django_datawatch/locale/de/LC_MESSAGES/django.mo index ff43f28115b53049fa66871eddde42bbb7ef7f4a..a6aefbfaae365bde20829862a34606c972accab4 100644 GIT binary patch delta 1814 zcma*nZ)jC@9LMqR+#lWU%>8pY&3Vk`)@GTdWi1ouWFo0eT5~?T+YZ-jc{cC4N5P1j z`J^Y+gNs85l0AtKq0y=bBZ$%msgGoVj6eiOdk_Va34ssv{W<$>TqVdK_jg{u-|w8? zpY#3w&YfyF-z0`HVPy#y(r~HS3Os=cJcG6P8P?#ayZ^oOifjKJ z*KmIZo3XfzjBTj#Hse!fIonBrpB><`9D9&HHh|Tb#wPp_`Pryz{|5E^1TMqhQ2lP8 z7I@Ru|3ZE?>--n>A`h^X`7N>3Y$FZTsF&W36}ZFI_o61)kLvgyR$@Q$vmq`7%}${v zKJD5+caESI`V}&#UBZ<(j`hrMS1HutZFl1yHc(G6s~&8^a%@K}a1UyreaNHMje5Qh zHBN|I@KbyhCs6%oQP1B;jay7(P6IYjP>35*4|HNjk=fg*z50T1C?XfQ)Z<0e*8GCH zKZ#X%$2o^uP$}VR!F8y4_M#%RAGOeAg7`Csouol4`WRW9ji3g+fEr*76~ZZe7H3?& zf_>2Qt5AEt9(CxNQ4!kh+~@2@)@FxYeV~%~x6*Ko1`TiyIhFP$YQ+~(r~F&gf_`;Q zq82ob+WWh>7Vn`ZuH~lgujZouEvSXIyLu-o61_PJdh_>DAvuG3gArVX-{I4E9TllN zs0sdd&Y`yCKB|8WtKWpp*p2U^`hD#jLy!6dYC*X_D6~uh5+w}btxp?m&PA@s%M1^u8 z>621gKTmvZ;i@k{Y4tpDG+i-1OWH=A9c>TiHr`&HEo*8SOy++mxwd>)7^YK!Kj=lF zpUHnz`tzo)%uqCJ@zpyXg;}rP_o5(uBJ#cDKxQZmz3frn%lKjRK{nrBcJ`g*!Vsyr z=b`wH_h@Tqk2h<+|7;A$e=MsjYU<(s|IR;tb!p?0x+77_Kb7`FKk&T{KZvrYd6hrQ z|Jl-&iH3)L-bDo7@ykDjJmekoy*-(f-yfty&rb(~ew0n8ypUpkvSO^daJaZ@{ziFy hYe8#faTY6RU2GixByq54>%u~wU{>o*W`Zb4*+U>2_@w{< delta 1255 zcmXxkPe_zO7{~EvxBaKPmTjwL`QNtHvRrG+4qdDuiV*ZtcI%=_yXhj7I)xBOyZjR{HHS%F14p{51(f+gQrkKUPX;}9V2F* zdE8Xca38ts5!H5_#56ufI$FWCub}#W#~A)bE(_903yYziYfa(rMilj$}x@Am_=py40`I& z&rKy>aSo#v@&H@$8ES%0sBgZ6T((Z71#F@+7iAWWQ;B-736+s^*o+t5^9ZW{sPeD9 z8lypnWdccpy>z~IzC$j1Po@6LxD{7X3;TvTj6YBd2(W)LiCRDlYU?_&4s)n^`r_nY z9r|g|0E4KN4Wkxx4|m}c+=;JH-~2ObfK}%jDz#rxTlW{+F~tGu#ctGlzH Date: Tue, 16 Jan 2024 15:34:44 +0200 Subject: [PATCH 3/5] Fixed queryset filter and added extensive tests --- django_datawatch/querysets.py | 7 +- django_datawatch/tests/test_querysets.py | 112 +++++++++++++++++++---- 2 files changed, 99 insertions(+), 20 deletions(-) diff --git a/django_datawatch/querysets.py b/django_datawatch/querysets.py index bd6423e..1eb9bd8 100644 --- a/django_datawatch/querysets.py +++ b/django_datawatch/querysets.py @@ -11,9 +11,10 @@ class ResultQuerySet(models.QuerySet): def for_user(self, user): user_groups = user.groups.all() return self.filter( - (Q(assigned_groups__in=user_groups) | Q(assigned_users=user)) - | (Q(assigned_groups__isnull=True) & Q(assigned_users__isnull=True)), - ) + 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/tests/test_querysets.py b/django_datawatch/tests/test_querysets.py index f637aec..9a08d64 100644 --- a/django_datawatch/tests/test_querysets.py +++ b/django_datawatch/tests/test_querysets.py @@ -1,3 +1,4 @@ +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 @@ -6,23 +7,100 @@ User = get_user_model() + class ResultQuerySetTestCase(TestCase): - def setUp(self): - self.user = User.objects.create_user(**{User.USERNAME_FIELD: 'test_user'}) - self.group = Group.objects.create(name='test_group') + 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): - result_without_users_and_groups = Result.objects.create(slug='test_slug', identifier='test_identifier') - result_with = Result.objects.create(slug='test_slug2', identifier='test_identifier') - result_with.assigned_users.add(self.user) - result_with.assigned_groups.add(self.group) - - # User should have access to both results - queryset = Result.objects.for_user(self.user) - self.assertEqual(queryset.count(), 2) - - # User should have access to only one result - other_user = User.objects.create_user(**{User.USERNAME_FIELD: 'test_user2'}) - queryset = Result.objects.for_user(other_user) - self.assertEqual(queryset.count(), 1) - self.assertEqual(queryset.get(), result_without_users_and_groups) + 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, + ], + ) From 0072d8fccd131bd62cea0d9f69e3cfc254ad8686 Mon Sep 17 00:00:00 2001 From: Abdelrahman Kandil Date: Mon, 22 Jan 2024 12:14:06 +0200 Subject: [PATCH 4/5] Added missing type hints and clearing of users and groups --- django_datawatch/base.py | 10 ++++++++-- django_datawatch/tests/test_base_check.py | 10 +++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/django_datawatch/base.py b/django_datawatch/base.py index 55cae59..3b74840 100644 --- a/django_datawatch/base.py +++ b/django_datawatch/base.py @@ -1,7 +1,9 @@ import logging from contextlib import contextmanager +from typing import Optional from django import forms +from django.contrib.auth.models import AbstractUser, Group from django.db import transaction from django.utils import timezone @@ -146,9 +148,13 @@ def save(self, payload, status, data=None, unacknowledge=False): # 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 @@ -186,10 +192,10 @@ def get_payload_description(self, payload): def format_result_data(self, result): return '' - def get_assigned_users(self, payload, result): + def get_assigned_users(self, payload, result) -> Optional[list[AbstractUser]]: return None - def get_assigned_groups(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/tests/test_base_check.py b/django_datawatch/tests/test_base_check.py index c6176ff..9b7c291 100644 --- a/django_datawatch/tests/test_base_check.py +++ b/django_datawatch/tests/test_base_check.py @@ -76,6 +76,14 @@ def test_save_sets_groups_and_users(self): 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]) @@ -96,4 +104,4 @@ def test_unique_users(self): # 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) \ No newline at end of file + self.assertEqual(result.assigned_users.first(), user) From b29958c1863a89d831ec92d5a79453eeaf0628c4 Mon Sep 17 00:00:00 2001 From: Abdelrahman Kandil Date: Mon, 22 Jan 2024 12:21:47 +0200 Subject: [PATCH 5/5] Fix python3.8 compatibility --- django_datawatch/base.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/django_datawatch/base.py b/django_datawatch/base.py index 3b74840..79baef8 100644 --- a/django_datawatch/base.py +++ b/django_datawatch/base.py @@ -1,6 +1,6 @@ import logging from contextlib import contextmanager -from typing import Optional +from typing import Optional, List from django import forms from django.contrib.auth.models import AbstractUser, Group @@ -192,10 +192,10 @@ def get_payload_description(self, payload): def format_result_data(self, result): return '' - def get_assigned_users(self, payload, result) -> Optional[list[AbstractUser]]: + def get_assigned_users(self, payload, result) -> Optional[List[AbstractUser]]: return None - def get_assigned_groups(self, payload, result) -> Optional[list[Group]]: + def get_assigned_groups(self, payload, result) -> Optional[List[Group]]: return None def get_context_data(self, result):