Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,10 @@ public ResponseEntity<ApiResponse<Void>> updateHabit(
@Operation(summary = "습관 재도전")
@PatchMapping("/failed/{habitId}/status/in-progress")
public ResponseEntity<ApiResponse<Void>> retryFailedHabits(
@AuthenticationPrincipal Long userId, @PathVariable Long habitId) {
habitService.retryFailedHabit(userId, habitId);
@AuthenticationPrincipal Long userId,
@PathVariable Long habitId,
@Valid @RequestBody HabitRetryRequest request) {
habitService.retryFailedHabit(userId, habitId, request);
return ResponseEntity.ok(ApiResponse.success(null));
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.swyp.server.domain.habit.dto;

import com.swyp.server.domain.habit.entity.HabitDuration;
import jakarta.validation.constraints.NotNull;

public record HabitRetryRequest(
@NotNull(message = "HABIT_DURATION_REQUIRED") HabitDuration duration, String reward) {}
Comment on lines +6 to +7

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

reward lacks validation and user-type constraints.

Unlike HabitRewardUpdateRequest which uses @NotNull(message = "HABIT_REWARD_REQUIRED"), and unlike HabitCreateRequest/HabitUpdateRequest handling in HabitService which enforces non-blank reward for CHILD users (HABIT_REWARD_REQUIRED) and nulls it for PARENT users, this DTO accepts any reward value without constraints.

Per issue #114, parents should only modify the duration on retry (not the reward), while children must provide a valid reward. The current DTO + Habit.retry implementation lets a parent overwrite reward and lets a child submit null/blank reward, silently persisting it (the reward column has no DB-level constraint either — see Habit.java:38).

Consider aligning with the existing pattern by applying the same user-type-aware reward handling in HabitService.retryFailedHabit (see next comment on Habit.java).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/com/swyp/server/domain/habit/dto/HabitRetryRequest.java` around
lines 6 - 7, The HabitRetryRequest currently allows any reward value; update it
to enforce reward validation and mirror existing patterns: annotate the reward
field in HabitRetryRequest with `@NotNull`(message = "HABIT_REWARD_REQUIRED") and
`@NotBlank` (matching HabitRewardUpdateRequest semantics), and then update
HabitService.retryFailedHabit to be user-type-aware (use the same logic as in
HabitService for HabitCreateRequest/HabitUpdateRequest): for CHILD users require
a non-blank reward (throw/return a validation error with HABIT_REWARD_REQUIRED
if missing), and for PARENT users ignore any supplied reward (do not overwrite
Habit.reward in Habit.retry; leave it null or existing value as appropriate).
Reference: HabitRetryRequest, HabitRewardUpdateRequest,
HabitService.retryFailedHabit, Habit.retry, HabitCreateRequest,
HabitUpdateRequest, Habit.java.

11 changes: 10 additions & 1 deletion src/main/java/com/swyp/server/domain/habit/entity/Habit.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import com.swyp.server.domain.user.entity.User;
import com.swyp.server.domain.user.entity.UserType;
import com.swyp.server.global.SoftDeletableEntity;
import com.swyp.server.global.exception.CustomException;
import com.swyp.server.global.exception.ErrorCode;
import jakarta.persistence.*;
import java.time.LocalDateTime;
import java.time.ZoneId;
Expand Down Expand Up @@ -89,7 +91,14 @@ public void updateRewardStatus(RewardStatus rewardStatus) {
this.status = rewardStatus;
}

public void retry(User user) {
public void retry(User user, HabitDuration duration, String reward) {
if (user.getUserType() == UserType.CHILD) {
if (reward == null || reward.isBlank())
throw new CustomException(ErrorCode.HABIT_REWARD_REQUIRED);
}

this.duration = duration;
this.reward = reward;
this.status =
user.getUserType() == UserType.CHILD
? RewardStatus.REWARD_CHECKING
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,15 @@ List<Habit> findAllByUserIdsAndStatusOptional(

@Modifying(clearAutomatically = true)
@Query(
"UPDATE Habit h SET h.status = 'FAIL' "
"UPDATE Habit h SET h.status = 'FAIL', h.isCompleted = false "
+ " WHERE h.isCompleted = false "
+ " AND h.status IN ('REWARD_CHECKING', 'IN_PROGRESS')"
+ " AND h.duration IN ('THREE_DAYS', 'SEVEN_DAYS')")
void updateImmediateFailureHabits();

@Modifying(clearAutomatically = true)
@Query(
"UPDATE Habit h SET h.status = 'FAIL' "
"UPDATE Habit h SET h.status = 'FAIL', h.isCompleted = false "
+ " WHERE h.failCount > 1 "
+ " AND h.status IN ('REWARD_CHECKING', 'IN_PROGRESS')"
+ " AND h.duration NOT IN ('THREE_DAYS', 'SEVEN_DAYS')")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@ public HabitRewardListResponse getHabitRewards(Long userId, RewardStatus status)

List<RewardStatus> searchStatuses = determineSearchStatuses(user.getUserType(), status);

if(targetUserIds.isEmpty() || searchStatuses.isEmpty()) return HabitRewardListResponse.empty();
if (targetUserIds.isEmpty() || searchStatuses.isEmpty())
return HabitRewardListResponse.empty();

List<Habit> habits =
habitRepository.findAllByUserIdsAndStatusOptional(targetUserIds, searchStatuses);
Expand Down Expand Up @@ -227,7 +228,7 @@ public void updateHabit(Long userId, Long habitId, HabitUpdateRequest request) {
}

@Transactional
public void retryFailedHabit(Long userId, Long habitId) {
public void retryFailedHabit(Long userId, Long habitId, HabitRetryRequest request) {
Habit habit =
habitRepository
.findByIdAndUserIdAndStatus(habitId, userId, RewardStatus.FAIL)
Expand All @@ -238,7 +239,7 @@ public void retryFailedHabit(Long userId, Long habitId) {
.findById(userId)
.orElseThrow(() -> new CustomException(ErrorCode.USER_NOT_FOUND));

habit.retry(user);
habit.retry(user, request.duration(), request.reward());
}

@Transactional
Expand Down Expand Up @@ -345,7 +346,11 @@ private List<RewardStatus> determineSearchStatuses(UserType userType, RewardStat
}

if (status == RewardStatus.ALL) {
return List.of(RewardStatus.REWARD_CHECKING, RewardStatus.IN_PROGRESS, RewardStatus.REWARD_WAITING, RewardStatus.COMPLETE);
return List.of(
RewardStatus.REWARD_CHECKING,
RewardStatus.IN_PROGRESS,
RewardStatus.REWARD_WAITING,
RewardStatus.COMPLETE);
}

if (userType == UserType.CHILD && status == RewardStatus.IN_PROGRESS) {
Expand Down
37 changes: 24 additions & 13 deletions src/test/java/com/swyp/server/domain/habit/HabitRepositoryTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
import com.swyp.server.domain.user.entity.User;
import com.swyp.server.domain.user.entity.UserType;
import com.swyp.server.domain.user.repository.UserRepository;
import java.sql.Timestamp;
import java.time.LocalDateTime;
import java.time.ZoneId;
import java.util.List;
import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
Expand All @@ -20,11 +24,6 @@
import org.springframework.test.util.ReflectionTestUtils;
import org.springframework.transaction.annotation.Transactional;

import java.sql.Timestamp;
import java.time.LocalDateTime;
import java.time.ZoneId;
import java.util.List;

@SpringBootTest
@Transactional
@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE)
Expand All @@ -40,7 +39,7 @@ public class HabitRepositoryTest {

@Test
@DisplayName("status가 'FAIL'인 습관은 보상 조회 시 조회되지 않아야 한다.")
void getHabitRewards(){
void getHabitRewards() {

User user =
User.builder()
Expand Down Expand Up @@ -95,17 +94,22 @@ void getHabitRewards(){

habitFailed.updateRewardStatus(RewardStatus.FAIL);

habitRepository.saveAll(
List.of(habitInProgress, habitRewardWaiting, habitCompleted, habitFailed));

habitRepository.saveAll(List.of(habitInProgress, habitRewardWaiting, habitCompleted, habitFailed));

HabitRewardListResponse habitRewardsStatusAll = habitService.getHabitRewards(user.getId(), RewardStatus.ALL);
HabitRewardListResponse habitRewardsFail = habitService.getHabitRewards(user.getId(), RewardStatus.FAIL);
HabitRewardListResponse habitRewardsStatusAll =
habitService.getHabitRewards(user.getId(), RewardStatus.ALL);
HabitRewardListResponse habitRewardsFail =
habitService.getHabitRewards(user.getId(), RewardStatus.FAIL);

Assertions.assertThat(habitRewardsStatusAll.habitRewards())
.hasSize(3)
.extracting("status")
.doesNotContain(RewardStatus.FAIL)
.containsExactlyInAnyOrder(RewardStatus.IN_PROGRESS, RewardStatus.REWARD_WAITING, RewardStatus.COMPLETE);
.containsExactlyInAnyOrder(
RewardStatus.IN_PROGRESS,
RewardStatus.REWARD_WAITING,
RewardStatus.COMPLETE);

Assertions.assertThat(habitRewardsFail.habitRewards()).isEmpty();
}
Expand Down Expand Up @@ -182,7 +186,8 @@ void updateHabitFailCount() {
}

@Test
@DisplayName("매일 자정 보상 확인중, 진행중 상태이며 수행 기간이 '3일', '7일'인 습관들은 당일 미실행 시 실패 상태가 되어야 한다.")
@DisplayName(
"매일 자정 보상 확인중, 진행중 상태이며 수행 기간이 '3일', '7일'인 습관들은 당일 미실행 시 실패 상태가 되어야 하며 일일 성공 여부가 false가 되어야 한다.")
void updateImmediateFailureHabits() {
User user =
User.builder()
Expand Down Expand Up @@ -228,12 +233,16 @@ void updateImmediateFailureHabits() {

Assertions.assertThat(threeDaysNotCompletedHabitInDB.getStatus())
.isEqualTo(RewardStatus.FAIL);
Assertions.assertThat(threeDaysNotCompletedHabitInDB.isCompleted()).isEqualTo(false);

Assertions.assertThat(sevenDaysNotCompletedHabitInDB.getStatus())
.isEqualTo(RewardStatus.FAIL);
Assertions.assertThat(sevenDaysNotCompletedHabitInDB.isCompleted()).isEqualTo(false);
}

@Test
@DisplayName("매일 자정 보상 확인중, 진행중 상태이며 수행 기간이 '3일', '7일' 외 습관들은 실패 횟수가 2회 이상이면 실패 상태가 되어야 한다.")
@DisplayName(
"매일 자정 보상 확인중, 진행중 상태이며 수행 기간이 '3일', '7일' 외 습관들은 실패 횟수가 2회 이상이면 실패 상태가 되어야 하며 일일 성공 여부가 false가 되어야 한다.")
void updateCumulativeFailureHabits() {
User user =
User.builder()
Expand Down Expand Up @@ -281,8 +290,10 @@ void updateCumulativeFailureHabits() {

Assertions.assertThat(fourteenDaysOneFailedHabitInDB.getStatus())
.isNotEqualTo(RewardStatus.FAIL);

Assertions.assertThat(fourteenDaysTwoFailedHabitInDB.getStatus())
.isEqualTo(RewardStatus.FAIL);
Assertions.assertThat(fourteenDaysTwoFailedHabitInDB.isCompleted()).isEqualTo(false);
}

@Test
Expand Down