-
Notifications
You must be signed in to change notification settings - Fork 40
Add emails REST API doc. #211
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
base: 5.x
Are you sure you want to change the base?
Conversation
|
I have tried to the best of my knowledge to convert the Markdown to rst. I will add a commits to this PR, which will just have the updates based on mautic/mautic#13775 |
RCheesley
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 @abhisekmazumdar thanks so much for bringing this content over to the developer docs, huge appreciation!
I've made a few comments but there's some things you should tweak for all occurrences and I didn't want to flood the PR with all the recommended changes, so here they are:
When referring to an Email in Mautic, use the capital letter. Only lower case if you're mentioning the act of sending an email or referring to email address, for example.
Please enclose any code snippets or references in either inline code (double backticks) or a code block - in the table each subject should be annotated like this (I've given some examples).
In some cases the content seems to be breaking onto two lines, I don't know how that will render but it's worth looking at whether that has any impact (I haven't built the docs locally to check at this point).
All headings need to be unique, because they generate their URLs and cross-linking snippets from the title. If we can't put that in the actual header - I noticed you're using custom anchors in some places - let's make those anchors relevant to the thing we're describing (e.g. _http-request-get-emails rather than _http-request-1). It makes it much easier for someone reading through the code to know what it's referring to.
It's also helpful to use contractions - don't rather than do not - Vale will pick up more recommendations as you type. If you need help getting set up with Vale in your IDE so that it can hint as you type, let us know - this repo should initialise it for VSCode users with all the plugins, I think.
Thanks again for bringing this over, we desperately want to finish this content migration so we can finally close down developer.mautic.org so we really appreciate your help with this!
docs/rest_api/emails.rst
Outdated
| +--------------+--------------+----------------------------------------+ | ||
| | isPublished | bool | Published state | | ||
| +--------------+--------------+----------------------------------------+ | ||
| | publishUp | d | Date/time when the email should be | |
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.
| | publishUp | d | Date/time when the email should be | | |
| | ``publishUp`` | d | Date/time when the email should be | |
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.
Please also check the wording here - we changed all the states from Published to Active or Available, so we might also need to update it here.
docs/rest_api/emails.rst
Outdated
| | publishUp | d | Date/time when the email should be | | ||
| | | atetime/null | published | | ||
| +--------------+--------------+----------------------------------------+ | ||
| | publishDown | d | Date/time the email should be un | |
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.
| | publishDown | d | Date/time the email should be un | | |
| | ``publishDown`` | d | Date/time the email should be un | |
|
I tried to make the suggested changes and also added the |
|
@escopecz can you please help review the content here? TIA! ✨ |
escopecz
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.
Looks fine 👍
PR covers the following: