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

feat: add validate schedule middleware #34

Merged
merged 3 commits into from
Dec 6, 2024
Merged

feat: add validate schedule middleware #34

merged 3 commits into from
Dec 6, 2024

Conversation

joonamin
Copy link
Member

@joonamin joonamin commented Dec 1, 2024

No description provided.

@joonamin joonamin self-assigned this Dec 1, 2024
1 similar comment
@@ -13,6 +13,7 @@
"private": true,
"dependencies": {
"@typegoose/typegoose": "^12.9.0",
"@types/node": "^20",
Copy link
Member

Choose a reason for hiding this comment

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

추가하신 이유가 어떤걸까요??

Copy link
Member Author

Choose a reason for hiding this comment

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

이건 제가 실수로 다른 작업하다가 섞인 것 같아요 제거하고 다시 커밋 올리겠습니다!

@@ -41,8 +42,7 @@ router.get('/', async (req: Request, res: Response) => {
res.status(200).json(schedules)
})

router.delete('/:id', async (req: Request, res: Response) => {
// TODO: 작성자가 jwt에 있는 userId와 일치하는지 검증하는 middleware 추가
router.delete('/:id', middlewares.schedules.verifyAuthorMiddleware, async (req: Request, res: Response) => {
await ScheduleModel.deleteOne({ _id: req.params.id })
res.sendStatus(204)
})
Copy link
Member

Choose a reason for hiding this comment

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

밑에 put 에도 middleware 추가해주세요

@@ -0,0 +1,9 @@
import { APIError } from '@/types/errors/error'

export class UnAuthorizedSchedule extends APIError {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
export class UnAuthorizedSchedule extends APIError {
export class UnauthorizedSchedule extends APIError {


export class UnAuthorizedSchedule extends APIError {
constructor(cause: Error | string = null) {
super(401, 40100, 'unauthorized schedule', cause)
Copy link
Member

Choose a reason for hiding this comment

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

별거 아니라 넘어가도 되는데 만 단위는 너무 큰거 같아요
에러 십만개 만들거 같지 않아서요...

Suggested change
super(401, 40100, 'unauthorized schedule', cause)
super(401, 4010, 'unauthorized schedule', cause)

Copy link
Member Author

Choose a reason for hiding this comment

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

xxx00 이런식으로 xxx status code에 대한 세부 원인을 00 ~ 99 까지 2자리 수로 표현하고 싶었어요.
한 자리 수는 너무 적다는 생각을 했었거든요..! 한 자리 수도 적당하다고 생각하시는거라면 리뷰에 반영하겠습니다!

Copy link
Member

Choose a reason for hiding this comment

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

굳이 앞에 3자리를 status code 와 안맞춰도 됩니다

@joonamin joonamin requested a review from laggu December 5, 2024 15:19
@joonamin joonamin merged commit 6b9e96e into develop Dec 6, 2024
2 checks passed
@joonamin joonamin deleted the FIENMEE-149 branch December 6, 2024 07:25
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