From a0e39b8904b19b4a77714fec5c1c2bd857263eef Mon Sep 17 00:00:00 2001 From: Bartuccio Antoine Date: Wed, 1 May 2019 03:32:28 +0200 Subject: [PATCH] clubs: rewrite MailingForm to include everything in one place Everything is handled on the same view, no more redirection hacks Remove get_context_data in DetailFormView since it's already done by django --- club/forms.py | 131 +++++++++++++++++++++++------- club/templates/club/mailing.jinja | 40 +++++++-- club/urls.py | 16 +--- club/views.py | 131 +++++++++++++++--------------- core/views/__init__.py | 5 -- 5 files changed, 202 insertions(+), 121 deletions(-) diff --git a/club/forms.py b/club/forms.py index fdfa8256..5b62df89 100644 --- a/club/forms.py +++ b/club/forms.py @@ -46,43 +46,118 @@ class ClubEditForm(forms.ModelForm): self.fields["short_description"].widget = forms.Textarea() -class MailingForm(forms.ModelForm): - class Meta: - model = Mailing - fields = ("email", "club", "moderator") +class MailingForm(forms.Form): + """ + Form handling mailing lists right + """ + + ACTION_NEW_MAILING = 1 + ACTION_NEW_SUBSCRIPTION = 2 + + subscription_users = AutoCompleteSelectMultipleField( + "users", + label=_("Users to add"), + help_text=_("Search users to add (one or more)."), + required=False, + ) def __init__(self, *args, **kwargs): club_id = kwargs.pop("club_id", None) user_id = kwargs.pop("user_id", -1) # Remember 0 is treated as None super(MailingForm, self).__init__(*args, **kwargs) + + self.fields["action"] = forms.TypedChoiceField( + ( + (self.ACTION_NEW_MAILING, _("New Mailing")), + (self.ACTION_NEW_SUBSCRIPTION, _("Subscribe")), + ), + coerce=int, + label=_("Action"), + initial=1, + required=True, + widget=forms.HiddenInput(), + ) + + # Include fields for handling mailing creation + mailing_fields = ("email", "club", "moderator") + self.fields.update(forms.fields_for_model(Mailing, fields=mailing_fields)) + for field in mailing_fields: + self.fields["mailing_" + field] = self.fields.pop(field) + self.fields["mailing_" + field].required = False + + # Include fields for handling subscription creation + subscription_fields = ("mailing", "email") + self.fields.update( + forms.fields_for_model(MailingSubscription, fields=subscription_fields) + ) + for field in subscription_fields: + self.fields["subscription_" + field] = self.fields.pop(field) + self.fields["subscription_" + field].required = False + if club_id: - self.fields["club"].queryset = Club.objects.filter(id=club_id) - self.fields["club"].initial = club_id - self.fields["club"].widget = forms.HiddenInput() - if user_id >= 0: - self.fields["moderator"].queryset = User.objects.filter(id=user_id) - self.fields["moderator"].initial = user_id - self.fields["moderator"].widget = forms.HiddenInput() - - -class MailingSubscriptionForm(forms.ModelForm): - class Meta: - model = MailingSubscription - fields = ("mailing", "user", "email") - - def __init__(self, *args, **kwargs): - kwargs.pop("user_id", None) # For standart interface - club_id = kwargs.pop("club_id", None) - super(MailingSubscriptionForm, self).__init__(*args, **kwargs) - self.fields["email"].required = False - if club_id: - self.fields["mailing"].queryset = Mailing.objects.filter( + self.fields["mailing_club"].queryset = Club.objects.filter(id=club_id) + self.fields["mailing_club"].initial = club_id + self.fields["mailing_club"].widget = forms.HiddenInput() + self.fields["subscription_mailing"].queryset = Mailing.objects.filter( club__id=club_id, is_moderated=True ) + if user_id >= 0: + self.fields["mailing_moderator"].queryset = User.objects.filter(id=user_id) + self.fields["mailing_moderator"].initial = user_id + self.fields["mailing_moderator"].widget = forms.HiddenInput() - user = AutoCompleteSelectField( - "users", label=_("User"), help_text=None, required=False - ) + def check_required(self, cleaned_data, field): + """ + If the given field doesn't exist or has no value, add a required error on it + """ + if not cleaned_data.get(field, None): + self.add_error(field, _("This field is required")) + + def clean_subscription_users(self): + """ + Convert given users into real users and check their validity + """ + cleaned_data = super(MailingForm, self).clean() + users = [] + for user in cleaned_data["subscription_users"]: + user = User.objects.filter(id=user).first() + if not user: + raise forms.ValidationError( + _("One of the selected users doesn't exist"), code="invalid" + ) + if not user.email: + raise forms.ValidationError( + _("One of the selected users doesn't have an email address"), + code="invalid", + ) + users.append(user) + return users + + def clean(self): + cleaned_data = super(MailingForm, self).clean() + + print(cleaned_data) + + if not "action" in cleaned_data: + # If there is no action provided, we can stop here + raise forms.ValidationError(_("An action is required"), code="invalid") + + if cleaned_data["action"] == self.ACTION_NEW_MAILING: + self.check_required(cleaned_data, "mailing_email") + self.check_required(cleaned_data, "mailing_club") + self.check_required(cleaned_data, "mailing_moderator") + + if cleaned_data["action"] == self.ACTION_NEW_SUBSCRIPTION: + self.check_required(cleaned_data, "subscription_mailing") + if not cleaned_data.get( + "subscription_users", None + ) and not cleaned_data.get("subscription_email", None): + raise forms.ValidationError( + _("You must specify at least an user or an email address"), + code="invalid", + ) + + return cleaned_data class SellingsFormBase(forms.Form): diff --git a/club/templates/club/mailing.jinja b/club/templates/club/mailing.jinja index 9c2af2b8..7d485338 100644 --- a/club/templates/club/mailing.jinja +++ b/club/templates/club/mailing.jinja @@ -5,11 +5,11 @@ {% endblock %} {% block content %} - {% if has_objects %} + {% if mailings %} {% trans %}Remember : mailing lists need to be moderated, if your new created list is not shown wait until moderation takes action{% endtrans %} - {% for mailing in object_list %} + {% for mailing in mailings %} {% if mailing.is_moderated %}

{% trans %}Mailing{% endtrans %} {{ mailing.email_full }} {%- if user.is_owner(mailing) -%} @@ -47,19 +47,45 @@

{% trans %}No mailing list existing for this club{% endtrans %}

{% endif %} - {% if has_objects %} +

{{ form.non_field_errors() }}

+ {% if mailings_moderated %}

{% trans %}New member{% endtrans %}

-
+ {% csrf_token %} - {{ add_member.as_p() }} +

+ {{ form.subscription_mailing.errors }} + + {{ form.subscription_mailing }} +

+

+ {{ form.subscription_users.errors }} + + {{ form.subscription_users }} + {{ form.subscription_users.help_text }} +

+

+ {{ form.subscription_email.errors }} + + {{ form.subscription_email }} +

+

{% endif %}

{% trans %}New mailing{% endtrans %}

-
+ {% csrf_token %} - {{ add_mailing.as_p() }} + {{ form.mailing_club.errors }} + {{ form.mailing_moderator.errors }} +

+ {{ form.mailing_email.errors }} + + {{ form.mailing_email }} +

+ {{ form.mailing_club }} + {{ form.mailing_moderator }} +

diff --git a/club/urls.py b/club/urls.py index 8f446d65..d560a560 100644 --- a/club/urls.py +++ b/club/urls.py @@ -64,21 +64,7 @@ urlpatterns = [ ), url(r"^(?P[0-9]+)/prop$", ClubEditPropView.as_view(), name="club_prop"), url(r"^(?P[0-9]+)/tools$", ClubToolsView.as_view(), name="tools"), - url( - r"^(?P[0-9]+)/mailing$", - ClubMailingView.as_view(action="display"), - name="mailing", - ), - url( - r"^(?P[0-9]+)/mailing/new/mailing$", - ClubMailingView.as_view(action="add_mailing"), - name="mailing_create", - ), - url( - r"^(?P[0-9]+)/mailing/new/subscription$", - ClubMailingView.as_view(action="add_member"), - name="mailing_subscription_create", - ), + url(r"^(?P[0-9]+)/mailing$", ClubMailingView.as_view(), name="mailing"), url( r"^(?P[0-9]+)/mailing/generate$", MailingAutoGenerationView.as_view(), diff --git a/club/views.py b/club/views.py index c51b174f..7aae48f6 100644 --- a/club/views.py +++ b/club/views.py @@ -59,12 +59,7 @@ from com.views import ( ) from club.models import Club, Membership, Mailing, MailingSubscription -from club.forms import ( - MailingForm, - MailingSubscriptionForm, - ClubEditForm, - ClubMemberForm, -) +from club.forms import MailingForm, ClubEditForm, ClubMemberForm class ClubTabsMixin(TabedViewMixin): @@ -507,94 +502,98 @@ class ClubStatView(TemplateView): return kwargs -class ClubMailingView(ClubTabsMixin, ListView): +class ClubMailingView(ClubTabsMixin, DetailFormView): """ A list of mailing for a given club """ - action = None - model = Mailing + model = Club + form_class = MailingForm + pk_url_kwarg = "club_id" template_name = "club/mailing.jinja" current_tab = "mailing" def authorized(self): return ( - self.club.has_rights_in_club(self.user) - or self.user.is_root - or self.user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID) + self.get_object().has_rights_in_club(self.request.user) + or self.request.user.is_root + or self.request.user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID) ) + def get_form_kwargs(self, *args, **kwargs): + kwargs = super(ClubMailingView, self).get_form_kwargs(*args, **kwargs) + kwargs["club_id"] = self.get_object().id + kwargs["user_id"] = self.request.user.id + return kwargs + def dispatch(self, request, *args, **kwargs): - self.club = get_object_or_404(Club, pk=kwargs["club_id"]) - self.user = request.user - self.object = self.club + res = super(ClubMailingView, self).dispatch(request, *args, **kwargs) if not self.authorized(): raise PermissionDenied - self.member_form = MailingSubscriptionForm(club_id=self.club.id) - self.mailing_form = MailingForm(club_id=self.club.id, user_id=self.user.id) - return super(ClubMailingView, self).dispatch(request, *args, **kwargs) - - def post(self, request, *args, **kwargs): - res = super(ClubMailingView, self).get(request, *args, **kwargs) - if self.action != "display": - if self.action == "add_mailing": - form = MailingForm - model = Mailing - elif self.action == "add_member": - form = MailingSubscriptionForm - model = MailingSubscription - return MailingGenericCreateView.as_view( - model=model, list_view=self, form_class=form - )(request, *args, **kwargs) return res - def get_queryset(self): - return Mailing.objects.filter(club_id=self.club.id).all() - def get_context_data(self, **kwargs): + self.object = self.get_object() kwargs = super(ClubMailingView, self).get_context_data(**kwargs) - kwargs["add_member"] = self.member_form - kwargs["add_mailing"] = self.mailing_form - kwargs["club"] = self.club - kwargs["user"] = self.user - kwargs["has_objects"] = len(kwargs["object_list"]) > 0 + kwargs["club"] = self.get_object() + kwargs["user"] = self.request.user + kwargs["mailings"] = Mailing.objects.filter(club_id=self.get_object().id).all() + kwargs["mailings_moderated"] = ( + kwargs["mailings"].exclude(is_moderated=False).all() + ) + kwargs["form_actions"] = { + "NEW_MALING": self.form_class.ACTION_NEW_MAILING, + "NEW_SUBSCRIPTION": self.form_class.ACTION_NEW_SUBSCRIPTION, + } return kwargs - def get_object(self): - return self.club + def add_new_mailing(self, cleaned_data): + """ + Create a new mailing list from the form + """ + mailing = Mailing( + club=cleaned_data["mailing_club"], + email=cleaned_data["mailing_email"], + moderator=cleaned_data["mailing_moderator"], + is_moderated=False, + ) + mailing.clean() + mailing.save() + def add_new_subscription(self, cleaned_data): + """ + Add mailing subscriptions for each user given and/or for the specified email in form + """ + for user in cleaned_data["subscription_users"]: + sub = MailingSubscription( + mailing=cleaned_data["subscription_mailing"], user=user + ) + sub.clean() + sub.save() -class MailingGenericCreateView(CreateView, SingleObjectMixin): - """ - Create a new mailing list - """ + if cleaned_data["subscription_email"]: + sub = MailingSubscription( + mailing=cleaned_data["subscription_mailing"], + email=cleaned_data["subscription_email"], + ) + sub.clean() + sub.save() - list_view = None - form_class = None + def form_valid(self, form): + resp = super(ClubMailingView, self).form_valid(form) - def get_context_data(self, **kwargs): - view_kwargs = self.list_view.get_context_data(**kwargs) - for key, data in ( - super(MailingGenericCreateView, self).get_context_data(**kwargs).items() - ): - view_kwargs[key] = data - view_kwargs[self.list_view.action] = view_kwargs["form"] - return view_kwargs + cleaned_data = form.clean() - def get_form_kwargs(self): - kwargs = super(MailingGenericCreateView, self).get_form_kwargs() - kwargs["club_id"] = self.list_view.club.id - kwargs["user_id"] = self.list_view.user.id - return kwargs + if cleaned_data["action"] == self.form_class.ACTION_NEW_MAILING: + self.add_new_mailing(cleaned_data) - def dispatch(self, request, *args, **kwargs): - if not self.list_view.authorized(): - raise PermissionDenied - self.template_name = self.list_view.template_name - return super(MailingGenericCreateView, self).dispatch(request, *args, **kwargs) + if cleaned_data["action"] == self.form_class.ACTION_NEW_SUBSCRIPTION: + self.add_new_subscription(cleaned_data) + + return resp def get_success_url(self, **kwargs): - return reverse_lazy("club:mailing", kwargs={"club_id": self.list_view.club.id}) + return reverse_lazy("club:mailing", kwargs={"club_id": self.get_object().id}) class MailingDeleteView(CanEditMixin, DeleteView): diff --git a/core/views/__init__.py b/core/views/__init__.py index c4972bd0..202b08fa 100644 --- a/core/views/__init__.py +++ b/core/views/__init__.py @@ -296,11 +296,6 @@ class DetailFormView(SingleObjectMixin, FormView): """ return super(DetailFormView, self).get_object() - def get_context_data(self, *args, **kwargs): - kwargs = super(DetailFormView, self).get_context_data() - kwargs["object"] = self.get_object() - return kwargs - from .user import * from .page import *