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: CHAT-179-BE-태그-검색-API #22

Merged
merged 10 commits into from
Jan 30, 2024

Conversation

Mouon
Copy link
Member

@Mouon Mouon commented Jan 21, 2024

요약 (Summary)

  • 태그를 기반으로 일기 검색 기능을 구현
  • TagSearchController, TagSearchService, TagSearchRepository TagSearchResponse 클래스를 작성
  • 사용자는 태그 이름을 기반으로 일기를 검색할 수 있고, 일기의 세부 정보와 함께 결과가 반환

변경 사항 (Changes)

TagSearchController 클래스: 태그 기반 검색(필터링)을 위한 API 엔드포인트(diary/search_tag)를 제공합니다.
TagSearchService 클래스: TagSearchRepository를 사용하여 태그 이름에 따라 일기를 검색하는 비즈니스 로직을 처리.
TagSearchRepository 클래스: JPA의 EntityManager를 사용하여 데이터베이스에서 태그에 해당하는 일기를 검색.
TagSearchResponse DTO: 일기의 ID, 제목, 내용, 날짜, 태그 목록, 사진 목록을 포함.(상태나 수정,생성 일시는 제외하였습니다.)
TagSearchRepository 클래스: Tag를 리스트로 받아서 중복 선택 가능하게

리뷰 요구사항

  • TagSearchController의 REST API 설계가 적절한지 확인해 주세요.
  • TagSearchResponse DTO가 필요한 모든 필드를 적절하게 포함하고 있는지 확인해 주세요.
  • API가 제대로 작동하는지 확인해주세요. (테스트 코드로 확인했습니다.)
  • 쿼리 진짜 잘 모르겠습니다. 자꾸 원하는 데로 안되서 HAVING BY를 사용했는데 이게 잘 작동하는지 검토해주세요

확인 방법 (선택)

스크린샷 2024-01-21 오후 1 09 27

mock을 이용해서 테스트를 진행하였습니다.

스크린샷 2024-01-22 오전 2 54 01 더미 데이터를 이용한 테스트 또한 완료했습니다.

http://localhost:8080/diary/list/tag?tagName=가족&tagName=기쁨

쿼리는 다인님 일기상세 쿼리를 이용했습니다.( #14 )

<sample diary 정보 넣는 SQL>
INSERT INTO member (email, password, create_at, update_at)
VALUES ("aa@aa", "123", NOW(), NOW());

INSERT INTO diary (user_id, diary_date, title, content,create_at, update_at)
VALUES (1,"20240101","1title", "1content", NOW(), NOW()),
(1,"20240102","2title", "2content", NOW(), NOW());

INSERT INTO tag (category, tag_name)
VALUES ("감정", "기쁨"), ("감정", "슬픔"), ("인물", "친구"), ("인물", "가족");

INSERT INTO diarytag (diary_id, tag_id)
VALUES (1,1),(1,2),(1,3),(1,4),(2,3),(2,4);

INSERT INTO chat (sender, content, chat_type, user_id, create_at)
VALUES ("USER", "", "IMG", 1, NOW()),("USER", "", "IMG", 1, NOW()),("LULU", "", "CHAT", 1, NOW()),("CHICHI", "", "CHAT", 1, NOW());

INSERT INTO photo (image_url)
VALUES ("1imageUrl"), ("2imageUrl");

INSERT INTO diaryphoto (diary_id, photo_id)
VALUES (1, (SELECT photo_id FROM photo WHERE image_url = "1imageUrl")),
(1, (SELECT photo_id FROM photo WHERE image_url = "2imageUrl"));

Copy link
Contributor

@nonaninona nonaninona left a comment

Choose a reason for hiding this comment

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

코드에 대한 리뷰에 앞서서 캘린더 API와 더불어 얘기하기 위해서
API 명세에 대한 얘기를 하겠습니다

저는 캘린더도 그냥 지금 작성된 태그 검색과 같은 형식으로
진행하면 된다고 생각한 거였습니다.

태그 검색은 해당하는 일기의 List를 보내주는 것 처럼
캘린더는 날짜마다 존재하는 sender들의 정보를 List로 보내는 것이 좋겠다고
말한겁니다.

얘는 바로 코드 리뷰 이어서 할게용~

Copy link
Contributor

@nonaninona nonaninona left a comment

Choose a reason for hiding this comment

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

우선... 까다로운 내용이 될 수 있다고 생각하는데(쿼리문 작성시)
디자인 보면 아시겠지만 태그를 여러개 적용할 수 있습니다
그래서 TagName을 여러개(아마 List)로 받아야해요..
이것만으로 매우 큰 수정이 되지않을까 싶습니다

저걸 제외하고는 현재 상태에서 종합적으로 크게 문제되는 부분이 코드에 없어보입니다!
API 명세서는 템플릿을 복사해서 활용하시길 바랍니다
리뷰 내용은 대부분 필수가 아니고 권장하는 개선점들입니다

요약

  1. Tag 여러개를 이용한 검색이 가능하다. 큰 수정 필요
  2. API 명세서 템플릿 복사해서 작성하길 바람. 서식 통일 필요
  3. 리뷰 내용은 대부분 개선점에 대한 내용. 수정 필수 아님

Copy link
Contributor

@nonaninona nonaninona left a comment

Choose a reason for hiding this comment

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

태그 복수로 검색 기능
저는 문제 없다고 생각합니다!

이제 남은 코멘트들 잘 갈무리하셔서 반영하시면 될 것 같습니다~

@Kjiw0n
Copy link
Collaborator

Kjiw0n commented Jan 27, 2024

스크린샷 2024-01-27 12 14 50

테스트 결과 api 정상 작동하는 것 확인하였습니다! 수고하셨습니다 :)

