NamedBlobFile in event description always modified

flipmcf · November 13, 2019, 10:45pm

Can I get a second set of eyes on this?

I have a field:

    video_file = NamedBlobFile(
        title=_('video file'),
    )

I've found that when I register an event with zope.lifecycleevent.interfaces.IObjectModifiedEvent and check what fields have been modified,

in my event handler I have:

for d in event.descriptions:
    'video_file' in d.attributes:
        file_changed = True

(source: Modified Event Handler for Dexterity Form Field? )

this namedblobfile always pops up on the list of modified fields, even though "Keep existing file" is selected with the widget.

I'm thinking this is a bug, and looks like it's a bug in the widget not setting the value to z3c.form.interfaces.NOT_CHANGED

Analysis:

z3c.form.form.applyChanges computes what fields have changed. It specifically looks for this:

github.com

zopefoundation/z3c.form/blob/e882d357190f6a5bc48dcefe7adc9cfacc9a0ad8/src/z3c/form/form.py#L43






def applyChanges(form, content, data):
    changes = {}
    for name, field in form.fields.items():
        # If the field is not in the data, then go on to the next one
        try:
            newValue = data[name]
        except KeyError:
            continue
        # If the value is NOT_CHANGED, ignore it, since the widget/converter
        # sent a strong message not to do so.
        if newValue is interfaces.NOT_CHANGED:
            continue
        if util.changedField(field.field, newValue, context=content):
            # Only update the data, if it is different
            dm = zope.component.getMultiAdapter(
                (content, field.field), interfaces.IDataManager)
            dm.set(newValue)
            # Record the change using information required later
            changes.setdefault(dm.field.interface, []).append(name)

A namedblobfile will fail this check and move on to util.changedField whose job it is to really check if a value changed. This is hot wired to return true b/c namedblobfile implements zope.schema.interfaces.IObject

github.com

zopefoundation/z3c.form/blob/e882d357190f6a5bc48dcefe7adc9cfacc9a0ad8/src/z3c/form/util.py#L191




def changedField(field, value, context=None):
    """Figure if a field's value changed


    Comparing the value of the context attribute and the given value"""
    if context is None:
        context = field.context
    if context is None:
        # IObjectWidget madness
        return True
    if zope.schema.interfaces.IObject.providedBy(field):
        return True


    # Get the datamanager and get the original value
    dm = zope.component.getMultiAdapter(
        (context, field), interfaces.IDataManager)
    # now figure value chaged status
    # Or we can not get the original value, in which case we can not check
    # Or it is an Object, in case we'll never know
    if (not dm.canAccess() or dm.query() != value):
        return True

I have some other options here to avoid triggering 'the video file data changed' but wanted to check here first.

Anyone know of a better file field widget?

Should I file a bug against plone.formwidget.namedfile?

Am I just doing something wrong?

fredvd · November 14, 2019, 10:09am

Your analysis seems sound to me, but:

I'm thinking this is a bug, and looks like it's a bug in the widget not setting the value to z3c.form.interfaces.NOT_CHANGED

This seems to be the place where the error is made and where something should be fixed, the other code you mention seems to be correct.

flipmcf · November 20, 2019, 5:58pm

I've self-assigned the ticket, but currently quite lost on how to fix it. I might have something in a day or so.

flipmcf · November 25, 2019, 8:32pm

Doctests even got updated.