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

After drag and drop implementation, cover breaks if you remove the referenced item in a tile #588

Closed
idgserpro opened this issue Feb 24, 2016 · 10 comments · Fixed by #589
Closed
Assignees
Labels

Comments

@idgserpro
Copy link
Member

After the commit adb1a67, if you normally add a File to a tile in compose view, them remove the File from folder_contents and go to compose again you get

Traceback (innermost last):
  Module ZPublisher.Publish, line 138, in publish
  Module ZPublisher.mapply, line 77, in mapply
  Module ZPublisher.Publish, line 48, in call_object
  Module grokcore.view.components, line 150, in __call__
  Module grokcore.view.components, line 154, in _render_template
  Module five.grok.components, line 130, in render
  Module zope.pagetemplate.pagetemplate, line 132, in pt_render
  Module zope.pagetemplate.pagetemplate, line 240, in __call__
  Module zope.tal.talinterpreter, line 271, in __call__
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 888, in do_useMacro
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 533, in do_optTag_tal
  Module zope.tal.talinterpreter, line 518, in do_optTag
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 954, in do_defineSlot
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 533, in do_optTag_tal
  Module zope.tal.talinterpreter, line 518, in do_optTag
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 858, in do_defineMacro
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 954, in do_defineSlot
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 533, in do_optTag_tal
  Module zope.tal.talinterpreter, line 518, in do_optTag
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 946, in do_defineSlot
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 533, in do_optTag_tal
  Module zope.tal.talinterpreter, line 518, in do_optTag
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 531, in do_optTag_tal
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 742, in do_insertStructure_tal
  Module Products.PageTemplates.Expressions, line 218, in evaluateStructure
  Module zope.tales.tales, line 696, in evaluate
   - URL: /home/user/Desktop/collective.cover/src/collective/cover/browser/templates/compose.pt
   - Line 33, Column 4
   - Expression: <PathExpr standard:u'layout/render_compose'>
   - Names:
      {'args': (),
       'container': <Cover at /Plone/capa>,
       'context': <Cover at /Plone/capa>,
       'default': <object object at 0xb74ff858>,
       'here': <Cover at /Plone/capa>,
       'loop': {},
       'nothing': None,
       'options': {},
       'repeat': <Products.PageTemplates.Expressions.SafeMapping object at 0xb12fec0c>,
       'request': <HTTPRequest, URL=http://localhost:8080/Plone/capa/compose>,
       'root': <Application at >,
       'static': None,
       'template': <Products.Five.browser.pagetemplatefile.ViewPageTemplateFile object at 0xb394e96c>,
       'traverse_subpath': [],
       'user': <PropertiedUser 'admin'>,
       'view': <collective.cover.browser.cover.Compose object at 0xb17ace6c>,
       'views': <Products.Five.browser.pagetemplatefile.ViewMapper object at 0xb4300b4c>}
  Module zope.tales.expressions, line 217, in __call__
  Module Products.PageTemplates.Expressions, line 155, in _eval
  Module Products.PageTemplates.Expressions, line 117, in render
  Module collective.cover.layout, line 135, in render_compose
  Module zope.browserpage.viewpagetemplatefile, line 83, in __call__
  Module zope.browserpage.viewpagetemplatefile, line 51, in __call__
  Module zope.pagetemplate.pagetemplate, line 132, in pt_render
  Module zope.pagetemplate.pagetemplate, line 240, in __call__
  Module zope.tal.talinterpreter, line 271, in __call__
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 531, in do_optTag_tal
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 531, in do_optTag_tal
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 533, in do_optTag_tal
  Module zope.tal.talinterpreter, line 518, in do_optTag
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 821, in do_loop_tal
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 533, in do_optTag_tal
  Module zope.tal.talinterpreter, line 518, in do_optTag
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 531, in do_optTag_tal
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 742, in do_insertStructure_tal
  Module zope.tales.tales, line 696, in evaluate
   - URL: /home/user/Desktop/collective.cover/src/collective/cover/layout_templates/pagelayout.pt
   - Line 11, Column 16
   - Expression: <PythonExpr (view.render_section(section, mode))>
   - Names:
      {'args': (),
       'context': <Cover at /Plone/capa>,
       'default': <object object at 0xb74ff858>,
       'loop': {},
       'nothing': None,
       'options': {'mode': 'compose'},
       'repeat': {},
       'request': <HTTPRequest, URL=http://localhost:8080/Plone/capa/compose>,
       'template': <zope.browserpage.viewpagetemplatefile.ViewPageTemplateFile object at 0xb3be6e6c>,
       'view': <collective.cover.layout.PageLayout object at 0xb17712ac>,
       'views': <zope.browserpage.viewpagetemplatefile.ViewMapper object at 0xb177172c>}
  Module zope.tales.pythonexpr, line 59, in __call__
   - __traceback_info__: (view.render_section(section, mode))
  Module <string>, line 1, in <module>
  Module collective.cover.layout, line 87, in render_section
  Module zope.browserpage.viewpagetemplatefile, line 83, in __call__
  Module zope.browserpage.viewpagetemplatefile, line 51, in __call__
  Module zope.pagetemplate.pagetemplate, line 132, in pt_render
  Module zope.pagetemplate.pagetemplate, line 240, in __call__
  Module zope.tal.talinterpreter, line 271, in __call__
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 531, in do_optTag_tal
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 531, in do_optTag_tal
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 533, in do_optTag_tal
  Module zope.tal.talinterpreter, line 518, in do_optTag
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 852, in do_condition
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 533, in do_optTag_tal
  Module zope.tal.talinterpreter, line 518, in do_optTag
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 852, in do_condition
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 821, in do_loop_tal
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 533, in do_optTag_tal
  Module zope.tal.talinterpreter, line 518, in do_optTag
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 531, in do_optTag_tal
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 742, in do_insertStructure_tal
  Module zope.tales.tales, line 696, in evaluate
   - URL: /home/user/Desktop/collective.cover/src/collective/cover/layout_templates/row.pt
   - Line 35, Column 24
   - Expression: <PythonExpr (view.render_section(child, mode))>
   - Names:
      {'args': (),
       'context': <Cover at /Plone/capa>,
       'default': <object object at 0xb74ff858>,
       'loop': {},
       'nothing': None,
       'options': {'mode': 'compose',
                   'section': {u'children': [{u'children': [{'class': 'tile',
                                                             'css_class': u'tile tile-default',
                                                             u'id': u'a288bf9d-58dd-41f0-b762-6b15bac74a6c',
                                                             'tile-title': u'File Tile',
                                                             u'tile-type': u'collective.cover.file',
                                                             u'type': u'tile'}],
                                              'class': 'cell width-16 position-0',
                                              u'column-size': 16,
                                              u'roles': [u'Manager'],
                                              u'type': u'group'}],
                               'class': 'row',
                               u'type': u'row'}},
       'repeat': {},
       'request': <HTTPRequest, URL=http://localhost:8080/Plone/capa/compose>,
       'template': <zope.browserpage.viewpagetemplatefile.ViewPageTemplateFile object at 0xb3be6fcc>,
       'view': <collective.cover.layout.PageLayout object at 0xb17712ac>,
       'views': <zope.browserpage.viewpagetemplatefile.ViewMapper object at 0xb14e7bac>}
  Module zope.tales.pythonexpr, line 59, in __call__
   - __traceback_info__: (view.render_section(child, mode))
  Module <string>, line 1, in <module>
  Module collective.cover.layout, line 89, in render_section
  Module zope.browserpage.viewpagetemplatefile, line 83, in __call__
  Module zope.browserpage.viewpagetemplatefile, line 51, in __call__
  Module zope.pagetemplate.pagetemplate, line 132, in pt_render
  Module zope.pagetemplate.pagetemplate, line 240, in __call__
  Module zope.tal.talinterpreter, line 271, in __call__
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 531, in do_optTag_tal
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 531, in do_optTag_tal
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 533, in do_optTag_tal
  Module zope.tal.talinterpreter, line 518, in do_optTag
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 852, in do_condition
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 533, in do_optTag_tal
  Module zope.tal.talinterpreter, line 518, in do_optTag
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 852, in do_condition
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 533, in do_optTag_tal
  Module zope.tal.talinterpreter, line 518, in do_optTag
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 821, in do_loop_tal
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 533, in do_optTag_tal
  Module zope.tal.talinterpreter, line 518, in do_optTag
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 531, in do_optTag_tal
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 742, in do_insertStructure_tal
  Module zope.tales.tales, line 696, in evaluate
   - URL: /home/user/Desktop/collective.cover/src/collective/cover/layout_templates/group.pt
   - Line 34, Column 28
   - Expression: <PythonExpr (view.render_section(child, mode))>
   - Names:
      {'args': (),
       'context': <Cover at /Plone/capa>,
       'default': <object object at 0xb74ff858>,
       'loop': {},
       'nothing': None,
       'options': {'mode': 'compose',
                   'section': {u'children': [{'class': 'tile',
                                              'css_class': u'tile tile-default',
                                              u'id': u'a288bf9d-58dd-41f0-b762-6b15bac74a6c',
                                              'tile-title': u'File Tile',
                                              u'tile-type': u'collective.cover.file',
                                              u'type': u'tile'}],
                               'class': 'cell width-16 position-0',
                               u'column-size': 16,
                               u'roles': [u'Manager'],
                               u'type': u'group'}},
       'repeat': {},
       'request': <HTTPRequest, URL=http://localhost:8080/Plone/capa/compose>,
       'template': <zope.browserpage.viewpagetemplatefile.ViewPageTemplateFile object at 0xb3bef0cc>,
       'view': <collective.cover.layout.PageLayout object at 0xb17712ac>,
       'views': <zope.browserpage.viewpagetemplatefile.ViewMapper object at 0xb14e7b4c>}
  Module zope.tales.pythonexpr, line 59, in __call__
   - __traceback_info__: (view.render_section(child, mode))
  Module <string>, line 1, in <module>
  Module collective.cover.layout, line 103, in render_section
  Module zope.browserpage.viewpagetemplatefile, line 83, in __call__
  Module zope.browserpage.viewpagetemplatefile, line 51, in __call__
  Module zope.pagetemplate.pagetemplate, line 132, in pt_render
  Module zope.pagetemplate.pagetemplate, line 240, in __call__
  Module zope.tal.talinterpreter, line 271, in __call__
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 531, in do_optTag_tal
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 531, in do_optTag_tal
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 533, in do_optTag_tal
  Module zope.tal.talinterpreter, line 518, in do_optTag
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 852, in do_condition
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 533, in do_optTag_tal
  Module zope.tal.talinterpreter, line 518, in do_optTag
  Module zope.tal.talinterpreter, line 513, in no_tag
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 852, in do_condition
  Module zope.tal.talinterpreter, line 343, in interpret
  Module zope.tal.talinterpreter, line 405, in do_startTag
  Module zope.tal.talinterpreter, line 482, in attrAction_tal
  Module zope.tales.tales, line 704, in evaluateText
  Module zope.tales.tales, line 696, in evaluate
   - URL: /home/user/Desktop/collective.cover/src/collective/cover/layout_templates/tile.pt
   - Line 48, Column 20
   - Expression: <PythonExpr (view.get_content_portal_type(tile_type, tile_id))>
   - Names:
      {'args': (),
       'context': <Cover at /Plone/capa>,
       'default': <object object at 0xb74ff858>,
       'loop': {},
       'nothing': None,
       'options': {'mode': 'compose',
                   'section': {'class': 'tile',
                               'css_class': u'tile tile-default',
                               u'id': u'a288bf9d-58dd-41f0-b762-6b15bac74a6c',
                               'tile-title': u'File Tile',
                               u'tile-type': u'collective.cover.file',
                               u'type': u'tile'},
                   'tile_url': '@@collective.cover.file/a288bf9d-58dd-41f0-b762-6b15bac74a6c'},
       'repeat': {},
       'request': <HTTPRequest, URL=http://localhost:8080/Plone/capa/compose>,
       'template': <zope.browserpage.viewpagetemplatefile.ViewPageTemplateFile object at 0xb3bef12c>,
       'view': <collective.cover.layout.PageLayout object at 0xb17712ac>,
       'views': <zope.browserpage.viewpagetemplatefile.ViewMapper object at 0xb179076c>}
  Module zope.tales.pythonexpr, line 59, in __call__
   - __traceback_info__: (view.get_content_portal_type(tile_type, tile_id))
  Module <string>, line 1, in <module>
  Module collective.cover.layout, line 157, in get_content_portal_type
