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

Problem to install cmsplugin-contact with django-cms version 2.4.2 #53

Closed rgcarrasqueira closed 6 years ago

rgcarrasqueira commented 10 years ago

Hi,

I'm trying to update my website running django-cms 2.4.2 with the latest version of cmsplugin-contact and occured the error bellow

Traceback (most recent call last):

  File "/usr/local/lib/python2.7/dist-packages/django/core/handlers/base.py", line 111, in get_response
    response = callback(request, *callback_args, **callback_kwargs)

  File "/usr/local/lib/python2.7/dist-packages/cms/views.py", line 148, in details
    return render_to_response(template_name, context_instance=context)

  File "/usr/local/lib/python2.7/dist-packages/django/shortcuts/__init__.py", line 20, in render_to_response
    return HttpResponse(loader.render_to_string(*args, **kwargs), **httpresponse_kwargs)

  File "/usr/local/lib/python2.7/dist-packages/django/template/loader.py", line 176, in render_to_string
    return t.render(context_instance)

  File "/usr/local/lib/python2.7/dist-packages/django/template/base.py", line 140, in render
    return self._render(context)

  File "/usr/local/lib/python2.7/dist-packages/django/template/base.py", line 134, in _render
    return self.nodelist.render(context)

  File "/usr/local/lib/python2.7/dist-packages/django/template/base.py", line 823, in render
    bit = self.render_node(node, context)

  File "/usr/local/lib/python2.7/dist-packages/django/template/base.py", line 837, in render_node
    return node.render(context)

  File "/usr/local/lib/python2.7/dist-packages/django/template/loader_tags.py", line 123, in render
    return compiled_parent._render(context)

  File "/usr/local/lib/python2.7/dist-packages/django/template/base.py", line 134, in _render
    return self.nodelist.render(context)

  File "/usr/local/lib/python2.7/dist-packages/django/template/base.py", line 823, in render
    bit = self.render_node(node, context)

  File "/usr/local/lib/python2.7/dist-packages/django/template/base.py", line 837, in render_node
    return node.render(context)

  File "/usr/local/lib/python2.7/dist-packages/classytags/core.py", line 106, in render
    return self.render_tag(context, **kwargs)

  File "/usr/local/lib/python2.7/dist-packages/sekizai/templatetags/sekizai_tags.py", line 76, in render_tag
    rendered_contents = nodelist.render(context)

  File "/usr/local/lib/python2.7/dist-packages/django/template/base.py", line 823, in render
    bit = self.render_node(node, context)

  File "/usr/local/lib/python2.7/dist-packages/django/template/base.py", line 837, in render_node
    return node.render(context)

  File "/usr/local/lib/python2.7/dist-packages/django/template/loader_tags.py", line 62, in render
    result = block.nodelist.render(context)

  File "/usr/local/lib/python2.7/dist-packages/django/template/base.py", line 823, in render
    bit = self.render_node(node, context)

  File "/usr/local/lib/python2.7/dist-packages/django/template/base.py", line 837, in render_node
    return node.render(context)

  File "/usr/local/lib/python2.7/dist-packages/classytags/core.py", line 106, in render
    return self.render_tag(context, **kwargs)

  File "/usr/local/lib/python2.7/dist-packages/cms/templatetags/cms_tags.py", line 264, in render_tag
    content = get_placeholder_content(context, request, page, name, inherit)

  File "/usr/local/lib/python2.7/dist-packages/cms/templatetags/cms_tags.py", line 182, in get_placeholder_content
    content = render_placeholder(placeholder, context, name)

  File "/usr/local/lib/python2.7/dist-packages/cms/plugin_rendering.py", line 127, in render_placeholder
    content.extend(render_plugins(plugins, context, placeholder, processors))

  File "/usr/local/lib/python2.7/dist-packages/cms/plugin_rendering.py", line 81, in render_plugins
    out.append(plugin.render_plugin(context, placeholder, processors=processors))

  File "/usr/local/lib/python2.7/dist-packages/cms/models/pluginmodel.py", line 173, in render_plugin
    context = plugin.render(context, instance, placeholder_slot)

  File "/home/blabla/production/web_app/portal/src/cmsplugin-contact/cmsplugin_contact/cms_plugins.py", line 150, in render
    form = self.create_form(instance, request)

  File "/home/blabla/production/web_app/portal/src/cmsplugin-contact/cmsplugin_contact/cms_plugins.py", line 82, in create_form
    ContactFormBase = class_for_path(instance.form_layout)

  File "/home/blabla/production/web_app/portal/src/cmsplugin-contact/cmsplugin_contact/utils.py", line 6, in class_for_path
    module_name, class_name = class_path.rsplit(".", 1)

