zopefoundation / Products.GenericSetup

Mini-framework for expressing the configured state of a Zope Site as a set of filesystem artifacts
Other
3 stars 12 forks source link

GS can't eat its own dog food. #32

Open iham opened 8 years ago

iham commented 8 years ago

situation set up a plone 5.1a2 site. create a snapshot in portal_setup. change the day the week should start with in the control panel "Date and Time Settings". create a snapshot in portal_setup again and make a diff between those two.

i then safed this changed settings to my registry.xml

  <record name="plone.first_weekday" interface="Products.CMFPlone.interfaces.controlpanel.IDateAndTimeSchema" field="first_weekday">
    <field type="plone.registry.field.Choice">
      <description xmlns:ns0="http://xml.zope.org/namespaces/i18n" ns0:domain="plone" ns0:translate="help_first_weekday">First day in the week.</description>
      <title xmlns:ns0="http://xml.zope.org/namespaces/i18n" ns0:domain="plone" ns0:translate="label_first_weekday">First weekday</title>
      <vocabulary>plone.app.vocabularies.Weekdays</vocabulary>
    </field>
    <value>3</value>
  </record>

now i set up another plone site and also install my package with this settings stored inside and get an error:

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.browser.admin, line 272, in __call__
  Module Products.CMFPlone.factory, line 168, in addPloneSite
  Module Products.GenericSetup.tool, line 379, in runAllImportStepsFromProfile
   - __traceback_info__: profile-akbild.site:default
  Module Products.GenericSetup.tool, line 1414, in _runImportStepsFromContext
  Module Products.GenericSetup.tool, line 1226, in _doRunImportStep
   - __traceback_info__: plone.app.registry
  Module plone.app.registry.exportimport.handler, line 47, in importRegistry
  Module plone.app.registry.exportimport.handler, line 90, in importDocument
  Module plone.app.registry.exportimport.handler, line 246, in importRecord
   - __traceback_info__: record name: plone.first_weekday
  Module plone.supermodel.utils, line 144, in elementToValue
  Module zope.schema._field, line 322, in fromUnicode
  Module zope.schema._bootstrapfields, line 183, in validate
  Module zope.schema._field, line 338, in _validate
ConstraintNotSatisfied: (u'0', '')

0 -> Monday (Europe), but every other number from 0 (monday) to 6 (sunday) generates the same error.

why does that happen? the choice field (zope.schema) validates a given value (the one from the xml file -> u'0') against its vocabulary, using by_value, which is - in case of plone.app.vocabularies.datetimerelated.WeekdaysFactory - a dict of number (keys) and simpleterm (value) - pairs. but the value from the xml is a unicode string, not an int. -> so its not a key in the vocabulary. see zope.schema._field.Choice._validate() for that.

u'0' in {0:term_a, 1:term_b} == False

further down the rabbit hole... most SimpleTerms in plone.app.vocabularies are instantiated by a value (string) and a title (term); only the Month and Weekday Vocabularies are build different. the come with a value, token and title. value is an index, token a string representation of that index and the title (term). this is, at least, inconsistent.

solutions

  1. rewrite the vocabularies to have better key/value consistency and fix cmfplone Products.CMFPlone.setuphandlers.first_weekday_setup to use strings instead of integers.
  2. extend the choice field to also handle value_type; like IDict and ICollection does.
  3. make all GS setup files JSON instead of XML (no... just kidding... but would be awesome)

i will implement the first solution, as i am not able (too heavy) to extend the Choice field or reimplement the whole GS-setup infrastructure.

take a look at: https://github.com/plone/Products.CMFPlone/issues/1794 and https://github.com/plone/plone.app.vocabularies/issues/41

sadly they are cross depending ...

tseaver commented 8 years ago

@iham Thanks for the report. I'm not sure that there is anything this project can do to fix the issue: your suggested solutions reference Products.CMFPlone and plone.app.vocabularies. Maybe plone.supermodel would be the right place to tackle it?

iham commented 8 years ago

@tseaver i tried it there, but it seems the wrong way, as there is no way to decide if a string only containing numbers should be a string or number ...

as far as i can see, i stirred up a small hornets' nest: https://github.com/plone/plone.supermodel/pull/17#event-825410280

icemac commented 6 years ago

@iham plone/plone.supermodel#17 is merged. Did this fix your problem, so this issue here in can be closed?