plone / Products.CMFPlone

The core of the Plone content management system
https://plone.org
GNU General Public License v2.0
254 stars 191 forks source link

plone.app.iterate - checking a folder with a new child back in is throwing an exception. #665

Closed neilferreira closed 2 years ago

neilferreira commented 9 years ago

hey

I would just like to know if this is a bug or simply an unsupported feature.

I'm using plone.app.iterate to 'check out' and 'check in' pages. In this scenario the exception below is raised:

1) Create folder /foo 2) Check out /foo under /foo_copy 3) Create /bar under /foo_copy 4) 'Check In' /foo_copy 5) Exception below is raised, as '/bar' is a new child

  Module plone.app.stagingbehavior.policy, line 56, in checkin
  Module zope.event, line 31, in notify
  Module zope.component.event, line 24, in dispatch
  Module zope.component._api, line 136, in subscribers
  Module zope.component.registry, line 321, in subscribers
  Module zope.interface.adapter, line 585, in subscribers
  Module plone.app.iterate.subscribers.versioning, line 36, in handleAfterCheckin
  Module plone.app.iterate.archiver, line 43, in save
  Module Products.CMFEditions.CopyModifyMergeRepositoryTool, line 305, in save
  Module Products.CMFEditions.CopyModifyMergeRepositoryTool, line 451, in _recursiveSave
  Module Products.CMFEditions.ArchivistTool, line 267, in prepare
  Module Products.CMFEditions.ArchivistTool._cloneByPickle, line 207, in _cloneByPickle
  Module Products.CMFEditions.ModifierRegistryTool, line 193, in getOnCloneModifiers
  Module Products.CMFEditions.StandardModifiers, line 465, in getOnCloneModifiers
  Module Products.CMFEditions.StandardModifiers, line 410, in _getOnCloneModifiers
  Module Products.ZCatalog.Lazy, line 190, in __getitem__
  Module plone.folder.ordered, line 59, in _getOb
AttributeError: 'bar'

plone.app.iterate = 3.0.1 plone.locking = 2.0.5 plone.app.stagingbehavior = 0.1

vangheem commented 9 years ago

What happens when you check out a folderish content item that has existing items?

I would imagine this wouldn't work; however, the UI shouldn't allow you to create new contained objects when you check out.

I'd also expect that when you checkout a folder with items already in it that the check out copy does not also have copies of the contained items. I imagine it does a shallow copy. Otherwise, with large folders, the overhead of copying all that data would be enormous.

neilferreira commented 9 years ago

It doesn't actually do a shallow copy - that is another issue on a sidenote.

If you check out an item with children then it will merge the children back in just fine, it is just when there is a new child present that it messes up.

@dnouri was working on a copier that does a shallow copy but it was never finished and the SVN branch wasn't ported across to git. See http://plone.293351.n2.nabble.com/RFC-plone-app-iterate-copier-behaviour-for-folders-td2522621.html

vangheem commented 9 years ago

Oh my. This is a shame it wasn't implemented. We really need this IMO with dexterity as people will be doing folder types quite often.

vangheem commented 9 years ago

the branch isn't on the old svn either though: http://svn.plone.org/svn/plone/plone.app.iterate/branches/

pbauer commented 9 years ago

It would indeed be nice to have this. @dnouri do you still have your code lying around somewhere so we can resurrect it?

danmur commented 9 years ago

We've been looking at addressing this for one of our clients. Our plan was to add another copier implementation that checks registry settings to determine whether child content should be copied, so it could be controlled per-type for example. Does this sound like a reasonable approach? We'd be happy to contribute the code back.

vangheem commented 7 years ago

@danmur did you ever implement your idea? Sounds like a fine idea.

tkimnguyen commented 7 years ago

pinging @neilferreira too

tkimnguyen commented 7 years ago

https://community.plone.org/t/plone-app-iterate-shallow-copy/3996

danmur commented 7 years ago

@vangheem No, we've not had a client who needed it enough thus far (and have stopped including the addon as default in our projects).

From memory I got as far as working out that due to how child items are stored on each object it was quite difficult to make a copy without touching them; my notes say that there is a property called _tree, which is an OOBTree (implemented in C), so a first port of call might be to try using zope.copy and using a copy hook to avoid that property.

Sorry I can't be more help. We probably would end up trying this out at some point, but I can't say when or if what I described would even work.

jensens commented 6 years ago

is this still an issue in Plone 5.1 latest?

tomgross commented 6 years ago

It is. Here is a version we @ FHNW use successfully with c.folderishtypes. It removes all folder contents on checkout and restores it on checkin. I can't guarantee this is a generic solution usable for everyone.

https://github.com/FHNW/plone.app.iterate/tree/tg_norecursive_clean

Obviously there are issues with (path) query-selectors, which probably could be solved with special adapters but we never dug into it.

davisagli commented 2 years ago

Fixed in Plone 6: https://github.com/plone/plone.app.iterate/pull/84