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

[16.0][MIG] base_multi_image #2618

Closed
wants to merge 49 commits into from
Closed

Conversation

yibudak
Copy link

@yibudak yibudak commented Apr 26, 2023

No description provided.

yajo and others added 30 commits April 26, 2023 09:58
…xisting attachment record + Fix the 'pre_init_hook_for_submodules()' hook to extract the images from the ir_attachment table for binary fields initialized with the 'attachment=True' parameter
…dme example * Only except for ImportError * Fix note regarding need of hook use
* Update openerp to odoo
* Bump versions
* Installable to True
* Add catch in owner unlink to allow for image delete bypass via context
lasley and others added 18 commits April 26, 2023 09:58
improve uninstall hook to move images from multi to single mode
Currently translated at 100.0% (48 of 48 strings)

Translation: server-tools-14.0/server-tools-14.0-base_multi_image
Translate-URL: https://translation.odoo-community.org/projects/server-tools-14-0/server-tools-14-0-base_multi_image/es_AR/
Currently translated at 58.3% (28 of 48 strings)

Translation: server-tools-15.0/server-tools-15.0-base_multi_image
Translate-URL: https://translation.odoo-community.org/projects/server-tools-15-0/server-tools-15-0-base_multi_image/it/
Currently translated at 64.5% (31 of 48 strings)

Translation: server-tools-15.0/server-tools-15.0-base_multi_image
Translate-URL: https://translation.odoo-community.org/projects/server-tools-15-0/server-tools-15-0-base_multi_image/sl/
Currently translated at 58.3% (28 of 48 strings)

Translation: server-tools-15.0/server-tools-15.0-base_multi_image
Translate-URL: https://translation.odoo-community.org/projects/server-tools-15-0/server-tools-15-0-base_multi_image/it/
Currently translated at 58.3% (28 of 48 strings)

Translation: server-tools-15.0/server-tools-15.0-base_multi_image
Translate-URL: https://translation.odoo-community.org/projects/server-tools-15-0/server-tools-15-0-base_multi_image/it/
@clb-openfire
Copy link

Hi,
To me this module has several problems that should be fixed (mentioned in #2601 ):

  • restrict the folders from which files can be read for security reasons, through the odoo config file.
  • fix hooks
  • remove "database" type of storage, which was actually storing in filestore since v13 anyway (I don't think that users can or should decide if files are stored in DB or in filestore)
  • remove "url" and "file" types of storage. image.mixin class stores all versions of images for performance reasons. I believe it would be even slower to get them from a file/remote url before resizing them. We should as well store everything in filestore. And by the way, restrict the size of those images, like it is done in base_import module, _import_image_by_url()
  • simplify the filestore type to make it work like the current database type.
  • adapt module to image.mixin (remove image_main, image_main_medium, etc. from owner)

I've done all that work already in the following fork if anyone interested: https://github.com/odof/server-tools/commits/16.0-base_multi_image

@thomaspaulb
Copy link
Contributor

Superseded by #3010 as well.

@clb-openfire We're going to try integrating your code, it looks like a lot of improvements.

Maybe you can give it a review once it's done?

Next time you can also just propose it yourself in a new PR.

@thomaspaulb thomaspaulb closed this Aug 9, 2024
@clb-openfire
Copy link

@thomaspaulb thanks, review done.
I wasn't confident enough to post a PR because a migration + rework seemed a lot to review, I'll be sure to do it next time.

@yibudak yibudak deleted the 16.0-mig-base_multi_image branch November 25, 2024 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.