maccesch / cmsplugin-contact

Extendable contact plugin for django-cms with spam protection and i18n
BSD 2-Clause "Simplified" License
71 stars 62 forks source link

This plugin doesn't auto-redirect on submit? #29

Closed samzhao closed 10 years ago

samzhao commented 12 years ago

I realize that whenever I refresh the page localhost/contact/ the browser prompts me a form resubmission warning. Doesn't the plugin redirect after rendering the form. The same goes to when the form is submitted and the thank you message is displayed.

Is it something I need to configure or is it a bug?

Thanks

maccesch commented 12 years ago

No, it doesn't redirect. You're right, it's a bug.

samzhao commented 12 years ago

So when are you planning on fixing it? Also, should I get the version on github or just use pip install -U? Thanks

mitar commented 12 years ago

You can try first to fix yourself and give us a pull request? This would be a major help!

samzhao commented 12 years ago

I honestly have no idea how to fix it. I'm a super noob, thought I went through some tutorials on how to make a simple form which redirects to the page itself in case people fresh the page and have the data sent again. I've looked at the source of this plugin, but I can't figure out where am I supposed to be looking at.

samzhao commented 12 years ago

maybe in this line?

 return super(ContactPlugin, self).render_change_form(request, context, add, change, form_url, obj)

I don't know.

I can't even refresh the page when I visit the contact page that contains this contact form

maccesch commented 12 years ago

Yeah, never mind, that is truly not a task for a beginner. I will take a look at it. But I can't promise when. Currently I'm away having holidays.

samzhao commented 12 years ago

@maccesch, no worries. Enjoy your vacation! It doesn't hurt to take a guess. You need to import Page from django-cms in a view (probably in cms_plugins.py), and modify something in the class which creates the form and add in HttpRedirect( Page.get_absolute_url()) Hopefully my idea is right :)

mitar commented 11 years ago

I looked into this a bit. I found that there is no simple way to redirect inside of render method. So I am proposing that we require this middleware and then throw a redirect exception.

In fact, the way we are doing this, is not the best. Because currently we are intercepting POST request. But what if there are multiple contact forms on one page? Which one then gets processed? :-) The proposed way is to submit to some other view. But the issue is then how to display an error to the user if form is invalid. So it seems this is not really something Django CMS would support easily.

maccesch commented 11 years ago

You are right. Another possibility would be to implement submission through AJAX. What do you think?

mitar commented 11 years ago

Hm, rendering errors would still be a problem. I think the best way is to use django-excepted and throw in render method redirect exception on POST. Furthermore, we should just add a hidden ID for each plugin instance to the form, which would then be checked in POST handler to see if the given instance render method should process it or it is for some other contact form on the same page.

maccesch commented 11 years ago

Actually Errors wouldn't be a problem. We could send a simple Ajax post to a seperate view which validates the form and sends html as response which is placed in the same space the original form was placed in. that way we could avoid an additional dependency and the qirky exception redirect.

mitar commented 11 years ago

I am skeptical about Ajax in the contact form. You want it to work even without JavaScript, because contact form can sometimes be used to inform site about errors they have on the site. So it has to be stable. Not too many moving parts which can fail as well. I am also not sure, how recaptcha integrates with Ajax?

I tried this approach with middleware in some other project and it works nicely. I think this is the best way to handle it. So it is not a problem how to display an error, the biggest problem is that after POST we want a redirect. Not just that we output the results.

m000 commented 10 years ago

Just to add my 2c:

Django-CMS already uses sekizai to add content from plugins etc. to the proper location in the page.

Additionally, it requires having a sekizai block named "css" in your template. That block is located inside the <head> </head> HTML block of the page.

So I think the simplest way for a redirect would be to change the final lines of cmsplugin_contact/contact.html to something like this:

{% else %}
  {{ contact.thanks|safe }}

  {% load sekizai_tags %}
  {% addtoblock "css" %}
  <meta http-equiv="refresh" content="5;URL= {% url "pages-root" %}">
  {% endaddtoblock "css" %}
{% endif %}

The advantage of this approach is that we don't bloat our installations with yet another middleware.

What do you think?

maccesch commented 10 years ago

Thanks m000,

that's certainly something to consider.

mitar commented 10 years ago

The point of the POST and redirect response is that it is a known HTTP design pattern. Using meta HTML tag is not.

m000 commented 10 years ago

Pattern != rule. I could surely live with a meta tag since it improves the current situation without degrading the overall quality of the codebase (we only change a template).

Regarding a "proper" fix to the problem, I'm afraid it is not directly possible. Django cms intends plugins to be used mostly for displaying information. This is implied in the app integration documentation and that's why render offers no easy way to a redirect.

The common practice when it comes to processing user submitted data (which would also allow you to properly handle redirects) is to add a view and point your form action to the url of that view. This is the approach also taken by other cms plugins that process user input (e.g. poll plugin).

mitar commented 10 years ago

I could surely live with a meta tag

Maybe you could, but I believe it is not the right way. Django uses clear pattern here (Post/Redirect/Get) and we should follow it ourselves as well. Frameworks work because they are intuitive and they are intuitive because they use patterns. Furthermore, we should not put plugin logic into a template. There should be a clean separation between logic and template.

So I see two paths forward with this:

Here is a relevant issue: https://github.com/divio/django-cms/issues/79

And a proposed workaround: http://djangosnippets.org/snippets/2541/

mitar commented 10 years ago

I opened a ticket on django CMS.

m000 commented 10 years ago

Thanks @mitar for opening the ticket. Well written :-). Let's wait the response to the ticket and see what happens.

mitar commented 10 years ago

Hm. What about using iframe? But then it is a problem that you have to negotiate size of the iframe vs. internal and external content. Or can iframe has a "floating" size based on the content around?

luisza commented 10 years ago

Hi, I fixed using middleware inspirated in https://djangosnippets.org/snippets/2541/ because django cms render as a template plugin and don't call the process_exception, because this is for views.

# in a plugin 

     def render(self, context, instance, placeholder):
            if instance.redirect_url:
                   setattr(request, 'django_cms_contact_redirect_to', HttpResponseRedirect(instance.redirect_url)) 

I used request attr because only request is shared in both method

# in middleware.py
class ForceResponseMiddleware:

    def process_response(self, request, response):
        if getattr(request, 'django_cms_contact_redirect_to', None) :
            return request.django_cms_contact_redirect_to
    return response

I made a pull request with this.