Update `/assessments` endpoint #729

bnjmnt4n · 2021-02-23T17:21:49Z

This PR updates the /assessments endpoint by:

creating /admin/assessments endpoints:
- POST /admin/assessments to add new assessments
- POST /admin/assessments/{assessmentId} to update properties of assessments
- DELETE /admin/assessments/{assessmentId} to delete assessments
shifting role validation into AnswerController

Todo

Split POST /assessments/{assessmentId} into two separate requests to unlock assessments with passwords as suggested by @angelsl
- I'm not sure how we might want to do this for now: maybe add a password column to the submissions table, so that POST /assessments/{assessmentId}/unlock would add the user-input password into that column, and GET /assessments/{assessmentId} can compare passwords between the Assessment and Submission?

Reference

https://docs.google.com/document/d/1SF0w0OxQFHecu3JBjyUPHEIidFlyGrbY5efwKGWcAOk/edit#

…ller

… AnswerController

qreoct

Looks great.

angelsl

I took a glance, looks generally OK. (Note, I didn't read in detail!)

See my comments on the routes.

Also, test coverage seems to have dropped a bit? (The test coverage on master is 97.095%)

lib/cadet_web/router.ex

`AssesmentsController#unlock` unlocks password-protected assessments and creates a Submission for them, following which the assessments do not require requests with the password.

…ller

… AnswerController

`AssesmentsController#unlock` unlocks password-protected assessments and creates a Submission for them, following which the assessments do not require requests with the password.

coveralls · 2021-03-29T15:51:07Z

Pull Request Test Coverage Report for Build a6e47b02113c7bca39c93ece5e29fa588862de15-PR-729

73 of 76 (96.05%) changed or added relevant lines in 5 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.6%) to 96.344%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
lib/cadet_web/admin_controllers/admin_assessments_controller.ex	31	34	91.18%

Totals
Change from base Build 27658cb5299a0fad2a275223511c302848a8a92d-PR-721:	0.6%
Covered Lines:	1502
Relevant Lines:	1559

💛 - Coveralls

… bnjmnt4n/assessments

angelsl

This is great, thank you! Just a few minor tweaks.

angelsl · 2021-04-01T19:47:16Z

lib/cadet_web/admin_controllers/admin_assessments_controller.ex

+  def create(conn, %{"assessment" => assessment, "forceUpdate" => force_update}) do
+    file =
+      assessment["file"].path
+      |> File.read!()
+
+    result =
+      case force_update do
+        "true" -> parse_xml(file, true)
+        "false" -> parse_xml(file, false)
+      end


Just checking, forceUpdate is a string because this is submitted as multipart/form-data, right?

Yeah, this is the case.

lib/cadet_web/controllers/answer_controller.ex

lib/cadet_web/controllers/assessments_controller.ex

bnjmnt4n · 2021-04-08T05:50:18Z

I'm going to merge this into v2 since I've addressed the feedback. If there are changes to the coverage, we can look into it in v2 also.

Ref. source-academy/backend#729. Prettier

