Conversation
|
✅ PR 검증 성공 브랜치: 모든 검증을 통과했습니다. 리뷰를 요청해주세요. |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the DeviceTokenController to use a standardized ApiResponse wrapper for all endpoint return values, replacing the previous inconsistent response formats (HashMap for register, direct DTOs for other endpoints).
Key changes:
- All endpoint methods now return
ResponseEntity<ApiResponse<T>>instead of varied return types - Removed HashMap-based response construction in favor of
ApiResponse.success() - Added consistent success messages for all operations
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| */ | ||
| @PostMapping("/register") | ||
| public ResponseEntity<Map<String, String>> registerToken( | ||
| public ResponseEntity<ApiResponse<UserDeviceInfo>> registerToken( |
There was a problem hiding this comment.
Inconsistent API response pattern across controllers. AlarmController returns ApiResponse<T> directly without wrapping in ResponseEntity, while this controller wraps it in ResponseEntity<ApiResponse<T>>. Both NotificationController and this controller use ResponseEntity<ApiResponse<T>>, but AlarmController doesn't. Consider standardizing the approach across all controllers for consistency.
| "FCM 토큰이 성공적으로 등록되었습니다.", | ||
| HttpStatus.OK |
There was a problem hiding this comment.
The HttpStatus.OK parameter is redundant when already using ResponseEntity.ok(). Since ResponseEntity.ok() sets the status to 200 OK, passing HttpStatus.OK to the ApiResponse creates duplication. Consider using a different constructor or removing the status parameter from ApiResponse.success() for 200 OK responses, or use ResponseEntity.status(status).body() if you need to vary the status code.
| "FCM 토큰이 성공적으로 등록되었습니다.", | |
| HttpStatus.OK | |
| "FCM 토큰이 성공적으로 등록되었습니다." |
요약
주요 변경 사항
관련 이슈
closes #21
테스트/검증
확인 사항
스크린샷/로그(선택)