-
Notifications
You must be signed in to change notification settings - Fork 70
[REF] Outlook: rework logging attachments #50
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
Conversation
a76dfd1 to
079f87a
Compare
Currently logging emails with attachments can encounter several issues, including: - Error if there's a mismatch between the number of img tags and inline attachments - Images will sometimes be embedded in the wrong order - URL images are incorrectly overwritten This commit refactors logRequest to be invariant to the email body and possible missing, extra, and/or out-of-order attachments. This is done by processing the .eml data directly and mapping attachments directly to img tags via contentId. Processing the .eml data is needed since the outlook-js API does not expose a method to fetch the attachment's contentId.
079f87a to
0836d2a
Compare
| "fontawesome-4.7": "^4.7.0", | ||
| "node-forge": "^0.10.0", | ||
| "office-ui-fabric-react": "^7.139.0", | ||
| "postal-mime": "^1.1.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm really not a fan of adding dependencies :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand as well, and I'm not a fan either. Unfortunately, parsing the eml data is necessary since there the native office api does not expose the attachments' contentId. postal-mime was the smallest and most focused library I found to get this working end-to-end.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I the end I found no other solution during my refactor, and I parsed the body like you did...
#54
|
@tde-banana-odoo Hi 🙂 Can you merge the pr? I investigated during my refactor, and there's indeed no better solution... |
|
@tde-banana-odoo Ping 🙂 there's now a ticket about it: https://www.odoo.com/odoo/my-support-tasks/5168674 |
|
@juwu-odoo Hi 🙂 can you re-target the branch to 19.0? |
|
Retargeted to 19.0 #55 |
Currently logging emails with attachments can encounter several issues, including:
This commit refactors logRequest to be invariant to the email body and possible missing, extra, and/or out-of-order attachments. This is done by processing the .eml data directly and mapping attachments directly to img tags via contentId. Processing the .eml data is needed since the outlook-js API does not expose a method to fetch the attachment's contentId.