Skip to content

Feature/#33 문항세트 단일 조회, 검색 구현 및 변경사항 반영#35

Merged
sejoon00 merged 9 commits intodevelopfrom
feature/#33
Feb 8, 2025
Merged

Feature/#33 문항세트 단일 조회, 검색 구현 및 변경사항 반영#35
sejoon00 merged 9 commits intodevelopfrom
feature/#33

Conversation

@seokbeom00
Copy link
Contributor

🌱 관련 이슈

📌 작업 내용 및 특이사항

문항세트 조회

image
  • 문항세트 각각을 조회하는 api입니다.
  • 현재 조회 문항세트ID를 기준으로 해당 문항세트에 속한 문항들, 해당 문항들에 속한 태그명들을 각각 select한 후 하나의 DTO로 합치는 형식입니다.
  • 필요 이상의 쿼리가 발생하기 때문에(N+1문제) 추후 QueryDsl 등의 동적 쿼리로 변경할 예정입니다.

문항세트 검색

image
  • 문항세트 타이틀(포함), 문항의 코멘트(포함), 문항 내 개념태그를 기준으로 문항세트를 필터링하는 api입니다.
  • 문항 검색 api와 유사한 방식의 QueryDsl을 적용하여 N+1 문제를 방지하고, 조회하도록 구현했습니다.

변경 사항

  • 문항세트 name -> 문항세트 title
  • 문항세트 등록, 수정 시 문항 리스트가 비어있는지의 유효성 검사만 진행
  • 문항세트 title 빈값 -> "제목 없음"으로 자동 지정
  • 문항세트 내 문항 순서 변경 api swagger에서 삭제

@seokbeom00 seokbeom00 added Type: Test [이슈 목적] 테스트 코드 추가, 수정 Type: Feature [이슈 목적] 새로운 기능 추가 labels Feb 7, 2025
@seokbeom00 seokbeom00 self-assigned this Feb 7, 2025
Copy link
Contributor

@sejoon00 sejoon00 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!

Copy link
Contributor

Choose a reason for hiding this comment

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

복수로 조회이니 findAllByIdsElseThrow 네이밍은 어떨까요?
In은 중간에 오타난것 같아요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

포함의 의미를 담으려고 In을 쓴건데 findAllByIdsElseThrow이 더 직관적이네요! 반영하겠습니다

public ResponseEntity<List<ProblemSetSearchGetResponse>> search(
@RequestParam(value = "problemSetTitle", required = false) String problemSetTitle,
@RequestParam(value = "problemTitle", required = false) String problemTitle,
@RequestParam(value = "conceptTagNames", required = false) List<String> conceptTagNames
Copy link
Contributor

Choose a reason for hiding this comment

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

image 문항 메모가 들어가야될 것 같아요

Copy link
Contributor

Choose a reason for hiding this comment

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

문항 변동 사항 반영 전이라 아직 problem에 메모가 없는데 이후에 수정이 나을까요?

Copy link
Contributor

Choose a reason for hiding this comment

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

comment가 memo로 바뀌는걸로 알고 있습니다

Copy link
Contributor Author

Choose a reason for hiding this comment

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

문항 title외에도 문항memo 필터링 요소가 추가되는 것일까요?
만약 그렇다면, 추가된 이후에 수정하겠습니다!

Copy link
Contributor

Choose a reason for hiding this comment

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

해당 로직은 비즈니스 로직이라고 생각이들어서 set안으로 넣는게 좋을 것 같아요.
Title이라는 클래스를 만들어서 문항에서도 사용할 수 있게 하는거 어떨까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Title 클래스 만들어서 도메인 로직에 추가하겠습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

일단 problemSet.domain에 추가해서 적용했습니다!

Copy link
Contributor

Choose a reason for hiding this comment

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

) { 한칸 내리는게 어떨까요 컨밴션도 한번 정해봐요 우리

Copy link
Contributor Author

Choose a reason for hiding this comment

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

좋습니다!

Copy link
Contributor

Choose a reason for hiding this comment

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

문항 정보를 가져오는 로직은 메서드로 분리하면 가독성이 더 좋을것 같아요.
혹시 summary에서 sequence가 필요한 이유가 있나요? list 순서 그대로 주면 index가 필요없 stream으로 변환할 수도 잇을 것 같아요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

제가 실수했습니다 sequence 삭제하겠습니다
메서드 분리는 추후 쿼리로 변경하면서 좀 더 깔끔하게 해볼게요!

@sejoon00 sejoon00 merged commit 7189a70 into develop Feb 8, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: Feature [이슈 목적] 새로운 기능 추가 Type: Test [이슈 목적] 테스트 코드 추가, 수정

Projects

None yet

Development

Successfully merging this pull request may close these issues.

✨ 문항세트 단일 조회 및 검색 구현

2 participants

Comments