Refactor views and forms for creating/editing persons

aleksis/core/forms.py

@@ -13,7 +13,6 @@ from allauth.account.forms import SignupForm
 from allauth.account.utils import get_user_model, setup_user_email
 from django_select2.forms import ModelSelect2MultipleWidget, ModelSelect2Widget, Select2Widget
 from dynamic_preferences.forms import PreferenceForm
-from guardian.core import ObjectPermissionChecker
 from material import Fieldset, Layout, Row
 
 from .mixins import ExtensibleForm, SchoolTermRelatedExtensibleForm
@@ -82,8 +81,8 @@ PersonsAccountsFormSet = forms.modelformset_factory(
 )
 
 
-class EditPersonForm(ExtensibleForm):
-    """Form to edit an existing person object in the frontend."""
+class PersonForm(ExtensibleForm):
+    """Form to edit or add a person object in the frontend."""
 
     layout = Layout(
         Fieldset(
@@ -142,7 +141,8 @@ class EditPersonForm(ExtensibleForm):
         required=False, label=_("New user"), help_text=_("Create a new account")
     )
 
-    def __init__(self, request: HttpRequest, *args, **kwargs):
+    def __init__(self, *args, **kwargs):
+        request = kwargs.pop("request", None)
         super().__init__(*args, **kwargs)
 
         # Disable non-editable fields

aleksis/core/models.py

@@ -32,6 +32,7 @@ from model_utils import FieldTracker
 from model_utils.models import TimeStampedModel
 from phonenumber_field.modelfields import PhoneNumberField
 from polymorphic.models import PolymorphicModel
+from templated_email import send_templated_mail
 
 from aleksis.core.data_checks import BrokenDashboardWidgetDataCheck, DataCheck, DataCheckRegistry
 
@@ -329,6 +330,22 @@ class Person(ExtensibleModel):
             if force or not self.primary_group:
                 self.primary_group = self.member_of.filter(**{f"{field}__regex": pattern}).first()
 
+    def notify_about_changed_data(
+        self, changed_fields: Iterable[str], recipients: Optional[List[str]] = None
+    ):
+        """Notify (configured) recipients about changed data of this person."""
+        context = {"person": self, "changed_fields": changed_fields}
+        recipients = recipients or [
+            get_site_preferences()["account__person_change_notification_contact"]
+        ]
+        send_templated_mail(
+            template_name="person_changed",
+            from_email=self.mail_sender_via,
+            headers={"Reply-To": self.mail_sender, "Sender": self.mail_sender,},
+            recipient_list=recipients,
+            context=context,
+        )
+
 
 class DummyPerson(Person):
     """A dummy person that is not stored into the database.

aleksis/core/templates/core/person/create.html

+{# -*- engine:django -*- #}
+
+{% extends "core/base.html" %}
+
+{% load material_form i18n any_js %}
+
+{% block extra_head %}
+  {{ form.media.css }}
+  {% include_css "select2-materialize" %}
+{% endblock %}
+
+{% block browser_title %}{% blocktrans %}Create person{% endblocktrans %}{% endblock %}
+{% block page_title %}{% blocktrans %}Create person{% endblocktrans %}{% endblock %}
+
+
+{% block content %}
+  <form method="post" enctype="multipart/form-data">
+    {% csrf_token %}
+    {% form form=form %}{% endform %}
+    {% include "core/partials/save_button.html" %}
+  </form>
+  {% include_js "select2-materialize" %}
+  {{ form.media.js }}
+{% endblock %}

aleksis/core/templates/core/person/edit.html

@@ -5,7 +5,7 @@
 {% load material_form i18n any_js %}
 
 {% block extra_head %}
-  {{ edit_person_form.media }}
+  {{ form.media.css }}
   {% include_css "select2-materialize" %}
 {% endblock %}
 
@@ -14,13 +14,11 @@
 
 
 {% block content %}
-
   <form method="post" enctype="multipart/form-data">
     {% csrf_token %}
-    {% form form=edit_person_form %}{% endform %}
+    {% form form=form %}{% endform %}
     {% include "core/partials/save_button.html" %}
   </form>
   {% include_js "select2-materialize" %}
-  {{ edit_group_form.media.js }}
-
+  {{ form.media.js }}
 {% endblock %}

aleksis/core/templates/templated_email/person_changed.email

@@ -11,7 +11,7 @@
    the person {{ person }} recently changed the following fields:
  {% endblocktrans %}
 
- {% for field in send_notification_fields %}
+ {% for field in changed_fields %}
   * {{ field }}
  {% endfor %}
 {% endblock %}
@@ -25,7 +25,7 @@
  </p>
 
  <ul>
-  {% for field in send_notification_fields %}
+  {% for field in changed_fields %}
    <li>{{ field }}</li>
   {% endfor %}
  </ul>

aleksis/core/urls.py

@@ -47,11 +47,11 @@ urlpatterns = [
     path("school_terms/<int:pk>/", views.SchoolTermEditView.as_view(), name="edit_school_term"),
     path("persons", views.persons, name="persons"),
     path("persons/accounts", views.persons_accounts, name="persons_accounts"),
-    path("person", views.person, name="person"),
-    path("person/create", views.edit_person, name="create_person"),
-    path("person/<int:id_>", views.person, name="person_by_id"),
-    path("person/<int:id_>/edit", views.edit_person, name="edit_person_by_id"),
-    path("person/<int:id_>/delete", views.delete_person, name="delete_person_by_id"),
+    path("person/", views.person, name="person"),
+    path("person/create/", views.CreatePersonView.as_view(), name="create_person"),
+    path("person/<int:id_>/", views.person, name="person_by_id"),
+    path("person/<int:pk>/edit/", views.EditPersonView.as_view(), name="edit_person_by_id"),
+    path("person/<int:id_>/delete/", views.delete_person, name="delete_person_by_id"),
     path("groups", views.groups, name="groups"),
     path("groups/additional_fields", views.additional_fields, name="additional_fields"),
     path("groups/child_groups/", views.groups_child_groups, name="groups_child_groups"),

aleksis/core/views.py

@@ -47,7 +47,6 @@ from oauth2_provider.models import Application
 from reversion import set_user
 from reversion.views import RevisionMixin
 from rules.contrib.views import PermissionRequiredMixin, permission_required
-from templated_email import send_templated_mail
 
 from aleksis.core.data_checks import DataCheckRegistry, check_data
 
@@ -60,8 +59,8 @@ from .forms import (
     EditAdditionalFieldForm,
     EditGroupForm,
     EditGroupTypeForm,
-    EditPersonForm,
     GroupPreferenceForm,
+    PersonForm,
     PersonPreferenceForm,
     PersonsAccountsFormSet,
     SchoolTermForm,
@@ -420,59 +419,38 @@ def groups_child_groups(request: HttpRequest) -> HttpResponse:
     return render(request, "core/group/child_groups.html", context)
 
 
-@never_cache
-@permission_required("core.edit_person_rule", fn=objectgetter_optional(Person))
-def edit_person(request: HttpRequest, id_: Optional[int] = None) -> HttpResponse:
-    """Edit view for a single person, defaulting to logged-in person."""
-    context = {}
-
-    person = objectgetter_optional(Person)(request, id_)
-    context["person"] = person
-
-    if id_:
-        # Edit form for existing group
-        edit_person_form = EditPersonForm(
-            request, request.POST or None, request.FILES or None, instance=person
-        )
-    else:
-        # Empty form to create a new group
-        if request.user.has_perm("core.create_person_rule"):
-            edit_person_form = EditPersonForm(request, request.POST or None, request.FILES or None)
-        else:
-            raise PermissionDenied()
-    if request.method == "POST":
-        if edit_person_form.is_valid():
-            if person and person == request.user.person:
-                # Check if user edited non-editable field
-                notification_fields = get_site_preferences()[
-                    "account__notification_on_person_change"
-                ]
-                send_notification_fields = set(edit_person_form.changed_data).intersection(
-                    set(notification_fields)
-                )
-                context["send_notification_fields"] = send_notification_fields
-                if send_notification_fields:
-                    context["send_notification_fields"] = send_notification_fields
-                    send_templated_mail(
-                        template_name="person_changed",
-                        from_email=request.user.person.mail_sender_via,
-                        headers={
-                            "Reply-To": request.user.person.mail_sender,
-                            "Sender": request.user.person.mail_sender,
-                        },
-                        recipient_list=[
-                            get_site_preferences()["account__person_change_notification_contact"]
-                        ],
-                        context=context,
-                    )
-            with reversion.create_revision():
-                set_user(request.user)
-                edit_person_form.save(commit=True)
-            messages.success(request, _("The person has been saved."))
+@method_decorator(never_cache, name="dispatch")
+class CreatePersonView(PermissionRequiredMixin, AdvancedCreateView):
+    form_class = PersonForm
+    model = Person
+    permission_required = "core.create_person_rule"
+    template_name = "core/person/create.html"
+    success_message = _("The person has been saved.")
 
-    context["edit_person_form"] = edit_person_form
 
-    return render(request, "core/person/edit.html", context)
+@method_decorator(never_cache, name="dispatch")
+class EditPersonView(PermissionRequiredMixin, RevisionMixin, AdvancedEditView):
+    form_class = PersonForm
+    model = Person
+    permission_required = "core.edit_person_rule"
+    context_object_name = "person"
+    template_name = "core/person/edit.html"
+    success_message = _("The person has been saved.")
+
+    def get_form_kwargs(self):
+        kwargs = super().get_form_kwargs()
+        kwargs["request"] = self.request
+        return kwargs
+
+    def form_valid(self, form):
+        if self.object == self.request.user.person:
+            # Get all changed fields and send a notification about them
+            notification_fields = get_site_preferences()["account__notification_on_person_change"]
+            send_notification_fields = set(form.changed_data).intersection(set(notification_fields))
+
+            if send_notification_fields:
+                self.object.notify_about_changed_data(send_notification_fields)
+        return super().form_valid(form)
 
 
 def get_group_by_id(request: HttpRequest, id_: Optional[int] = None):