feat(api): 가능 시간 관리 API 구현 - #94
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Walkthrough조직의 활성 플래닝 기간 조회·업서트·삭제와 근무자 가용 시간 조회·일괄 교체 API가 추가되었습니다. 관련 검증, 테스트 기반, OpenAPI 문서도 함께 갱신되었습니다. Changes조직 일정 API
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
apps/api/src/modules/availability/availability.service.ts (1)
22-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrisma 조회 결과 타입을 수동으로 복제하지 마세요.
OrganizationForAvailability는findUnique({ include: { businessHours: true } })의 결과 모양을 손으로 다시 적고 있어서, 선택 필드나 relation 타입이 바뀌면 이 서비스 타입이 바로 드리프트합니다. generated payload type으로 붙여 두는 편이 안전합니다.♻️ 제안
-type OrganizationForAvailability = { - id: bigint; - businessHours: OrganizationBusinessHour[]; -}; +type OrganizationForAvailability = Prisma.OrganizationGetPayload<{ + include: { businessHours: true }; +}>;As per coding guidelines,
apps/api/src/**/*.ts: Use Prisma-generated result types for query results and do not redefine those types manually.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/modules/availability/availability.service.ts` around lines 22 - 25, The OrganizationForAvailability type is manually duplicating a Prisma query result shape and can drift from the actual include payload. Replace the hand-written type in availability.service.ts with the Prisma-generated payload type for the findUnique call that includes businessHours, and update any references in the availability service to use that generated type instead of redefining id/businessHours manually. Keep the type tied to the relevant Prisma model/result so changes to selected fields or relations stay in sync automatically.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/middlewares/validate.ts`:
- Around line 30-32: In validate.ts, the query handling in the validation
middleware currently merges validated data into req.query, which leaves removed
or unvalidated keys behind and breaks the Zod contract. Update the query branch
in the validate middleware so it effectively replaces the existing query
contents while preserving object identity, for example by clearing req.query
before assigning result.data.query back into it. Use the validate request flow
around result.data.query and req.query as the place to make this change, and
keep the same approach consistent with body and params handling if needed.
In `@apps/api/src/modules/availability/availability.service.spec.ts`:
- Around line 308-447: Add a regression test in the availability service spec to
cover the missing startsAt >= endsAt validation. Use replaceAvailability with
createPrismaMock to pass an item whose startsAt is equal to or after endsAt, and
assert it rejects with the same invalid time-range error shape. Keep the new
case alongside the existing validation tests in the availability.service.spec.ts
suite so the behavior is locked in.
In `@apps/api/src/modules/availability/availability.service.ts`:
- Around line 127-135: The availability validation in availability.service.ts
only checks that the range fits within business hours, so reverse or zero-length
intervals can still pass. Update the validation around startsAt, endsAt, openAt,
and closeAt to also reject any item where startsAt is greater than or equal to
endsAt before reaching createMany, and keep using createInvalidTimeRangeError
for the failure case.
In `@apps/api/src/modules/organization/planning-period.service.ts`:
- Around line 29-30: The organization-context guard in
planning-period.service.ts is returning the wrong contract for organization
subresources. Update the check around the organization lookup to follow the same
organization-route behavior used by apps/api/src/routes/organization.routes.ts,
returning NOT_FOUND-style handling instead of HttpError(403,
ERROR_CODES.ORGANIZATION_REQUIRED, ...). Keep the logic aligned with the
existing organization guard and add/adjust a regression test for this route to
verify the expected organization-specific error contract.
In `@apps/api/test/availability.routes.test.ts`:
- Around line 84-250: The availability routes tests are missing
cross-organization access coverage for worker scoping, so add cases that call
get /api/availability and put /api/availability/bulk with a workerId belonging
to a different organization than the authenticated user. Use the existing
createFakePrisma, createApp, and authHeader setup in availability.routes.test.ts
to verify both endpoints reject cross-org worker access with the expected error
response, while keeping the current same-org tests intact.
In `@apps/api/test/organization.routes.test.ts`:
- Around line 209-318: The organization active planning period route tests are
missing a regression case for users without an organization, so add a new test
in organization.routes.test.ts using createApp and the
active-schedule-planning-period endpoint to verify the existing
/api/organization contract still returns 404 NOT_FOUND for a no-organization
authenticated user. Reuse the same route-level patterns already present around
the active planning period tests, and assert the response status/body for the
Organization subresource behavior rather than only the
null/success/validation/delete cases.
In `@packages/shared/src/schemas/common.ts`:
- Around line 33-39: In isDateTimeOnThirtyMinuteBoundary, the boundary check is
using new Date(...) plus getUTCMinutes(), which can reject valid :00/:30 inputs
when the datetime includes an offset. Update the logic to inspect the original
dateTime string’s hour/minute/second values instead of converting to UTC, and
keep the existing zero-seconds/milliseconds requirement when validating the
boundary.
---
Nitpick comments:
In `@apps/api/src/modules/availability/availability.service.ts`:
- Around line 22-25: The OrganizationForAvailability type is manually
duplicating a Prisma query result shape and can drift from the actual include
payload. Replace the hand-written type in availability.service.ts with the
Prisma-generated payload type for the findUnique call that includes
businessHours, and update any references in the availability service to use that
generated type instead of redefining id/businessHours manually. Keep the type
tied to the relevant Prisma model/result so changes to selected fields or
relations stay in sync automatically.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3a6eaa96-bb12-4aff-99ce-affeea0da9d6
📒 Files selected for processing (17)
apps/api/src/middlewares/validate.tsapps/api/src/modules/availability/availability.handlers.tsapps/api/src/modules/availability/availability.service.spec.tsapps/api/src/modules/availability/availability.service.tsapps/api/src/modules/organization/planning-period.handlers.tsapps/api/src/modules/organization/planning-period.service.spec.tsapps/api/src/modules/organization/planning-period.service.tsapps/api/src/routes/availability.routes.tsapps/api/src/routes/index.tsapps/api/src/routes/organization.routes.tsapps/api/test/availability.routes.test.tsapps/api/test/helpers/fake-prisma.tsapps/api/test/organization.routes.test.tsapps/api/tsconfig.build.jsonapps/api/tsconfig.jsondocs/openapi.yamlpackages/shared/src/schemas/common.ts
|
CodeRabbit 리뷰 반영/미반영 정리입니다. 반영했습니다:
반영하지 않았습니다:
반영 커밋: |
개요
가능 시간 관리와 active schedule planning period API를 구현합니다.
Closes #92
변경 사항
변경 유형
feat— 새로운 기능fix— 버그 수정refactor— 리팩터링 (기능 변경 없음)test— 테스트 코드perf— 성능 개선docs— 문서 수정chore— 빌드·설정 변경영향 범위
web—apps/webapi—apps/apidb—packages/databaseshared—packages/sharedrepo— 루트 설정·CI/CD·문서테스트 방법
pnpm --filter @fragment/api testpnpm --filter @fragment/api typecheckpnpm --filter @fragment/api lintpnpm --filter @fragment/api build체크리스트
packages/shared) 업데이트 (타입 변경 시)리뷰 포인트
PUT /availability/bulk의 전체 교체 방식과 transaction 처리Summary by CodeRabbit