feat: 캘린더 조회 API 및 선택적 인증 구현#1
Conversation
1. 40글자 넘어 가는 테스트 에러 2. 최소길이 4글자 이상 테스트 에러
- DuplicatedEmailError 예외 클래스 추가 - signup 엔드포인트에 이메일 중복 검증 로직 구현 - 이메일 중복 오류 테스트 케이스 추가 - Race condition 대비 IntegrityError 처리 개선 - username과 email 중복 체크 로직 명확화"
- Calendar 엔드포인트 및 스키마 추가 - 선택적 사용자 인증(CurrentUserOptionalDep) 구현 - 호스트/게스트 권한별 응답 스키마 분리 (CalendarOut/CalendarDetailOut) - pytest parametrize를 활용한 테스트 리팩토링 - 중복 테스트 코드 제거 및 fixture 추가
- 선택적 사용자 인증 의존성 추가 (CurrentUserOptionalDep) - 호스트 캘린더 조회 엔드포인트 구현 - 권한별 응답 스키마 분리 (CalendarOut/CalendarDetailOut) - 캘린더 관련 예외 처리 추가 (HostNotFoundError, CalendarNotFoundError) - pytest parametrize를 활용한 테스트 리팩토링 - API 통합 테스트 추가"
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2482b6143b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| app = FastAPI() | ||
| def include_routers(_app: FastAPI): | ||
| _app.include_router(account_router) |
There was a problem hiding this comment.
The new calendar endpoint is never reachable because include_routers only mounts account_router, so requests to /calendar/{host_username} will always return 404 even after this feature commit. This effectively ships the calendar API as dead code.
Useful? React with 👍 / 👎.
| # payload.model_dump(): SignupPayload 객체 → dict 변환 | ||
| # User.model_validate(): dict → User 모델 변환 (검증 포함) | ||
| # from_attributes=True: 객체의 속성에서도 값을 가져올 수 있게 함 | ||
| user = User.model_validate(payload.model_dump(), from_attributes=True) |
There was a problem hiding this comment.
signup validates payload directly into User without setting hashed_password, but the User model now requires that field. In practice this raises a validation error during signup (or stores the wrong password fields), so new account creation is broken in the normal registration path.
Useful? React with 👍 / 👎.
| if user is not None and user.id == host.id: | ||
| return CalendarDetailOut.model_validate(calendar) |
There was a problem hiding this comment.
host_calendar_detail only returns a value when the requester is the host; guest and anonymous requests fall through with no return. That causes an invalid response (None/validation failure) instead of the intended CalendarOut, breaking the optional-auth behavior for public calendar reads.
Useful? React with 👍 / 👎.
| if now + timedelta(minutes=ACCESS_TOKEN_EXPIRE_MINUTES) < expires_at: | ||
| raise ExpiredTokenError() from e |
There was a problem hiding this comment.
The expiration branch compares times in the wrong direction and then does raise ExpiredTokenError() from e where e is out of scope. If this branch is hit, it will raise NameError instead of an auth error, and the current predicate does not correctly detect expired tokens.
Useful? React with 👍 / 👎.
|
@codex address 피드백줘 |
|
To use Codex here, create an environment for this repo. |
1 participant
Summary
Notes
@codex review