Copy link
Collaborator

@dainshon dainshon left a comment

Choose a reason for hiding this comment

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

수고하셨어요:)


@RestController
@RequestMapping("diary")
public class TagSearchController {
Copy link
Collaborator

Choose a reason for hiding this comment

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

상위 주소가 diary/ 라면 컨트롤러 이름을 DiaryController로 바꾸는게 나을것같습니다!
아니면 TagController로 하고 상위 주소를 tag/ 로 바꾸는것도 좋을것같네용

Copy link
Member Author

Choose a reason for hiding this comment

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

월별이랑 태그검색둘다 diarycontroller로 들어갈거같은데 그러면 충돌날거같아서요
그래서 파일이름을 어떻게해야할지 모르겠었습니다..

private List<String> photoList;

@Builder
public TagSearchResponse(Diary diary){
Copy link
Collaborator

Choose a reason for hiding this comment

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

TagInfoDTO를 따로 빼는거 좋은 방법인 것 같습니다:)

정한 네이밍 규칙을 따르기위해

  1. TagSearchResponse뒤에 DTO 붙여주시구
  2. dto내에 하위 패키지 만들어서 사용해주세요!

Copy link
Member Author

Choose a reason for hiding this comment

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

태그 검색을 위한 DTO 작성
태그 검색을 위한 레포지토리 작성, DTO 인스턴스를 반환하도록 설계
태그 이름을 통해 레포지토리를 호출하도록 설계
태그 이름을 파라매터로 받아서 서비스를 호출하도록 설계
status나 createAt등은 필요 없을 것으로 판단
Tag 엔티티 직접 가져오려니 데이터 더 추가시 순환 참조 오류 발생 우려있어서
TagDTO 추가
쿼리는 대폭 수정되었고 나머지는 자료형정도만 수정되었습니다.
파일 구조 또한 변경
diaryphoto가 추가됨에 따라 쿼리 로직 수정
레포지 쪽 대폭 수정
엔티티 나중에 머지 필요
Copy link
Contributor

@nonaninona nonaninona left a comment

Choose a reason for hiding this comment

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

바뀐 ERD로 rebase 해두었습니다

@nonaninona nonaninona merged commit 88f72d5 into develop Jan 30, 2024
1 check passed
@nonaninona nonaninona deleted the CHAT-179-BE-태그-검색-API branch January 30, 2024 09:52
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.

4 participants