-
Notifications
You must be signed in to change notification settings - Fork 0
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
[Region] 지역 API 작업 완료 #4
Conversation
- 메소드 명 변경 - RestDocs 명세 변경 - DTO 패키지 내부로 이동
|
||
@Id | ||
@Column(name = "id", nullable = false) | ||
private Long id; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아이디 직접 넣어주시나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
원우님 이거 실수가 아니라 의도한 거였습니다.
데이터를 넣어주는 역할은 StudayServer 가 아니라 Studay_Data_Invocator 이고,
이쪽에서 그저 받아서 쓰기만 하기 때문에 @column으로 생성 자체를 막아두려는 의도입니다.
근데 저희 지금 테스트 특성 상 데이터를 넣어줄 필요가 있으니 setter 메소드가 따로 필요하거나 생성자가 따로 필요할 것 같은데 이 부분에 관해서 조금 고민이 필요할 것 같습니다.
@@ -7,7 +7,7 @@ HELP.md | |||
.gradle/ | |||
build/ | |||
studay_server.iml | |||
src/main/resources/application.yaml | |||
*.yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
헉.. 저도 설정했는데 나중에 conflict 처리해야겠군요 ㅠㅠ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ㅠㅠ 컨플릭트 싫은뎅 ㅠㅠ
import org.springframework.web.bind.annotation.RestController; | ||
|
||
@RestController | ||
@RequestMapping(path = "/regions", produces = APPLICATION_JSON_VALUE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저는 지금 produces
메서드에서 사용하고 있는데 이렇게 변경할까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
메서드단으로 합의!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오케!!!
@GetMapping(path = "/beopjungdong") | ||
public ResponseEntity<RegionResponse> getSubRegions( | ||
@RequestParam(required = false) String sido, | ||
@RequestParam(required = false) String sigungu | ||
) { | ||
if (sido == null) { | ||
return getSidoData(); | ||
} else if (sigungu == null) { | ||
return getSigunguData(sido); | ||
} | ||
return getUpmyeondongData(sido, sigungu); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
밑에 구현 여러 개 잘해놓으셔서
required = false
처리 안해도 괜찮지 않을까 싶은데
required = false
로 하신 이유가 있을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1안 (현재안)
/regions/beopjungdong?sido=XX&sigungu=XX
2안
/regions/beopjungdong/sido?sido=XX
/regions/beopjungdong/sigungu?sido=XX&sigungu=XX
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
세영님 고생하셨습니다!
코드 깔끔해서 너무 쉽게 쉽게 읽혔어요
@@ -28,7 +28,14 @@ public ResponseEntity<RegionResponse> getSigungus( | |||
@RequestParam(required = false) String sido, | |||
@RequestParam(required = false) String sigungu | |||
) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
서비스 단으로 가기 전에 끝에 커스텀 어노테이션을 통해서 "시도"로 끝나는지 "시군구"로 끝나는지 확인해 보는 것은 어떨까요? @ValidSido
, @ValidSigungu
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오 좋네요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ValidSido
: 서울특별시, 경기도 아니면 날림 (null 인 건 괜찮음)
@ValidSigungu
: 시, 군, 구 포함되지 않으면 날림 (Null 인 건 괜찮음)
@ValidUpmyeondong
: 읍, 면, 동, 군, 구 포함되지 않으면 날림
법정동
위치