NamedImage and NamedFile widgets do not retain content when modified

[zedr]

Description

I have the following content type type defined:

class IImageRow(interface.Interface):
    home_image = NamedImage(
        title=_(u"Image"),
        required=False,
    )
    url = schema.TextLine(title=_(u'URL'), 
                             required=False)

class IMyContent(form.Schema):
    """My content type
    """
    form.widget(gallery=DataGridFieldFactory)
    gallery= schema.List(
            title=_(u"A simple gallery of images"),
            value_type=DictRow(schema=IImageRow),
            required=True,
        )

    (...)

I can load and store a datagrid of images, but when I edit the content and save it, asking to "Keep the existing image", the reference to the image is not kept. The "url" is kept however.

Steps to reproduce

Actions

1. Define a datagridfield of NamedImages in code
2. Add the content in the site
3. Upload some images in the datagrid
4. Edit the content again
5. Save the content, keeping the existing images

   Outcome

   • The stored images are lost

     Expected result

   • The stored images should be kept

[zedr]

I can also reproduce this on Plone 4.1.4 and Dexterity 1.2.

[davisagli]

Unfortunately I think when the datagridfield is submitted it builds the value to be stored from scratch from the input in the request -- and the widgets making up each row do not have simple access to the old value of the row. So fixing this will probably require someone to dig pretty deep into z3c.form and collective.z3cform.datagridfield.

[neilferreira]

Anyone know if there is a way around this yet? 2 years later.

[miohtama]

This is most likely because NamedBlobImage does special things deep down with z3c.form. It's a place where nobody wants to go.

[puittenbroek]

Same problem with NamedFile, which makes sense since they use the same base

[2silver]

Still open Plone 4.3.4.1, dexterity 2.0.12

[b4oshany]

Wow... I guess I fell into this trap as well...

[b4oshany]

So this is the gymnastics I did:

For datagridfield model

import uuid
from plone.autoform import directives
from zope.schema.interfaces import IFromUnicode
from plone.autoform.interfaces import IFormFieldProvider
from zope import schema
from plone.supermodel import model
from plone.autoform import directives
from zope.interface import implementer
from zope.interface import provider
from plone.schema import Email

from plone.formwidget.namedfile.widget import NamedImageFieldWidget
from collective.z3cform.datagridfield import BlockDataGridFieldFactory
from collective.z3cform.datagridfield import DictRow
from my.package import _

@implementer(IFromUnicode)
class ISponsor(model.Schema):

    directives.mode(oid='hidden')
    oid = schema.TextLine(
        title=u"UUID",
        default=uuid.uuid4().hex
    )

    name = schema.TextLine(
        title=_(u"Name")
    )
    email = Email(
        title=_(u'label_email', default=u'Email'),
        description=_(u'help_email', default=u''),
        required=False
    )
    website = schema.URI(
        title=_(u'label_website', default=u'Website'),
        description=_(u'help_website', default=u''),
        required=False
    )
    picture = schema.ASCII(
        title=_(u"Please upload an image"),
        required=False,
    )
    directives.widget(
        'picture',
        NamedImageFieldWidget
    )

@provider(IFormFieldProvider)
class ISponsors(model.Schema):

    sponsors = schema.List(
        title=_(u'Event Sponsors'),
        value_type=DictRow(title=u"sponsors", schema=ISponsor),
        required=False
    )
    directives.widget(
        'sponsors',
        BlockDataGridFieldFactory
    )

    model.fieldset(
        'event_sponsors',
        label=_(u"Sponsors"),
        fields=['sponsors']
    )

@implementer(ISponsors)
class Sponsors(object):

    _sponsors = None

    def __init__(self, context):
        self.context = context

    @property
    def sponsors(self):
        return self.context.sponsors

    @sponsors.setter
    def sponsors(self, data):
        if data is None:
            data = []

        # Create a dictionary of sponsors by their oid (id)
        sponsors = {
            v['oid']: v
            for v in (self.context.sponsors or [])
        }
        for index, item in enumerate(data):
            # check if an image was submitted with each individual sponsor
            if not item['picture']:
                key = item['oid']

                # check if the submitted id is present in the existing sponsors' id
                # if yes, store the image in the new field
                if key in sponsors:
                    data[index]['picture'] = sponsors[key]['picture']

        self.context.sponsors = data

Rendering the base64 image

Create a helper tool for converting base64 to data uri

View:

# -*- coding: utf-8 -*-\
from plone.formwidget.namedfile.converter import b64decode_file
from Products.Five import BrowserView
from plone.namedfile.file import NamedImage

