Skip to content

Commit

Permalink
feat(client): show errors and warnings in log
Browse files Browse the repository at this point in the history
* handle import errors and warnings
* show errors and warnings in log

Closes #936
  • Loading branch information
philippfromme committed Oct 12, 2018
1 parent 0874fdb commit b2b9614
Show file tree
Hide file tree
Showing 19 changed files with 9,138 additions and 13,853 deletions.
1 change: 1 addition & 0 deletions client/karma.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ module.exports = function(karma) {
],
alias: {
'bpmn-js/lib/Modeler': 'test/mocks/bpmn-js/Modeler',
'cmmn-js/lib/Modeler': 'test/mocks/cmmn-js/Modeler',
'dmn-js/lib/Modeler': 'test/mocks/dmn-js/Modeler'
}
}
Expand Down
74 changes: 71 additions & 3 deletions client/src/app/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,16 @@ export class App extends Component {
this.handleError(error, tab);
}

/**
* Handle tab warning.
*
* @param {Object} tab descriptor
*
* @return {Function} tab warning callback
*/
handleTabWarning = (tab) => (warning) => {
this.handleWarning(warning, tab);
}

/**
* Handle tab changed.
Expand Down Expand Up @@ -542,16 +552,58 @@ export class App extends Component {
}

handleError(error, ...args) {

const {
onError
} = this.props;

const {
message
} = error;

if (typeof onError === 'function') {
onError(error, ...args);
}

// TODO: show error in log
this.logEntry(message, 'error');
}

handleWarning(warning, ...args) {
const {
onWarning
} = this.props;

const {
message
} = warning;

if (typeof onWarning === 'function') {
onWarning(warning, ...args);
}

this.logEntry(message, 'warning');
}

/**
*
* @param {String} message - Message to be logged.
* @param {String} category - Category of message.
*/
logEntry(message, category) {
const {
logEntries
} = this.state;

this.toggleLog(true);

this.setState({
logEntries: [
...logEntries,
{
category,
message
}
]
});
}

setLayout(layout) {
Expand Down Expand Up @@ -688,7 +740,22 @@ export class App extends Component {
}

askExportAs = (file, filters) => {
return this.props.globals.dialog.askExportAs(file, filters);
const {
globals
} = this.props;

return globals.dialog.askExportAs(file, filters);
}

/**
* Show a generic dialog.
*/
showDialog(options = {}) {
const {
globals
} = this.props;

return globals.dialog.show(options);
}

triggerAction = (action, options) => {
Expand Down Expand Up @@ -915,6 +982,7 @@ export class App extends Component {
layout={ layout }
onChanged={ this.handleTabChanged(activeTab) }
onError={ this.handleTabError(activeTab) }
onWarning={ this.handleTabWarning(activeTab) }
onShown={ this.handleTabShown(activeTab) }
onLayoutChanged={ this.handleLayoutChanged }
onContextMenu={ this.openTabMenu }
Expand Down
9 changes: 9 additions & 0 deletions client/src/app/AppParent.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,14 @@ export default class AppParent extends Component {
return log('app error', error);
}

handleWarning = (warning, tab) => {
if (tab) {
return log('tab warning', warning, tab);
}

return log('app warning', warning);
}

handleReady = async () => {

try {
Expand Down Expand Up @@ -199,6 +207,7 @@ export default class AppParent extends Component {
onWorkspaceChanged={ this.handleWorkspaceChanged }
onReady={ this.handleReady }
onError={ this.handleError }
onWarning={ this.handleWarning }
/>
);
}
Expand Down
99 changes: 99 additions & 0 deletions client/src/app/__tests__/AppSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -700,6 +700,28 @@ describe('<App>', function() {
});


it('should show in log', async function() {

// given
const {
app
} = createApp(mount);

await app.createDiagram();

const tabInstance = app.tabRef.current;

const error = new Error('YZO!');

// when
tabInstance.triggerAction('error', error);

// then
expect(app.state.layout.log.open).to.be.true;
expect(app.state.logEntries).to.have.length(1);
});


describe('should catch', function() {

const errorHandler = window.onerror;
Expand Down Expand Up @@ -750,6 +772,59 @@ describe('<App>', function() {
});


describe('tab warnings', function() {

it('should propagate', async function() {

// given
const warningSpy = spy();

const {
app
} = createApp({ onWarning: warningSpy }, mount);

const tab = await app.createDiagram();

const tabInstance = app.tabRef.current;

// when
const warning = {
message: 'warning'
};

tabInstance.triggerAction('warning', warning);

// then
expect(warningSpy).to.have.been.calledWith(warning, tab);
});


it('should show in log', async function() {

// given
const {
app
} = createApp(mount);

await app.createDiagram();

const tabInstance = app.tabRef.current;

// when
const warning = {
message: 'warning'
};

tabInstance.triggerAction('warning', warning);

// then
expect(app.state.layout.log.open).to.be.true;
expect(app.state.logEntries).to.have.length(1);
});

});


describe('workspace integration', function() {

describe('should notify #onWorkspaceChanged', function() {
Expand Down Expand Up @@ -859,6 +934,28 @@ describe('<App>', function() {
expect(log.props().expanded).to.be.true;
});


it('#logEntry', function() {

// given
const { tree, app } = createApp();

app.setLayout({ log: { open: false } });

// when
app.logEntry('foo', 'bar');

// then
const log = tree.find(Log).first();

expect(app.state.logEntries).to.eql([{
message: 'foo',
category: 'bar'
}]);

expect(log.props().expanded).to.be.true;
});

});


Expand Down Expand Up @@ -945,6 +1042,7 @@ function createApp(options = {}, mountFn=shallow) {

const onReady = options.onReady;
const onError = options.onError;
const onWarning = options.onWarning;

const tree = mountFn(
<App
Expand All @@ -953,6 +1051,7 @@ function createApp(options = {}, mountFn=shallow) {
tabsProvider={ tabsProvider }
onReady={ onReady }
onError={ onError }
onWarning={ onWarning }
onTabChanged={ onTabChanged }
onTabShown={ onTabShown }
onWorkspaceChanged={ onWorkspaceChanged }
Expand Down
4 changes: 4 additions & 0 deletions client/src/app/__tests__/mocks/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ class FakeTab extends Component {
this.props.onError(options);
}

if (action === 'warning') {
this.props.onWarning(options);
}

if (action === 'errorThrow') {
this.setState({
error: options
Expand Down
26 changes: 25 additions & 1 deletion client/src/app/tabs/MultiSheetTab.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
import css from './MultiSheetTab.less';


class MultiSheetTab extends CachedComponent {
export class MultiSheetTab extends CachedComponent {

constructor(props) {
super(props);
Expand Down Expand Up @@ -90,6 +90,29 @@ class MultiSheetTab extends CachedComponent {
onError(error);
}

handleWarning = (warning) => {
const {
onWarning
} = this.props;

onWarning(warning);
}

handleImport = (error, warnings) => {

if (error) {
// TODO: open fallback

return this.handleError(error);
}

if (warnings && warnings.length) {
warnings.forEach(warning => {
this.handleWarning(warning);
});
}
}

handleContextMenu = (event, context) => {

const {
Expand Down Expand Up @@ -238,6 +261,7 @@ class MultiSheetTab extends CachedComponent {
onContextMenu={ this.handleContextMenu }
onChanged={ this.handleChanged }
onError={ this.handleError }
onImport={ this.handleImport }
onLayoutChanged={ this.handleLayoutChanged } />
</TabContainer>

Expand Down
Loading

0 comments on commit b2b9614

Please sign in to comment.