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

feat: 특정 맵 > 전체 공간에 대한 사용가능여부 조회 API 추가 #890

Merged
merged 11 commits into from
Feb 13, 2023

Conversation

sakjung
Copy link
Collaborator

@sakjung sakjung commented Jan 28, 2023

구현 기능

  • 특정 datetime slot (start ~ end) 이 주어질 때, 전체 공간의 사용 가능 여부를 조회하는 API 추가

기타

Close #888

@sakjung sakjung self-assigned this Jan 28, 2023
@sakjung sakjung changed the title feat: 특정 맵 > 전체 공간에 대한 사용여부 조회 API 추가 feat: 특정 맵 > 전체 공간에 대한 사용가능여부 조회 API 추가 Jan 28, 2023
Copy link
Collaborator

@tributetothemoon tributetothemoon left a comment

Choose a reason for hiding this comment

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

신경 못 쓰는 사이 시간 관련 도메인 양이 많아져 맥락 파악이 잘 안 되어... 주요 로직상에 코멘트를 못 달아 죄송함다 삭😭.
다만 모르는 사이에 전만적으로 포매팅이 많이 헝클어진 느낌이 있어서 조금만 봐주시면 좋겠어욥!
도메인 파악은 차근차근 해보도록 하겠습ㄴ디ㅏ...

xrabcde
xrabcde previously approved these changes Feb 8, 2023
Copy link
Collaborator

@xrabcde xrabcde left a comment

Choose a reason for hiding this comment

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

리뷰가 많이 늦었네요!!!! 수고하셨ㅆ브니다 !!!!

Comment on lines +119 to +120
.filter(reservation -> reservationTime.hasConflictWith(reservation.getReservationTime()))
.map(Reservation::getSpace)
Copy link
Collaborator

Choose a reason for hiding this comment

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

일부러 hasConflictWith() = true 인 공간 (== 예약이 이미 있는 공간)들을 뽑은 다음
전체 공간에서 뺴주는 건가욤?? 조금 로직이 복잡하긴 하네ㅠㅜ

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

정확히 말하면 전체 공간에서 빼주는 건 아니구, 전체공간 루프돌면서 사용 가능 여부 판별하기 위한 용 입니다.
이전에 만들어둔 메서드 hasConflictWith가 있어서 이거 활용해서 occupiedSpaces로 뽑아내는게 자연스러울 것 같아 이렇게 구현했습니다~

.map(Reservation::getSpace)
.collect(Collectors.toSet());

return SpaceFindAllAvailabilityResponse.of(mapId, allSpaces, occupiedSpaces);
Copy link
Collaborator

@xrabcde xrabcde Feb 8, 2023

Choose a reason for hiding this comment

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

서비스에서 전체공간(allSpaces) - 예약이 불가능한 공간(occupiedSpaces) 를 계산해서 Dto 에 넘겨주는 건 어떄여?? Dto에서 allSpaces 를 다 알아야할 필요가 있을가 싶은데..

+) 조회만 하는 함수면 @Transactional(readOnly = true) 추가해주세욥

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

저도 DTO에 저 로직이 들어가는게 싫어서 짤 때 고민을 좀했는데... response의 isAvailable 필드는 allSpaces 상대로 값이 들어가야해서 allSpaces를 알긴 해야하더라구요 ㅠ 아니면 Service에서 로직처리해서 Map<Space, Boolean> 같은식으로 넘기는 게 좋을까요? 의견쩜...

@xrabcde
Copy link
Collaborator

xrabcde commented Feb 9, 2023

@sakjung 공간의 예약조건을 검증한 뒤, 예약 가능여부 검증하도록 검증로직 추가되어야 할 것 같습니당

@sakjung sakjung merged commit 820b833 into dev Feb 13, 2023
@sakjung sakjung deleted the feat/888-space-occupancy branch February 13, 2023 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BE] 특정 날짜 특정 시간대에 맵의 전체 예약 가능 여부 내려주는 API
4 participants