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

[ Feature ] 번쩍 개설 시 최대 인원이 최소 인원보다 작을 때 에러 메시지 출력시키기 #1004

Merged
merged 10 commits into from
Feb 5, 2025

Conversation

borimong
Copy link
Contributor

@borimong borimong commented Feb 2, 2025

🚩 관련 이슈

📋 작업 내용

  • 기능 구현
  • 페이지 구조화 및 스타일링

📌 PR Point

  • 무슨 이유로 어떻게 코드를 변경했는지
    refine 은 에러 메시지를 필드값의 변경에 따라 즉각적으로 보여주지 못해, superRefine 을 이용해 구현하였습니다. (zod 의 문서를 보니 refine 은 superRefine 의 syntax sugar 였습니다. refine 이 사용하기 편리하긴 하지만, 이와 같은 제약이 있을 수 있으니 유의하여 사용하면 좋을 것 같습니다!!)
  • 어떤 위험이나 우려가 발견되었는지
    edit 페이지에서 formData 의 placeDetail 값이 undefined 가 아니라 null이었어요.
    zod 문서에 따르면 optional 은 undefined 에 대한 type narrowing 이기 때문에 null 은 인식하지 못했어요. 그래서 nullable 을 추가해주었습니다. 현재는 nullable 과 optional 을 모두 사용하고 있으니, undefined 와 null 일 경우 isValid 에 false 를 반환하게 될 거에요!
  • 어떤 부분에 리뷰어가 집중해야 하는지
  • 개발하면서 어떤 점이 궁금했는지

📸 스크린샷

Copy link

height bot commented Feb 2, 2025

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

Copy link
Member

@j-nary j-nary left a comment

Choose a reason for hiding this comment

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

LGTM! 즉각적인 반영이 필요하면서, minCapacity, maxCapacity 처럼 여러 필드 간 관계 검증할 때는 superRefine이 더 안정적인 것 같아요 배워갑니다~

Comment on lines 142 to 167
capacityInfo: z
.object({
minCapacity: z
.number({
required_error: '모집 인원을 입력해주세요.',
invalid_type_error: '모집 인원을 입력해주세요.',
})
.gt(0, { message: '0보다 큰 값을 입력해주세요.' })
.lte(999, { message: '모집 인원을 다시 입력해주세요.' }),
maxCapacity: z
.number({
required_error: '모집 인원을 입력해주세요.',
invalid_type_error: '모집 인원을 입력해주세요.',
})
.gt(0, { message: '0보다 큰 값을 입력해주세요.' })
.lte(999, { message: '모집 인원을 다시 입력해주세요.' }),
})
.gt(0, { message: '0보다 큰 값을 입력해주세요.' })
.lte(999, { message: '모집 인원을 다시 입력해주세요.' }),
.superRefine(({ minCapacity, maxCapacity }, ctx) => {
if (minCapacity > maxCapacity) {
ctx.addIssue({
code: z.ZodIssueCode.custom,
message: '최소 인원수가 최대 인원수보다 큽니다.',
path: ['maxCapacity'],
});
}
}),
Copy link
Member

Choose a reason for hiding this comment

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

capacityInfo 로 묶은 거 깔끔하고 좋은 것 같아요!
revalid_errorinvalid_type_error 가 불필요하게 중복되는 것 같은데 공통으로 스키마 분리해서 사용하는 것은 어떨까요 ?

const capacitySchema = z
  .number({
    required_error: '모집 인원을 입력해주세요.',
    invalid_type_error: '모집 인원을 입력해주세요.',
  })
  .gt(0, { message: '0보다 큰 값을 입력해주세요.' })
  .lte(999, { message: '모집 인원을 다시 입력해주세요.' });

capacityInfo: z
  .object({
    minCapacity: capacitySchema,
    maxCapacity: capacitySchema,
  })
  ...

@borimong borimong merged commit 0b36357 into develop Feb 5, 2025
1 check passed
@borimong borimong deleted the feat/#1003 branch February 5, 2025 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

번쩍 개설 최소 최대 인원 유효성 검사 조건 추가
2 participants