Import of nonexisting skin gives a formulator SelectionValidator error.

Bug #128518 reported by todd
2
Affects Status Importance Assigned to Milestone
Silva
Fix Released
Medium
Eric Casteleijn

Bug Description

If you're using silva layout, you can set the skin in the metadata user interface for publications. This is the metadata set silva-layout, metadata element 'skin'. The skin selection shows a list of skins that are installed that you can use. If you select an installed skin, then export that publication as XML,
and then import this into another Silva which doesn't have that skin installed,
you see the following exception:

  Module Products.Silva.silvaxml.xmlimport, line 222, in endElementNS
  Module Products.Silva.silvaxml.xmlimport, line 85, in storeMetadata
  Module Products.Formulator.Validator, line 54, in deserializeValue
  Module Products.Formulator.Validator, line 468, in validate
  Module Products.Formulator.Validator, line 41, in raise_error
ValidationError: unknown_selection

This is because the skin with the selected name is not recognized by the other Silva and the formulator SelectionValidator trips over it.

We should modify the importing system to give a better error message so it is clearer to the user what's going on. Alternatively we can set the skin to a default skin that *is* installed and send a warning message to the log.

Revision history for this message
Kit Blake (kitblake) wrote :

I think the second option – set the skin to a default skin that *is* installed and send a warning message to the log – is much better. I can't imagine that someone would abort an import if a skin wasn't installed, so we shouldn't do that either.

Changed in silva:
assignee: nobody → thisfred
importance: Undecided → Medium
Changed in silva:
assignee: thisfred → aaltepet
Revision history for this message
Andy Altepeter (aaltepet) wrote :

* 128518 - Import of nonexisting skin gives a formulator SelectionValidator error.
         - created a new formulator field that will fallback to a default skin, or
           acquire default setting if none exists.

This needs more testing. You need to refresh SilvaLayout from service_extensions for the new field to be used.

Changed in silva:
status: New → In Progress
Revision history for this message
Andy Altepeter (aaltepet) wrote :

thisfred was going to check on:
1) adjusting the metadata system to fallback to the default value if the metadata field value for an object isn't valid.
2) Does this mean that the silvalayout metadata set will need to be updated to specify a default value (?)
3) or if there is no default value use the first item?
4) it can't just be set to None, since there was value initially?
And, a warning should be logged about this...

Changed in silva:
assignee: aaltepet → thisfred
Revision history for this message
Eric Casteleijn (thisfred) wrote :

I think the metadata import was already meant to just log and not error in this case:

                    errors = binding._setData(
                            namespace_key=set.metadata_uri,
                            data={
                                element_name: field.validator.deserializeValue(
                                    field, elements[element_name])},
                            reindex=1)
                    if errors:
                        zLOG.LOG(
                            'Silva', zLOG.WARNING,
                            "Value %s is not allowed for element %s in set %s." % (elements[element_name], element_name, set_id))

but maybe formulator changed, because now an error is thrown. I've changed this (for testing) to:

                    try:
                        errors = binding._setData(
                            namespace_key=set.metadata_uri,
                            data={
                                element_name: field.validator.deserializeValue(
                                    field, elements[element_name])},
                            reindex=1)
                    except ValidationError:
                        zLOG.LOG(
                            'Silva', zLOG.WARNING,
                            "Value %s is not allowed for element %s in set %s." % (elements[element_name], element_name, set_id))
                    if errors:
                        zLOG.LOG(
                            'Silva', zLOG.WARNING,
                            "Value %s is not allowed for element %s in set %s." % (elements[element_name], element_name, set_id))

I'll check it in on the Silva 2.0 branch. Can you test if that fixes the error in your set-up? Then I will try to clean it up a little, and at least write a test for it.

Revision history for this message
Andy Altepeter (aaltepet) wrote :

Yes, this seems to fix the error in my setup. I see the following in the error log:
2008-01-24 20:42:39 WARNING Silva Value SilvaLayoutTemplate is not allowed for element skin in set silva-layout.

and when I go to the properties tab, I see the default value of 'acquire'.

Revision history for this message
Eric Casteleijn (thisfred) wrote :

Ok, cool, I will try to write a test for this and also port it to the trunk.

Changed in silva:
status: In Progress → Fix Committed
Revision history for this message
Andy Altepeter (aaltepet) wrote :

thisfred, could you port this back to the trunk too?

Changed in silva:
milestone: 2.0 → 2.0.6
status: Fix Committed → Fix Released
status: Fix Released → In Progress
Revision history for this message
Eric Casteleijn (thisfred) wrote :

checked into the trunk

Changed in silva:
status: In Progress → Fix Committed
Changed in silva:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.