diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index b6e8fc864b8f3830a20c52c2b2c31f2d0658092e..2143c71079fee7f6a646683581d585f070957b4e 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -14,6 +14,10 @@ Added
* Provide an ``ExtensiblePolymorphicModel`` to support the features of extensible models for polymorphic models and vice-versa.
+Changed
+~~~~~~~
+
+* Refactor views/forms for creating/editing persons.
Fixed
~~~~~
@@ -21,6 +25,8 @@ Fixed
* Fix order of submit buttons in login form and restructure login template
to make 2FA work correctly.
* Fix page title bug on the impersonate page.
+* Users were able to edit the linked user if self-editing was activated.
+* Users weren't able to edit the allowed fields although they were configured correctly.
* Provide `style.css` and icon files without any authentication to avoid caching issues.
`2.0rc7`_ - 2021-10-18
diff --git a/aleksis/core/forms.py b/aleksis/core/forms.py
index 642364b1b331f37720bdbb5798ac20b33786b79b..f789386bc31c369d44efb28edddf1ae39ca91fcd 100644
--- a/aleksis/core/forms.py
+++ b/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,21 +141,25 @@ 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
- person_fields = set([field.name for field in Person.syncable_fields()]).intersection(
- set(self.fields)
- )
+ allowed_person_fields = get_site_preferences()["account__editable_fields_person"]
- if self.instance:
- checker = ObjectPermissionChecker(request.user)
- checker.prefetch_perms([self.instance])
+ if (
+ request
+ and self.instance
+ and not request.user.has_perm("core.change_person", self.instance)
+ ):
+ # First, disable all fields
+ for field in self.fields:
+ self.fields[field].disabled = True
- for field in person_fields:
- if not checker.has_perm(f"core.change_person_field_{field}", self.instance):
- self.fields[field].disabled = True
+ # Then, activate allowed fields
+ for field in allowed_person_fields:
+ self.fields[field].disabled = False
def clean(self) -> None:
# Use code implemented in dedicated form to verify user selection
diff --git a/aleksis/core/models.py b/aleksis/core/models.py
index 8fffbe122638eebee19e199914ce0b7edbc91837..f391fd02fedc22d31e1b96c78e7c8509e7a54b33 100644
--- a/aleksis/core/models.py
+++ b/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.
diff --git a/aleksis/core/rules.py b/aleksis/core/rules.py
index 9b8f7cc1f3c9aafcbb91a5ed06685d33e15db7d5..99c0493620f4411847e142ff986292e9947436d7 100644
--- a/aleksis/core/rules.py
+++ b/aleksis/core/rules.py
@@ -2,7 +2,6 @@ import rules
from .models import AdditionalField, Announcement, Group, GroupType, Person
from .util.predicates import (
- contains_site_preference_value,
has_any_object,
has_global_perm,
has_object_perm,
@@ -350,15 +349,3 @@ rules.add_perm("core.upload_files_ckeditor_rule", upload_files_ckeditor_predicat
test_pdf_generation_predicate = has_person & has_global_perm("core.test_pdf")
rules.add_perm("core.test_pdf_rule", test_pdf_generation_predicate)
-
-# Generate rules for syncable fields
-for field in Person._meta.fields:
- perm = (
- has_global_perm("core.edit_person")
- | has_object_perm("core.edit_person")
- | (
- is_current_person
- & contains_site_preference_value("account", "editable_fields_person", field.name)
- )
- )
- rules.add_perm(f"core.change_person_field_{field.name}_rule", perm)
diff --git a/aleksis/core/templates/core/person/create.html b/aleksis/core/templates/core/person/create.html
new file mode 100644
index 0000000000000000000000000000000000000000..08a6639e05e45f82912fd3e4adb25021264010c9
--- /dev/null
+++ b/aleksis/core/templates/core/person/create.html
@@ -0,0 +1,24 @@
+{# -*- 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 %}
diff --git a/aleksis/core/templates/core/person/edit.html b/aleksis/core/templates/core/person/edit.html
index 3bf16ca3521ea744acfe48c6829dfae21db06eb5..788721c3541cba056c1ef05214eb0c39eb1b6f69 100644
--- a/aleksis/core/templates/core/person/edit.html
+++ b/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 %}
diff --git a/aleksis/core/templates/templated_email/person_changed.email b/aleksis/core/templates/templated_email/person_changed.email
index 2e1db653256873d3f72afcd34a1e19914d116480..1e0d8be25684486bfdceac0b979b461253c21df7 100644
--- a/aleksis/core/templates/templated_email/person_changed.email
+++ b/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>
diff --git a/aleksis/core/urls.py b/aleksis/core/urls.py
index b09c6ff9456e1b7e749301a4f261fb5e642e380c..92107e30f5a237ef0efd834f3e5bd4279e19b705 100644
--- a/aleksis/core/urls.py
+++ b/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"),
diff --git a/aleksis/core/views.py b/aleksis/core/views.py
index e24bafa1705465173a8ae9d54669ae23d880ee04..7e7fd7739b68c30fb8d8de6c268fd6a4bf21a521 100644
--- a/aleksis/core/views.py
+++ b/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):