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

Prevent upload modal & flash message from showing stale messages #550

Merged
merged 1 commit into from
May 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion .babelrc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
],
"plugins": [
"@babel/plugin-proposal-class-properties",
"@babel/plugin-transform-runtime"
"@babel/plugin-transform-runtime",
"@babel/plugin-proposal-optional-chaining"
]
}
89 changes: 86 additions & 3 deletions __tests__/components/editor/ImportResourceTemplate.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,44 +124,125 @@ describe('<ImportResourceTemplate />', () => {

expect(wrapper.state().message).toEqual([])

wrapper.instance().updateStateFromServerResponse({ status: 200, headers: {} })
wrapper.instance().updateStateFromServerResponse({ status: 200, headers: {} }, 0)

expect(wrapper.state().message).toEqual(['Unexpected response (200)! '])
})

it('sets modalShow to true when receiving HTTP 409 and errors >= profileCount', () => {
// Set new wrapper in each of these tests because we are changing state
wrapper = shallow(<ImportResourceTemplate.WrappedComponent authenticationState={authenticationState}/>)

expect(wrapper.state().modalShow).toBe(false)

wrapper.instance().updateStateFromServerResponse({ status: 409 , headers: {} }, 0)

expect(wrapper.state().modalShow).toBe(true)
})

it('sets message in state with any create operation not resulting in HTTP 409', () => {
// Set new wrapper in each of these tests because we are changing state
wrapper = shallow(<ImportResourceTemplate.WrappedComponent authenticationState={authenticationState}/>)

expect(wrapper.state().message).toEqual([])

wrapper.instance().updateStateFromServerResponse({ status: 201 , headers: { location: 'http://foo.bar' } })
wrapper.instance().updateStateFromServerResponse({ status: 201 , headers: { location: 'http://sinopia.io/repository/ld4p/myResourceTemplate' } }, 0)

expect(wrapper.state().message).toEqual(['Created http://sinopia.io/repository/ld4p/myResourceTemplate'])
})

it('resets createResourceError and message in component state when response is OK', () => {
// Set new wrapper in each of these tests because we are changing state
wrapper = shallow(<ImportResourceTemplate.WrappedComponent authenticationState={authenticationState}/>)

wrapper.setState({ createResourceError: ['Could not update resource!'], message: ['Updated http://sinopia.io/repository/ld4p/myResourceTemplate'] })
expect(wrapper.state().createResourceError).toEqual(['Could not update resource!'])
expect(wrapper.state().message).toEqual(['Updated http://sinopia.io/repository/ld4p/myResourceTemplate'])

wrapper.instance().updateStateFromServerResponse({ ok: true, status: 204 }, 0)

expect(wrapper.state().message).toEqual(['Created http://foo.bar'])
expect(wrapper.state().createResourceError).toEqual([])
expect(wrapper.state().message).toEqual(['Updated '])
})

it('leaves createResourceError alone and appends message in component state when response is not OK', () => {
// Set new wrapper in each of these tests because we are changing state
wrapper = shallow(<ImportResourceTemplate.WrappedComponent authenticationState={authenticationState}/>)

wrapper.setState({ createResourceError: ['Could not update resource!'], message: ['Updated http://sinopia.io/repository/ld4p/myResourceTemplate'] })
expect(wrapper.state().createResourceError).toEqual(['Could not update resource!'])
expect(wrapper.state().message).toEqual(['Updated http://sinopia.io/repository/ld4p/myResourceTemplate'])

wrapper.instance().updateStateFromServerResponse({ ok: false, status: 400 }, 0)

expect(wrapper.state().createResourceError).toEqual(['Could not update resource!'])
expect(wrapper.state().message).toEqual(['Updated http://sinopia.io/repository/ld4p/myResourceTemplate', 'Unexpected response (400)! '])
})
})

describe('humanReadableLocation()', () => {
it('returns human-readable label when Location header exists', () => {
const response = {
headers: {
location: 'http://sinopia.io/repository/ld4p/myResourceTemplate'
},
req: {
url: 'http://sinopia.io/repository/ld4p',
_data: {
id: 'myResourceTemplate'
}
}
}
expect(wrapper.instance().humanReadableLocation(response)).toEqual('http://sinopia.io/repository/ld4p/myResourceTemplate')
})

it('returns human-readable label when data ID exists', () => {
const response = {
req: {
url: 'http://sinopia.io/repository/ld4p',
_data: {
id: 'myResourceTemplate'
}
}
}
expect(wrapper.instance().humanReadableLocation(response)).toEqual('http://sinopia.io/repository/ld4p/myResourceTemplate')
})

it('returns human-readable label when request URL already includes data ID', () => {
const response = {
req: {
url: 'http://sinopia.io/repository/ld4p/myResourceTemplate',
_data: {
id: 'myResourceTemplate'
}
}
}
expect(wrapper.instance().humanReadableLocation(response)).toEqual('http://sinopia.io/repository/ld4p/myResourceTemplate')
})

it('returns an empty label otherwise', () => {
const response = undefined
expect(wrapper.instance().humanReadableLocation(response)).toEqual('')
})
})

