-
Notifications
You must be signed in to change notification settings - Fork 0
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/#40 news category service #41
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
critical한 문제는 안보이구, 자잘한 부분만 수정해주시면 좋을 것 같아요!
수고하셨습니다아 :)
@ApiDocumentResponse | ||
@Operation(summary = "Today News List", description = "type이 Recent일 경우 최신 뉴스만 반환, type이 My일 경우 맞춤형 뉴스만 반환, 지정 하지 않을 시 모두 반환") | ||
@GetMapping("/today/list") | ||
public ResponseEntity<List<TodayHomeResponseDto>> getTodayNews(@ReqUser User user, @RequestParam(value = "type", defaultValue = "All") String type) { | ||
List<TodayHomeResponseDto> responseDtoList = new ArrayList<>(); | ||
if(type.equals("Recent")){ | ||
responseDtoList.add(new TodayHomeResponseDto("Recent",newsService.getRecentNewsList(user))); | ||
} | ||
else if(type.equals("My")){ | ||
responseDtoList.add(new TodayHomeResponseDto("My",newsService.getUserReferencedNewsList(user))); | ||
} | ||
else{ | ||
responseDtoList.add(new TodayHomeResponseDto("Recent",newsService.getRecentNewsList(user))); | ||
responseDtoList.add(new TodayHomeResponseDto("My",newsService.getUserReferencedNewsList(user))); | ||
} | ||
return ResponseDto.ok(responseDtoList); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이부분 Service method로 빼는게 좋을거같아요!
컨트롤러에서는 return만 하도록..
Quiz quiz = quizRepository.findById(solvedQuizRequestDto.getQuizId()).orElseThrow(() -> new CustomException(ErrorCode.QUIZ_NOT_FOUND)); | ||
|
||
SolvedQuiz solvedQuiz = new SolvedQuiz(user,quiz, solvedQuizRequestDto.getSelectedAnswer()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이거는 빼도,, 되겠죠?
NewsReadingTime readingTime = newsReadingTimeRepository.findByUserIdAndNewsId(user.getId(), newsId).orElseGet( | ||
()-> new NewsReadingTime(user.getId(), newsId) | ||
()-> { | ||
News news = newsRepository.findById(newsId).get(); | ||
return new NewsReadingTime(user.getId(), news); | ||
} | ||
); | ||
readingTime.updateReadingTime(secondTime); | ||
newsReadingTimeRepository.save(readingTime); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
findById에서 존재하지 않을 경우 처리가 필요할 것 같아요
); | ||
readingTime.updateReadingTime(secondTime); | ||
newsReadingTimeRepository.save(readingTime); | ||
} | ||
|
||
public Field mostOfCategoryReadingTime(User user){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getMostReadingTimeCategory 혹은 getCategoryOfMostReadingTime으로 네이밍 바꾸는게 좋을 것 같아요
for(NewsReadingTime readingTime : newsReadingTimeList) { | ||
Field field = readingTime.getNews().getField(); | ||
if(categoryTimeMap.containsKey(field)) | ||
categoryTimeMap.put(field, categoryTimeMap.get(field) + readingTime.getSecondTime()); | ||
else | ||
categoryTimeMap.put(field, readingTime.getSecondTime()); | ||
} | ||
int mostReadingTime = 0; | ||
Field mostReadingTimeField = Field.DEFAULT; | ||
for (Field key : categoryTimeMap.keySet()) { | ||
if (categoryTimeMap.get(key) > mostReadingTime) { | ||
mostReadingTime = categoryTimeMap.get(key); | ||
mostReadingTimeField = key; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
카테고리별 시간 계산하면서 최고 시간까지 같이 계산해버리면 for문을 한번만 써도 될 것 같아요!
public ResponseEntity<UserReferencedFieldResponseDto> getUserReferencedField(@ReqUser User user){ | ||
UserReferencedFieldResponseDto fieldResponseDto = new UserReferencedFieldResponseDto(newsReadingTimeService.mostOfCategoryReadingTime(user)); | ||
return ResponseDto.ok(fieldResponseDto); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return 한줄로 줄이면 좋을거같아요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
확인했구 나중에 생각나면 로직 수정해주시면 될거같아요
머지하셔두 될 것 같습니다
수고하셨어요!!
PR 요약
(맞춤형 같은 경우, 최근 10개 가장 많이 읽은 카테고리와 온보딩 과정에서 선호하는 카테고리를 적절히 섞어서 추천)
📌 변경 사항
✅ PR check list
기존에 있는 ~/news/list 쓸 수 있도록 해놨는데 혹시 뭐 잘 못 지웠거나 기존꺼에서 충돌일어나는지
Linked Issue
close #40