feat : 스터디 그룹 공개 여부 설정 API 추가 #144
Conversation
RESTful API 원칙 상 엔드포인트에 자원을 쓰는게 맞긴 합니다! 따라서 형용사, 동사보단 명사를 쓰는게 맞아서 visibility가 그래도 제일 알맞는 것 같기도 하네요 |
| @@ -34,16 +34,22 @@ public class GroupMember { | |||
| private LocalDate joinDate; | |||
| @Enumerated(EnumType.STRING) | |||
| private RoleOfGroupMember role; | |||
| private boolean isPublic; | |||
There was a problem hiding this comment.
Boolean + @NotNull 겁시다~
그리고 필드 이름이랑, 엔드포인트 이름이랑 어느정도 통합하게 isVisible 정도 어때요?
There was a problem hiding this comment.
오 isVisible 좋네요! 수정해야겠어요 의견 감삼다
There was a problem hiding this comment.
Boolean +
@NotNull겁시다
갑자기 생각난건데요 왜 엔티티에는 primitive type으로 안쓰나요?
There was a problem hiding this comment.
primitive 타입은 null을 소화할수 없어서 그렇습니다
ex) int age 했는데 db에 널 들어있는 경우
There was a problem hiding this comment.
null이 불가능하단건 알고있었는데, reference type + NotNull을 하게 되면 결국 primitive type과 같아지는게 아닌건가요??
There was a problem hiding this comment.
넵 맞긴합니다~
그냥 표현의 차이긴 합니다. primitive 쓴다고 문제가 생기진 않아서, 작업할때 원하는 방향 가도 되는 부분입니다
다만 notnull은 꼭 걸어줘야겠네용
| .orElseThrow(() -> new GroupMemberValidationException(HttpStatus.FORBIDDEN.value(), "참여하지 않은 그룹입니다.")); | ||
|
|
||
| member.updateVisibility(request.isPublic()); | ||
| log.info("success to update group visibility"); |
There was a problem hiding this comment.
[nit] 로깅에 식별자정도 남기면 좋을 것 같아요~
- isPublic -> isVisible 수정
- boolean -> Boolean 수정 - NotNull 추가
📌 Related Issue
close #143
🚀 Description
isPublic을 추가해 공개 여부를 전달합니다.GroupVisibility엔티티를 또 팔까 했는데, 새로 만들어도 추가될 필드가isPublic하나이기도하고 확장 가능성도 적다고 생각해 기존의GroupMember에만 추가했습니다.📢 Review Point
/visibility로 되어있습니다. 내부 필드명은isPublic으로 되어있는데, visibility로 통일하는 것이 더 좋은 설계일지 아님 서로 다르더라도 의미를 잘 보이는 네이밍을 하는게 나은지 고민입니다.📚Etc (선택)