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.

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

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

Merged
merged 8 commits into from
Jul 7, 2017
Merged

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

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6cae012
Only verify email settings when saving settings
Jun 30, 2017
936fca4
Move method call to child component's WillMount method
Jul 3, 2017
360cac9
Refactor. Move server method call to container component
Jul 3, 2017
a7238ae
Merge branch 'development' into awa-fix-2464-email/verifySettings
brent-hoover Jul 5, 2017
05b10a1
Merge branch 'development' into awa-fix-2464-email/verifySettings
jshimko Jul 5, 2017
c23aed8
Merge branch 'development' into awa-fix-2464-email/verifySettings
brent-hoover Jul 6, 2017
32a6e98
Fix import order
brent-hoover Jul 7, 2017
0f9b450
Merge branch 'development' into awa-fix-2464-email/verifySettings
brent-hoover Jul 7, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions imports/plugins/core/email/client/components/emailConfig.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React, { Component } from "react";
import PropTypes from "prop-types";
import { Meteor } from "meteor/meteor";
import { Card, CardHeader, CardBody, CardGroup, Icon, Translation } from "/imports/plugins/core/ui/client/components";
import EmailSettings from "../containers/emailSettings";

Expand All @@ -9,6 +10,8 @@ class EmailConfig extends Component {
super(props);

this.state = {
status: null,
error: null,
showPassword: false,
showSettings: false
};
Expand All @@ -17,6 +20,21 @@ class EmailConfig extends Component {
this.toggleSettings = this.toggleSettings.bind(this);
}

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) {
this.setState({ status: "error", error: error.reason });
}
this.setState({ status: "valid", error: null });
});
} else {
this.setState({ status: "error", error: null });
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This kind of logic should not be in a UI component. Data retrieval and/or interaction with the server should always be done in the container component and then results passed into the UI component. The UI component should only have markup and interactivity handling (onClick, etc).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay. Will change it. Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jshimko I have moved the method call to the container component.


togglePassword() {
this.setState({
Expand All @@ -31,9 +49,9 @@ class EmailConfig extends Component {
}

renderSettingsDisplay() {
const { settings, status } = this.props;
const { settings } = this.props;
const { service, host, port, user, password } = settings;
const { showPassword } = this.state;
const { showPassword, status } = this.state;

const NotSet = () => <span data-i18n="admin.settings.fieldNotSet">Not set</span>;

Expand Down
14 changes: 1 addition & 13 deletions imports/plugins/core/email/client/containers/emailConfig.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,7 @@ const composer = ({}, onData) => {
settings.port = config.port;
}

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 });
}
return onData(null, { settings, status: "valid", error: null });
});
} else {
onData(null, { settings, status: "error", error: null });
}
return onData(null, { settings, status: "error", error: null });
}
};

Expand Down