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: 리뷰 목록 재구현 #293

Merged
merged 14 commits into from
Aug 11, 2024
Merged

Conversation

skylar1220
Copy link
Contributor


🚀 어떤 기능을 구현했나요 ?

  • 바뀐 도메인과 API에 따라 리뷰 목록을 재구현했습니다!

🔥 어떻게 해결했나요 ?

  • 카테고리만 가져오는 등의 필터링 작업을 위해 OptionItem에 OptionType 컬럼을 추가하였습니다.

📝 어떤 부분에 집중해서 리뷰해야 할까요?

  • 현재는 List 속에 첫질문(카테고리 선택), 꼬리질문(키워드 선택) selectedOptionId가 섞여있습니다. 따라서 리뷰 목록에서 카테고리 선택지만 가져오기 위해서는 selectedOptionId를 갖고 DB에 접근해서 OptionType이 CATEGORY인지를 확인하고 있습니다. 따라서, 리뷰 목록을 응답하기 위해서 리뷰 갯수만큼 exist 확인 쿼리가 나갑니다.
  • 약속한 API 문서 응답 내에서 작업하기 위해 우선 이렇게 구현하였는데, 리뷰 작성 요청에서 선택된 optionId와 함께 optionType을 함께 받아서 CheckBoxAnswer에 OptionType 컬럼을 추가한다면 DB에 접근하지 않고 리뷰 목록을 위한 카테고리 선택지만 가져오는 것이 쉬울 것 같아 논의해보고 싶습니다!

📚 참고 자료, 할 말

  • 용어 통일이 필요할 것 같습니다. 첫번째 화면 선택지는 카테고리, 꼬리 질문 선택지는 키워드라고 일단 통일하면 될까요?

Copy link

github-actions bot commented Aug 10, 2024

Test Results

26 tests  +1   26 ✅ +1   1s ⏱️ ±0s
11 suites ±0    0 💤 ±0 
11 files   ±0    0 ❌ ±0 

Results for commit 18a3911. ± Comparison against base commit 013d2b0.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@nayonsoso nayonsoso 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 32 to 34

@Column(name = "review_id", insertable = false, updatable = false)
private long reviewId;
Copy link
Contributor

Choose a reason for hiding this comment

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

CheckBoxAnswer가 reviewId 를 갖게 추가한 이유가 있나요? Review2 가 가지고 있는 List CheckBoxAnswer 만으로는 충분하지 않은 상황이 있었나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

실험삼아 넣었다가 안뺐네요😂 바로 빼겠습니다~~

@@ -17,4 +18,15 @@ public String generatePreview(List<ReviewContent> reviewContents) {
}
return answer;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

이 부분 이제 없애주세용~

.stream()
.filter(answer -> optionRepository.existsByOptionTypeAndId(OptionType.CATEGORY, answer.getSelectedOptionIds().get(0)))
.findFirst()
.orElseThrow();
Copy link
Contributor

Choose a reason for hiding this comment

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

무엇을 Throw 하나요?😱

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋㅋㅋㅋㅋㅋ냅다 던지기.. 반영하겠습니다😱

import reviewme.question.domain.OptionType;

@Repository
public interface OptionRepository extends JpaRepository<OptionItem, Long> {
Copy link
Contributor

Choose a reason for hiding this comment

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

OptionItemRepository 로 이름 변경해주세용~

Comment on lines +286 to +291
assertThat(response.reviews())
.map(review -> review.categories()
.stream()
.map(ReceivedReviewCategoryResponse::content)
.toList())
.allSatisfy(content -> assertThat(categoryContents).containsAll(content));
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 280 to 283
List<String> categoryContents = optionRepository.findAllByOptionType(OptionType.CATEGORY)
.stream()
.map(OptionItem::getContent)
.toList();
Copy link
Contributor

Choose a reason for hiding this comment

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

이거슨 then 부분으로 가야 할 것 같아용~

Comment on lines 158 to 163
String groupAccessCode = "groupAccessCode";
Question question1 = questionRepository.save(new Question("프로젝트 기간 동안, 팀원의 강점이 드러났던 순간을 선택해주세요. (1~2개)"));
OptionGroup categoryOptionGroup = optionGroupRepository.save(new OptionGroup(question1.getId(), 1, 2));
OptionItem categoryOption1 = new OptionItem("커뮤니케이션 능력 ", categoryOptionGroup.getId(), 1, OptionType.CATEGORY);
OptionItem categoryOption2 = new OptionItem("시간 관리 능력", categoryOptionGroup.getId(), 2, OptionType.CATEGORY);
optionRepository.saveAll(List.of(categoryOption1, categoryOption2));
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

@Kimprodp Kimprodp left a comment

Choose a reason for hiding this comment

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

CheckBoxAnswer에 OptionType 컬럼을 추가한다면 DB에 접근하지 않고 리뷰 목록을 위한 카테고리 선택지만 가져오는 것이 쉬울 것 같아 논의해보고 싶습니다!

이 부분 동의합니다.
나머지 수정부분 확인 부탁해요

@@ -29,4 +29,12 @@ public class CheckboxAnswer {
@ElementCollection
@CollectionTable(name = "selected_option_ids")
private List<Long> selectedOptionIds;

@Column(name = "review_id", insertable = false, updatable = false)
private long reviewId;
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.

실험삼아 넣었다가 안뺐네요😂 바로 빼겠습니다~~

.filter(answer -> optionRepository.existsByOptionTypeAndId(OptionType.CATEGORY,
answer.getSelectedOptionIds().get(0)))
.findFirst()
.orElseThrow();
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.

ㅋㅋㅋㅋㅋㅋㅋㅋㅋ그저 던졌군요😱

import reviewme.question.domain.OptionType;

@Repository
public interface OptionRepository extends JpaRepository<OptionItem, Long> {
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

@donghoony donghoony 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 41 to 42
public Review2(long templateId, long reviewGroupId, List<TextAnswer> textAnswers,
List<CheckboxAnswer> checkboxAnswers,
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
public Review2(long templateId, long reviewGroupId, List<TextAnswer> textAnswers,
List<CheckboxAnswer> checkboxAnswers,
public Review2(long templateId, long reviewGroupId,
List<TextAnswer> textAnswers, List<CheckboxAnswer> checkboxAnswers,

Comment on lines 46 to 47
public Section(VisibleType visibleType, List<Long> questionIds, Long onSelectedOptionId, String header,
int position) {
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 166 to 169
CheckboxAnswer checkboxAnswer = review.getCheckboxAnswers()
.stream()
.filter(answer -> optionRepository.existsByOptionTypeAndId(OptionType.CATEGORY, answer.getSelectedOptionIds().get(0)))
.findFirst()
Copy link
Contributor

Choose a reason for hiding this comment

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

이 부분 SQL 쿼리로 하면 한 번에 나가지 않을까요 ?

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

@Kimprodp Kimprodp 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

@nayonsoso nayonsoso left a comment

Choose a reason for hiding this comment

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

변경 사항 확인했습니다 ㅎㅎ 수고했습니당~

@skylar1220 skylar1220 merged commit 3f8deba into develop Aug 11, 2024
3 checks passed
@donghoony donghoony deleted the be/feature/285-review-list branch August 20, 2024 01:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[BE] 리뷰 목록을 재구현한다.
4 participants