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

[BE] feat: 놀이터 참여하기 #637

Merged
merged 52 commits into from
Oct 18, 2024
Merged

Conversation

ehtjsv2
Copy link
Contributor

@ehtjsv2 ehtjsv2 commented Oct 10, 2024

이슈

개발 사항

  • 이슈내용과 같습니다. 이슈가 좀 많지만, 기능이 복잡한 것은 없습니다

@ehtjsv2 ehtjsv2 requested a review from jimi567 as a code owner October 10, 2024 08:55
Copy link
Contributor

@takoyakimchi takoyakimchi 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 151 to 154

// then
assertThat(isPlaygroundMemberPresent).isFalse();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// then
assertThat(isPlaygroundMemberPresent).isFalse();
}
// then
assertThat(playgroundMemberRepository.findById(playgroundMember.getId())).isEmpty();
}

Comment on lines 97 to 103
double latitude299mFromA = GeoCalculator.calculateLatitudeOffset(latitudeA, 299);
double latitude299mFromA = GeoCalculator.calculateLatitudeOffset(latitudeA, 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

299 해도 안터지네용

Comment on lines 351 to 353

@DisplayName("놀이터에 참여한 멤벙 메세지 수정")
@Test
Copy link
Contributor

Choose a reason for hiding this comment

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

귀여운 오타 발견

Comment on lines 258 to 260
1,
List.of("url1", "url2", "url3")
);
Copy link
Contributor

Choose a reason for hiding this comment

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

문서화가 좀 더 친절하면 좋을듯해요!!
URL 형식으로 넣어주면 어떨까요??

Comment on lines 25 to 26
private static final int MAX_PET_IMAGE_COUNT = 5;

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private static final int MAX_PET_IMAGE_COUNT = 5;
private static final int MAX_PET_PREVIEW_IMAGE_COUNT = 5;

상수이름 제안입니다

Comment on lines 3 to 7
public record UpdatePlaygroundMemberMessageRequest(
String message //todo: 검증추가
) {

}
Copy link
Contributor

Choose a reason for hiding this comment

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

검증 추가 하는건가요 ?!

takoyakimchi
takoyakimchi previously approved these changes Oct 18, 2024
Copy link
Contributor

@takoyakimchi takoyakimchi left a comment

Choose a reason for hiding this comment

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

굿 굿 확인했습니다.

petImages = cutPeImagesCount(petImages);
petImages = cutPetImagesCount(petImages);
Copy link
Contributor

Choose a reason for hiding this comment

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

오타 수정 굿

Comment on lines +123 to +125
if (!isInsideBoundary) {
playgroundMember.updateExitTime(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.

사용자가 놀이터에 도착한 이력이 없더라도 놀이터 범위 밖에서 update API를 호출하면 exitTime이 갱신되네요!
지금은 문제가 없겠지만, 추후에 exitTime으로 무언가 처리하는 로직이 추가된다면 문제가 생길 수도 있다는걸 인지하는게 좋겠어요.

Comment on lines +63 to +66
playgroundPetDetails.sort(Comparator
.comparing(PlaygroundPetDetail::isMine).reversed()
.thenComparing(Comparator.comparing(PlaygroundPetDetail::isArrival).reversed())
);
Copy link
Contributor

Choose a reason for hiding this comment

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

좋네요! 그런데 두개의 Comparator의 개행을 맞춰주면 더 가독성이 올라갈 것 같아요~

Comment on lines 123 to 124
petImages = cutPetImagesCount(petImages);
}
Copy link
Contributor

@J-I-H-O J-I-H-O Oct 18, 2024

Choose a reason for hiding this comment

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

Suggested change
petImages = cutPetImagesCount(petImages);
}
}
petImages = cutPetImagesCount(petImages);

@ehtjsv2 ehtjsv2 merged commit 0fc0d88 into develop Oct 18, 2024
3 checks passed
@J-I-H-O J-I-H-O deleted the feature/be-playground-join branch October 18, 2024 07:23
takoyakimchi pushed a commit that referenced this pull request Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants