Skip to content

Conversation

@jqu-odoo
Copy link
Contributor

Customer may want to use their own SMTP for email marketing, and Odoo default SMTP for everything else. There is no good way to do this. Previously we suggested adding Odoo SMTP as an outgoing email server but we don't want to share credentials anymore.
R&D is working on a solution (e.g. showing the Odoo default email server in the list rather than keeping it as an "hidden feature") but for the time being, if you use an outgoing email server, you give up using the Odoo default SMTP.

@jqu-odoo jqu-odoo requested a review from Abridbus May 24, 2022 06:54
@robodoo
Copy link
Collaborator

robodoo commented May 24, 2022

@C3POdoo C3POdoo requested a review from a team May 24, 2022 06:55
@Abridbus
Copy link
Contributor

LGTM
Thanks!

Probably wording might be worth checking, I'm leaving this to @tiku-odoo

Copy link
Contributor

@tiku-odoo tiku-odoo left a comment

Choose a reason for hiding this comment

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

Hi Jonathan, just a few wording changes to eliminate the 2nd person references. Thanks for drafting this change :)

Tim

e-mails and mass mailings.
Example: use Odoo's own mail server for transactional e-mails, and Sendgrid, Amazon SES, or Mailgun
for mass mailings. Another alternative is to use Postmark for transactional e-mails, and Amazon SES
You can use a separate Mail Transfer Agent (MTA) servers for transactional e-mails and mass mailings.
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi Jonathan, thanks for submitting this change. I am just going to change some of the wording, everything else looks great.

In Odoo a separate Mail Transfer Agent (MTA) server can be used for transactional e-mails and mass mailings.

Copy link
Contributor

@tiku-odoo tiku-odoo left a comment

Choose a reason for hiding this comment

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

A couple more changes... can you update the RST and we can move this change along? :) Thank you for your help!

@jqu-odoo jqu-odoo requested a review from tiku-odoo May 25, 2022 07:46
Copy link
Contributor

@tiku-odoo tiku-odoo left a comment

Choose a reason for hiding this comment

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

Looks great, thank you for making those changes!

@tiku-odoo
Copy link
Contributor

@mivu-odoo This PR is ready for your review. It has been reviewed by the technical email expert and I have done a content review as well. Thanks, Tim

**This change is for 13/14/15

Copy link
Contributor

@mivu-odoo mivu-odoo left a 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 the hard work on this PR! I found one edit that needs to be made in the lines changed. I do want to note that a separate PR might need to be opened to correct some instances of second-person pronoun use and capitalizations in headers. Just something to keep in mind, but otherwise, once my suggested edit is addressed, I can approve this PR.

Please let me know when you're ready. Thank you! 😸

Copy link
Contributor

@mivu-odoo mivu-odoo left a comment

Choose a reason for hiding this comment

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

Sorry @samueljlieber caught some more corrections that I missed in my first pass!

@samueljlieber samueljlieber force-pushed the jqu-odoo-doc-2 branch 2 times, most recently from faa9a61 to 39f12ee Compare August 31, 2022 13:35
@samueljlieber
Copy link
Contributor

Hi @mivu-odoo! I updated the RST throughout the document and included your corrections. Let me know when you are ready for me to correct the instances of second-person pronoun use :)

Copy link
Contributor

@tiku-odoo tiku-odoo left a comment

Choose a reason for hiding this comment

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

Adding this small edit in, per documentation ticket: https://www.odoo.com/web#id=2828976&cids=3&menu_id=4720&action=333&active_id=3835&model=project.task&view_type=form

I've added Odoo.sh to line 63, customers were confused that it wasn't listed with Odoo Online

Thanks, Tim

Copy link
Contributor

@mivu-odoo mivu-odoo left a 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!

I just finished my second pass; thank you for the speedy turnaround on this!

Let me know if you have any questions about my feedback. We're almost there! Once you're ready, ping me again for a (hopefully) final content check. Thank you 😸

@samueljlieber
Copy link
Contributor

Hi @mivu-odoo!
Thanks for getting these suggestions together so quickly! I've updated the document per your changes, and @tiku-odoo I included your change as well!

Copy link
Contributor

@mivu-odoo mivu-odoo left a 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 hard work on this!

I caught a couple more things in this pass, sorry! When you're ready, please let me know so I can take another look and approve the PR for ZST and the /doc-review team.

@samueljlieber
Copy link
Contributor

Hi @mivu-odoo 👋 I made those changes and I'm ready for you to take a look again!

Copy link
Contributor

@mivu-odoo mivu-odoo left a comment

Choose a reason for hiding this comment

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

Hello @samueljlieber! One last change for Line 2, the title of the user doc, because headers should be in sentence case as per content guidelines.

Again, great job to both you and @tiku-odoo on this PR! Once the header is fixed, feel free to move forward and tag ZST, no need to tag me again. Thank you 😸

@samueljlieber
Copy link
Contributor

Hi @StraubCreative 🖖, this PR is ready for a final technical review!