ValueError: need more than 1 value to unpack

I'm running with django 1.4.7, python 2.7.3, ubuntu 12.04 and mysql 5.1

Any Ideas?

driesdesmet commented 10 years ago

I'm running into the same problem. Did you find a workaround?

maccesch commented 10 years ago

I guess with latest version you mean from repository. Could you please print the class_path in cmsplugin_contact/utils.py?

driesdesmet commented 10 years ago

I'm using the latest commit in master. So I have this:

# coding=utf-8
import importlib

def class_for_path(class_path):
    module_name, class_name = class_path.rsplit(".", 1)
    m = importlib.import_module(module_name)
    c = getattr(m, class_name)
    return c

I should elaborate more on the circumstances I'm using your plugin for:

Lots of moving parts, I need some time to pin it more. Just have a deadline for other stuff on my hands. Let me know if I can help in any way.

maccesch commented 10 years ago

Oh, I'm sorry. I meant: Please put a print class_path right after def class_for_path(...)

driesdesmet commented 10 years ago

class_path is empty

driesdesmet commented 10 years ago

Hm, my problem is solved. It was actually a typo in settings.py: CMSPLUGIN_CONTACT_FORMS lost the s. So my problem is probably unrelated to the previous bug reporter.

Come to think of it, it wasn't exactly clear from the documentation that you don't need to subclass ContactPlugin if all you want is add some fields to a form. You DO need to subclass if you want to set it's own templates however, which imho should work differently. If you'd store the render_template, subject_template and email_template with the plugin model in the database it would simplify the code needed for a simple form layout adjustment. Is that something you would take a pull request for?

Thanks.

maccesch commented 10 years ago

Glad to hear that.

To overwrite template you can copy the cmsplugin_plugin folder inside the templates folder into your own templates directory and just customize the files inside. That is common practice.

Do you think it would be beneficial to have the possibility to store this setting in the database?

mitar commented 10 years ago

Hm, I am not sure if I would like that users can configure templates from admin interface. This could open security issues when somebody enters some other system path and reads data from there. I have also not seen any plugin doing this. It is a normal practice to have to subclass if you want to rename a template and simply add your own template file if you want to override the template.

driesdesmet commented 10 years ago

@maccesch: yes I know you can easily copy over the template files and work from there, but the template path is hard-coded. Say I want 2 different form-layouts each with their own template, then I need to subclass the plugin to be able to set a custom template path. If the template path would be saved to the plugin, I can choose the template in the plugin admin, no subclassing needed.

@mitar ok, I totally agree to follow common practice rules. What about linking the form_layout with the template path? So the default is cmsplugin_contact/contact.html which is totally cool, but maybe a slugified version of the name form name defined in CMSPLUGIN_CONTACT_FORMS setting could be used as a base for the template name. Like so:

CMSPLUGIN_CONTACT_FORMS = (
    ('cmsplugin_contact.forms.ContactForm', 'contact'), # loads cmsplugin/contact.html and related templates
    ('amitabha.contacts.forms.JoinUsForm', 'join us'), # loads cmsplugin/join-us.html and related templates
)

I'm not a security expert, so I'd totally listen to what your concerns are with this.

mitar commented 10 years ago

@driesdesmet one common way is to include some instance data into template context. And then you simply define your template as:

{% if name == "contact" %}
    {% include "cmsplugin/contact.html" %}
{% else %}
    {% include "cmsplugin/join-us.html" %}
{% endif %}

No need for any code change.

maccesch commented 10 years ago

@mitar that is a good idea. Maybe you could add this to our docs?

mitar commented 10 years ago

I am not sure if form name is currently really exposed in the template?

maccesch commented 10 years ago

Me neither :smile: @driesdesmet Would you mind checking that and if it isn't exposed yet doing that? Thanks