Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

Copy/pasting a Form/Fieldset/Field plugins through CMS copy & paste interface causes a KeyError #167

Closed
xc3seziu opened this issue Aug 13, 2018 · 13 comments
Assignees

Comments

@xc3seziu
Copy link

xc3seziu commented Aug 13, 2018

Technical analysis report from @victor-yunenko:

The problem lies in the code that dynamically builds a Form class or collects Form fields - it accumulates the fields in a dict using Field.name as a key (the same field name that user can set to anything), since Field.name doesn't enforce uniqueness in any manner a lot of places either ignore the duplicates or raise errors.

Original briefing by theitsmith

I was creating a contact form in Structure mode. To create a Bootstrap form (perhaps there's a better way?), I wrapped the form fields in a div to apply the appropriate Bootstrap classes--this worked well for the first form field.

To add the second, I copied the div structure of the first field and pasted into the appropriate place in the form--the plan was then to edit that newly pasted field and change a couple of attributes (such as 'id', 'name', 'placeholder', etc. However, immediately after pasting the form field, that page now errors with the following (while viewing in with DEBUG=True):

Request Method: 	GET
Request URL: 	http://domain.demo/en/contact/
Django Version: 	1.11.15
Exception Type: 	KeyError
Exception Value: 	

560

Exception Location: 	/home/domain/env/lib/python3.6/site-packages/aldryn_forms/models.py in get_form_field_name, line 286
Python Executable: 	/usr/local/bin/uwsgi
Python Version: 	3.6.4
Python Path: 	

['.',
 '/home/domain/app',
 '/home/domain/env/lib/python36.zip',
 '/home/domain/env/lib/python3.6',
 '/home/domain/env/lib/python3.6/lib-dynload',
 '/usr/local/lib/python3.6',
 '/home/domain/env/lib/python3.6/site-packages',
 '/home/domain/env/lib/python3.6/site-packages/odf',
 '/home/domain/env/lib/python3.6/site-packages/odf',
 '/home/domain/env/lib/python3.6/site-packages/odf',
 '/home/domain/env/lib/python3.6/site-packages/odf',
 '/home/domain/env/lib/python3.6/site-packages/odf',
 '/home/domain/env/lib/python3.6/site-packages/odf',
 '/home/domain/env/lib/python3.6/site-packages/odf']

Now the page won't load at all in Edit mode, it just generates that error.

Two questions:

  1. Is it possible to fix this by removing the 'pasted' content from the database (I'm not sure how to do this, but if that's causing the error, it may resolve it enough that I can continue working on the site).
  2. Is there additional information I can provide to aide in troubleshooting this? This seems like a forms issues but since the copy/paste action seems to have caused this, I'm not sure if it might be a CMS issue instead (the copy/paste functionality).
@wfehr
Copy link

wfehr commented Sep 19, 2018

I had the same issue just one day ago, now I know why it happened!
What I did was deleting the plugin with the KeyError-ID, in your case 560. Then I was able to view the page in edit-mode again and could do a revert-to-live.

@xc3seziu
Copy link
Author

Thank you for the reply. Sorry for the delay in responding. 🙂

I'm a bit new to Django CMS. How would I go about deleting a plugin? Are you referring going into the database and removing the plugin there? Or is there a management layer for doing this (such as manage.py or the web UI)?

@wfehr
Copy link

wfehr commented Oct 10, 2018

The easiest approach for this would be to use the python interactive interpreter of django. There you can import the model and fetch an instance from your DB. When you got it and checked if it is the one you want to delete you can use .delete() on the instance.
Example:

>>> from some_app.models import SomeModel
>>> instance = SomeModel.objects.get(id=id)
>>> instance.delete()

@PeterW-LWL
Copy link
Contributor

I have stepped into this error as well.

Would a stacktrace help to fix the error?

@xc3seziu
Copy link
Author

I haven't had time to investigate further, but I think the problem is that the copy/paste function (insofar as I currently understand it) is duplicating the field name--and that duplication is causing the issue; two fields in the same 'node' in the tree with the same name.

Hope that helps.

When time allows, I'll spend more time looking into the root cause of that issue--what I mentioned is based on my observations of the problem in my environment, so that may or may not be on the right track.

@viktor-yunenko viktor-yunenko changed the title Copy/pasting form data causes KeyError Copy/pasting a Form/Fieldset plugins causes a KeyError Dec 26, 2019
@viktor-yunenko
Copy link

viktor-yunenko commented Dec 26, 2019

Turns out I raised a duplicate of this ticket - #197, I'm posting the most important data from it below:

Copying the whole page using Page -> Create Page -> Duplicate this Page action works normally though.

My current assumption is that adding self._form_field_key_cache = {} to the copy_relations method might help. Although I wasn't able to reproduce the issue locally in order to confirm it. Maybe because the cache gets disabled outside of production mode?

Last part of the stack trace:

File "/usr/local/lib/python3.6/site-packages/cms/templatetags/cms_tags.py" in render_plugin
  199.         editable=renderer._placeholders_are_editable,
File "/usr/local/lib/python3.6/site-packages/cms/plugin_rendering.py" in render_plugin
  429.         context = plugin.render(context, instance, placeholder.slot)
File "/usr/local/lib/python3.6/site-packages/aldryn_forms/cms_plugins.py" in render
  371.             field_name = form_plugin.get_form_field_name(field=instance)
File "/usr/local/lib/python3.6/site-packages/aldryn_forms/models.py" in get_form_field_name
  293.         return self._form_field_key_cache[field.pk]
Exception Type: KeyError at /support/support-request/pilatus/pilatus3-processing-unit/
Exception Value: 6746

@viktor-yunenko viktor-yunenko changed the title Copy/pasting a Form/Fieldset plugins causes a KeyError Copy/pasting a Form/Fieldset plugins through CMS copy & paste interface causes a KeyError Dec 26, 2019
@viktor-yunenko
Copy link

viktor-yunenko commented Dec 26, 2019

I haven't had time to investigate further, but I think the problem is that the copy/paste function (insofar as I currently understand it) is duplicating the field name--and that duplication is causing the issue; two fields in the same 'node' in the tree with the same name.

I thought about it as well, but not sure anymore since the whole page can be copied normally.

The stacktrace indicates that the problem occurs because aldryn-forms notices that the form cache self._form_field_key_cache contains field.pk and decides that it must contain the field that it needs, while in reality the required field is not accessible.

Here's the whole method, it fails on the last line:

    def get_form_field_name(self, field):
        if self._form_field_key_cache is None:
            self._form_field_key_cache = {}

        if field.pk not in self._form_field_key_cache:
            fields_by_key = self.get_form_fields_by_name()

            for name, _field in fields_by_key.items():
                self._form_field_key_cache[_field.plugin_instance.pk] = name
        return self._form_field_key_cache[field.pk]

@viktor-yunenko
Copy link

Here's a breakdown of what happens in that piece of code

    def get_form_field_name(self, field: Field) -> str:
        if self._form_field_key_cache is None:
            self._form_field_key_cache = {}

        # true
        if field.pk not in self._form_field_key_cache:
            fields_by_key = self.get_form_fields_by_name()
            # field.pk = 10
            # fields_by_key doesn't contain the required field, hence it's never added to self._form_field_key_cache
            for name, _field in fields_by_key.items():
                self._form_field_key_cache[_field.plugin_instance.pk] = name
        return self._form_field_key_cache[field.pk]

@viktor-yunenko
Copy link

I noticed that this issue affects copying of any individual field as well (at least for email and text fields).

@viktor-yunenko viktor-yunenko self-assigned this May 27, 2020
@viktor-yunenko viktor-yunenko changed the title Copy/pasting a Form/Fieldset plugins through CMS copy & paste interface causes a KeyError Copy/pasting a Form/Fieldset/Field plugins through CMS copy & paste interface causes a KeyError May 27, 2020
@viktor-yunenko
Copy link

The problem lies in the code that dynamically builds a Form class or collects Form fields - it accumulates the fields in a dict using Field.name as a key, since Field.name doesn't enforce uniqueness in any manner a lot of places either ignore the duplicates or raise errors.

@viktor-yunenko
Copy link

viktor-yunenko commented May 27, 2020

I pushed a fix to the master branch and will be testing on a project that's going to go live in ~2 weeks.

@viktor-yunenko
Copy link

viktor-yunenko commented May 27, 2020

A related bug remains though - fields with identical names are being merged into one field, eg

Form submission:

  • name = value1
  • name = value2
  • name = value3

Is going to be saved as:

  • name = value3
  • name = value3
  • name = value3

@viktor-yunenko
Copy link

viktor-yunenko commented Jun 2, 2020

I created 5.0.5 release which can be installed by adding to requirements.txt as:

https://github.com/divio/aldryn-forms/archive/5.0.5.zip#egg=aldryn-forms

As I don't have permissions to push it to pypi we have to use github for now.

The name uniqueness issue we can track in #207

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants