refactor: 이달의 현황 및 피드 랭킹 API 응답 필드 "개수 -> 가중치 점수"로 변경#390
Conversation
- 응답 필드를 개수(Count)에서 가중치 점수(Score)로 변경 (feedScore/viewScore/likeScore/commentScore/totalScore) - 저번 달 순위(lastMonthRank) 필드 추가 - 1월 조회 시 전년도 12월 순위를 조회하도록 처리 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- 기존 3개 테스트: feedCount→feedScore 등 필드명 변경 + lastMonthRank 검증 추가 - 신규 3개 테스트: 저번 달 피드 있을 때/없을 때, 1월→전년도 12월 순위 조회 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough클럽 월간 상태 및 랭킹 관련 DTO와 서비스 매핑을 개편하여 개별 가중치 점수 필드(feedScore, viewScore, likeScore, commentScore, totalScore)와 지난달 순위(lastMonthRank)를 도입하고, 관련 서비스 로직·팩토리 시그니처·테스트·문서를 이에 맞춰 업데이트했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Controller as FeedController
participant Service as GeneralFeedRankingService
participant Repo as FeedRankingRepository
participant DTO as ClubMonthlyStatusQuery/Response
Client->>Controller: 월간 상태 요청
Controller->>Service: getClubMonthlyStatus(year, month, clubId)
Service->>Repo: 해당 월 랭킹 데이터 조회
Repo-->>Service: 월별 랭킹 데이터 반환
Service->>Service: getLastMonthRank(clubId, year, month)
Service->>Repo: 지난달 랭킹 조회
Repo-->>Service: lastMonthRank (없으면 0)
Service->>DTO: toMonthlyStatus(..., lastMonthRank) -> ClubMonthlyStatusQuery.of(...)
Service-->>Controller: ClubMonthlyStatusResponse(개별 score, totalScore, lastMonthRank)
Controller-->>Client: 응답 반환
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
||
| @Operation(summary = "동아리 이달의 현황 조회 API") | ||
| @ApiResponse(responseCode = "200", description = "동아리 이달의 현황 조회 성공", | ||
| @ApiResponse(responseCode = "200", description = "동아리 이달의 현황 조회 성공 - 가중치 점수(feedScore/viewScore/likeScore/commentScore/totalScore)와 저번 달 순위(lastMonthRank) 반환", |
There was a problem hiding this comment.
간결하게 바꿔도 될 것 같은데? 이전 것이 더 낫은 것 같아
| private static final int FEED_WEIGHT = 10; | ||
| private static final int VIEW_WEIGHT = 1; | ||
| private static final int LIKE_WEIGHT = 3; | ||
| private static final int COMMENT_WEIGHT = 5; |
There was a problem hiding this comment.
dto 외부에서 하는게 좋을 것 같은데? 이것도 도메인 개념이라 dto 책임은 아닌 것 같아
src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingService.java (2)
21-24: 가중치 상수가ClubMonthlyStatusQuery와 중복입니다
ClubMonthlyStatusQueryLines 18–21과 동일한 상수가 정의되어 있습니다.ClubMonthlyStatusQuery.from()이 이미 가중치 적용을 담당하므로, 이 클래스에서는 공통 상수 클래스를 참조하는 방식으로 중복을 제거하는 것을 권장합니다. (ClubMonthlyStatusQuery파일 리뷰 코멘트 참조)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingService.java` around lines 21 - 24, The FEED_WEIGHT, VIEW_WEIGHT, LIKE_WEIGHT, and COMMENT_WEIGHT constants in GeneralFeedRankingService duplicate the weights already defined for ClubMonthlyStatusQuery; remove these four constants from GeneralFeedRankingService and update any usages to reference the shared source of truth instead (either ClubMonthlyStatusQuery's public constants or a new common constants class, e.g., FeedWeights) so that GeneralFeedRankingService uses ClubMonthlyStatusQuery.from()’s weighting logic or the shared constants rather than re-defining them.
66-77: 성능 고려:getLastMonthRank의 이중 DB 쿼리PR 작성자도 언급했듯이,
getClubMonthlyStatus()에서getClubFeedRanking()이 현재 달(Line 55)과 저번 달(Line 70) 두 번 호출됩니다. 호출량이 많아지면 병목이 될 수 있으므로, 캐싱 적용 혹은 두 달의 데이터를 단일 쿼리로 조회하는 방식을 중·장기적으로 고려하길 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingService.java` around lines 66 - 77, getLastMonthRank currently calls getClubFeedRanking again causing duplicate DB queries when getClubMonthlyStatus already fetched current month data; update the logic to avoid the second query by either (A) modifying getClubMonthlyStatus to fetch both months in one call and pass the last-month list into getLastMonthRank, or (B) add a cache/map in getClubMonthlyStatus that stores results of getClubFeedRanking(year, month) and make getLastMonthRank accept/lookup the pre-fetched List<ClubFeedRankingQuery> instead of calling getClubFeedRanking itself (refer to methods getClubMonthlyStatus, getLastMonthRank, and getClubFeedRanking to locate changes).src/main/java/ddingdong/ddingdongBE/domain/feed/service/dto/query/ClubMonthlyStatusQuery.java (1)
18-21: 가중치 상수가GeneralFeedRankingService와 중복 정의되어 있습니다
GeneralFeedRankingService의 Lines 21–24에 동일한FEED_WEIGHT,VIEW_WEIGHT,LIKE_WEIGHT,COMMENT_WEIGHT가 선언되어 있습니다. 두 곳에서 관리되면 한쪽만 수정했을 때 정렬 기준(calculateScore)과 응답 점수(feedScore등)가 불일치하는 버그로 이어질 수 있습니다.별도 상수 클래스나 인터페이스(
FeedScoreWeights등)로 추출해 단일 정의(Single Source of Truth)를 유지하길 권장합니다.♻️ 리팩터링 제안 예시
// FeedScoreWeights.java (신규 파일) public final class FeedScoreWeights { public static final int FEED = 10; public static final int VIEW = 1; public static final int LIKE = 3; public static final int COMMENT = 5; private FeedScoreWeights() {} }- private static final int FEED_WEIGHT = 10; - private static final int VIEW_WEIGHT = 1; - private static final int LIKE_WEIGHT = 3; - private static final int COMMENT_WEIGHT = 5; + // FeedScoreWeights 상수 참조🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/ddingdong/ddingdongBE/domain/feed/service/dto/query/ClubMonthlyStatusQuery.java` around lines 18 - 21, The FEED_WEIGHT, VIEW_WEIGHT, LIKE_WEIGHT, COMMENT_WEIGHT constants are duplicated in ClubMonthlyStatusQuery and GeneralFeedRankingService; extract them into a single constants holder (e.g., a final class FeedScoreWeights with public static final ints) and replace uses in both ClubMonthlyStatusQuery and GeneralFeedRankingService to reference FeedScoreWeights.FEED, .VIEW, .LIKE, .COMMENT so there is a single source of truth; ensure the original constant names in methods like calculateScore and feedScore are updated to use the new class and delete the duplicated fields from both classes.src/test/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingServiceTest.java (1)
287-335:createEmpty경로의lastMonthRank유실 시나리오 테스트 누락현재 추가된 테스트들은 모두 이번 달 피드가 있는(score > 0) 상황만 검증합니다.
GeneralFeedRankingService의 실제 버그(lastMonthRank가createEmpty경로에서 유실됨)는 "저번 달에 피드가 있었지만 이번 달에는 피드가 없는" 케이스에서 발생하는데, 이를 검증하는 테스트가 없습니다.아래 케이스를 추가하면 해당 버그를 커버할 수 있습니다:
🧪 추가 테스트 케이스 예시
`@DisplayName`("동아리 이달의 현황 조회 - 성공: 이번 달 피드가 없어도 저번 달 순위가 반환된다") `@Test` void getClubMonthlyStatus_noCurrentMonthFeed_butHasLastMonthRank() { // given User user = userRepository.save(UserFixture.createClubUser()); Club club = clubRepository.save(ClubFixture.createClub(user)); // 저번 달 피드 생성 LocalDate lastMonth = LocalDate.now().minusMonths(1); Feed lastMonthFeed = feedRepository.save(FeedFixture.createImageFeed(club, "저번달 피드")); jdbcTemplate.update("UPDATE feed SET created_at = ? WHERE id = ?", Timestamp.valueOf(lastMonth.atStartOfDay()), lastMonthFeed.getId()); // 이번 달 피드 없음 int year = LocalDate.now().getYear(); int month = LocalDate.now().getMonthValue(); // when ClubMonthlyStatusQuery result = feedRankingService.getClubMonthlyStatus(user.getId(), year, month); // then — 이번 달 score=0이어도 저번 달 순위가 정상 반환되어야 한다 assertSoftly(softly -> { softly.assertThat(result.rank()).isEqualTo(0); softly.assertThat(result.lastMonthRank()).isEqualTo(1); // 현재 구현에서는 0으로 잘못 반환됨 }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingServiceTest.java` around lines 287 - 335, Add a test covering the "createEmpty" path where the club has last month's feed but no current-month feed: in GeneralFeedRankingServiceTest add a test method (e.g. getClubMonthlyStatus_noCurrentMonthFeed_butHasLastMonthRank) that creates a User and Club via userRepository.save/UserFixture.createClubUser and clubRepository.save/ClubFixture.createClub, inserts a feed for last month using feedRepository.save(FeedFixture.createImageFeed(...)) and then uses jdbcTemplate.update to set its created_at to lastMonth.atStartOfDay(), do NOT create any current-month feed, call feedRankingService.getClubMonthlyStatus(user.getId(), year, month) and assert that result.rank() == 0 and result.lastMonthRank() == 1 to catch the bug where lastMonthRank is lost in the createEmpty branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/skills/review-fix-plan/SKILL.md:
- Around line 74-75: Add a blank line before and after the table row "| {이번 리뷰에서
얻은 교훈} | {재발 방지를 위한 규칙} |" to satisfy MD058, and change the fenced code block
from a language-less "```" to include a language tag (e.g., "```text" or
"```md") to satisfy MD040; locate the table line and the following code fence in
SKILL.md and update the surrounding blank lines and the opening triple-backtick
to include an appropriate language.
In
`@src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingService.java`:
- Around line 56-63: The code computes lastMonthRank via getLastMonthRank(...)
but loses it when falling back to ClubMonthlyStatusQuery.createEmpty(year,
month); update the fallback in GeneralFeedRankingService (the stream ending that
uses .orElse(...)) to pass the computed lastMonthRank into
ClubMonthlyStatusQuery.createEmpty (i.e., call the overload that accepts year,
month, lastMonthRank), and ensure ClubMonthlyStatusQuery has a matching
createEmpty(year, month, lastMonthRank) factory if not present; keep
ClubMonthlyStatusQuery.from(...) usage unchanged.
In
`@src/test/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingServiceTest.java`:
- Around line 316-335: In getClubMonthlyStatus_noLastMonthFeeds test, avoid
unstable clubRepository.findAll().get(0) lookup; capture the Club returned by
clubRepository.save(ClubFixture.createClub(user)) into a variable (e.g.,
savedClub) and pass that savedClub into FeedFixture.createImageFeed instead of
calling clubRepository.findAll().get(0) so the test always uses the club you
just saved when creating the feed and asserting lastMonthRank.
---
Nitpick comments:
In
`@src/main/java/ddingdong/ddingdongBE/domain/feed/service/dto/query/ClubMonthlyStatusQuery.java`:
- Around line 18-21: The FEED_WEIGHT, VIEW_WEIGHT, LIKE_WEIGHT, COMMENT_WEIGHT
constants are duplicated in ClubMonthlyStatusQuery and
GeneralFeedRankingService; extract them into a single constants holder (e.g., a
final class FeedScoreWeights with public static final ints) and replace uses in
both ClubMonthlyStatusQuery and GeneralFeedRankingService to reference
FeedScoreWeights.FEED, .VIEW, .LIKE, .COMMENT so there is a single source of
truth; ensure the original constant names in methods like calculateScore and
feedScore are updated to use the new class and delete the duplicated fields from
both classes.
In
`@src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingService.java`:
- Around line 21-24: The FEED_WEIGHT, VIEW_WEIGHT, LIKE_WEIGHT, and
COMMENT_WEIGHT constants in GeneralFeedRankingService duplicate the weights
already defined for ClubMonthlyStatusQuery; remove these four constants from
GeneralFeedRankingService and update any usages to reference the shared source
of truth instead (either ClubMonthlyStatusQuery's public constants or a new
common constants class, e.g., FeedWeights) so that GeneralFeedRankingService
uses ClubMonthlyStatusQuery.from()’s weighting logic or the shared constants
rather than re-defining them.
- Around line 66-77: getLastMonthRank currently calls getClubFeedRanking again
causing duplicate DB queries when getClubMonthlyStatus already fetched current
month data; update the logic to avoid the second query by either (A) modifying
getClubMonthlyStatus to fetch both months in one call and pass the last-month
list into getLastMonthRank, or (B) add a cache/map in getClubMonthlyStatus that
stores results of getClubFeedRanking(year, month) and make getLastMonthRank
accept/lookup the pre-fetched List<ClubFeedRankingQuery> instead of calling
getClubFeedRanking itself (refer to methods getClubMonthlyStatus,
getLastMonthRank, and getClubFeedRanking to locate changes).
In
`@src/test/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingServiceTest.java`:
- Around line 287-335: Add a test covering the "createEmpty" path where the club
has last month's feed but no current-month feed: in
GeneralFeedRankingServiceTest add a test method (e.g.
getClubMonthlyStatus_noCurrentMonthFeed_butHasLastMonthRank) that creates a User
and Club via userRepository.save/UserFixture.createClubUser and
clubRepository.save/ClubFixture.createClub, inserts a feed for last month using
feedRepository.save(FeedFixture.createImageFeed(...)) and then uses
jdbcTemplate.update to set its created_at to lastMonth.atStartOfDay(), do NOT
create any current-month feed, call
feedRankingService.getClubMonthlyStatus(user.getId(), year, month) and assert
that result.rank() == 0 and result.lastMonthRank() == 1 to catch the bug where
lastMonthRank is lost in the createEmpty branch.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.claude/skills/review-fix-plan/SKILL.mdCLAUDE.mdsrc/main/java/ddingdong/ddingdongBE/domain/feed/api/ClubFeedApi.javasrc/main/java/ddingdong/ddingdongBE/domain/feed/controller/dto/response/ClubMonthlyStatusResponse.javasrc/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingService.javasrc/main/java/ddingdong/ddingdongBE/domain/feed/service/dto/query/ClubMonthlyStatusQuery.javasrc/test/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingServiceTest.java
| | {이번 리뷰에서 얻은 교훈} | {재발 방지를 위한 규칙} | | ||
| ``` |
There was a problem hiding this comment.
마크다운 린팅 경고 2건
정적 분석 도구가 두 가지 경고를 감지했습니다:
- Line 74: 테이블 앞뒤에 빈 줄 추가 필요 (MD058)
- Line 75: 코드 블록에 언어 지정 필요 (MD040)
🔧 수정 제안
**해결 방법**: {구체적인 수정 방안 서술}
+
| 상황 | 패턴 |
|------|------|
| {이번 리뷰에서 얻은 교훈} | {재발 방지를 위한 규칙} |
+
-```
+```text🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 74-74: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 75-75: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/skills/review-fix-plan/SKILL.md around lines 74 - 75, Add a blank
line before and after the table row "| {이번 리뷰에서 얻은 교훈} | {재발 방지를 위한 규칙} |" to
satisfy MD058, and change the fenced code block from a language-less "```" to
include a language tag (e.g., "```text" or "```md") to satisfy MD040; locate the
table line and the following code fence in SKILL.md and update the surrounding
blank lines and the opening triple-backtick to include an appropriate language.
src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingService.java
Outdated
Show resolved
Hide resolved
src/test/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingServiceTest.java
Show resolved
Hide resolved
- Swagger description 원복 (간결하게) - 가중치 계산 로직을 Query DTO에서 Service로 이동 (도메인 로직은 DTO 책임 아님) - Query DTO의 from()을 of()로 변경하여 계산된 값을 직접 받도록 수정 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingService.java (1)
56-63:⚠️ Potential issue | 🟠 Major
createEmptyfallback에서lastMonthRank가 유실됩니다 (버그)Line 56에서
lastMonthRank를 계산하지만, Line 63의.orElse(ClubMonthlyStatusQuery.createEmpty(year, month))는 이 값을 전달받지 않아 항상0으로 초기화된 객체를 반환합니다.이번 달 활동이 없지만 지난달에 순위가 존재하는 동아리의 경우, 응답에서
lastMonthRank=0이 잘못 반환됩니다.🐛 수정 제안
ClubMonthlyStatusQuery.java:-public static ClubMonthlyStatusQuery createEmpty(int year, int month) { +public static ClubMonthlyStatusQuery createEmpty(int year, int month, int lastMonthRank) { return ClubMonthlyStatusQuery.builder() .year(year) .month(month) .rank(0) - .lastMonthRank(0) + .lastMonthRank(lastMonthRank) .feedScore(0) .viewScore(0) .likeScore(0) .commentScore(0) .totalScore(0) .build(); }
GeneralFeedRankingService.java:- .orElse(ClubMonthlyStatusQuery.createEmpty(year, month)); + .orElse(ClubMonthlyStatusQuery.createEmpty(year, month, lastMonthRank));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingService.java` around lines 56 - 63, The code computes lastMonthRank via getLastMonthRank(...) but loses it in the fallback because ClubMonthlyStatusQuery.createEmpty(year, month) doesn't receive lastMonthRank; update the logic so the fallback includes lastMonthRank — either add a new factory ClubMonthlyStatusQuery.createEmpty(int year, int month, int lastMonthRank) and call .orElse(ClubMonthlyStatusQuery.createEmpty(year, month, lastMonthRank)), or change to .orElseGet(() -> ClubMonthlyStatusQuery.createEmpty(year, month, lastMonthRank)); ensure references to lastMonthRank, getLastMonthRank(...), toMonthlyStatus(...), and ClubMonthlyStatusQuery.createEmpty(...) are updated accordingly.
🧹 Nitpick comments (1)
src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingService.java (1)
55-55:getClubMonthlyStatus호출 시getClubFeedRanking이 2회 실행됩니다Line 55에서 현재 달 랭킹을 조회하고, Line 81에서
getLastMonthRank가 이전 달 랭킹을 조회하여getClubMonthlyStatus1회 호출 시feedRepository.findMonthlyRankingByClub이 총 2번 실행됩니다.PR에서 저자가 인지하고 있는 부분이지만, 두 조회 결과를 함께 반환하는 단일 쿼리 또는 Spring Cache(
@Cacheable)를 적용해 중복 DB 접근을 줄이는 것을 권장합니다.Also applies to: 77-88
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingService.java` at line 55, getClubMonthlyStatus currently calls getClubFeedRanking(year, month) and getLastMonthRank which both invoke feedRepository.findMonthlyRankingByClub, causing duplicate DB hits; either combine both month queries into a single repository method that returns current and previous month rankings in one call (e.g., new repo method like findMonthlyRankingsByClubForMonths(year, month, prevYear, prevMonth) and update getClubFeedRanking/getLastMonthRank to use it) or annotate getClubFeedRanking (or both ranking methods) with `@Cacheable` so the second call hits the cache; update GeneralFeedRankingService.getClubMonthlyStatus to use the combined result or cached result and remove the redundant repository invocation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingService.java`:
- Around line 56-63: The code computes lastMonthRank via getLastMonthRank(...)
but loses it in the fallback because ClubMonthlyStatusQuery.createEmpty(year,
month) doesn't receive lastMonthRank; update the logic so the fallback includes
lastMonthRank — either add a new factory ClubMonthlyStatusQuery.createEmpty(int
year, int month, int lastMonthRank) and call
.orElse(ClubMonthlyStatusQuery.createEmpty(year, month, lastMonthRank)), or
change to .orElseGet(() -> ClubMonthlyStatusQuery.createEmpty(year, month,
lastMonthRank)); ensure references to lastMonthRank, getLastMonthRank(...),
toMonthlyStatus(...), and ClubMonthlyStatusQuery.createEmpty(...) are updated
accordingly.
---
Nitpick comments:
In
`@src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingService.java`:
- Line 55: getClubMonthlyStatus currently calls getClubFeedRanking(year, month)
and getLastMonthRank which both invoke feedRepository.findMonthlyRankingByClub,
causing duplicate DB hits; either combine both month queries into a single
repository method that returns current and previous month rankings in one call
(e.g., new repo method like findMonthlyRankingsByClubForMonths(year, month,
prevYear, prevMonth) and update getClubFeedRanking/getLastMonthRank to use it)
or annotate getClubFeedRanking (or both ranking methods) with `@Cacheable` so the
second call hits the cache; update
GeneralFeedRankingService.getClubMonthlyStatus to use the combined result or
cached result and remove the redundant repository invocation.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingService.javasrc/main/java/ddingdong/ddingdongBE/domain/feed/service/dto/query/ClubMonthlyStatusQuery.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/ddingdong/ddingdongBE/domain/feed/service/dto/query/ClubMonthlyStatusQuery.java
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
이번 달 피드가 없는 동아리가 createEmpty 분기로 빠질 때 계산된 lastMonthRank가 전달되지 않고 항상 0으로 반환되던 버그를 수정했다. 테스트에서 clubRepository.findAll().get(0) 대신 save 반환값을 직접 사용하도록 개선했다. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/main/java/ddingdong/ddingdongBE/domain/feed/service/dto/query/ClubMonthlyStatusQuery.java (1)
18-29:totalScore합산이 여전히of()팩토리 메서드 내부에 남아 있습니다가중치 곱셈(×10, ×1, ×3, ×5)은 서비스로 이동됐지만, Line 29의
feedScore + viewScore + likeScore + commentScore합산은 여전히 DTO 내부에서 수행됩니다. 이전 리뷰에서 지적된 것처럼 DTO 외부(서비스)에서totalScore를 계산한 뒤 명시적으로 전달하면 DTO를 순수 데이터 컨테이너로 유지할 수 있습니다.♻️ 리팩토링 제안
of()파라미터에totalScore를 추가:- public static ClubMonthlyStatusQuery of(int year, int month, int rank, int lastMonthRank, - long feedScore, long viewScore, long likeScore, long commentScore) { + public static ClubMonthlyStatusQuery of(int year, int month, int rank, int lastMonthRank, + long feedScore, long viewScore, long likeScore, long commentScore, long totalScore) { return ClubMonthlyStatusQuery.builder() .year(year) .month(month) .rank(rank) .lastMonthRank(lastMonthRank) .feedScore(feedScore) .viewScore(viewScore) .likeScore(likeScore) .commentScore(commentScore) - .totalScore(feedScore + viewScore + likeScore + commentScore) + .totalScore(totalScore) .build(); }그리고
GeneralFeedRankingService.toMonthlyStatus()에서 호출 시:long totalScore = feedScore + viewScore + likeScore + commentScore; ClubMonthlyStatusQuery.of(year, month, rank, lastMonthRank, feedScore, viewScore, likeScore, commentScore, totalScore);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/ddingdong/ddingdongBE/domain/feed/service/dto/query/ClubMonthlyStatusQuery.java` around lines 18 - 29, The DTO factory ClubMonthlyStatusQuery.of currently computes totalScore internally (feedScore + viewScore + likeScore + commentScore); change the signature to accept a totalScore parameter and remove the in-method sum so the DTO is a pure data container, then update callers (e.g., GeneralFeedRankingService.toMonthlyStatus) to compute long totalScore = feedScore + viewScore + likeScore + commentScore and pass it into ClubMonthlyStatusQuery.of(year, month, rank, lastMonthRank, feedScore, viewScore, likeScore, commentScore, totalScore).
🧹 Nitpick comments (2)
src/test/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingServiceTest.java (1)
297-298: 동일한 SQL 문자열이 4곳에 중복됩니다
"UPDATE feed SET created_at = ? WHERE id = ?"문자열이 lines 297, 346, 373, 378에서 그대로 반복됩니다. 헬퍼 메서드로 추출하면 테이블/컬럼명 변경 시 수정 지점을 최소화할 수 있습니다.♻️ 헬퍼 메서드 추출 제안
private void updateFeedCreatedAt(Feed feed, LocalDateTime createdAt) { jdbcTemplate.update("UPDATE feed SET created_at = ? WHERE id = ?", Timestamp.valueOf(createdAt), feed.getId()); }호출부 변경:
- jdbcTemplate.update("UPDATE feed SET created_at = ? WHERE id = ?", - Timestamp.valueOf(lastMonth.atStartOfDay()), lastMonthFeed.getId()); + updateFeedCreatedAt(lastMonthFeed, lastMonth.atStartOfDay());- jdbcTemplate.update("UPDATE feed SET created_at = ? WHERE id = ?", - Timestamp.valueOf(lastMonth.atStartOfDay()), lastMonthFeed.getId()); + updateFeedCreatedAt(lastMonthFeed, lastMonth.atStartOfDay());- jdbcTemplate.update("UPDATE feed SET created_at = ? WHERE id = ?", - Timestamp.valueOf(LocalDateTime.of(currentYear - 1, 12, 15, 10, 0)), decemberFeed.getId()); - jdbcTemplate.update("UPDATE feed SET created_at = ? WHERE id = ?", - Timestamp.valueOf(LocalDateTime.of(currentYear, 1, 15, 10, 0)), januaryFeed.getId()); + updateFeedCreatedAt(decemberFeed, LocalDateTime.of(currentYear - 1, 12, 15, 10, 0)); + updateFeedCreatedAt(januaryFeed, LocalDateTime.of(currentYear, 1, 15, 10, 0));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingServiceTest.java` around lines 297 - 298, Duplicate SQL string "UPDATE feed SET created_at = ? WHERE id = ?" appears multiple times in GeneralFeedRankingServiceTest; extract a helper method (e.g., private void updateFeedCreatedAt(Feed feed, LocalDateTime createdAt)) that calls jdbcTemplate.update with the Timestamp.valueOf(createdAt) and feed.getId(), then replace the four inline jdbcTemplate.update(...) occurrences with calls to updateFeedCreatedAt(feed, createdAt) to centralize the SQL and reduce maintenance points.src/test/java/ddingdong/ddingdongBE/domain/feed/controller/ClubFeedStatusE2ETest.java (1)
94-104:viewScore명시적 검증 누락
feedScore,likeScore,commentScore는 개별적으로 검증하지만viewScore는 어서션이 없습니다.totalScore(28)에서 암묵적으로 0임을 확인할 수 있지만, 다른 점수 필드들과의 일관성을 위해 명시적으로 추가를 고려해볼 수 있습니다.💡 제안
softly.assertThat(((Number) response.get("feedScore")).longValue()).isEqualTo(20L); + softly.assertThat(((Number) response.get("viewScore")).longValue()).isEqualTo(0L); softly.assertThat(((Number) response.get("likeScore")).longValue()).isEqualTo(3L);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/ddingdong/ddingdongBE/domain/feed/controller/ClubFeedStatusE2ETest.java` around lines 94 - 104, The test omits an explicit assertion for viewScore in ClubFeedStatusE2ETest; inside the existing assertSoftly block that checks response.get("feedScore"), "likeScore", "commentScore", "totalScore", "rank", and "lastMonthRank", add a similar assertion to verify ((Number) response.get("viewScore")).longValue() isEqualTo(0L) so viewScore is explicitly validated against the expected zero value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@src/main/java/ddingdong/ddingdongBE/domain/feed/service/dto/query/ClubMonthlyStatusQuery.java`:
- Around line 18-29: The DTO factory ClubMonthlyStatusQuery.of currently
computes totalScore internally (feedScore + viewScore + likeScore +
commentScore); change the signature to accept a totalScore parameter and remove
the in-method sum so the DTO is a pure data container, then update callers
(e.g., GeneralFeedRankingService.toMonthlyStatus) to compute long totalScore =
feedScore + viewScore + likeScore + commentScore and pass it into
ClubMonthlyStatusQuery.of(year, month, rank, lastMonthRank, feedScore,
viewScore, likeScore, commentScore, totalScore).
---
Nitpick comments:
In
`@src/test/java/ddingdong/ddingdongBE/domain/feed/controller/ClubFeedStatusE2ETest.java`:
- Around line 94-104: The test omits an explicit assertion for viewScore in
ClubFeedStatusE2ETest; inside the existing assertSoftly block that checks
response.get("feedScore"), "likeScore", "commentScore", "totalScore", "rank",
and "lastMonthRank", add a similar assertion to verify ((Number)
response.get("viewScore")).longValue() isEqualTo(0L) so viewScore is explicitly
validated against the expected zero value.
In
`@src/test/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingServiceTest.java`:
- Around line 297-298: Duplicate SQL string "UPDATE feed SET created_at = ?
WHERE id = ?" appears multiple times in GeneralFeedRankingServiceTest; extract a
helper method (e.g., private void updateFeedCreatedAt(Feed feed, LocalDateTime
createdAt)) that calls jdbcTemplate.update with the Timestamp.valueOf(createdAt)
and feed.getId(), then replace the four inline jdbcTemplate.update(...)
occurrences with calls to updateFeedCreatedAt(feed, createdAt) to centralize the
SQL and reduce maintenance points.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingService.javasrc/main/java/ddingdong/ddingdongBE/domain/feed/service/dto/query/ClubMonthlyStatusQuery.javasrc/test/java/ddingdong/ddingdongBE/domain/feed/controller/ClubFeedStatusE2ETest.javasrc/test/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingService.java
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/test/java/ddingdong/ddingdongBE/domain/feed/controller/AdminFeedControllerE2ETest.java (1)
110-114: 두 번째 항목의viewScore,likeScore,commentScore검증이 누락되었습니다.첫 번째 항목(동아리B)은 모든 필드를 검증하지만, 두 번째 항목(동아리A)은
feedScore와totalScore만 검증하고viewScore(0),likeScore(0),commentScore(0)검증이 생략되어 있습니다.✅ 누락된 어설션 추가 제안
assertThat(second.clubName()).isEqualTo("동아리A"); assertThat(second.rank()).isEqualTo(2); assertThat(second.feedScore()).isEqualTo(1 * 10); + assertThat(second.viewScore()).isEqualTo(0); + assertThat(second.likeScore()).isEqualTo(0); + assertThat(second.commentScore()).isEqualTo(0); assertThat(second.totalScore()).isEqualTo(10);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/ddingdong/ddingdongBE/domain/feed/controller/AdminFeedControllerE2ETest.java` around lines 110 - 114, The test AdminFeedControllerE2ETest is missing assertions for viewScore, likeScore, and commentScore on the second AdminClubFeedRankingResponse ("second"); add assertions that second.viewScore() isEqualTo(0), second.likeScore() isEqualTo(0), and second.commentScore() isEqualTo(0) alongside the existing checks for clubName(), rank(), feedScore(), and totalScore() so the second entry is fully verified.src/main/java/ddingdong/ddingdongBE/domain/feed/controller/dto/response/AdminClubFeedRankingResponse.java (1)
28-32: 가중치 상수의 중복 제거를 권장합니다.
totalScore와 개별 점수 합계는 실제로 정합성이 보장됩니다.totalScore(query.score())는GeneralFeedRankingService.calculateScore()메서드에서 계산되며, 동일한 가중치(FEED_WEIGHT=10, VIEW_WEIGHT=1, LIKE_WEIGHT=3, COMMENT_WEIGHT=5)와 동일한 공식을 사용합니다.다만 이 가중치 상수들이 다음 여러 클래스에서 중복 정의되어 있습니다:
AdminClubFeedRankingResponse(현재 파일)GeneralFeedRankingServiceFeedMonthlyRankingDDD 구조를 따르면서 단일 소스 오브 트루스 원칙을 지키기 위해, 이 상수들을 공유 클래스(예:
FeedRankingConstants또는FeedScoreWeights)로 추출하여 중복을 제거하는 것을 권장합니다. 이렇게 하면 향후 가중치 조정 시 한 곳에서만 수정하면 됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/ddingdong/ddingdongBE/domain/feed/controller/dto/response/AdminClubFeedRankingResponse.java` around lines 28 - 32, Extract the duplicated weight constants into a single shared constants class (e.g., FeedScoreWeights or FeedRankingConstants) and replace the local constants in AdminClubFeedRankingResponse, GeneralFeedRankingService, and FeedMonthlyRanking with references to that shared class; update AdminClubFeedRankingResponse's builder calls (.feedScore, .viewScore, .likeScore, .commentScore) to multiply by FeedScoreWeights.FEED_WEIGHT / VIEW_WEIGHT / LIKE_WEIGHT / COMMENT_WEIGHT and keep .totalScore(query.score()) as-is so it stays consistent with GeneralFeedRankingService.calculateScore(). Ensure the new constants class is package-visible where needed and import it in each of the three classes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/main/java/ddingdong/ddingdongBE/domain/feed/controller/dto/response/AdminClubFeedRankingResponse.java`:
- Around line 28-32: Extract the duplicated weight constants into a single
shared constants class (e.g., FeedScoreWeights or FeedRankingConstants) and
replace the local constants in AdminClubFeedRankingResponse,
GeneralFeedRankingService, and FeedMonthlyRanking with references to that shared
class; update AdminClubFeedRankingResponse's builder calls (.feedScore,
.viewScore, .likeScore, .commentScore) to multiply by
FeedScoreWeights.FEED_WEIGHT / VIEW_WEIGHT / LIKE_WEIGHT / COMMENT_WEIGHT and
keep .totalScore(query.score()) as-is so it stays consistent with
GeneralFeedRankingService.calculateScore(). Ensure the new constants class is
package-visible where needed and import it in each of the three classes.
In
`@src/test/java/ddingdong/ddingdongBE/domain/feed/controller/AdminFeedControllerE2ETest.java`:
- Around line 110-114: The test AdminFeedControllerE2ETest is missing assertions
for viewScore, likeScore, and commentScore on the second
AdminClubFeedRankingResponse ("second"); add assertions that second.viewScore()
isEqualTo(0), second.likeScore() isEqualTo(0), and second.commentScore()
isEqualTo(0) alongside the existing checks for clubName(), rank(), feedScore(),
and totalScore() so the second entry is fully verified.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/ddingdong/ddingdongBE/domain/feed/controller/dto/response/AdminClubFeedRankingResponse.javasrc/test/java/ddingdong/ddingdongBE/domain/feed/controller/AdminFeedControllerE2ETest.java
.../ddingdong/ddingdongBE/domain/feed/controller/dto/response/AdminClubFeedRankingResponse.java
Outdated
Show resolved
Hide resolved
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingService.java (2)
85-100:calculateScore()와toClubFeedRankingQuery()에서 동일한 가중치 계산이 중복됩니다.
calculateScore()는 정렬용으로만 사용되고,toClubFeedRankingQuery()에서 동일한 곱셈을 다시 수행합니다.calculateScore를 제거하고, 정렬 단계에서도toClubFeedRankingQuery를 활용하거나, 한번 계산된 결과를 재사용하는 방식으로 중복을 제거할 수 있습니다.♻️ 제안: 정렬 시 미리 변환 후 재사용
`@Override` public List<ClubFeedRankingQuery> getClubFeedRanking(int year, int month) { List<MonthlyFeedRankingDto> rawRankings = feedRepository.findMonthlyRankingByClub(year, month); - List<MonthlyFeedRankingDto> sorted = rawRankings.stream() - .sorted(Comparator.comparingLong(this::calculateScore).reversed()) + List<ClubFeedRankingQuery> sorted = rawRankings.stream() + .map(dto -> toClubFeedRankingQuery(0, dto)) + .sorted(Comparator.comparingLong(ClubFeedRankingQuery::totalScore).reversed()) .toList(); List<ClubFeedRankingQuery> result = new ArrayList<>(); long previousScore = Long.MAX_VALUE; int rank = 1; for (int i = 0; i < sorted.size(); i++) { - long totalScore = calculateScore(sorted.get(i)); + long totalScore = sorted.get(i).totalScore(); if (i > 0 && totalScore < previousScore) { rank = i + 1; } - result.add(toClubFeedRankingQuery(rank, sorted.get(i))); + result.add(ClubFeedRankingQuery.of(rank, sorted.get(i).clubId(), sorted.get(i).clubName(), + sorted.get(i).feedScore(), sorted.get(i).viewScore(), + sorted.get(i).likeScore(), sorted.get(i).commentScore(), totalScore)); previousScore = totalScore; } return result; } - - private long calculateScore(MonthlyFeedRankingDto dto) { - return dto.getFeedCount() * FEED_WEIGHT - + dto.getViewCount() * VIEW_WEIGHT - + dto.getLikeCount() * LIKE_WEIGHT - + dto.getCommentCount() * COMMENT_WEIGHT; - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingService.java` around lines 85 - 100, The weight calculation is duplicated between calculateScore(...) and toClubFeedRankingQuery(...); remove this redundancy by computing the weighted total once and reusing it: either eliminate calculateScore and map MonthlyFeedRankingDto -> ClubFeedRankingQuery first (using toClubFeedRankingQuery(...) which computes totalScore via the FEED/VIEW/LIKE/COMMENT_WEIGHTs and ClubFeedRankingQuery.of(...)), then sort by the resulting ClubFeedRankingQuery.getTotalScore(), or keep calculateScore but have toClubFeedRankingQuery accept the precomputed score (e.g., toClubFeedRankingQuery(int rank, MonthlyFeedRankingDto dto, long totalScore)) so the multiplication logic exists in only one place (identify methods: calculateScore, toClubFeedRankingQuery, and ClubFeedRankingQuery.of to update accordingly).
72-83:getLastMonthRank()가getClubFeedRanking()을 다시 호출하여 DB 쿼리가 중복 실행됩니다.
getClubMonthlyStatus에서 이미 이번 달getClubFeedRanking을 호출하고,getLastMonthRank에서 지난 달getClubFeedRanking을 추가로 호출합니다. 각 호출마다feedRepository.findMonthlyRankingByClub이 실행되므로, 단일 월간 현황 조회에 DB 쿼리가 2회 발생합니다.PR 리뷰 노트에서도 언급된 사항이며, 현재 트래픽 수준에서는 문제없을 수 있지만, 향후 캐싱 또는 단일 쿼리로 최적화하는 것을 고려해 보세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingService.java` around lines 72 - 83, getLastMonthRank currently calls getClubFeedRanking again causing duplicate DB queries; change the API so getLastMonthRank does not call getClubFeedRanking but instead accepts the already-fetched lastMonthRankings (List<ClubFeedRankingQuery>) or a precomputed map from getClubMonthlyStatus, and use that collection to find the club's rank. Update callers (e.g., getClubMonthlyStatus) to pass the last-month rankings returned by getClubFeedRanking (which internally calls feedRepository.findMonthlyRankingByClub) so the repository method is invoked only once per month.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingService.java`:
- Around line 85-100: The weight calculation is duplicated between
calculateScore(...) and toClubFeedRankingQuery(...); remove this redundancy by
computing the weighted total once and reusing it: either eliminate
calculateScore and map MonthlyFeedRankingDto -> ClubFeedRankingQuery first
(using toClubFeedRankingQuery(...) which computes totalScore via the
FEED/VIEW/LIKE/COMMENT_WEIGHTs and ClubFeedRankingQuery.of(...)), then sort by
the resulting ClubFeedRankingQuery.getTotalScore(), or keep calculateScore but
have toClubFeedRankingQuery accept the precomputed score (e.g.,
toClubFeedRankingQuery(int rank, MonthlyFeedRankingDto dto, long totalScore)) so
the multiplication logic exists in only one place (identify methods:
calculateScore, toClubFeedRankingQuery, and ClubFeedRankingQuery.of to update
accordingly).
- Around line 72-83: getLastMonthRank currently calls getClubFeedRanking again
causing duplicate DB queries; change the API so getLastMonthRank does not call
getClubFeedRanking but instead accepts the already-fetched lastMonthRankings
(List<ClubFeedRankingQuery>) or a precomputed map from getClubMonthlyStatus, and
use that collection to find the club's rank. Update callers (e.g.,
getClubMonthlyStatus) to pass the last-month rankings returned by
getClubFeedRanking (which internally calls
feedRepository.findMonthlyRankingByClub) so the repository method is invoked
only once per month.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/ddingdong/ddingdongBE/domain/feed/controller/dto/response/AdminClubFeedRankingResponse.javasrc/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingService.javasrc/main/java/ddingdong/ddingdongBE/domain/feed/service/dto/query/ClubFeedRankingQuery.javasrc/test/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedRankingServiceTest.java
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> (cherry picked from commit c1fd293)
🚀 작업 내용
feedCount/viewCount/likeCount/commentCount/score→feedScore/viewScore/likeScore/commentScore/totalScoreclubId필드를 제거했습니다createEmpty분기로 빠질 때lastMonthRank가 유실되던 버그를 수정했습니다🤔 고민했던 내용
toMonthlyStatus())와 Response 레이어(from())에서 각각 수행합니다. raw count는ClubFeedRankingQuery에 그대로 유지하고, 필요한 시점에 score로 변환하는 구조입니다createdAt이@Column(updatable = false)라setCreatedAt()후 save해도 DB에 반영되지 않는 문제가 있어JdbcTemplate으로 직접 업데이트하는 방식을 사용했습니다💬 리뷰 중점사항
getLastMonthRank()에서 전월 랭킹을 조회할 때getClubFeedRanking()을 한 번 더 호출하는 구조인데, 성능 이슈가 우려된다면 캐싱이나 쿼리 최적화를 고려할 수 있습니다🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation