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

Rewrite queue page with hooks #263

Merged
merged 14 commits into from
Apr 9, 2019
Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ with the current semantic version and the next changes should go under a **[Next
* Add logging for new socket initialization error. ([@nwalters512](https://github.com/nwalters512) in [#258](https://github.com/illinois/queue/pull/258))
* Remove unused style tag. ([@nwalters512](https://github.com/nwalters512) in [#259](https://github.com/illinois/queue/pull/259))
* Fix active staff socket errors. ([@nwalters512](https://github.com/nwalters512) in [#262](https://github.com/illinois/queue/pull/262))
* Rewrite queue page with hooks; fix miscellaneous bugs. ([@nwalters512](https://github.com/nwalters512) in [#263](https://github.com/illinois/queue/pull/263))

## v1.0.4

Expand Down
3 changes: 1 addition & 2 deletions src/actions/queue.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ export function createQueue(courseId, queue) {
/**
* Fetch a queue
*/

export const fetchQueueRequest = makeActionCreator(
types.FETCH_QUEUE.REQUEST,
'queueId'
Expand All @@ -88,7 +87,7 @@ export function fetchQueue(queueId) {
.then(res => dispatch(fetchQueueSuccess(queueId, res.data)))
.catch(err => {
console.error(err)
dispatch(fetchQueueFailure(queueId, err))
return dispatch(fetchQueueFailure(queueId, err))
})
}
}
Expand Down
2 changes: 0 additions & 2 deletions src/actions/socket.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import { makeActionCreator } from './util'
import * as types from '../constants/ActionTypes'

export const setSocketError = makeActionCreator(types.SET_SOCKET_ERROR, 'error')

export const setSocketStatus = makeActionCreator(
types.SET_SOCKET_STATUS,
'status'
Expand Down
2 changes: 1 addition & 1 deletion src/api/queues.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ router.get(
},
required: false,
include: [User],
attributes: ['startTime', 'endTime'],
attributes: ['id', 'startTime', 'endTime'],
},
{
model: Question,
Expand Down
94 changes: 43 additions & 51 deletions src/api/queues.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,75 +53,67 @@ describe('Queues API', () => {
})

describe('GET /api/queues/2', () => {
test('succeeds for admin', async () => {
const request = await requestAsUser(app, 'admin')
const testForUser = async (username, isAdmin) => {
const request = await requestAsUser(app, username)
const staffRequest = await requestAsUser(app, '241staff')
const res = await request.get('/api/queues/2')
expect(res.statusCode).toBe(200)
expect(res.body.id).toBe(2)
expect(res.body.name).toBe('CS241 Queue')
expect(res.body.location).toBe('There')
expect(res.body.courseId).toBe(2)

expect(res.body).toHaveProperty('questions')
expect(res.body.questions).toHaveLength(0)
expect(res.body).toHaveProperty('activeStaff')
expect(res.body.activeStaff).toHaveLength(0)
includesPrivateAttributes(res.body)
if (isAdmin) {
includesPrivateAttributes(res.body)
} else {
excludesPrivateAttributes(res.body)
}

// Have 241 staff member join the queue staff
const res2 = await staffRequest.post('/api/queues/2/staff/3')
const activeStaffId = res2.body.id

// Add a question
const question = { name: 'a', location: 'b', topic: 'c' }
const res2 = await request.post('/api/queues/2/questions').send(question)
expect(res2.body.askedBy.netid).toBe('admin')
const questionId = res2.body.id
const res3 = await request.post('/api/queues/2/questions').send(question)
expect(res3.body.askedBy.netid).toBe(username)
const questionId = res3.body.id

const res3 = await request.post(
// Answer the question
const res4 = await staffRequest.post(
`/api/queues/2/questions/${questionId}/answering`
)
expect(res3.body.answeredBy.name).toBe('Admin')
expect(res4.body.answeredBy.name).toBe('241 Staff')

const res5 = await request.get('/api/queues/2')
expect(res5.body).toHaveProperty('questions')
expect(res5.body.questions).toHaveLength(1)
expect(res5.body.questions[0]).toHaveProperty('askedBy')
expect(res5.body.questions[0].askedBy.netid).toBe(username)
expect(res5.body.questions[0]).toHaveProperty('answeredBy')
expect(res5.body.questions[0].answeredBy.name).toBe('241 Staff')
expect(res5.body).toHaveProperty('activeStaff')
expect(res5.body.activeStaff).toHaveLength(1)
expect(res5.body.activeStaff[0]).toHaveProperty('id')
expect(res5.body.activeStaff[0].id).toEqual(activeStaffId)
expect(res5.body.activeStaff[0].user.netid).toEqual('241staff')
expect(res5.body.activeStaff[0].user.name).toEqual('241 Staff')
if (isAdmin) {
includesPrivateAttributes(res5.body)
} else {
excludesPrivateAttributes(res5.body)
}
}

const res4 = await request.get('/api/queues/2')
expect(res4.body).toHaveProperty('questions')
expect(res4.body.questions).toHaveLength(1)
expect(res4.body.questions[0]).toHaveProperty('askedBy')
expect(res4.body.questions[0].askedBy.netid).toBe('admin')
expect(res4.body.questions[0]).toHaveProperty('answeredBy')
expect(res4.body.questions[0].answeredBy.name).toBe('Admin')
includesPrivateAttributes(res4.body)
test('succeeds for admin', async () => {
await testForUser('admin', true)
})

test('succeeds for non-admin', async () => {
const request = await requestAsUser(app, 'student')
const res = await request.get('/api/queues/2')
expect(res.statusCode).toBe(200)
expect(res.body.id).toBe(2)
expect(res.body.name).toBe('CS241 Queue')
expect(res.body.location).toBe('There')
expect(res.body.courseId).toBe(2)

expect(res.body).toHaveProperty('questions')
expect(res.body.questions).toHaveLength(0)
expect(res.body).toHaveProperty('activeStaff')
expect(res.body.activeStaff).toHaveLength(0)
excludesPrivateAttributes(res.body)

const question = { name: 'a', location: 'b', topic: 'c' }
const request2 = await requestAsUser(app, 'admin')
const res2 = await request2.post('/api/queues/2/questions').send(question)
expect(res2.body.askedBy.netid).toBe('admin')
const questionId = res2.body.id

const res3 = await request2.post(
`/api/queues/2/questions/${questionId}/answering`
)
expect(res3.body.answeredBy.name).toBe('Admin')

const res4 = await request.get('/api/queues/2')
expect(res4.body).toHaveProperty('questions')
expect(res4.body.questions).toHaveLength(1)
expect(res4.body.questions[0]).toHaveProperty('askedBy')
expect(res4.body.questions[0].askedBy.netid).toBe('admin')
expect(res4.body.questions[0]).toHaveProperty('answeredBy')
expect(res4.body.questions[0].answeredBy.name).toBe('Admin')
excludesPrivateAttributes(res4.body)
test('succeeds for student', async () => {
await testForUser('student', false)
})
})

Expand Down
19 changes: 13 additions & 6 deletions src/components/Error.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,20 @@ const styles = {

const Error = props => {
const { statusCode } = props
const title =
statusCode === 404
? 'This page could not be found'
: HTTPStatus[statusCode] || 'An unexpected error has occurred'
const title = statusCode !== null ? statusCode : 'RIP'
let message
if (statusCode) {
message = HTTPStatus[statusCode] || 'RIP'
} else if (props.message) {
message = props.message || 'An unexpected error occurred'
} else {
message = 'An unexpected error occurred'
}

return (
<div style={styles.error}>
<h1 className="display-2">{statusCode || 'Error!'}</h1>
<h6>{title}.</h6>
<h1 className="display-2">{title}</h1>
<h6>{message}</h6>
<Link passHref route="index">
<Button outline color="secondary" tag="a" className="mt-4">
Go to homepage
Expand All @@ -54,10 +59,12 @@ const Error = props => {

Error.defaultProps = {
statusCode: 404,
message: null,
}

Error.propTypes = {
statusCode: PropTypes.number,
message: PropTypes.string,
}

export default Error
6 changes: 5 additions & 1 deletion src/components/QueueMessageViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,14 @@ const QueueMessageViewer = props => {

QueueMessageViewer.propTypes = {
queueId: PropTypes.number.isRequired,
message: PropTypes.string.isRequired,
message: PropTypes.string,
collapsible: PropTypes.bool.isRequired,
editable: PropTypes.bool.isRequired,
onEdit: PropTypes.func.isRequired,
}

QueueMessageViewer.defaultProps = {
message: '',
}

export default QueueMessageViewer
20 changes: 2 additions & 18 deletions src/components/SocketErrorModal.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import React from 'react'
import PropTypes from 'prop-types'
import { Modal, ModalHeader, ModalBody } from 'reactstrap'

const SocketErrorModal = ({ isOpen, toggle, error }) => (
<Modal isOpen={isOpen} toggle={toggle}>
const SocketErrorModal = ({ isOpen }) => (
<Modal isOpen={isOpen}>
<ModalHeader>Socket error</ModalHeader>
<ModalBody>
<p>
Expand All @@ -14,28 +14,12 @@ const SocketErrorModal = ({ isOpen, toggle, error }) => (
developers can get in touch with you! In the meantime, please try
accessing the Queue from another browser or device.
</p>
{error && (
<>
<p>
<b>Please include the below message in your email:</b>
</p>
<pre>
<code>{error}</code>
</pre>
</>
)}
</ModalBody>
</Modal>
)

SocketErrorModal.propTypes = {
isOpen: PropTypes.bool.isRequired,
toggle: PropTypes.func.isRequired,
error: PropTypes.string,
}

SocketErrorModal.defaultProps = {
error: null,
}

export default SocketErrorModal
48 changes: 48 additions & 0 deletions src/components/SocketStatusAlert.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import React from 'react'
import PropTypes from 'prop-types'
import { Alert, Collapse } from 'reactstrap'
import {
SOCKET_CONNECTED,
SOCKET_CONNECTING,
SOCKET_ERROR,
SOCKET_STATUS_TYPES,
} from '../constants/socketStatus'

const SocketStatusAlert = ({ isOpen, status }) => {
let message
let color
switch (status) {
case SOCKET_CONNECTED:
message = 'Connected'
color = 'success'
break
case SOCKET_CONNECTING:
message = 'Trying to connect...'
color = 'warning'
break
case SOCKET_ERROR:
message = 'Lost connection. Check your network and refresh the page.'
color = 'danger'
break
default:
return null
}
return (
<Collapse isOpen={isOpen}>
<Alert fade={false} color={color}>
{message}
</Alert>
</Collapse>
)
}

SocketStatusAlert.propTypes = {
isOpen: PropTypes.bool.isRequired,
status: PropTypes.oneOf(SOCKET_STATUS_TYPES),
}

SocketStatusAlert.defaultProps = {
status: null,
}

export default SocketStatusAlert
1 change: 1 addition & 0 deletions src/components/StaffSidebar.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class StaffSidebar extends React.Component {
if (this.props.queue) {
const { removeStaff, queue } = this.props
const activeStaffIds = queue.activeStaff

if (activeStaffIds && activeStaffIds.length > 0) {
staffList = activeStaffIds.map(id => {
const activeStaffId = id
Expand Down
1 change: 0 additions & 1 deletion src/constants/ActionTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,4 @@ export const REPLACE_ACTIVE_STAFF = 'REPLACE_ACTIVE_STAFF'

/* Actions for socket status */
export const SET_SOCKET_STATUS = 'SET_SOCKET_STATUS'
export const SET_SOCKET_ERROR = 'SET_SOCKET_ERROR'
export const RESET_SOCKET_STATE = 'RESET_SOCKET_STATE'
10 changes: 10 additions & 0 deletions src/constants/socketStatus.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export const SOCKET_CONNECTING = 'connecting'
export const SOCKET_CONNECTED = 'connected'
export const SOCKET_ERROR = 'error'
export const SOCKET_AUTHENTICATION_ERROR = 'authentication_error'
export const SOCKET_STATUS_TYPES = [
SOCKET_CONNECTING,
SOCKET_CONNECTED,
SOCKET_ERROR,
SOCKET_AUTHENTICATION_ERROR,
]
19 changes: 14 additions & 5 deletions src/pages/_error.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable prefer-destructuring */
import React from 'react'
import PropTypes from 'prop-types'

Expand All @@ -7,26 +8,34 @@ import PageWithUser from '../components/PageWithUser'
export class ErrorPage extends React.Component {
static getInitialProps({ res, err }) {
let statusCode = null
let message = null
if (res) {
;({ statusCode } = res)
statusCode = res.statusCode
} else if (err) {
;({ statusCode } = err)
if (err.statusCode) {
statusCode = err.statusCode
} else {
message = err.message
}
}

return { statusCode }
return { statusCode, message }
}

render() {
return <Error statusCode={this.props.statusCode} />
const { statusCode, message } = this.props
return <Error statusCode={statusCode} message={message} />
}
}

ErrorPage.defaultProps = {
statusCode: 404,
statusCode: null,
message: null,
}

ErrorPage.propTypes = {
statusCode: PropTypes.number,
message: PropTypes.string,
}

export default PageWithUser(ErrorPage)
Loading