mkoistinen / cmsplugin-form-handler

This package provides a mechanism for handling form-submissions in django-CMS plugins.
MIT License
14 stars 10 forks source link

The form is executed twice #2

Closed edgarzamora closed 8 years ago

edgarzamora commented 8 years ago

Hello, I am trying to integrate this plugin on one of my projects. The problem is that the form that I am trying to do with this plugin is executed twice. It is a form that appears in the footer of all the pages.

I follow the documentation of this plugin to integrate this form. I am using Django 1.6.11 and Django-cms 3.1.

Debugging the application I noted that:

The view that handle the form petition is: https://github.com/mkoistinen/cmsplugin-form-handler/blob/master/cmsplugin_form_handler/views.py

This view inherits from FormMixin, which implements the method post. So, the post method of this FormMixin is the first method executed when I submit the form. Then it goes to get_form_class() method, which is declared in the view that I attached the link above, then this method return the plugin.get_form_class(), that comes back to the post method of FromMixin class. This post method executes the form, so the method clean is executed, the method is_valid, the method save...

When all this process ends, then the render method is executed on my plugin. In this plugin, as you explained in the documentation, the first step is to call the super render method. Like that:

def render(self, context, instance, placeholder):
        context = super(NewsletterPlugin, self).render(context, instance, placeholder)
        return context

And this render method will execute the form again.

Well, in my case I can not execute the form twice, because the first time it works fine, but the second time it will fail because the fields already exists. But, I only can handle this form the second time, because I need to modify the context data, but in this second time always fails.

In addition, I detected that when works fine (the first execution time), it's mandatory to declare the save method on the form. If you don't have this method save(), it will raise an exception about this method save is not found.
In the template I declare the form like that: <form class="newsletter__form" action="{% cmsplugin_form_action %}" method="post"> My plugin is declared like that:

class NewsletterPlugin(FormPluginBase):
    name = _("Newsletter Signup")
    render_template = "cms/plugins/newsletter.html"
    cache = False
    form_class = NewsletterSubscriptionForm

    def render(self, context, instance, placeholder):
        context = super(NewsletterPlugin, self).render(context, instance, placeholder)
        return context

plugin_pool.register_plugin(NewsletterPlugin)

And the form is declared like that:

class NewsletterSubscriptionForm(FormPluginFormMixin, forms.Form):

    email = forms.EmailField(
        widget=forms.TextInput(attrs={
            'class': u'textinput',
            'placeholder': u'Enter your email address',
            'type': u'email'  # HTML 5 (this is in the original design)
        })
    )

    def clean_email(self):
        """
        Subscribe the user if the email is valid.
        """
        email = self.cleaned_data['email']
    def save(self):
        pass

Note that I need to write the save method.

Do you have any idea if I am doing something wrong, or is normal than the form methods are executed twice?

Kind Regards, Edgar

mkoistinen commented 8 years ago

Couple things. You really shouldn't need to add a save() method to the form. And, if you do it should probably look like this:

class MyFormClass(forms.Form):
    def save(self, **kwargs):
        super(MyFormClass, self).save(**kwargs)

But to be clear, should shouldn't need to even define save() here unless you want to.

I suspect the reason you're seeing double-saves is because the FormPluginBase defines the default form_valid(...) method to include a save(). If you don't want this (possibly second save here), define:

class NewsletterSubscriptionForm(FormPluginFormMixin, forms.Form):
    ...
    def form_valid(self, request, instance, form):
        pass

I wonder if form saving happens a little differently in Django 1.6? I'm not sure I've tested under that version. Should we remove the save() call from the default implementation of form_valid()?

Let me know if the above helps.

edgarzamora commented 8 years ago

Hello, Thanks for your response! And sorry for the delay, but I have not been able to answer before. My problem was that the clean method, concretely the clean_email method was called twice. Finally, as you suggested me, I've overwrote the method form_valid(), and I've wrote the action code into this method instead of to do the action into the clean method. So my problem has been solved.

On the other hand, I saw that to pass the data between the view and the plugin you are using the request session object. But, in my case I have a form in all the views located on the footer, in the case that the form is invalid, I show the error of the form. But if then I will refresh the page or I visit another page of the same webpage, the error doesn't disappear, because the session has not been deleted, and the form always will be returned with the same data. To solve this problem, I add the following code in the render of the plugin:

from cmsplugin_form_handler.cms_plugins import FormPluginBase, get_session_key
class NewsletterPlugin(FormPluginBase):
    name = _("Newsletter Signup")
    render_template = "cms/plugins/newsletter.html"
    cache = False
    form_class = NewsletterSubscriptionForm

    def render(self, context, instance, placeholder):
        context = super(NewsletterPlugin, self).render(context, instance, placeholder)
        request = context.get('request')
        if getattr(request, 'session'):
            session_key = get_session_key(instance.pk)
            if request.session.get(session_key) is not None:
                del request.session[session_key]
        return context

So, the form with the error message only will send once, then the data has been removed, and for the next calls of the GET types, the page sends an empty form, until the next POST call. I think that this process can be done in the parent render, to solve the problem.

Many thanks!

Kind Regards, Edgar

mkoistinen commented 8 years ago

Interesting use-case you have there and I'm glad you found a solution to the issue you reported above. I'm not sure it would be a good idea to always delete the session in the base render method, but I'm glad that this solution is extensible enough that you were able to do what you needed easily for your project. =)