novacode-nl / odoo-formio

Odoo Forms app & modules
https://apps.odoo.com/apps/modules/18.0/formio/
Other
69 stars 90 forks source link

[FIX] serve css content from ir.attachment instead of filesystem #243

Closed daemonblade closed 1 year ago

daemonblade commented 1 year ago

This fixes https://github.com/novacode-nl/odoo-formio/issues/242

bobslee commented 1 year ago

@daemonblade Thanks for your PR. However, I can't accept this now - because it's too specific for your case.

Please improve with following suggestion.

Revert the code removed. Instead, add the idea (check) like below. In case you don't understand, let me know.. so I can do this within a few days.

if self.env['ir.attachment']._storage() == 'db':
    # ... your code ...
else:
    # expect the default storage, which is 'file'
    # ... orignal code ...
daemonblade commented 1 year ago

I don't quite agree with your suggestion, as _storage() can return other types aside from the 'db' and 'file'; depending on whether a particular Odoo instance has installed other storage mechanisms - eg S3 buckets, azure files.

Additionally, the code (without my fix) will not work for a multi-node installation. If the call to the controller ends up on an instance other than the one that store the filed, an Exception will occur.

The data is already stored in the ir.attachment; why not use it? Let the ORM take care of where it's pulling the data from.

bobslee commented 1 year ago

Ok I agree with you. I didn't remember the pluggable storage feature from Odoo, but indeed and it's nice ! it would be great to support the pluggable storage for this formio module.

However I notice the font (eg. fontawesome) files don't get resolved / found, because the form builder and components templates (in DOM) have the "fa-" class set (and expect this). You'll already notice that in the Form Builder's components sidebar, the icons are missing.

I tried to wrap my head against a solution, to provide those font files. But as fas as I know, those need to be resolved in a static way. Or can you come up with an idea or solution?

To pinpoint the fontfiles issue from a code perspective - You removed the code which copies the fonts within the Odoo filestore:

Suggestions If it's not possible to solve the fontfiles (icons) issue, we really first need to obtain whether _storage() == 'file' and still support the original implementation (so font/icons will appear). All other implementations can then become in a generic way, your code approach.

Also it would even be better to refactor this formio.js download/install and file serving code - To get better abstractions and modularity for extensions. But that's a different effort to undertake.

Looking forward for your answer or changes. Thanks!

bobslee commented 1 year ago

By the way, when setting a Python breakpoint after logger.info('download => %s' % self.archive_url) in file formio_version_github_tag.py ...

When you perform in Odoo:

You'll see the contents of the font files (formio.js tarball archive) directory contains, the list in screenshot.

It's located in the filestore/DB/formiojs/src/formio.js-VERSION/dist/fonts/

image

daemonblade commented 1 year ago

I've changed formio.version.asset to keep track of font-files as well. This allows the controller to retrieve the font-file from ir.attachment.

bobslee commented 1 year ago

@daemonblade thanks for your contribution! I tested, merged and did some additional changes, to still determine and support the static storage and serving of font files.

Remark, good to know: When swithing the ir_attachment.location the "formio.js versions" can be reset (download and install) to support that particular setup. I tested this case.