Skip to content
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

주차장 상세조회 #56

Merged
merged 14 commits into from
Apr 5, 2024
Merged

주차장 상세조회 #56

merged 14 commits into from
Apr 5, 2024

Conversation

youngh0
Copy link
Contributor

@youngh0 youngh0 commented Mar 27, 2024

👍관련 이슈

🤔세부 내용

  • parkingId 기반 주차장 상세조회입니다.
  • 예상요금, 예상도보시간, 즐겨찾기 값을 구하는건 주차장 반경조회 PR 에 구현되어 있어서 이 값들을 제외하고 우선 구현했습니다.
    • 주차장 목록 조회에서 이미 반환해주는 정보라 저는 상세조회에서는 빼도 괜찮지 않을까.. 라는 입장입니다.
  • 노션 api 스펙을 보면 평일 요금 정보, 주말 요금 정보를 보여주게 되어 있는데 저희 Parking 도메인은 기본 요금, 기본 시간 단위, 추가 요금, 추가 시간 단위만 저장하고 있습니다. 그래서 일단 기본 요금과 기본 시간 단위만 반환하도록 했습니다. 어떻게 할지 얘기 해봐야할듯 합니다.

🫵리뷰 중점사항

마지막 업데이트가 몇 분 전인지 구하는 로직 테스트

상세조회에서 단순 조회 말고 로직이 있는건 마지막 업데이트가 몇 분 전에 이뤄졌는지 구하는 기능입니다. 그런데, updatedAt 값이 JpaAuditing 에 의해서만 만들어져서 테스트가 어렵습니다.

그래서 JpaAuditing 에 (createdAt, updatedAt) 생성자를 생성하려 했는데 이렇게 하니 모든 도메인의 생성자에 (createdAt, updatedAt) 을 받는 생성자를 추가해서 여러분 의견을 반영해서 어떻게 할지 정하는게 맞는것같아서 일단 해당 테스트는 진행하지 못했습니다.

@youngh0 youngh0 requested review from This2sho and jundonghyuk March 27, 2024 16:29
Copy link

github-actions bot commented Mar 27, 2024

Test Results

129 tests   129 ✅  3s ⏱️
 26 suites    0 💤
 26 files      0 ❌

Results for commit 29b0092.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@jundonghyuk jundonghyuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 비지니스상 의미있게 쓰이는 createdAt, updatedAt같은 경우 어플리케이션에서 직접적으로 넣어주는 것을 선호하는 편입니다 ㅎㅎ

Comment on lines +43 to +51
void 주차장_상세조회() {
//given
String parkingName = "호이주차장";
Parking parking = makeParking(parkingName);
List<Parking> parkings = List.of(parking);
parkingService.saveAll(parkings);

Member member = new Member("하디", "email", "동혁", new Password("qwer1234"));
memberRepository.save(member);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MethodSource로 인자로 받아도 될것같아용

Copy link
Contributor

@This2sho This2sho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

비즈니스 로직이 실시간 주차대수 업데이트가 필요한거니까 update메소드에 updatedAt을 직접 변경해줘도 좋을거 같네유

private Integer lastUpdated;
private String tel;
private String paymentType;
private ReviewInfoResponse reviewInfo;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+ 운영시간

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

api 스펙에 없어서 놓쳤네여 추가햇슴다

import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RestController;

@RequiredArgsConstructor
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+Swagger

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

반영했슴다

@@ -23,4 +29,32 @@ public void saveAll(List<Parking> parkingLots) {
public Set<Parking> getParkingLots(Set<String> parkingNames) {
return parkingRepository.findAllByBaseInformationNameIn(parkingNames);
}

@Transactional(readOnly = true)
public ParkingDetailInfoResponse findParking(Long parkingId, LocalDateTime now) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LocalDateTime now를 인자로 받는 이유가 있나여?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

최대한 요청한 시각과 가까운 곳이 service 보단 controller 라고 생각해서 controller 에서 생성해서 넘겨줬습니다. 도메인에서 쓰는게 더 나아보이시나요?

사실 오차가 존재해도 무방한 기능이라 고민하긴 했어여

youngh0 added 5 commits April 2, 2024 00:22
# Conflicts:
#	src/main/java/com/example/parking/api/parking/ParkingController.java
#	src/main/java/com/example/parking/application/parking/ParkingService.java
#	src/main/java/com/example/parking/domain/parking/Parking.java
#	src/test/java/com/example/parking/fake/FakeParkingService.java
Copy link
Contributor

@This2sho This2sho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

리뷰 반영한거 확인했습니다
고생하셨어유 👍

@This2sho This2sho added 📝feature 기능 추가 🎅🏼deploy 해당 PR 머지시 배포 labels Apr 5, 2024
@This2sho This2sho merged commit 2d48a51 into main Apr 5, 2024
3 checks passed
@This2sho This2sho deleted the feat/55-find-parking branch April 5, 2024 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎅🏼deploy 해당 PR 머지시 배포 📝feature 기능 추가
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

주차장 상세조회
3 participants