* Update `/assets` endpoint (#713) * Assets: remove role validation in model * AssetsController: update API endpoint and validation The endpoint is updated to `/v2/admin/assets`, and validation is performed within the router instead. * Shift to `AdminAssetsController` * Rename `AdminAssetsController` to `.exs` * Assets: switch to `case` instead of `with` * (1) Clearer Swagger documentation in Grading (2) Code documentation cleaup in auth_controller * Notifications changed to plural throughout * Settings swagger docs cleanup * User docs typo fix * CI fix formatting * Update `/settings` endpoint (#722) * `/auth`: shift `/auth` to `/auth/login` * `/devices`: fix typo and shift to v2 * Update `/grading` endpoint (#727) * shift role validation for Settings * shift Grading controller to AdminGrading * fix formatting and add details to docs Co-authored-by: Benjamin Tan <benjamin@dev.ofcr.se> * Rename `assets_controller_test` to `admin_assets_controller_test` * update swagger docs for /grading * Update `/assessments` endpoint (#729) * `/assessments`: split out separate `/admin` endpoint * Consistently use `/assessments/:assessmentid` * Merge `/admin/assessments/:assessmentid/{update,publish}` endpoints * Quick swagger docs fix for `AdminAssessmentsController` * Format code via `mix format` * Formatting fix * `/assessments/{assessmentid}/submit`: shift role validation to controller * Add more guards * `/assessments/question/{questionid}/submit`: shift role validation to AnswerController * clean up some docs * Modify `AnswerController#submit` path * Assessments: remove unused definitions * Remove leftover unused route * Split `AssesmentsController#unlock` out of `AssessmentsController#show` `AssesmentsController#unlock` unlocks password-protected assessments and creates a Submission for them, following which the assessments do not require requests with the password. * Fix formatting issues * `/assessments`: split out separate `/admin` endpoint * Consistently use `/assessments/:assessmentid` * Merge `/admin/assessments/:assessmentid/{update,publish}` endpoints * Quick swagger docs fix for `AdminAssessmentsController` * Format code via `mix format` * `/assessments/{assessmentid}/submit`: shift role validation to controller * Add more guards * `/assessments/question/{questionid}/submit`: shift role validation to AnswerController * clean up some docs * Modify `AnswerController#submit` path * Assessments: remove unused definitions * Remove leftover unused route * Split `AssesmentsController#unlock` out of `AssessmentsController#show` `AssesmentsController#unlock` unlocks password-protected assessments and creates a Submission for them, following which the assessments do not require requests with the password. * Fix formatting issues * formatting * Update per PR feedback * Remove unused variable Co-authored-by: qreoct <9080974+qreoct@users.noreply.github.com> * Change to v2 API throughout * Fix Swagger references * AdminGradingControllerTest: Add swagger path definition * Remove duplicated route * Rename file as per convention * Swagger documentation: Switch to enums for string values * /auth/login: shift parameters to query string in docs * UserController: add an empty UserGameStates map to the schema * Add comments to clarify invalid OpenAPI 2.0 types * Swagger: shift request parameters to separate schemas Request parameters were previously declared as separate body parameters in the Swagger schema, which was technically invalid. For endpoints which require more than 1 parameter, which is sent as JSON, I've created separated schemas which represent the parameters, to make the generated `swagger.json` more compliant. * Story: Fix inaccurate Swagger documentation * Shift all Swagger enums into their own schemas This allows `swagger-typescript-api` in `cadet-frontend` to generate TypeScript `enum`s for the corresponding type. * Fix inaccurate types * Update based on code review feedback * clean up docs * clean up docs * Endpoints: update to mark as required * More changes to the Swagger types * Even more type updates * more swagger docs consistency * /admin/assessments: update swagger to FormData parameters Co-authored-by: qreoct <9080974+qreoct@users.noreply.github.com>

bnjmnt4n added 9 commits February 23, 2021 23:43

/assessments: split out separate /admin endpoint

d7da86f

Consistently use /assessments/:assessmentid

01de819

Merge /admin/assessments/:assessmentid/{update,publish} endpoints

2b247c9

Quick swagger docs fix for AdminAssessmentsController

956abc2

Format code via mix format

0f48423

Formatting fix

3e33845

/assessments/{assessmentid}/submit: shift role validation to contro…

abd2fa2

…ller

Add more guards

41cd47d

/assessments/question/{questionid}/submit: shift role validation to…

10d5ec1

… AnswerController

bnjmnt4n requested review from angelsl and qreoct February 24, 2021 04:34

clean up some docs

af263bc

qreoct approved these changes Feb 25, 2021

View reviewed changes

angelsl reviewed Mar 5, 2021

View reviewed changes

lib/cadet_web/router.ex Outdated Show resolved Hide resolved

lib/cadet_web/router.ex Outdated Show resolved Hide resolved

lib/cadet_web/router.ex Outdated Show resolved Hide resolved

qreoct mentioned this pull request Mar 18, 2021

Cadet API v2 #721

Merged

bnjmnt4n added 5 commits March 26, 2021 14:57

Modify AnswerController#submit path

6370c36

Assessments: remove unused definitions

646c514

Remove leftover unused route

2706c44

Split AssesmentsController#unlock out of AssessmentsController#show

1d2428d

`AssesmentsController#unlock` unlocks password-protected assessments and creates a Submission for them, following which the assessments do not require requests with the password.

Merge remote-tracking branch 'origin/master' into v2

18e7d19

bnjmnt4n force-pushed the bnjmnt4n/assessments branch 2 times, most recently from 57ecf0a to 8704108 Compare March 29, 2021 14:15

Fix formatting issues

27658cb

bnjmnt4n force-pushed the v2 branch from 8648a73 to 27658cb Compare March 29, 2021 14:30

bnjmnt4n added 6 commits March 29, 2021 23:19

/assessments: split out separate /admin endpoint

21b114e

Consistently use /assessments/:assessmentid

5428e03

Merge /admin/assessments/:assessmentid/{update,publish} endpoints

a99e907

Quick swagger docs fix for AdminAssessmentsController

666df2f

Format code via mix format

af1bfda

/assessments/{assessmentid}/submit: shift role validation to contro…

0802b86

…ller

bnjmnt4n and others added 8 commits March 29, 2021 23:20

Add more guards

638e8ed

/assessments/question/{questionid}/submit: shift role validation to…

961df0f

… AnswerController

clean up some docs

0fb4505

Modify AnswerController#submit path

09b9255

Assessments: remove unused definitions

7057cf2

Remove leftover unused route

7c909a3

Split AssesmentsController#unlock out of AssessmentsController#show

5bf6a0c

`AssesmentsController#unlock` unlocks password-protected assessments and creates a Submission for them, following which the assessments do not require requests with the password.

Fix formatting issues

d9aaeba

bnjmnt4n force-pushed the bnjmnt4n/assessments branch from 8704108 to d9aaeba Compare March 29, 2021 15:20

qreoct added 3 commits April 1, 2021 13:04

pull master

991b025

formatting

3911707

Merge branch 'master' of https://github.com/source-academy/cadet into…

a6e47b0

… bnjmnt4n/assessments

qreoct force-pushed the v2 branch from 27658cb to 7d2e7ad Compare April 1, 2021 08:32

angelsl reviewed Apr 1, 2021

View reviewed changes

bnjmnt4n added 3 commits April 8, 2021 12:04

Update per PR feedback

c18d9e3

Merge remote-tracking branch 'origin/v2' into bnjmnt4n/assessments

748bb57

Remove unused variable

13a7da6

bnjmnt4n merged commit 77dc886 into v2 Apr 8, 2021

bnjmnt4n deleted the bnjmnt4n/assessments branch April 8, 2021 05:50

bnjmnt4n added a commit to source-academy/frontend that referenced this pull request Apr 9, 2021

Update for new assessments endpoints

459fd7b

Ref. source-academy/backend#729. Prettier

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Update `/assessments` endpoint #729

Update `/assessments` endpoint #729

Uh oh!

bnjmnt4n commented Feb 23, 2021 •

edited

Loading

Uh oh!

qreoct left a comment

Uh oh!

angelsl left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coveralls commented Mar 29, 2021 •

edited

Loading

Uh oh!

angelsl left a comment

Uh oh!

angelsl Apr 1, 2021

Uh oh!

bnjmnt4n Apr 8, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bnjmnt4n commented Apr 8, 2021

Uh oh!

Uh oh!

Update /assessments endpoint #729

Update /assessments endpoint #729

Uh oh!

Conversation

bnjmnt4n commented Feb 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Todo

Reference

Uh oh!

qreoct left a comment

Choose a reason for hiding this comment

Uh oh!

angelsl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coveralls commented Mar 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build a6e47b02113c7bca39c93ece5e29fa588862de15-PR-729

💛 - Coveralls

Uh oh!

angelsl left a comment

Choose a reason for hiding this comment

Uh oh!

angelsl Apr 1, 2021

Choose a reason for hiding this comment

Uh oh!

bnjmnt4n Apr 8, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bnjmnt4n commented Apr 8, 2021

Uh oh!

Uh oh!

Update `/assessments` endpoint #729

Update `/assessments` endpoint #729

bnjmnt4n commented Feb 23, 2021 •

edited

Loading

coveralls commented Mar 29, 2021 •

edited

Loading