Skip to content

chore: Add Mailgun send email action #1210

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Mar 31, 2021
Merged

chore: Add Mailgun send email action #1210

merged 8 commits into from
Mar 31, 2021

Conversation

@lahirumaramba
Copy link
Member

@lahirumaramba lahirumaramba commented Mar 29, 2021 •
edited
Loading

  • Add GitHub Action to send email using mailgun.js npm module.
  • Update nightly workflow to use the send email action.
  • Add a failing integration test to trigger the email notification workflow (this test will be removed once we confirm the email notifications be working as expected).

@lahirumaramba lahirumaramba added the release:stage Stage a release candidate label Mar 30, 2021
@lahirumaramba
Copy link
Member Author

lahirumaramba commented Mar 30, 2021 •
edited
Loading

Note: release:stage is added to trigger the integration tests. Integration tests should fail with the added test in this PR. This new test will trigger the send email workflow once merged to master.

lahirumaramba
lahirumaramba commented Mar 30, 2021
View reviewed changes
.github/actions/send-email/index.js Outdated
});

let from = core.getInput('from');
const cc = core.getInput('cc');
Copy link
Member Author

@lahirumaramba lahirumaramba Mar 30, 2021

Choose a reason for hiding this comment

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

We probably do not need cc filed. I am open to remove it and keep this action as simple as possible if we need.

hiranya911
hiranya911 suggested changes Mar 30, 2021
View reviewed changes
Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

Looks pretty good. A few thoughts for improvement.

@lahirumaramba
Copy link
Member Author

lahirumaramba commented Mar 31, 2021

Thanks, @hiranya911!
Used a bit of a lazy approach to validate all the required fields. Kept cc, text, html, as optional. If html is provided and not text the API extracts the text content from the html. Let me know what you think.

hiranya911
hiranya911 suggested changes Mar 31, 2021
View reviewed changes
Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

Just a couple of readability nits. And remember to update the binary every time you push a code change.

.github/actions/send-email/index.js Outdated

function validate(config) {
for (param in config) {
if (['cc', 'text', 'html'].includes(param)) {
Copy link
Contributor

@hiranya911 hiranya911 Mar 31, 2021

Choose a reason for hiding this comment

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

const optionalFields = ['cc', 'text', 'html'];

.github/actions/send-email/index.js Outdated
}
}

function validateParameter(value, name) {
Copy link
Contributor

@hiranya911 hiranya911 Mar 31, 2021

Choose a reason for hiding this comment

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

validateRequiredParameter

@lahirumaramba
Copy link
Member Author

lahirumaramba commented Mar 31, 2021

Thanks @hiranya911! Updated the binary.

This was referenced Jun 20, 2024
Open
Open
Open
Open
@davidhin davidhin mentioned this pull request Jul 6, 2024
Open
This was referenced Jul 17, 2024
Open
Open
This was referenced Jul 27, 2024
Open
Open
Open
Open
This was referenced Aug 10, 2024
Open
Open
This was referenced Aug 17, 2024
Open
Open
Open
Open
Open
This was referenced Aug 28, 2024
Open
Open
This was referenced Sep 5, 2024
Open
Open
This was referenced Sep 24, 2024
Open
Open
Open
Open
Open
This was referenced Oct 26, 2024
Open
Open
@davidhin davidhin mentioned this pull request Nov 9, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@hiranya911 hiranya911 hiranya911 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@lahirumaramba lahirumaramba

Labels

release:stage Stage a release candidate

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@lahirumaramba @hiranya911