Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,13 @@ public class WeeklyRanking extends TimeBaseEntity {
@Column(name = "week_end", nullable = false)
private LocalDate weekEnd;

public void setRank(int rank) {
if (this.rank != rank) {
this.rank = rank;
}
public void updatePointAndCertification(BigDecimal totalPoint, int certificationCount) {
this.totalPoint = totalPoint;
this.certificationCount = certificationCount;
}

public void updateRank(int rank) {
this.rank = rank;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@
import org.springframework.data.jpa.repository.JpaRepository;

import com.example.green.domain.dashboard.rankingmodule.entity.WeeklyRanking;
import com.querydsl.core.Tuple;

public interface WeeklyRankingRepository extends JpaRepository<WeeklyRanking, Long>, WeeklyRankingRepositoryCustom {
List<Tuple> findByWeekStart(LocalDate weekStart);

void deleteByWeekStart(LocalDate weekStart);
List<WeeklyRanking> findByWeekStart(LocalDate weekStart);
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,7 @@ public interface WeeklyRankingRepositoryCustom {
List<WeeklyRanking> findTopNByWeekStart(LocalDate weekStart, int topN);

List<WeeklyRanking> findAllRankings();

List<WeeklyRanking> findTop8ByWeekStart(LocalDate weekStart);
}

Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import java.util.List;
import java.util.Optional;

import org.springframework.data.repository.query.Param;

import com.example.green.domain.challenge.entity.challenge.QParticipation;
import com.example.green.domain.dashboard.rankingmodule.entity.QWeeklyRanking;
import com.example.green.domain.dashboard.rankingmodule.entity.WeeklyRanking;
Expand Down Expand Up @@ -97,4 +99,17 @@ public List<WeeklyRanking> findAllRankings() {
.fetch();
}

@Override
public List<WeeklyRanking> findTop8ByWeekStart(@Param("weekStart") LocalDate weekStart) {

QWeeklyRanking weeklyRanking = QWeeklyRanking.weeklyRanking;

return queryFactory
.selectFrom(weeklyRanking)
.where(weeklyRanking.weekStart.eq(weekStart))
.orderBy(weeklyRanking.rank.asc())
.limit(8) // TOP 8
.fetch();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,6 @@ public class WeeklyRankingService {
@Transactional
public void updateWeeklyRanks(LocalDate weekStart, LocalDate weekEnd) {

// 기존 주차 랭킹 삭제
weeklyRankingRepository.deleteByWeekStart(weekStart);

// 모든 회원 조회
List<Member> allMembers = memberRepository.findAll();
List<Long> memberIds = allMembers.stream()
Expand All @@ -65,60 +62,62 @@ public void updateWeeklyRanks(LocalDate weekStart, LocalDate weekEnd) {
MemberCertifiedCountProjection::getCertifiedCount
));

List<WeeklyRanking> weeklyRankings = allMembers.stream()
.map(member -> {
BigDecimal point = memberPoints.getOrDefault(member.getId(), BigDecimal.ZERO);
int certificationCount = certifiedCounts.getOrDefault(member.getId(), 0);
String profileImageUrl = (member.getProfile() != null) ?
member.getProfile().getProfileImageUrl() : null;

return WeeklyRanking.builder()
.memberId(member.getId())
.memberName(member.getName())
.profileImageUrl(profileImageUrl)
.totalPoint(point) // DB 저장/정렬용
.certificationCount(certificationCount)
.weekStart(weekStart)
.weekEnd(weekEnd)
.rank(0)
.build();
})
.toList();
Map<Long, WeeklyRanking> existingMap = weeklyRankingRepository
.findByWeekStart(weekStart)
.stream()
.collect(Collectors.toMap(
WeeklyRanking::getMemberId,
w -> w
));

List<WeeklyRanking> updatedRanks = new ArrayList<>();

for (Member member : allMembers) {

BigDecimal point = memberPoints.getOrDefault(member.getId(), BigDecimal.ZERO);

int certificationCount = certifiedCounts.getOrDefault(member.getId(), 0);

String profileImageUrl = (member.getProfile() != null) ?
member.getProfile().getProfileImageUrl() : null;

WeeklyRanking existing = existingMap.get(member.getId());

if (existing != null) {
existing.updatePointAndCertification(point, certificationCount);
updatedRanks.add(existing);
}
}
Comment on lines +75 to +90

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

New members will never appear in rankings.

The current logic only updates existing WeeklyRanking entries. Members without a pre-existing entry for weekStart are silently skipped (lines 86-89). This means:

  • New members can never enter the rankings
  • The initial population of rankings is not handled

If this is intentional (e.g., initial entries are created elsewhere), please clarify. Otherwise, you need to create new entries for members not in existingMap.

 if (existing != null) {
     existing.updatePointAndCertification(point, certificationCount);
     updatedRanks.add(existing);
+} else {
+    WeeklyRanking newRanking = WeeklyRanking.builder()
+        .memberId(member.getId())
+        .memberName(member.getName())
+        .profileImageUrl(profileImageUrl)
+        .totalPoint(point)
+        .certificationCount(certificationCount)
+        .rank(0) // will be set below
+        .weekStart(weekStart)
+        .weekEnd(weekEnd)
+        .build();
+    updatedRanks.add(newRanking);
 }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In
src/main/java/com/example/green/domain/dashboard/rankingmodule/service/WeeklyRankingService.java
around lines 75 to 90, the loop only updates existing WeeklyRanking objects and
skips members missing from existingMap, preventing new members from appearing in
rankings; modify the loop so that when existingMap.get(member.getId()) is null
you create a new WeeklyRanking for the current weekStart (populate member
id/reference, point, certificationCount and any required default fields), add it
to updatedRanks (and to whatever repository/collection will be persisted), and
ensure any necessary constructors or factory methods are used to set initial
rank state consistently with existing entries.


// 정렬 (포인트 내림차순, 동점이면 인증 수 내림차순)
weeklyRankings.sort(
updatedRanks.sort(
Comparator.comparing(WeeklyRanking::getTotalPoint, Comparator.reverseOrder())
.thenComparing(WeeklyRanking::getCertificationCount, Comparator.reverseOrder())
);

int rankCounter = 1;
for (int i = 0; i < weeklyRankings.size(); i++) {
int ranking = 1;
for (int i = 0; i < updatedRanks.size(); i++) {

if (i > 0 &&
weeklyRankings.get(i).getTotalPoint().compareTo(weeklyRankings.get(i - 1).getTotalPoint()) == 0 &&
weeklyRankings.get(i).getCertificationCount() ==
weeklyRankings.get(i - 1).getCertificationCount()) {
updatedRanks.get(i).getTotalPoint().compareTo(updatedRanks.get(i - 1).getTotalPoint()) == 0 &&
updatedRanks.get(i).getCertificationCount() ==
updatedRanks.get(i - 1).getCertificationCount()) {

// 완전 동점 → 이전 rank와 동일
weeklyRankings.get(i).setRank(weeklyRankings.get(i - 1).getRank());
updatedRanks.get(i).updateRank(updatedRanks.get(i - 1).getRank());
} else {
weeklyRankings.get(i).setRank(rankCounter);
updatedRanks.get(i).updateRank(ranking);
}
rankCounter++;

ranking++;
}

// DB 저장
weeklyRankingRepository.saveAll(weeklyRankings);
weeklyRankingRepository.saveAll(updatedRanks);
}

public List<TopMemberPointResponse> getAllRankData(LocalDate weekStart, int topN) {

// 상위 N명 랭킹 엔티티 조회
List<WeeklyRanking> topMembersFromDb = weeklyRankingRepository.findTopNByWeekStart(weekStart,
topN);

if (topMembersFromDb.isEmpty()) {
return new ArrayList<>();
}
List<WeeklyRanking> topMembersFromDb = weeklyRankingRepository.findTop8ByWeekStart(weekStart);
Comment on lines 117 to +120

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

The topN parameter is ignored.

The method signature accepts topN but always calls findTop8ByWeekStart, ignoring the parameter. This is misleading to callers.

Either use the parameter:

-List<WeeklyRanking> topMembersFromDb = weeklyRankingRepository.findTop8ByWeekStart(weekStart);
+List<WeeklyRanking> topMembersFromDb = weeklyRankingRepository.findTopNByWeekStart(weekStart, topN);

Or remove it from the signature if top-8 is always the requirement:

-public List<TopMemberPointResponse> getAllRankData(LocalDate weekStart, int topN) {
+public List<TopMemberPointResponse> getAllRankData(LocalDate weekStart) {
🤖 Prompt for AI Agents
In
src/main/java/com/example/green/domain/dashboard/rankingmodule/service/WeeklyRankingService.java
around lines 117-120, the method getAllRankData(LocalDate weekStart, int topN)
ignores the topN parameter and always calls
weeklyRankingRepository.findTop8ByWeekStart(weekStart); change this so the
method uses the topN value (e.g., call a repository method that accepts a limit
such as findTopNByWeekStart(weekStart, topN) or use a pageable/limit query) or
if the business requirement is fixed to 8, remove the topN parameter from the
signature and update callers accordingly; ensure repository and tests are
updated to match the chosen approach.


// 엔티티 → DTO 변환
return topMembersFromDb.stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,15 @@

import java.math.BigDecimal;
import java.time.LocalDate;
import java.util.List;
import java.util.Optional;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;

import com.example.green.domain.dashboard.rankingmodule.dto.response.MemberPointResponse;
import com.example.green.domain.dashboard.rankingmodule.dto.response.TopMemberPointResponse;
import com.example.green.domain.dashboard.rankingmodule.entity.WeeklyRanking;
import com.example.green.domain.dashboard.rankingmodule.repository.WeeklyRankingRepository;
import com.example.green.domain.dashboard.rankingmodule.service.WeeklyRankingService;
Expand Down Expand Up @@ -66,24 +63,6 @@ void setUp() {
.build();
}

@Test
@DisplayName("상위 랭킹 조회 성공")
void 상위_랭킹_조회_성공() {
// given
when(weeklyRankingRepository.findTopNByWeekStart(any(), anyInt()))
.thenReturn(List.of(rank1, rank2));

// when
List<TopMemberPointResponse> response = weeklyRankingService.getAllRankData(weekStart, 2);

// then
assertThat(response).hasSize(2);
assertThat(response.get(0).nickname()).isEqualTo("홍길동");
assertThat(response.get(1).nickname()).isEqualTo("김철수");

verify(weeklyRankingRepository, times(1)).findTopNByWeekStart(any(), anyInt());
}

@Test
void 내_주간_기록_조회_성공() {
// given
Expand Down