describe('humanReadableStatus()', () => {
it('returns human-readable label for HTTP 201', () => {
expect(wrapper.instance().humanReadableStatus(201)).toEqual('Created')
})

it('returns human-readable label for HTTP 204', () => {
expect(wrapper.instance().humanReadableStatus(204)).toEqual('Updated')
})

it('returns human-readable label for HTTP 401', () => {
expect(wrapper.instance().humanReadableStatus(401)).toEqual('You are not authorized to do this. Try logging in again!')
})

it('returns human-readable label for HTTP 409', () => {
expect(wrapper.instance().humanReadableStatus(409)).toEqual('Attempting to update')
})

it('returns human-readable label for any other HTTP status', () => {
expect(wrapper.instance().humanReadableStatus(400)).toEqual('Unexpected response (400)!')
})
Expand All @@ -181,11 +262,13 @@ describe('<ImportResourceTemplate />', () => {
const updateResourceSpy = jest.spyOn(wrapper.instance(), 'updateResource').mockImplementation(async () => {})
const updateStateSpy = jest.spyOn(wrapper.instance(), 'updateStateFromServerResponse').mockReturnValue(null)
const modalCloseSpy = jest.spyOn(wrapper.instance(), 'modalClose').mockReturnValue(null)
const setStateSpy = jest.spyOn(wrapper.instance(), 'setState').mockReturnValue(null)

await wrapper.instance().handleUpdateResource(templates, 'ld4p')

expect(updateResourceSpy).toHaveBeenCalledTimes(2)
expect(updateStateSpy).toHaveBeenCalledTimes(2)
expect(setStateSpy).toHaveBeenNthCalledWith(1, { message: [] })
expect(modalCloseSpy).toHaveBeenCalledTimes(1)
})
})
Expand Down
19 changes: 19 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
"@babel/core": "^7.4.3",
"@babel/node": "^7.4.5",
"@babel/plugin-proposal-class-properties": "^7.4.0",
"@babel/plugin-proposal-optional-chaining": "^7.2.0",
jermnelson marked this conversation as resolved.
Show resolved Hide resolved
"@babel/plugin-transform-runtime": "^7.4.3",
"@babel/preset-env": "^7.4.3",
"@babel/preset-react": "^7.0.0",
Expand Down
36 changes: 29 additions & 7 deletions src/components/editor/ImportResourceTemplate.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,20 +85,39 @@ class ImportResourceTemplate extends Component {
updateStateFromServerResponse = (response, profileCount, updateKey) => {
// HTTP status 409 == Conflict
const showModal = response.status === 409 && this.state.createResourceError.length >= profileCount

const location = response.headers.location || ''
const newMessage = `${this.humanReadableStatus(response.status)} ${location}`
const newState = {
message: [...this.state.message, newMessage],
updateKey: updateKey
}
const newMessage = `${this.humanReadableStatus(response.status)} ${this.humanReadableLocation(response)}`
const newState = { updateKey: updateKey }

if (showModal)
newState.modalShow = true

if (response.ok) {
// Wipe out past messages and errors if response was successful
newState.createResourceError = []
newState.message = [newMessage]
} else {
newState.message = [...this.state.message, newMessage]
jermnelson marked this conversation as resolved.
Show resolved Hide resolved
}

this.setState(newState)
}

// Returns a URL or an empty string
humanReadableLocation = response => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

like this method naming.

if (response?.headers?.location)
return response.headers.location

if (response?.req?._data?.id) {
// If the request URL already contains the ID, don't bother appending
if (response.req.url.endsWith(response.req._data.id))
return response.req.url
return `${response.req.url}/${response.req._data.id}`
}

// Welp, we tried anyway
return ''
jermnelson marked this conversation as resolved.
Show resolved Hide resolved
}

humanReadableStatus = status => {
switch(status) {
case 201:
Expand All @@ -121,6 +140,9 @@ class ImportResourceTemplate extends Component {
// The 0 is the profileCount which is only used for tracking create errors
ndushay marked this conversation as resolved.
Show resolved Hide resolved
this.updateStateFromServerResponse(response, 0, incrementedKey)
})
this.setState({
message: []
})
this.modalClose()
}

Expand Down
3 changes: 1 addition & 2 deletions src/components/editor/SinopiaResourceTemplates.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,7 @@ class SinopiaResourceTemplates extends Component {
)
}

let createResourceMessage = <div className="alert alert-info alert-dismissible">
<button className="close" data-dismiss="alert" aria-label="close">&times;</button>
let createResourceMessage = <div className="alert alert-info">
{ this.props.message.join(', ') }
</div>;

Expand Down