-
Couldn't load subscription status.
- Fork 1.2k
dev/email plugin docs update #872
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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Thanks, Shaun. I've added some comments and questions.
On top of that, I think some of the questions or concerns that were raised in the first round of reviews were not handled, but I can't see check as they were already marked as resolved. For further reviews or next PRs, could you please reply directly to comments? It will be easier to follow up.
| |-----------------|--------------------------------------------------------------------------------------------------------------------------------------------|----------|---------| | ||
| | `emailOptions` | Contains email addressing properties: `to`, `from`, `replyTo`, `cc`, and `bcc` | `object` | { } | | ||
| | `emailTemplate` | Contains email content properties: `subject`, `text`, and `html` using [Lodash string templates](https://lodash.com/docs/4.17.15#template) | `object` | { } | | ||
| | `data` | Contains the data used to compile the templates | `object` | { } | |
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.
data seems optional (I can't find it in the code example, maybe I misunderstood something 🤔) so I think you can mention it in the table (e.g. "data
(optional)"
The intro paragraph implies that emailOptions is optional as well, but is it? I'm trying to figure out how the email is sent if neither to or from parameters are defined (just an open question) 🤔
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.
Added "optional" to the table.
I think the answer is because you can also hard code the email address information. Here the source code, which as I understand, only throws an error is emailTemplate is not supplied.
const attributes = ['subject', 'text', 'html'];
const missingAttributes = _.difference(attributes, Object.keys(emailTemplate));
if (missingAttributes.length > 0) {
throw new Error(
`Following attributes are missing from your email template : ${missingAttributes.join(', ')}`
);
}```
|
|
||
| ### Using the `send()` function | ||
|
|
||
| To trigger an email in response to a user action add the following function to a [controller](/developer-docs/latest/development/backend-customization/controllers.md) or [service](/developer-docs/latest/development/backend-customization/services.md). |
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 think it would be worth adding a table here, for the parameters.
Generally, the structure of this kind of subsection is:
- little introduction to get the context and understand what the described product item does
- more details if necessary (here, parameters — we need to know which are optional, for instance, and the accepted types for each parameter)
- an example (code example in dev docs, usually)
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.
table added. and sentence added to the paragraph.
Co-authored-by: Pierre Wizla <pwizla@users.noreply.github.com>
I went back and commented on the earlier comments |
|
Closed in favor of #992, thank you, guys! |
What does it do?
Why is it needed?
Describe the issue you are solving.
Related issue(s)/PR(s)
Let us know if this is related to any issue/pull request