class DataGridImage(BrowserView):
    def get(self, picture):
        if not picture:
            return None
        filename, data = b64decode_file(picture)
        data = NamedImage(data=data, filename=filename)
        return data

ZCML

<configure
    xmlns="http://namespaces.zope.org/zope"
    xmlns:browser="http://namespaces.zope.org/browser"
    xmlns:plone="http://namespaces.plone.org/plone"
    i18n_domain="leap.site">

  <!-- Set overrides folder for Just-a-Bunch-Of-Templates product -->
  <include package="z3c.jbot" file="meta.zcml" />

 <browser:page
    for="plone.app.layout.navigation.interfaces.INavigationRoot"
    name="datagrid_image"
    permission="zope2.Public"
    class=".datagrid.DataGridImage"
    allowed_attributes="get"
    />

</configure>

Template

  <tal:block tal:define="portal context/@@plone_portal_state/portal;
                         datagrid_image portal/@@datagrid_image">
    <div class="i-event-sponsors">
       <h4 class="i-event-sponsors-title">MAIN SPONSORS:</h4>
       <tal:block tal:repeat="sponsor data/context/sponsors"  >
           <tal:block tal:condition="python: sponsor['picture'] is not None">
               <img title="${sponsor/name}"
                    src="data:${python: image.contentType};base64, ${python: image.data.encode('base64')}"
                    tal:define="image python:datagrid_image.get(sponsor['picture'])" 
                    class="i-event-sponsor-img" />
           </tal:block>
        </tal:block>
    </div>
  </tal:block>

[tkimnguyen]

@b4oshany that was mind blowing :)

Here's what I have:

class ICourseRowSchema(Interface):
    foreign_course_syllabus = field.NamedFile(
        title=_(u'Foreign Course Syllabus'),
        description=_(u'Upload the syllabus that corresponds to the most recent date of review'),
        required=True,
    )

class IOIEStudyAbroadProgram2(Interface):
    title = schema.TextLine(
        title=_(u'Program Title'),
        description=_(
            u'The full Program Title will be displayed in all print and on-line marketing'),
        required=True,
    )

    widget('courses', DataGridFieldFactory)
    courses = schema.List(
        title=_(u'Courses'),
        description=_(
            u'List existing courses only.'),
        value_type=DictRow(title=u'Course', schema=ICourseRowSchema),
        required=False,
    )

When I view the POST, here are the relevant bits (the "Bleah" is the value I gave as the title). The action value nochange seems like it could be useful.

Content-Disposition: form-data; name="form.widgets.title"

Bleah
------WebKitFormBoundaryycjQMH910dvyh2bA
Content-Disposition: form-data; name="form.widgets.courses.0.widgets.foreign_course_syllabus.action"

nochange
------WebKitFormBoundaryycjQMH910dvyh2bA
Content-Disposition: form-data; name="form.widgets.courses.0-empty-marker"

1
------WebKitFormBoundaryycjQMH910dvyh2bA
Content-Disposition: form-data; name="form.widgets.courses.1.widgets.foreign_course_syllabus"; filename=""
Content-Type: application/octet-stream

------WebKitFormBoundaryycjQMH910dvyh2bA
Content-Disposition: form-data; name="form.widgets.courses.1-empty-marker"

1
------WebKitFormBoundaryycjQMH910dvyh2bA
Content-Disposition: form-data; name="form.widgets.courses.TT.widgets.foreign_course_syllabus"; filename=""
Content-Type: application/octet-stream

------WebKitFormBoundaryycjQMH910dvyh2bA
Content-Disposition: form-data; name="form.widgets.courses.TT-empty-marker"

1
------WebKitFormBoundaryycjQMH910dvyh2bA
Content-Disposition: form-data; name="form.widgets.courses.count"

1
------WebKitFormBoundaryycjQMH910dvyh2bA

[b4oshany]

lol @tkimnguyen, well, that works...

[tkimnguyen]

I think this problem was similar: https://github.com/plone/Products.CMFPlone/issues/1144 and it was fixed by https://github.com/plone/plone.formwidget.namedfile/commit/40f95a9762d4df0255d45c084ed7861ab64f1773

[tkimnguyen]

Ugh, been trying to track down where I could keep the existing value of NamedFile so that the 'nochange' action doesn't result in a new empty NamedFile from being built from the request. z3c.form is very convoluted. :(

[tkimnguyen]

@b4oshany I'm tempted to try your method, though my content type is really big and has quite a few PDFs that need to be uploaded. I'm afraid how big things will get when b64 encoded.

