Skip to content

Commit

Permalink
show error when api_key changed (and automatically reset) (#18)
Browse files Browse the repository at this point in the history
  • Loading branch information
natterstefan authored Jul 4, 2018
1 parent c70fe56 commit 9f3331e
Show file tree
Hide file tree
Showing 22 changed files with 574 additions and 202 deletions.
5 changes: 5 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

## [Unreleased][1]

### Added

* When Trello-API returns 401 (eg. when `api_key` changed and old token is still
present in the localStorage) show error notification and force re-authentication.

## 2018/07/03 [0.2.0][6]

### Added
Expand Down
5 changes: 5 additions & 0 deletions setup-jest.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ window.Trello = {
get: jest.fn(() => Promise.resolve({})),
}

// mock browser's localStorage
global.localStorage = {
removeItem: jest.fn(),
}

// mock trello's embed.min.js
window.TrelloCards = {
create: jest.fn((cardId, cardSelector, options) => {
Expand Down
4 changes: 2 additions & 2 deletions src/actions/boards/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ const requestBoards = () => async dispatch => {
// then update the state once we got all boards
// TODO: evaluate if board.idx should be removed
dispatch(receiveBoards(sortBy(boards, ['idx'])))
} catch (e) {
dispatch(receiveBoards(e, true))
} catch (error) {
dispatch(receiveBoards(null, error))
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/actions/cards/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ const requestCards = (list, config) => async dispatch => {
})

dispatch(receiveCards(list.id, cards))
} catch (errorMsg) {
dispatch(receiveCards(list.id, null, errorMsg))
} catch (error) {
dispatch(receiveCards(list.id, null, error))
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/actions/lists/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ const requestLists = (board, config) => async dispatch => {
})

dispatch(receiveLists(board.id, lists))
} catch (errorMsg) {
dispatch(receiveLists(board.id, null, errorMsg))
} catch (error) {
dispatch(receiveLists(board.id, null, error))
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/actions/user/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ const authenticateUser = () => async dispatch => {
try {
await doAuthenticateUser()
dispatch(doneAuthentication(true))
} catch (e) {
dispatch(doneAuthentication(null, e))
} catch (error) {
dispatch(doneAuthentication(null, error))
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

exports[`Component/MainApp should render without throwing an error 1`] = `
<React.Fragment>
<Connect(WithStyles(Notification)) />
<div
style={
Object {
Expand Down
12 changes: 1 addition & 11 deletions src/components/main-app/__tests__/component.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ describe('Component/MainApp', () => {
test('should render without throwing an error', () => {
const wrapper = shallow(<MainApp {...props} />).dive()
expect(wrapper.find(EstimationCard).length).toEqual(0) // only visible after toggle
expect(wrapper.find(Notification).length).toEqual(0) // only visible when property appErrors is valid
expect(wrapper.find(Notification).length).toEqual(1)
expect(wrapper.find(BoardsList).length).toEqual(1)
expect(wrapper.find(MembersList).length).toEqual(1)
expect(wrapper).toMatchSnapshot()
Expand All @@ -58,16 +58,6 @@ describe('Component/MainApp', () => {
expect(wrapper.find(EstimationCard).length).toEqual(1)
})

test('should show Notification when appErrors is a valid array', () => {
const wrapper = shallow(<MainApp {...props} />).dive()
expect(wrapper.find(Notification).length).toEqual(0)
wrapper.setProps({
appErrors: ['example error'],
})
wrapper.update()
expect(wrapper.find(Notification).length).toEqual(1)
})

test('should render an LinearProgress when isLoading is true', () => {
const wrapper = shallow(<MainApp {...props} isLoading />).dive()
expect(wrapper.find(LinearProgress).length).toEqual(1)
Expand Down
7 changes: 2 additions & 5 deletions src/components/main-app/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class MainApp extends React.Component {

render() {
const { showEstimationCard } = this.state
const { app, appErrors, classes, isAppLoading, isLoading } = this.props
const { app, classes, isAppLoading, isLoading } = this.props
const togglePreferred = get(app, 'memberToggle.togglePreferred', false)
const togglePreferredMember = get(app, 'memberToggle.togglePreferredMember', false)

Expand All @@ -100,10 +100,7 @@ class MainApp extends React.Component {
// vheight sticky footer trick, see: https://blog.hellojs.org/flexbox-sticky-footer-and-react-d116e4cfca5
return (
<React.Fragment>
{appErrors &&
appErrors.length > 0 && (
<Notification message="An error occured. Please try it again later." />
)}
<Notification />
<div style={{ minHeight: '100vh', marginBottom: 30, paddingBottom: 50 }}>
<BlockContainer>
<Typography variant="headline" component="h2">
Expand Down
31 changes: 3 additions & 28 deletions src/components/main-app/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { connect } from 'react-redux'
import { forOwn, get, includes } from 'lodash'
import { get, includes } from 'lodash'

// actions
import { push } from 'connected-react-router'
Expand All @@ -9,39 +9,14 @@ import { resetBoards, requestBoards } from '../../actions/boards'
import { loadPreferredMembers } from '../../actions/members'

import MainApp from './component'

const checkState = (state, type, prop) => {
forOwn(get(state, type, {}) || {}, value => value && value[prop])
}
import { isAppLoading as isAppLoadingCheck } from '../../utils/utils'

const mapStateToProps = state => {
// we check if the app is still in a loading state
const isAppLoading = {
isBoardsLoading: get(state, 'boards.isLoading') || undefined,
isCardsLoading: checkState(state, 'cards', 'isLoading') || undefined,
isListsLoading: checkState(state, 'lists', 'isLoading') || undefined,
isMembersLoading: get(state, 'members.isLoading') || undefined,
}

// evaluate if one of the stores has an error and make them (if one occurs)
// available to the <Component /> so we can tell the user about it
const appErrors = []
forOwn(
{
boardError: get(state, 'boards.error') || undefined,
cardError: checkState(state, 'cards', 'error') || undefined,
listError: checkState(state, 'lists', 'error') || undefined,
memberError: get(state, 'members.error') || undefined,
userError: get(state, 'user.error') || undefined,
},
(item, key) => item && appErrors.push({ key, message: item.message }),
)
const isAppLoading = isAppLoadingCheck(state)

return {
app: get(state, 'app', {}),
appErrors,
error: get(state, 'boards.error', null),
info: get(state, 'info', {}),
isAppLoading: includes(isAppLoading, true),
isLoading: get(state, 'boards.isLoading', false),
isMembersLoading: isAppLoading.isMembersLoading,
Expand Down
34 changes: 34 additions & 0 deletions src/components/notification/__mocks__/notification.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
export const props = { message: 'some notification' }

export const storeStateMock = {
boards: {
isLoading: false,
error: null,
},
lists: {
'board-1': {
isLoading: false,
error: null,
},
},
members: {
isLoading: false,
error: null,
},
}

export const storeWithBoardError = {
boards: {
isLoading: false,
error: {
status: 401,
responseText: 'example response',
},
},
}

export const storeWithAnyError = {
user: {
error: { message: 'some error user message ' },
},
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Component/Notification should render without throwing an error 1`] = `
<WithStyles(Snackbar)
ContentProps={
Object {
"aria-describedby": "message-id",
}
}
action={
Array [
<WithStyles(IconButton)
aria-label="Close"
className="Notification-close-1"
color="inherit"
onClick={[Function]}
>
<pure(Close) />
</WithStyles(IconButton)>,
]
}
anchorOrigin={
Object {
"horizontal": "right",
"vertical": "bottom",
}
}
autoHideDuration={10000}
message={
<span
id="message-id"
>
Test Message
</span>
}
onClose={[Function]}
open={true}
/>
`;

This file was deleted.

55 changes: 55 additions & 0 deletions src/components/notification/__tests__/component.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import React from 'react'
import { shallow } from 'enzyme'

import Snackbar from '@material-ui/core/Snackbar'

import Notification from '../component'

describe('Component/Notification', () => {
const props = {
message: 'Test Message',
}

test('should render without throwing an error', () => {
const wrapper = shallow(<Notification {...props} />).dive()
expect(wrapper).toMatchSnapshot()
expect(wrapper.find(Snackbar)).toHaveLength(1)
})

test('should render null when message is not present', () => {
const wrapper = shallow(<Notification />).dive()
expect(wrapper.html()).toEqual(null)
})

test('should have a default open state of false when no props.message is available', () => {
const wrapper = shallow(<Notification />).dive()
const instance = wrapper.instance()
expect(instance.state).toEqual({ open: false })
})

test('should have a default open state of true when props.message is available', () => {
const wrapper = shallow(<Notification {...props} />).dive()
const instance = wrapper.instance()
expect(instance.state).toEqual({ open: true })
})

test('should adjust the autoHideDuration when props.autoHideDuration is set', () => {
const wrapper = shallow(<Notification {...props} autoHideDuration={1000} />).dive()
expect(wrapper.find(Snackbar).prop('autoHideDuration')).toEqual(1000)
})

test('should return undefined when handleClose was called with reason "clickaway"', () => {
const wrapper = shallow(<Notification {...props} />).dive()
const instance = wrapper.instance()
expect(instance.handleClose(null, 'clickaway')).toBe(undefined)
})

test('should change the state of state.open when handleClose was called with another reason than "clickaway"', () => {
const wrapper = shallow(<Notification {...props} />).dive()
const instance = wrapper.instance()

expect(instance.state).toEqual({ open: true })
instance.handleClose(null)
expect(instance.state).toEqual({ open: false })
})
})
Loading

0 comments on commit 9f3331e

Please sign in to comment.