[Team Offline Merge] updates models + created and updates routes #44
Conversation
Course now refers to a creator instead of a user
applications/content-creator-applications/controller/contentCreatorApplicationControllerspec.js
Outdated
Show resolved
Hide resolved
applications/content-creator-applications/data-access/contentCreatorApplicationListspec.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Needs some tests for handling errors
There was a problem hiding this comment.
har lavet sindssygt mange tests for handling errors
There was a problem hiding this comment.
Looks good, only needs some more test cases for errors, and maybe take a look at the file errorCodes.js in the helpers folder. It would be great to implement them as soon as possible, there is a readme file in the resources repo for them. Quick note, why are you adding an .idea folder.
|
lots of error handling and tests of error handling added :)) Made sure everything is following microservices REST API guidelines |
These files were added to the .gitignore, they should not be.
There was a problem hiding this comment.
Great that you're using the error codes! Remember to add the to ERRORS.md in the resources repo
| /*** SUBSCRIPTION ROUTES ***/ | ||
|
|
||
| // Subscribe to course | ||
| router.post('/:id/subscribe', async (req, res) => { |
There was a problem hiding this comment.
Right now our api guidelines specify that actions like subscribe should be formatted as e.g.
/couses/some-id:subscribe.
We should either change the guidelines or follow them. Maybe this is a question for the integration team?
How to add colon in express endpoints:
expressjs/express#3857 (comment)
There was a problem hiding this comment.
Jeg tror ikke subscribe er "action" som beskrevet i API guidelines, siden den opdaterer (updates) modellen til user: defor tror jeg det går under denne definition:
⛔ DO NOT use an action operation when the operation behavior could reasonably be defined as one of the standard REST Create, Read, Update, Delete, or List operations.
There was a problem hiding this comment.
altså subscribe tror jeg ikke er action som beskrevet her: Performing an Action
The REST specification is used to model the state of a resource, and is primarily intended to handle CRUD (Create, Read, Update, Delete) operations. However, many services require the ability to perform an action on a resource, e.g. getting the thumbnail of an image or rebooting a VM. It is also sometimes useful to perform an action on a collection.
There was a problem hiding this comment.
og chatGPT mente at courses/:id/subscribe var rigtigt
helpers/errorCodes.js
Outdated
| }, | ||
| E0007: { | ||
| code: 'E0007', | ||
| message: 'No sections not found' |
There was a problem hiding this comment.
"No sections not found" maybe "No sections found"
routes/userRoutes.js
Outdated
|
|
||
|
|
||
| // Checks if user is subscribed to a specific course | ||
| router.get('', async (req, res) => { |
Description
last updated 19:57 12/10/23 : added extra error handling as well as testing error handling
We have changed and created some new routes and updated models
Changes
We have updated models so they follow the structure on Miro.
We have commented out all the routes we are not using.
The new routes are found in courseRoutes and are related to fetching courses and sections, as well as subscribing to course.
We also did some modifications in the authRoute so that the user id is stored in addition to mail and name from before.
Related Issues
Not really.
Checklist
Notes for Reviewers
Changed the naming of some .spec files in the application / content-creator application folder, since the tests are from last year and all the tests fail. they are now named with notInUseTest_ at the beginning and removed the .specs to avoid testing them for now.
All routes have been tested in the test folder