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

@@ -46,43 +46,118 @@ class ClubEditForm(forms.ModelForm):
         self.fields["short_description"].widget = forms.Textarea()
 
 
-class MailingForm(forms.ModelForm):
+class MailingForm(forms.Form):
-    class Meta:
+    """
-        model = Mailing
+    Form handling mailing lists right
-        fields = ("email", "club", "moderator")
+    """
+
+    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["mailing_club"].queryset = Club.objects.filter(id=club_id)
-            self.fields["club"].initial = club_id
+            self.fields["mailing_club"].initial = club_id
-            self.fields["club"].widget = forms.HiddenInput()
+            self.fields["mailing_club"].widget = forms.HiddenInput()
-        if user_id >= 0:
+            self.fields["subscription_mailing"].queryset = Mailing.objects.filter(
-            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(
                 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(
+    def check_required(self, cleaned_data, field):
-        "users", label=_("User"), help_text=None, required=False
+        """
+        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):

club/templates/club/mailing.jinja

@@ -5,11 +5,11 @@
 {% endblock %}
 
 {% block content %}
-    {% if has_objects %}
+    {% if mailings %}
 
     <b>{% trans %}Remember : mailing lists need to be moderated, if your new created list is not shown wait until moderation takes action{% endtrans %}</b>
 
-    {% for mailing in object_list %}
+    {% for mailing in mailings %}
     {% if mailing.is_moderated %}
     <h2>{% trans %}Mailing{% endtrans %} {{ mailing.email_full }} 
         {%- if user.is_owner(mailing) -%}
@@ -47,19 +47,45 @@
     <p>{% trans %}No mailing list existing for this club{% endtrans %}</p>
     {% endif %}
 
-    {% if has_objects %}
+    <p>{{ form.non_field_errors() }}</p>
+    {% if mailings_moderated %}
         <h2>{% trans %}New member{% endtrans %}</h2>
-        <form action="{{ url('club:mailing_subscription_create', club_id=club.id) }}" method="post" enctype="multipart/form-data">
+        <form action="{{ url('club:mailing', club_id=club.id) }}" method="post" enctype="multipart/form-data">
             {% csrf_token %}
-            {{ add_member.as_p() }}
+            <p>
+                {{ form.subscription_mailing.errors }}
+                <label for="{{ form.subscription_mailing.id_for_label }}">{{ form.subscription_mailing.label }}</label>
+                {{ form.subscription_mailing }}
+            </p>
+            <p>
+                {{ form.subscription_users.errors }}
+                <label for="{{ form.subscription_users.id_for_label }}">{{ form.subscription_users.label }}</label>
+                {{ form.subscription_users }}
+                <span class="helptext">{{ form.subscription_users.help_text }}</span>
+            </p>
+            <p>
+                {{ form.subscription_email.errors }}
+                <label for="{{ form.subscription_email.id_for_label }}">{{ form.subscription_email.label }}</label>
+                {{ form.subscription_email }}
+            </p>
+            <input hidden type="number" name="{{ form.action.name }}" value="{{ form_actions.NEW_SUBSCRIPTION }}" />
             <p><input type="submit" value="{% trans %}Add to mailing list{% endtrans %}" /></p>
         </form>
     {% endif %}
 
     <h2>{% trans %}New mailing{% endtrans %}</h2>
-    <form action="{{ url('club:mailing_create', club_id=club.id) }}" method="post" enctype="multipart/form-data">
+    <form action="{{ url('club:mailing', club_id=club.id) }}" method="post" enctype="multipart/form-data">
         {% csrf_token %}
-        {{ add_mailing.as_p() }}
+        {{ form.mailing_club.errors }}
+        {{ form.mailing_moderator.errors }}
+        <p>
+            {{ form.mailing_email.errors }}
+            <label for="{{ form.mailing_email.id_for_label }}">{{ form.mailing_email.label }}</label>
+            {{ form.mailing_email }}
+        </p>
+        {{ form.mailing_club }}
+        {{ form.mailing_moderator }}
+        <input hidden type="number" name="{{ form.action.name }}" value="{{ form_actions.NEW_MALING }}" />
         <p><input type="submit" value="{% trans %}Create mailing list{% endtrans %}" /></p>
     </form>
 

club/urls.py

@@ -64,21 +64,7 @@ urlpatterns = [
     ),
     url(r"^(?P<club_id>[0-9]+)/prop$", ClubEditPropView.as_view(), name="club_prop"),
     url(r"^(?P<club_id>[0-9]+)/tools$", ClubToolsView.as_view(), name="tools"),
-    url(
+    url(r"^(?P<club_id>[0-9]+)/mailing$", ClubMailingView.as_view(), name="mailing"),
-        r"^(?P<club_id>[0-9]+)/mailing$",
-        ClubMailingView.as_view(action="display"),
-        name="mailing",
-    ),
-    url(
-        r"^(?P<club_id>[0-9]+)/mailing/new/mailing$",
-        ClubMailingView.as_view(action="add_mailing"),
-        name="mailing_create",
-    ),
-    url(
-        r"^(?P<club_id>[0-9]+)/mailing/new/subscription$",
-        ClubMailingView.as_view(action="add_member"),
-        name="mailing_subscription_create",
-    ),
     url(
         r"^(?P<mailing_id>[0-9]+)/mailing/generate$",
         MailingAutoGenerationView.as_view(),

club/views.py

@@ -59,12 +59,7 @@ from com.views import (
 )
 
 from club.models import Club, Membership, Mailing, MailingSubscription
-from club.forms import (
+from club.forms import MailingForm, ClubEditForm, ClubMemberForm
-    MailingForm,
-    MailingSubscriptionForm,
-    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 = Club
-    model = Mailing
+    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)
+            self.get_object().has_rights_in_club(self.request.user)
-            or self.user.is_root
+            or self.request.user.is_root
-            or self.user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID)
+            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"])
+        res = super(ClubMailingView, self).dispatch(request, *args, **kwargs)
-        self.user = request.user
-        self.object = self.club
         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["club"] = self.get_object()
-        kwargs["add_mailing"] = self.mailing_form
+        kwargs["user"] = self.request.user
-        kwargs["club"] = self.club
+        kwargs["mailings"] = Mailing.objects.filter(club_id=self.get_object().id).all()
-        kwargs["user"] = self.user
+        kwargs["mailings_moderated"] = (
-        kwargs["has_objects"] = len(kwargs["object_list"]) > 0
+            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):
+    def add_new_mailing(self, cleaned_data):
-        return self.club
-
-
-class MailingGenericCreateView(CreateView, SingleObjectMixin):
         """
-    Create a new mailing list
+        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()
 
-    list_view = None
+    def add_new_subscription(self, cleaned_data):
-    form_class = None
+        """
+        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()
 
-    def get_context_data(self, **kwargs):
+        if cleaned_data["subscription_email"]:
-        view_kwargs = self.list_view.get_context_data(**kwargs)
+            sub = MailingSubscription(
-        for key, data in (
+                mailing=cleaned_data["subscription_mailing"],
-            super(MailingGenericCreateView, self).get_context_data(**kwargs).items()
+                email=cleaned_data["subscription_email"],
-        ):
+            )
-            view_kwargs[key] = data
+            sub.clean()
-        view_kwargs[self.list_view.action] = view_kwargs["form"]
+            sub.save()
-        return view_kwargs
 
-    def get_form_kwargs(self):
+    def form_valid(self, form):
-        kwargs = super(MailingGenericCreateView, self).get_form_kwargs()
+        resp = super(ClubMailingView, self).form_valid(form)
-        kwargs["club_id"] = self.list_view.club.id
-        kwargs["user_id"] = self.list_view.user.id
-        return kwargs
 
-    def dispatch(self, request, *args, **kwargs):
+        cleaned_data = form.clean()
-        if not self.list_view.authorized():
+
-            raise PermissionDenied
+        if cleaned_data["action"] == self.form_class.ACTION_NEW_MAILING:
-        self.template_name = self.list_view.template_name
+            self.add_new_mailing(cleaned_data)
-        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):

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 *