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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ with the current date and the next changes should go under a **[Next]** header.
* Update all `npm` dependencies. ([@nwalters512](https://github.com/nwalters512) in [#91](https://github.com/illinois/queue/pull/91))
* Show NetID for non-active staff; add tests for question rendering. ([@nwalters512](https://github.com/nwalters512) in [#93](https://github.com/illinois/queue/pull/93))
* Redesign course homepage to use the new queue cards; add information about the course shortcode link. ([@nwalters512](https://github.com/nwalters512) in [#88](https://github.com/illinois/queue/pull/88))
* Allow course staff and admins to add questions on behalf of other students. ([@nwalters512](https://github.com/nwalters512) in [#96](https://github.com/illinois/queue/pull/96))

## 31 March 2018

Expand Down
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