Copy link
Contributor

@StraubCreative StraubCreative left a comment

Choose a reason for hiding this comment

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

Hi there 👋
I have a few change requests below, mostly around sentence structure and wording to help flow and clarity. There's a missing :guilabel: too. Should be quick fixes 🙂

@samueljlieber
Copy link
Contributor

Hi @StraubCreative, I made the suggested changes! 🙂

Copy link
Contributor

@StraubCreative StraubCreative left a comment

Choose a reason for hiding this comment

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

Seems good 👍
Passing to @odoo/doc-review .

Side notes regarding custom anchors (for discussion / exploring later)--

  • it seems strange why there would only be a couple specific ones on this doc instead of at every section, especially for ones that refer to platform-specific servers like Office 365 or G Suite.
  • as well, it seems like these custom anchors might not be necessary at all since a) no where on the doc are they referenced and b) the headings on the page ToC already generate custom anchors when clicked (e.g. pick a random doc that doesn't have custom anchors, like this, and click on any of the heading titles under On This Page sidebar. You'll see that the URL endpoint adapts based on the heading clicked).

@StraubCreative StraubCreative requested a review from a team September 16, 2022 19:20
@StraubCreative
Copy link
Contributor

Quick follow up @odoo/doc-review --
This PR can be forward-ported to v14 and v15 as well plz 🚀

Copy link
Collaborator

@AntoineVDV AntoineVDV left a comment

Choose a reason for hiding this comment

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

Hi, could you (or anyone) please update the commit message to follow the guidelines? Something like [IMP] email_communication: can't use Odoo STMP with another mail server would do.

it seems like these custom anchors might not be necessary at all since a) no where on the doc are they referenced and b) the headings on the page ToC already generate custom anchors when clicked

@StraubCreative It's actually a good practice to add anchors for sections that may be referenced later, so you don't have to add the anchors when you reference them from another PR. These auto-generated anchors should not be blindly trusted. You can share a link to one of those but keep in mind that updating the section's title will break the anchor, whereas the "manual/code" anchor will remain untouched.

This PR can be forward-ported to v14 and v15 as well plz

@StraubCreative Forward-port PRs are automatically created unless requested otherwise. You can even help me by replying with @fw-bot r+ on the forward-port PR targeting master; this will merge the whole chain at once (unless there is a diff conflict).

@samueljlieber
Copy link
Contributor

Hi @AntoineVDV I updated the commit message per your suggestion and updated the rst doc with your changes. Please see my comment about the image alignment 🧐 Thank you!

Customer may want to use their own SMTP for email marketing, and Odoo default SMTP for everything else. There is no good way to do this. Previously we suggested adding Odoo SMTP as an outgoing email server but we don't want to share credentials anymore.
R&D is working on a solution (e.g. showing the Odoo default email server in the list rather than keeping it as an "hidden feature") but for the time being, if you use an outgoing email server, you give up using the Odoo default SMTP.
@samueljlieber
Copy link
Contributor

@AntoineVDV should be all set now! 😅 Thank you!

Copy link
Collaborator

@AntoineVDV AntoineVDV left a comment

Choose a reason for hiding this comment

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

robodoo pushed a commit that referenced this pull request Sep 20, 2022
Customer may want to use their own SMTP for email marketing, and Odoo default SMTP for everything else. There is no good way to do this. Previously we suggested adding Odoo SMTP as an outgoing email server but we don't want to share credentials anymore.
R&D is working on a solution (e.g. showing the Odoo default email server in the list rather than keeping it as an "hidden feature") but for the time being, if you use an outgoing email server, you give up using the Odoo default SMTP.

closes #2075

Signed-off-by: Antoine Vandevenne (anv) <anv@odoo.com>
@robodoo robodoo temporarily deployed to merge September 20, 2022 15:47 Inactive
@robodoo robodoo closed this Sep 20, 2022
@fw-bot
Copy link
Collaborator

fw-bot commented Sep 24, 2022

@jqu-odoo @AntoineVDV this pull request has forward-port PRs awaiting action (not merged or closed):
#2741

4 similar comments
@fw-bot
Copy link
Collaborator

fw-bot commented Sep 25, 2022

@jqu-odoo @AntoineVDV this pull request has forward-port PRs awaiting action (not merged or closed):
#2741

@fw-bot
Copy link
Collaborator

fw-bot commented Sep 26, 2022

@jqu-odoo @AntoineVDV this pull request has forward-port PRs awaiting action (not merged or closed):
#2741

@fw-bot
Copy link
Collaborator

fw-bot commented Sep 27, 2022

@jqu-odoo @AntoineVDV this pull request has forward-port PRs awaiting action (not merged or closed):
#2741

@fw-bot
Copy link
Collaborator

fw-bot commented Sep 28, 2022

@jqu-odoo @AntoineVDV this pull request has forward-port PRs awaiting action (not merged or closed):
#2741

@fw-bot
Copy link
Collaborator

fw-bot commented Sep 29, 2022

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.

9 participants