km: use context free document lookup in "document renderer"

This gadget looks up a document by reference, by using
getDocumentValueList on the context of the web section.
getDocumentValueList also applies the criterions from the context, but
because web page references are already unique, adding more criterions
can just cause the document to be not found and the gadget to display
the "No document" error.

bt5/erp5_km/SkinTemplateItem/portal_skins/erp5_km_widget_library/WebSite_viewDocumentRender.py

@@ -8,7 +8,7 @@ if box_relative_url:
 preferences = box.KnowledgeBox_getDefaultPreferencesDict()
 reference=preferences.get('preferred_document_reference', None)
 if reference is not None:
-  web_page_list = context.getDocumentValueList(reference=reference, all_languages=True,
+  web_page_list = portal.portal_catalog.getDocumentValueList(reference=reference, all_languages=True,
                                          portal_type='Web Page')
   if len(web_page_list):
     return '<div class="web-page-renderer">%s</div>' %web_page_list[0].getObject().asStrippedHTML()

[Kazuhiko Shiozaki]

As this is WebSite_viewDocumentRender, it should be rather context.getWebSiteValue().getDocumentValueList() and this is what I already did on my project.

[Jérome Perrin]

but if the web site has predicate configured, there's a risk that the web page is not found, when the predicate configuration does not match, isn't it ?

[Kazuhiko Shiozaki]

It's not a risk, it can be a benefit. :)

We should not publish a Web Page that does not match site's predicate, otherwise it can be more serious risk if we provide several Web Sites on a single ERP5.

[Jérome Perrin]

Ah that's interesting. Actually, this was my initial though, but the traversal (ie. looking up "document_reference" when visiting https://erp5.example.com/web_site_module/web_site_id/document_reference ) uses a context free lookup ( https://lab.nexedi.com/nexedi/erp5/blob/65dde4a1342a3ad4a5c64cb316a0176a8c46e8f3/product/ERP5/bootstrap/erp5_core/MixinTemplateItem/portal_components/mixin.erp5.BaseExtensibleTraversableMixin.py#L162 / https://lab.nexedi.com/nexedi/erp5/blob/e48145b956cb2fcbd5a5fbc176a0c218d72d8626/bt5/erp5_web/SkinTemplateItem/portal_skins/erp5_web/WebSection_getDocumentValue.py#L16 ) and can find documents not "members" of the web section. I read a bit history of !1549 (comment 152817) and talked a bit with @vpelletier and came up with this patch, which is consistent with traversal behavior, but the more I look at this, the more I think the behavior of websection traversal is not correct, this is maybe the reason for this kind of things:

https://www.nexedi.com/wendelin/documentation/developer/howto/erp5-Guideline.Business.Library.Fields.Proxy.From.Core.Fields "nexedi" / "wendelin"/ "erp5" in the same URL does not look correct.

maybe @romain also has an opinion on this. If we change web section traversal rules now, it would be a big breaking change.

[Kazuhiko Shiozaki]

Hmm...

!1549 (comment 152817)

we rather need to use a context-free alternative to getDocumentValueList, which applies all publication rules without depending on a predicate.

Before, we only had WebSection_getDocumentValueList (and somehow similar NotificationTool_getDocumentValue) and it is me who introduced generic getDocumentValueList in catalog level (d307b9cb).

If you want anything predicate-free, please name it like ERP5Site_xxx, not WebSite_xxx or WebSection_xxx.

[Jérome Perrin]

If you want anything predicate-free, please name it like ERP5Site_xxx, not WebSite_xxx or WebSection_xxx.

Ah yes, you are right. We could easily rename this skin WebSite_viewDocumentRender -> ERP5Site_viewDocumentRender and by looking a bit at the code, it seems we can also rename WebSection_getDocumentValue to ERP5Site_getDocumentValue.

This would already be a step in the good direction, we would not feel bad that we have WebSection_ scripts ignoring context and this would not be a breaking change, right ?

[Kazuhiko Shiozaki]

it seems we can also rename WebSection_getDocumentValue to ERP5Site_getDocumentValue.

Oh... WebSection_getDocumentValueList passes search_context to getDocumentValueList but WebSection_getDocumentValue does not...

[Romain Courteaud]

maybe @romain also has an opinion on this. If we change web section traversal rules now, it would be a big breaking change.

I don't think we could safely restrict such traversal based on the web section predicates. That would probably break most of our sites, and it will break all existing links linking to them.

I was wondering if I could try such change in erp5_web_js_style but I don't see how this could be done, as it would increase the web site/sections configuration complexity, without seeing the advantages for now (from user point of view, I do not easily know how a web section predicate is working when writing a web page document).

I'm more concerned about uncontrolled link generation (like this MR), but that's another topic.

[Jérome Perrin]

Thanks @romain . Maybe the configuration won't become more complex, if the default predicate is to match all documents, then we have the same behavior as before ( I did not check what is the default predicate ).

So the problem here is that WebSection.getDocumentValue (and traversal) is different from WebSection.getDocumentValueList and that did not seem to be intentional and if we change this it may break many web sites. Personally I don't really have any interest in changing this.