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

Backend endpoints #14

Merged
merged 17 commits into from
Dec 4, 2024
Merged

Backend endpoints #14

merged 17 commits into from
Dec 4, 2024

Conversation

parkrafael
Copy link
Member

@parkrafael parkrafael commented Nov 26, 2024

Tasks finished

  • Created new endpoints for logbooks and logs (adult cardiac)
  • Added linting to backend

API Endpoints

Screenshot 2024-11-28 at 4 42 15 PM

Copy link
Contributor

@TonyLiu0226 TonyLiu0226 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work so far with the routes. I have suggested few modifications to be made, mostly related to error handling and file structuring.

One main thing is that in the service side, if an error is encountered, the server will crash. So we should return the error back to the controller and handle it there through returning a error status code with the error message, rather than throwing the error directly in the service. I have added a commit as a suggestion for how to do this, please feel free to refactor the rest of the routes in a similar way.

backend/test/backend.test.js Outdated Show resolved Hide resolved
backend/src/services/logbooks-service.js Outdated Show resolved Hide resolved
backend/src/services/logbooks-service.js Outdated Show resolved Hide resolved
backend/src/services/logbooks-service.js Outdated Show resolved Hide resolved
backend/src/services/logbooks-service.js Outdated Show resolved Hide resolved
Copy link
Contributor

@TonyLiu0226 TonyLiu0226 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for refactoring things! Just a couple of edge cases to fix now, once thats done feel free to merge in

backend/src/utils/get-logbook-type.js Outdated Show resolved Hide resolved
backend/src/services/logbooks-service.js Show resolved Hide resolved
backend/src/services/logbooks-service.js Show resolved Hide resolved
@parkrafael parkrafael merged commit 49f2322 into main Dec 4, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants