Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom validator will prevent records from being reimported #21

Open
frapell opened this issue Jan 16, 2019 · 2 comments
Open

Custom validator will prevent records from being reimported #21

frapell opened this issue Jan 16, 2019 · 2 comments

Comments

@frapell
Copy link
Member

frapell commented Jan 16, 2019

Steps to reproduce:

  1. Create a control panel following instructions from https://pypi.org/project/plone.app.registry/#creating-a-custom-control-panel
  2. Add a validator for a field as explained in https://docs.plone.org/develop/addons/schema-driven-forms/customising-form-behaviour/validation.html#field-widget-validators
  3. Add initial values for that same field in registry.xml
  4. Install the product

You will see a traceback like this:

Traceback (innermost last):
  Module ZPublisher.Publish, line 138, in publish
  Module ZPublisher.mapply, line 77, in mapply
  Module ZPublisher.Publish, line 48, in call_object
  Module Products.CMFPlone.controlpanel.browser.quickinstaller, line 213, in __call__
  Module <string>, line 3, in installProducts
  Module AccessControl.requestmethod, line 70, in _curried
  Module Products.CMFQuickInstallerTool.QuickInstallerTool, line 686, in installProducts
  Module Products.CMFQuickInstallerTool.QuickInstallerTool, line 602, in installProduct
  Module Products.GenericSetup.tool, line 388, in runAllImportStepsFromProfile
  Module Products.GenericSetup.tool, line 1433, in _runImportStepsFromContext
  Module Products.GenericSetup.tool, line 1245, in _doRunImportStep
  Module plone.app.registry.exportimport.handler, line 70, in importRegistry
  Module plone.app.registry.exportimport.handler, line 116, in importDocument
  Module plone.app.registry.exportimport.handler, line 324, in importRecord
  Module plone.registry.record, line 34, in __init__
ValueError: Field is not persistent

If the product was installed and the validator is added afterwards, when you try to reimport the registry.xml the traceback looks like this:

Traceback (innermost last):
  Module ZPublisher.Publish, line 138, in publish
  Module ZPublisher.mapply, line 77, in mapply
  Module ZPublisher.Publish, line 48, in call_object
  Module Products.GenericSetup.tool, line 559, in manage_importSelectedSteps
  Module Products.GenericSetup.tool, line 358, in runImportStepFromProfile
  Module Products.GenericSetup.tool, line 1245, in _doRunImportStep
  Module plone.app.registry.exportimport.handler, line 70, in importRegistry
  Module plone.app.registry.exportimport.handler, line 116, in importDocument
  Module plone.app.registry.exportimport.handler, line 282, in importRecord
  Module plone.registry.record, line 60, in _set_field
  Module plone.registry.registry, line 283, in _setField
ValueError: The record's field must be an IPersistentField.

This is what happens in collective/collective.z3cform.datagridfield#14

As I explain in my comment (collective/collective.z3cform.datagridfield#14 (comment)), I found that when the persistent field is created, the __provides__ attribute would be overridden. This only happens when the field has a custom validator defined.

In order to fix it, I have added the __provides__ to the list of ignored attributes by adding to my __init.py__:

from plone.registry.field import DisallowedProperty
DisallowedProperty('__provides__')

I found that the vocabulary attribute is already ignored in

DisallowedProperty.uses.append('vocabulary')

should I add the __provides__ here as well as a solution?

@frapell
Copy link
Member Author

frapell commented Jan 16, 2019

@mauritsvanrees What do you think?

@mauritsvanrees
Copy link
Member

The way you put it, such a change would seem to make sense.

But wouldn't this raise a ValueError here? Ah, probably not, because the property would no longer be set, but be filtered out. Seems okay then.

I wonder if we should disallow anything starting with an underscore. That would probably mean changing

context_dict = dict(
[(k, v) for k, v in context.__dict__.items() if k not in ignored]
)

into something like this:

  context_dict = dict(
        [(k, v) for k, v in context.__dict__.items() if k not in ignored
        and not k.startswith('_')]
    )

I don't know, I don't remember having touched any code in plone.registry, or having run into problems with it.

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

No branches or pull requests

2 participants