[Phase 1] map viewport contract - #54
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughPhase 1 adds a zoom-aware ChangesMap Viewport API Contract (Phase 1)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
backend/src/test/java/com/ssafy/salmanhae/controller/map/MapViewportControllerTest.java (1)
23-81: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd explicit zoom boundary tests for the controller contract.
Please add assertions for
zoom=0(SIGUNGU_AVG),zoom=21(PROPERTY_MARKER), and out-of-rangezoom=22returningINVALID_REQUESTto lock down@Min/@Maxboundary behavior.🤖 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 `@backend/src/test/java/com/ssafy/salmanhae/controller/map/MapViewportControllerTest.java` around lines 23 - 81, The MapViewportControllerTest coverage is missing explicit zoom boundary checks for the controller contract. Extend the existing `getViewport...` tests to assert that `zoom=0` returns `SIGUNGU_AVG`, `zoom=21` returns `PROPERTY_MARKER`, and `zoom=22` is rejected with `INVALID_REQUEST`, using the same `baseViewportRequest()`/`mockMvc.perform(...)` pattern so the `@Min/@Max` behavior is locked down in the controller tests.
🤖 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 `@docs/08_API_SPEC.md`:
- Around line 168-178: The example payload in the API spec is still mixing
PROPERTY_CLUSTER data into the PROPERTY_MARKER response, which conflicts with
the split mode contract. Update the relevant examples in the spec so the
PROPERTY_MARKER example contains only individual marker items, and keep any
clustered items shown only in the PROPERTY_CLUSTER example or section. Use the
mode labels PROPERTY_CLUSTER and PROPERTY_MARKER to verify the examples match
the intended payload shape.
In `@phases/map-viewport-zoom/phase1-map-viewport-contract.md`:
- Around line 15-20: The phase 1 checklist is still marked incomplete even
though the phase status says it is done, so update the checklist in
phase1-map-viewport-contract.md to match the implemented/completed state. Make
the entries for the GET /api/v1/map/viewport accessibility, zoom threshold
behavior, bounds validation, backend controller tests, and API spec reflect
completion so the checklist and phase status stay consistent.
---
Nitpick comments:
In
`@backend/src/test/java/com/ssafy/salmanhae/controller/map/MapViewportControllerTest.java`:
- Around line 23-81: The MapViewportControllerTest coverage is missing explicit
zoom boundary checks for the controller contract. Extend the existing
`getViewport...` tests to assert that `zoom=0` returns `SIGUNGU_AVG`, `zoom=21`
returns `PROPERTY_MARKER`, and `zoom=22` is rejected with `INVALID_REQUEST`,
using the same `baseViewportRequest()`/`mockMvc.perform(...)` pattern so the
`@Min/@Max` behavior is locked down in the controller tests.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4517fee2-ca13-4853-8c97-286f0f9ce93e
📒 Files selected for processing (22)
.gitignorebackend/src/main/java/com/ssafy/salmanhae/controller/map/MapViewportController.javabackend/src/main/java/com/ssafy/salmanhae/model/dto/map/MapViewportItemResponse.javabackend/src/main/java/com/ssafy/salmanhae/model/dto/map/MapViewportItemType.javabackend/src/main/java/com/ssafy/salmanhae/model/dto/map/MapViewportMode.javabackend/src/main/java/com/ssafy/salmanhae/model/dto/map/MapViewportRequest.javabackend/src/main/java/com/ssafy/salmanhae/model/dto/map/MapViewportResponse.javabackend/src/main/java/com/ssafy/salmanhae/model/dto/map/PropertyClusterViewportItem.javabackend/src/main/java/com/ssafy/salmanhae/model/dto/map/PropertyViewportItem.javabackend/src/main/java/com/ssafy/salmanhae/model/dto/map/RegionAverageViewportItem.javabackend/src/main/java/com/ssafy/salmanhae/service/map/MapViewportService.javabackend/src/main/java/com/ssafy/salmanhae/service/map/MapViewportServiceImpl.javabackend/src/test/java/com/ssafy/salmanhae/controller/map/MapViewportControllerTest.javabackend/src/test/resources/.env.propertiesbackend/src/test/resources/application-test.propertiesdocs/08_API_SPEC.mdphases/map-viewport-zoom/phase1-map-viewport-contract.mdphases/map-viewport-zoom/phase1.status.jsonphases/map-viewport-zoom/phase2-region-and-cluster-query.mdphases/map-viewport-zoom/phase3-frontend-map-viewport-api.mdphases/map-viewport-zoom/phase4-frontend-zoom-rendering.mdphases/map-viewport-zoom/phase5-qa-docs-deploy.md
변경 내용
/api/v1/map/viewport계약과 최소 구현 추가SIGUNGU_AVG,DONG_AVG,PROPERTY_CLUSTER,PROPERTY_MARKER로 분기docs/08_API_SPEC.md에 threshold 정책 반영phases/map-viewport-zoomphase 문서 5개 작성연결 이슈
closes #53
테스트
./mvnw.cmd -q -Dtest=MapViewportControllerTest test./mvnw.cmd -q -Dtest=PropertyControllerTest,MapViewportControllerTest test./mvnw.cmd -q testSummary by CodeRabbit
GET /api/v1/map/viewportendpoint that returns amode, viewport items, andtotalCountbased on zoom (region average → cluster → property markers).400with error codes).