From ce812669539a64a7706eda111885e0bdce671c05 Mon Sep 17 00:00:00 2001 From: Dominik George <dominik.george@teckids.org> Date: Fri, 1 May 2020 18:25:54 +0200 Subject: [PATCH] [Review] Add doc strings, comments, and clean up some minor issues --- aleksis/core/apps.py | 7 +- aleksis/core/forms.py | 93 ++++++++++++------------ aleksis/core/menus.py | 18 +---- aleksis/core/mixins.py | 28 ++++---- aleksis/core/models.py | 100 +++++++++++++++++++------- aleksis/core/registries.py | 8 +++ aleksis/core/search_indexes.py | 4 ++ aleksis/core/signals.py | 13 ---- aleksis/core/tables.py | 4 ++ aleksis/core/tasks.py | 10 +++ aleksis/core/urls.py | 1 - aleksis/core/util/apps.py | 23 ++++-- aleksis/core/util/core_helpers.py | 37 ++++++++++ aleksis/core/util/messages.py | 36 ++++++++++ aleksis/core/util/predicates.py | 3 - aleksis/core/util/sass_helpers.py | 20 ++++++ aleksis/core/views.py | 114 ++++++++++++++++++------------ 17 files changed, 350 insertions(+), 169 deletions(-) delete mode 100644 aleksis/core/signals.py diff --git a/aleksis/core/apps.py b/aleksis/core/apps.py index df5403b0b..3e2c5d5d0 100644 --- a/aleksis/core/apps.py +++ b/aleksis/core/apps.py @@ -7,9 +7,9 @@ from django.http import HttpRequest from dynamic_preferences.registries import preference_models from .registries import group_preferences_registry, person_preferences_registry, site_preferences_registry -from .signals import clean_scss from .util.apps import AppConfig from .util.core_helpers import has_person +from .util.sass_helpers import clean_scss class CoreConfig(AppConfig): @@ -26,8 +26,8 @@ class CoreConfig(AppConfig): ([2018, 2019, 2020], "Julian Leucker", "leuckeju@katharineum.de"), ([2018, 2019, 2020], "Hangzhi Yu", "yuha@katharineum.de"), ([2019, 2020], "Dominik George", "dominik.george@teckids.org"), - ([2019, 2020], "mirabilos", "thorsten.glaser@teckids.org"), ([2019, 2020], "Tom Teichler", "tom.teichler@teckids.org"), + ([2019], "mirabilos", "thorsten.glaser@teckids.org"), ) def ready(self): @@ -51,6 +51,7 @@ class CoreConfig(AppConfig): **kwargs, ) -> None: if section == "theme": + # Clean compiled SCSS to invalidate it after theme changes; recreated on request clean_scss() def post_migrate( @@ -65,7 +66,7 @@ class CoreConfig(AppConfig): ) -> None: super().post_migrate(app_config, verbosity, interactive, using, plan, apps) - # Ensure presence of a OTP YubiKey default config + # Ensure presence of an OTP YubiKey default config apps.get_model("otp_yubikey", "ValidationService").objects.using(using).update_or_create( name="default", defaults={"use_ssl": True, "param_sl": "", "param_timeout": ""} ) diff --git a/aleksis/core/forms.py b/aleksis/core/forms.py index cd7f21845..92faded47 100644 --- a/aleksis/core/forms.py +++ b/aleksis/core/forms.py @@ -18,6 +18,8 @@ from .registries import site_preferences_registry, person_preferences_registry, class PersonAccountForm(forms.ModelForm): + """ Form to assign user accounts to persons in the frontend :""" + class Meta: model = Person fields = ["last_name", "first_name", "user"] @@ -27,6 +29,8 @@ class PersonAccountForm(forms.ModelForm): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) + + # Fields displayed only for informational purposes self.fields["first_name"].disabled = True self.fields["last_name"].disabled = True @@ -35,13 +39,16 @@ class PersonAccountForm(forms.ModelForm): if self.cleaned_data.get("new_user", None): if self.cleaned_data.get("user", None): + # The user selected both an existing user and provided a name to create a new one self.add_error( "new_user", _("You cannot set a new username when also selecting an existing user."), ) elif User.objects.filter(username=self.cleaned_data["new_user"]).exists(): + # The user tried to create a new user with the name of an existing user self.add_error("new_user", _("This username is already in use.")) else: + # Create new User object and assign to form field for existing user new_user_obj = User.objects.create_user( self.cleaned_data["new_user"], self.instance.email, @@ -52,12 +59,15 @@ class PersonAccountForm(forms.ModelForm): self.cleaned_data["user"] = new_user_obj +# Formset for batch-processing of assignments of users to persons PersonsAccountsFormSet = forms.modelformset_factory( Person, form=PersonAccountForm, max_num=0, extra=0 ) class EditPersonForm(ExtensibleForm): + """ Form to edit an existing person object in the frontend """ + layout = Layout( Fieldset( _("Base data"), @@ -106,28 +116,13 @@ class EditPersonForm(ExtensibleForm): ) def clean(self) -> None: - User = get_user_model() - - if self.cleaned_data.get("new_user", None): - if self.cleaned_data.get("user", None): - self.add_error( - "new_user", - _("You cannot set a new username when also selecting an existing user."), - ) - elif User.objects.filter(username=self.cleaned_data["new_user"]).exists(): - self.add_error("new_user", _("This username is already in use.")) - else: - new_user_obj = User.objects.create_user( - self.cleaned_data["new_user"], - self.instance.email, - first_name=self.instance.first_name, - last_name=self.instance.last_name, - ) - - self.cleaned_data["user"] = new_user_obj + # Use code implemented in dedicated form to verify user selection + return PersonAccountForm.clean(self) class EditGroupForm(ExtensibleForm): + """ Form to edit an existing group in the frontend """ + layout = Layout( Fieldset(_("Common data"), "name", "short_name"), Fieldset(_("Persons"), "members", "owners", "parent_groups"), @@ -158,6 +153,8 @@ class EditGroupForm(ExtensibleForm): class AnnouncementForm(ExtensibleForm): + """ Form to create or edit an announcement in the frontend """ + valid_from = forms.DateTimeField(required=False) valid_until = forms.DateTimeField(required=False) @@ -183,6 +180,7 @@ class AnnouncementForm(ExtensibleForm): def __init__(self, *args, **kwargs): if "instance" not in kwargs: + # Default to today and whole day for new announcements kwargs["initial"] = { "valid_from_date": datetime.now(), "valid_from_time": time(0, 0), @@ -201,20 +199,17 @@ class AnnouncementForm(ExtensibleForm): "groups": announcement.get_recipients_for_model(Group), "persons": announcement.get_recipients_for_model(Person), } + super().__init__(*args, **kwargs) def clean(self): data = super().clean() - # Check date and time - from_date = data["valid_from_date"] - from_time = data["valid_from_time"] - until_date = data["valid_until_date"] - until_time = data["valid_until_time"] - - valid_from = datetime.combine(from_date, from_time) - valid_until = datetime.combine(until_date, until_time) + # Combine date and time fields into datetime objects + valid_from = datetime.combine(data["valid_from_date"], data["valid_from_time"]) + valid_until = datetime.combine(data["valid_until_date"], data["valid_until_time"]) + # Sanity check validity range if valid_until < datetime.now(): raise ValidationError( _("You are not allowed to create announcements which are only valid in the past.") @@ -224,37 +219,38 @@ class AnnouncementForm(ExtensibleForm): _("The from date and time must be earlier then the until date and time.") ) + # Inject real time data if all went well data["valid_from"] = valid_from data["valid_until"] = valid_until - # Check recipients + # Ensure at least one group or one person is set as recipient if "groups" not in data and "persons" not in data: raise ValidationError(_("You need at least one recipient.")) - recipients = [] - recipients += data.get("groups", []) - recipients += data.get("persons", []) - - data["recipients"] = recipients + # Unwrap all recipients into single user objects and generate final list + data["recipients"] = [] + data["recipients"] += data.get("groups", []) + data["recipients"] += data.get("persons", []) return data def save(self, _=False): - # Save announcement - a = self.instance if self.instance is not None else Announcement() - a.valid_from = self.cleaned_data["valid_from"] - a.valid_until = self.cleaned_data["valid_until"] - a.title = self.cleaned_data["title"] - a.description = self.cleaned_data["description"] - a.save() + # Save announcement, respecting data injected in clean() + if self.instance is None: + self.instance = Announcement() + self.instance.valid_from = self.cleaned_data["valid_from"] + self.instance.valid_until = self.cleaned_data["valid_until"] + self.instance.title = self.cleaned_data["title"] + self.instance.description = self.cleaned_data["description"] + self.instance.save() # Save recipients - a.recipients.all().delete() + self.instance.recipients.all().delete() for recipient in self.cleaned_data["recipients"]: - a.recipients.create(recipient=recipient) - a.save() + self.instance.recipients.create(recipient=recipient) + self.instance.save() - return a + return self.instance class Meta: model = Announcement @@ -262,17 +258,24 @@ class AnnouncementForm(ExtensibleForm): class ChildGroupsForm(forms.Form): + """ Inline form for group editing to select child groups """ + child_groups = forms.ModelMultipleChoiceField(queryset=Group.objects.all()) class SitePreferenceForm(PreferenceForm): + """ Form to edit site preferences """ + registry = site_preferences_registry class PersonPreferenceForm(PreferenceForm): + """ Form to edit preferences valid for one person""" + registry = person_preferences_registry class GroupPreferenceForm(PreferenceForm): - registry = group_preferences_registry + """ Form to edit preferences valid for members of a group""" + registry = group_preferences_registry diff --git a/aleksis/core/menus.py b/aleksis/core/menus.py index fab92ff4c..985273c96 100644 --- a/aleksis/core/menus.py +++ b/aleksis/core/menus.py @@ -38,12 +38,11 @@ MENUS = { "validators": ["menu_generator.validators.is_authenticated"], }, { - "name": _("Two factor auth"), + "name": _("2FA"), "url": "two_factor:profile", "icon": "phonelink_lock", "validators": [ "menu_generator.validators.is_authenticated", - lambda request: "two_factor" in settings.INSTALLED_APPS, ], }, { @@ -106,14 +105,6 @@ MENUS = { ("aleksis.core.util.predicates.permission_validator", "core.impersonate"), ], }, - { - "name": _("Manage school"), - "url": "school_management", - "icon": "school", - "validators": [ - ("aleksis.core.util.predicates.permission_validator", "core.manage_school"), - ], - }, { "name": _("Configuration"), "url": "preferences_site", @@ -160,8 +151,7 @@ MENUS = { "url": "persons_accounts", "icon": "person_add", "validators": [ - "menu_generator.validators.is_authenticated", - "menu_generator.validators.is_superuser", + ("aleksis.core.util.predicates.permission_validator", "core.link_persons_accounts") ], }, { @@ -184,8 +174,4 @@ MENUS = { ], }, ], - "SCHOOL_MANAGEMENT_MENU": [ - {"name": _("Edit school information"), "url": "edit_school_information", }, - {"name": _("Edit school term"), "url": "edit_school_term", }, - ], } diff --git a/aleksis/core/mixins.py b/aleksis/core/mixins.py index 1b49ab042..1695b7a42 100644 --- a/aleksis/core/mixins.py +++ b/aleksis/core/mixins.py @@ -16,21 +16,6 @@ import reversion from rules.contrib.admin import ObjectPermissionsModelAdmin -class CRUDMixin(models.Model): - class Meta: - abstract = True - - @property - def crud_events(self) -> QuerySet: - """Get all CRUD events connected to this object from easyaudit.""" - - content_type = ContentType.objects.get_for_model(self) - - return CRUDEvent.objects.filter( - object_id=self.pk, content_type=content_type - ).select_related("user") - - @reversion.register() class ExtensibleModel(CRUDMixin): """ Base model for all objects in AlekSIS apps @@ -88,6 +73,16 @@ class ExtensibleModel(CRUDMixin): """ Get the URL o a view representing this model instance """ pass + @property + def crud_events(self) -> QuerySet: + """ Get all CRUD events connected to this object from easyaudit """ + + content_type = ContentType.objects.get_for_model(self) + + return CRUDEvent.objects.filter( + object_id=self.pk, content_type=content_type + ).select_related("user") + @property def crud_event_create(self) -> Optional[CRUDEvent]: """ Return create event of this object """ @@ -201,6 +196,7 @@ class _ExtensibleFormMetaclass(ModelFormMetaclass): def __new__(mcs, name, bases, dct): x = super().__new__(mcs, name, bases, dct) + # Enforce a default for the base layout for forms that o not specify one if hasattr(x, "layout"): base_layout = x.layout.elements else: @@ -246,4 +242,6 @@ class ExtensibleForm(ModelForm, metaclass=_ExtensibleFormMetaclass): class BaseModelAdmin(GuardedModelAdmin, ObjectPermissionsModelAdmin): + """ A base class for ModelAdmin combining django-guardian and rules """ + pass diff --git a/aleksis/core/models.py b/aleksis/core/models.py index 2617e7ae2..8f112c57b 100644 --- a/aleksis/core/models.py +++ b/aleksis/core/models.py @@ -26,20 +26,18 @@ from .util.model_helpers import ICONS FIELD_CHOICES = ( - ("booleanfield", "BooleanField"), - ("charfield", "CharField"), - ("datefield", "DateField"), - ("datetimefield", "DateTimeField"), - ("decimalfield", "DecimalField"), - ("emailfield", "EmailField"), - ("floatfield", "FloatField"), - ("Integerfield", "IntegerField"), - ("ipaddressfield", "IPAddressField"), - ("genericipaddressfield", "GenericIPAddressField"), - ("nullbooleanfield", "NullBooleanField"), - ("textfield", "TextField"), - ("timefield", "TimeField"), - ("urlfield", "URLField"), + ("BooleanField", _("Boolean (Yes/No)")), + ("CharField", _("Text (one line)")), + ("DateField", _("Date")), + ("DateTimeField", _("Date and time")), + ("DecimalField", _("Decimal number")), + ("EmailField", _("E-mail address")), + ("IntegerField", _("Integer")), + ("GenericIPAddressField", _("IP address")), + ("NullBooleanField", _("Boolean or empty (Yes/No/Neither)")), + ("TextField", _("Text (multi-line)")), + ("TimeField", _("Time")), + ("URLField", _("URL / Link")), ) @@ -129,22 +127,28 @@ class Person(ExtensibleModel): @property def full_name(self) -> str: + """ Full name of person in last name, first name order """ + return f"{self.last_name}, {self.first_name}" @property def adressing_name(self) -> str: - if get_site_preferences()["notification__addressing_name_format"] == "dutch": - return f"{self.last_name} {self.first_name}" - elif get_site_preferences()["notification__addressing_name_format"] == "english": + """ Full name of person in format configured for addressing """ + + if get_site_preferences()["notification__addressing_name_format"] == "last_first": return f"{self.last_name}, {self.first_name}" - else: + elif get_site_preferences()["notification__addressing_name_format"] == "first_last": return f"{self.first_name} {self.last_name}" @property def age(self): + """ Age of the person at current time """ + return self.age_at(timezone.datetime.now().date()) def age_at(self, today): + """ Age of the person at a given date and time """ + years = today.year - self.date_of_birth.year if (self.date_of_birth.month > today.month or (self.date_of_birth.month == today.month @@ -166,6 +170,7 @@ class Person(ExtensibleModel): for group in self.member_of.union(self.owner_of.all()).all(): group.save() + # Select a primary group if none is set self.auto_select_primary_group() def __str__(self) -> str: @@ -173,7 +178,7 @@ class Person(ExtensibleModel): @classmethod def maintain_default_data(cls): - # First, ensure we have an admin user + # Ensure we have an admin user User = get_user_model() if not User.objects.filter(is_superuser=True).exists(): admin = User.objects.create_superuser( @@ -183,10 +188,6 @@ class Person(ExtensibleModel): ) admin.save() - # Ensure this admin user has a person linked to it - person = Person(user=admin) - person.save() - def auto_select_primary_group(self, pattern: Optional[str] = None, force: bool = False) -> None: """ Auto-select the primary group among the groups the person is member of @@ -216,10 +217,13 @@ class DummyPerson(Person): is_dummy = True def save(self, *args, **kwargs): + # Do nothing, not even call Model's save(), so this is never persisted pass class AdditionalField(ExtensibleModel): + """ An additional field that can be linked to a group """ + title = models.CharField(verbose_name=_("Title of field"), max_length=255) field_type = models.CharField(verbose_name=_("Type of field"), choices=FIELD_CHOICES, max_length=50) @@ -266,6 +270,8 @@ class Group(ExtensibleModel): @property def announcement_recipients(self): + """ Flat list of all members and owners to fulfill announcement API contract """ + return list(self.members.all()) + list(self.owners.all()) def __str__(self) -> str: @@ -287,6 +293,12 @@ class Group(ExtensibleModel): class PersonGroupThrough(ExtensibleModel): + """ Through table for many-to-many relationship of group members. + + It does not have any fields on its own; these are generated upon instantiation + by inspecting the additional fields selected for the linked group. + """ + group = models.ForeignKey(Group, on_delete=models.CASCADE) person = models.ForeignKey(Person, on_delete=models.CASCADE) @@ -294,12 +306,14 @@ class PersonGroupThrough(ExtensibleModel): super().__init__(*args, **kwargs) for field in self.group.additional_fields: - field_class = getattr(jsonstore, field.get_field_type_display()) + field_class = getattr(jsonstore, field.field_type) field_name = slugify(field.title).replace("-", "_") field_instance = field_class(verbose_name=field.title) setattr(self, field_name, field_instance) class Activity(ExtensibleModel): + """ Activity of a user to trace some actions done in AlekSIS in displayable form """ + user = models.ForeignKey("Person", on_delete=models.CASCADE, related_name="activities", verbose_name=_("User")) title = models.CharField(max_length=150, verbose_name=_("Title")) @@ -316,6 +330,8 @@ class Activity(ExtensibleModel): class Notification(ExtensibleModel): + """ Notification to submit to a user """ + sender = models.CharField(max_length=100, verbose_name=_("Sender")) recipient = models.ForeignKey("Person", on_delete=models.CASCADE, related_name="notifications", verbose_name=_("Recipient")) @@ -342,6 +358,8 @@ class Notification(ExtensibleModel): class AnnouncementQuerySet(models.QuerySet): + """ Queryset for announcements providing time-based utility functions """ + def relevant_for(self, obj: Union[models.Model, models.QuerySet]) -> models.QuerySet: """ Get a QuerySet with all announcements relevant for a certain Model (e.g. a Group) or a set of models in a QuerySet. @@ -397,6 +415,10 @@ class AnnouncementQuerySet(models.QuerySet): class Announcement(ExtensibleModel): + """ Persistent announcement to display to groups or persons in various places during a + specific time range. + """ + objects = models.Manager.from_queryset(AnnouncementQuerySet)() title = models.CharField(max_length=150, verbose_name=_("Title")) @@ -435,6 +457,13 @@ class Announcement(ExtensibleModel): class AnnouncementRecipient(ExtensibleModel): + """ Generalisation of a recipient for an announcement, used to wrap arbitrary + objects that can receive announcements. + + Contract: Objects to serve as recipient have a property announcement_recipients + returning a flat list of Person objects. + """ + announcement = models.ForeignKey(Announcement, on_delete=models.CASCADE, related_name="recipients") content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE) @@ -513,9 +542,15 @@ class DashboardWidget(PolymorphicModel, PureDjangoModel): active = models.BooleanField(blank=True, verbose_name=_("Activate Widget")) def get_context(self): + """ Get the context dictionary to pass to the widget template """ + raise NotImplementedError("A widget subclass needs to implement the get_context method.") def get_template(self): + """ Get the template to render the widget with. Defaults to the template attribute, + but can be overridden to allow more complex template generation scenarios. + """ + return self.template def __str__(self): @@ -527,6 +562,8 @@ class DashboardWidget(PolymorphicModel, PureDjangoModel): class CustomMenu(ExtensibleModel): + """ A custom menu to display in the footer """ + name = models.CharField(max_length=100, verbose_name=_("Menu ID"), unique=True) def __str__(self): @@ -534,6 +571,7 @@ class CustomMenu(ExtensibleModel): @classmethod def get_default(cls, name): + """ Get a menu by name or create if it does not exist """ menu, _ = cls.objects.get_or_create(name=name) return menu @@ -543,6 +581,8 @@ class CustomMenu(ExtensibleModel): class CustomMenuItem(ExtensibleModel): + """ Single item in a custom menu """ + menu = models.ForeignKey( CustomMenu, models.CASCADE, verbose_name=_("Menu"), related_name="items" ) @@ -561,6 +601,10 @@ class CustomMenuItem(ExtensibleModel): class GroupType(ExtensibleModel): + """ Descriptive type of a group; used to tag groups and for apps to distinguish + how to display or handle a certain group. + """ + name = models.CharField(verbose_name=_("Title of type"), max_length=50) description = models.CharField(verbose_name=_("Description"), max_length=500) @@ -570,6 +614,8 @@ class GroupType(ExtensibleModel): class GlobalPermissions(ExtensibleModel): + """ Container for global permissions """ + class Meta: managed = False permissions = ( @@ -585,6 +631,8 @@ class GlobalPermissions(ExtensibleModel): class SitePreferenceModel(PerInstancePreferenceModel, PureDjangoModel): + """ Preference model to hold pereferences valid for a site """ + instance = models.ForeignKey(Site, on_delete=models.CASCADE) class Meta: @@ -592,6 +640,8 @@ class SitePreferenceModel(PerInstancePreferenceModel, PureDjangoModel): class PersonPreferenceModel(PerInstancePreferenceModel, PureDjangoModel): + """ Preference model to hold pereferences valid for a person """ + instance = models.ForeignKey(Person, on_delete=models.CASCADE) class Meta: @@ -599,6 +649,8 @@ class PersonPreferenceModel(PerInstancePreferenceModel, PureDjangoModel): class GroupPreferenceModel(PerInstancePreferenceModel, PureDjangoModel): + """ Preference model to hold pereferences valid for members of a group """ + instance = models.ForeignKey(Group, on_delete=models.CASCADE) class Meta: diff --git a/aleksis/core/registries.py b/aleksis/core/registries.py index eab1cb333..cdcd93a68 100644 --- a/aleksis/core/registries.py +++ b/aleksis/core/registries.py @@ -1,15 +1,23 @@ +""" Custom registries for some preference containers """ + from dynamic_preferences.registries import PerInstancePreferenceRegistry class SitePreferenceRegistry(PerInstancePreferenceRegistry): + """ Registry for preferences valid for a site """ + pass class PersonPreferenceRegistry(PerInstancePreferenceRegistry): + """ Registry for preferences valid for a person """ + pass class GroupPreferenceRegistry(PerInstancePreferenceRegistry): + """ Registry for preferences valid for members of a group """ + pass diff --git a/aleksis/core/search_indexes.py b/aleksis/core/search_indexes.py index 5828e0c52..39d6be990 100644 --- a/aleksis/core/search_indexes.py +++ b/aleksis/core/search_indexes.py @@ -3,8 +3,12 @@ from .util.search import Indexable, SearchIndex class PersonIndex(SearchIndex, Indexable): + """ Haystack index for searching persons """ + model = Person class GroupIndex(SearchIndex, Indexable): + """ Haystack index for searching groups """ + model = Group diff --git a/aleksis/core/signals.py b/aleksis/core/signals.py deleted file mode 100644 index bcd55fe62..000000000 --- a/aleksis/core/signals.py +++ /dev/null @@ -1,13 +0,0 @@ -import os -from glob import glob - -from django.conf import settings - - -def clean_scss(*args, **kwargs) -> None: - for source_map in glob(os.path.join(settings.STATIC_ROOT, "*.css.map")): - try: - os.unlink(source_map) - except OSError: - # Ignore because old is better than nothing - pass # noqa diff --git a/aleksis/core/tables.py b/aleksis/core/tables.py index b81902729..8c99bee43 100644 --- a/aleksis/core/tables.py +++ b/aleksis/core/tables.py @@ -3,6 +3,8 @@ from django_tables2.utils import A class PersonsTable(tables.Table): + """ Table to list persons """ + class Meta: attrs = {"class": "table table-striped table-bordered table-hover table-responsive-xl"} @@ -11,6 +13,8 @@ class PersonsTable(tables.Table): class GroupsTable(tables.Table): + """ Table to list groups """ + class Meta: attrs = {"class": "table table-striped table-bordered table-hover table-responsive-xl"} diff --git a/aleksis/core/tasks.py b/aleksis/core/tasks.py index 2c4c40a8f..6fea93569 100644 --- a/aleksis/core/tasks.py +++ b/aleksis/core/tasks.py @@ -6,13 +6,23 @@ from .util.notifications import send_notification as _send_notification @celery_optional def send_notification(notification: int, resend: bool = False) -> None: + """ Send a notification object to its recipient. + + :param notification: primary key of the notification object to send + :param resend: Define whether to also send if the notification was already sent + """ + _send_notification(notification, resend) @celery_optional def backup_data() -> None: + """ Backup database and media using django-dbbackup """ + + # Assemble command-line options for dbbackup management command db_options = "-z " * settings.DBBACKUP_COMPRESS_DB + "-e" * settings.DBBACKUP_ENCRYPT_DB media_options = "-z " * settings.DBBACKUP_COMPRESS_MEDIA + "-e" * settings.DBBACKUP_ENCRYPT_MEDIA + # Hand off to dbbackup's management commands management.call_command("dbbackup", db_options) management.call_command("mediabackup", media_options) diff --git a/aleksis/core/urls.py b/aleksis/core/urls.py index c8995b4c3..8fa00fb04 100644 --- a/aleksis/core/urls.py +++ b/aleksis/core/urls.py @@ -89,4 +89,3 @@ for app_config in apps.app_configs.values(): except ModuleNotFoundError: # Ignore exception as app just has no URLs pass # noqa - diff --git a/aleksis/core/util/apps.py b/aleksis/core/util/apps.py index 45cde67f1..7646acacf 100644 --- a/aleksis/core/util/apps.py +++ b/aleksis/core/util/apps.py @@ -47,11 +47,14 @@ class AppConfig(django.apps.AppConfig): @classmethod def get_name(cls): + """ Get name of application package """ + return getattr(cls, "verbose_name", cls.name) # TODO Try getting from distribution if not set @classmethod def get_version(cls): + """ Get version of application package """ try: from .. import __version__ # noqa except ImportError: @@ -61,6 +64,9 @@ class AppConfig(django.apps.AppConfig): @classmethod def get_licence(cls) -> Tuple: + """ Get tuple of licence information of application package """ + + # Get string representation of licence in SPDX format licence = getattr(cls, "licence", None) default_dict = { @@ -72,27 +78,32 @@ class AppConfig(django.apps.AppConfig): 'referenceNumber': -1, 'url': '', } - if licence: + # Parse licence string into object format licensing = Licensing(LICENSES.keys()) parsed = licensing.parse(licence).simplify() readable = parsed.render_as_readable() + # Collect flags about licence combination (drop to False if any licence is False) flags = { "isFsfLibre": True, "isOsiApproved": True, } + # Fill information dictionaries with missing data licence_dicts = [] - for symbol in parsed.symbols: + # Get licence base information, stripping the "or later" mark licence_dict = LICENSES.get(symbol.key.rstrip("+"), None) if licence_dict is None: + # Fall back to the default dict licence_dict = default_dict else: + # Add missing licence link to SPDX data licence_dict["url"] = "https://spdx.org/licenses/{}.html".format(licence_dict["licenseId"]) + # Drop summed up flags to False if this licence is False flags["isFsfLibre"] = flags["isFsfLibre"] and licence_dict["isFsfLibre"] flags["isOsiApproved"] = flags["isOsiApproved"] and licence_dict["isOsiApproved"] @@ -100,22 +111,27 @@ class AppConfig(django.apps.AppConfig): return (readable, flags, licence_dicts) else: + # We could not find a valid licence return ("Unknown", [default_dict]) @classmethod def get_urls(cls): + """ Get list of URLs for this application package """ + return getattr(cls, "urls", {}) # TODO Try getting from distribution if not set @classmethod def get_copyright(cls) -> Sequence[Tuple[str, str, str]]: + """ Get copyright information tuples for application package """ + copyrights = getattr(cls, "copyright", tuple()) copyrights_processed = [] - for copyright in copyrights: copyrights_processed.append( ( + # Sort copyright years and combine year ranges for display copyright[0] if isinstance(copyright[0], str) else copyright_years(copyright[0]), copyright[1], copyright[2], @@ -123,7 +139,6 @@ class AppConfig(django.apps.AppConfig): ) return copyrights_processed - # TODO Try getting from distribution if not set def preference_updated( diff --git a/aleksis/core/util/core_helpers.py b/aleksis/core/util/core_helpers.py index e63203695..3c624f853 100644 --- a/aleksis/core/util/core_helpers.py +++ b/aleksis/core/util/core_helpers.py @@ -26,6 +26,12 @@ def copyright_years(years: Sequence[int], seperator: str = ", ", joiner: str = " return seperator.join(years_strs) def dt_show_toolbar(request: HttpRequest) -> bool: + """ Helper to determin if Django debug toolbar should be displayed + + Extends the default behaviour by enabling DJDT for superusers independent + of source IP. + """ + from debug_toolbar.middleware import show_toolbar # noqa if not settings.DEBUG: @@ -107,6 +113,8 @@ def lazy_preference(section: str, name: str) -> Callable[[str, str], Any]: def is_impersonate(request: HttpRequest) -> bool: + """ Check whether the user was impersonated by an admin """ + if hasattr(request, "user"): return getattr(request.user, "is_impersonate", False) else: @@ -182,3 +190,32 @@ def custom_information_processor(request: HttpRequest) -> dict: def now_tomorrow() -> datetime: """ Return current time tomorrow """ return timezone.now() + timedelta(days=1) + + +def get_person_by_pk(request: HttpRequest, id_: Optional[int] = None): + """ Get a person by its ID, defaulting to person in request's user """ + + from ..models import Person # noqa + + if id_: + return get_object_or_404(Person, pk=id_) + else: + return request.user.person + + +def get_group_by_pk(request: HttpRequest, id_: Optional[int] = None) -> Group: + """ Get a group by its ID, defaulting to None """ + + if id_: + return get_object_or_404(Group, id=id_) + + return None + + +def get_announcement_by_pk(request: HttpRequest, id_: Optional[int] = None): + """ Get an announcement by its ID; defaulting to None """ + + if id_: + return get_object_or_404(Announcement, pk=pk) + + return None diff --git a/aleksis/core/util/messages.py b/aleksis/core/util/messages.py index e3c93dbb0..4a6d2d430 100644 --- a/aleksis/core/util/messages.py +++ b/aleksis/core/util/messages.py @@ -8,6 +8,12 @@ from django.http import HttpRequest def add_message( request: Optional[HttpRequest], level: int, message: str, **kwargs ) -> Optional[Any]: + """ Add a message to either Django's message framework, if called from a web request, + or to the default logger. + + Default to DEBUG level. + """ + if request: return messages.add_message(request, level, message, **kwargs) else: @@ -15,20 +21,50 @@ def add_message( def debug(request: Optional[HttpRequest], message: str, **kwargs) -> Optional[Any]: + """ Add a message to either Django's message framework, if called from a web request, + or to the default logger. + + Default to DEBUG level. + """ + return add_message(request, messages.DEBUG, message, **kwargs) def info(request: Optional[HttpRequest], message: str, **kwargs) -> Optional[Any]: + """ Add a message to either Django's message framework, if called from a web request, + or to the default logger. + + Default to INFO level. + """ + return add_message(request, messages.INFO, message, **kwargs) def success(request: Optional[HttpRequest], message: str, **kwargs) -> Optional[Any]: + """ Add a message to either Django's message framework, if called from a web request, + or to the default logger. + + Default to SUCCESS level. + """ + return add_message(request, messages.SUCCESS, message, **kwargs) def warning(request: Optional[HttpRequest], message: str, **kwargs) -> Optional[Any]: + """ Add a message to either Django's message framework, if called from a web request, + or to the default logger. + + Default to WARNING level. + """ + return add_message(request, messages.WARNING, message, **kwargs) def error(request: Optional[HttpRequest], message: str, **kwargs) -> Optional[Any]: + """ Add a message to either Django's message framework, if called from a web request, + or to the default logger. + + Default to ERROR level. + """ + return add_message(request, messages.ERROR, message, **kwargs) diff --git a/aleksis/core/util/predicates.py b/aleksis/core/util/predicates.py index 4ea9226c3..396fdbf3a 100644 --- a/aleksis/core/util/predicates.py +++ b/aleksis/core/util/predicates.py @@ -8,9 +8,6 @@ from rules import predicate from .core_helpers import has_person as has_person_helper -# 1. Global permissions (view all, add, change all, delete all) -# 2. Object permissions (view, change, delete) -# 3. Rules from ..models import Group diff --git a/aleksis/core/util/sass_helpers.py b/aleksis/core/util/sass_helpers.py index 11c5608a5..50e886f23 100644 --- a/aleksis/core/util/sass_helpers.py +++ b/aleksis/core/util/sass_helpers.py @@ -1,3 +1,8 @@ +""" Helpers for SASS/SCSS compilation """ + +import os +from glob import glob + from django.conf import settings from colour import web2hex @@ -6,6 +11,8 @@ from sass import SassColor from .core_helpers import get_site_preferences def get_colour(html_colour: str) -> SassColor: + """ Get a SASS colour object from an HTML colour string """ + rgb = web2hex(html_colour, force_long=True)[1:] r, g, b = int(rgb[0:2], 16), int(rgb[2:4], 16), int(rgb[4:6], 16) @@ -13,4 +20,17 @@ def get_colour(html_colour: str) -> SassColor: def get_preference(section: str, name: str) -> str: + """ Get a preference from dynamic-preferences """ + return get_site_preferences()["%s__%s" % (section, name)] + + +def clean_scss(*args, **kwargs) -> None: + """ Unlink compiled CSS (i.e. cache invalidation) """ + + for source_map in glob(os.path.join(settings.STATIC_ROOT, "*.css.map")): + try: + os.unlink(source_map) + except OSError: + # Ignore because old is better than nothing + pass # noqa diff --git a/aleksis/core/views.py b/aleksis/core/views.py index 45b8d36fa..f005233c4 100644 --- a/aleksis/core/views.py +++ b/aleksis/core/views.py @@ -33,10 +33,13 @@ from .registries import site_preferences_registry, group_preferences_registry, p from .tables import GroupsTable, PersonsTable from .util import messages from .util.apps import AppConfig +from .util.core_helpers import get_announcement_by_pk, get_group_by_pk, get_person_by_pk @permission_required("core.view_dashboard") def index(request: HttpRequest) -> HttpResponse: + """ Dashboard """ + context = {} activities = request.user.person.activities.all()[:5] @@ -59,11 +62,15 @@ def index(request: HttpRequest) -> HttpResponse: return render(request, "core/index.html", context) -def offline(request): +def offline(request: HttpRequest) -> HttpResponse: + """ Offline message for PWA """ + return render(request, "core/offline.html") -def about(request): +def about(request: HttpRequest) -> HttpResponse: + """ About page listing all apps """ + context = {} context["app_configs"] = list(filter(lambda a: isinstance(a, AppConfig), apps.get_app_configs())) @@ -73,6 +80,8 @@ def about(request): @permission_required("core.view_persons") def persons(request: HttpRequest) -> HttpResponse: + """ List view listing all persons """ + context = {} # Get all persons @@ -88,20 +97,13 @@ def persons(request: HttpRequest) -> HttpResponse: return render(request, "core/persons.html", context) -def get_person_by_pk(request, id_: Optional[int] = None): - if id_: - return get_object_or_404(Person, pk=id_) - else: - return request.user.person - - @permission_required("core.view_person", fn=get_person_by_pk) def person(request: HttpRequest, id_: Optional[int] = None) -> HttpResponse: + """ Detail view for one person; defaulting to logged-in person """ + context = {} - # Get person and check access person = get_person_by_pk(request, id_) - context["person"] = person # Get groups where person is member of @@ -115,16 +117,13 @@ def person(request: HttpRequest, id_: Optional[int] = None) -> HttpResponse: return render(request, "core/person_full.html", context) -def get_group_by_pk(request: HttpRequest, id_: int) -> Group: - return get_object_or_404(Group, pk=id_) - - @permission_required("core.view_group", fn=get_group_by_pk) def group(request: HttpRequest, id_: int) -> HttpResponse: + """ Detail view for one group """ + context = {} group = get_group_by_pk(request, id_) - context["group"] = group # Get group @@ -151,6 +150,8 @@ def group(request: HttpRequest, id_: int) -> HttpResponse: @permission_required("core.view_groups") def groups(request: HttpRequest) -> HttpResponse: + """ List view for listing all groups """ + context = {} # Get all groups @@ -166,9 +167,14 @@ def groups(request: HttpRequest) -> HttpResponse: @permission_required("core.link_persons_accounts") def persons_accounts(request: HttpRequest) -> HttpResponse: + """ View allowing to batch-process linking of users to persons """ + context = {} + # Get all persons persons_qs = Person.objects.all() + + # Form set with one form per known person persons_accounts_formset = PersonsAccountsFormSet(request.POST or None, queryset=persons_qs) if request.method == "POST": @@ -182,7 +188,8 @@ def persons_accounts(request: HttpRequest) -> HttpResponse: @permission_required("core.assign_child_groups_to_groups") def groups_child_groups(request: HttpRequest) -> HttpResponse: - """ Assign child groups to groups (for matching by MySQL importer) """ + """ View for batch-processing assignment from child groups to groups """ + context = {} # Apply filter @@ -198,7 +205,6 @@ def groups_child_groups(request: HttpRequest) -> HttpResponse: group = page[0] if "save" in request.POST: - # Save form = ChildGroupsForm(request.POST) form.is_valid() @@ -214,28 +220,27 @@ def groups_child_groups(request: HttpRequest) -> HttpResponse: context["page"] = page context["group"] = group context["form"] = form - return render(request, "core/groups_child_groups.html", context) + return render(request, "core/groups_child_groups.html", context) -def get_person_by_id(request: HttpRequest, id_:int): - return get_object_or_404(Person, id=id_) +@permission_required("core.edit_person", fn=get_person_by_pk) +def edit_person(request: HttpRequest, id_: Optional[int] = None) -> HttpResponse: + """ Edit view for a single person, defaulting to logged-in person """ -@permission_required("core.edit_person", fn=get_person_by_id) -def edit_person(request: HttpRequest, id_: int) -> HttpResponse: context = {} - person = get_person_by_id(request, id_) + person = get_person_by_pk(request, id_) + context["person"] = person edit_person_form = EditPersonForm(request.POST or None, request.FILES or None, instance=person) - context["person"] = person - if request.method == "POST": if edit_person_form.is_valid(): edit_person_form.save(commit=True) - messages.success(request, _("The person has been saved.")) + + # Redirect to self to ensure post-processed data is displayed return redirect("edit_person_by_id", id_=person.id) context["edit_person_form"] = edit_person_form @@ -250,15 +255,20 @@ def get_group_by_id(request: HttpRequest, id_: Optional[int] = None): return None -@permission_required("core.edit_group", fn=get_group_by_id) +@permission_required("core.edit_group", fn=get_group_by_pk) def edit_group(request: HttpRequest, id_: Optional[int] = None) -> HttpResponse: + """ View to edit or create a group """ + context = {} - group = get_group_by_id(request, id_) + group = get_group_by_pk(request, id_) + context["group"] = group if id_: + # Edit form for existing group edit_group_form = EditGroupForm(request.POST or None, instance=group) else: + # Empty form to create a new group edit_group_form = EditGroupForm(request.POST or None) if request.method == "POST": @@ -266,9 +276,9 @@ def edit_group(request: HttpRequest, id_: Optional[int] = None) -> HttpResponse: edit_group_form.save(commit=True) messages.success(request, _("The group has been saved.")) + return redirect("groups") - context["group"] = group context["edit_group_form"] = edit_group_form return render(request, "core/edit_group.html", context) @@ -276,18 +286,24 @@ def edit_group(request: HttpRequest, id_: Optional[int] = None) -> HttpResponse: @permission_required("core.manage_data") def data_management(request: HttpRequest) -> HttpResponse: + """ View with special menu for data management """ + context = {} return render(request, "core/data_management.html", context) @permission_required("core.view_system_status") def system_status(request: HttpRequest) -> HttpResponse: + """ View giving information about the system status """ + context = {} return render(request, "core/system_status.html", context) def notification_mark_read(request: HttpRequest, id_: int) -> HttpResponse: + """ Mark a notification read """ + context = {} notification = get_object_or_404(Notification, pk=id_) @@ -298,38 +314,40 @@ def notification_mark_read(request: HttpRequest, id_: int) -> HttpResponse: else: raise PermissionDenied(_("You are not allowed to mark notifications from other users as read!")) + # Redirect to dashboard as this is only used from there if JavaScript is unavailable return redirect("index") @permission_required("core.view_announcements") def announcements(request: HttpRequest) -> HttpResponse: + """ List view of announcements """ + context = {} - # Get all persons + # Get all announcements announcements = Announcement.objects.all() context["announcements"] = announcements return render(request, "core/announcement/list.html", context) -def get_announcement_by_pk(request: HttpRequest, pk: Optional[int] = None): - if pk: - return get_object_or_404(Announcement, pk=pk) - return None - - @permission_required("core.create_or_edit_announcement", fn=get_announcement_by_pk) def announcement_form(request: HttpRequest, pk: Optional[int] = None) -> HttpResponse: + """ View to create or edit an announcement """ + context = {} - if pk: - announcement = get_announcement_by_pk(request, pk) + announcement = get_announcement_by_pk(request, pk) + + if announcement: + # Edit form for existing announcement form = AnnouncementForm( request.POST or None, instance=announcement ) context["mode"] = "edit" else: + # Empty form to create new announcement form = AnnouncementForm(request.POST or None) context["mode"] = "add" @@ -347,6 +365,8 @@ def announcement_form(request: HttpRequest, pk: Optional[int] = None) -> HttpRes @permission_required("core.delete_announcement", fn=get_announcement_by_pk) def delete_announcement(request: HttpRequest, pk: int) -> HttpResponse: + """ View to delete an announcement """ + if request.method == "POST": announcement = get_announcement_by_pk(request, pk) announcement.delete() @@ -357,6 +377,8 @@ def delete_announcement(request: HttpRequest, pk: int) -> HttpResponse: @permission_required("core.search") def searchbar_snippets(request: HttpRequest) -> HttpResponse: + """ View to return HTML snippet with searchbar autocompletion results """ + query = request.GET.get('q', '') limit = int(request.GET.get('limit', '5')) @@ -367,6 +389,8 @@ def searchbar_snippets(request: HttpRequest) -> HttpResponse: class PermissionSearchView(PermissionRequiredMixin, SearchView): + """ Wrapper to apply permission to haystack's search view """ + permission_required = "core.search" def create_response(self): @@ -381,6 +405,7 @@ def preferences(request: HttpRequest, registry_name: str = "person", pk: Optiona context = {} + # Decide which registry to use and check preferences if registry_name == "site": registry = site_preferences_registry instance = request.site @@ -390,31 +415,30 @@ def preferences(request: HttpRequest, registry_name: str = "person", pk: Optiona raise PermissionDenied() elif registry_name == "person": registry = person_preferences_registry - if pk: - instance = get_object_or_404(Person, pk=pk) - else: - instance = request.user.person + instance = get_person_by_pk(request, pk) form_class = PersonPreferenceForm if not request.user.has_perm("core.change_person_preferences", instance): raise PermissionDenied() elif registry_name == "group": registry = group_preferences_registry - instance = get_object_or_404(Group, pk=pk) + instance = get_group_by_pk(request, pk) form_class = GroupPreferenceForm if not request.user.has_perm("core.change_group_preferences", instance): raise PermissionDenied() else: + # Invalid registry name passed from URL return HttpResponseNotFound() + # Build final form from dynamic-preferences form_class = preference_form_builder(form_class, instance=instance, section=section) if request.method == "POST": form = form_class(request.POST) if form.is_valid(): form.update_preferences() - messages.success(request, _("The preferences has been saved successfully.")) + messages.success(request, _("The preferences have been saved successfully.")) else: form = form_class() -- GitLab