Skip to content

feat: 게시물 통계 API 구현#40

Open
seoyoon047 wants to merge 6 commits into
developfrom
feature/28-statistics
Open

feat: 게시물 통계 API 구현#40
seoyoon047 wants to merge 6 commits into
developfrom
feature/28-statistics

Conversation

@seoyoon047

@seoyoon047 seoyoon047 commented Oct 30, 2023

Copy link
Copy Markdown
Collaborator

🚀 풀 리퀘스트 요약

  • 요구 사항에 따라 게시물 통계 API 구현했습니다
  • 쿼리 파라미터로 Start, End기간을 설정해서 해당 기간의 게시물을 볼 수 있습니다

📋 변경 사항

  • 새로운 기능 추가
  • 코드에 영향을 주지 않는 변경사항(오타 수정, 탭 사이즈 변경, 변수명 변경 등)
  • 코드 리팩토링

📎 관련 이슈

#37

@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "user_id", nullable = false)
private User userId;
@OneToMany(mappedBy = "statistics")

@hyerijang hyerijang Oct 30, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
@OneToMany(mappedBy = "statistics")
@OneToOne
Caused by: org.hibernate.AnnotationException: Property 'com.skeleton.feed.entity.Statistics.post' is not a collection and may not be a '@OneToMany', '@ManyToMany', or '@ElementCollection'

// 컬렉션이 아닌 곳에 @OneToMany 사용해서 발생하는 문제

서윤님 지금 Statistics와 Post의 연관 관계 어노테이션이 잘못 설정되어 있어서 애플리케이션 실행 시 위와 같은 에러가 발생합니다. PR 하시기 전에는 애플리케이션이 동작하는 지 확인 하신 후 PR 해주시면 감사하겠습니다. 😂

@O0oO0Oo O0oO0Oo requested review from O0oO0Oo and removed request for O0oO0Oo October 30, 2023 12:33

@hyerijang hyerijang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

StatisticsCalculator에 @component를 써서 싱글톤으로 관리하시는 등, 전체적으로 많이 고민하신게 느껴집니다. 다만, 코드가 복잡하고 양이 많은 만큼 주석/ 이슈/ PR 등을 활용하여 어떤 메소드이고 왜 이렇게 했는지 정도는 알려주시면 좀 더 읽기가 쉬울 것 같습니다.

}

public StatisticsResponse getStatistics(StatisticsRequest request, Authentication authentication) {
String userId = ((UserDetails) authentication.getPrincipal()).getUsername();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
String userId = ((UserDetails) authentication.getPrincipal()).getUsername();
String userEmail= ((UserDetails) authentication.getPrincipal()).getUsername();

userEmail이 좀 더 정확한 명칭인 것 같습니다.

애초에 User 엔티티 의 id는 Long 타입이라서 타입이 맞지 않고,

getStatistics 메서드 내 filter에서는 String userId를 getEmail과 비교합니다. 이걸 본 후에야 해당 String이 User엔티티의 Id(PK)가 아닌 이메일 이라는 걸 알 수 있었네요. 😅

  • filter(stat -> userId .equals(stat.getUserId().getEmail())

오해를 줄이기 위해 다음부터는 명명에 좀 더 신경써주시면 좋겠어요👍

Comment on lines +35 to +36
@Temporal(TemporalType.TIMESTAMP)
private Date postTime;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

이 필드는 왜 만드신건지 궁금합니다. 또, postTime이란 건 정확히 무슨 의미인가요?

만약 statistic의 생성/수정일자가 궁금한거라면 common.entity.BaseTimeEntity를 상속 받아서 하셨으면 될 것 같습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants