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

NamedImage and NamedFile widgets do not retain content when modified #2

Open
zedr opened this issue Feb 28, 2012 · 28 comments
Open

NamedImage and NamedFile widgets do not retain content when modified #2

zedr opened this issue Feb 28, 2012 · 28 comments
Labels

Comments

@zedr
Copy link

zedr commented Feb 28, 2012

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
Copy link
Author

zedr commented Feb 29, 2012

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

@davisagli
Copy link
Member

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
Copy link
Contributor

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

@miohtama
Copy link
Contributor

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

@puittenbroek
Copy link

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

@2silver
Copy link
Contributor

2silver commented May 3, 2015

Still open Plone 4.3.4.1, dexterity 2.0.12

@jensens jensens added the bug label Jul 30, 2015
@b4oshany
Copy link

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

@b4oshany
Copy link

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
Copy link
Member

tkimnguyen commented May 2, 2018

@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
Copy link

b4oshany commented May 2, 2018

lol @tkimnguyen, well, that works...

@tkimnguyen tkimnguyen changed the title NamedImage widgets do not retain content when modified NamedImage and NamedFile widgets do not retain content when modified May 3, 2018
@tkimnguyen
Copy link
Member

I think this problem was similar: plone/Products.CMFPlone#1144 and it was fixed by plone/plone.formwidget.namedfile@40f95a9

@tkimnguyen
Copy link
Member

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
Copy link
Member

@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
Copy link
Member

@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
Copy link

b4oshany commented May 17, 2018

@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
Copy link

@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
Copy link

b4oshany commented May 17, 2018

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

@b4oshany
Copy link

b4oshany commented May 17, 2018

@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
Copy link
Member

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
Copy link

b4oshany commented May 18, 2018

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
Copy link
Member

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
Copy link

b4oshany commented May 18, 2018 via email

@tkimnguyen
Copy link
Member

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
Copy link

b4oshany commented May 19, 2018 via email

@Pernath
Copy link

Pernath commented Jul 1, 2019

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

@tkimnguyen
Copy link
Member

@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
Copy link
Member

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
Copy link

@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 :(

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

No branches or pull requests