Skip to content
This repository has been archived by the owner on Oct 9, 2020. It is now read-only.

Setup backend DB and add example service #8

Open
wants to merge 28 commits into
base: master
Choose a base branch
from
Open

Conversation

Qrtn
Copy link
Member

@Qrtn Qrtn commented Oct 1, 2020

Status:

🚀 Ready

Description

Setup backend DB and add example service.

  • Add script for creating Postgres role, databases, and seeding with data
  • Update README with DB setup info
  • Add example service for accessing DB
  • Add example route that uses this service

Example route /students/{id} works for testing DB.

Why a service layer

  • Avoid writing SQL queries in the routes
  • Separation of business logic from HTTP route logic
  • If method of accessing data changes, we can just update the services, w/o touching the routes
  • Possible to unit test services only, rather than whole route

Workflow for service layer

  • When implementing a new api method, add a new "service" in ../services/MODULE.js
  • Then add a new route in ../routes/MODULE.js
  • Each route imports the service functions it needs from ../services/MODULE.js
  • Think about how to generalize "services" into reusable methods, rather than being specifically for 1 route (although that's ok too)

@Qrtn Qrtn self-assigned this Oct 1, 2020
@vercel
Copy link

vercel bot commented Oct 1, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/hack4impact/closegap/701ab026u
✅ Preview: https://closegap-git-setupbackenddb.hack4impact1.vercel.app

Copy link
Contributor

@yousefa00 yousefa00 left a comment

Choose a reason for hiding this comment

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

Amazing, left some small comments 🚀

api/infra/setup_dev_db.sh Outdated Show resolved Hide resolved
api/src/routes/checkin.js Outdated Show resolved Hide resolved
Comment on lines 1 to 6
const express = require("express");
const { StatusCodes } = require("http-status-codes");

const { getStudentById } = require("../services/student.js");

const router = express.Router();
Copy link
Contributor

Choose a reason for hiding this comment

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

Very nitpicky, but do you think we should reorder these imports, moving router under express and the two others immediately below and without a new line between them?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed! In general, I think it's still a good idea to separate third-party requires and internal requires with a blank line, though. So we can differentiate them at a glance. I pushed a new commit as an example.

Copy link
Member Author

@Qrtn Qrtn Oct 1, 2020

Choose a reason for hiding this comment

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

const express = require("express");
const router = express.Router();
const statusCodes = require("http-status-codes");

const { getStudentById } = require("../services/student.js");

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep we should do this instead, thanks for pointing that out :)

api/src/routes/index.js Outdated Show resolved Hide resolved
api/src/routes/index.js Outdated Show resolved Hide resolved
@yousefa00
Copy link
Contributor

Status:

🚀 Ready

Description

Setup backend DB and add example service.

  • Add script for creating Postgres role, databases, and seeding with data
  • Update README with DB setup info
  • Add example service for accessing DB
  • Add example route that uses this service

Example route /students/{id} works for testing DB.

Why a service layer

  • Avoid writing SQL queries in the routes
  • Separation of business logic from HTTP route logic
  • If method of accessing data changes, we can just update the services, w/o touching the routes
  • Possible to unit test services only, rather than whole route

Workflow for service layer

  • When implementing a new api method, add a new "service" in ../services/MODULE.js
  • Then add a new route in ../routes/MODULE.js
  • Each route imports the service functions it needs from ../services/MODULE.js
  • Think about how to generalize "services" into reusable methods, rather than being specifically for 1 route (although that's ok too)

Also this is a phenomenal description, definitely setting a great precedent, we all have to step up the game now lol

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants