collective / collective.nitf

A Dexterity-based content type inspired on the News Industry Text Format specification
8 stars 3 forks source link

Error while rendering plone.belowcontenttitle.contents when adding a Link or a File inside a nitf content and plone.app.contenttypes is installed #228

Closed idgserpro closed 3 years ago

idgserpro commented 4 years ago

Do a collective.nitf checkout. Add plone.app.contenttypes and bootstrap the project, creating a Plone Site. Add a nitf content and an image and a pdf and a Link inside the nitf. When rendering the nitf view, you get:

Error while rendering plone.belowcontenttitle.contents

And the traceback:

2019-12-13 16:45:50 ERROR plone.app.viewletmanager Error while rendering viewlet-manager=plone.belowcontentbody, viewlet=plone.belowcontenttitle.contents
Traceback (most recent call last):
  File "/home/user/.buildout/eggs/plone.app.viewletmanager-2.0.11-py2.7.egg/plone/app/viewletmanager/manager.py", line 112, in render
    html.append(viewlet.render())
  File "/home/user/.buildout/eggs/zope.browserpage-3.12.2-py2.7.egg/zope/browserpage/simpleviewclass.py", line 44, in __call__
    return self.index(*args, **kw)
  File "/home/user/.buildout/eggs/Zope2-2.13.28-py2.7.egg/Products/Five/browser/pagetemplatefile.py", line 125, in __call__
    return self.im_func(im_self, *args, **kw)
  File "/home/user/.buildout/eggs/Zope2-2.13.28-py2.7.egg/Products/Five/browser/pagetemplatefile.py", line 59, in __call__
    sourceAnnotations=getattr(debug_flags, 'sourceAnnotations', 0),
  File "/home/user/.buildout/eggs/zope.pagetemplate-3.6.3-py2.7.egg/zope/pagetemplate/pagetemplate.py", line 132, in pt_render
    strictinsert=0, sourceAnnotations=sourceAnnotations
  File "/home/user/.buildout/eggs/zope.pagetemplate-3.6.3-py2.7.egg/zope/pagetemplate/pagetemplate.py", line 240, in __call__
    interpreter()
  File "/home/user/.buildout/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 271, in __call__
    self.interpret(self.program)
  File "/home/user/.buildout/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 343, in interpret
    handlers[opcode](self, args)
  File "/home/user/.buildout/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 852, in do_condition
    self.interpret(block)
  File "/home/user/.buildout/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 343, in interpret
    handlers[opcode](self, args)
  File "/home/user/.buildout/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 533, in do_optTag_tal
    self.do_optTag(stuff)
  File "/home/user/.buildout/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 518, in do_optTag
    return self.no_tag(start, program)
  File "/home/user/.buildout/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 513, in no_tag
    self.interpret(program)
  File "/home/user/.buildout/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 343, in interpret
    handlers[opcode](self, args)
  File "/home/user/.buildout/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 888, in do_useMacro
    self.interpret(macro)
  File "/home/user/.buildout/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 343, in interpret
    handlers[opcode](self, args)
  File "/home/user/.buildout/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 533, in do_optTag_tal
    self.do_optTag(stuff)
  File "/home/user/.buildout/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 518, in do_optTag
    return self.no_tag(start, program)
  File "/home/user/.buildout/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 513, in no_tag
    self.interpret(program)
  File "/home/user/.buildout/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 343, in interpret
    handlers[opcode](self, args)
  File "/home/user/.buildout/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 583, in do_setLocal_tal
    self.engine.setLocal(name, self.engine.evaluateValue(expr))
  File "/home/user/.buildout/eggs/zope.tales-3.5.3-py2.7.egg/zope/tales/tales.py", line 696, in evaluate
    return expression(self)
  File "/home/user/.buildout/eggs/zope.tales-3.5.3-py2.7.egg/zope/tales/expressions.py", line 217, in __call__
    return self._eval(econtext)
  File "/home/user/.buildout/eggs/Zope2-2.13.28-py2.7.egg/Products/PageTemplates/Expressions.py", line 147, in _eval
    ob = self._subexprs[-1](econtext)
  File "/home/user/.buildout/eggs/zope.tales-3.5.3-py2.7.egg/zope/tales/expressions.py", line 124, in _eval
    ob = self._traverser(ob, element, econtext)
  File "/home/user/.buildout/eggs/Zope2-2.13.28-py2.7.egg/Products/PageTemplates/Expressions.py", line 97, in trustedBoboAwareZopeTraverse
    request=request)
  File "/home/user/.buildout/eggs/zope.traversing-3.13.2-py2.7.egg/zope/traversing/adapters.py", line 136, in traversePathElement
    return traversable.traverse(nm, further_path)
  File "/home/user/.buildout/eggs/zope.traversing-3.13.2-py2.7.egg/zope/traversing/adapters.py", line 50, in traverse
    raise LocationError(subject, name)
