Update /assets endpoint
#713
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The endpoint is updated to `/v2/admin/assets`, and validation is performed within the router instead.
bnjmnt4n
commented
Jan 27, 2021
| assert response(conn, 403) =~ "User not allowed to manage assets" | ||
| assert response(conn, 403) =~ "Forbidden" |
There was a problem hiding this comment.
We've lost the more informative error message, but I think there's a fair trade-off in terms of improving code de-duplication.
bnjmnt4n
commented
Jan 27, 2021
angelsl
reviewed
Jan 31, 2021
angelsl
pushed a commit
that referenced
this pull request
Apr 17, 2021
* 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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've created a
v2branch, which me and @qreoct will use over the course of the semester to stage our changes.This PR removes the role validation from the
Assetsmodel, since models should not be concerned with validation, and removes theuserparameter from the function definitions. It also updates the routes to shift the/v1/assetsendpoint into/v2/admin/assetsand performs validation in the router.