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

Limit "already answering" check to one queue #90

Merged
merged 3 commits into from
Mar 31, 2018
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ with the current date and the next changes should go under a **[Next]** header.
## [Next]

* Show queue name and location on queue page. ([@sgorse](https://github.com/sgorse) in [#81](https://github.com/illinois/queue/pull/81))
* Add ability to edit existing queues. ([@zwang180](https://github.com/zwang180) in
[#78](https://github.com/illinois/queue/pull/78))
* Add ability to edit existing queues. ([@zwang180](https://github.com/zwang180) in [#78](https://github.com/illinois/queue/pull/78))
* Fix [#89](https://github.com/illinois/queue/issues/89) by only checking in one queue for another question being answered by the same user. ([@nwalters512](https://github.com/nwalters512) in [#90](https://github.com/illinois/queue/pull/90))

## 28 March 2018

Expand Down
14 changes: 11 additions & 3 deletions api/questions.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,14 @@ router.get(
// Mark a question as being answered
router.post(
'/:questionId/answering',
[requireCourseStaffForQueueForQuestion, requireQuestion, failIfErrors],
[
requireCourseStaffForQueueForQuestion,
requireQuestion,
requireQueueForQuestion,
failIfErrors,
],
async (req, res, _next) => {
const { question } = res.locals
const { queue, question } = res.locals

if (question.beingAnswered) {
// Forbid someone else from taking over this question
Expand All @@ -125,11 +130,14 @@ router.post(
where: {
answeredById: res.locals.userAuthn.id,
dequeueTime: null,
queueId: queue.id,
},
})

if (otherQuestions !== null) {
res.status(403).send('You are already answering this question')
res
.status(403)
.send('You are already answering another question on this queue')
return
}

Expand Down
15 changes: 15 additions & 0 deletions api/questions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,21 @@ describe('Questions API', () => {
expect(res.body.beingAnswered).toBe(true)
})

test('succeeds if user is answering a question on another queue', async () => {
// Mark question as being answered by admin
const res = await request(app).post(
'/api/queues/1/questions/1/answering?forceuser=admin'
)
expect(res.statusCode).toBe(200)
// Attempt to answer as another user
const res2 = await request(app).post(
'/api/queues/3/questions/3/answering?forceuser=admin'
)
expect(res2.statusCode).toBe(200)
const question = await Question.findById(3)
expect(question.answeredById).toBe(2)
})

test('fails if another user is already answering the question', async () => {
// Mark question as being answered by admin
const res = await request(app).post(
Expand Down