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

[15.0][MIG] mail_preview_audio: Migration to 15.0 #1117

Merged
merged 7 commits into from
Apr 13, 2023

Conversation

chienandalu
Copy link
Member

@chienandalu chienandalu commented Mar 29, 2023

  • Standard migration

@Tecnativa TT41546

please review @CarlosRoca13

ping @etobella

Copy link
Member

@etobella etobella left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on runbot

@pedrobaeza
Copy link
Member

I think this is worth to be proposed to Odoo upstream to master, don't you think?

@pedrobaeza
Copy link
Member

/ocabot migration mail_preview_audio

@OCA-git-bot OCA-git-bot added this to the 15.0 milestone Mar 29, 2023
@OCA-git-bot OCA-git-bot mentioned this pull request Mar 29, 2023
36 tasks
@chienandalu
Copy link
Member Author

I think this is worth to be proposed to Odoo upstream to master, don't you think?

I spent some time thinking that it was indeed added in v14 as you can preview some audio files in the core attachments widget (you can with mp3, but you can't with wav, for example) that are run in the browser's video player.

But I was missing the point of the base module that adds a field widget for binary files that allows that previews in place.

image

So the whole binary field widget should be moved to have that.

@pedrobaeza
Copy link
Member

I don't quite get you. The idea is to use this preview option in the DMS. If not needed for most audio file (OGG, MP3, OPUS...), then let's get rid off this module. WAV is not so important IMO.

@chienandalu
Copy link
Member Author

chienandalu commented Mar 30, 2023

I don't quite get you. The idea is to use this preview option in the DMS. If not needed for most audio file (OGG, MP3, OPUS...), then let's get rid off this module. WAV is not so important IMO.

dms has mail_preview_base as a dependency which adds the mentioned preview_binary field widget (nothing to do with core) which is extended by this module to support audio files. (https://github.com/OCA/dms/blob/0142cb1fe6ae0ee3316d493e3d3c981f881f3ef0/dms/views/dms_file.xml#L418)

What core does have already is support for audio files in the attachments widget which this module used to support as well but now delegates to Odoo.

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 15.0-ocabot-merge-pr-1117-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit e82ff45 into OCA:15.0 Apr 13, 2023
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 018be18. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants