-
Notifications
You must be signed in to change notification settings - Fork 1
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 : 버스 긴급 공지 API 구현 #1136
feat : 버스 긴급 공지 API 구현 #1136
Conversation
# Conflicts: # src/main/java/in/koreatech/koin/domain/bus/controller/BusApi.java # src/main/java/in/koreatech/koin/domain/bus/controller/BusController.java # src/main/java/in/koreatech/koin/domain/bus/service/BusService.java
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.
고생하셨습니다!
if (article.isEmpty()) { | ||
return null; | ||
} |
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.
null로 반환했을때 생기는 문제가 없을까요?
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.
넵 서비스에서 null이나 empty인 경우에 대해 처리해서 괜찮습니다
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.
TTL
이 없어도 괜찮은건가요??
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.
게시글이 얼마나 미리 올라오는지가 전부 달라 유효기간은 따로 추가하지 않았고, 다음 게시글이 새로 생길 때 교체하는 방식으로 구현했습니다.
ex) 공지글1은 해당날짜 3일전에 올라옴, 공지글2는 당일 긴급으로 올라옴 -> 유효기간을 고정해두면 일찍 게시될 경우 해당날짜 이전에 공지가 내려감
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.
그럼 1년지난 긴급공지도 그대로 반환하는 건가요?? 이런 부분은 숨기는게 좋을 것 같은데 관련 논의가 있었는지, 클라나 백 중에서 어느 단에서 이를 숨겨야할지 논의된 바가 있는지 궁금합니다!
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.
만약 1년간 새로운 '통학버스|등교버스|하교버스|셔틀버스' 키워드를 가진 게시글이 올라오지 않는다면 계속 1년전인 마지막 게시글이 반환되긴 합니다.
새로운 게시글이 올라오지 않는 경우는 생각을 못해봤는데, 이 부분은 상의를 해봐야겠네요.(정적으로 일주일이나 한달? or 변경 적용 기간을 추출할 수 있다면 그때까지로)
우선 QA를 위해 배포는 해놓고 이후 빠르게 추가 수정하겠습니다!
if (latestArticles.size() >= 2) { | ||
latestArticles = latestArticles.stream() | ||
.sorted((first, second) -> { | ||
// title에 '사과'가 포함되면 후순위 |
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.
🍎
if (first.getTitle().contains("사과") && !second.getTitle().contains("사과")) { | ||
return 1; | ||
} | ||
if (!first.getTitle().contains("사과") && second.getTitle().contains("사과")) { |
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.
사과
랑 긴급
까지 상수화 해버리는건 어떤가요
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.
좋네요!
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.
고생하셨어요~👍
@Scheduled(cron = "0 0 * * * *") | ||
public void getBusNoticeArticle() { | ||
try { | ||
articleService.updateBusNoticeArticle(); | ||
} catch (Exception e) { | ||
log.error("버스 공지 게시글 조회 중에 오류가 발생했습니다.", e); | ||
} | ||
} |
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.
1시간 마다 갱신하는게 맞나요?
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.
넵 맞습니다
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.
고생하셨습니다! 🥇
@@ -37,6 +25,7 @@ public class BusController implements BusApi { | |||
|
|||
private final BusService busService; | |||
private final ShuttleBusService shuttleBusService; | |||
private final ArticleService articleService; |
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.
C
이 필드는 어디에 사용되나요??
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.
앗 지우겠습니다..
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.
그럼 1년지난 긴급공지도 그대로 반환하는 건가요?? 이런 부분은 숨기는게 좋을 것 같은데 관련 논의가 있었는지, 클라나 백 중에서 어느 단에서 이를 숨겨야할지 논의된 바가 있는지 궁금합니다!
|
||
@Query(value = "SELECT * FROM koreatech_articles a " | ||
+ "WHERE a.title REGEXP '통학버스|등교버스|셔틀버스|하교버스' " | ||
+ "ORDER BY a.created_at DESC LIMIT 5", nativeQuery = true) | ||
List<Article> findTop5OrderByCreatedAtDesc(); |
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.
A
이 쿼리는 각 단어 중 하나가 포함된 게시글을 찾나요??
+) 메서드명에 버스관련 쿼리임을 명시하면 좋을 것 같아요!
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.
맞습니다 조건이 '네 단어 중 하나라도 포함되면' 을 뜻합니다.
수정하겠습니다!
// title에 '긴급'이 포함되면 우선순위 | ||
if (first.getTitle().contains("긴급") && !second.getTitle().contains("긴급")) { | ||
return -1; | ||
} | ||
if (!first.getTitle().contains("긴급") && second.getTitle().contains("긴급")) { | ||
return 1; | ||
} |
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.
C
지금 로직은
- "사과" 미포함 공지
- "긴급" 포함 공지
인 것 같은데, 요구사항은 둘의 순서가 반대 아닌가요??
그리고 1에서 바로 반환하기보다 1, 2에서 각각 가중치를 더해 최종 값을 return하는 방향으로 정렬 로직을 수행하는 건 어떻게 생각하시는지 궁금해요! 긴급이나 사과 중 누락되는 계산이 생기지 않을지 걱정되서 제안드립니다!
ex)
int result = 0;
if (...) result++;
if (!...) result--;
if(...) result++;
if(!...) result--;
return result;
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.
생각해보니 두 개를 함께 가지는 경우에는 제대로 동작하지 않을 수 있겠네요.
가중치 누적 방법 좋네요! 수정하겠습니다.
template.setHashKeySerializer(new StringRedisSerializer()); | ||
template.setHashValueSerializer(serializer); |
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.
👍
🔥 연관 이슈
🚀 작업 내용
bus
에서는 캐시를 가져오기만 하고,article
에서 버스 공지를 캐싱합니다.사과
키워드가 포함되지 않음긴급
키워드가 포함됨💬 리뷰 중점사항
참고: 노션 버스 긴급공지 요구명세