LocationError: (<Products.Five.viewlet.viewlet.SimpleViewletClass from /home/user/plone/collective.nitf/src/collective/nitf/browser/templates/nitf_contents.pt object at 0x7f7a770f1610>, 'batch')

In https://github.com/collective/collective.nitf/blob/729a6248ef455ec1be5d46e7ed38e5817678c63f/src/collective/nitf/browser/templates/nitf_contents.pt#L4 it seems the the snippet was based on plone/dexterity/browser/containercontentcore.pt:

  <fieldset id="folder-listing">
      <legend>Contents</legend>
      <tal:block define="listing_macro context/folder_listing/macros/listing">
          <metal:use_macro use-macro="listing_macro" />
      </tal:block>
  </fieldset>

But the snippet that works is from plone/app/dexterity/browser/container.pt:

      <legend i18n:translate="" i18n:domain="plone">Contents</legend>
      <tal:block define="view nocall:context/folder_listing; listing_macro view/macros/listing|view/index/macros/listing">
          <metal:use_macro use-macro="listing_macro" />
      </tal:block>

But when using it, contentFilter python:{'portal_type': ['File', 'Link']}" doesn't work anymore and now all images of a nitf content are being shown as related when only Link and File should be.

In the top of https://github.com/collective/collective.nitf/blob/729a6248ef455ec1be5d46e7ed38e5817678c63f/src/collective/nitf/browser/templates/nitf_contents.pt#L4 we have TODO: Fix format; see: plone/app/layout/viewlets/document_relateditems.pt. So, the question is:

  1. Should we change the snippet to use the one from plone/app/dexterity/browser/container.pt, and read the documentation/source code to see if somehow it's possible to have a similar contentFilter functionality?
  2. Should we do like the TODO and copy what's in https://github.com/plone/plone.app.layout/blob/66defc53c59426a45adde215f8d8c17e11c592db/plone/app/layout/viewlets/document_relateditems.pt or even create a new viewlet class for plone.belowcontenttitle.contents in collective.nitf that inherits from it?

Workaround to remove the Error while rendering plone.belowcontenttitle.contents: you need to remove the files or links content types created as children for the nitf content.

idgserpro commented 4 years ago

I think 2 above is the best approach.

nitf_contents.pt can be removed, and a new class can be created for the plone.belowcontenttile.contents viewlet:

    <browser:viewlet
        for="collective.nitf.content.INITF"
        name="plone.belowcontenttitle.contents"
        manager="plone.app.layout.viewlets.interfaces.IBelowContentBody"
        class=".NITFBelowContentTitleContents"
        view="plone.app.layout.globals.interfaces.IViewView"
        permission="zope2.View"
        />

This new class inherit from plone.app.layout.viewlets.content import ContentRelatedItems, overriding related_items:

class NITFBelowContentTitleContents(ContentRelatedItems):
    def related_items(self):
        catalog = api.portal.get_tool('portal_catalog')
        path = '/'.join(self.context.getPhysicalPath())
        brains = catalog(
            Type=['Link', 'File'], path=path, sort_on='getObjPositionInParent',
        )
        return brains

This way, we fix TODO: Fix format; see: plone/app/layout/viewlets/document_relateditems.pt.

The disadvantage is that it may break some very specific layouts since the markup changed. But since it isn't working in the first pĺace if you have plone.app.contenttypes installed and it's using the related items markup this shouldn't raise any concerns IMHO.

What do you think, @rodfersou and @hvelarde ? In a nutshell, we show "related items" but limited to the nitf children excluding images.

rodfersou commented 4 years ago

Understand, agree with the changes.

@agnogueira are we still using those layouts? Do you see any problem?

hvelarde commented 4 years ago

@idgserpro I don't get it: the items inside the news article are not related items.

idgserpro commented 4 years ago

I inferred the proposal from the comment on the original code TODO: Fix format; see: plone/app/layout/viewlets/document_relateditems.pt. I know the items inside news article aren't "related items" the way Plone see this feature, but by doing so I could reuse the template and code from related items but now showing the children of a nitf content.

If you don't want this approach, we can try to keep what's right now on the viewlet, but we need a way to have contentFilter feature available as well.

rodfersou commented 4 years ago

If I understand correctly you have a different use case that is not Plone default.

So you should add a patch or override template into brasil.gov.portal package instead.

idgserpro commented 4 years ago

There's no different use case. plone.belowcontenttitle.contents is broken in collective.nitf as shown in https://github.com/collective/collective.nitf/issues/228#issue-537754019.

What we did was show alternatives to fix the problem and based on comments around the code (like TODO: Fix format; see: plone/app/layout/viewlets/document_relateditems.pt). The alternative is to use the code from related items: you can keep the template or copy it entirely changing the title of the pt becoming Contents instead of Related Items.

If you don't like the alternatives, we would like to know what you think that should be done to fix the problem.

rodfersou commented 4 years ago

Okay, I didn't understand right than ;-)

Really don't have a clue on how it should be fixed (sorry).