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

[ci skip] Update Action Mailer guide on how to add images #49712

Conversation

MatheusRich
Copy link
Contributor

Motivation / Background

Fixes #49656

Unlike controllers, mailers can't infer the protocol from the request, so users need to specify it.

@rails-bot rails-bot bot added the docs label Oct 20, 2023
@MatheusRich
Copy link
Contributor Author

cc @rafaelfranca

@MatheusRich MatheusRich force-pushed the mr/docs-for-action-mailer-asset-host branch 2 times, most recently from 4b7dbea to 908c0ba Compare October 20, 2023 02:20
Copy link
Member

@akhilgkrishnan akhilgkrishnan left a comment

Choose a reason for hiding this comment

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

In this document, it mentioned not to add include the protocol in those asset host config, If it not correct, can we remove that one also?

@MatheusRich
Copy link
Contributor Author

@akhilgkrishnan I think that's correct in that case because controllers can infer the protocol from the request.

guides/source/action_mailer_basics.md Outdated Show resolved Hide resolved
```

NOTE: Because we can't infer the protocol from the request, you'll need to
specify a protocol or "scheme" such as `http://` or `https://.` in the
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
specify a protocol or "scheme" such as `http://` or `https://.` in the
specify a protocol such as `http://` or `https://.` in the

Nit -- let's be concise and just call it a protocol?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm good with that. I was just mirroring other parts of the documentation.

Copy link
Contributor

@adrianna-chang-shopify adrianna-chang-shopify left a comment

Choose a reason for hiding this comment

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

@MatheusRich can you squash your commits please? Thank you!

Unlike controllers, mailers can't infer the protocol from the request,
so users need to specify it.
@MatheusRich MatheusRich force-pushed the mr/docs-for-action-mailer-asset-host branch from 599d98b to f18e3d3 Compare October 25, 2023 13:26
@MatheusRich
Copy link
Contributor Author

@adrianna-chang-shopify done!

@adrianna-chang-shopify adrianna-chang-shopify merged commit 02fa5c2 into rails:main Oct 25, 2023
3 checks passed
adrianna-chang-shopify added a commit that referenced this pull request Oct 25, 2023
…asset-host

[ci skip] Update Action Mailer guide on how to add images
@MatheusRich MatheusRich deleted the mr/docs-for-action-mailer-asset-host branch October 25, 2023 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

action_mailer.asset_host is missing protocol/scheme (http/https)
3 participants