Conversation
2 tasks
📝 WalkthroughWalkthrough미인증자 수집 및 부실 인증 신고 흐름을 추가하고, 경고 집계·동기화로 UserChallenge 상태(KICKED)와 Challenge.currentParticipants를 갱신하는 배치·서비스·엔드포인트·DB 마이그레이션을 도입했습니다. Changes
Sequence DiagramsequenceDiagram
actor User
participant Controller as ReportController
participant ReportSvc as ReportServiceImpl
participant WeakRepo as WeakVerificationReportRepository
participant RRService as RoundRecordServiceImpl
participant RRRepo as RoundRecordRepository
participant AbsenceSvc as VerificationAbsenceService
participant DB as Database
User->>Controller: POST /api/v1/report/verification/weak
Controller->>ReportSvc: reportWeakVerification(user, verificationId)
ReportSvc->>DB: SELECT Verification FOR UPDATE (pessimistic)
ReportSvc->>ReportSvc: validateChallengeParticipation(...)
alt valid
ReportSvc->>WeakRepo: save(WeakVerificationReport)
WeakRepo->>DB: INSERT weak_verification_report
ReportSvc->>RRService: synchronizeWarnCount(roundRecordId)
RRService->>RRRepo: findByIdWithPessimisticLock(id)
RRRepo->>DB: SELECT RoundRecord FOR UPDATE
RRService->>DB: COUNT weak_verification_report, COUNT verification_absence_log
RRService->>RRService: calculate newWarnCount
RRService->>DB: UPDATE round_record.warn_count
alt newWarnCount >= 3
RRService->>DB: UPDATE user_challenge.status = KICKED
RRService->>DB: COUNT user_challenge WHERE status = JOINED
RRService->>DB: UPDATE challenge.current_participants
end
end
Note over RRService,Controller: 성공 응답 반환
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@src/main/java/com/hrr/backend/domain/report/service/ReportServiceImpl.java`:
- Around line 45-68: Add application-level checks in reportWeakVerification
mirroring reportVerificationPost: add existsByReporterAndVerification to
WeakVerificationReportRepository (Spring Data query method) and then, in
ReportServiceImpl.reportWeakVerification after loading targetVerification, first
check if reporter.equals(targetVerification.getUserChallenge().getUser()) and
throw the same GlobalException/ErrorCode used in reportVerificationPost for
self-reports; next call
weakVerificationReportRepository.existsByReporterAndVerification(reporter,
targetVerification) and if true throw the same GlobalException/ErrorCode used in
reportVerificationPost for duplicate reports; only proceed to save the
WeakVerificationReport if both checks pass.
In
`@src/main/java/com/hrr/backend/domain/round/repository/RoundRecordRepository.java`:
- Around line 93-110: The query in RoundRecordRepository.findAbsentees selects
RoundRecord across all rounds; restrict it to the challenge's current round by
adding a JPQL predicate that compares the RoundRecord round field to
Challenge.currentRound (e.g., "AND rr.round = c.currentRound" or "AND
rr.roundNumber = c.currentRound" depending on the RoundRecord field name).
Update the `@Query` to include this condition so only RoundRecord entries for the
ongoing challenge's currentRound are considered.
In
`@src/main/java/com/hrr/backend/domain/round/service/RoundRecordServiceImpl.java`:
- Around line 39-40: Replace the direct RuntimeException thrown in
RoundRecordServiceImpl when a RoundRecord is not found with a GlobalException
using a new or existing error code; specifically change the orElseThrow on
roundRecordRepository.findById(roundRecordId) to throw new
GlobalException(ErrorCode.ROUND_RECORD_NOT_FOUND) (add
ErrorCode.ROUND_RECORD_NOT_FOUND if it doesn't exist) so the global handler
returns the consistent error response across the application.
- Around line 42-50: The RoundRecord warn-count sync is vulnerable to race
conditions because you fetch and update RoundRecord without a lock so concurrent
calls to synchronizeWarnCount can trigger OptimisticLockException; fix by either
adding a pessimistic lock on the entity retrieval (add a repository method like
findByIdWithPessimisticLock annotated with `@Lock`(LockModeType.PESSIMISTIC_WRITE)
and use it instead of the current findById) or implement a retry on
OptimisticLockException (catch OptimisticLockException around the
synchronizeWarnCount/update sequence and retry a few times or annotate the
service with `@Retryable`); update the code paths that compute calculatedWarnCount
(using weakVerificationReportRepository and verificationAbsenceLogRepository) to
use the locked entity before calling roundRecord.synchronizeWarnCount.
In `@src/main/java/com/hrr/backend/global/response/ErrorCode.java`:
- Line 139: The enum entry CANNOT_REPORT_OTHER_CHALLENGE_VERIFICATION has a
mismatched error code string "VERIFICATION409110" that breaks the existing
VERIFICATION409XX naming pattern; update the string to follow the established
pattern (e.g., change to "VERIFICATION40910" or the next correct sequence such
as "VERIFICATION40920" after confirming the intended code) so the ErrorCode
constant CANNOT_REPORT_OTHER_CHALLENGE_VERIFICATION uses a consistent
VERIFICATION409XX identifier.
In `@src/main/java/com/hrr/backend/global/scheduler/VerificationScheduler.java`:
- Around line 29-53: The checkAbsence method currently runs as one transaction
and must be changed to per-record transactions: remove `@Transactional` from
VerificationScheduler.checkAbsence, extract the per-record work (saving
VerificationAbsenceLog and calling roundRecordService.synchronizeWarnCount) into
a separate Spring bean method (e.g., AbsenceProcessor.processAbsentee) annotated
with `@Transactional`(propagation = REQUIRES_NEW), and have checkAbsence call that
bean for each RoundRecord; wrap each call in try/catch so failures log the
record id and exception (do not rethrow) and continue, and add simple counters
(processed/failures) to log a summary at the end. Ensure you reference
VerificationAbsenceLog saving (verificationAbsenceLogRepository.save) and
roundRecordService.synchronizeWarnCount(record.getId()) inside the new
transactional method.
In `@src/main/resources/db/migration/V2.33__create_table_related_warncount.sql`:
- Around line 6-8: The information_schema.COLUMNS check for IF NOT EXISTS is
missing a TABLE_SCHEMA filter, which can cause false negatives if the same
table/column exists in another schema; modify the IF NOT EXISTS query that
currently filters WHERE TABLE_NAME = 'round_record' AND COLUMN_NAME = 'version'
to also restrict by TABLE_SCHEMA (e.g. TABLE_SCHEMA = DATABASE() or the specific
schema name used by your migrations) so the check only examines the intended
schema.
In `@src/test/resources/application-test.yml`:
- Line 9: globally_quoted_identifiers 속성이 현재 잘못된 위치에 있어 Hibernate에 전달되지 않으므로
application-test.yml에서 해당 설정을
spring.jpa.properties.hibernate.globally_quoted_identifiers: true 아래로 이동하세요; 즉
기존 globally_quoted_identifiers 항목을 제거하고 spring.jpa.properties.hibernate 계층(예:
spring.jpa.properties.hibernate.globally_quoted_identifiers)으로 옮겨 Hibernate 네이티브
속성으로 전달되도록 수정하세요.
🧹 Nitpick comments (6)
src/main/java/com/hrr/backend/domain/challenge/entity/Challenge.java (1)
109-112: 입력값 검증 없이 참가자 수를 직접 설정하는 점 확인
decreaseCurrentParticipants()는 음수 방지 가드가 있지만, 이 메서드는 음수나maxParticipants초과 값도 허용합니다. 현재 호출부에서 DB 집계 결과를 전달하므로 즉시 문제가 되진 않지만, 방어적 검증을 추가하면 향후 오용을 예방할 수 있습니다.🛡️ 방어 로직 제안
public void updateCurrentParticipants(int currentParticipants) { + if (currentParticipants < 0) { + throw new IllegalArgumentException("currentParticipants must be non-negative"); + } this.currentParticipants = currentParticipants; }src/main/java/com/hrr/backend/domain/round/repository/RoundRecordRepository.java (1)
102-106:CAST(v.createdAt AS LocalDate)성능 고려
NOT EXISTS서브쿼리 내에서 행마다CAST를 수행하면 인덱스 활용이 어려워 대량 데이터에서 성능이 저하될 수 있습니다.verification테이블에(round_record_id, created_at)복합 인덱스가 있다면, 범위 조건(v.createdAt >= :yesterdayStart AND v.createdAt < :todayStart)으로 변경하는 것이 인덱스 스캔에 유리합니다.현재 스케줄러가 새벽에 1회 실행되므로 당장 병목은 아닐 수 있지만, 데이터가 커지면 고려해 볼 사항입니다.
src/main/java/com/hrr/backend/domain/round/entity/RoundRecord.java (1)
90-99: 엔티티가 다른 엔티티의 상태를 직접 변경하고 있습니다.
RoundRecord엔티티가UserChallenge의 상태를 직접 변경하는 것은 도메인 경계(aggregate boundary)를 넘는 행위입니다. 이 로직은 서비스 레이어(RoundRecordServiceImpl)에서 처리하는 것이 더 적절합니다. 서비스에서synchronizeWarnCount호출 후 별도로userChallenge.updateStatus()를 호출하면, 각 엔티티의 책임이 명확해지고 테스트도 용이해집니다.또한 두 가지 추가 사항:
- 매직 넘버
3: 퇴출 기준 경고 횟수를 상수로 추출하세요. 비즈니스 규칙 변경 시 여러 곳을 수정해야 하는 리스크가 생깁니다.- Line 93의 불필요한 주석
//: 빈 주석이 남아있습니다.♻️ 리팩토링 제안
+ private static final int KICK_THRESHOLD = 3; + public void synchronizeWarnCount(int newWarnCount) { - this.warnCount = newWarnCount; // - - // 경고가 3회 이상이면 해당 유저의 챌린지 참여 상태를 KICKED로 변경 - if (this.warnCount >= 3) { - this.userChallenge.updateStatus(ChallengeJoinStatus.KICKED); - } + this.warnCount = newWarnCount; + } + + public boolean shouldBeKicked() { + return this.warnCount >= KICK_THRESHOLD; }그리고 서비스 레이어에서:
record.synchronizeWarnCount(calculatedWarnCount); if (record.shouldBeKicked()) { record.getUserChallenge().updateStatus(ChallengeJoinStatus.KICKED); processKickOutSideEffects(challengeId); }src/main/java/com/hrr/backend/domain/report/controller/ReportController.java (1)
36-47:@Schema대신@Parameter를 사용하세요.
@RequestParam에는@Schema가 아닌 Swagger의@Parameter어노테이션이 적합합니다.@Schema는 주로 DTO 필드나@RequestBody에 사용됩니다. 이미 다른 파라미터에서@Parameter를 사용하고 있으므로 일관성을 맞추는 것이 좋습니다.참고: SpringDoc OpenAPI 공식 문서 -
@Parametervs@Schema♻️ 수정 제안
public ApiResponse<Void> reportWeakVerification( - `@Schema`(description = "신고하려는 인증 ID; verificationID", example = "1") + `@Parameter`(description = "신고하려는 인증 ID (verificationId)", example = "1") `@NotNull`(message = "신고 대상 ID는 필수입니다.") `@RequestParam` Long targetId,src/test/java/com/hrr/backend/domain/round/RoundRecordServiceTest.java (1)
43-68: 경고 3회 미만(퇴출 미발생) 시나리오 테스트가 누락되었습니다.현재 두 테스트 모두
warnCount == 3으로 퇴출이 발생하는 케이스만 검증합니다. 경고가 3회 미만일 때KICKED상태로 변경되지 않는 것을 확인하는 테스트도 추가하면,synchronizeWarnCount내부 분기 로직의 정확성을 보다 완전히 검증할 수 있습니다.💡 추가 테스트 예시
`@Test` `@DisplayName`("경고 횟수가 3 미만이면 퇴출되지 않는다") void shouldNotKickWhenWarnCountBelowThreshold() { // given Long recordId = 1L; Challenge challenge = Challenge.builder().id(100L).currentParticipants(10).build(); UserChallenge userChallenge = UserChallenge.builder().challenge(challenge).status(ChallengeJoinStatus.JOINED).build(); RoundRecord roundRecord = RoundRecord.builder().id(recordId).userChallenge(userChallenge).warnCount(0).build(); given(roundRecordRepository.findById(recordId)).willReturn(Optional.of(roundRecord)); given(weakReportRepository.countByRoundRecordId(recordId)).willReturn(2L); // 2 / 3 = 0 given(absenceLogRepository.countByRoundRecordId(recordId)).willReturn(1L); // 1 // when roundRecordService.synchronizeWarnCount(recordId); // then: (2 / 3) + 1 = 1, 퇴출 조건 미달 assertEquals(1, roundRecord.getWarnCount()); assertEquals(ChallengeJoinStatus.JOINED, userChallenge.getStatus()); verify(challengeRepository, never()).findById(any()); }src/main/java/com/hrr/backend/domain/verification/entity/VerificationAbsenceLog.java (1)
24-39:@AllArgsConstructor의 접근 제한자를PROTECTED로 통일하세요.같은 PR에서 추가된
WeakVerificationReport엔티티는@AllArgsConstructor(access = AccessLevel.PROTECTED)를 사용하고 있습니다 (WeakVerificationReport.javaLine 17). 반면 이 엔티티는 기본값(public)으로 되어 있어 일관성이 떨어집니다.JPA 엔티티에서
@Builder와 함께 사용할 때도PROTECTED면 충분하며, 외부에서 직접 생성자를 호출하는 것을 방지할 수 있습니다. 참고: JPA 엔티티 생성자 접근 제한 관련 Hibernate 문서🔧 수정 제안
-@AllArgsConstructor +@AllArgsConstructor(access = AccessLevel.PROTECTED)
src/main/java/com/hrr/backend/domain/round/repository/RoundRecordRepository.java
Show resolved
Hide resolved
src/main/java/com/hrr/backend/domain/round/service/RoundRecordServiceImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/com/hrr/backend/domain/round/service/RoundRecordServiceImpl.java
Show resolved
Hide resolved
src/main/java/com/hrr/backend/global/scheduler/VerificationScheduler.java
Show resolved
Hide resolved
src/main/resources/db/migration/V2.33__create_table_related_warncount.sql
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/main/java/com/hrr/backend/domain/report/service/ReportServiceImpl.java`:
- Around line 55-63: The indicated if-blocks are over-indented relative to
surrounding code; normalize their indentation to match the method's standard
level so both checks (the self-report check using
targetVerification.getUserChallenge().getUser().getId().equals(reporter.getId())
which throws new GlobalException(ErrorCode.CANNOT_REPORT_OWN_POST) and the
duplicate-report check using
weakVerificationReportRepository.existsByReporterAndVerification(reporter,
targetVerification) which throws new
GlobalException(ErrorCode.ALREADY_REPORTED)) align with the other statements in
the method; adjust whitespace only—no logic changes.
- Around line 45-78: After loading targetVerification in reportWeakVerification,
add the same BLOCKED-state guard used in reportVerificationPost: check if
targetVerification.getStatus() == VerificationStatus.BLOCKED and if so throw new
GlobalException(ErrorCode.ACCESS_DENIED_REPORTED_POST); place this check
immediately after the findByIdWithPessimisticLock result (before
duplicate-report and self-report checks) so blocked verifications cannot be
reported and won't accumulate warnings.
- Around line 85-89: The validateChallengeParticipation method currently only
checks existence via userChallengeRepository.existsByUserAndChallenge and
therefore allows users with KICKED or DROPPED statuses to pass; update the
repository to expose a method like existsByUserAndChallengeAndStatus(User user,
Challenge challenge, ChallengeJoinStatus status) and change
validateChallengeParticipation to call
userChallengeRepository.existsByUserAndChallengeAndStatus(reporter, challenge,
ChallengeJoinStatus.JOINED), throwing
GlobalException(ErrorCode.CANNOT_REPORT_OTHER_CHALLENGE_VERIFICATION) when false
so only JOINED participants can report.
🧹 Nitpick comments (5)
src/main/resources/db/migration/V2.33__create_table_related_warncount.sql (1)
13-25: FKON DELETE정책 검토 필요모든 외래 키에
ON DELETE절이 생략되어 기본값인RESTRICT가 적용됩니다. 만약verification이나user레코드 삭제 시 해당 신고 데이터도 함께 정리되어야 한다면, 부모 행 삭제가 차단될 수 있습니다.의도적으로
RESTRICT를 선택한 것이라면 괜찮지만, 향후 챌린지/유저 탈퇴 등의 시나리오에서 삭제 순서를 고려해야 합니다. 참고: MySQL FK Constraints 문서또한
round_record_id는verification→round_record관계에서 파생 가능한 값이므로 비정규화된 컬럼입니다. 경고 집계 쿼리 성능을 위한 의도적 설계라면 문제없지만, 데이터 정합성 유지에 주의가 필요합니다.src/main/java/com/hrr/backend/domain/round/service/RoundRecordServiceImpl.java (3)
35-56: 이미 KICKED 상태인 사용자에 대해processKickOutSideEffects가 반복 실행될 수 있습니다.
synchronizeWarnCount가 호출될 때마다 Line 53에서 현재 상태가KICKED인지만 확인합니다. 이전 호출에서 이미 퇴출 처리된 사용자에 대해 추가 신고가 들어오면,processKickOutSideEffects가 불필요하게 재실행됩니다. 현재 로직은 멱등(idempotent)하기 때문에 데이터 무결성 문제는 없지만, 이전 warn count와 비교하여 새로 KICKED 상태가 된 경우에만 실행하도록 개선하면 불필요한 DB 조회를 줄일 수 있습니다.♻️ 개선 제안
synchronizeWarnCount엔티티 메서드가 상태 변경 여부를 반환하도록 하거나, 호출 전 상태를 캡처하여 비교하는 방식을 고려하세요:+ ChallengeJoinStatus previousStatus = roundRecord.getUserChallenge().getStatus(); + // 경고 횟수 동기화 및 챌린지 퇴출 여부 판단 roundRecord.synchronizeWarnCount(calculatedWarnCount); - if (roundRecord.getUserChallenge().getStatus() == ChallengeJoinStatus.KICKED) { + if (previousStatus != ChallengeJoinStatus.KICKED + && roundRecord.getUserChallenge().getStatus() == ChallengeJoinStatus.KICKED) { processKickOutSideEffects(roundRecord.getUserChallenge().getChallenge().getId()); }
58-72:processKickOutSideEffects에서 Challenge를 별도 조회하는 것은 불필요할 수 있습니다.
synchronizeWarnCount메서드 내에서 이미roundRecord.getUserChallenge().getChallenge()를 통해 Challenge에 접근 가능합니다. 같은 트랜잭션 내 영속성 컨텍스트에서 관리되는 엔티티이므로,challengeRepository.findById로 재조회하지 않고 직접 전달하면 불필요한 쿼리를 줄일 수 있습니다.다만,
findById가 1차 캐시에서 반환될 가능성이 높아 실질적인 성능 영향은 크지 않습니다. 참고 수준의 개선 사항입니다.
46-47: 경고 횟수 계산에 사용되는 매직 넘버를 상수로 추출하세요.
3(부실 신고 → 경고 전환 기준)이 여기와 비즈니스 로직 설명에 하드코딩되어 있습니다. 추후 정책 변경 시 여러 곳을 수정해야 할 수 있으므로, 의미 있는 상수로 추출하면 가독성과 유지보수성이 향상됩니다.♻️ 상수 추출 제안
public class RoundRecordServiceImpl implements RoundRecordService { + private static final int WEAK_REPORTS_PER_WARNING = 3; + private final RoundRecordRepository roundRecordRepository; ... - int calculatedWarnCount = (int) (weakReportCount / 3) + (int) absenceCount; + int calculatedWarnCount = (int) (weakReportCount / WEAK_REPORTS_PER_WARNING) + (int) absenceCount;src/main/java/com/hrr/backend/domain/round/repository/RoundRecordRepository.java (1)
102-119:CAST(v.createdAt AS LocalDate)사용 시 DB 인덱스 활용 불가 가능성을 인지하세요.Line 114의
CAST(v.createdAt AS LocalDate) = :yesterdayDate는createdAt컬럼에 함수를 적용하므로, 해당 컬럼의 인덱스를 사용할 수 없습니다 (full scan on the NOT EXISTS subquery). 현재는 야간 스케줄러에서 실행되므로 즉각적인 성능 문제는 낮지만, 데이터가 증가하면 병목이 될 수 있습니다.대안으로 날짜 범위 비교(
v.createdAt >= :yesterdayStart AND v.createdAt < :todayStart)를 사용하면 인덱스를 활용할 수 있습니다.♻️ 범위 비교 방식 제안
"AND NOT EXISTS ( " + " SELECT v FROM Verification v " + " WHERE v.roundRecord = rr " + - " AND CAST(v.createdAt AS LocalDate) = :yesterdayDate " + + " AND v.createdAt >= :yesterdayStart " + + " AND v.createdAt < :todayStart " + ")") List<RoundRecord> findAbsentees( - `@Param`("yesterdayChallengeDay") ChallengeDays yesterdayChallengeDay, - `@Param`("yesterdayDate") LocalDate yesterdayDate + `@Param`("yesterdayChallengeDay") ChallengeDays yesterdayChallengeDay, + `@Param`("yesterdayStart") LocalDateTime yesterdayStart, + `@Param`("todayStart") LocalDateTime todayStart );호출부에서
yesterdayStart = yesterday.atStartOfDay(),todayStart = today.atStartOfDay()로 전달하면 됩니다.,
src/main/java/com/hrr/backend/domain/report/service/ReportServiceImpl.java
Show resolved
Hide resolved
src/main/java/com/hrr/backend/domain/report/service/ReportServiceImpl.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/main/java/com/hrr/backend/domain/report/service/ReportServiceImpl.java`:
- Around line 46-85: reportWeakVerification must also bail out when the owner of
the verification is already kicked; after loading targetRecord (or after
fetching targetVerification), check the kicked status on targetRecord and/or
targetVerification.getUserChallenge() (e.g., compare to a KICKED enum on
RoundRecord or UserChallenge) and return early by throwing an appropriate
GlobalException (reuse ErrorCode.ACCESS_DENIED_REPORTED_POST or add a new error
code) instead of continuing to save a report and calling
roundRecordService.synchronizeWarnCount.
In `@src/main/java/com/hrr/backend/global/scheduler/VerificationScheduler.java`:
- Around line 27-30: Remove the unused injected dependencies from
VerificationScheduler: delete the fields verificationAbsenceLogRepository and
roundRecordService and remove them from the constructor (and any constructor
assignments), leaving only verificationAbsenceService; also remove any imports
that were only used by those two types so the class compiles cleanly and relies
on VerificationAbsenceService for the moved logic.
🧹 Nitpick comments (2)
src/main/java/com/hrr/backend/domain/verification/service/VerificationAbsenceService.java (2)
16-18:@Component대신@Service사용을 권장합니다.이 클래스는 비즈니스 로직을 처리하는 서비스 레이어에 해당합니다. Spring에서
@Service는@Component의 특수화로, 서비스 계층의 의도를 명확히 전달합니다. (Spring 공식 문서 -@Service참고)♻️ 수정 제안
-import org.springframework.stereotype.Component; +import org.springframework.stereotype.Service; -@Component +@Service
22-32:REQUIRES_NEW트랜잭션에서 외부 트랜잭션의 엔티티 참조 시 Best Practice 적용을 권장합니다.현재 코드는 실제로 안전합니다.
VerificationAbsenceLog.roundRecord관계에 cascade 설정이 없으므로PersistentObjectException이 발생하지 않습니다. FK만 저장되고 엔티티 자체는 cascade persist되지 않기 때문입니다.다만,
REQUIRES_NEW는 새로운 트랜잭션과 독립된 영속성 컨텍스트를 생성하므로, 외부 트랜잭션에서 로드된 managed 엔티티를 직접 참조하는 것은 의도와 명확성 측면에서 개선할 여지가 있습니다.권장 개선안: ID만 전달하고 새 트랜잭션 내에서 재조회하면 다음 이점이 있습니다:
- 명시적으로 트랜잭션 경계를 분리
- 외부 트랜잭션의 엔티티 상태에 대한 의존성 제거
- 향후 cascade 설정 추가 시에도 안전
🛡️ 권장 구현 방식
- public void processAbsentee(RoundRecord record, LocalDate date) { + public void processAbsentee(Long roundRecordId, LocalDate date) { + RoundRecord record = roundRecordRepository.findById(roundRecordId) + .orElseThrow(() -> new IllegalStateException("RoundRecord not found: " + roundRecordId)); + // 미인증 로그 저장 absenceLogRepository.save(VerificationAbsenceLog.builder() .roundRecord(record) .absenceDate(date) .build()); // 경고 횟수를 업데이트 후 퇴출 여부를 결정하는 메소드 호출 - roundRecordService.synchronizeWarnCount(record.getId()); + roundRecordService.synchronizeWarnCount(roundRecordId); }
Comment on lines
+46
to
+85
| @Override | ||
| @Transactional | ||
| public void reportWeakVerification(User reporter,Long verificationId) { | ||
| // 신고 대상 조회(Verification) | ||
| Verification targetVerification = verificationRepository.findByIdWithPessimisticLock(verificationId) | ||
| .orElseThrow(() -> new GlobalException(ErrorCode.VERIFICATION_NOT_FOUND)); | ||
|
|
||
| // 차단 확인 (가장 먼저!) | ||
| // 이미 게시글 신고 5회 누적으로 차단된 글이라면 다른 검증을 할 필요도 없이 바로 예외를 던짐 | ||
| if (VerificationStatus.BLOCKED.equals(targetVerification.getStatus())) { | ||
| throw new GlobalException(ErrorCode.ACCESS_DENIED_REPORTED_POST); | ||
| } | ||
|
|
||
| // 피신고자 정보 조회(RoundRecord) | ||
| RoundRecord targetRecord = targetVerification.getRoundRecord(); | ||
|
|
||
| // 자기 신고 방지 | ||
| if (targetVerification.getUserChallenge().getUser().getId().equals(reporter.getId())) { | ||
| throw new GlobalException(ErrorCode.CANNOT_REPORT_OWN_POST); | ||
| } | ||
|
|
||
| // 중복 신고 방지 | ||
| if (weakVerificationReportRepository.existsByReporterAndVerification(reporter, targetVerification)) { | ||
| throw new GlobalException(ErrorCode.ALREADY_REPORTED); | ||
| } | ||
|
|
||
| // 권한 검증: 신고자와 피신고자가 동일 챌린지에 참여 중인지 확인 - 검증 완료 시 다음 단계로 이동 | ||
| validateChallengeParticipation(reporter, targetVerification.getUserChallenge().getChallenge()); | ||
|
|
||
| // 신고 내역 저장 | ||
| WeakVerificationReport report = WeakVerificationReport.builder() | ||
| .reporter(reporter) | ||
| .verification(targetVerification) | ||
| .roundRecord(targetRecord) | ||
| .build(); | ||
| weakVerificationReportRepository.save(report); | ||
|
|
||
| // 경고 횟수를 업데이트 후 퇴출 여부를 결정하는 메소드 호출 | ||
| roundRecordService.synchronizeWarnCount(targetRecord.getId()); | ||
| } |
There was a problem hiding this comment.
이미 퇴출(KICKED)된 사용자의 인증에 대한 부실 인증 신고 처리가 누락되어 있습니다.
현재 로직은 인증 글이 BLOCKED인지만 확인하지만, 해당 인증의 소유자가 이미 챌린지에서 퇴출된 상태인지는 검증하지 않습니다. 퇴출된 사용자의 인증에 대해 신고가 접수되면 불필요한 경고가 누적되고, synchronizeWarnCount가 이미 KICKED 상태인 레코드를 다시 처리하게 됩니다.
targetRecord 또는 관련 UserChallenge의 상태를 확인하여 이미 퇴출된 경우 조기 반환하는 것을 고려해 주세요.
🤖 Prompt for AI Agents
In `@src/main/java/com/hrr/backend/domain/report/service/ReportServiceImpl.java`
around lines 46 - 85, reportWeakVerification must also bail out when the owner
of the verification is already kicked; after loading targetRecord (or after
fetching targetVerification), check the kicked status on targetRecord and/or
targetVerification.getUserChallenge() (e.g., compare to a KICKED enum on
RoundRecord or UserChallenge) and return early by throwing an appropriate
GlobalException (reuse ErrorCode.ACCESS_DENIED_REPORTED_POST or add a new error
code) instead of continuing to save a report and calling
roundRecordService.synchronizeWarnCount.
src/main/java/com/hrr/backend/global/scheduler/VerificationScheduler.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/hrr/backend/domain/challenge/service/ChallengeServiceImpl.java (1)
521-534:⚠️ Potential issue | 🟡 MinorKICKED 상태의 유저가 대기 신청할 수 있는 허점이 있습니다.
JOINED상태만 체크하도록 변경한 것은DROPPED유저의 재참여 흐름을 위해 올바른 방향입니다. 👍하지만
validateJoinRequest(Line 698-700)에서KICKED유저는 챌린지 참가 자체가 차단되므로, 대기 신청도 함께 차단해야 합니다. 현재 코드에서는 퇴출된 유저가 대기 목록에 등록은 되지만 실제 참여는 불가능한 불일치가 발생합니다.🛡️ KICKED 유저 대기 신청 차단 제안
public void registerChallengeWait(User user, Long challengeId) { // 챌린지 조회 Challenge challenge = findChallenge(challengeId); - // 챌린지 참여 여부 확인 - if (userChallengeRepository.existsByUserAndChallengeAndStatus(user, challenge, ChallengeJoinStatus.JOINED)) { - throw new GlobalException(ErrorCode.CHALLENGE_ALREADY_JOINED); - } + // 챌린지 참여/퇴출 여부 확인 + Optional<UserChallenge> existingUc = userChallengeRepository.findByUserAndChallenge(user, challenge); + if (existingUc.isPresent()) { + ChallengeJoinStatus status = existingUc.get().getStatus(); + if (status == ChallengeJoinStatus.JOINED) { + throw new GlobalException(ErrorCode.CHALLENGE_ALREADY_JOINED); + } + if (status == ChallengeJoinStatus.KICKED) { + throw new GlobalException(ErrorCode.CHALLENGE_KICKED_USER); + } + }이렇게 하면
JOINED와KICKED모두 차단하면서,DROPPED유저만 대기 신청이 가능해집니다.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#️⃣ 연관된 이슈
✨ 작업 내용 (Summary)
인증 관련 경고 누적 시 해당 챌린지에서 퇴출되는 기능을 구현하였습니다. 플로우는 다음과 같습니다.
미인증 1회 또는 부실인증 신고 3회 누적 -> 경고 1회
경고 3회 누적 -> 챌린지 퇴출
미인증은 매일 00시 05분에 전날 인증 요일이었는데 인증 기록이 없을 경우 DB에 기록이 남고,
부실신고는 인증 게시글에서, 해당 챌린지에 참여하고 있는 챌린저들이 신고 시 DB에 기록이 남고 중복 없이 1회만 신고 가능합니다.
✅ 변경 사항 체크리스트
🧪 테스트 결과
📸 스크린샷
💬 리뷰 요구사항
📎 참고 자료
Summary by CodeRabbit
New Features
Data & Schema
Tests