From 8c660e9856f737e908a087ef34eb7fbc4b454cc5 Mon Sep 17 00:00:00 2001 From: imperosol Date: Wed, 20 Nov 2024 17:10:57 +0100 Subject: [PATCH] Make `core.User` inherit from `AbstractUser` instead of `AbstractBaseUser` --- accounting/tests.py | 13 ++- accounting/views.py | 20 ++--- club/models.py | 27 +++--- com/models.py | 26 +++--- com/tests.py | 6 +- com/views.py | 52 ++++++----- core/management/commands/populate.py | 10 +-- core/management/commands/populate_more.py | 6 +- ..._options_user_user_permissions_and_more.py | 82 ++++++++++++++++++ core/models.py | 86 ++++--------------- core/tests/test_core.py | 4 +- core/tests/test_files.py | 8 +- core/views/files.py | 26 +++--- core/views/forms.py | 4 +- core/views/group.py | 4 +- pedagogy/tests/test_api.py | 6 +- pedagogy/views.py | 27 +++--- rootplace/tests.py | 8 +- sas/tests/test_api.py | 6 +- sas/tests/test_views.py | 6 +- 20 files changed, 215 insertions(+), 212 deletions(-) create mode 100644 core/migrations/0040_alter_user_options_user_user_permissions_and_more.py diff --git a/accounting/tests.py b/accounting/tests.py index c66558e0..1140acc7 100644 --- a/accounting/tests.py +++ b/accounting/tests.py @@ -216,7 +216,7 @@ class TestOperation(TestCase): self.journal.operations.filter(target_label="Le fantome du jour").exists() ) - def test__operation_simple_accounting(self): + def test_operation_simple_accounting(self): sat = SimplifiedAccountingType.objects.all().first() response = self.client.post( reverse("accounting:op_new", args=[self.journal.id]), @@ -237,15 +237,14 @@ class TestOperation(TestCase): "done": False, }, ) - self.assertFalse(response.status_code == 403) - self.assertTrue(self.journal.operations.filter(amount=23).exists()) + assert response.status_code != 403 + assert self.journal.operations.filter(amount=23).exists() response_get = self.client.get( reverse("accounting:journal_details", args=[self.journal.id]) ) - self.assertTrue( - "Le fantome de l'aurore" in str(response_get.content) - ) - self.assertTrue( + assert "Le fantome de l'aurore" in str(response_get.content) + + assert ( self.journal.operations.filter(amount=23) .values("accounting_type") .first()["accounting_type"] diff --git a/accounting/views.py b/accounting/views.py index 928dc009..ce0ae45b 100644 --- a/accounting/views.py +++ b/accounting/views.py @@ -215,17 +215,14 @@ class JournalTabsMixin(TabedViewMixin): return _("Journal") def get_list_of_tabs(self): - tab_list = [] - tab_list.append( + return [ { "url": reverse( "accounting:journal_details", kwargs={"j_id": self.object.id} ), "slug": "journal", "name": _("Journal"), - } - ) - tab_list.append( + }, { "url": reverse( "accounting:journal_nature_statement", @@ -233,9 +230,7 @@ class JournalTabsMixin(TabedViewMixin): ), "slug": "nature_statement", "name": _("Statement by nature"), - } - ) - tab_list.append( + }, { "url": reverse( "accounting:journal_person_statement", @@ -243,9 +238,7 @@ class JournalTabsMixin(TabedViewMixin): ), "slug": "person_statement", "name": _("Statement by person"), - } - ) - tab_list.append( + }, { "url": reverse( "accounting:journal_accounting_statement", @@ -253,9 +246,8 @@ class JournalTabsMixin(TabedViewMixin): ), "slug": "accounting_statement", "name": _("Accounting statement"), - } - ) - return tab_list + }, + ] class JournalCreateView(CanCreateMixin, CreateView): diff --git a/club/models.py b/club/models.py index 573fd176..5300057d 100644 --- a/club/models.py +++ b/club/models.py @@ -31,14 +31,14 @@ from django.core.cache import cache from django.core.exceptions import ObjectDoesNotExist, ValidationError from django.core.validators import RegexValidator, validate_email from django.db import models, transaction -from django.db.models import Q +from django.db.models import Exists, OuterRef, Q from django.urls import reverse from django.utils import timezone from django.utils.functional import cached_property from django.utils.timezone import localdate from django.utils.translation import gettext_lazy as _ -from core.models import Group, MetaGroup, Notification, Page, RealGroup, SithFile, User +from core.models import Group, MetaGroup, Notification, Page, SithFile, User # Create your models here. @@ -438,19 +438,18 @@ class Mailing(models.Model): def save(self, *args, **kwargs): if not self.is_moderated: - for user in ( - RealGroup.objects.filter(id=settings.SITH_GROUP_COM_ADMIN_ID) - .first() - .users.all() + unread_notif_subquery = Notification.objects.filter( + user=OuterRef("pk"), type="MAILING_MODERATION", viewed=False + ) + for user in User.objects.filter( + ~Exists(unread_notif_subquery), + groups__id__in=[settings.SITH_GROUP_COM_ADMIN_ID], ): - if not user.notifications.filter( - type="MAILING_MODERATION", viewed=False - ).exists(): - Notification( - user=user, - url=reverse("com:mailing_admin"), - type="MAILING_MODERATION", - ).save(*args, **kwargs) + Notification( + user=user, + url=reverse("com:mailing_admin"), + type="MAILING_MODERATION", + ).save(*args, **kwargs) super().save(*args, **kwargs) def clean(self): diff --git a/com/models.py b/com/models.py index cf2cb961..f3076174 100644 --- a/com/models.py +++ b/com/models.py @@ -34,7 +34,7 @@ from django.utils import timezone from django.utils.translation import gettext_lazy as _ from club.models import Club -from core.models import Notification, Preferences, RealGroup, User +from core.models import Notification, Preferences, User class Sith(models.Model): @@ -108,17 +108,15 @@ class News(models.Model): def save(self, *args, **kwargs): super().save(*args, **kwargs) - for u in ( - RealGroup.objects.filter(id=settings.SITH_GROUP_COM_ADMIN_ID) - .first() - .users.all() + for user in User.objects.filter( + groups__id__in=[settings.SITH_GROUP_COM_ADMIN_ID] ): - Notification( - user=u, + Notification.objects.create( + user=user, url=reverse("com:news_admin_list"), type="NEWS_MODERATION", param="1", - ).save() + ) def get_absolute_url(self): return reverse("com:news_detail", kwargs={"news_id": self.id}) @@ -336,16 +334,14 @@ class Poster(models.Model): def save(self, *args, **kwargs): if not self.is_moderated: - for u in ( - RealGroup.objects.filter(id=settings.SITH_GROUP_COM_ADMIN_ID) - .first() - .users.all() + for user in User.objects.filter( + groups__id__in=[settings.SITH_GROUP_COM_ADMIN_ID] ): - Notification( - user=u, + Notification.objects.create( + user=user, url=reverse("com:poster_moderate_list"), type="POSTER_MODERATION", - ).save() + ) return super().save(*args, **kwargs) def clean(self, *args, **kwargs): diff --git a/com/tests.py b/com/tests.py index 1c39fa36..399eb0e8 100644 --- a/com/tests.py +++ b/com/tests.py @@ -23,7 +23,7 @@ from django.utils.translation import gettext as _ from club.models import Club, Membership from com.models import News, Poster, Sith, Weekmail, WeekmailArticle -from core.models import AnonymousUser, RealGroup, User +from core.models import AnonymousUser, Group, User @pytest.fixture() @@ -49,9 +49,7 @@ class TestCom(TestCase): @classmethod def setUpTestData(cls): cls.skia = User.objects.get(username="skia") - cls.com_group = RealGroup.objects.filter( - id=settings.SITH_GROUP_COM_ADMIN_ID - ).first() + cls.com_group = Group.objects.get(id=settings.SITH_GROUP_COM_ADMIN_ID) cls.skia.groups.set([cls.com_group]) def setUp(self): diff --git a/com/views.py b/com/views.py index c3835605..d4136d20 100644 --- a/com/views.py +++ b/com/views.py @@ -28,7 +28,7 @@ from smtplib import SMTPRecipientsRefused from django import forms from django.conf import settings from django.core.exceptions import PermissionDenied, ValidationError -from django.db.models import Max +from django.db.models import Exists, Max, OuterRef from django.forms.models import modelform_factory from django.http import HttpResponseRedirect from django.shortcuts import get_object_or_404, redirect @@ -42,7 +42,7 @@ from django.views.generic.edit import CreateView, DeleteView, UpdateView from club.models import Club, Mailing from com.models import News, NewsDate, Poster, Screen, Sith, Weekmail, WeekmailArticle -from core.models import Notification, RealGroup, User +from core.models import Notification, User from core.views import ( CanCreateMixin, CanEditMixin, @@ -278,21 +278,18 @@ class NewsEditView(CanEditMixin, UpdateView): else: self.object.is_moderated = False self.object.save() - for u in ( - RealGroup.objects.filter(id=settings.SITH_GROUP_COM_ADMIN_ID) - .first() - .users.all() + unread_notif_subquery = Notification.objects.filter( + user=OuterRef("pk"), type="NEWS_MODERATION", viewed=False + ) + for user in User.objects.filter( + ~Exists(unread_notif_subquery), + groups__id__in=[settings.SITH_GROUP_COM_ADMIN_ID], ): - if not u.notifications.filter( - type="NEWS_MODERATION", viewed=False - ).exists(): - Notification( - user=u, - url=reverse( - "com:news_detail", kwargs={"news_id": self.object.id} - ), - type="NEWS_MODERATION", - ).save() + Notification.objects.create( + user=user, + url=self.object.get_absolute_url(), + type="NEWS_MODERATION", + ) return super().form_valid(form) @@ -323,19 +320,18 @@ class NewsCreateView(CanCreateMixin, CreateView): self.object.is_moderated = True self.object.save() else: - for u in ( - RealGroup.objects.filter(id=settings.SITH_GROUP_COM_ADMIN_ID) - .first() - .users.all() + unread_notif_subquery = Notification.objects.filter( + user=OuterRef("pk"), type="NEWS_MODERATION", viewed=False + ) + for user in User.objects.filter( + ~Exists(unread_notif_subquery), + groups__id__in=[settings.SITH_GROUP_COM_ADMIN_ID], ): - if not u.notifications.filter( - type="NEWS_MODERATION", viewed=False - ).exists(): - Notification( - user=u, - url=reverse("com:news_admin_list"), - type="NEWS_MODERATION", - ).save() + Notification.objects.create( + user=user, + url=reverse("com:news_admin_list"), + type="NEWS_MODERATION", + ) return super().form_valid(form) diff --git a/core/management/commands/populate.py b/core/management/commands/populate.py index 7098101a..5770c715 100644 --- a/core/management/commands/populate.py +++ b/core/management/commands/populate.py @@ -261,19 +261,19 @@ class Command(BaseCommand): User.groups.through.objects.bulk_create( [ User.groups.through( - realgroup_id=settings.SITH_GROUP_COUNTER_ADMIN_ID, user=counter + group_id=settings.SITH_GROUP_COUNTER_ADMIN_ID, user=counter ), User.groups.through( - realgroup_id=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID, user=comptable + group_id=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID, user=comptable ), User.groups.through( - realgroup_id=settings.SITH_GROUP_COM_ADMIN_ID, user=comunity + group_id=settings.SITH_GROUP_COM_ADMIN_ID, user=comunity ), User.groups.through( - realgroup_id=settings.SITH_GROUP_PEDAGOGY_ADMIN_ID, user=tutu + group_id=settings.SITH_GROUP_PEDAGOGY_ADMIN_ID, user=tutu ), User.groups.through( - realgroup_id=settings.SITH_GROUP_SAS_ADMIN_ID, user=skia + group_id=settings.SITH_GROUP_SAS_ADMIN_ID, user=skia ), ] ) diff --git a/core/management/commands/populate_more.py b/core/management/commands/populate_more.py index eaac58c0..f8ac1cef 100644 --- a/core/management/commands/populate_more.py +++ b/core/management/commands/populate_more.py @@ -11,7 +11,7 @@ from django.utils.timezone import localdate, make_aware, now from faker import Faker from club.models import Club, Membership -from core.models import RealGroup, User +from core.models import Group, User from counter.models import ( Counter, Customer, @@ -225,9 +225,7 @@ class Command(BaseCommand): ae = Club.objects.get(unix_name="ae") other_clubs = random.sample(list(Club.objects.all()), k=3) groups = list( - RealGroup.objects.filter( - name__in=["Subscribers", "Old subscribers", "Public"] - ) + Group.objects.filter(name__in=["Subscribers", "Old subscribers", "Public"]) ) counters = list( Counter.objects.filter(name__in=["Foyer", "MDE", "La Gommette", "Eboutic"]) diff --git a/core/migrations/0040_alter_user_options_user_user_permissions_and_more.py b/core/migrations/0040_alter_user_options_user_user_permissions_and_more.py new file mode 100644 index 00000000..43e4911c --- /dev/null +++ b/core/migrations/0040_alter_user_options_user_user_permissions_and_more.py @@ -0,0 +1,82 @@ +# Generated by Django 4.2.16 on 2024-11-20 16:22 + +import django.contrib.auth.validators +import django.utils.timezone +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("auth", "0012_alter_user_first_name_max_length"), + ("core", "0039_alter_user_managers"), + ] + + operations = [ + migrations.AlterModelOptions( + name="user", + options={"verbose_name": "user", "verbose_name_plural": "users"}, + ), + migrations.AddField( + model_name="user", + name="user_permissions", + field=models.ManyToManyField( + blank=True, + help_text="Specific permissions for this user.", + related_name="user_set", + related_query_name="user", + to="auth.permission", + verbose_name="user permissions", + ), + ), + migrations.AlterField( + model_name="user", + name="date_joined", + field=models.DateTimeField( + default=django.utils.timezone.now, verbose_name="date joined" + ), + ), + migrations.AlterField( + model_name="user", + name="is_superuser", + field=models.BooleanField( + default=False, + help_text="Designates that this user has all permissions without explicitly assigning them.", + verbose_name="superuser status", + ), + ), + migrations.AlterField( + model_name="user", + name="username", + field=models.CharField( + error_messages={"unique": "A user with that username already exists."}, + help_text="Required. 150 characters or fewer. Letters, digits and @/./+/-/_ only.", + max_length=150, + unique=True, + validators=[django.contrib.auth.validators.UnicodeUsernameValidator()], + verbose_name="username", + ), + ), + migrations.AlterField( + model_name="user", + name="groups", + field=models.ManyToManyField( + blank=True, + help_text="The groups this user belongs to. A user will get all permissions granted to each of their groups.", + related_name="user_set", + related_query_name="user", + to="auth.group", + verbose_name="groups", + ), + ), + migrations.AlterField( + model_name="user", + name="groups", + field=models.ManyToManyField( + blank=True, + help_text="The groups this user belongs to. A user will get all permissions granted to each of their groups.", + related_name="users", + to="core.group", + verbose_name="groups", + ), + ), + ] diff --git a/core/models.py b/core/models.py index 2d578d6d..48355b67 100644 --- a/core/models.py +++ b/core/models.py @@ -30,19 +30,13 @@ import string import unicodedata from datetime import timedelta from pathlib import Path -from typing import TYPE_CHECKING, Any, Optional, Self +from typing import TYPE_CHECKING, Optional, Self from django.conf import settings -from django.contrib.auth.models import AbstractBaseUser, UserManager -from django.contrib.auth.models import ( - AnonymousUser as AuthAnonymousUser, -) -from django.contrib.auth.models import ( - Group as AuthGroup, -) -from django.contrib.auth.models import ( - GroupManager as AuthGroupManager, -) +from django.contrib.auth.models import AbstractUser, UserManager +from django.contrib.auth.models import AnonymousUser as AuthAnonymousUser +from django.contrib.auth.models import Group as AuthGroup +from django.contrib.auth.models import GroupManager as AuthGroupManager from django.contrib.staticfiles.storage import staticfiles_storage from django.core import validators from django.core.cache import cache @@ -242,7 +236,7 @@ class CustomUserManager(UserManager.from_queryset(UserQuerySet)): pass -class User(AbstractBaseUser): +class User(AbstractUser): """Defines the base user class, useable in every app. This is almost the same as the auth module AbstractUser since it inherits from it, @@ -253,51 +247,22 @@ class User(AbstractBaseUser): Required fields: email, first_name, last_name, date_of_birth """ - username = models.CharField( - _("username"), - max_length=254, - unique=True, - help_text=_( - "Required. 254 characters or fewer. Letters, digits and ./+/-/_ only." - ), - validators=[ - validators.RegexValidator( - r"^[\w.+-]+$", - _( - "Enter a valid username. This value may contain only " - "letters, numbers " - "and ./+/-/_ characters." - ), - ) - ], - error_messages={"unique": _("A user with that username already exists.")}, - ) first_name = models.CharField(_("first name"), max_length=64) last_name = models.CharField(_("last name"), max_length=64) email = models.EmailField(_("email address"), unique=True) date_of_birth = models.DateField(_("date of birth"), blank=True, null=True) nick_name = models.CharField(_("nick name"), max_length=64, null=True, blank=True) - is_staff = models.BooleanField( - _("staff status"), - default=False, - help_text=_("Designates whether the user can log into this admin site."), - ) - is_active = models.BooleanField( - _("active"), - default=True, - help_text=_( - "Designates whether this user should be treated as active. " - "Unselect this instead of deleting accounts." - ), - ) - date_joined = models.DateField(_("date joined"), auto_now_add=True) last_update = models.DateTimeField(_("last update"), auto_now=True) - is_superuser = models.BooleanField( - _("superuser"), - default=False, - help_text=_("Designates whether this user is a superuser. "), + groups = models.ManyToManyField( + Group, + verbose_name=_("groups"), + help_text=_( + "The groups this user belongs to. A user will get all permissions " + "granted to each of their groups." + ), + related_name="users", + blank=True, ) - groups = models.ManyToManyField(RealGroup, related_name="users", blank=True) home = models.OneToOneField( "SithFile", related_name="home_of", @@ -401,8 +366,6 @@ class User(AbstractBaseUser): objects = CustomUserManager() - USERNAME_FIELD = "username" - def __str__(self): return self.get_display_name() @@ -422,12 +385,6 @@ class User(AbstractBaseUser): settings.BASE_DIR / f"core/static/core/img/promo_{self.promo}.png" ).exists() - def has_module_perms(self, package_name: str) -> bool: - return self.is_active - - def has_perm(self, perm: str, obj: Any = None) -> bool: - return self.is_active and self.is_superuser - @cached_property def was_subscribed(self) -> bool: return self.subscriptions.exists() @@ -599,11 +556,6 @@ class User(AbstractBaseUser): "date_of_birth": self.date_of_birth, } - def get_full_name(self): - """Returns the first_name plus the last_name, with a space in between.""" - full_name = "%s %s" % (self.first_name, self.last_name) - return full_name.strip() - def get_short_name(self): """Returns the short name for the user.""" if self.nick_name: @@ -982,13 +934,11 @@ class SithFile(models.Model): if copy_rights: self.copy_rights() if self.is_in_sas: - for u in ( - RealGroup.objects.filter(id=settings.SITH_GROUP_SAS_ADMIN_ID) - .first() - .users.all() + for user in User.objects.filter( + groups__id__in=[settings.SITH_GROUP_SAS_ADMIN_ID] ): Notification( - user=u, + user=user, url=reverse("sas:moderation"), type="SAS_MODERATION", param="1", diff --git a/core/tests/test_core.py b/core/tests/test_core.py index 9b70e886..a33a8705 100644 --- a/core/tests/test_core.py +++ b/core/tests/test_core.py @@ -118,7 +118,9 @@ class TestUserRegistration: response = client.post(reverse("core:register"), valid_payload) assert response.status_code == 200 - error_html = "
  • Un objet User avec ce champ Adresse email existe déjà.
    • " + error_html = ( + "
  • Un objet Utilisateur avec ce champ Adresse email existe déjà.
    • " + ) assertInHTML(error_html, str(response.content.decode())) def test_register_fail_with_not_existing_email( diff --git a/core/tests/test_files.py b/core/tests/test_files.py index 1f39fcd8..998ceab5 100644 --- a/core/tests/test_files.py +++ b/core/tests/test_files.py @@ -14,7 +14,7 @@ from PIL import Image from pytest_django.asserts import assertNumQueries from core.baker_recipes import board_user, old_subscriber_user, subscriber_user -from core.models import Group, RealGroup, SithFile, User +from core.models import Group, SithFile, User from sas.models import Picture from sith import settings @@ -26,12 +26,10 @@ class TestImageAccess: [ lambda: baker.make(User, is_superuser=True), lambda: baker.make( - User, - groups=[RealGroup.objects.get(pk=settings.SITH_GROUP_SAS_ADMIN_ID)], + User, groups=[Group.objects.get(pk=settings.SITH_GROUP_SAS_ADMIN_ID)] ), lambda: baker.make( - User, - groups=[RealGroup.objects.get(pk=settings.SITH_GROUP_COM_ADMIN_ID)], + User, groups=[Group.objects.get(pk=settings.SITH_GROUP_COM_ADMIN_ID)] ), ], ) diff --git a/core/views/files.py b/core/views/files.py index 0d083a84..f8539080 100644 --- a/core/views/files.py +++ b/core/views/files.py @@ -21,6 +21,7 @@ from wsgiref.util import FileWrapper from django import forms from django.conf import settings from django.core.exceptions import PermissionDenied +from django.db.models import Exists, OuterRef from django.forms.models import modelform_factory from django.http import Http404, HttpRequest, HttpResponse from django.shortcuts import get_object_or_404, redirect @@ -31,7 +32,7 @@ from django.views.generic import DetailView, ListView from django.views.generic.detail import SingleObjectMixin from django.views.generic.edit import DeleteView, FormMixin, UpdateView -from core.models import Notification, RealGroup, SithFile, User +from core.models import Notification, SithFile, User from core.views import ( AllowFragment, CanEditMixin, @@ -159,19 +160,18 @@ class AddFilesForm(forms.Form): % {"file_name": f, "msg": repr(e)}, ) if notif: - for u in ( - RealGroup.objects.filter(id=settings.SITH_GROUP_COM_ADMIN_ID) - .first() - .users.all() + unread_notif_subquery = Notification.objects.filter( + user=OuterRef("pk"), type="FILE_MODERATION", viewed=False + ) + for user in User.objects.filter( + ~Exists(unread_notif_subquery), + groups__id__in=[settings.SITH_GROUP_COM_ADMIN_ID], ): - if not u.notifications.filter( - type="FILE_MODERATION", viewed=False - ).exists(): - Notification( - user=u, - url=reverse("core:file_moderation"), - type="FILE_MODERATION", - ).save() + Notification.objects.create( + user=user, + url=reverse("core:file_moderation"), + type="FILE_MODERATION", + ) class FileListView(ListView): diff --git a/core/views/forms.py b/core/views/forms.py index de01f7aa..ea9c27f0 100644 --- a/core/views/forms.py +++ b/core/views/forms.py @@ -167,9 +167,7 @@ class RegisteringForm(UserCreationForm): class Meta: model = User fields = ("first_name", "last_name", "email") - field_classes = { - "email": AntiSpamEmailField, - } + field_classes = {"email": AntiSpamEmailField} class UserProfileForm(forms.ModelForm): diff --git a/core/views/group.py b/core/views/group.py index abb0097f..b6e77b54 100644 --- a/core/views/group.py +++ b/core/views/group.py @@ -23,9 +23,7 @@ from django.views.generic.edit import CreateView, DeleteView, UpdateView from core.models import RealGroup, User from core.views import CanCreateMixin, CanEditMixin, DetailFormView -from core.views.widgets.select import ( - AutoCompleteSelectMultipleUser, -) +from core.views.widgets.select import AutoCompleteSelectMultipleUser # Forms diff --git a/pedagogy/tests/test_api.py b/pedagogy/tests/test_api.py index b8fb90b4..cbb99c18 100644 --- a/pedagogy/tests/test_api.py +++ b/pedagogy/tests/test_api.py @@ -8,7 +8,7 @@ from model_bakery import baker from model_bakery.recipe import Recipe from core.baker_recipes import subscriber_user -from core.models import RealGroup, User +from core.models import Group, User from pedagogy.models import UV @@ -80,9 +80,7 @@ class TestUVSearch(TestCase): subscriber_user.make(), baker.make( User, - groups=[ - RealGroup.objects.get(pk=settings.SITH_GROUP_PEDAGOGY_ADMIN_ID) - ], + groups=[Group.objects.get(pk=settings.SITH_GROUP_PEDAGOGY_ADMIN_ID)], ), ): # users that have right diff --git a/pedagogy/views.py b/pedagogy/views.py index ca2c712e..99dd8168 100644 --- a/pedagogy/views.py +++ b/pedagogy/views.py @@ -24,6 +24,7 @@ from django.conf import settings from django.contrib.auth.mixins import LoginRequiredMixin from django.core.exceptions import PermissionDenied +from django.db.models import Exists, OuterRef from django.shortcuts import get_object_or_404 from django.urls import reverse, reverse_lazy from django.views.generic import ( @@ -34,7 +35,7 @@ from django.views.generic import ( UpdateView, ) -from core.models import Notification, RealGroup +from core.models import Notification, User from core.views import ( CanCreateMixin, CanEditPropMixin, @@ -156,21 +157,19 @@ class UVCommentReportCreateView(CanCreateMixin, CreateView): def form_valid(self, form): resp = super().form_valid(form) - # Send a message to moderation admins - for user in ( - RealGroup.objects.filter(id=settings.SITH_GROUP_PEDAGOGY_ADMIN_ID) - .first() - .users.all() + unread_notif_subquery = Notification.objects.filter( + user=OuterRef("pk"), type="PEDAGOGY_MODERATION", viewed=False + ) + for user in User.objects.filter( + ~Exists(unread_notif_subquery), + groups__id__in=[settings.SITH_GROUP_PEDAGOGY_ADMIN_ID], ): - if not user.notifications.filter( - type="PEDAGOGY_MODERATION", viewed=False - ).exists(): - Notification( - user=user, - url=reverse("pedagogy:moderation"), - type="PEDAGOGY_MODERATION", - ).save() + Notification.objects.create( + user=user, + url=reverse("pedagogy:moderation"), + type="PEDAGOGY_MODERATION", + ) return resp diff --git a/rootplace/tests.py b/rootplace/tests.py index 0d0f1542..a2bbee81 100644 --- a/rootplace/tests.py +++ b/rootplace/tests.py @@ -19,7 +19,7 @@ from django.urls import reverse from django.utils.timezone import localtime, now from club.models import Club -from core.models import RealGroup, User +from core.models import Group, User from counter.models import Counter, Customer, Product, Refilling, Selling from subscription.models import Subscription @@ -50,9 +50,9 @@ class TestMergeUser(TestCase): self.to_keep.address = "Jerusalem" self.to_delete.parent_address = "Rome" self.to_delete.address = "Rome" - subscribers = RealGroup.objects.get(name="Subscribers") - mde_admin = RealGroup.objects.get(name="MDE admin") - sas_admin = RealGroup.objects.get(name="SAS admin") + subscribers = Group.objects.get(name="Subscribers") + mde_admin = Group.objects.get(name="MDE admin") + sas_admin = Group.objects.get(name="SAS admin") self.to_keep.groups.add(subscribers.id) self.to_delete.groups.add(mde_admin.id) self.to_keep.groups.add(sas_admin.id) diff --git a/sas/tests/test_api.py b/sas/tests/test_api.py index 733838d2..9b24688b 100644 --- a/sas/tests/test_api.py +++ b/sas/tests/test_api.py @@ -7,7 +7,7 @@ from model_bakery import baker from model_bakery.recipe import Recipe from core.baker_recipes import old_subscriber_user, subscriber_user -from core.models import RealGroup, SithFile, User +from core.models import Group, SithFile, User from sas.baker_recipes import picture_recipe from sas.models import Album, PeoplePictureRelation, Picture, PictureModerationRequest @@ -155,7 +155,7 @@ class TestPictureRelation(TestSas): def test_delete_relation_with_authorized_users(self): """Test that deletion works as intended when called by an authorized user.""" relation: PeoplePictureRelation = self.user_a.pictures.first() - sas_admin_group = RealGroup.objects.get(pk=settings.SITH_GROUP_SAS_ADMIN_ID) + sas_admin_group = Group.objects.get(pk=settings.SITH_GROUP_SAS_ADMIN_ID) sas_admin = baker.make(User, groups=[sas_admin_group]) root = baker.make(User, is_superuser=True) for user in sas_admin, root, self.user_a: @@ -189,7 +189,7 @@ class TestPictureModeration(TestSas): def setUpTestData(cls): super().setUpTestData() cls.sas_admin = baker.make( - User, groups=[RealGroup.objects.get(pk=settings.SITH_GROUP_SAS_ADMIN_ID)] + User, groups=[Group.objects.get(pk=settings.SITH_GROUP_SAS_ADMIN_ID)] ) cls.picture = Picture.objects.filter(parent=cls.album_a)[0] cls.picture.is_moderated = False diff --git a/sas/tests/test_views.py b/sas/tests/test_views.py index 2a73b90b..ff8dd21d 100644 --- a/sas/tests/test_views.py +++ b/sas/tests/test_views.py @@ -23,7 +23,7 @@ from model_bakery import baker from pytest_django.asserts import assertInHTML, assertRedirects from core.baker_recipes import old_subscriber_user, subscriber_user -from core.models import RealGroup, User +from core.models import Group, User from sas.baker_recipes import picture_recipe from sas.models import Album, Picture @@ -38,7 +38,7 @@ from sas.models import Album, Picture old_subscriber_user.make, lambda: baker.make(User, is_superuser=True), lambda: baker.make( - User, groups=[RealGroup.objects.get(pk=settings.SITH_GROUP_SAS_ADMIN_ID)] + User, groups=[Group.objects.get(pk=settings.SITH_GROUP_SAS_ADMIN_ID)] ), lambda: baker.make(User), ], @@ -80,7 +80,7 @@ class TestSasModeration(TestCase): cls.to_moderate.is_moderated = False cls.to_moderate.save() cls.moderator = baker.make( - User, groups=[RealGroup.objects.get(pk=settings.SITH_GROUP_SAS_ADMIN_ID)] + User, groups=[Group.objects.get(pk=settings.SITH_GROUP_SAS_ADMIN_ID)] ) cls.simple_user = subscriber_user.make()