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

[fix] 식당 제보 validation 검증 조건 추가 #169

Merged
merged 5 commits into from
Aug 22, 2024
Merged

Conversation

kgy1008
Copy link
Member

@kgy1008 kgy1008 commented Aug 20, 2024

Related Issue 📌

close #168

Description ✔️

  • 같은 건물 다른 층에 위치한 식당의 경우, 위 경도 값이 일치하는 문제가 발생하여 식당 이름을 검증 조건을 추가합니다.

To Reviewers

추가적으로 식당 삭제할 때 어떻게 처리할지에 대해 기획분들께 물어봤는데 릴리즈할 때까지 시간이 얼마 남지 않아 식당 삭제 제보가 들어오고 나서 재제보하는 것을 막지 않는다고 합니다. 때문에 식당 등록 로직에서 검증 로직 빼야할 것 같습니다.

if (storeExists(command.latitude(), command.longitude())) {
throw new BadRequestException(StoreErrorCode.BAD_STORE_INFO);
}

이 부분 삭제해야함다. (지금 저 로직이 포함되어 있으면 삭제된 식당에 대한 재제보가 막히게돼요!)

@@ -29,8 +29,8 @@ protected Store findByIdWithHeartAndIsDeletedFalse(final Long id) {
.orElseThrow(() -> new NotFoundException(StoreErrorCode.STORE_NOT_FOUND));
}

protected Optional<Store> findByLatitudeAndLongitudeWhereIsDeletedFalse(final double latitude, final double longitude) {
return storeRepository.findByPoint_LatitudeAndPoint_LongitudeAndIsDeletedFalse(latitude, longitude);
protected Optional<Store> findByLatitudeAndLongitudeAndNameWhereIsDeletedFalse(final double latitude, final double longitude, String name) {
Copy link
Contributor

Choose a reason for hiding this comment

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

통일성을 위해 name에도 final 키워드 붙이면 좋을 것 같아요!

@@ -92,7 +92,7 @@ private boolean hasSameUserId(final Long id, final Heart heart) {

@Transactional(readOnly = true)
public StoreDuplicateValidationResponse validateDuplicatedStore(final StoreValidationCommand command) {
return storeFinder.findByLatitudeAndLongitudeWhereIsDeletedFalse(command.latitude(), command.longitude())
return storeFinder.findByLatitudeAndLongitudeAndNameWhereIsDeletedFalse(command.latitude(), command.longitude(), command.name())
.map(store -> StoreDuplicateValidationResponse.of(store.getId(), universityStoreFinder.existsByUniversityIdAndStore(command.universityId(), store)))
Copy link
Contributor

Choose a reason for hiding this comment

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

이 부분 함수로 분리해서 가독성을 높이면 어떨지요,,.?

@@ -3,6 +3,7 @@
public record StoreDuplicateValidationRequest(
long universityId,
double latitude,
double longitude
double longitude,
String storeName
Copy link
Contributor

@PicturePark1101 PicturePark1101 Aug 21, 2024

Choose a reason for hiding this comment

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

여기서 @notblank과 같은 어노테이션 등등이 있으면 빈 문자열이나 null을 거를 수 있을 것 같아용

Copy link
Contributor

@Parkjyun Parkjyun left a comment

Choose a reason for hiding this comment

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

위경도가 겹치는 경우는 정말 드물게 일어나는 일이라 생각합니다.
그래서 먼저 위경도로만 검증 후 결과가 존재한다면 이때 추가적으로 이름을 검증하는 로직에 대해서는 어떻게 생각하세요?
이때 이름을 검증하는 로직은 그냥 stream 돌려서 해도 된다 생각합니다. 정말 많아야 2-3개 일테니..
어떻게 생각하시는지

@kgy1008 kgy1008 merged commit 981ef9e into develop Aug 22, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[fix] 식당 제보 validation 검증 조건 추가
3 participants