AttributeError: 'NoneType' object has no attribute 'portal_type'

The error happens here adb1a67#diff-37e197e3d8e3a93bbdc7cb51b15f57a3R156: there's an uuid in the tile, but the object that has that uuid doesn't exist anymore.

Don't know what is the best solution here: if obj is None, return None? Say a warning and return None? Throw an exception, but just for the tile (like it happens in the cover view if a tile throws an exception)? Create a subscriber when deleting items to see if there's any reference in tiles? @rodfersou

This has to be fixed before a new release. The cover isn't broken after removing the item, but /compose breaks.

@hvelarde
Copy link
Member

thanks for your report! we are going to fix this before making a new release.

@idgserpro
Copy link
Member Author

...would it be the case to add an upgradeStep that:

  • Search through all tiles in zodb;
  • Check if they have uuid that is not None;
  • If None, do nothing;
  • If it's not None, see if the object exists. If it exists, do nothing, if it doesn't, set uuid to None.

What do you think?

@rodfersou
Copy link
Member

we are talking about what tile?

@rodfersou
Copy link
Member

found it, collective.cover.file

@idgserpro
Copy link
Member Author

This is just as an example, every tile that references to a content by uuid will have this problem if the content is excluded from ZODB. Do you agree with the upgradeStep?

@rodfersou
Copy link
Member

