Skip to content

[16.0][MIG] attachment_synchronize#2632

Merged
OCA-git-bot merged 47 commits intoOCA:16.0from
akretion:16.0-mig-attachment_synchronize-fs
Jan 30, 2024
Merged

[16.0][MIG] attachment_synchronize#2632
OCA-git-bot merged 47 commits intoOCA:16.0from
akretion:16.0-mig-attachment_synchronize-fs

Conversation

@florian-dacosta
Copy link
Contributor

Depends on OCA/storage#252

Adapt the module to the new fs_storage module which will replace storage_backend very soon.
Aims to replace #2562 when fs_storage will be merged.

@bealdav @kevinkhao

@florian-dacosta florian-dacosta force-pushed the 16.0-mig-attachment_synchronize-fs branch from 59fe02e to 172f09f Compare May 29, 2023 14:57
@florian-dacosta florian-dacosta force-pushed the 16.0-mig-attachment_synchronize-fs branch from 172f09f to f578312 Compare June 5, 2023 20:01
help="Used to fill the 'Failure Emails' field in the 'Attachments Queues' "
"related to this task.\nAn alert will be sent to these emails if any operation "
"on these Attachment Queue's file type fails.",
)
Copy link
Member

@bealdav bealdav Jun 6, 2023

Choose a reason for hiding this comment

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

Hi @florian-dacosta thanks for this nice integration.
Side consideration. Odoo introduced activities in 11.0 as internal communication system.
Clearly, users in odoo do not use it as much as they could do.
Flows in this module are fully managed by Odoo objects but mail is a communication mode to exchange informations between two systems. Here we have a unique system, Odoo
Currently our odoo users in our projects complains they receive too many mails in a day from misc sources for so many usage.
I don't talk about environmental considerations, but we could have this argument too
I think we should promote definitively this kind of communication in a such version.
Replace here failure_emails here by failure_activity_user_ids could definitively imply users to use activities instead of emails.
It's an opportunity to improve communication, make it smarter, interactive, only few bytes consumer

#makestheplanetgreenagain

My 2 cts

cc @kevinkhao

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @bealdav
Thanks for the remark. It is true we may want to implement activities.
This module is not responsible for this part though, as it just extends it, but the base implementation of these failure email is in the base module (attachment_queue) which is merged.

I think it would be nice to have an implementation with the activities indeed, and we could have another one for those still prefering email.
We'd just have to decide if we'd implement both in base module, or only activities and email as a separated module, or both in separated modules...

Overall I agree, but I think it should be put on the roadmap. (I may work on it but I don't want it to block the PR)

florian-dacosta and others added 27 commits September 7, 2023 11:17
- match pattern filename to download
- empty list as default domain
- prefer enabled in place of active
- log info on run completed
- correct `directory_output` value

- mute logger for desired raise exception

- change how the "already imported files" are checked when
"check_duplicated_files = True" :
It occurs that a task o2m field `attachment_ids` returns an empty record
when checked during the `_file_to_import()` method... but only when
the module `autovacuum_message_attachment` is installed !
@florian-dacosta florian-dacosta force-pushed the 16.0-mig-attachment_synchronize-fs branch from f578312 to deaa0b8 Compare September 7, 2023 09:18
@florian-dacosta
Copy link
Contributor Author

@bealdav @kevinkhao @bguillot
Ready for review now that https://github.com/OCA/storage/tree/16.0/fs_storage has been merged.

@florian-dacosta
Copy link
Contributor Author

Hi @bealdav @kevinkhao @bguillot
I would still apreciate if you could do a review on this one.
It is used in production since some weeks now with no issue.

Copy link
Contributor

@kevinkhao kevinkhao left a comment

Choose a reason for hiding this comment

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

code review LGTM

Copy link
Member

@clementmbr clementmbr left a comment

Choose a reason for hiding this comment

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

LGTM!

@bguillot
Copy link
Contributor

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-2632-by-bguillot-bump-nobump, awaiting test results.

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@OCA-git-bot OCA-git-bot merged commit 1fb41c2 into OCA:16.0 Jan 30, 2024
@OCA-git-bot
Copy link
Contributor

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

@bealdav bealdav deleted the 16.0-mig-attachment_synchronize-fs branch January 30, 2024 11:18
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.

10 participants

Comments