From f88b8ce757f62486d5843e1ea0191db1e5c61194 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 18 Oct 2016 17:09:45 +0200 Subject: [PATCH] Improve how errors are displayed in the UI --- .../components/actions/accounts.jsx | 7 -- .../components/actions/compose.jsx | 2 - .../javascripts/components/actions/follow.jsx | 1 - .../components/actions/interactions.jsx | 4 -- .../components/actions/notifications.jsx | 9 +++ .../components/actions/statuses.jsx | 2 - .../components/actions/suggestions.jsx | 1 - .../components/actions/timelines.jsx | 2 - .../components/middleware/errors.jsx | 31 ++++++++ .../components/reducers/notifications.jsx | 72 ++++--------------- .../components/store/configureStore.jsx | 3 +- 11 files changed, 54 insertions(+), 80 deletions(-) create mode 100644 app/assets/javascripts/components/middleware/errors.jsx diff --git a/app/assets/javascripts/components/actions/accounts.jsx b/app/assets/javascripts/components/actions/accounts.jsx index 1d1835b1df..4847c37e29 100644 --- a/app/assets/javascripts/components/actions/accounts.jsx +++ b/app/assets/javascripts/components/actions/accounts.jsx @@ -48,7 +48,6 @@ export function fetchAccount(id) { axios.all([boundApi.get(`/api/v1/accounts/${id}`), boundApi.get(`/api/v1/accounts/relationships?id=${id}`)]).then(values => { dispatch(fetchAccountSuccess(values[0].data, values[1].data[0])); }).catch(error => { - console.error(error); dispatch(fetchAccountFail(id, error)); }); }; @@ -61,7 +60,6 @@ export function fetchAccountTimeline(id) { api(getState).get(`/api/v1/accounts/${id}/statuses`).then(response => { dispatch(fetchAccountTimelineSuccess(id, response.data)); }).catch(error => { - console.error(error); dispatch(fetchAccountTimelineFail(id, error)); }); }; @@ -76,7 +74,6 @@ export function expandAccountTimeline(id) { api(getState).get(`/api/v1/accounts/${id}/statuses?max_id=${lastId}`).then(response => { dispatch(expandAccountTimelineSuccess(id, response.data)); }).catch(error => { - console.error(error); dispatch(expandAccountTimelineFail(id, error)); }); }; @@ -112,7 +109,6 @@ export function followAccount(id) { api(getState).post(`/api/v1/accounts/${id}/follow`).then(response => { dispatch(followAccountSuccess(response.data)); }).catch(error => { - console.error(error); dispatch(followAccountFail(error)); }); }; @@ -125,7 +121,6 @@ export function unfollowAccount(id) { api(getState).post(`/api/v1/accounts/${id}/unfollow`).then(response => { dispatch(unfollowAccountSuccess(response.data)); }).catch(error => { - console.error(error); dispatch(unfollowAccountFail(error)); }); } @@ -226,7 +221,6 @@ export function blockAccount(id) { api(getState).post(`/api/v1/accounts/${id}/block`).then(response => { dispatch(blockAccountSuccess(response.data)); }).catch(error => { - console.error(error); dispatch(blockAccountFail(id, error)); }); }; @@ -239,7 +233,6 @@ export function unblockAccount(id) { api(getState).post(`/api/v1/accounts/${id}/unblock`).then(response => { dispatch(unblockAccountSuccess(response.data)); }).catch(error => { - console.error(error); dispatch(unblockAccountFail(id, error)); }); }; diff --git a/app/assets/javascripts/components/actions/compose.jsx b/app/assets/javascripts/components/actions/compose.jsx index d436792b9f..402c59dc6e 100644 --- a/app/assets/javascripts/components/actions/compose.jsx +++ b/app/assets/javascripts/components/actions/compose.jsx @@ -43,7 +43,6 @@ export function submitCompose() { }).then(function (response) { dispatch(submitComposeSuccess(response.data)); }).catch(function (error) { - console.error(error); dispatch(submitComposeFail(error)); }); }; @@ -83,7 +82,6 @@ export function uploadCompose(files) { }).then(function (response) { dispatch(uploadComposeSuccess(response.data)); }).catch(function (error) { - console.error(error); dispatch(uploadComposeFail(error)); }); }; diff --git a/app/assets/javascripts/components/actions/follow.jsx b/app/assets/javascripts/components/actions/follow.jsx index 74981cd17d..8eb4407894 100644 --- a/app/assets/javascripts/components/actions/follow.jsx +++ b/app/assets/javascripts/components/actions/follow.jsx @@ -22,7 +22,6 @@ export function submitFollow(router) { dispatch(submitFollowSuccess(response.data)); router.push(`/accounts/${response.data.id}`); }).catch(function (error) { - console.error(error); dispatch(submitFollowFail(error)); }); }; diff --git a/app/assets/javascripts/components/actions/interactions.jsx b/app/assets/javascripts/components/actions/interactions.jsx index 8a409dd5db..ce7797eaab 100644 --- a/app/assets/javascripts/components/actions/interactions.jsx +++ b/app/assets/javascripts/components/actions/interactions.jsx @@ -25,7 +25,6 @@ export function reblog(status) { // interested in how the original is modified, hence passing it skipping the wrapper dispatch(reblogSuccess(status, response.data.reblog)); }).catch(function (error) { - console.error(error); dispatch(reblogFail(status, error)); }); }; @@ -38,7 +37,6 @@ export function unreblog(status) { api(getState).post(`/api/v1/statuses/${status.get('id')}/unreblog`).then(response => { dispatch(unreblogSuccess(status, response.data)); }).catch(error => { - console.error(error); dispatch(unreblogFail(status, error)); }); }; @@ -97,7 +95,6 @@ export function favourite(status) { api(getState).post(`/api/v1/statuses/${status.get('id')}/favourite`).then(function (response) { dispatch(favouriteSuccess(status, response.data)); }).catch(function (error) { - console.error(error); dispatch(favouriteFail(status, error)); }); }; @@ -110,7 +107,6 @@ export function unfavourite(status) { api(getState).post(`/api/v1/statuses/${status.get('id')}/unfavourite`).then(response => { dispatch(unfavouriteSuccess(status, response.data)); }).catch(error => { - console.error(error); dispatch(unfavouriteFail(status, error)); }); }; diff --git a/app/assets/javascripts/components/actions/notifications.jsx b/app/assets/javascripts/components/actions/notifications.jsx index f19a356c26..79457eba68 100644 --- a/app/assets/javascripts/components/actions/notifications.jsx +++ b/app/assets/javascripts/components/actions/notifications.jsx @@ -1,3 +1,4 @@ +export const NOTIFICATION_SHOW = 'NOTIFICATION_SHOW'; export const NOTIFICATION_DISMISS = 'NOTIFICATION_DISMISS'; export const NOTIFICATION_CLEAR = 'NOTIFICATION_CLEAR'; @@ -13,3 +14,11 @@ export function clearNotifications() { type: NOTIFICATION_CLEAR }; }; + +export function showNotification(title, message) { + return { + type: NOTIFICATION_SHOW, + title: title, + message: message + }; +}; diff --git a/app/assets/javascripts/components/actions/statuses.jsx b/app/assets/javascripts/components/actions/statuses.jsx index f7b7b9f737..2fb2d1ba12 100644 --- a/app/assets/javascripts/components/actions/statuses.jsx +++ b/app/assets/javascripts/components/actions/statuses.jsx @@ -25,7 +25,6 @@ export function fetchStatus(id) { axios.all([boundApi.get(`/api/v1/statuses/${id}`), boundApi.get(`/api/v1/statuses/${id}/context`)]).then(values => { dispatch(fetchStatusSuccess(values[0].data, values[1].data)); }).catch(error => { - console.error(error); dispatch(fetchStatusFail(id, error)); }); }; @@ -54,7 +53,6 @@ export function deleteStatus(id) { api(getState).delete(`/api/v1/statuses/${id}`).then(response => { dispatch(deleteStatusSuccess(id)); }).catch(error => { - console.error(error); dispatch(deleteStatusFail(id, error)); }); }; diff --git a/app/assets/javascripts/components/actions/suggestions.jsx b/app/assets/javascripts/components/actions/suggestions.jsx index 103502a2f2..c70a4d121e 100644 --- a/app/assets/javascripts/components/actions/suggestions.jsx +++ b/app/assets/javascripts/components/actions/suggestions.jsx @@ -11,7 +11,6 @@ export function fetchSuggestions() { api(getState).get('/api/v1/accounts/suggestions').then(response => { dispatch(fetchSuggestionsSuccess(response.data)); }).catch(error => { - console.error(error); dispatch(fetchSuggestionsFail(error)); }); }; diff --git a/app/assets/javascripts/components/actions/timelines.jsx b/app/assets/javascripts/components/actions/timelines.jsx index 7b01d43d91..f92f758f5a 100644 --- a/app/assets/javascripts/components/actions/timelines.jsx +++ b/app/assets/javascripts/components/actions/timelines.jsx @@ -48,7 +48,6 @@ export function refreshTimeline(timeline) { api(getState).get(`/api/v1/statuses/${timeline}`).then(function (response) { dispatch(refreshTimelineSuccess(timeline, response.data)); }).catch(function (error) { - console.error(error); dispatch(refreshTimelineFail(timeline, error)); }); }; @@ -71,7 +70,6 @@ export function expandTimeline(timeline) { api(getState).get(`/api/v1/statuses/${timeline}?max_id=${lastId}`).then(response => { dispatch(expandTimelineSuccess(timeline, response.data)); }).catch(error => { - console.error(error); dispatch(expandTimelineFail(timeline, error)); }); }; diff --git a/app/assets/javascripts/components/middleware/errors.jsx b/app/assets/javascripts/components/middleware/errors.jsx new file mode 100644 index 0000000000..9d2aa19d06 --- /dev/null +++ b/app/assets/javascripts/components/middleware/errors.jsx @@ -0,0 +1,31 @@ +import { showNotification } from '../actions/notifications'; + +const defaultFailSuffix = 'FAIL'; + +export default function errorsMiddleware() { + return ({ dispatch }) => next => action => { + if (action.type) { + const isFail = new RegExp(`${defaultFailSuffix}$`, 'g'); + + if (action.type.match(isFail)) { + if (action.error.response) { + const { data, status, statusText } = action.error.response; + + let message = statusText; + let title = `${status}`; + + if (data.error) { + message = data.error; + } + + dispatch(showNotification(title, message)); + } else { + console.error(action.error); + dispatch(showNotification('Oops!', 'An unexpected error occurred. Inspect the console for more details')); + } + } + } + + return next(action); + }; +}; diff --git a/app/assets/javascripts/components/reducers/notifications.jsx b/app/assets/javascripts/components/reducers/notifications.jsx index 63a814bef5..886587bdbe 100644 --- a/app/assets/javascripts/components/reducers/notifications.jsx +++ b/app/assets/javascripts/components/reducers/notifications.jsx @@ -1,68 +1,20 @@ -import { COMPOSE_SUBMIT_FAIL, COMPOSE_UPLOAD_FAIL } from '../actions/compose'; -import { FOLLOW_SUBMIT_FAIL } from '../actions/follow'; import { - REBLOG_FAIL, - UNREBLOG_FAIL, - FAVOURITE_FAIL, - UNFAVOURITE_FAIL -} from '../actions/interactions'; -import { - TIMELINE_REFRESH_FAIL, - TIMELINE_EXPAND_FAIL -} from '../actions/timelines'; -import { NOTIFICATION_DISMISS, NOTIFICATION_CLEAR } from '../actions/notifications'; -import { - ACCOUNT_FETCH_FAIL, - ACCOUNT_FOLLOW_FAIL, - ACCOUNT_UNFOLLOW_FAIL, - ACCOUNT_TIMELINE_FETCH_FAIL, - ACCOUNT_TIMELINE_EXPAND_FAIL -} from '../actions/accounts'; -import { - STATUS_FETCH_FAIL, - STATUS_DELETE_FAIL -} from '../actions/statuses'; -import Immutable from 'immutable'; - -const initialState = Immutable.List(); + NOTIFICATION_SHOW, + NOTIFICATION_DISMISS, + NOTIFICATION_CLEAR +} from '../actions/notifications'; +import Immutable from 'immutable'; -function notificationFromError(state, error) { - let n = Immutable.Map({ - key: state.size > 0 ? state.last().get('key') + 1 : 0, - message: '' - }); - - if (error.response) { - n = n.withMutations(map => { - map.set('message', error.response.statusText); - map.set('title', `${error.response.status}`); - }); - } else { - n = n.set('message', `${error}`); - } - - return state.push(n); -}; +const initialState = Immutable.List([]); export default function notifications(state = initialState, action) { switch(action.type) { - case COMPOSE_SUBMIT_FAIL: - case COMPOSE_UPLOAD_FAIL: - case FOLLOW_SUBMIT_FAIL: - case REBLOG_FAIL: - case FAVOURITE_FAIL: - case TIMELINE_REFRESH_FAIL: - case TIMELINE_EXPAND_FAIL: - case ACCOUNT_FETCH_FAIL: - case ACCOUNT_FOLLOW_FAIL: - case ACCOUNT_UNFOLLOW_FAIL: - case ACCOUNT_TIMELINE_FETCH_FAIL: - case ACCOUNT_TIMELINE_EXPAND_FAIL: - case STATUS_FETCH_FAIL: - case STATUS_DELETE_FAIL: - case UNREBLOG_FAIL: - case UNFAVOURITE_FAIL: - return notificationFromError(state, action.error); + case NOTIFICATION_SHOW: + return state.push(Immutable.Map({ + key: state.size > 0 ? state.last().get('key') + 1 : 0, + title: action.title, + message: action.message + })); case NOTIFICATION_DISMISS: return state.filterNot(item => item.get('key') === action.notification.key); case NOTIFICATION_CLEAR: diff --git a/app/assets/javascripts/components/store/configureStore.jsx b/app/assets/javascripts/components/store/configureStore.jsx index 89f498217e..3d03d4c193 100644 --- a/app/assets/javascripts/components/store/configureStore.jsx +++ b/app/assets/javascripts/components/store/configureStore.jsx @@ -2,9 +2,10 @@ import { createStore, applyMiddleware, compose } from 'redux'; import thunk from 'redux-thunk'; import appReducer from '../reducers'; import { loadingBarMiddleware } from 'react-redux-loading-bar'; +import errorsMiddleware from '../middleware/errors'; export default function configureStore(initialState) { return createStore(appReducer, initialState, compose(applyMiddleware(thunk, loadingBarMiddleware({ promiseTypeSuffixes: ['REQUEST', 'SUCCESS', 'FAIL'], - })), window.devToolsExtension ? window.devToolsExtension() : f => f)); + }), errorsMiddleware()), window.devToolsExtension ? window.devToolsExtension() : f => f)); };