From 7bc2e8f02b5427fccad141cedb16a317965a56bc Mon Sep 17 00:00:00 2001 From: Adam Butterworth Date: Wed, 6 Mar 2019 12:17:04 -0500 Subject: [PATCH] Photo upload error handling (#57) * Parse error response for photo upload * Show profile photo upload errors * Update test to match new shape of error * Clearer comments --- src/components/ProfilePage.jsx | 10 +++++----- src/reducers/ProfilePageReducer.js | 2 +- src/sagas/RootSaga.js | 16 ++++++++++++---- src/sagas/RootSaga.test.js | 6 ++++-- src/selectors/ProfilePageSelector.js | 5 ++++- src/services/ProfileApiService.js | 13 ++++++------- 6 files changed, 32 insertions(+), 20 deletions(-) diff --git a/src/components/ProfilePage.jsx b/src/components/ProfilePage.jsx index 38e65ea..0ed1ebb 100644 --- a/src/components/ProfilePage.jsx +++ b/src/components/ProfilePage.jsx @@ -1,6 +1,6 @@ import React from 'react'; import PropTypes from 'prop-types'; -import { Container, Row, Col } from 'reactstrap'; +import { Container, Row, Col, Alert } from 'reactstrap'; import { connect } from 'react-redux'; import { logEvent } from '../analytics/analytics'; @@ -77,7 +77,7 @@ export class ProfilePage extends React.Component { profileImage, username, dateJoined, - errors, + photoUploadError, name, visibilityName, country, @@ -99,7 +99,6 @@ export class ProfilePage extends React.Component { closeHandler: this.handleClose, submitHandler: this.handleSubmit, changeHandler: this.handleChange, - errors, }; return ( @@ -119,6 +118,7 @@ export class ProfilePage extends React.Component { isEditable={this.props.isCurrentUserProfile} />
+ {photoUploadError !== null ? {photoUploadError.userMessage} : null}

{username}

@@ -240,7 +240,7 @@ ProfilePage.propTypes = { savePhotoState: PropTypes.oneOf([null, 'pending', 'complete', 'error']), // Page state helpers - errors: PropTypes.objectOf(PropTypes.string), + photoUploadError: PropTypes.objectOf(PropTypes.string), // Actions fetchProfile: PropTypes.func.isRequired, @@ -262,7 +262,7 @@ ProfilePage.propTypes = { ProfilePage.defaultProps = { saveState: null, savePhotoState: null, - errors: {}, + photoUploadError: {}, profileImage: {}, name: null, username: null, diff --git a/src/reducers/ProfilePageReducer.js b/src/reducers/ProfilePageReducer.js index 2c543f9..640ef12 100644 --- a/src/reducers/ProfilePageReducer.js +++ b/src/reducers/ProfilePageReducer.js @@ -76,7 +76,7 @@ const profilePage = (state = initialState, action) => { return { ...state, savePhotoState: 'error', - errors: Object.assign({}, state.errors, action.payload.errors), + errors: Object.assign({}, state.errors, { photo: action.payload.error }), }; case SAVE_PROFILE_PHOTO.RESET: return { diff --git a/src/sagas/RootSaga.js b/src/sagas/RootSaga.js index cc1681e..e0f2285 100644 --- a/src/sagas/RootSaga.js +++ b/src/sagas/RootSaga.js @@ -122,9 +122,12 @@ export function* handleSaveProfile(action) { yield put(saveProfileReset()); yield put(resetDrafts()); } catch (e) { - // TODO: If this is any other kind of exception than a known validation error from the server, - // this code will fail gracelessly when it can't find fieldErrors on the error. - yield put(saveProfileFailure(e.fieldErrors)); + if (e.processedData.fieldErrors) { + yield put(saveProfileFailure(e.processedData.fieldErrors)); + } else { + // TODO: Currently failing silently on other kinds of errors + yield put(saveProfileReset()); + } } } @@ -141,7 +144,12 @@ export function* handleSaveProfilePhoto(action) { yield put(saveProfilePhotoSuccess()); yield put(saveProfilePhotoReset()); } catch (e) { - yield put(saveProfilePhotoFailure(e.message)); + if (e.processedData) { + yield put(saveProfilePhotoFailure(e.processedData)); + } else { + // TODO: Currently failing silently on other kinds of errors + yield put(saveProfilePhotoReset()); + } } } diff --git a/src/sagas/RootSaga.test.js b/src/sagas/RootSaga.test.js index a1c4eed..b5b2fbe 100644 --- a/src/sagas/RootSaga.test.js +++ b/src/sagas/RootSaga.test.js @@ -74,8 +74,10 @@ describe('RootSaga', () => { it('should successfully publish a failure action on exception', () => { const error = new Error('uhoh'); - error.fieldErrors = { - uhoh: 'not good', + error.processedData = { + fieldErrors: { + uhoh: 'not good', + }, }; const action = profileActions.saveProfile( 'my username', diff --git a/src/selectors/ProfilePageSelector.js b/src/selectors/ProfilePageSelector.js index 47913bb..0c46377 100644 --- a/src/selectors/ProfilePageSelector.js +++ b/src/selectors/ProfilePageSelector.js @@ -67,7 +67,7 @@ export const visibilityDraftsFieldSelector = createSelector( export const formErrorSelector = createSelector( accountErrorsSelector, formIdSelector, - (errors, formId) => errors[formId] || null, + (errors, formId) => (errors[formId] ? errors[formId].userMessage : null), ); export const editableFormSelector = createSelector( @@ -264,6 +264,7 @@ export const profilePageSelector = createSelector( savePhotoStateSelector, isCurrentUserProfileSelector, draftSocialLinksByPlatformSelector, + accountErrorsSelector, ( account, formValues, @@ -272,6 +273,7 @@ export const profilePageSelector = createSelector( savePhotoState, isCurrentUserProfile, draftSocialLinksByPlatform, + errors, ) => ({ // Account data we need username: account.username, @@ -312,5 +314,6 @@ export const profilePageSelector = createSelector( saveState, savePhotoState, isCurrentUserProfile, + photoUploadError: errors.photo || null, }), ); diff --git a/src/services/ProfileApiService.js b/src/services/ProfileApiService.js index 4ff9785..3d7ccb0 100644 --- a/src/services/ProfileApiService.js +++ b/src/services/ProfileApiService.js @@ -39,12 +39,7 @@ export async function patchProfile(username, params) { }, ).catch((error) => { const processedError = Object.create(error); - const fieldErrors = Object.entries(processAccountData(error.response.data.field_errors)) - .reduce((acc, [fieldKey, messages]) => { - acc[fieldKey] = messages.userMessage; - return acc; - }, {}); - processedError.fieldErrors = fieldErrors; + processedError.processedData = processAccountData(error.response.data); throw processedError; }); @@ -63,7 +58,11 @@ export async function postProfilePhoto(username, formData) { 'Content-Type': 'multipart/form-data', }, }, - ); + ).catch((error) => { + const processedError = Object.create(error); + processedError.processedData = camelCaseObject(error.response.data); + throw processedError; + }); return camelCaseObject(data); }