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

feat: decouple SMTP email sending logic from core to allow plugins to override #4740

Merged
merged 9 commits into from
Oct 25, 2018

Conversation

dancastellon
Copy link
Contributor

@dancastellon dancastellon commented Oct 15, 2018

Resolves #4730
Impact: minor
Type: feature

Issue

Currently, SMTP email sending logic is hard-coded in core. This makes it impossible to integrate with a third-party email provider that does not support SMTP, without modifying core code.

Solution

This PR decouples SMTP from core by creating a new reaction-email-smtp included plugin. Core now emits a sendEmail event for each email job, which any plugin can respond to. The included SMTP plugin responds to it, checks if an SMTP email provider is configured, and sends the email if so.

The email provider config form found at Dashboard -> Emails -> Mail Provider is now also able to be overridden. Plugins can use register.js to provide a React component to use here.

Breaking changes

None

Testing

  1. Login as operator -> Dashboard -> Email -> Mail Provider. Confirm the existing config component works as expected (shows status icon & existing settings, gear icon shows config form, you can successfully configure an SMTP provider, etc)
  2. After configuring an SMTP provider, complete an order and confirm the email was successfully delivered
  3. Confirm in database that Emails.status is set to "completed"
  4. Set SMTP settings to something invalid & save
  5. Complete another order, confirm email error in logs, and that Emails.status is set to "failed" in database
  6. Create an email-provider-test plugin in imports/plugins/custom with 3 files:
  • register.js
import Reaction from "/imports/plugins/core/core/server/Reaction";

Reaction.registerPackage({
  label: "Email Provider Test",
  name: "reaction-email-provider-test",
  icon: "fa fa-envelope-o",
  autoEnable: true,
  registry: [
    {
      provides: ["emailProviderConfig"],
      template: "TestEmailConfig"
    }
  ]
});
  • client/TestEmailConfig.js:
import React, { Component } from "react";
import { registerComponent } from "@reactioncommerce/reaction-components";

class TestEmailConfig extends Component {
  render() {
    return (
      <div>Test email provider config here</div>
    );
  }
}

registerComponent("TestEmailConfig", TestEmailConfig);
  • client/index.js:
import "./TestEmailConfig";
  1. Restart Reaction and confirm you see "Test email provider config here" at Dashboard -> Email -> Mail Provider

@dancastellon dancastellon changed the base branch from master to release-2.0.0-rc.5 October 15, 2018 19:20
@dancastellon dancastellon changed the title [WIP] feat: decouple SMTP email sending logic from core to allow plugins to override feat: decouple SMTP email sending logic from core to allow plugins to override Oct 16, 2018
@aldeed aldeed self-requested a review October 18, 2018 21:48
Copy link
Contributor

@nnnnat nnnnat left a comment

Choose a reason for hiding this comment

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

I was able to complete all the testing steps and everything worked as described. @aldeed I'm going to hold off merging until you've had a chance to do a code review.

@aldeed aldeed changed the base branch from release-2.0.0-rc.5 to release-2.0.0-rc.6 October 24, 2018 23:29
Copy link
Contributor

@aldeed aldeed left a comment

Choose a reason for hiding this comment

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

Just one small change to make, but looks good

imports/plugins/included/email-smtp/server/startup.js Outdated Show resolved Hide resolved
@aldeed aldeed merged commit 289dc70 into release-2.0.0-rc.6 Oct 25, 2018
@aldeed aldeed deleted the feat-4730-dancastellon-email-appevents branch October 25, 2018 15:14
@spencern spencern mentioned this pull request Nov 6, 2018
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.

3 participants