Skip to content

[민지영] sprint6&7#14

Merged
rjc1704 merged 3 commits intocodeit-sprint-fullstack:express-민지영from
Obsessive-Curiosity:express-민지영-mission07
Jan 28, 2025

Hidden character warning

The head ref may contain hidden characters: "express-\ubbfc\uc9c0\uc601-mission07"
Merged

[민지영] sprint6&7#14
rjc1704 merged 3 commits intocodeit-sprint-fullstack:express-민지영from
Obsessive-Curiosity:express-민지영-mission07

Conversation

@Obsessive-Curiosity
Copy link
Collaborator

요구사항

기본

  • [x]
  • []
  • []

심화

  • [x]
  • []

주요 변경사항

스크린샷

image

멘토에게

  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

article, comment, likes, product, user
add article service
app.use(cookieParser());
app.use(
cors({
origin: "http://localhost:5173",
Copy link

Choose a reason for hiding this comment

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

배포 시 cors 허용을 로컬서버 5173포트에서만 허용하는 것은 적절해보이지 않습니다. 개발 중에는 app.use(cors()) 로 해서 모든 오리진에서 오는 요청을 오픈해놓고 api 서버 운영하셔도 좋고, 배포 후에는 리액트로 배포한 도메인을 origin 에 등록하는 방향으로 보안을 강화하는 방향으로 가는 것을 권장 드립니다.

@rjc1704
Copy link

rjc1704 commented Jan 28, 2025

스프린트6,7을 모두 작성하셨군요. 고생하셨습니다. 이번에는 스프린트 6 기준으로 검토해서 코멘트 해드리도록 하고, 추후에 스프린트7 제출기간에 필요한 부분 보충해서 다시 제출하시면 그 때 스프린트7 기준으로 추가로 리뷰해드리도록 하겠습니다.

  • render.com 에 배포여부를 알 수 없습니다. 만약 하지 않으셨다면 배포 진행해서 리드미나 PR 디스크립션에 올려보시면 좋겠습니다.
  • asyncHandler 를 이용해서 에러핸들링 코드를 재사용한 부분 아주 좋습니다.
  • createProduct 와 같이 Post 요청 또는 patch 요청은 서버 api 에서도 반드시 유효성검사를 진행하셔야 합니다. DB에 insert 요청전에 DB 테이블에서 지정한 타입이 맞는지, 문자열이면 특정 길이 이상인지 등 요구사항들을 점검해서 맞지 않으면 400번대 에러를 응답으로 줘야 합니다. 유효성검사는 프론트 파트에서도 해야하지만 백엔드 파트에서도 각 관문마다 해주는 것이 강력하게 권장됩니다.
  • router.post("/", verifyToken, createProduct); 처럼 미들웨어로 토큰검증을 사용하셨는데 미들웨어를 이용해서 유효성검사를 하게 만드실 수도 있겠습니다. router.post("/", verifyToken, validateProduct, createProduct); 와 같은 형식으로 유효성검사 로직을 별도로 재사용할 수 있게 모듈화해서 진행하셔도 괜찮겠습니다.

@rjc1704 rjc1704 merged commit f5634bf into codeit-sprint-fullstack:express-민지영 Jan 28, 2025
1 check 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