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

Allow course staff to add students to queue #96

Merged
merged 9 commits into from
Apr 10, 2018
26 changes: 21 additions & 5 deletions api/questions.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const { check } = require('express-validator/check')
const { matchedData } = require('express-validator/filter')

const constants = require('../constants')
const { Course, Queue, Question } = require('../models/')
const { Course, Queue, Question, User } = require('../models/')
const {
requireQueue,
requireQueueForQuestion,
Expand Down Expand Up @@ -51,18 +51,34 @@ router.post(
check('topic')
.isLength({ min: 1, max: constants.QUESTION_TOPIC_MAX_LENGTH })
.trim(),
check('netid').optional({ nullable: true }),
checkLocation,
failIfErrors,
],
safeAsync(async (req, res, _next) => {
const data = matchedData(req)
const { id: queueId } = res.locals.queue
const { userAuthz, queue: { id: queueId, courseId } } = res.locals

// First, let's check if the request is coming from course staff and
// includes a specific netid
let askerId = res.locals.userAuthn.id
if (data.netid) {
if (userAuthz.isAdmin || userAuthz.staffedCourseIds.includes(courseId)) {
// The user is allowed to do this!
const [netid] = data.netid.split('@')
const [user] = await User.findOrCreate({ where: { netid } })
askerId = user.id
} else {
res.status(403).send("You don't have authorization to set a netid")
return
}
}

// Let's check if the user already has a question for this queue
const existingQuestion = await Question.findOne({
where: {
queueId,
askedById: res.locals.userAuthn.id,
askedById: askerId,
dequeueTime: null,
},
})
Expand All @@ -78,7 +94,7 @@ router.post(
topic: data.topic,
enqueueTime: new Date(),
queueId,
askedById: res.locals.userAuthn.id,
askedById: askerId,
})

await question.save()
Expand Down Expand Up @@ -259,7 +275,7 @@ router.delete(

if (
question.askedById === userAuthn.id ||
userAuthz.staffedCourseIds.indexOf(course.id) !== -1
userAuthz.staffedCourseIds.includes(course.id)
) {
await question.update({
dequeueTime: new Date(),
Expand Down
53 changes: 51 additions & 2 deletions api/questions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,63 @@ describe('Questions API', () => {
test('succeeds for student with well-formed request', async () => {
const question = { name: 'a', location: 'b', topic: 'c' }
const res = await request(app)
.post('/api/queues/1/questions')
.post('/api/queues/1/questions?forceuser=otherstudent')
.send(question)
expect(res.statusCode).toBe(201)
expect(res.body.name).toBe('a')
expect(res.body.location).toBe('b')
expect(res.body.topic).toBe('c')
expect(res.body).toHaveProperty('askedBy')
expect(res.body.askedBy.netid).toBe('dev')
expect(res.body.askedBy.netid).toBe('otherstudent')
})

test('succeeds for course staff with specific netid', async () => {
const question = {
name: 'a',
location: 'b',
topic: 'c',
netid: 'otherstudent',
}
const res = await request(app)
.post('/api/queues/1/questions?forceuser=225staff')
.send(question)
expect(res.statusCode).toBe(201)
expect(res.body.name).toBe('a')
expect(res.body.location).toBe('b')
expect(res.body.topic).toBe('c')
expect(res.body).toHaveProperty('askedBy')
expect(res.body.askedBy.netid).toBe('otherstudent')
})

test('succeeds for admin with specific netid', async () => {
const question = {
name: 'a',
location: 'b',
topic: 'c',
netid: 'otherstudent',
}
const res = await request(app)
.post('/api/queues/1/questions?forceuser=admin')
.send(question)
expect(res.statusCode).toBe(201)
expect(res.body.name).toBe('a')
expect(res.body.location).toBe('b')
expect(res.body.topic).toBe('c')
expect(res.body).toHaveProperty('askedBy')
expect(res.body.askedBy.netid).toBe('otherstudent')
})

test('fails for student with specific netid', async () => {
const question = {
name: 'a',
location: 'b',
topic: 'c',
netid: 'otherstudent',
}
const res = await request(app)
.post('/api/queues/1/questions?forceuser=student')
.send(question)
expect(res.statusCode).toBe(403)
})

test('removes location for fixed-location queue', async () => {
Expand Down
28 changes: 27 additions & 1 deletion components/NewQuestion.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,12 @@ export default class NewQuestion extends React.Component {
fieldErrors,
})
if (!valid) return

const netid = this.props.isUserCourseStaff
? this.state.netid || undefined
: undefined
const question = {
netid,
name: this.state.name,
location: this.state.location,
topic: this.state.topic,
Expand All @@ -85,7 +90,7 @@ export default class NewQuestion extends React.Component {
}

render() {
const { queue: { location, fixedLocation } } = this.props
const { queue: { location, fixedLocation }, isUserCourseStaff } = this.props

const queueLocation = fixedLocation ? location : this.state.location

Expand All @@ -94,6 +99,26 @@ export default class NewQuestion extends React.Component {
<CardHeader sm={2}>New question</CardHeader>
<CardBody>
<Form onSubmit={this.handleSubmit} autoComplete="off">
{isUserCourseStaff && (
<FormGroup row>
<Label for="netid" sm={2}>
Net ID
</Label>
<Col sm={10}>
<Input
name="netid"
id="netid"
placeholder="Enter a Net ID (optional)"
value={this.state.netid}
onChange={this.handleInputChange}
valid={isValid(this.state.fieldErrors.name)}
Copy link
Collaborator

Choose a reason for hiding this comment

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

should this be this.state.fieldErrors.netid?

/>
<FormText color="muted">
This allows you to add a question on behalf of a student.
</FormText>
</Col>
</FormGroup>
)}
<FormGroup row>
<Label for="name" sm={2}>
Name
Expand Down Expand Up @@ -179,4 +204,5 @@ NewQuestion.propTypes = {
name: PropTypes.string,
}).isRequired,
createQuestion: PropTypes.func.isRequired,
isUserCourseStaff: PropTypes.bool.isRequired,
}
4 changes: 4 additions & 0 deletions containers/NewQuestionContainer.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
import { connect } from 'react-redux'
import { createQuestion } from '../actions/question'

import { isUserCourseStaffForQueue, isUserAdmin } from '../selectors'
import NewQuestion from '../components/NewQuestion'

function mapStateToProps(state, ownProps) {
return {
user: state.user.user,
queueId: ownProps.queueId,
queue: state.queues.queues[ownProps.queueId],
isUserCourseStaff:
isUserCourseStaffForQueue(state, ownProps) ||
isUserAdmin(state, ownProps),
}
}

Expand Down
2 changes: 2 additions & 0 deletions containers/QuestionPanelContainer.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { connect } from 'react-redux'

import {
getUserActiveQuestionIdForQueue,
isUserCourseStaffForQueue,
isUserActiveStaffForQueue,
} from '../selectors'

Expand All @@ -10,6 +11,7 @@ import QuestionPanel from '../components/QuestionPanel'
const mapStateToProps = (state, ownProps) => ({
user: state.user.user,
userActiveQuestionId: getUserActiveQuestionIdForQueue(state, ownProps),
isUserCourseStaff: isUserCourseStaffForQueue(state, ownProps),
isUserActiveStaff: isUserActiveStaffForQueue(state, ownProps),
})

Expand Down