I think every tile can check if the content don't exist and then set as None when try to show the tile.

@rodfersou
Copy link
Member

no need to create an upgrade step

@idgserpro
Copy link
Member Author

Indeed, much more simple! Great idea.

idgserpro added a commit to plonegovbr/brasil.gov.portal that referenced this issue Feb 25, 2016
Existem tiles do tipo "Header" que referenciam, através do uuid,
conteúdos inexistentes. Após a implementação de arrastar e soltar entre
tiles na próxima versão 1.0a13 de collective.cover em

collective/collective.cover@adb1a67

se um objeto possui um uuid a um objeto que não existe, isso quebra a
tela "Compor."

Essa correção também precisa ser efetuada no collective.cover e será
feita em

collective/collective.cover#588

Portais já criados, que por ventura já tenham persistido esse tile e o
usuário apenas mudou o título (e não refez a capa) não terão esse
erro uma vez que uma lógica que seta "None" caso seja inválido será
feita como definido em

collective/collective.cover#588 (comment)

Dessa forma, não é necessário um upgradeStep em brasil.gov.portal.
@idgserpro
Copy link
Member Author

Will you add the implementation of setting "None" in the same pull request?

@rodfersou
Copy link
Member

The problem was another.. I added a method to add portal_types into data attribute, and didn't check if the object exists.

Tested with all tiles that you can drop an image or file.

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