-
Notifications
You must be signed in to change notification settings - Fork 10.7k
[IMP] email: add mail.default.from_filter #2694
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
b15a224 to
81ad8f0
Compare
|
ok for me for the default email part :) |
tiku-odoo
left a comment
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.
@samueljlieber This PR is ready for a technical review.
@std-odoo Can formally approve this PR when you have a moment?
std-odoo
left a comment
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.
LGTM for the default email stuff :)
(I don't know enough the rest to know if it's 100% correct)
81ad8f0 to
068f0da
Compare
|
Thanks @std-odoo for your speedy review! I'm tagging @mivu-odoo for a final content review 🙂 CC @tiku-odoo |
mivu-odoo
left a comment
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.
Hello @samueljlieber and @tiku-odoo! Thank you for your patience on this PR.
I found a couple of instances of familiar second-person pronoun use and awkward language. Please let me know if you need any clarifications on my suggestions.
When you're ready, please tag me for a final look. Thank you 😸
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
068f0da to
fcc53a2
Compare
|
Hi @mivu-odoo 🙂 |
mivu-odoo
left a comment
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.
Hello @samueljlieber and @tiku-odoo!
Couple of revisions needed for abbreviations. Let me know when you're ready, and I can do a final check. Thank you!
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
fcc53a2 to
39dc233
Compare
|
Hi @mivu-odoo ready for a final check, thank you! 🙂 |
mivu-odoo
left a comment
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.
Hello @samueljlieber and @tiku-odoo!
Some small changes left, but once those go through feel free to send it straight to ZST for final technical review. No need to tag me again! Thank you 😸
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
39dc233 to
a26c623
Compare
|
Hi @StraubCreative, this doc is ready for a final technical review 🙂 |
a26c623 to
b5773fb
Compare
|
Merge conflict solved on b5773fb. Technical review to follow... |
StraubCreative
left a comment
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.
Hi @tiku-odoo
I had a couple of content suggestions and a few small edits for the RST.
Can you please approve/improve the content suggestions and then work with @samueljlieber to get an updated version before I pass to DR?
Thank you 🙏
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
b5773fb to
af820a8
Compare
|
@StraubCreative 👍 Thanks for making the suggestions on this doc. I've successfully made the changes and pushed the doc. It is ready for you to review again. Thanks, |
StraubCreative
left a comment
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.
Hi @tiku-odoo
Nice job on the revisions.
I only caught one thing where line breaks too early. I'll fix it and send over to DR 👍
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.
line 16 breaks too early.
| Messages sent in the chatter from internal database users to external users (such as partners, | |
| customers, or vendors) are relayed on behalf of the database users. Messages | |
| sent back to the chatter from external users will appear in the chatter from their respective email | |
| addresses, or as they are listed in their Contacts record. | |
| Messages sent in the chatter from internal database users to external users (such as partners, | |
| customers, or vendors) are relayed on behalf of the database users. Messages sent back to the | |
| chatter from external users will appear in the chatter from their respective email addresses, or as | |
| they are listed in their Contacts record. |
af820a8 to
058e099
Compare
|
All good to me as of 058e099. @odoo/doc-review this doc about email domain configuration is ready for your review 🙂 |
Feyensv
left a comment
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.
@robodoo delegate+
LGTM, only one minor remark on the fly ;)
content/applications/general/email_communication/email_domain.rst
Outdated
Show resolved
Hide resolved
058e099 to
266e4fb
Compare
|
266e4fb removed the comma in the title on line 152: #2694 (review) |
|
@robodoo r+ |
|
I'm sorry, @samueljlieber. I'm afraid I can't do that. |
closes #2694 Signed-off-by: Victor Feyens (vfe) <vfe@odoo.com>
Cherry-pick of #2055