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

Fix Exception in delivering result of invoking 'email/verifySettings' #2512

Merged
merged 8 commits into from
Jul 7, 2017
67 changes: 52 additions & 15 deletions imports/plugins/core/email/client/containers/emailConfig.js
Original file line number Diff line number Diff line change
@@ -1,38 +1,75 @@
import React, { Component } from "react";
import { useDeps } from "react-simple-di";
import getServiceConfig from "nodemailer-wellknown";
import { Meteor } from "meteor/meteor";
import PropTypes from "prop-types";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Import order

import { Reaction } from "/client/api";
import { Loading } from "/imports/plugins/core/ui/client/components";
import actions from "../actions";
import EmailConfig from "../components/emailConfig";
import { composeWithTracker, merge } from "/lib/api/compose";

const composer = ({}, onData) => {
if (Meteor.subscribe("Packages").ready()) {
const shopSettings = Reaction.getShopSettings();
const settings = shopSettings.mail || {};
class EmailConfigContainer extends Component {

if (settings.service && settings.service !== "custom") {
const config = getServiceConfig(settings.service);
constructor(props) {
super(props);

// show localhost for test providers like Maildev that have no host
settings.host = config.host || "localhost";
settings.port = config.port;
}
this.state = {
status: null,
error: null
};
}

componentWillMount() {
const { settings } = this.props;
const { service, host, port, user, password } = settings;

// if all settings exist, check if they work
if (service && host && port && user && password) {
Meteor.call("email/verifySettings", (error) => {
if (error) {
return onData(null, { settings, status: "error", error: error.reason });
this.setState({ status: "error" });
Copy link
Collaborator

Choose a reason for hiding this comment

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

So we aren't using the data from error any more?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zenweasel it wasn't being used before.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, it should be but that's not what this ticket is about so that's why I already approved it.

}
return onData(null, { settings, status: "valid", error: null });
this.setState({ status: "valid" });
});
} else {
onData(null, { settings, status: "error", error: null });
this.setState({ status: "error" });
}
}

render() {
const { status } = this.state;
return (
<EmailConfig {...this.props} status={status} />
);
}
}

EmailConfigContainer.propTypes = {
settings: PropTypes.shape({
host: PropTypes.string,
password: PropTypes.string,
port: PropTypes.oneOfType([
PropTypes.number,
PropTypes.string
]),
service: PropTypes.string,
user: PropTypes.string
})
};

const composer = ({}, onData) => {
if (Meteor.subscribe("Packages").ready()) {
const shopSettings = Reaction.getShopSettings();
const settings = shopSettings.mail || {};

if (settings.service && settings.service !== "custom") {
const config = getServiceConfig(settings.service);

// show localhost for test providers like Maildev that have no host
settings.host = config.host || "localhost";
settings.port = config.port;
}
return onData(null, { settings });
}
};

Expand All @@ -43,4 +80,4 @@ const depsMapper = () => ({
export default merge(
composeWithTracker(composer, Loading),
useDeps(depsMapper)
)(EmailConfig);
)(EmailConfigContainer);