basxsoftwareassociation / bread

Engine to create database applications based on Django and the IBM Carbon Design System
BSD 3-Clause "New" or "Revised" License
19 stars 2 forks source link

Feature/improve objectfieldvalue for onetomany #148

Closed wipascal closed 2 years ago

wipascal commented 2 years ago

this is for the file downloads inside ponds tables

saemideluxe commented 2 years ago

Okay, now after the fix on fix/svg I am not sure this is necessary anymore.

Also, as a side note, using str.join should be avoided in most places where we generate any HTML output. It basically enforces any BaseElements to be converted to strings, which should normally only be done by htmlgenerator's internal methods.

wipascal commented 2 years ago

Yes, it's still necessary because it makes it possible to format "candidatereferences.file" as a list of downloadable files...

saemideluxe commented 2 years ago

In that case I suggest we do not use ObjectFieldValue directly but just add a custom BaseElement-tree (or pass a custom formatter to ObjectFieldValue, but that might be less straight-forward).

The problem here is that ObjectFieldValue is a very generic element but join will not always work as expected, because it is not guaranteed that the resolved object is a string. And, maybe more importantly, when using mark_safe we should guarantee that the provided argument will never contain any html-strings, otherwise we have a potential XSS vulnerability.

We could theoretically also use hg.Iterator inside ObjectFieldValue.resolve to avoid those two issues. I am not too sure about that though. Sometimes we want a list of related items to be rendered as UL, sometimes with a comma, sometimes just with a single space, etc. To me it seems easier to just write something like

  hg.Iterator(
    hg.C("row.items"),
    "item",
    hg.BaseElement(hg.ObjectFieldValue("item", "file"), " ")
  )

or similar, instead of letting ObjectFieldValue do everything.

wipascal commented 2 years ago

Yes, I agree.

I think it will be better to just use the cell parameter each time we want something as complex as candidatereferences.file, ok?