[tkimnguyen]

@b4oshany does your method work even when a person edits then saves without modifying the sponsor image fields? I just tried switching my NamedFile field to a ASCII field, along with the widget directive to use NamedFileFieldWidget, but that ASCII field still seems subject to this bug.

[b4oshany]

@tkimnguyen based on my understanding of datagridfield, it is using schema.Object to store the data directly inside the contexted object, but as json data, not as an Object data. Therefore, whenever you call the datagridfield property, you should get something like:

[{'file': <NamedFile - Object>, ....},
 {'file': <NamedFile - Object>, ....}]

In my example above, I used schema.ASCII with NamedImageFieldWidget to generate a base64 format of the file, which resulted in the following structure of datagridfiled property:

[{'file':  'data:image/jpeg;base64, LzlqLzRBQ...',
  ....
},{
 'file': 'data:image/jpeg;base64, Mzr7LkP0M...',
 ...
}]

After re-implementing and improving my approach in another project, I realized that I didn't have to use schema.ASCII with NamedImageFieldWidget to accomplish the rendering and downloading of the image. All I needed to do was to ensure that the file wasn't overwritten. This was already done by the sponsors.setter method. Therefore, I changed schema.ASCII with NamedImageFieldWidget to NamedBlobImage field. Afterwards, I used existing plone portal functions to render the image object.

In your case, you would have to find a portal function that generates a download link from a file object or create a poral function that does this. See Timestamp Download URL in the Plone documentation.

[b4oshany]

@tkimnguyen I think I should make it clear that based on the things I've tried, ${context/absolute_url}/@@download/sponsors/${sponsor_index}/${picture/filename} was out of the picture. Apparently, the @@download portal function does not support list traversal.

[b4oshany]

@tkimnguyen Yes, it does work, even when a person edits then saves without modifying the sponsor image fields

[b4oshany]

@tkimnguyen Please note, that the method below is the reason why it works. The method basically copies the image data from the previously saved JSON data to the newly saved JSON data.

   @sponsors.setter
    def sponsors(self, data):
        if data is None:
            data = []

        # Create a dictionary of sponsors by their oid (id)
        sponsors = {
            v['oid']: v
            for v in (self.context.sponsors or [])
        }
        for index, item in enumerate(data):
            # check if an image was submitted with each individual sponsor
            if not item['picture']:
                key = item['oid']

                # check if the submitted id is present in the existing sponsors' id
                # if yes, store the image in the new field
                if key in sponsors:
                    data[index]['picture'] = sponsors[key]['picture']

        self.context.sponsors = data

[tkimnguyen]

When does that sponsors method get called? When I created my equivalent of your Sponsors class and its sponsors method (Courses and courses in my case) the setter does not get called at all.

[b4oshany]

The Sponsors class is called during the loading and saving of the edit form. It is a factory class of the schema class, ISponsors.

@implementer(ISponsors)
class Sponsors(object):
   ...

Please note, I did this as a behaviour, but it should be able to replicate as a non-behaviour factory.

    <plone:behavior
        title="Event Sponsors"
        description="Sponsors for events."
        provides=".events.ISponsors"
        factory=".events.Sponsors"
        />

[tkimnguyen]

