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

Extract Alert State, AlertManagerState #2163

Merged
merged 24 commits into from
Jun 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
a38be90
Extract Alert State, AlertManagerState
askvortsov1 May 12, 2020
24eb104
Make the backend return the proper error code for incorrect password …
askvortsov1 May 16, 2020
a150ea4
Modify alert state extraction so that active alerts are stored in an …
askvortsov1 May 16, 2020
75f15c0
Add missing error.alertKey store
askvortsov1 May 16, 2020
1d541b6
Use random integer for alert key instead of date
askvortsov1 May 16, 2020
88bf1b2
Allow passing an alert class in alert.show
askvortsov1 May 16, 2020
ed7f382
Change alertProps to alertAttrs
askvortsov1 May 16, 2020
c6866a9
Revert change of exception thrown when incorrect password on email ch…
askvortsov1 May 17, 2020
4e34797
Add in missing formattedError parameter
askvortsov1 May 18, 2020
887803e
Require an alert state instance to show an alert, put key back into a…
askvortsov1 May 23, 2020
2c915e7
prettify
askvortsov1 May 23, 2020
22adc26
Show alerts by passing an AlertState
askvortsov1 May 23, 2020
35afd50
Prettify?
askvortsov1 May 23, 2020
7b01596
Encapsulate alertstate, alertmanagerstate
askvortsov1 May 28, 2020
5f394d7
Take out unnecessary revert
askvortsov1 Jun 19, 2020
bcaae0b
Use alert state as internal API only
askvortsov1 Jun 19, 2020
ac48153
Make AlertState obsolete
franzliedke Jun 26, 2020
03d1bf6
Clean up variable names, doc blocks and imports
franzliedke Jun 26, 2020
cc0b228
Fix alert attrs for modal errors
franzliedke Jun 26, 2020
5e78478
Rename alert "type" to "componentClass"
askvortsov1 Jun 30, 2020
a61f8a8
Add safety check for improper stuff being passed to alerts
askvortsov1 Jun 30, 2020
7638478
Apply alert BC compliance warning to first argument
askvortsov1 Jun 30, 2020
fb9676c
Prettify
askvortsov1 Jun 30, 2020
8653bf5
Reverse order of arguments in alert POJO, as order doesn't matter, an…
askvortsov1 Jun 30, 2020
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
6 changes: 4 additions & 2 deletions js/src/admin/components/BasicsPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import Page from '../../common/components/Page';
import FieldSet from '../../common/components/FieldSet';
import Select from '../../common/components/Select';
import Button from '../../common/components/Button';
import Alert from '../../common/components/Alert';
import saveSettings from '../utils/saveSettings';
import ItemList from '../../common/utils/ItemList';
import Switch from '../../common/components/Switch';
Expand Down Expand Up @@ -186,7 +185,10 @@ export default class BasicsPage extends Page {

saveSettings(settings)
.then(() => {
app.alerts.show((this.successAlert = new Alert({ type: 'success', children: app.translator.trans('core.admin.basics.saved_message') })));
this.successAlert = app.alerts.show({
type: 'success',
children: app.translator.trans('core.admin.basics.saved_message'),
});
})
.catch(() => {})
.then(() => {
Expand Down
12 changes: 8 additions & 4 deletions js/src/admin/components/MailPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,10 @@ export default class MailPage extends Page {
})
.then((response) => {
this.sendingTest = false;
app.alerts.show(
(this.testEmailSuccessAlert = new Alert({ type: 'success', children: app.translator.trans('core.admin.email.send_test_mail_success') }))
);
this.testEmailSuccessAlert = app.alerts.show({
type: 'success',
children: app.translator.trans('core.admin.email.send_test_mail_success'),
});
})
.catch((error) => {
this.sendingTest = false;
Expand All @@ -204,7 +205,10 @@ export default class MailPage extends Page {

saveSettings(settings)
.then(() => {
app.alerts.show((this.successAlert = new Alert({ type: 'success', children: app.translator.trans('core.admin.basics.saved_message') })));
this.successAlert = app.alerts.show({
type: 'success',
children: app.translator.trans('core.admin.basics.saved_message'),
});
})
.catch(() => {})
.then(() => {
Expand Down
29 changes: 16 additions & 13 deletions js/src/common/Application.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import ItemList from './utils/ItemList';
import Alert from './components/Alert';
import Button from './components/Button';
import ModalManager from './components/ModalManager';
import AlertManager from './components/AlertManager';
Expand All @@ -23,6 +22,7 @@ import Group from './models/Group';
import Notification from './models/Notification';
import { flattenDeep } from 'lodash-es';
import PageState from './states/PageState';
import AlertManagerState from './states/AlertManagerState';

/**
* The `App` class provides a container for an application, as well as various
Expand Down Expand Up @@ -109,13 +109,13 @@ export default class Application {
booted = false;

/**
* An Alert that was shown as a result of an AJAX request error. If present,
* it will be dismissed on the next successful request.
* The key for an Alert that was shown as a result of an AJAX request error.
* If present, it will be dismissed on the next successful request.
*
* @type {null|Alert}
* @type {int}
* @private
*/
requestError = null;
requestErrorAlert = null;

/**
* The page the app is currently on.
Expand All @@ -139,6 +139,11 @@ export default class Application {
*/
previous = new PageState(null);

/*
* An object that manages the state of active alerts.
*/
alerts = new AlertManagerState();

data;

title = '';
Expand Down Expand Up @@ -175,7 +180,7 @@ export default class Application {

mount(basePath = '') {
this.modal = m.mount(document.getElementById('modal'), <ModalManager />);
this.alerts = m.mount(document.getElementById('alerts'), <AlertManager />);
m.mount(document.getElementById('alerts'), <AlertManager state={this.alerts} />);

this.drawer = new Drawer();

Expand Down Expand Up @@ -310,7 +315,7 @@ export default class Application {
}
};

if (this.requestError) this.alerts.dismiss(this.requestError.alert);
if (this.requestErrorAlert) this.alerts.dismiss(this.requestErrorAlert);

// Now make the request. If it's a failure, inspect the error that was
// returned and show an alert containing its contents.
Expand All @@ -319,8 +324,6 @@ export default class Application {
m.request(options).then(
(response) => deferred.resolve(response),
(error) => {
this.requestError = error;

let children;

switch (error.status) {
Expand Down Expand Up @@ -354,15 +357,15 @@ export default class Application {
// the details property is decoded to transform escaped characters such as '\n'
const formattedError = error.response && Array.isArray(error.response.errors) && error.response.errors.map((e) => decodeURI(e.detail));

error.alert = new Alert({
error.alert = {
type: 'error',
children,
controls: isDebug && [
<Button className="Button Button--link" onclick={this.showDebug.bind(this, error, formattedError)}>
Debug
</Button>,
],
});
};

try {
options.errorHandler(error);
Expand All @@ -378,7 +381,7 @@ export default class Application {
console.groupEnd();
}

this.alerts.show(error.alert);
this.requestErrorAlert = this.alerts.show(error.alert);
}

deferred.reject(error);
Expand All @@ -394,7 +397,7 @@ export default class Application {
* @private
*/
showDebug(error, formattedError) {
this.alerts.dismiss(this.requestError.alert);
this.alerts.dismiss(this.requestErrorAlert);

this.modal.show(new RequestErrorModal({ error, formattedError }));
}
Expand Down
56 changes: 5 additions & 51 deletions js/src/common/components/AlertManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,16 @@ import Alert from './Alert';
*/
export default class AlertManager extends Component {
init() {
/**
* An array of Alert components which are currently showing.
*
* @type {Alert[]}
* @protected
*/
this.components = [];
this.state = this.props.state;
}

view() {
return (
<div className="AlertManager">
{this.components.map((component) => (
<div className="AlertManager-alert">{component}</div>
{Object.entries(this.state.getActiveAlerts()).map(([key, alert]) => (
<div className="AlertManager-alert">
{(alert.componentClass || Alert).component({ ...alert.attrs, ondismiss: this.state.dismiss.bind(this.state, key) })}
</div>
))}
</div>
);
Expand All @@ -32,46 +28,4 @@ export default class AlertManager extends Component {
// to be retained across route changes.
context.retain = true;
}

/**
* Show an Alert in the alerts area.
*
* @param {Alert} component
* @public
*/
show(component) {
if (!(component instanceof Alert)) {
throw new Error('The AlertManager component can only show Alert components');
}

component.props.ondismiss = this.dismiss.bind(this, component);

this.components.push(component);
m.redraw();
}

/**
* Dismiss an alert.
*
* @param {Alert} component
* @public
*/
dismiss(component) {
const index = this.components.indexOf(component);

if (index !== -1) {
this.components.splice(index, 1);
m.redraw();
}
}

/**
* Clear all alerts.
*
* @public
*/
clear() {
this.components = [];
m.redraw();
}
}
14 changes: 7 additions & 7 deletions js/src/common/components/Modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,16 @@ import Button from './Button';
export default class Modal extends Component {
init() {
/**
* An alert component to show below the header.
* Attributes for an alert component to show below the header.
*
* @type {Alert}
* @type {object}
*/
this.alert = null;
this.alertAttrs = null;
}

view() {
if (this.alert) {
this.alert.props.dismissible = false;
if (this.alertAttrs) {
this.alertAttrs.dismissible = false;
}

return (
Expand All @@ -43,7 +43,7 @@ export default class Modal extends Component {
<h3 className="App-titleControl App-titleControl--text">{this.title()}</h3>
</div>

{alert ? <div className="Modal-alert">{this.alert}</div> : ''}
{this.alertAttrs ? <div className="Modal-alert">{Alert.component(this.alertAttrs)}</div> : ''}

{this.content()}
</form>
Expand Down Expand Up @@ -123,7 +123,7 @@ export default class Modal extends Component {
* @param {RequestError} error
*/
onerror(error) {
this.alert = error.alert;
this.alertAttrs = error.alert;

m.redraw();

Expand Down
50 changes: 50 additions & 0 deletions js/src/common/states/AlertManagerState.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import Alert from '../components/Alert';

export default class AlertManagerState {
constructor() {
this.activeAlerts = {};
this.alertId = 0;
}

getActiveAlerts() {
return this.activeAlerts;
}

/**
* Show an Alert in the alerts area.
*/
show(attrs, componentClass = Alert) {
// Breaking Change Compliance Warning, Remove in Beta 15.
// This is applied to the first argument (attrs) because previously, the alert was passed as the first argument.
if (attrs === Alert || attrs instanceof Alert) {
// This is duplicated so that if the error is caught, an error message still shows up in the debug console.
console.error('The AlertManager can only show Alerts. Whichever extension triggered this alert should be updated to comply with beta 14.');
throw new Error('The AlertManager can only show Alerts. Whichever extension triggered this alert should be updated to comply with beta 14.');
}
// End Change Compliance Warning, Remove in Beta 15
this.activeAlerts[++this.alertId] = { attrs, componentClass };
m.redraw();

return this.alertId;
}

/**
* Dismiss an alert.
*/
dismiss(key) {
if (!key || !(key in this.activeAlerts)) return;

delete this.activeAlerts[key];
m.redraw();
}

/**
* Clear all alerts.
*
* @public
*/
clear() {
this.activeAlerts = {};
m.redraw();
}
}
2 changes: 1 addition & 1 deletion js/src/forum/components/ChangeEmailModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ export default class ChangeEmailModal extends Modal {

onerror(error) {
if (error.status === 401) {
error.alert.props.children = app.translator.trans('core.forum.change_email.incorrect_password_message');
error.alert.children = app.translator.trans('core.forum.change_email.incorrect_password_message');
}

super.onerror(error);
Expand Down
13 changes: 5 additions & 8 deletions js/src/forum/components/EditPostComposer.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import ComposerBody from './ComposerBody';
import Alert from '../../common/components/Alert';
import Button from '../../common/components/Button';
import icon from '../../common/helpers/icon';

Expand Down Expand Up @@ -101,13 +100,11 @@ export default class EditPostComposer extends ComposerBody {
app.alerts.dismiss(alert);
},
});
app.alerts.show(
(alert = new Alert({
type: 'success',
children: app.translator.trans('core.forum.composer_edit.edited_message'),
controls: [viewButton],
}))
);
alert = app.alerts.show({
type: 'success',
children: app.translator.trans('core.forum.composer_edit.edited_message'),
controls: [viewButton],
});
}

app.composer.hide();
Expand Down
2 changes: 1 addition & 1 deletion js/src/forum/components/ForgotPasswordModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export default class ForgotPasswordModal extends Modal {

onerror(error) {
if (error.status === 404) {
error.alert.props.children = app.translator.trans('core.forum.forgot_password.not_found_message');
error.alert.children = app.translator.trans('core.forum.forgot_password.not_found_message');
}

super.onerror(error);
Expand Down
2 changes: 1 addition & 1 deletion js/src/forum/components/LogInModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ export default class LogInModal extends Modal {

onerror(error) {
if (error.status === 401) {
error.alert.props.children = app.translator.trans('core.forum.log_in.invalid_login_message');
error.alert.children = app.translator.trans('core.forum.log_in.invalid_login_message');
}

super.onerror(error);
Expand Down
13 changes: 5 additions & 8 deletions js/src/forum/components/ReplyComposer.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import ComposerBody from './ComposerBody';
import Alert from '../../common/components/Alert';
import Button from '../../common/components/Button';
import icon from '../../common/helpers/icon';
import extractText from '../../common/utils/extractText';
Expand Down Expand Up @@ -104,13 +103,11 @@ export default class ReplyComposer extends ComposerBody {
app.alerts.dismiss(alert);
},
});
app.alerts.show(
(alert = new Alert({
type: 'success',
children: app.translator.trans('core.forum.composer_reply.posted_message'),
controls: [viewButton],
}))
);
alert = app.alerts.show({
type: 'success',
children: app.translator.trans('core.forum.composer_reply.posted_message'),
controls: [viewButton],
});
}

app.composer.hide();
Expand Down
Loading