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

Prevent upload modal & flash message from showing stale messages
LD4P · May 23, 2019 · 4f8a5f2 · 4f8a5f2
2 parents deee924 + afc9e90
commit 4f8a5f2
Show file tree

Hide file tree

Showing 6 changed files with 138 additions and 13 deletions.
diff --git a/.babelrc b/.babelrc
@@ -5,6 +5,7 @@
   ],
   "plugins": [
     "@babel/plugin-proposal-class-properties",
-    "@babel/plugin-transform-runtime"
+    "@babel/plugin-transform-runtime",
+    "@babel/plugin-proposal-optional-chaining"
   ]
 }
diff --git a/__tests__/components/editor/ImportResourceTemplate.test.js b/__tests__/components/editor/ImportResourceTemplate.test.js
@@ -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)!')
     })
@@ -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)
     })
   })

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -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",
     "@babel/plugin-transform-runtime": "^7.4.3",
     "@babel/preset-env": "^7.4.3",
     "@babel/preset-react": "^7.0.0",

diff --git a/src/components/editor/ImportResourceTemplate.jsx b/src/components/editor/ImportResourceTemplate.jsx
@@ -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]
+    }
+
     this.setState(newState)
   }
 
+  // Returns a URL or an empty string
+  humanReadableLocation = response => {
+    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 ''
+  }
+
   humanReadableStatus = status => {
     switch(status) {
       case 201:
@@ -121,6 +140,9 @@ class ImportResourceTemplate extends Component {
       // The 0 is the profileCount which is only used for tracking create errors
       this.updateStateFromServerResponse(response, 0, incrementedKey)
     })
+    this.setState({
+      message: []
+    })
     this.modalClose()
   }
 

diff --git a/src/components/editor/SinopiaResourceTemplates.jsx b/src/components/editor/SinopiaResourceTemplates.jsx
@@ -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>;