Thx @b4oshany. I gave up :) I made a new Course content type which is addable to the Program content type (since it's folderish), rather than use the data grid.

[b4oshany]

lol.... I don't mind building out a behavior for you. In fact, I've been thinking of building an collective.behaviors paclage for cases like these.

On Fri, May 18, 2018, 3:41 PM T. Kim Nguyen notifications@github.com wrote:

Thx @b4oshany https://github.com/b4oshany. I gave up :) I made a new Course content type which is addable to the Program content type (since it's folderish), rather than use the data grid.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/collective/collective.z3cform.datagridfield/issues/2#issuecomment-390326280, or mute the thread https://github.com/notifications/unsubscribe-auth/ABvuXszugWGiO_bYkdic1ogE4eKF7dNdks5tzzHrgaJpZM4CHu2H .

[tkimnguyen]

It's very nice of you! The problem I have is that I'm confused by the (at least 3) different ways of defining content types. There's the old grok, using Interface, using schemas, and I am not grasping the behind the scenes logic that involves the actual objects.

"One day" I will come back and figure this bug out. Plone deserves it. Because we are Groot.

[b4oshany]

Honestly, from my baby days in Plone. David advice me to ignore grok. It's nice and all but its future is uncertain. From that moment on, I've focus on schema based content type. I can do the others, but it seems like not a lot of plone developers use them.

On Fri, May 18, 2018, 6:54 PM T. Kim Nguyen notifications@github.com wrote:

It's very nice of you! The problem I have is that I'm confused by the (at least 3) different ways of defining content types. There's the old grok, using Interface, using schemas, and I am not grasping the behind the scenes logic that involves the actual objects.

"One day" I will come back and figure this bug out. Plone deserves it. Because we are Groot.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/collective/collective.z3cform.datagridfield/issues/2#issuecomment-390360479, or mute the thread https://github.com/notifications/unsubscribe-auth/ABvuXvFEDrK2mLhYDMQ9fqlEGp0uDzEKks5tz187gaJpZM4CHu2H .

[Pernath]

Hello! I'm wondering if nowadays is there a simple fix to this troublesome bug.

[tkimnguyen]

@Pernath what I ended up doing is making a folderish content type (ie. Dexterity container) so instead of adding files via a File field in a data grid field, the files got added to the folder.

[david-batranu]

After two days of debugging z3c.form and attempting to have a working edit with NamedBlobImage fields in a collective.z3cform.datagridfield, this is my solution:

Hope this helps someone in the future, as @b4oshany's code helped me investigate.

# datagrid field definition

@provider(IFormFieldProvider)
class ICustomContent(model.Schema):
    """Marker interface and Dexterity Python Schema for CustomContent"""

    directives.widget(discover_more=DataGridFieldFactory)
    discover_more = DiscoverMoreField(
        title="Discover more",
        description="Items listing",
        value_type=DictRow(title="Item", schema=IDiscoverMore),
    )

# row schema

@implementer(IFromUnicode)
class IDiscoverMore(model.Schema):
    directives.mode(uid="hidden")
    uid = schema.TextLine(
        title="UID",
        required=True,
        defaultFactory=lambda: uuid.uuid4().hex,
    )
    icon = namedfile.NamedBlobImage(title="Icon")

# Factory adapter: this defines the data structure for 
# https://github.com/plone/plone.formwidget.namedfile/blob/master/plone/formwidget/namedfile/widget.py#L309-L311 
# so that it gets the NamedBlobImage instance, instead of <NO_VALUE>.
# 
# Had to implement my own FactoryAdapter and registerFactoryAdapter because the default one in 
# https://github.com/zopefoundation/z3c.form/blob/master/src/z3c/form/object.py#L422
# doesn't even pass the value to the adapter class (DiscoverMore).
# 
# DiscoverMore inherits from dict (as this is the type of value that the datagrid field is expecting) and refuses
# to set "image" to a NOT_CHANGED value.

@implementer(IDiscoverMore)
class DiscoverMore(dict):
    def __init__(self, wrapper, value):
        self["icon"] = self.get_icon(wrapper, value)

    def __setitem__(self, __k, v):
        if __k != "icon" or v != NOT_CHANGED:
            return super().__setitem__(__k, v)

    @staticmethod
    def get_icon(wrapper, value):
        context = wrapper.context or wrapper.form.context
        widget = wrapper.widget

        _, _, field_name, list_index = widget.name.split(".")

        from_value = value["icon"]

        if from_value == NOT_CHANGED:
            context_value = getattr(context, field_name, None)
            if context_value:
                return context_value[int(list_index)]["icon"]

class DiscoverMoreFactoryAdapter(FactoryAdapter):
    def __call__(self, value):
        # value is the extracted data from the form
        obj = self.factory(self, value)
        notify(ObjectCreatedEvent(obj))
        return obj

def registerFactoryAdapter(for_, klass):
    """register the basic FactoryAdapter for a given interface and class"""
    name = getIfName(for_)

    class temp(DiscoverMoreFactoryAdapter):
        factory = klass

    provideAdapter(temp, name=name)

registerFactoryAdapter(IDiscoverMore, DiscoverMore)

These steps are not enough for a working save functionality, as the result of the FactoryAdapter: DiscoverMore is NOT the same value that is saved on the context! A custom datamanager is required for that:

class IDiscoverMoreField(Interface):
    """ """

@implementer(IDiscoverMoreField)
class DiscoverMoreField(schema.List):
    """ """

@adapter(Interface, IDiscoverMoreField)
class DiscoverMoreDataManager(z3c.form.datamanager.AttributeField):
    """ """

    def set(self, value):
        """See z3c.form.interfaces.IDataManager"""

        existing = self.get()
        existing_by_uid = {d.get("uid", "<NO_VALUE>"): d for d in existing} if existing else {}

        for data in value:
            if data["icon"] == NOT_CHANGED:
                data["icon"] = existing_by_uid[data["uid"]]["icon"]

        super(DiscoverMoreDataManager, self).set(value)

provideAdapter(DiscoverMoreDataManager)

A custom field sub-classing schema.List is also required, so that we have a custom interface to register the data manager on.

Everything in one file:

# -*- coding: utf-8 -*-
import uuid

import z3c.form.datamanager
from z3c.form.interfaces import NOT_CHANGED
from z3c.form.object import FactoryAdapter
from z3c.form.object import getIfName
from zope.component import adapter
from zope.component import provideAdapter
from zope.event import notify
from zope.interface import Interface
from zope.interface import implementer
from zope.interface import provider
from zope.lifecycleevent import ObjectCreatedEvent
from zope.schema.interfaces import IFromUnicode

from collective.z3cform.datagridfield.datagridfield import DataGridFieldFactory
from collective.z3cform.datagridfield.row import DictRow
from plone import schema
from plone.autoform import directives
from plone.autoform.interfaces import IFormFieldProvider
from plone.dexterity.content import Container
from plone.namedfile import field as namedfile
from plone.supermodel import model

class IDiscoverMoreField(Interface):
    """ """

@implementer(IDiscoverMoreField)
class DiscoverMoreField(schema.List):
    """ """

@adapter(Interface, IDiscoverMoreField)
class DiscoverMoreDataManager(z3c.form.datamanager.AttributeField):
    """ """

    def set(self, value):
        """See z3c.form.interfaces.IDataManager"""

        existing = self.get()
        existing_by_uid = {d.get("uid", "<NO_VALUE>"): d for d in existing} if existing else {}

        for data in value:
            if data["icon"] == NOT_CHANGED:
                data["icon"] = existing_by_uid[data["uid"]]["icon"]

        super(DiscoverMoreDataManager, self).set(value)

provideAdapter(DiscoverMoreDataManager)

@implementer(IFromUnicode)
class IDiscoverMore(model.Schema):
    directives.mode(uid="hidden")
    uid = schema.TextLine(
        title="UID",
        required=True,
        defaultFactory=lambda: uuid.uuid4().hex,
    )
    icon = namedfile.NamedBlobImage(title="Icon")

@provider(IFormFieldProvider)
class ICustomContent(model.Schema):
    """Marker interface and Dexterity Python Schema for CustomContent"""

    directives.widget(discover_more=DataGridFieldFactory)
    discover_more = DiscoverMoreField(
        title="Discover more",
        description="Items listing",
        value_type=DictRow(title="Item", schema=IDiscoverMore),
    )

@implementer(IDiscoverMore)
class DiscoverMore(dict):
    def __init__(self, wrapper, value):
        self["icon"] = self.get_icon(wrapper, value)

    def __setitem__(self, __k, v):
        if __k != "icon" or v != NOT_CHANGED:
            return super().__setitem__(__k, v)

    @staticmethod
    def get_icon(wrapper, value):
        context = wrapper.context or wrapper.form.context
        widget = wrapper.widget

        _, _, field_name, list_index = widget.name.split(".")

        from_value = value["icon"]

        if from_value == NOT_CHANGED:
            context_value = getattr(context, field_name, None)
            if context_value:
                return context_value[int(list_index)]["icon"]

class DiscoverMoreFactoryAdapter(FactoryAdapter):
    def __call__(self, value):
        # value is the extracted data from the form
        obj = self.factory(self, value)
        notify(ObjectCreatedEvent(obj))
        return obj

def registerFactoryAdapter(for_, klass):
    """register the basic FactoryAdapter for a given interface and class"""
    name = getIfName(for_)

    class temp(DiscoverMoreFactoryAdapter):
        factory = klass

    provideAdapter(temp, name=name)

registerFactoryAdapter(IDiscoverMore, DiscoverMore)

@implementer(ICustomContent)
class CustomContent(Container):
    """ """

[ewohnlich]

@david-batranu 's solution worked for me except if the form as a whole fails validation. In this case the reloaded form has the file as 'no change' so it's ignored on the resubmit. This is in 2.0.1, on Plone 5.2.

I also can't get it to properly validate on that file itself. Client wants to impose a size limit. I tried with a constraint on the NamedBlobImage field and it does not seem to be called. I also attempted to do this in the handleAdd handler by raising a WidgetActionExecutionError, but this just errors out the page.

I think I'm going to have to make it a sub content type instead :(